Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ground work: prison controller and single prisoner details endpoint #532

Merged
merged 3 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Original file line number Diff line number Diff line change
@@ -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/{encodedHmppsId}")
wcdkj marked this conversation as resolved.
Show resolved Hide resolved
@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 encodedHmppsId: String,
): DataResponse<Person?> {
val decodedHmppsId = encodedHmppsId.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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<NomisNumber?> {
return when (identifyHmppsId(hmppsId)) {
fun getNomisNumber(hmppsId: String): Response<NomisNumber?> =
when (identifyHmppsId(hmppsId)) {
IdentifierType.NOMS -> Response(data = NomisNumber(hmppsId))

IdentifierType.CRN -> {
Expand Down Expand Up @@ -82,7 +82,6 @@ class GetPersonService(
),
)
}
}

fun getCombinedDataForPerson(hmppsId: String): Response<OffenderSearchResponse> {
val probationResponse = probationOffenderSearchGateway.getPerson(id = hmppsId)
Expand All @@ -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<Person?> {
val prisonerNomisNumber = getNomisNumber(hmppsId)
wcdkj marked this conversation as resolved.
Show resolved Hide resolved

if (prisonerNomisNumber.errors.isNotEmpty()) {
return Response(
data = null,
errors = prisonerNomisNumber.errors,
)
}

val nomisNumber = prisonerNomisNumber.data?.nomisNumber
if (nomisNumber == null) {
wcdkj marked this conversation as resolved.
Show resolved Hide resolved
return Response(
data = null,
errors = prisonerNomisNumber.errors,
)
}

val prisonResponse =
try {
getPersonFromNomis(nomisNumber)
} catch (e: RuntimeException) {
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
1 change: 1 addition & 0 deletions src/main/resources/application-dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/[^/]*$"
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/application-integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ authorisation:
- "/health/readiness"
- "/health/liveness"
- "/info"
- "/v1/prison/prisoners/[^/]*$"
config-test:
- "/v1/config/authorisation"
all-access:
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/application-local-docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ authorisation:
- "/health/readiness"
- "/health/liveness"
- "/info"
- "/v1/prison/prisoners/[^/]*$"
config-test:
- "/v1/config/authorisation"
all-access:
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/application-local.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ authorisation:
- "/health/liveness"
- "/info"
- "/v1/hmpps/reference-data"
- "/v1/prison/prisoners/[^/]*$"
config-test:
- "/v1/config/authorisation"
all-access:
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/application-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,6 @@ authorisation:
- "/health/liveness"
- "/info"
- "/v1/hmpps/reference-data"
- "/v1/prison/prisoners/[^/]*$"
config-test:
- "/v1/config/authorisation"
Original file line number Diff line number Diff line change
@@ -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)
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Original file line number Diff line number Diff line change
@@ -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")))
}
}
Loading
Loading