Skip to content

Commit 234b785

Browse files
authored
SDIT-2021 Cater for retries to the mapping creation by performing an upsert (#369)
1 parent 9b199d3 commit 234b785

File tree

5 files changed

+97
-32
lines changed

5 files changed

+97
-32
lines changed

src/main/kotlin/uk/gov/justice/digital/hmpps/nomisvisitsmappingservice/prisonperson/PrisonPersonMigrationMapping.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ data class PrisonPersonMigrationMapping(
1212

1313
val migrationType: PrisonPersonMigrationType,
1414

15-
val dpsIds: String,
15+
var dpsIds: String,
1616

1717
val label: String,
1818

src/main/kotlin/uk/gov/justice/digital/hmpps/nomisvisitsmappingservice/prisonperson/PrisonPersonMigrationMappingRepository.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import org.springframework.stereotype.Repository
88
@Repository
99
interface PrisonPersonMigrationMappingRepository : CoroutineCrudRepository<PrisonPersonMigrationMapping, String> {
1010
suspend fun findByNomisPrisonerNumberAndMigrationType(nomisPrisonerNumber: String, migrationType: PrisonPersonMigrationType): PrisonPersonMigrationMapping?
11+
suspend fun findByNomisPrisonerNumberAndMigrationTypeAndLabel(nomisPrisonerNumber: String, migrationType: PrisonPersonMigrationType, label: String): PrisonPersonMigrationMapping?
1112
fun findAllByLabelOrderByNomisPrisonerNumberAsc(label: String, pageable: Pageable): Flow<PrisonPersonMigrationMapping>
1213
suspend fun countAllByLabel(label: String): Long
1314
}

src/main/kotlin/uk/gov/justice/digital/hmpps/nomisvisitsmappingservice/prisonperson/PrisonPersonMigrationService.kt

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,24 @@ package uk.gov.justice.digital.hmpps.nomisvisitsmappingservice.prisonperson
33
import kotlinx.coroutines.async
44
import kotlinx.coroutines.coroutineScope
55
import kotlinx.coroutines.flow.toList
6+
import kotlinx.coroutines.reactor.awaitSingle
67
import org.springframework.data.domain.Page
78
import org.springframework.data.domain.PageImpl
89
import org.springframework.data.domain.Pageable
10+
import org.springframework.data.r2dbc.core.R2dbcEntityTemplate
11+
import org.springframework.data.r2dbc.core.insert
12+
import org.springframework.data.r2dbc.core.update
13+
import org.springframework.data.relational.core.query.Criteria.where
14+
import org.springframework.data.relational.core.query.Query.query
15+
import org.springframework.data.relational.core.query.Update.update
916
import org.springframework.stereotype.Service
17+
import org.springframework.transaction.annotation.Transactional
1018

1119
@Service
1220
class PrisonPersonMigrationService(
1321
private val repository: PrisonPersonMigrationMappingRepository,
1422
private val prisonPersonMigrationMappingRepository: PrisonPersonMigrationMappingRepository,
23+
private val template: R2dbcEntityTemplate,
1524
) {
1625

1726
suspend fun create(mappingRequest: PrisonPersonMigrationMappingRequest) {
@@ -25,6 +34,41 @@ class PrisonPersonMigrationService(
2534
)
2635
}
2736

37+
@Transactional
38+
suspend fun upsert(mappingRequest: PrisonPersonMigrationMappingRequest): PrisonPersonMigrationMapping =
39+
mappingRequest.find()
40+
?.let { mappingRequest.update() }
41+
?: let { mappingRequest.insert() }
42+
43+
private suspend fun PrisonPersonMigrationMappingRequest.find() =
44+
repository.findByNomisPrisonerNumberAndMigrationTypeAndLabel(nomisPrisonerNumber, migrationType, label)
45+
46+
private suspend fun PrisonPersonMigrationMappingRequest.update(): PrisonPersonMigrationMapping =
47+
template.update<PrisonPersonMigrationMapping>()
48+
.inTable("prison_person_migration_mapping")
49+
.matching(
50+
query(
51+
where("nomis_prisoner_number").`is`(nomisPrisonerNumber)
52+
.and(where("migration_type").`is`(migrationType))
53+
.and(where("label").`is`(label)),
54+
),
55+
)
56+
.apply(update("dps_ids", dpsIds.toString()))
57+
.awaitSingle()
58+
.let { find()!! }
59+
60+
private suspend fun PrisonPersonMigrationMappingRequest.insert(): PrisonPersonMigrationMapping =
61+
template.insert<PrisonPersonMigrationMapping>()
62+
.using(
63+
PrisonPersonMigrationMapping(
64+
nomisPrisonerNumber = nomisPrisonerNumber,
65+
migrationType = migrationType,
66+
dpsIds = dpsIds.toString(),
67+
label = label,
68+
),
69+
)
70+
.awaitSingle()
71+
2872
suspend fun find(nomisPrisonerNumber: String, migrationType: PrisonPersonMigrationType) = repository.findByNomisPrisonerNumberAndMigrationType(nomisPrisonerNumber, migrationType)
2973

3074
suspend fun getMappings(

src/main/kotlin/uk/gov/justice/digital/hmpps/nomisvisitsmappingservice/prisonperson/PrisonPersonResource.kt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import org.springframework.validation.annotation.Validated
1616
import org.springframework.web.bind.annotation.GetMapping
1717
import org.springframework.web.bind.annotation.PathVariable
1818
import org.springframework.web.bind.annotation.PostMapping
19+
import org.springframework.web.bind.annotation.PutMapping
1920
import org.springframework.web.bind.annotation.RequestBody
2021
import org.springframework.web.bind.annotation.RequestMapping
2122
import org.springframework.web.bind.annotation.ResponseStatus
@@ -32,6 +33,7 @@ class PrisonPersonResource(private val service: PrisonPersonMigrationService) {
3233

3334
@PostMapping("/migration")
3435
@ResponseStatus(HttpStatus.CREATED)
36+
@Deprecated("Use the PUT endpoint which is idempotent")
3537
@Operation(
3638
summary = "Creates a new prison person migration mapping",
3739
description = "Creates a mapping between nomis alert ids and dps alert id. Requires ROLE_NOMIS_PRISONPERSON",
@@ -76,6 +78,31 @@ class PrisonPersonResource(private val service: PrisonPersonMigrationService) {
7678
)
7779
}
7880

81+
@PutMapping("/migration")
82+
@Operation(
83+
summary = "Creates or updates a prison person migration mapping",
84+
description = "Creates or updates a mapping between nomis prisoner numbers and prison person history ids. Requires ROLE_NOMIS_PRISONPERSON",
85+
requestBody = io.swagger.v3.oas.annotations.parameters.RequestBody(
86+
content = [Content(mediaType = "application/json", schema = Schema(implementation = PrisonPersonMigrationMappingRequest::class))],
87+
),
88+
responses = [
89+
ApiResponse(responseCode = "200", description = "Mapping created or updated"),
90+
ApiResponse(
91+
responseCode = "401",
92+
description = "Unauthorized to access this endpoint",
93+
content = [Content(mediaType = "application/json", schema = Schema(implementation = ErrorResponse::class))],
94+
),
95+
ApiResponse(
96+
responseCode = "403",
97+
description = "Access forbidden for this endpoint",
98+
content = [Content(mediaType = "application/json", schema = Schema(implementation = ErrorResponse::class))],
99+
),
100+
],
101+
)
102+
suspend fun upsertMapping(
103+
@RequestBody @Valid mappingRequest: PrisonPersonMigrationMappingRequest,
104+
) = service.upsert(mappingRequest)
105+
79106
@GetMapping("/migration/migration-id/{migrationId}")
80107
@Operation(
81108
summary = "get paged mappings by migration id",

src/test/kotlin/uk/gov/justice/digital/hmpps/nomisvisitsmappingservice/prisonerperson/PrisonPersonResourceIntTest.kt

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class PrisonPersonResourceIntTest : IntegrationTestBase() {
2828
@Nested
2929
inner class CreateMigrationMapping {
3030
@AfterEach
31-
fun teatDown() = runTest {
31+
fun tearDown() = runTest {
3232
repository.deleteAll()
3333
}
3434

@@ -71,42 +71,35 @@ class PrisonPersonResourceIntTest : IntegrationTestBase() {
7171
inner class HappyPath {
7272
@Test
7373
fun `should create migration mapping`() = runTest {
74-
webTestClient.createMigrationMapping(request("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES, listOf(1, 2, 3), "label"))
75-
.expectStatus().isCreated
76-
77-
val mapping = repository.findByNomisPrisonerNumberAndMigrationType("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)
78-
assertThat(mapping?.nomisPrisonerNumber).isEqualTo("A1234AA")
79-
assertThat(mapping?.migrationType).isEqualTo(PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)
80-
assertThat(mapping?.dpsIds).isEqualTo("[1, 2, 3]")
81-
assertThat(mapping?.label).isEqualTo("label")
82-
assertThat(mapping?.whenCreated?.toLocalDate()).isEqualTo(LocalDate.now())
83-
}
84-
}
85-
86-
@Nested
87-
inner class Validation {
88-
@Test
89-
fun `should return 409 if migration mapping already exists`() = runTest {
90-
webTestClient.createMigrationMapping(request("A1234AA", PHYSICAL_ATTRIBUTES, listOf(1), "label"))
91-
.expectStatus().isCreated
92-
93-
webTestClient.createMigrationMapping(request("A1234AA", PHYSICAL_ATTRIBUTES, listOf(1), "label"))
94-
.expectStatus().isEqualTo(409)
74+
webTestClient.upsertMigrationMapping(request("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES, listOf(1, 2, 3), "label"))
75+
.expectStatus().isOk
76+
77+
val mapping = repository.findByNomisPrisonerNumberAndMigrationType("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)!!
78+
assertThat(mapping.nomisPrisonerNumber).isEqualTo("A1234AA")
79+
assertThat(mapping.migrationType).isEqualTo(PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)
80+
assertThat(mapping.dpsIds).isEqualTo("[1, 2, 3]")
81+
assertThat(mapping.label).isEqualTo("label")
82+
assertThat(mapping.whenCreated?.toLocalDate()).isEqualTo(LocalDate.now())
9583
}
9684

97-
// The migration is idempotent and sometimes we want to migrate the same entity multiple times
9885
@Test
99-
fun `should return OK if migration mapping already exists for different migration run`() = runTest {
100-
webTestClient.createMigrationMapping(request("A1234AA", PHYSICAL_ATTRIBUTES, listOf(1), "first-migration"))
101-
.expectStatus().isCreated
102-
103-
webTestClient.createMigrationMapping(request("A1234AA", PHYSICAL_ATTRIBUTES, listOf(1), "second-migration"))
104-
.expectStatus().isCreated
86+
fun `should update migration mapping`() = runTest {
87+
webTestClient.upsertMigrationMapping(request("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES, listOf(1, 2, 3), "label"))
88+
.expectStatus().isOk
89+
webTestClient.upsertMigrationMapping(request("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES, listOf(4, 5, 6), "label"))
90+
.expectStatus().isOk
91+
92+
val mapping = repository.findByNomisPrisonerNumberAndMigrationType("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)!!
93+
assertThat(mapping.nomisPrisonerNumber).isEqualTo("A1234AA")
94+
assertThat(mapping.migrationType).isEqualTo(PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)
95+
assertThat(mapping.dpsIds).isEqualTo("[4, 5, 6]")
96+
assertThat(mapping.label).isEqualTo("label")
97+
assertThat(mapping.whenCreated?.toLocalDate()).isEqualTo(LocalDate.now())
10598
}
10699
}
107100

108-
private fun WebTestClient.createMigrationMapping(request: PrisonPersonMigrationMappingRequest) =
109-
post()
101+
private fun WebTestClient.upsertMigrationMapping(request: PrisonPersonMigrationMappingRequest) =
102+
put()
110103
.uri("/mapping/prisonperson/migration")
111104
.headers(setAuthorisation(roles = listOf("ROLE_NOMIS_PRISONPERSON")))
112105
.contentType(MediaType.APPLICATION_JSON)

0 commit comments

Comments
 (0)