diff --git a/README.md b/README.md index b01444d..d57e559 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ Implemented: - Audit events for shift creation, updates, cancellation, leave approval and leave cancellation - Before/after state snapshots stored as JSONB, with actor and timestamp - Audit writes participate in the same transaction as the business mutation -- Validation and API error handling +- Validation and API error handling, including PostgreSQL exclusion violations mapped to stable HTTP 409 responses - Unit and integration tests - GitHub Actions CI @@ -83,7 +83,6 @@ Migration `V4__audit_events.sql` adds the audit event table. Audit writes use th - leave and time-off - department constraints - schedule queries -- map PostgreSQL exclusion violations to HTTP 409 consistently for create and update paths - extend integration coverage to concurrent updates and leave/shift interactions ### Milestone 3 — Workforce Operations diff --git a/src/main/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandler.kt b/src/main/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandler.kt index 07e5eff..0af52ae 100644 --- a/src/main/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandler.kt +++ b/src/main/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandler.kt @@ -6,10 +6,14 @@ import dev.whrite.opsgrid.shift.EmployeeInactiveException import dev.whrite.opsgrid.shift.EmployeeNotFoundException import dev.whrite.opsgrid.shift.ShiftConflictException import dev.whrite.opsgrid.shift.ShiftNotFoundException +import org.hibernate.exception.ConstraintViolationException +import org.springframework.dao.DataIntegrityViolationException import org.springframework.http.HttpStatus +import org.springframework.http.ResponseEntity import org.springframework.web.bind.annotation.ExceptionHandler import org.springframework.web.bind.annotation.ResponseStatus import org.springframework.web.bind.annotation.RestControllerAdvice +import java.sql.SQLException data class ApiError(val code: String, val message: String) @@ -45,8 +49,39 @@ class ApiExceptionHandler { fun leaveConflict(ex: LeaveConflictException) = ApiError("LEAVE_CONFLICT", ex.message ?: "Leave conflict") + @ExceptionHandler(DataIntegrityViolationException::class) + fun dataIntegrityViolation(ex: DataIntegrityViolationException): ResponseEntity { + val causes = generateSequence(ex) { it.cause }.toList() + val sqlState = causes.filterIsInstance().firstOrNull()?.sqlState + val constraintName = causes + .filterIsInstance() + .firstOrNull() + ?.constraintName + + val error = when { + sqlState == "23P01" && constraintName == SHIFT_EXCLUSION_CONSTRAINT -> + ApiError("SHIFT_CONFLICT", "The shift overlaps an existing scheduled shift") + + sqlState == "23P01" && constraintName == LEAVE_EXCLUSION_CONSTRAINT -> + ApiError("LEAVE_CONFLICT", "The leave window overlaps existing approved leave") + + sqlState == "23P01" -> + ApiError("SCHEDULE_CONFLICT", "The requested time window conflicts with an existing schedule") + + else -> + ApiError("DATA_INTEGRITY_VIOLATION", "The request violates a data integrity constraint") + } + + return ResponseEntity.status(HttpStatus.CONFLICT).body(error) + } + @ExceptionHandler(IllegalArgumentException::class) @ResponseStatus(HttpStatus.BAD_REQUEST) fun badRequest(ex: IllegalArgumentException) = ApiError("INVALID_REQUEST", ex.message ?: "Invalid request") + + private companion object { + const val SHIFT_EXCLUSION_CONSTRAINT = "shifts_no_overlapping_employee_windows" + const val LEAVE_EXCLUSION_CONSTRAINT = "leave_no_overlapping_employee_windows" + } } diff --git a/src/test/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandlerTest.kt b/src/test/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandlerTest.kt new file mode 100644 index 0000000..3b7a3ea --- /dev/null +++ b/src/test/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandlerTest.kt @@ -0,0 +1,57 @@ +package dev.whrite.opsgrid.api + +import org.hibernate.exception.ConstraintViolationException +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test +import org.springframework.dao.DataIntegrityViolationException +import org.springframework.http.HttpStatus +import java.sql.SQLException + +class ApiExceptionHandlerTest { + private val handler = ApiExceptionHandler() + + @Test + fun mapsShiftExclusionViolationToShiftConflict() { + val response = handler.dataIntegrityViolation( + dataIntegrityFailure("23P01", "shifts_no_overlapping_employee_windows") + ) + + assertEquals(HttpStatus.CONFLICT, response.statusCode) + assertEquals("SHIFT_CONFLICT", response.body?.code) + assertEquals("The shift overlaps an existing scheduled shift", response.body?.message) + } + + @Test + fun mapsLeaveExclusionViolationToLeaveConflict() { + val response = handler.dataIntegrityViolation( + dataIntegrityFailure("23P01", "leave_no_overlapping_employee_windows") + ) + + assertEquals(HttpStatus.CONFLICT, response.statusCode) + assertEquals("LEAVE_CONFLICT", response.body?.code) + } + + @Test + fun doesNotExposeDatabaseDetailsForOtherIntegrityViolations() { + val response = handler.dataIntegrityViolation( + dataIntegrityFailure("23505", "employees_employee_number_key") + ) + + assertEquals(HttpStatus.CONFLICT, response.statusCode) + assertEquals("DATA_INTEGRITY_VIOLATION", response.body?.code) + assertEquals( + "The request violates a data integrity constraint", + response.body?.message + ) + } + + private fun dataIntegrityFailure(sqlState: String, constraint: String) = + DataIntegrityViolationException( + "database write failed", + ConstraintViolationException( + "constraint violation", + SQLException("database detail must not reach the client", sqlState), + constraint + ) + ) +}