PRSD-689: Refactors interactions with journeyDataKey

communitiesuk · Feb 28, 2025 · 9d5e593 · 9d5e593
1 parent 7c279d4
commit 9d5e593
Show file tree

Hide file tree

Showing 9 changed files with 45 additions and 55 deletions.
diff --git a/src/main/kotlin/uk/gov/communities/prsdb/webapp/forms/journeys/Journey.kt b/src/main/kotlin/uk/gov/communities/prsdb/webapp/forms/journeys/Journey.kt
@@ -61,11 +61,11 @@ abstract class Journey<T : StepId>(
         stepId: StepId,
         model: Model,
         subPageNumber: Int?,
-        journeyDataKey: String = journeyType.name,
         submittedPageData: PageData? = null,
+        journeyDataKey: String? = null,
     ): String {
-        journeyDataService.journeyDataKey = journeyDataKey
-        val journeyData: JourneyData = journeyDataService.getJourneyDataFromSession()
+        val journeyData: JourneyData =
+            journeyDataService.getJourneyDataFromSession(journeyDataKeyOrDefault(journeyDataKey))
         val requestedStep = getStep(stepId)
         if (!isStepReachable(requestedStep, subPageNumber)) {
             return "redirect:${getUnreachableStepRedirect(journeyData)}"
@@ -93,14 +93,16 @@ abstract class Journey<T : StepId>(
         model: Model,
         subPageNumber: Int?,
         principal: Principal,
-        journeyDataKey: String = journeyType.name,
+        journeyDataKey: String? = null,
     ): String {
-        journeyDataService.journeyDataKey = journeyDataKey
+        val journeyDataKeyOrDefault = journeyDataKeyOrDefault(journeyDataKey)
+        val journeyData = journeyDataService.getJourneyDataFromSession(journeyDataKeyOrDefault)
+
         val currentStep = getStep(stepId)
         if (!currentStep.isSatisfied(validator, pageData)) {
-            return populateModelAndGetViewName(stepId, model, subPageNumber, journeyDataKey, pageData)
+            return populateModelAndGetViewName(stepId, model, subPageNumber, pageData, journeyDataKeyOrDefault)
         }
-        val journeyData = journeyDataService.getJourneyDataFromSession()
+
         val newJourneyData = currentStep.updatedJourneyData(journeyData, pageData, subPageNumber)
         journeyDataService.setJourneyDataInSession(newJourneyData)
 

diff --git a/src/main/kotlin/uk/gov/communities/prsdb/webapp/forms/journeys/JourneyWithTaskList.kt b/src/main/kotlin/uk/gov/communities/prsdb/webapp/forms/journeys/JourneyWithTaskList.kt
@@ -20,10 +20,9 @@ abstract class JourneyWithTaskList<T : StepId>(
 
     fun populateModelAndGetTaskListViewName(
         model: Model,
-        journeyDataKey: String = journeyType.name,
+        journeyDataKey: String? = null,
     ): String {
-        journeyDataService.journeyDataKey = journeyDataKey
-        val journeyData = journeyDataService.getJourneyDataFromSession()
+        val journeyData = journeyDataService.getJourneyDataFromSession(journeyDataKeyOrDefault(journeyDataKey))
         model.addAttribute("taskListViewModel", taskListFactory.getTaskListViewModel(journeyData))
         return "taskList"
     }

diff --git a/src/main/kotlin/uk/gov/communities/prsdb/webapp/forms/journeys/LaUserRegistrationJourney.kt b/src/main/kotlin/uk/gov/communities/prsdb/webapp/forms/journeys/LaUserRegistrationJourney.kt
@@ -45,12 +45,8 @@ class LaUserRegistrationJourney(
 
     override val journeyPathSegment = REGISTER_LA_USER_JOURNEY_URL
 
-    fun initialiseJourneyData(
-        token: String,
-        journeyDataKey: String = journeyType.name,
-    ) {
-        journeyDataService.journeyDataKey = journeyDataKey
-        val journeyData = journeyDataService.getJourneyDataFromSession()
+    fun initialiseJourneyData(token: String) {
+        val journeyData = journeyDataService.getJourneyDataFromSession(journeyType.name)
         val formData: PageData = mapOf("emailAddress" to invitationService.getEmailAddressForToken(token))
         val emailStep = steps.single { step -> step.id == RegisterLaUserStepId.Email }
 

diff --git a/...ain/kotlin/uk/gov/communities/prsdb/webapp/forms/journeys/UpdateLandlordDetailsJourney.kt b/...ain/kotlin/uk/gov/communities/prsdb/webapp/forms/journeys/UpdateLandlordDetailsJourney.kt
@@ -244,12 +244,8 @@ class UpdateLandlordDetailsJourney(
         return LandlordDetailsController.LANDLORD_DETAILS_ROUTE
     }
 
-    fun initialiseJourneyDataIfNotInitialised(
-        landlordId: String,
-        journeyDataKey: String = journeyType.name,
-    ) {
-        journeyDataService.journeyDataKey = journeyDataKey
-        val journeyData = journeyDataService.getJourneyDataFromSession()
+    fun initialiseJourneyDataIfNotInitialised(landlordId: String) {
+        val journeyData = journeyDataService.getJourneyDataFromSession(journeyType.name)
         if (!journeyData.containsKey(originalDataKey)) {
             val landlord = landlordService.retrieveLandlordByBaseUserId(landlordId)!!
             val newJourneyData =

diff --git a/src/main/kotlin/uk/gov/communities/prsdb/webapp/services/JourneyDataService.kt b/src/main/kotlin/uk/gov/communities/prsdb/webapp/services/JourneyDataService.kt
@@ -2,10 +2,8 @@ package uk.gov.communities.prsdb.webapp.services
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import jakarta.servlet.http.HttpSession
-import org.springframework.context.annotation.Scope
-import org.springframework.context.annotation.ScopedProxyMode
 import org.springframework.stereotype.Service
-import org.springframework.web.context.WebApplicationContext
+import org.springframework.web.context.annotation.RequestScope
 import uk.gov.communities.prsdb.webapp.constants.enums.JourneyType
 import uk.gov.communities.prsdb.webapp.database.entity.FormContext
 import uk.gov.communities.prsdb.webapp.database.repository.FormContextRepository
@@ -16,36 +14,33 @@ import uk.gov.communities.prsdb.webapp.forms.journeys.objectToStringKeyedMap
 import java.security.Principal
 
 @Service
-@Scope(value = WebApplicationContext.SCOPE_REQUEST, proxyMode = ScopedProxyMode.TARGET_CLASS)
+@RequestScope
 class JourneyDataService(
     private val session: HttpSession,
     private val formContextRepository: FormContextRepository,
     private val oneLoginUserRepository: OneLoginUserRepository,
     private val objectMapper: ObjectMapper,
 ) {
-    var journeyDataKey: String? = null
+    // This service can have class attributes as it is request scoped
+    final lateinit var journeyDataKey: String private set
 
-    fun getJourneyDataFromSession(): JourneyData =
-        if (journeyDataKey != null) {
-            objectToStringKeyedMap(session.getAttribute(journeyDataKey)) ?: mapOf()
+    fun getJourneyDataFromSession(journeyDataKey: String): JourneyData {
+        if (this::journeyDataKey.isInitialized && journeyDataKey != this.journeyDataKey) {
+            throw PrsdbWebException("journeyDataKey has already been set to ${this.journeyDataKey}")
         } else {
-            throw PrsdbWebException("journeyDataKey not set")
+            this.journeyDataKey = journeyDataKey
         }
+        return getJourneyDataFromSession()
+    }
+
+    fun getJourneyDataFromSession(): JourneyData = objectToStringKeyedMap(session.getAttribute(journeyDataKey)) ?: mapOf()
 
     fun setJourneyDataInSession(journeyData: JourneyData) {
-        if (journeyDataKey != null) {
-            session.setAttribute(journeyDataKey, journeyData)
-        } else {
-            throw PrsdbWebException("journeyDataKey not set")
-        }
+        session.setAttribute(journeyDataKey, journeyData)
     }
 
     fun clearJourneyDataFromSession() {
-        if (journeyDataKey != null) {
-            session.setAttribute(journeyDataKey, null)
-        } else {
-            throw PrsdbWebException("journeyDataKey not set")
-        }
+        session.setAttribute(journeyDataKey, null)
     }
 
     fun getContextId(): Long? = session.getAttribute("contextId") as? Long

diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/controllers/RegisterLAUserControllerTests.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/controllers/RegisterLAUserControllerTests.kt
@@ -15,7 +15,6 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers
 import org.springframework.web.context.WebApplicationContext
 import uk.gov.communities.prsdb.webapp.constants.LA_USER_ID
 import uk.gov.communities.prsdb.webapp.constants.REGISTER_LA_USER_JOURNEY_URL
-import uk.gov.communities.prsdb.webapp.constants.enums.JourneyType
 import uk.gov.communities.prsdb.webapp.controllers.RegisterLAUserController.Companion.CONFIRMATION_PAGE_PATH_SEGMENT
 import uk.gov.communities.prsdb.webapp.forms.journeys.LaUserRegistrationJourney
 import uk.gov.communities.prsdb.webapp.forms.steps.RegisterLaUserStepId
@@ -43,7 +42,6 @@ class RegisterLAUserControllerTests(
     @BeforeEach
     fun setupMocks() {
         whenever(laUserRegistrationJourney.initialStepId).thenReturn(RegisterLaUserStepId.LandingPage)
-        whenever(laUserRegistrationJourney.journeyType).thenReturn(JourneyType.LA_USER_REGISTRATION)
         whenever(invitationService.tokenIsValid(validToken)).thenReturn(true)
         whenever(invitationService.getTokenFromSession()).thenReturn(validToken)
         whenever(invitationService.tokenIsValid(invalidToken)).thenReturn(false)
@@ -58,7 +56,7 @@ class RegisterLAUserControllerTests(
 
         verify(invitationService).tokenIsValid(validToken)
         verify(invitationService).storeTokenInSession(validToken)
-        verify(laUserRegistrationJourney).initialiseJourneyData(validToken, JourneyType.LA_USER_REGISTRATION.name)
+        verify(laUserRegistrationJourney).initialiseJourneyData(validToken)
     }
 
     @Test
@@ -70,10 +68,7 @@ class RegisterLAUserControllerTests(
 
         verify(invitationService).tokenIsValid(invalidToken)
         verify(invitationService, never()).storeTokenInSession(invalidToken)
-        verify(laUserRegistrationJourney, never()).initialiseJourneyData(
-            validToken,
-            JourneyType.LA_USER_REGISTRATION.name,
-        )
+        verify(laUserRegistrationJourney, never()).initialiseJourneyData(validToken)
     }
 
     @Test

diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/forms/JourneyTests.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/forms/JourneyTests.kt
@@ -16,6 +16,7 @@ import org.mockito.Mock
 import org.mockito.Mockito.mock
 import org.mockito.internal.matchers.apachecommons.ReflectionEquals
 import org.mockito.junit.jupiter.MockitoExtension
+import org.mockito.kotlin.any
 import org.mockito.kotlin.argumentCaptor
 import org.mockito.kotlin.eq
 import org.mockito.kotlin.never
@@ -36,6 +37,7 @@ import uk.gov.communities.prsdb.webapp.forms.journeys.JourneyData
 import uk.gov.communities.prsdb.webapp.forms.journeys.JourneySection
 import uk.gov.communities.prsdb.webapp.forms.journeys.JourneyTask
 import uk.gov.communities.prsdb.webapp.forms.journeys.PageData
+import uk.gov.communities.prsdb.webapp.forms.journeys.PropertyRegistrationJourney
 import uk.gov.communities.prsdb.webapp.forms.journeys.objectToStringKeyedMap
 import uk.gov.communities.prsdb.webapp.forms.pages.Page
 import uk.gov.communities.prsdb.webapp.forms.steps.Step
@@ -259,7 +261,7 @@ class JourneyTests {
 
             // Act
             val result =
-                testJourney.populateModelAndGetViewName(TestStepId.StepTwo, model, null, submittedPageData = null)
+                testJourney.populateModelAndGetViewName(TestStepId.StepTwo, model, null, null)
 
             // Assert
             assertEquals("redirect:${TestStepId.StepOne.urlPathSegment}", result)
@@ -304,7 +306,7 @@ class JourneyTests {
 
             // Act
             val result =
-                testJourney.populateModelAndGetViewName(TestStepId.StepTwo, model, null, submittedPageData = null)
+                testJourney.populateModelAndGetViewName(TestStepId.StepTwo, model, null, null)
 
             // Assert
             assertEquals("redirect:${TestStepId.StepOne.urlPathSegment}", result)
@@ -350,7 +352,7 @@ class JourneyTests {
 
             // Act
             val result =
-                testJourney.populateModelAndGetViewName(TestStepId.StepTwo, model, null, submittedPageData = pageData)
+                testJourney.populateModelAndGetViewName(TestStepId.StepTwo, model, null, pageData)
 
             // Assert
             assertIs<BindingResult>(model[BindingResult.MODEL_KEY_PREFIX + "formModel"])
@@ -460,7 +462,6 @@ class JourneyTests {
                     mockOneLoginUserRepository,
                     ObjectMapper(),
                 )
-            journeyDataService.journeyDataKey = "test-journey-key"
         }
 
         @Test
@@ -534,14 +535,17 @@ class JourneyTests {
                     TestStepId.StepOne.urlPathSegment to pageDataStepOne,
                     TestStepId.StepThree.urlPathSegment to pageDataStepThree,
                 )
+            whenever(spiedOnJourneyDataService.getJourneyDataFromSession(testJourney.journeyType.name)).thenReturn(
+                journeyData,
+            )
             whenever(spiedOnJourneyDataService.getJourneyDataFromSession()).thenReturn(journeyData)
 
             // Act
             testJourney.populateModelAndGetViewName(
                 TestStepId.StepFour,
                 model,
                 null,
-                submittedPageData = pageDataStepFour,
+                pageDataStepFour,
             )
 
             // Assert

diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/forms/TaskListTests.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/forms/TaskListTests.kt
@@ -6,6 +6,7 @@ import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Nested
 import org.mockito.Mock
 import org.mockito.Mockito.mock
+import org.mockito.kotlin.any
 import org.mockito.kotlin.whenever
 import org.springframework.ui.ExtendedModelMap
 import org.springframework.validation.Validator
@@ -67,6 +68,7 @@ class TaskListTests {
 
         // Ensure form data for each page is never null
         val journeyData = TestStepId.entries.associate { it.urlPathSegment to mutableMapOf<String, String>() }
+        whenever(mockJourneyDataService.getJourneyDataFromSession(any())).thenReturn(journeyData.toMutableMap())
         whenever(mockJourneyDataService.getJourneyDataFromSession()).thenReturn(journeyData.toMutableMap())
     }
 

diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/services/JourneyDataServiceTests.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/services/JourneyDataServiceTests.kt
@@ -47,7 +47,6 @@ class JourneyDataServiceTests {
                 mockOneLoginUserRepository,
                 ObjectMapper(),
             )
-        journeyDataService.journeyDataKey = "test-journey-key"
     }
 
     @Nested
@@ -177,6 +176,7 @@ class JourneyDataServiceTests {
             val journeyType = JourneyType.LANDLORD_REGISTRATION
 
             // JourneyData
+            val journeyDataKey = "journey-data-key"
             val pageName = "testPage"
             val key = "testKey"
             val value = "testValue"
@@ -195,9 +195,10 @@ class JourneyDataServiceTests {
             whenever(mockFormContextRepository.findById(contextId)).thenReturn(Optional.ofNullable(formContext))
 
             // Act
+            journeyDataService.getJourneyDataFromSession(journeyDataKey)
             journeyDataService.loadJourneyDataIntoSession(contextId)
             val formContextCaptor = captor<JourneyData>()
-            verify(mockHttpSession).setAttribute(eq(journeyDataService.journeyDataKey), formContextCaptor.capture())
+            verify(mockHttpSession).setAttribute(eq(journeyDataKey), formContextCaptor.capture())
             val contextIdCaptor = captor<Long>()
             verify(mockHttpSession).setAttribute(eq("contextId"), contextIdCaptor.capture())