SDIT-2021 Cater for retries to the mapping creation by performing an …

…upsert (#369)

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

@@ -12,7 +12,7 @@ data class PrisonPersonMigrationMapping(
 
   val migrationType: PrisonPersonMigrationType,
 
-  val dpsIds: String,
+  var dpsIds: String,
 
   val label: String,
 

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

@@ -8,6 +8,7 @@ import org.springframework.stereotype.Repository
 @Repository
 interface PrisonPersonMigrationMappingRepository : CoroutineCrudRepository<PrisonPersonMigrationMapping, String> {
   suspend fun findByNomisPrisonerNumberAndMigrationType(nomisPrisonerNumber: String, migrationType: PrisonPersonMigrationType): PrisonPersonMigrationMapping?
+  suspend fun findByNomisPrisonerNumberAndMigrationTypeAndLabel(nomisPrisonerNumber: String, migrationType: PrisonPersonMigrationType, label: String): PrisonPersonMigrationMapping?
   fun findAllByLabelOrderByNomisPrisonerNumberAsc(label: String, pageable: Pageable): Flow<PrisonPersonMigrationMapping>
   suspend fun countAllByLabel(label: String): Long
 }

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

@@ -3,15 +3,24 @@ package uk.gov.justice.digital.hmpps.nomisvisitsmappingservice.prisonperson
 import kotlinx.coroutines.async
 import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.flow.toList
+import kotlinx.coroutines.reactor.awaitSingle
 import org.springframework.data.domain.Page
 import org.springframework.data.domain.PageImpl
 import org.springframework.data.domain.Pageable
+import org.springframework.data.r2dbc.core.R2dbcEntityTemplate
+import org.springframework.data.r2dbc.core.insert
+import org.springframework.data.r2dbc.core.update
+import org.springframework.data.relational.core.query.Criteria.where
+import org.springframework.data.relational.core.query.Query.query
+import org.springframework.data.relational.core.query.Update.update
 import org.springframework.stereotype.Service
+import org.springframework.transaction.annotation.Transactional
 
 @Service
 class PrisonPersonMigrationService(
   private val repository: PrisonPersonMigrationMappingRepository,
   private val prisonPersonMigrationMappingRepository: PrisonPersonMigrationMappingRepository,
+  private val template: R2dbcEntityTemplate,
 ) {
 
   suspend fun create(mappingRequest: PrisonPersonMigrationMappingRequest) {
@@ -25,6 +34,41 @@ class PrisonPersonMigrationService(
     )
   }
 
+  @Transactional
+  suspend fun upsert(mappingRequest: PrisonPersonMigrationMappingRequest): PrisonPersonMigrationMapping =
+    mappingRequest.find()
+      ?.let { mappingRequest.update() }
+      ?: let { mappingRequest.insert() }
+
+  private suspend fun PrisonPersonMigrationMappingRequest.find() =
+    repository.findByNomisPrisonerNumberAndMigrationTypeAndLabel(nomisPrisonerNumber, migrationType, label)
+
+  private suspend fun PrisonPersonMigrationMappingRequest.update(): PrisonPersonMigrationMapping =
+    template.update<PrisonPersonMigrationMapping>()
+      .inTable("prison_person_migration_mapping")
+      .matching(
+        query(
+          where("nomis_prisoner_number").`is`(nomisPrisonerNumber)
+            .and(where("migration_type").`is`(migrationType))
+            .and(where("label").`is`(label)),
+        ),
+      )
+      .apply(update("dps_ids", dpsIds.toString()))
+      .awaitSingle()
+      .let { find()!! }
+
+  private suspend fun PrisonPersonMigrationMappingRequest.insert(): PrisonPersonMigrationMapping =
+    template.insert<PrisonPersonMigrationMapping>()
+      .using(
+        PrisonPersonMigrationMapping(
+          nomisPrisonerNumber = nomisPrisonerNumber,
+          migrationType = migrationType,
+          dpsIds = dpsIds.toString(),
+          label = label,
+        ),
+      )
+      .awaitSingle()
+
   suspend fun find(nomisPrisonerNumber: String, migrationType: PrisonPersonMigrationType) = repository.findByNomisPrisonerNumberAndMigrationType(nomisPrisonerNumber, migrationType)
 
   suspend fun getMappings(

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

@@ -16,6 +16,7 @@ import org.springframework.validation.annotation.Validated
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.PathVariable
 import org.springframework.web.bind.annotation.PostMapping
+import org.springframework.web.bind.annotation.PutMapping
 import org.springframework.web.bind.annotation.RequestBody
 import org.springframework.web.bind.annotation.RequestMapping
 import org.springframework.web.bind.annotation.ResponseStatus
@@ -32,6 +33,7 @@ class PrisonPersonResource(private val service: PrisonPersonMigrationService) {
 
   @PostMapping("/migration")
   @ResponseStatus(HttpStatus.CREATED)
+  @Deprecated("Use the PUT endpoint which is idempotent")
   @Operation(
     summary = "Creates a new prison person migration mapping",
     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) {
       )
     }
 
+  @PutMapping("/migration")
+  @Operation(
+    summary = "Creates or updates a prison person migration mapping",
+    description = "Creates or updates a mapping between nomis prisoner numbers and prison person history ids. Requires ROLE_NOMIS_PRISONPERSON",
+    requestBody = io.swagger.v3.oas.annotations.parameters.RequestBody(
+      content = [Content(mediaType = "application/json", schema = Schema(implementation = PrisonPersonMigrationMappingRequest::class))],
+    ),
+    responses = [
+      ApiResponse(responseCode = "200", description = "Mapping created or updated"),
+      ApiResponse(
+        responseCode = "401",
+        description = "Unauthorized to access this endpoint",
+        content = [Content(mediaType = "application/json", schema = Schema(implementation = ErrorResponse::class))],
+      ),
+      ApiResponse(
+        responseCode = "403",
+        description = "Access forbidden for this endpoint",
+        content = [Content(mediaType = "application/json", schema = Schema(implementation = ErrorResponse::class))],
+      ),
+    ],
+  )
+  suspend fun upsertMapping(
+    @RequestBody @Valid mappingRequest: PrisonPersonMigrationMappingRequest,
+  ) = service.upsert(mappingRequest)
+
   @GetMapping("/migration/migration-id/{migrationId}")
   @Operation(
     summary = "get paged mappings by migration id",

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

@@ -28,7 +28,7 @@ class PrisonPersonResourceIntTest : IntegrationTestBase() {
   @Nested
   inner class CreateMigrationMapping {
     @AfterEach
-    fun teatDown() = runTest {
+    fun tearDown() = runTest {
       repository.deleteAll()
     }
 
@@ -71,42 +71,35 @@ class PrisonPersonResourceIntTest : IntegrationTestBase() {
     inner class HappyPath {
       @Test
       fun `should create migration mapping`() = runTest {
-        webTestClient.createMigrationMapping(request("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES, listOf(1, 2, 3), "label"))
-          .expectStatus().isCreated
-
-        val mapping = repository.findByNomisPrisonerNumberAndMigrationType("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)
-        assertThat(mapping?.nomisPrisonerNumber).isEqualTo("A1234AA")
-        assertThat(mapping?.migrationType).isEqualTo(PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)
-        assertThat(mapping?.dpsIds).isEqualTo("[1, 2, 3]")
-        assertThat(mapping?.label).isEqualTo("label")
-        assertThat(mapping?.whenCreated?.toLocalDate()).isEqualTo(LocalDate.now())
-      }
-    }
-
-    @Nested
-    inner class Validation {
-      @Test
-      fun `should return 409 if migration mapping already exists`() = runTest {
-        webTestClient.createMigrationMapping(request("A1234AA", PHYSICAL_ATTRIBUTES, listOf(1), "label"))
-          .expectStatus().isCreated
-
-        webTestClient.createMigrationMapping(request("A1234AA", PHYSICAL_ATTRIBUTES, listOf(1), "label"))
-          .expectStatus().isEqualTo(409)
+        webTestClient.upsertMigrationMapping(request("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES, listOf(1, 2, 3), "label"))
+          .expectStatus().isOk
+
+        val mapping = repository.findByNomisPrisonerNumberAndMigrationType("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)!!
+        assertThat(mapping.nomisPrisonerNumber).isEqualTo("A1234AA")
+        assertThat(mapping.migrationType).isEqualTo(PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)
+        assertThat(mapping.dpsIds).isEqualTo("[1, 2, 3]")
+        assertThat(mapping.label).isEqualTo("label")
+        assertThat(mapping.whenCreated?.toLocalDate()).isEqualTo(LocalDate.now())
       }
 
-      // The migration is idempotent and sometimes we want to migrate the same entity multiple times
       @Test
-      fun `should return OK if migration mapping already exists for different migration run`() = runTest {
-        webTestClient.createMigrationMapping(request("A1234AA", PHYSICAL_ATTRIBUTES, listOf(1), "first-migration"))
-          .expectStatus().isCreated
-
-        webTestClient.createMigrationMapping(request("A1234AA", PHYSICAL_ATTRIBUTES, listOf(1), "second-migration"))
-          .expectStatus().isCreated
+      fun `should update migration mapping`() = runTest {
+        webTestClient.upsertMigrationMapping(request("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES, listOf(1, 2, 3), "label"))
+          .expectStatus().isOk
+        webTestClient.upsertMigrationMapping(request("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES, listOf(4, 5, 6), "label"))
+          .expectStatus().isOk
+
+        val mapping = repository.findByNomisPrisonerNumberAndMigrationType("A1234AA", PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)!!
+        assertThat(mapping.nomisPrisonerNumber).isEqualTo("A1234AA")
+        assertThat(mapping.migrationType).isEqualTo(PROFILE_DETAILS_PHYSICAL_ATTRIBUTES)
+        assertThat(mapping.dpsIds).isEqualTo("[4, 5, 6]")
+        assertThat(mapping.label).isEqualTo("label")
+        assertThat(mapping.whenCreated?.toLocalDate()).isEqualTo(LocalDate.now())
       }
     }
 
-    private fun WebTestClient.createMigrationMapping(request: PrisonPersonMigrationMappingRequest) =
-      post()
+    private fun WebTestClient.upsertMigrationMapping(request: PrisonPersonMigrationMappingRequest) =
+      put()
         .uri("/mapping/prisonperson/migration")
         .headers(setAuthorisation(roles = listOf("ROLE_NOMIS_PRISONPERSON")))
         .contentType(MediaType.APPLICATION_JSON)