diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/person/PersonController.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/person/PersonController.kt index 3be63eb6..55a3c7ff 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/person/PersonController.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/person/PersonController.kt @@ -157,12 +157,11 @@ class PersonController( return DataResponse(response.data) } - private fun isValidISODateFormat(dateString: String): Boolean { - return try { + private fun isValidISODateFormat(dateString: String): Boolean = + try { LocalDate.parse(dateString, DateTimeFormatter.ISO_DATE) true } catch (e: Exception) { false } - } } diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/prison/PrisonController.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/prison/PrisonController.kt new file mode 100644 index 00000000..562f010d --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/prison/PrisonController.kt @@ -0,0 +1,54 @@ +package uk.gov.justice.digital.hmpps.hmppsintegrationapi.controllers.v1.prison + +import io.swagger.v3.oas.annotations.Operation +import io.swagger.v3.oas.annotations.Parameter +import io.swagger.v3.oas.annotations.media.Content +import io.swagger.v3.oas.annotations.media.Schema +import io.swagger.v3.oas.annotations.responses.ApiResponse +import io.swagger.v3.oas.annotations.tags.Tag +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.web.bind.annotation.GetMapping +import org.springframework.web.bind.annotation.PathVariable +import org.springframework.web.bind.annotation.RequestMapping +import org.springframework.web.bind.annotation.RestController +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.exception.EntityNotFoundException +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.extensions.decodeUrlCharacters +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.DataResponse +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Person +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApi +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApiError.Type.ENTITY_NOT_FOUND +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.GetPersonService +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.internal.AuditService + +@RestController +@RequestMapping("/v1/prison") +@Tag(name = "prison") +class PrisonController( + @Autowired val getPersonService: GetPersonService, + @Autowired val auditService: AuditService, +) { + @GetMapping("/prisoners/{hmppsId}") + @Operation( + summary = "Returns a single prisoners details given an hmppsId, does not query for a probation person.", + responses = [ + ApiResponse(responseCode = "200", useReturnTypeSchema = true, description = "Successfully found a prisoner with the provided HMPPS ID."), + ApiResponse(responseCode = "404", content = [Content(schema = Schema(ref = "#/components/schemas/PersonNotFound"))]), + ApiResponse(responseCode = "500", content = [Content(schema = Schema(ref = "#/components/schemas/InternalServerError"))]), + ], + ) + fun getPerson( + @Parameter(description = "A HMPPS identifier", example = "2008%2F0545166T", required = true) @PathVariable hmppsId: String, + ): DataResponse { + val decodedHmppsId = hmppsId.decodeUrlCharacters() + + val response = getPersonService.getPrisoner(decodedHmppsId) + + if (response.hasErrorCausedBy(ENTITY_NOT_FOUND, causedBy = UpstreamApi.PROBATION_OFFENDER_SEARCH)) { + throw EntityNotFoundException("Could not find person with hmppsId: $decodedHmppsId") + } + + auditService.createEvent("GET_PERSON_DETAILS", mapOf("hmppsId" to decodedHmppsId)) + val data = response.data + return DataResponse(data) + } +} diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPersonService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPersonService.kt index a140d9e3..4cdbbe4d 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPersonService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPersonService.kt @@ -44,8 +44,8 @@ class GetPersonService( * When it is a noms number then return it. * When it is a CRN look up the prisoner in probation offender search and then return it */ - fun getNomisNumber(hmppsId: String): Response { - return when (identifyHmppsId(hmppsId)) { + fun getNomisNumber(hmppsId: String): Response = + when (identifyHmppsId(hmppsId)) { IdentifierType.NOMS -> Response(data = NomisNumber(hmppsId)) IdentifierType.CRN -> { @@ -82,7 +82,6 @@ class GetPersonService( ), ) } - } fun getCombinedDataForPerson(hmppsId: String): Response { val probationResponse = probationOffenderSearchGateway.getPerson(id = hmppsId) @@ -105,8 +104,47 @@ class GetPersonService( } fun getPersonFromNomis(nomisNumber: String) = prisonerOffenderSearchGateway.getPrisonOffender(nomisNumber) -} -fun isNomsNumber(id: String?): Boolean { - return id?.matches(Regex("^[A-Z]\\d{4}[A-Z]{2}+$")) == true + fun getPrisoner(hmppsId: String): Response { + val prisonerNomisNumber = getNomisNumber(hmppsId) + + if (prisonerNomisNumber.errors.isNotEmpty()) { + return Response( + data = null, + errors = prisonerNomisNumber.errors, + ) + } + + val nomisNumber = prisonerNomisNumber.data?.nomisNumber + + val prisonResponse = + try { + getPersonFromNomis(nomisNumber!!) + } catch (e: RuntimeException) { + if (nomisNumber == null) { + return Response( + data = null, + errors = prisonerNomisNumber.errors, + ) + } + return Response( + data = null, + errors = listOf(UpstreamApiError(description = e.message ?: "Service error", type = UpstreamApiError.Type.INTERNAL_SERVER_ERROR, causedBy = UpstreamApi.PRISONER_OFFENDER_SEARCH)), + ) + } + + if (prisonResponse.errors.isNotEmpty()) { + return Response( + data = null, + errors = prisonResponse.errors, + ) + } + + return Response( + data = prisonResponse.data?.toPerson(), + errors = prisonResponse.errors, + ) + } } + +fun isNomsNumber(id: String?): Boolean = id?.matches(Regex("^[A-Z]\\d{4}[A-Z]{2}+$")) == true diff --git a/src/main/resources/application-dev.yml b/src/main/resources/application-dev.yml index f3d1171b..25fafa23 100644 --- a/src/main/resources/application-dev.yml +++ b/src/main/resources/application-dev.yml @@ -108,6 +108,7 @@ authorisation: - "/v1/persons/.*/sentences/latest-key-dates-and-adjustments" - "/v1/persons/.*/status-information" - "/v1/persons/[^/]*$" + - "/v1/prison/prisoners/[^/]*$" kilco: - "/v1/persons" - "/v1/persons/[^/]*$" diff --git a/src/main/resources/application-integration-test.yml b/src/main/resources/application-integration-test.yml index 0a090091..c0558da7 100644 --- a/src/main/resources/application-integration-test.yml +++ b/src/main/resources/application-integration-test.yml @@ -79,6 +79,7 @@ authorisation: - "/health/readiness" - "/health/liveness" - "/info" + - "/v1/prison/prisoners/[^/]*$" config-test: - "/v1/config/authorisation" all-access: diff --git a/src/main/resources/application-local-docker.yml b/src/main/resources/application-local-docker.yml index 6bad83c5..628970b7 100644 --- a/src/main/resources/application-local-docker.yml +++ b/src/main/resources/application-local-docker.yml @@ -53,6 +53,7 @@ authorisation: - "/health/readiness" - "/health/liveness" - "/info" + - "/v1/prison/prisoners/[^/]*$" config-test: - "/v1/config/authorisation" all-access: diff --git a/src/main/resources/application-local.yml b/src/main/resources/application-local.yml index 74b55df3..6b3632a9 100644 --- a/src/main/resources/application-local.yml +++ b/src/main/resources/application-local.yml @@ -61,6 +61,7 @@ authorisation: - "/health/liveness" - "/info" - "/v1/hmpps/reference-data" + - "/v1/prison/prisoners/[^/]*$" config-test: - "/v1/config/authorisation" all-access: diff --git a/src/main/resources/application-test.yml b/src/main/resources/application-test.yml index 314afd35..4c577651 100644 --- a/src/main/resources/application-test.yml +++ b/src/main/resources/application-test.yml @@ -81,5 +81,6 @@ authorisation: - "/health/liveness" - "/info" - "/v1/hmpps/reference-data" + - "/v1/prison/prisoners/[^/]*$" config-test: - "/v1/config/authorisation" diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/prison/PrisonControllerTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/prison/PrisonControllerTest.kt new file mode 100644 index 00000000..eee6617e --- /dev/null +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/prison/PrisonControllerTest.kt @@ -0,0 +1,140 @@ +package uk.gov.justice.digital.hmpps.hmppsintegrationapi.controllers.v1.prison + +import io.kotest.core.spec.style.DescribeSpec +import io.kotest.matchers.shouldBe +import org.mockito.Mockito +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest +import org.springframework.test.context.ActiveProfiles +import org.springframework.test.context.bean.override.mockito.MockitoBean +import org.springframework.test.web.servlet.MockMvc +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.extensions.removeWhitespaceAndNewlines +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.helpers.IntegrationAPIMockMvc +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Person +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Response +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApi +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApiError +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApiError.Type.ENTITY_NOT_FOUND +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.GetPersonService +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.internal.AuditService +import java.time.LocalDate + +@WebMvcTest(controllers = [PrisonController::class]) +@ActiveProfiles("test") +internal class PrisonControllerTest( + @Autowired var springMockMvc: MockMvc, + @MockitoBean val getPersonService: GetPersonService, + @MockitoBean val auditService: AuditService, +) : DescribeSpec({ + val hmppsId = "200313116M" + val basePath = "/v1/prison" + val mockMvc = IntegrationAPIMockMvc(springMockMvc) + + describe("GET $basePath") { + } + + afterTest { + Mockito.reset(getPersonService) + Mockito.reset(auditService) + } + + it("returns 500 when service throws an exception") { + whenever(getPersonService.getPrisoner(hmppsId)).thenThrow(RuntimeException("Service error")) + + val result = mockMvc.performAuthorised("$basePath/prisoners/$hmppsId") + + result.response.status.shouldBe(500) + } + + it("returns a person with all fields populated") { + whenever(getPersonService.getPrisoner(hmppsId)).thenReturn( + Response( + data = + Person( + firstName = "Barry", + lastName = "Allen", + middleName = "Jonas", + dateOfBirth = LocalDate.parse("2023-03-01"), + gender = "Male", + ethnicity = "Caucasian", + pncId = "PNC123456", + ), + ), + ) + + val result = mockMvc.performAuthorised("$basePath/prisoners/$hmppsId") + + result.response.contentAsString.shouldBe( + """ + { + "data":{ + "firstName":"Barry", + "lastName":"Allen", + "middleName":"Jonas", + "dateOfBirth":"2023-03-01", + "gender":"Male", + "ethnicity":"Caucasian", + "aliases":[], + "identifiers":{ + "nomisNumber":null, + "croNumber":null, + "deliusCrn":null + }, + "pncId": "PNC123456", + "hmppsId": null, + "contactDetails": null + } + } + """.removeWhitespaceAndNewlines(), + ) + } + + it("logs audit event") { + whenever(getPersonService.getPrisoner(hmppsId)).thenReturn( + Response( + data = + Person( + firstName = "Barry", + lastName = "Allen", + middleName = "Jonas", + dateOfBirth = LocalDate.parse("2023-03-01"), + gender = "Male", + ethnicity = "Caucasian", + pncId = "PNC123456", + ), + ), + ) + + mockMvc.performAuthorised("$basePath/prisoners/$hmppsId") + verify( + auditService, + times(1), + ).createEvent( + "GET_PERSON_DETAILS", + mapOf("hmppsId" to hmppsId), + ) + } + + it("returns 404 when prisoner is not found") { + whenever(getPersonService.getPrisoner(hmppsId)).thenReturn( + Response( + data = null, + errors = + listOf( + UpstreamApiError( + type = ENTITY_NOT_FOUND, + causedBy = UpstreamApi.PROBATION_OFFENDER_SEARCH, + description = "Prisoner not found", + ), + ), + ), + ) + + val result = mockMvc.performAuthorised("$basePath/prisoners/$hmppsId") + + result.response.status.shouldBe(404) + } + }) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/helpers/IntegrationAPIMockMvc.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/helpers/IntegrationAPIMockMvc.kt index 361cdbf1..70f07d8d 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/helpers/IntegrationAPIMockMvc.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/helpers/IntegrationAPIMockMvc.kt @@ -23,7 +23,5 @@ class IntegrationAPIMockMvc( return mockMvc.perform(MockMvcRequestBuilders.get(path).header("subject-distinguished-name", subjectDistinguishedName)).andReturn() } - fun performUnAuthorised(path: String): MvcResult { - return mockMvc.perform(MockMvcRequestBuilders.get(path)).andReturn() - } + fun performUnAuthorised(path: String): MvcResult = mockMvc.perform(MockMvcRequestBuilders.get(path)).andReturn() } diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/integration/prison/PrisonIntegrationTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/integration/prison/PrisonIntegrationTest.kt new file mode 100644 index 00000000..ea511fbc --- /dev/null +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/integration/prison/PrisonIntegrationTest.kt @@ -0,0 +1,18 @@ +package uk.gov.justice.digital.hmpps.hmppsintegrationapi.integration.prison + +import org.junit.jupiter.api.Test +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.integration.IntegrationTestBase + +class PrisonIntegrationTest : IntegrationTestBase() { + private final val hmppsId = "G2996UX" + private final val basePrisonPath = "/v1/prison" + + @Test + fun `return a prisoner with all fields populated`() { + callApi("$basePrisonPath/prisoners/$hmppsId") + .andExpect(status().isOk) + .andExpect(content().json(getExpectedResponse("prisoner-response"))) + } +} diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPersonServiceTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPersonServiceTest.kt index 0b72ad97..ca9468c8 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPersonServiceTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPersonServiceTest.kt @@ -1,23 +1,20 @@ +@file:Suppress("ktlint:standard:no-wildcard-imports") + package uk.gov.justice.digital.hmpps.hmppsintegrationapi.services import io.kotest.core.spec.style.DescribeSpec import io.kotest.matchers.shouldBe +import io.kotest.matchers.types.shouldBeTypeOf import org.mockito.Mockito import org.mockito.internal.verification.VerificationModeFactory import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.springframework.boot.test.context.ConfigDataApplicationContextInitializer -import org.springframework.boot.test.mock.mockito.MockBean import org.springframework.test.context.ContextConfiguration +import org.springframework.test.context.bean.override.mockito.MockitoBean import uk.gov.justice.digital.hmpps.hmppsintegrationapi.gateways.PrisonerOffenderSearchGateway import uk.gov.justice.digital.hmpps.hmppsintegrationapi.gateways.ProbationOffenderSearchGateway -import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Identifiers -import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.OffenderSearchResponse -import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Person -import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.PersonOnProbation -import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Response -import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApi -import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApiError +import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.* import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.prisoneroffendersearch.POSPrisoner @ContextConfiguration( @@ -25,8 +22,8 @@ import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.prisoneroffenders classes = [GetPersonService::class], ) internal class GetPersonServiceTest( - @MockBean val prisonerOffenderSearchGateway: PrisonerOffenderSearchGateway, - @MockBean val probationOffenderSearchGateway: ProbationOffenderSearchGateway, + @MockitoBean val prisonerOffenderSearchGateway: PrisonerOffenderSearchGateway, + @MockitoBean val probationOffenderSearchGateway: ProbationOffenderSearchGateway, private val getPersonService: GetPersonService, ) : DescribeSpec({ val hmppsId = "2003/13116M" @@ -99,8 +96,70 @@ internal class GetPersonServiceTest( val result = getPersonService.getCombinedDataForPerson(hmppsId) result.data shouldBe OffenderSearchResponse(prisonerOffenderSearch = null, probationOffenderSearch = personFromProbationOffenderSearch) - result.errors.first().causedBy.shouldBe(UpstreamApi.PRISONER_OFFENDER_SEARCH) - result.errors.first().type.shouldBe(UpstreamApiError.Type.ENTITY_NOT_FOUND) - result.errors.first().description.shouldBe("MockError") + result.errors + .first() + .causedBy + .shouldBe(UpstreamApi.PRISONER_OFFENDER_SEARCH) + result.errors + .first() + .type + .shouldBe(UpstreamApiError.Type.ENTITY_NOT_FOUND) + result.errors + .first() + .description + .shouldBe("MockError") + } + + it("returns a prisoner when valid hmppsId is provided") { + val validHmppsId = "G2996UX" + val person = Person(firstName = "Sam", lastName = "Mills") + whenever(prisonerOffenderSearchGateway.getPrisonOffender(nomsNumber = "G2996UX")).thenReturn( + Response(data = POSPrisoner(firstName = "Sam", lastName = "Mills")), + ) + + val result = getPersonService.getPrisoner(validHmppsId) + + result.data.shouldBeTypeOf() + result.data!!.firstName.shouldBe(person.firstName) + result.data!!.lastName.shouldBe(person.lastName) + result.errors.shouldBe(emptyList()) + } + + it("returns null when prisoner is not found") { + val zeroHitHmppsId = "G2996UX" + val prisonResponse: Response = Response(data = null, errors = listOf(UpstreamApiError(UpstreamApi.PRISONER_OFFENDER_SEARCH, UpstreamApiError.Type.ENTITY_NOT_FOUND, "Not found"))) + + whenever(prisonerOffenderSearchGateway.getPrisonOffender("G2996UX")).thenReturn(Response(data = null, errors = listOf(UpstreamApiError(UpstreamApi.PRISONER_OFFENDER_SEARCH, UpstreamApiError.Type.ENTITY_NOT_FOUND, "Not found")))) + val result = getPersonService.getPrisoner(zeroHitHmppsId) + + result.data.shouldBe(null) + result.errors.shouldBe(prisonResponse.errors) + } + + it("returns error when invalid hmppsId is provided") { + val invalidHmppsId = "invalid_id" + val expectedError = UpstreamApiError(UpstreamApi.NOMIS, UpstreamApiError.Type.BAD_REQUEST, "Invalid HMPPS ID: $invalidHmppsId") + val result = getPersonService.getPrisoner(invalidHmppsId) + + result.data.shouldBe(null) + result.errors.shouldBe(listOf(expectedError)) + } + + it("returns error when nomis number is not found") { + val hmppsIdInCrnFormat = "AB123123" + val expectedError = + listOf( + UpstreamApiError(UpstreamApi.NOMIS, UpstreamApiError.Type.ENTITY_NOT_FOUND, "NOMIS number not found"), + UpstreamApiError(causedBy = UpstreamApi.PROBATION_OFFENDER_SEARCH, type = UpstreamApiError.Type.ENTITY_NOT_FOUND, description = "NOMIS number not found"), + ) + + whenever(probationOffenderSearchGateway.getPerson(id = hmppsIdInCrnFormat)).thenReturn( + Response(data = null, errors = listOf(UpstreamApiError(UpstreamApi.NOMIS, UpstreamApiError.Type.ENTITY_NOT_FOUND, "NOMIS number not found"))), + ) + + val result = getPersonService.getPrisoner(hmppsIdInCrnFormat) + + result.data.shouldBe(null) + result.errors.shouldBe(expectedError) } }) diff --git a/src/test/resources/expected-responses/prisoner-response b/src/test/resources/expected-responses/prisoner-response new file mode 100644 index 00000000..86ebf147 --- /dev/null +++ b/src/test/resources/expected-responses/prisoner-response @@ -0,0 +1 @@ +{"data":{"firstName":"Robert","lastName":"Larsen","middleName":"John James","dateOfBirth":"1975-04-02","gender":"Female","ethnicity":"White: Eng./Welsh/Scot./N.Irish/British","aliases":[{"firstName":"Robert","lastName":"Lorsen","middleName":"Trevor","dateOfBirth":"1975-04-02","gender":"Male","ethnicity":"White : Irish"}],"identifiers":{"nomisNumber":"A1234AA","croNumber":"29906/12J","deliusCrn":null},"pncId":"12/394773H","hmppsId":null,"contactDetails":null}}