Hdpi 6650 refactor org service to cache responses by m014389b · Pull Request #2178 · hmcts/pcs-api

m014389b · 2026-07-15T12:16:33Z

Jira link

See [HDPI-6550-](https://tools.hmcts.net/jira/browse/HDPI-6650)

Change description

Introduced a cache wrapper around organisationDetailsService.getOrganisationIdentifier.

Organisation details for a given user IdamId is stored in "cached_organisation_response"

Invocations to organisationService now will call the cached solution.

Cache service checks if a response exists for a idamId, if not present then calls rdProfessional.
If present but older than the configurable TTL then will be retrieved again otherwise the cached response is returned.

Testing done

Unit testing
Manual on preview, updating the "dbrefdata.organisation.id" in "rd-professional" for organisation with organisation_identifier='FHBG1F8' for user "possession-org-test-automation@test.com"

Security Vulnerability Assessment

CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?

Yes
No

Checklist

commit messages are meaningful and follow good commit message guidelines
README and other documentation has been updated / added (if needed)
tests have been updated / new tests has been added (if needed)
Does this PR introduce a breaking change

…he ResumePossessionClaim event.

…rformance issues in the start method. This found only the single database connection for the retrieval of the draft.

…414-PerfTest-Env-Make-a-Claim-Baseline

…aim-Baseline' into HDPI-6650_refactor_org_service_to_cache_responses

…resent or older than 24 hour

…ith lower ttl locally and in preview

…sponses # Conflicts: # src/main/resources/application.yaml

github-actions · 2026-07-15T12:19:19Z

DB breaking change scan

Recommended reading: HMCTS zero-downtime database deployments

No potentially breaking database changes were detected in the changed Flyway migration .sql files.

This check only scans changed .sql files under src/main/resources/db/migration and flags high-risk DDL patterns. It does not replace a full migration review.

Run details: https://github.com/hmcts/pcs-api/actions/runs/30100693784?check_suite_focus=true

github-actions · 2026-07-15T12:21:45Z

CCD diff summary

👉 Full report: https://github.com/hmcts/pcs-api/actions/runs/30100693688?check_suite_focus=true

No change

…sponses

tvr-hmcts

Looking good. Some initial comments.

tvr-hmcts · 2026-07-16T12:54:09Z

+            cachedOrganisationResponseRepository.findByIdamId(userIdam);
+
+        if (cachedResponse.isEmpty()) {
+            OrganisationDetailsResponse response = organisationDetailsService.getOrganisationDetails(userId);


perhaps a log in here to say "cache miss" or something along those lines?

tvr-hmcts · 2026-07-17T10:31:52Z

-                ex.getMessage(), ex);
-            // Return null instead of throwing to allow graceful degradation
+            log.error("Error retrieving organisation name and address from rd-professional API. Error: {}",
+                      ex.getMessage(), ex);


Please remove the ex.getMessage() Exception on the end is ok to keep though.

See ticket HDPI-7594

tvr-hmcts · 2026-07-17T10:32:38Z

+
+        } catch (FeignException ex) {
+            log.error("Feign error retrieving organisation details for userId: {}. Status: {}, Message: {}",
+                      userId, ex.status(), ex.getMessage(), ex);


Please remove the ex.getMessage() Exception on the end is ok to keep though.

See ticket HDPI-7594

tvr-hmcts · 2026-07-17T10:32:46Z

+            throw new OrganisationDetailsException("Failed to retrieve organisation details", ex);
+        } catch (Exception ex) {
+            log.error("Unexpected error retrieving organisation details for userId: {}. Error: {}",
+                      userId, ex.getMessage(), ex);


Please remove the ex.getMessage() Exception on the end is ok to keep though.

See ticket HDPI-7594

tvr-hmcts · 2026-07-17T10:36:32Z

+CREATE TABLE public.cached_organisation_response
+(
+  id UUID PRIMARY KEY,
+  idam_id UUID,


Think this should be NOT NULL

tvr-hmcts · 2026-07-17T10:37:40Z

+  id UUID PRIMARY KEY,
+  idam_id UUID,
+  organisation_id varchar(64),
+  last_modified_date TIMESTAMP,


Perhaps NOT NULL here also as it is needed in the checks.

This timestamp 'may' need double checking as to the agreed approach as there is a splattering of different approaches in the code base - which I have gotten wrong before so pls take a look.

- remove exception message being logged

…sponses

tvr-hmcts

Just some minors and small alterations from the save methods.

tvr-hmcts · 2026-07-20T14:39:17Z

+
+            CachedOrganisationResponseEntity newCachedResponse = mapResponseToEntity(userIdam, response);
+
+            cachedOrganisationResponseRepository.save(newCachedResponse);


normally fine but safer to return from this line itself incase we do anything else lower down.

tvr-hmcts · 2026-07-20T14:44:31Z

+            if (isDataRequiringResync(existingCachedResponse.getLastModifiedDate())) {
+                OrganisationDetailsResponse response = getOrganisationDetails(userId);
+                updateFields(existingCachedResponse, response, userId);
+                cachedOrganisationResponseRepository.save(existingCachedResponse);


safest to reasign existingCachedResponse = cachedOrgi...save(existingCachedResponse)

tvr-hmcts · 2026-07-20T15:03:52Z

+     * @param userId The user ID to get organisation details for
+     * @return OrganisationDetailsResponse containing organisation information
+     */
+    private OrganisationDetailsResponse getOrganisationDetails(String userId) {


As this can return null then should it be Optional<...> then there can be a test on the Optional being empty as there is no test on the null return at the moment.

tvr-hmcts · 2026-07-20T15:07:55Z

+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class CachingOrganisationDetailsServiceTest {


Think these should be assertThat(...).is... in this class

It is a bit missed at the moment.

tvr-hmcts · 2026-07-20T15:09:00Z

+        assertNull(actual.address().getAddressLine1());
+
+        verify(cachedOrganisationResponseRepository).save(cachedEntity);
+        assertNull(cachedEntity.getAddressLine1());


Perhaps all values of the existingCachedResponse can have an assertThat(x.y).isNull() To be sure to be sure.

tvr-hmcts · 2026-07-20T15:14:35Z

+            CachedOrganisationResponseEntity existingCachedResponse = cachedResponse.get();
+
+            if (isDataRequiringResync(existingCachedResponse.getLastModifiedDate())) {
+                OrganisationDetailsResponse response = getOrganisationDetails(userId);


Minor : log in here to say it is resyncing.

tvr-hmcts · 2026-07-20T15:16:23Z

-import uk.gov.hmcts.reform.pcs.service.FeatureFlag;
-import uk.gov.hmcts.reform.pcs.service.FeatureToggleService;
-import uk.gov.hmcts.reform.pcs.testingsupport.model.TestingSupportAccessCode;
+import uk.gov.hmcts.reform.pcs.ccd.entity.ClaimEntity;


unused imports - wonder why the pipeline did not catch them

…sponses

…d quicker

tvr-hmcts

Think it is good to go :)

tvr-hmcts · 2026-07-21T12:12:58Z

+                if (response.isPresent()) {
+                    updateFields(existingCachedResponse, response.get(), userId);
+                } else {
+                    clearCachedFields(existingCachedResponse);


This can still leave the NameAndAddress being returned with nulls set.

tvr-hmcts · 2026-07-21T12:26:43Z

        caseData.setClaimantType(claimantTypeList);

-        contactPreferences.setOrganisationAddress(organisationService.getOrganisationAddressForCurrentUser());
+        contactPreferences.setOrganisationAddress(null != nameAndAddress ? nameAndAddress.address() : null);


There is a slight difference here in that the AddressUK will not be null but the values within it now can be. Is that ok?

…sponses

guygrewal77 · 2026-07-21T15:39:58Z

+  id UUID PRIMARY KEY,
+  idam_id UUID NOT NULL,
+  organisation_id varchar(64),
+  last_modified_date TIMESTAMP NOT NULL,


Doesn't the date need to be with timezone ? In claim_issued_date and claim_submitted_date they're both timestamptz.

guygrewal77 · 2026-07-21T15:46:02Z

+                                             AuthTokenGenerator authTokenGenerator,
+                                             @Qualifier("prdAdminTokenProvider")
+                                                 IdamTokenProvider prdAdminTokenProvider) {
+        this(cachedOrganisationResponseRepository, ttlInMinutes, LocalDateTime::now,


Should we be using the same approach as in ClaimService to have similar date handling ?

…signed in) via postman

…sponses

tvr-solirius and others added 8 commits June 25, 2026 17:37

Reduction on the number of calls to Ref Data within the start up of t…

50f2c08

…he ResumePossessionClaim event.

Removal of hibernate statistics and SQL debugging logs to identify pe…

ef74f1f

…rformance issues in the start method. This found only the single database connection for the retrieval of the draft.

Merge remote-tracking branch 'refs/remotes/origin/master' into HDPI-5…

3638aab

…414-PerfTest-Env-Make-a-Claim-Baseline

Merge remote-tracking branch 'refs/remotes/origin/master' into HDPI-5…

917634b

…414-PerfTest-Env-Make-a-Claim-Baseline

Merge remote-tracking branch 'refs/remotes/origin/master' into HDPI-5…

3a63b2f

…414-PerfTest-Env-Make-a-Claim-Baseline

Merge remote-tracking branch 'origin/HDPI-5414-PerfTest-Env-Make-a-Cl…

5b85c3d

…aim-Baseline' into HDPI-6650_refactor_org_service_to_cache_responses

HDPI-6650: add a cache db for org response id, get from http if not p…

697f032

…resent or older than 24 hour

HDPI-6650: fix date check and add application properties configured w…

dde29aa

…ith lower ttl locally and in preview

m014389b added ns:pcs prd:pcs enable_keep_helm pr-values:ccd labels Jul 15, 2026

hmcts-jenkins-j-to-z Bot and others added 2 commits July 15, 2026 12:17

Bumping chart version/ fixing aliases

d306405

Merge branch 'master' into HDPI-6650_refactor_org_service_to_cache_re…

9aeb8f7

…sponses # Conflicts: # src/main/resources/application.yaml

m014389b and others added 5 commits July 15, 2026 13:50

HDPI-6650: linting

ce9b768

HDPI-6650: linting fixes

a0abb3a

HDPI-6650: improve caching coverage

c6517f1

Merge branch 'master' into HDPI-6650_refactor_org_service_to_cache_re…

5154454

…sponses

Bumping chart version/ fixing aliases

8d4f6f0

hmcts-jenkins-j-to-z Bot deployed to preview July 16, 2026 10:44 View deployment

hmcts-jenkins-j-to-z Bot added the rel:pcs-api-pr-2178 label Jul 16, 2026

m014389b and others added 2 commits July 16, 2026 13:22

HDPI-6650: add logging when no address

1caf3f7

Merge branch 'master' into HDPI-6650_refactor_org_service_to_cache_re…

0323e07

…sponses

m014389b marked this pull request as ready for review July 16, 2026 12:29

m014389b requested review from a team, linusnorton and saurabhvalsangkar-hmcts July 16, 2026 12:29

tvr-hmcts reviewed Jul 17, 2026

View reviewed changes

m014389b and others added 2 commits July 20, 2026 13:13

HDPI-6650: add not null to idam id and last modified date

5017d00

- remove exception message being logged

Merge branch 'master' into HDPI-6650_refactor_org_service_to_cache_re…

b9afc5c

…sponses

tvr-hmcts reviewed Jul 20, 2026

View reviewed changes

m014389b and others added 2 commits July 20, 2026 16:22

HDPI-6650: fix db versions

9de434a

Merge branch 'master' into HDPI-6650_refactor_org_service_to_cache_re…

96b9914

…sponses

hmcts-jenkins-j-to-z Bot deployed to preview July 20, 2026 16:08 View deployment

m014389b added 2 commits July 20, 2026 18:15

HDPI-6650: add optional to protect null org details response

c99804b

HDPI-6650: fix formatting and test coverage

f828a3b

hmcts-jenkins-j-to-z Bot deployed to preview July 21, 2026 09:11 View deployment

tvr-hmcts reviewed Jul 21, 2026

View reviewed changes

Comment thread src/main/java/uk/gov/hmcts/reform/pcs/reference/service/CachingOrganisationDetailsService.java

HDPI-6650: add an offset when org details response is null to resynce…

29a5bae

…d quicker

hmcts-jenkins-j-to-z Bot deployed to preview July 21, 2026 12:21 View deployment

tvr-hmcts approved these changes Jul 21, 2026

View reviewed changes

m014389b added the enable_e2e_test Currently used to test the common e2e test call label Jul 21, 2026

Merge branch 'master' into HDPI-6650_refactor_org_service_to_cache_re…

cda9de3

…sponses

hmcts-jenkins-j-to-z Bot deployed to preview July 21, 2026 12:53 View deployment

guygrewal77 reviewed Jul 21, 2026

View reviewed changes

HDPI-7834: convert internal UUID primary keys to bigint identity (#2227)

3950232

hmcts-jenkins-j-to-z Bot deployed to preview July 22, 2026 09:08 View deployment

HDPI-6650: use auth token user id rather than signed in user (as not …

67c94bd

…signed in) via postman

hmcts-jenkins-j-to-z Bot deployed to preview July 22, 2026 11:17 View deployment

Merge branch 'master' into HDPI-6650_refactor_org_service_to_cache_re…

68e2f40

…sponses

hmcts-jenkins-j-to-z Bot deployed to preview July 22, 2026 12:25 View deployment

HDPI-6650: use utc clock for localdatetime.now

4abfef7

hmcts-jenkins-j-to-z Bot deployed to preview July 22, 2026 13:35 View deployment

guygrewal77 approved these changes Jul 23, 2026

View reviewed changes

Merge branch 'master' into HDPI-6650_refactor_org_service_to_cache_re…

1d54787

…sponses

hmcts-jenkins-j-to-z Bot deployed to preview July 24, 2026 14:35 View deployment


		CachedOrganisationResponseEntity newCachedResponse = mapResponseToEntity(userIdam, response);

		cachedOrganisationResponseRepository.save(newCachedResponse);

Uh oh!

Conversation

m014389b commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jira link

Change description

Testing done

Security Vulnerability Assessment

Checklist

Uh oh!

github-actions Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DB breaking change scan

Uh oh!

github-actions Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CCD diff summary

Uh oh!

tvr-hmcts left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvr-hmcts left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tvr-hmcts left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

m014389b commented Jul 15, 2026 •

edited

Loading

github-actions Bot commented Jul 15, 2026 •

edited

Loading

github-actions Bot commented Jul 15, 2026 •

edited

Loading