PI-2228 Remove synchronized block from prison staff creation (#3990)

libs/prison-staff/src/main/kotlin/uk/gov/justice/digital/hmpps/service/AssignmentService.kt

@@ -1,6 +1,7 @@
 package uk.gov.justice.digital.hmpps.service
 
 import org.springframework.stereotype.Service
+import org.springframework.util.ConcurrentReferenceHashMap
 import uk.gov.justice.digital.hmpps.entity.PrisonStaff
 import uk.gov.justice.digital.hmpps.exception.NotFoundException
 import uk.gov.justice.digital.hmpps.exceptions.InvalidEstablishmentCodeException
@@ -9,13 +10,19 @@ import uk.gov.justice.digital.hmpps.repository.PrisonProbationAreaRepository
 import uk.gov.justice.digital.hmpps.repository.PrisonTeamRepository
 import uk.gov.justice.digital.hmpps.retry.retry
 import java.time.ZonedDateTime
+import java.util.concurrent.locks.ReentrantLock
+import kotlin.concurrent.withLock
 
 @Service
 class AssignmentService(
     private val probationAreaRepository: PrisonProbationAreaRepository,
     private val teamRepository: PrisonTeamRepository,
     private val staffService: StaffService
 ) {
+    companion object {
+        private val mutexMap = ConcurrentReferenceHashMap<Long, ReentrantLock>()
+        private fun getMutex(key: Long) = mutexMap.compute(key) { _, v -> v ?: ReentrantLock() }!!
+    }
 
     fun findAssignment(establishmentCode: String, staffName: StaffName): Triple<Long, Long, Long> {
         if (establishmentCode.length < 3) throw InvalidEstablishmentCodeException(establishmentCode)
@@ -36,7 +43,7 @@ class AssignmentService(
         teamId: Long,
         staffName: StaffName,
         allocationDate: ZonedDateTime? = null
-    ): PrisonStaff {
+    ): PrisonStaff = getMutex(probationAreaId).withLock {
         val findStaff = { staffService.findStaff(probationAreaId, staffName) }
         return retry(3) {
             findStaff() ?: staffService.create(probationAreaId, probationAreaCode, teamId, staffName, allocationDate)

libs/prison-staff/src/main/kotlin/uk/gov/justice/digital/hmpps/service/StaffService.kt

@@ -3,7 +3,6 @@ package uk.gov.justice.digital.hmpps.service
 import org.springframework.stereotype.Service
 import org.springframework.transaction.annotation.Propagation
 import org.springframework.transaction.annotation.Transactional
-import org.springframework.util.ConcurrentReferenceHashMap
 import uk.gov.justice.digital.hmpps.entity.PrisonStaff
 import uk.gov.justice.digital.hmpps.entity.PrisonStaffTeam
 import uk.gov.justice.digital.hmpps.model.StaffName
@@ -19,33 +18,22 @@ class StaffService(
     private val staffTeamRepository: PrisonStaffTeamRepository
 ) {
 
-    companion object {
-        private val mutexMap = ConcurrentReferenceHashMap<Long, Any>()
-        private fun getMutex(key: Long) {
-            mutexMap.compute(key) { _, v -> v ?: Any() }
-        }
-    }
-
     @Transactional(propagation = Propagation.REQUIRES_NEW)
     fun create(
-        paId: Long,
-        paCode: String,
+        probationAreaId: Long,
+        probationAreaCode: String,
         teamId: Long,
         staffName: StaffName,
         startDate: ZonedDateTime? = null
-    ): PrisonStaff = synchronized(getMutex(paId)) {
-        val staff = staffRepository.save(
-            PrisonStaff(
-                forename = staffName.forename,
-                surname = staffName.surname,
-                probationAreaId = paId,
-                code = officerCodeGenerator.generateFor(paCode),
-                startDate = startDate ?: ZonedDateTime.now()
-            )
+    ) = staffRepository.save(
+        PrisonStaff(
+            forename = staffName.forename,
+            surname = staffName.surname,
+            probationAreaId = probationAreaId,
+            code = officerCodeGenerator.generateFor(probationAreaCode),
+            startDate = startDate ?: ZonedDateTime.now()
         )
-        staffTeamRepository.save(PrisonStaffTeam(staff.id, teamId))
-        return staff
-    }
+    ).also { staffTeamRepository.save(PrisonStaffTeam(it.id, teamId)) }
 
     fun findStaff(probationAreaId: Long, staffName: StaffName) =
         staffRepository.findTopByProbationAreaIdAndForenameIgnoreCaseAndSurnameIgnoreCase(

libs/prison-staff/src/test/kotlin/uk/gov/justice/digital/hmpps/service/AssignmentServiceTest.kt

@@ -20,6 +20,8 @@ import uk.gov.justice.digital.hmpps.exceptions.InvalidEstablishmentCodeException
 import uk.gov.justice.digital.hmpps.model.StaffName
 import uk.gov.justice.digital.hmpps.repository.PrisonProbationAreaRepository
 import uk.gov.justice.digital.hmpps.repository.PrisonTeamRepository
+import java.util.concurrent.CompletableFuture.allOf
+import java.util.concurrent.CompletableFuture.runAsync
 
 @ExtendWith(MockitoExtension::class)
 class AssignmentServiceTest {
@@ -169,6 +171,35 @@ class AssignmentServiceTest {
             )
         }
     }
+
+    @Test
+    fun `prevents creation of duplicate staff`() {
+        val probationArea = ProbationAreaGenerator.DEFAULT
+        val team = TeamGenerator.DEFAULT
+        val newStaff = PrisonStaff(
+            code = "C12A001",
+            forename = staffName.forename,
+            surname = staffName.surname,
+            probationAreaId = probationArea.id
+        )
+
+        whenever(probationAreaRepository.findByInstitutionNomisCode("LEI"))
+            .thenReturn(probationArea)
+        whenever(teamRepository.findByCode(team.code)).thenReturn(team)
+        whenever(staffService.findStaff(probationArea.id, staffName))
+            .thenReturn(null)
+        whenever(staffService.create(probationArea.id, probationArea.code, team.id, staffName, null))
+            .thenAnswer {
+                whenever(staffService.findStaff(probationArea.id, staffName)).thenReturn(newStaff)
+                newStaff
+            }
+
+        allOf(*List(3) {
+            runAsync { assignmentService.findAssignment("LEI", staffName) }
+        }.toTypedArray()).join()
+
+        verify(staffService, times(1)).create(probationArea.id, probationArea.code, team.id, staffName)
+    }
 }
 
 object ProbationAreaGenerator {