Skip to content

Commit 19d0e8a

Browse files
HMAI-95 Add prison filters to prisoner endpoint (#558)
* If prisons filter present, only return prisoner if in matching prison. * Add tests for more of the error cases. * Add test for no conumer filters preserving backwards compatibility * Simplify if condition for verifying if match. * Extract function for whether a value should be filtered out. * Fix the integration test. * Add test for empty list of prisons has no access.
1 parent 906f25d commit 19d0e8a

File tree

14 files changed

+175
-25
lines changed

14 files changed

+175
-25
lines changed

src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/prison/PrisonController.kt

+4-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import jakarta.validation.ValidationException
1010
import org.springframework.beans.factory.annotation.Autowired
1111
import org.springframework.web.bind.annotation.GetMapping
1212
import org.springframework.web.bind.annotation.PathVariable
13+
import org.springframework.web.bind.annotation.RequestAttribute
1314
import org.springframework.web.bind.annotation.RequestMapping
1415
import org.springframework.web.bind.annotation.RequestParam
1516
import org.springframework.web.bind.annotation.RestController
@@ -19,6 +20,7 @@ import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Person
1920
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApi
2021
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApiError.Type.BAD_REQUEST
2122
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApiError.Type.ENTITY_NOT_FOUND
23+
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig.ConsumerFilters
2224
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.GetPersonService
2325
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.GetPrisonersService
2426
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.internal.AuditService
@@ -52,8 +54,9 @@ class PrisonController(
5254
)
5355
fun getPerson(
5456
@PathVariable hmppsId: String,
57+
@RequestAttribute filters: ConsumerFilters?,
5558
): DataResponse<Person?> {
56-
val response = getPersonService.getPrisoner(hmppsId)
59+
val response = getPersonService.getPrisoner(hmppsId, filters)
5760

5861
if (response.hasErrorCausedBy(BAD_REQUEST, causedBy = UpstreamApi.NOMIS)) {
5962
throw ValidationException("Invalid HMPPS ID: $hmppsId")

src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/extensions/ConsumerConfigConverter.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,5 @@ import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig.Consum
1010
@ConfigurationPropertiesBinding
1111
class ConsumerConfigConverter : Converter<String, ConsumerConfig> {
1212
// Specifically used in the case where there is a consumer config with no fields
13-
override fun convert(source: String): ConsumerConfig? = ConsumerConfig(include = emptyList(), filters = ConsumerFilters())
13+
override fun convert(source: String): ConsumerConfig? = ConsumerConfig(include = emptyList(), filters = ConsumerFilters(prisons = null))
1414
}

src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/extensions/ConsumerFilterConverter.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig.Consum
88
@Component
99
@ConfigurationPropertiesBinding
1010
class ConsumerFilterConverter : Converter<String, ConsumerFilters> {
11-
override fun convert(source: String): ConsumerFilters? = ConsumerFilters()
11+
override fun convert(source: String): ConsumerFilters? = ConsumerFilters(prisons = null)
1212
}
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,20 @@
11
package uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig
22

33
data class ConsumerFilters(
4-
val filters: Map<String, List<String>>? = emptyMap(),
5-
)
4+
val prisons: List<String>?,
5+
) {
6+
fun matchesPrison(prisonId: String?): Boolean = matchesFilterList(prisons, prisonId)
7+
8+
private fun matchesFilterList(
9+
filterList: List<String>?,
10+
value: String?,
11+
): Boolean {
12+
if (filterList == null) {
13+
return true
14+
}
15+
if (filterList.contains(value)) {
16+
return true
17+
}
18+
return false
19+
}
20+
}

src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPersonService.kt

+17-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Person
1010
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Response
1111
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApi
1212
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApiError
13+
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig.ConsumerFilters
1314

1415
@Service
1516
class GetPersonService(
@@ -109,7 +110,10 @@ class GetPersonService(
109110

110111
fun getPersonFromNomis(nomisNumber: String) = prisonerOffenderSearchGateway.getPrisonOffender(nomisNumber)
111112

112-
fun getPrisoner(hmppsId: String): Response<Person?> {
113+
fun getPrisoner(
114+
hmppsId: String,
115+
filters: ConsumerFilters?,
116+
): Response<Person?> {
113117
val prisonerNomisNumber = getNomisNumber(hmppsId)
114118

115119
if (prisonerNomisNumber.errors.isNotEmpty()) {
@@ -144,8 +148,19 @@ class GetPersonService(
144148
)
145149
}
146150

151+
val posPrisoner = prisonResponse.data
152+
153+
if (
154+
filters != null && !filters.matchesPrison(posPrisoner?.prisonId)
155+
) {
156+
return Response(
157+
data = null,
158+
errors = listOf(UpstreamApiError(UpstreamApi.PRISONER_OFFENDER_SEARCH, UpstreamApiError.Type.ENTITY_NOT_FOUND, "Not found")),
159+
)
160+
}
161+
147162
return Response(
148-
data = prisonResponse.data?.toPerson(),
163+
data = posPrisoner?.toPerson(),
149164
errors = prisonResponse.errors,
150165
)
151166
}

src/main/resources/application-integration-test.yml

+11
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,14 @@ authorisation:
9393
include:
9494
- "/.*"
9595
filters:
96+
limited-prisons:
97+
include:
98+
- "/v1/prison/prisoners/[^/]*$"
99+
filters:
100+
prisons:
101+
- ABC
102+
no-prisons:
103+
include:
104+
- "/v1/prison/prisoners/[^/]*$"
105+
filters:
106+
prisons:

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/prison/PrisonControllerTest.kt

+8-6
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package uk.gov.justice.digital.hmpps.hmppsintegrationapi.controllers.v1.prison
33
import io.kotest.core.spec.style.DescribeSpec
44
import io.kotest.matchers.shouldBe
55
import org.mockito.Mockito
6+
import org.mockito.kotlin.anyOrNull
7+
import org.mockito.kotlin.eq
68
import org.mockito.kotlin.times
79
import org.mockito.kotlin.verify
810
import org.mockito.kotlin.whenever
@@ -51,15 +53,15 @@ internal class PrisonControllerTest(
5153
}
5254

5355
it("returns 500 when service throws an exception") {
54-
whenever(getPersonService.getPrisoner(hmppsId)).thenThrow(RuntimeException("Service error"))
56+
whenever(getPersonService.getPrisoner(eq(hmppsId), anyOrNull())).thenThrow(RuntimeException("Service error"))
5557

5658
val result = mockMvc.performAuthorised("$basePath/prisoners/$hmppsId")
5759

5860
result.response.status.shouldBe(500)
5961
}
6062

6163
it("returns a person with all fields populated") {
62-
whenever(getPersonService.getPrisoner(hmppsId)).thenReturn(
64+
whenever(getPersonService.getPrisoner(eq(hmppsId), anyOrNull())).thenReturn(
6365
Response(
6466
data =
6567
Person(
@@ -110,7 +112,7 @@ internal class PrisonControllerTest(
110112
}
111113

112114
it("logs audit event") {
113-
whenever(getPersonService.getPrisoner(hmppsId)).thenReturn(
115+
whenever(getPersonService.getPrisoner(eq(hmppsId), anyOrNull())).thenReturn(
114116
Response(
115117
data =
116118
Person(
@@ -136,7 +138,7 @@ internal class PrisonControllerTest(
136138
}
137139

138140
it("returns 404 when prisoner is not found") {
139-
whenever(getPersonService.getPrisoner(hmppsId)).thenReturn(
141+
whenever(getPersonService.getPrisoner(eq(hmppsId), anyOrNull())).thenReturn(
140142
Response(
141143
data = null,
142144
errors =
@@ -156,7 +158,7 @@ internal class PrisonControllerTest(
156158
}
157159

158160
it("returns 404 when NOMIS number is not found") {
159-
whenever(getPersonService.getPrisoner(hmppsId)).thenReturn(
161+
whenever(getPersonService.getPrisoner(eq(hmppsId), anyOrNull())).thenReturn(
160162
Response(
161163
data = null,
162164
errors =
@@ -176,7 +178,7 @@ internal class PrisonControllerTest(
176178
}
177179

178180
it("returns 400 when HMPPS ID does not match format") {
179-
whenever(getPersonService.getPrisoner(hmppsId)).thenReturn(
181+
whenever(getPersonService.getPrisoner(eq(hmppsId), anyOrNull())).thenReturn(
180182
Response(
181183
data = null,
182184
errors =

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/extensions/AuthorisationFilterTest.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class AuthorisationFilterTest {
2727
val mockResponse = mock(HttpServletResponse::class.java)
2828
val mockChain = mock(FilterChain::class.java)
2929

30-
authorisationConfig.consumers = mapOf(exampleConsumer to ConsumerConfig(include = listOf(examplePath), filters = ConsumerFilters(emptyMap())))
30+
authorisationConfig.consumers = mapOf(exampleConsumer to ConsumerConfig(include = listOf(examplePath), filters = ConsumerFilters(prisons = null)))
3131

3232
// Act
3333
authorisationFilter.doFilter(mockRequest, mockResponse, mockChain)
@@ -44,7 +44,7 @@ class AuthorisationFilterTest {
4444
val mockResponse = mock(HttpServletResponse::class.java)
4545
val mockChain = mock(FilterChain::class.java)
4646

47-
authorisationConfig.consumers = mapOf(exampleConsumer to ConsumerConfig(include = emptyList(), filters = ConsumerFilters(emptyMap())))
47+
authorisationConfig.consumers = mapOf(exampleConsumer to ConsumerConfig(include = emptyList(), filters = ConsumerFilters(prisons = null)))
4848

4949
val authorisationFilter = AuthorisationFilter(authorisationConfig)
5050

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/extensions/ConsumerConfigConverterTest.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@ class ConsumerConfigConverterTest {
1313
val consumerConfig = ""
1414
val actual = ConsumerConfigConverter().convert(consumerConfig)
1515

16-
actual.shouldBe(ConsumerConfig(emptyList(), ConsumerFilters()))
16+
actual.shouldBe(ConsumerConfig(emptyList(), ConsumerFilters(prisons = null)))
1717
}
1818
}

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/extensions/ConsumerFilterConverterTest.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,6 @@ class ConsumerFilterConverterTest {
1212
val consumerConfig = ""
1313
val actual = ConsumerFilterConverter().convert(consumerConfig)
1414

15-
actual.shouldBe(ConsumerFilters())
15+
actual.shouldBe(ConsumerFilters(prisons = null))
1616
}
1717
}

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/extensions/FiltersExtractionFilterTest.kt

+3-3
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class FiltersExtractionFilterTest {
2525
val mockResponse = mock(HttpServletResponse::class.java)
2626
val mockChain = mock(FilterChain::class.java)
2727

28-
authorisationConfig.consumers = mapOf("consumer-name" to ConsumerConfig(include = null, filters = ConsumerFilters(emptyMap())))
28+
authorisationConfig.consumers = mapOf("consumer-name" to ConsumerConfig(include = null, filters = ConsumerFilters(prisons = null)))
2929

3030
// Act
3131
filtersExtractionFilter.doFilter(mockRequest, mockResponse, mockChain)
@@ -43,7 +43,7 @@ class FiltersExtractionFilterTest {
4343
val mockResponse = mock(HttpServletResponse::class.java)
4444
val mockChain = mock(FilterChain::class.java)
4545

46-
val expectedFilters = ConsumerFilters(mapOf("example-filter" to listOf("filter-1", "filter-2")))
46+
val expectedFilters = ConsumerFilters(prisons = listOf("filter-1", "filter-2"))
4747
authorisationConfig.consumers = mapOf("consumer-name" to ConsumerConfig(include = null, filters = expectedFilters))
4848

4949
// Act
@@ -62,7 +62,7 @@ class FiltersExtractionFilterTest {
6262
val mockResponse = mock(HttpServletResponse::class.java)
6363
val mockChain = mock(FilterChain::class.java)
6464

65-
val expectedFilters = ConsumerFilters(mapOf("example-filter" to listOf("filter-1", "filter-2")))
65+
val expectedFilters = ConsumerFilters(prisons = listOf("filter-1", "filter-2"))
6666
authorisationConfig.consumers = mapOf("consumer-name" to ConsumerConfig(include = null, filters = expectedFilters))
6767

6868
// Act

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/integration/prison/PrisonIntegrationTest.kt

+22
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package uk.gov.justice.digital.hmpps.hmppsintegrationapi.integration.prison
22

33
import org.junit.jupiter.api.Test
4+
import org.springframework.http.HttpHeaders
5+
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
46
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
57
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
68
import uk.gov.justice.digital.hmpps.hmppsintegrationapi.integration.IntegrationTestBase
@@ -19,6 +21,26 @@ class PrisonIntegrationTest : IntegrationTestBase() {
1921
.andExpect(content().json(getExpectedResponse("prisoner-response")))
2022
}
2123

24+
@Test
25+
fun `return a 404 for prisoner in wrong prison`() {
26+
val headers = HttpHeaders()
27+
headers.set("subject-distinguished-name", "C=GB,ST=London,L=London,O=Home Office,CN=limited-prisons")
28+
mockMvc.perform(
29+
get("$basePrisonPath/prisoners/$hmppsId").headers(headers),
30+
)
31+
.andExpect(status().isNotFound)
32+
}
33+
34+
@Test
35+
fun `return a 404 for if consumer has empty list of prisons`() {
36+
val headers = HttpHeaders()
37+
headers.set("subject-distinguished-name", "C=GB,ST=London,L=London,O=Home Office,CN=no-prisons")
38+
mockMvc.perform(
39+
get("$basePrisonPath/prisoners/$hmppsId").headers(headers),
40+
)
41+
.andExpect(status().isNotFound)
42+
}
43+
2244
@Test
2345
fun `return multiple prisoners when querying by complex parameters`() {
2446
callApi("$basePrisonPath/prisoners?first_name=$firstName&last_name=$lastName&dateOfBirth=$dateOfBirth")

0 commit comments

Comments
 (0)