diff --git a/docs/PORTFOLIO_COMPLETENESS.md b/docs/PORTFOLIO_COMPLETENESS.md index f0b71cb..f807543 100644 --- a/docs/PORTFOLIO_COMPLETENESS.md +++ b/docs/PORTFOLIO_COMPLETENESS.md @@ -1,6 +1,6 @@ # Portfolio completeness -Reviewed on 2026-10-15 against `main` at `6679d359adf5958d98e55f5f319a649d3ccdf235`. +Reviewed on 2026-10-17 against the validated code revision `e26ccc2812e712ebff17b947314aed8a8f25be5a` on the integrity-classification pull request. **Maturity:** Available (Unhardened) for the API and database scope listed here. This describes the implemented portfolio contract. It is not a production deployment or security certification. @@ -21,9 +21,9 @@ The supported statuses are `NEW`, `CONTACTED`, `QUALIFIED`, `CLOSED`, and `REJEC ## Evidence for the inspected revision -The latest recorded hosted run for the inspected `main` revision is [CI run 36242743644](https://github.com/bconnell/lead-intake-api/actions/runs/36242743644). +The hosted validation of the inspected code revision is [CI run 39081372196](https://github.com/bconnell/lead-intake-api/actions/runs/39081372196). -- `clean verify` completed with 39 application tests and 3 PostgreSQL Testcontainers integration tests. There were no failures, errors, or skipped tests. +- `clean verify` completed with 40 application tests and 4 PostgreSQL Testcontainers integration tests. There were no failures, errors, or skipped tests. - Spring Boot packaging completed successfully. - The Compose PostgreSQL service became healthy, passed the configured `pg_isready` check, and was removed with its container, network, and volume. - The run checked out the exact commit recorded above. Its results apply to that revision. @@ -34,17 +34,19 @@ The latest recorded hosted run for the inspected `main` revision is [CI run 3624 |---|---|---| | API behavior | Available (Unhardened) | HTTP tests cover validation, creation, retrieval, pagination, filtering, status updates, deletion, statistics, and safe error responses | | Persistence and recovery | Available (Unhardened) | Flyway owns two migrations; Hibernate validates the schema; PostgreSQL integration tests exercise persistence and constraints | -| Failure handling | In Progress | Expected not-found and duplicate-email responses are covered. One constraint-classification defect is listed below | +| Failure handling | Available (Unhardened) | HTTP tests cover safe not-found, duplicate email, and unrelated database integrity failure responses against H2 and PostgreSQL; full operational monitoring and recovery remain outside scope | | Performance and resource use | Available (Unhardened) | Database-backed paging, a maximum page size of 100, and count queries avoid returning whole tables; no high-volume performance budget has been measured | | Packaging and reproducibility | Available (Unhardened) | The Maven Wrapper and fresh Spring Boot packaging are exercised in hosted CI | | Accessibility and visual quality | Not applicable | This API has no visual interface | | Production operations | Not in scope | No deployment, authentication, monitoring, backup, or operational readiness claim is made | -## Remaining engineering finding +## Database integrity error classification -`ApiExceptionHandler.handleConflict` currently converts every `DataIntegrityViolationException` into a `409 Conflict` saying the email already exists. The schema also has required-field and status-check constraints. If one of those constraints fails because of a future code defect or an unexpected persistence condition, the response will incorrectly blame the user's email. +The API distinguishes the named `uk_leads_email` uniqueness violation from other database integrity failures. Only that recognized constraint maps to `409 Conflict` with the existing duplicate-email response. An unrelated or unrecognized violation maps to the same generic `500 Internal Server Error` response used for unexpected application failures, with no constraint or driver details in the HTTP body. -The next code loop should distinguish the named email uniqueness constraint from other integrity failures, retain the current duplicate-email `409`, return a generic safe server error for unexpected constraint failures, and add regression coverage against both the H2 application path and PostgreSQL Testcontainers path. Do not expose database constraint names or driver messages in HTTP responses. +Regression coverage includes end-to-end duplicate-email checks and HTTP-boundary checks using actual status-check failures from H2 and PostgreSQL. The hosted run above exercised the PostgreSQL cases through Testcontainers. + +**Validation status:** Hosted `clean verify` passed for the reviewed code revision, with no skipped tests, and the PostgreSQL Compose readiness and cleanup stages succeeded. Evidence is scoped to that commit; later code changes require fresh validation. This does not establish production deployment or security readiness. ## Explicit scope limits diff --git a/src/main/java/com/leadintake/api/common/error/ApiExceptionHandler.java b/src/main/java/com/leadintake/api/common/error/ApiExceptionHandler.java index c741a28..a98b4ed 100644 --- a/src/main/java/com/leadintake/api/common/error/ApiExceptionHandler.java +++ b/src/main/java/com/leadintake/api/common/error/ApiExceptionHandler.java @@ -1,6 +1,7 @@ package com.leadintake.api.common.error; import com.leadintake.api.lead.service.LeadNotFoundException; +import org.hibernate.exception.ConstraintViolationException; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; @@ -13,9 +14,16 @@ import org.springframework.web.method.annotation.HandlerMethodValidationException; import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; +import java.util.regex.Pattern; + @RestControllerAdvice public class ApiExceptionHandler { + // PostgreSQL reports the explicit constraint name. H2 may expose the generated + // backing index name with a numeric suffix and additional index details. + private static final Pattern EMAIL_UNIQUE_CONSTRAINT = Pattern.compile( + "(?i)(?:^|\\.)uk_leads_email(?:_index_[0-9]+)?(?=\\s|$)"); + @ExceptionHandler(LeadNotFoundException.class) ProblemDetail handleNotFound() { ProblemDetail problem = ProblemDetail.forStatusAndDetail( @@ -62,12 +70,37 @@ ProblemDetail handleInvalidParameters() { } @ExceptionHandler(DataIntegrityViolationException.class) - ProblemDetail handleConflict() { - ProblemDetail problem = ProblemDetail.forStatusAndDetail( - HttpStatus.CONFLICT, - "A lead with this email already exists."); - problem.setTitle("Conflict"); - return problem; + ProblemDetail handleIntegrityViolation(DataIntegrityViolationException exception) { + if (isEmailUniquenessViolation(exception)) { + ProblemDetail problem = ProblemDetail.forStatusAndDetail( + HttpStatus.CONFLICT, + "A lead with this email already exists."); + problem.setTitle("Conflict"); + return problem; + } + + return unexpectedFailure(); + } + + private boolean isEmailUniquenessViolation(Throwable exception) { + Throwable cause = exception; + while (cause != null) { + if (cause instanceof ConstraintViolationException violation) { + String constraintName = violation.getConstraintName(); + if (constraintName != null + && EMAIL_UNIQUE_CONSTRAINT.matcher(constraintName.replace("\"", "")).find()) { + return true; + } + } + + Throwable next = cause.getCause(); + if (next == cause) { + break; + } + cause = next; + } + + return false; } @ExceptionHandler(Exception.class) @@ -83,6 +116,10 @@ ProblemDetail handleUnexpectedFailure(Exception exception) { return problem; } + return unexpectedFailure(); + } + + private ProblemDetail unexpectedFailure() { ProblemDetail problem = ProblemDetail.forStatusAndDetail( HttpStatus.INTERNAL_SERVER_ERROR, "An unexpected error occurred."); diff --git a/src/test/java/com/leadintake/api/lead/LeadApiTests.java b/src/test/java/com/leadintake/api/lead/LeadApiTests.java index a3040d8..9a8cb61 100644 --- a/src/test/java/com/leadintake/api/lead/LeadApiTests.java +++ b/src/test/java/com/leadintake/api/lead/LeadApiTests.java @@ -10,6 +10,7 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; import org.springframework.data.domain.Pageable; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.jdbc.core.JdbcTemplate; @@ -23,6 +24,7 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.hamcrest.Matchers.containsString; import static org.mockito.Mockito.any; import static org.mockito.Mockito.clearInvocations; @@ -403,6 +405,38 @@ void rejectsDuplicateNormalizedEmailWithConflict() throws Exception { .andExpect(jsonPath("$.detail").value("A lead with this email already exists.")); } + @Test + void returnsSafeServerErrorForNonEmailDatabaseConstraintViolation() throws Exception { + DataIntegrityViolationException constraintFailure = assertThrows( + DataIntegrityViolationException.class, + () -> jdbcTemplate.update( + "INSERT INTO leads (id, name, email, status, created_at, updated_at) " + + "VALUES (?, ?, ?, ?, ?, ?)", + UUID.randomUUID(), + "Invalid Check Constraint", + "invalid-check-" + UUID.randomUUID() + "@example.test", + "NOT_A_SUPPORTED_STATUS", + OffsetDateTime.now(), + OffsetDateTime.now())); + + // Replay a real H2 check-constraint failure across the HTTP persistence boundary. + // The test deliberately passes no invalid DTO fields to the controller. + doThrow(constraintFailure).when(leadRepository).save(any(LeadEntity.class)); + + mockMvc.perform(post("/api/leads") + .contentType(MediaType.APPLICATION_JSON) + .content(createRequest( + "Valid Lead", + "server-error-" + UUID.randomUUID() + "@example.test", + null))) + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.status").value(500)) + .andExpect(jsonPath("$.title").value("Internal Server Error")) + .andExpect(jsonPath("$.detail").value("An unexpected error occurred.")) + .andExpect(jsonPath("$.detail").value( + org.hamcrest.Matchers.not(containsString("ck_leads_status")))); + } + private String createRequest(String name, String email, String phone) { return """ { diff --git a/src/test/java/com/leadintake/api/lead/LeadPostgresIT.java b/src/test/java/com/leadintake/api/lead/LeadPostgresIT.java index ea9791c..c8f3e68 100644 --- a/src/test/java/com/leadintake/api/lead/LeadPostgresIT.java +++ b/src/test/java/com/leadintake/api/lead/LeadPostgresIT.java @@ -1,6 +1,7 @@ package com.leadintake.api.lead; import com.leadintake.api.lead.model.LeadStatus; +import com.leadintake.api.lead.model.LeadEntity; import com.leadintake.api.lead.repository.LeadRepository; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -8,15 +9,21 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; import org.springframework.context.annotation.Import; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.test.context.bean.override.mockito.MockitoSpyBean; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MvcResult; import java.util.UUID; +import java.time.OffsetDateTime; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; import static org.hamcrest.Matchers.startsWith; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -34,7 +41,7 @@ class LeadPostgresIT { @Autowired private MockMvc mockMvc; - @Autowired + @MockitoSpyBean private LeadRepository leadRepository; @Autowired @@ -121,6 +128,37 @@ void updatesPagesCountsAndDeletesLeadsThroughPostgresBackedHttpRoutes() throws E assertThat(leadRepository.findById(firstQualifiedId)).isEmpty(); } + + @Test + void preservesSafeProblemDetailsForPostgresCheckConstraintViolations() throws Exception { + DataIntegrityViolationException constraintFailure = assertThrows( + DataIntegrityViolationException.class, + () -> jdbcTemplate.update( + "INSERT INTO leads (id, name, email, status, created_at, updated_at) " + + "VALUES (?, ?, ?, ?, ?, ?)", + UUID.randomUUID(), + "Invalid Check Constraint", + "invalid-check-" + UUID.randomUUID() + "@example.test", + "NOT_A_SUPPORTED_STATUS", + OffsetDateTime.now(), + OffsetDateTime.now())); + + // Use the actual PostgreSQL integrity exception rather than an invented message. + doThrow(constraintFailure).when(leadRepository).save(any(LeadEntity.class)); + + mockMvc.perform(post("/api/leads") + .contentType(MediaType.APPLICATION_JSON) + .content(createRequest( + "Valid Postgres Lead", + "unexpected-" + UUID.randomUUID() + "@example.test"))) + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.status").value(500)) + .andExpect(jsonPath("$.title").value("Internal Server Error")) + .andExpect(jsonPath("$.detail").value("An unexpected error occurred.")); + + assertThat(leadRepository.count()).isZero(); + } + private UUID createLead(String email) throws Exception { MvcResult result = mockMvc.perform(post("/api/leads") .contentType(MediaType.APPLICATION_JSON)