From a6ec8c51da89d24b5d6b46e3cb540e32f77ba6c7 Mon Sep 17 00:00:00 2001 From: kaanaktas Date: Tue, 20 Jan 2026 18:12:59 +0000 Subject: [PATCH 1/2] Remove `HalConfig` along with associated tests and clean up unused Jackson dependency. Update `CaseAssignedUserRolesController` to explicitly enforce `application/hal+json` to resolve Spring concurrency issue. --- build.gradle | 9 +- .../uk/gov/hmcts/ccd/config/HalConfig.java | 94 ----------- .../CaseAssignedUserRolesController.java | 20 ++- .../gov/hmcts/ccd/config/HalConfigTest.java | 148 ------------------ 4 files changed, 23 insertions(+), 248 deletions(-) delete mode 100644 src/main/java/uk/gov/hmcts/ccd/config/HalConfig.java delete mode 100644 src/test/java/uk/gov/hmcts/ccd/config/HalConfigTest.java diff --git a/build.gradle b/build.gradle index 62b1806117..042dbc3072 100644 --- a/build.gradle +++ b/build.gradle @@ -28,7 +28,6 @@ ext { set('spring-framework.version', '6.2.1') set('spring-security.version', '6.4.2') set('log4j2.version', '2.24.3') - set('jackson.version', '2.16.0') set('snakeyaml.version', '2.2') junit = '5.11.4' junitPlatform = '1.11.4' @@ -184,7 +183,7 @@ dependencies { // start::CVE Vulnerability dependency overrides // MAIN PARENT DEPENDEDNCY - implementation group: 'com.auth0', name: 'java-jwt', version: '4.5.0' + implementation group: 'com.auth0', name: 'java-jwt', version: '4.5.0' implementation group: 'com.google.code.gson', name: 'gson', version: '2.13.2' // java-property-to-json implementation group: 'com.google.guava', name: 'guava', version: '33.5.0-jre' // java-property-to-json implementation group: 'com.jayway.jsonpath', name: 'json-path', version: '2.10.0' // spring-hateoas @@ -210,7 +209,7 @@ dependencies { testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.27.6' // spring-cloud-starter-contract-stub-runner testImplementation group: 'org.mockito', name: 'mockito-core', version: mockito // spring-cloud-starter-contract-stub-runner testImplementation group: 'org.mockito', name: 'mockito-junit-jupiter', version: mockito // spring-boot-starter-test - + // end::CVE Vulnerability dependency overrides // Lombok @@ -270,7 +269,7 @@ dependencies { implementation group: 'org.jooq', name: 'jool-java-8', version: '0.9.15' implementation group: 'org.mapstruct', name: 'mapstruct', version: '1.6.3' implementation group: 'pl.jalokim.propertiestojson', name: 'java-properties-to-json', version: '5.3.0' - + annotationProcessor group: 'org.mapstruct', name: 'mapstruct-processor', version: '1.6.3' testAnnotationProcessor group: 'org.mapstruct', name: 'mapstruct-processor', version: '1.6.3' runtimeOnly group: 'org.postgresql', name: 'postgresql', version: '42.5.6' @@ -892,4 +891,4 @@ void loadEnvSecrets(String env) { project.file("./.${env}-remote-env").write(new String(os.toString().replace('\n', '').decodeBase64(), java.nio.charset.StandardCharsets.UTF_8)) } } -} \ No newline at end of file +} diff --git a/src/main/java/uk/gov/hmcts/ccd/config/HalConfig.java b/src/main/java/uk/gov/hmcts/ccd/config/HalConfig.java deleted file mode 100644 index 28008d3cd4..0000000000 --- a/src/main/java/uk/gov/hmcts/ccd/config/HalConfig.java +++ /dev/null @@ -1,94 +0,0 @@ -package uk.gov.hmcts.ccd.config; - -import java.util.Arrays; -import java.util.List; -import java.util.stream.Stream; - -import org.springframework.beans.factory.config.BeanPostProcessor; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.hateoas.MediaTypes; -import org.springframework.hateoas.RepresentationModel; -import org.springframework.hateoas.server.mvc.TypeConstrainedMappingJackson2HttpMessageConverter; -import org.springframework.http.MediaType; -import org.springframework.http.converter.HttpMessageConverter; -import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; -import org.springframework.web.client.RestTemplate; -import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter; - -@Configuration -public class HalConfig { - - public static final String APPLICATION_JSON_EXTENDED_VALUE = "application/*+json"; - public static final MediaType APPLICATION_JSON_EXTENDED = MediaType.valueOf(APPLICATION_JSON_EXTENDED_VALUE); - public static final String APPLICATION_JSON_EXTENDED_UTF8_VALUE = - APPLICATION_JSON_EXTENDED_VALUE + ";charset=UTF-8"; - public static final MediaType APPLICATION_JSON_EXTENDED_UTF8 = MediaType.valueOf( - APPLICATION_JSON_EXTENDED_UTF8_VALUE); - public static final String APPLICATION_HAL_JSON_EXTENDED_VALUE = "application/*+hal+json"; - public static final MediaType APPLICATION_HAL_JSON_EXTENDED = - MediaType.valueOf(APPLICATION_HAL_JSON_EXTENDED_VALUE); - public static final String APPLICATION_HAL_JSON_EXTENDED_UTF8_VALUE = - APPLICATION_HAL_JSON_EXTENDED_VALUE + ";charset=UTF-8"; - public static final MediaType APPLICATION_HAL_JSON_EXTENDED_UTF8 = MediaType.valueOf( - APPLICATION_HAL_JSON_EXTENDED_UTF8_VALUE); - - private static final MediaType[] HAL_MEDIA_TYPES = new MediaType[]{ - MediaTypes.HAL_JSON, - APPLICATION_JSON_EXTENDED, - APPLICATION_JSON_EXTENDED_UTF8, - APPLICATION_HAL_JSON_EXTENDED, - APPLICATION_HAL_JSON_EXTENDED_UTF8 - }; - - @Bean - public HalConverterPostProcessor halConverterPostProcessor() { - return new HalConverterPostProcessor(); - } - - /** - * Given Spring HATEOAS v0.25.0.RELEASE does not support HAL with vendor media types, - * those have to be enabled manually. This is adding the following media types to HAL: - * application/*+json, application/*+json;charset=UTF-8, application/*+hal+json - * and application/*+hal+json;charset=UTF-8. - */ - class HalConverterPostProcessor implements BeanPostProcessor { - - @Override - public Object postProcessBeforeInitialization(Object bean, String beanName) { - return bean; - } - - @Override - public Object postProcessAfterInitialization(Object bean, String beanName) { - - if (bean instanceof RequestMappingHandlerAdapter) { - RequestMappingHandlerAdapter adapter = (RequestMappingHandlerAdapter) bean; - enableCustomMediaTypesForHal(adapter.getMessageConverters()); - } - - if (bean instanceof RestTemplate) { - RestTemplate template = (RestTemplate) bean; - enableCustomMediaTypesForHal(template.getMessageConverters()); - } - - return bean; - } - - private void enableCustomMediaTypesForHal(List> converters) { - findHalConverters(converters) - .forEach(converter -> { - converter.setSupportedMediaTypes(Arrays.asList(HAL_MEDIA_TYPES)); - }); - } - - private Stream findHalConverters( - List> converters) { - return converters.stream() - .filter(converter -> - converter instanceof TypeConstrainedMappingJackson2HttpMessageConverter - && converter.canWrite(RepresentationModel.class, MediaTypes.HAL_JSON)) - .map(converter -> (MappingJackson2HttpMessageConverter) converter); - } - } -} diff --git a/src/main/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesController.java b/src/main/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesController.java index 6d2fb282e4..cd818a8bae 100644 --- a/src/main/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesController.java +++ b/src/main/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesController.java @@ -12,6 +12,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.hateoas.MediaTypes; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; @@ -54,7 +55,24 @@ import static uk.gov.hmcts.ccd.data.SecurityUtils.SERVICE_AUTHORIZATION; @RestController -@RequestMapping(path = "/") +/* + NOTE: Explicitly set to application/hal+json to bypass a Spring Framework concurrency problem + in AbstractJackson2HttpMessageConverter (Issue #36090). + * Although the response bodies may suppress _links (via @JsonIgnore), we strictly enforce + the HAL media type here for two critical reasons: + * 1. CRASH PREVENTION: It forces Spring to select the specific HAL converter (fast path) + instead of iterating over all converters to discover supported types. The iteration + path triggers an ArrayIndexOutOfBoundsException on a corrupted LinkedHashMap + during concurrent startup (Lazy Initialization race condition). + * 2. COMPATIBILITY: It preserves the existing Content-Type header (application/hal+json) + that clients expect, preventing contract breakage. + * WARNING: DO NOT change this to MediaType.APPLICATION_JSON_VALUE or remove it + without verifying that the upstream apps fix has been applied. + */ +@RequestMapping( + path = "/", + produces = MediaTypes.HAL_JSON_VALUE +) @ConditionalOnProperty(value = "ccd.conditional-apis.case-assigned-users-and-roles.enabled", havingValue = "true") public class CaseAssignedUserRolesController { diff --git a/src/test/java/uk/gov/hmcts/ccd/config/HalConfigTest.java b/src/test/java/uk/gov/hmcts/ccd/config/HalConfigTest.java deleted file mode 100644 index ed85a159d2..0000000000 --- a/src/test/java/uk/gov/hmcts/ccd/config/HalConfigTest.java +++ /dev/null @@ -1,148 +0,0 @@ -package uk.gov.hmcts.ccd.config; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Nested; -import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import org.springframework.hateoas.MediaTypes; -import org.springframework.hateoas.RepresentationModel; -import org.springframework.hateoas.server.mvc.TypeConstrainedMappingJackson2HttpMessageConverter; -import org.springframework.http.MediaType; -import org.springframework.http.converter.HttpMessageConverter; -import org.springframework.web.client.RestTemplate; -import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter; - -import java.util.Arrays; -import java.util.List; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.hasSize; -import static org.junit.jupiter.api.Assertions.assertAll; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.when; - -@DisplayName("HalConfig") -class HalConfigTest { - - private HalConfig halConfig; - - @BeforeEach - void setUp() { - halConfig = new HalConfig(); - } - - @Nested - @DisplayName("HalConverterPostProcessor") - class HalConverterPostProcessorTest { - private static final String BEAN_NAME = "Whatever"; - - @Mock - private RestTemplate restTemplateBean; - - @Mock - private RequestMappingHandlerAdapter requestAdapterBean; - - @Mock - private TypeConstrainedMappingJackson2HttpMessageConverter halConverter; - - @Mock - private HttpMessageConverter otherConverter; - - @Captor - private ArgumentCaptor> mediaTypesCaptor; - - private HalConfig.HalConverterPostProcessor halProcessor; - - @BeforeEach - void setUp() { - MockitoAnnotations.openMocks(this); - - final List> messageConverters = Arrays.asList(halConverter, otherConverter); - when(restTemplateBean.getMessageConverters()).thenReturn(messageConverters); - when(requestAdapterBean.getMessageConverters()).thenReturn(messageConverters); - - when(halConverter.canWrite(RepresentationModel.class, MediaTypes.HAL_JSON)).thenReturn(true); - - halProcessor = halConfig.halConverterPostProcessor(); - } - - @Test - @DisplayName("should not alter other beans") - void shouldNotAlterOtherBeans() { - final Object otherBean = mock(Object.class); - - halProcessor.postProcessBeforeInitialization(otherBean, BEAN_NAME); - halProcessor.postProcessAfterInitialization(otherBean, BEAN_NAME); - - verifyNoInteractions(otherBean); - } - - @Test - @DisplayName("should not alter RestTemplate beans before init") - void shouldNotAlterRestTemplateBeansBeforeInit() { - halProcessor.postProcessBeforeInitialization(restTemplateBean, BEAN_NAME); - - verifyNoInteractions(restTemplateBean); - } - - @Test - @DisplayName("should not alter RequestAdapter beans before init") - void shouldNotAlterRequestAdapterBeansBeforeInit() { - halProcessor.postProcessBeforeInitialization(requestAdapterBean, BEAN_NAME); - - verifyNoInteractions(requestAdapterBean); - } - - @Test - @DisplayName("should add custom media types to HAL message converter for RestTemplate bean") - void shouldAlterRestTemplateBeansAfterInit() { - halProcessor.postProcessAfterInitialization(restTemplateBean, BEAN_NAME); - - verifyMediaTypes(); - } - - @Test - @DisplayName("should add custom media types to HAL message converter for RequestAdepater bean") - void shouldAlterRequestAdepaterBeansAfterInit() { - halProcessor.postProcessAfterInitialization(restTemplateBean, BEAN_NAME); - - verifyMediaTypes(); - } - - @Test - @DisplayName("should not alter other message converters for ResTemplate bean") - void shouldNotAlterRestTemplateOtherMessageConverters() { - halProcessor.postProcessAfterInitialization(restTemplateBean, BEAN_NAME); - - verifyNoInteractions(otherConverter); - } - - @Test - @DisplayName("should not alter other message converters for RequestAdapter bean") - void shouldNotAlterRequestAdapterOtherMessageConverters() { - halProcessor.postProcessAfterInitialization(requestAdapterBean, BEAN_NAME); - - verifyNoInteractions(otherConverter); - } - - private void verifyMediaTypes() { - verify(halConverter).setSupportedMediaTypes(mediaTypesCaptor.capture()); - final List mediaTypes = mediaTypesCaptor.getValue(); - assertAll( - () -> assertThat(mediaTypes, hasSize(5)), - () -> assertThat(mediaTypes, hasItem(MediaTypes.HAL_JSON)), - () -> assertThat(mediaTypes, hasItem(HalConfig.APPLICATION_HAL_JSON_EXTENDED)), - () -> assertThat(mediaTypes, hasItem(HalConfig.APPLICATION_HAL_JSON_EXTENDED_UTF8)), - () -> assertThat(mediaTypes, hasItem(HalConfig.APPLICATION_JSON_EXTENDED)), - () -> assertThat(mediaTypes, hasItem(HalConfig.APPLICATION_JSON_EXTENDED_UTF8)) - ); - } - } -} From acf039e9d9d08758d4118937ac3f8c55fafcca2a Mon Sep 17 00:00:00 2001 From: kaanaktas Date: Tue, 17 Mar 2026 18:03:39 +0000 Subject: [PATCH 2/2] Add content negotiation tests for `CaseAssignedUserRolesController` and enable support for `application/json` in `GET /case-users` endpoint. --- .../CaseAssignedUserRolesController.java | 41 +++---- ...RolesControllerContentNegotiationTest.java | 105 ++++++++++++++++++ 2 files changed, 127 insertions(+), 19 deletions(-) create mode 100644 src/test/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesControllerContentNegotiationTest.java diff --git a/src/main/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesController.java b/src/main/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesController.java index cd818a8bae..8409d5d2a2 100644 --- a/src/main/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesController.java +++ b/src/main/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesController.java @@ -14,6 +14,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.hateoas.MediaTypes; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; @@ -55,24 +56,7 @@ import static uk.gov.hmcts.ccd.data.SecurityUtils.SERVICE_AUTHORIZATION; @RestController -/* - NOTE: Explicitly set to application/hal+json to bypass a Spring Framework concurrency problem - in AbstractJackson2HttpMessageConverter (Issue #36090). - * Although the response bodies may suppress _links (via @JsonIgnore), we strictly enforce - the HAL media type here for two critical reasons: - * 1. CRASH PREVENTION: It forces Spring to select the specific HAL converter (fast path) - instead of iterating over all converters to discover supported types. The iteration - path triggers an ArrayIndexOutOfBoundsException on a corrupted LinkedHashMap - during concurrent startup (Lazy Initialization race condition). - * 2. COMPATIBILITY: It preserves the existing Content-Type header (application/hal+json) - that clients expect, preventing contract breakage. - * WARNING: DO NOT change this to MediaType.APPLICATION_JSON_VALUE or remove it - without verifying that the upstream apps fix has been applied. - */ -@RequestMapping( - path = "/", - produces = MediaTypes.HAL_JSON_VALUE -) +@RequestMapping(path = "/") @ConditionalOnProperty(value = "ccd.conditional-apis.case-assigned-users-and-roles.enabled", havingValue = "true") public class CaseAssignedUserRolesController { @@ -214,8 +198,27 @@ public ResponseEntity removeCaseUserRoles( * `414 URI Too Long` issues, see CCD-3588. */ @Deprecated(forRemoval = true) + /* + NOTE: Explicitly set to application/hal+json to bypass a Spring Framework concurrency problem + in AbstractJackson2HttpMessageConverter (Issue #36090). + * Although the response bodies may suppress _links (via @JsonIgnore), we strictly enforce + the HAL media type here for two critical reasons: + * 1. CRASH PREVENTION: It forces Spring to select the specific HAL converter (fast path) + instead of iterating over all converters to discover supported types. The iteration + path triggers an ArrayIndexOutOfBoundsException on a corrupted LinkedHashMap + during concurrent startup (Lazy Initialization race condition). + * 2. COMPATIBILITY: It preserves the existing Content-Type header (application/hal+json) + that clients expect, preventing contract breakage. + * NOTE: GET /case-users additionally produces application/json to maintain backwards + compatibility with consuming services that send Accept: application/json. This is safe + as the response body is identical in both cases — HAL-specific _links are suppressed + via @JsonIgnore on CaseAssignedUserRolesResource. + * WARNING: DO NOT change this to MediaType.APPLICATION_JSON_VALUE or remove it + without verifying that the upstream apps fix has been applied. + */ @GetMapping( - path = "/case-users" + path = "/case-users", + produces = {MediaTypes.HAL_JSON_VALUE, MediaType.APPLICATION_JSON_VALUE} ) @Operation( summary = "Get Case-Assigned Users and Roles", diff --git a/src/test/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesControllerContentNegotiationTest.java b/src/test/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesControllerContentNegotiationTest.java new file mode 100644 index 0000000000..39dea24e54 --- /dev/null +++ b/src/test/java/uk/gov/hmcts/ccd/v2/external/controller/CaseAssignedUserRolesControllerContentNegotiationTest.java @@ -0,0 +1,105 @@ +package uk.gov.hmcts.ccd.v2.external.controller; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.springframework.hateoas.MediaTypes; +import org.springframework.http.MediaType; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import uk.gov.hmcts.ccd.ApplicationParams; +import uk.gov.hmcts.ccd.data.SecurityUtils; +import uk.gov.hmcts.ccd.domain.model.std.CaseAssignedUserRole; +import uk.gov.hmcts.ccd.domain.service.cauroles.CaseAssignedUserRolesOperation; +import uk.gov.hmcts.ccd.domain.service.common.UIDService; + +import java.util.List; + +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +class CaseAssignedUserRolesControllerContentNegotiationTest { + + private static final String CASE_ID_GOOD = "4444333322221111"; + + @Mock + private ApplicationParams applicationParams; + + @Mock + private UIDService caseReferenceService; + + @Mock + private CaseAssignedUserRolesOperation caseAssignedUserRolesOperation; + + @Mock + private SecurityUtils securityUtils; + + private MockMvc mockMvc; + + @BeforeEach + void setUp() { + MockitoAnnotations.openMocks(this); + + CaseAssignedUserRolesController controller = new CaseAssignedUserRolesController( + applicationParams, + caseReferenceService, + caseAssignedUserRolesOperation, + securityUtils + ); + + mockMvc = MockMvcBuilders + .standaloneSetup(controller) + .build(); + + when(caseReferenceService.validateUID(anyString())).thenReturn(true); + when(caseAssignedUserRolesOperation.findCaseUserRoles(anyList(), anyList())) + .thenReturn(List.of(new CaseAssignedUserRole())); + } + + @Nested + @DisplayName("GET /case-users content negotiation") + class GetCaseUserRolesContentNegotiation { + + @Test + @DisplayName("should return 200 when Accept header is application/json") + void getCaseUserRoles_shouldReturn200_whenAcceptHeaderIsApplicationJson() throws Exception { + mockMvc.perform(get("/case-users") + .param("case_ids", CASE_ID_GOOD) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()); + } + + @Test + @DisplayName("should return 200 when Accept header is application/hal+json") + void getCaseUserRoles_shouldReturn200_whenAcceptHeaderIsHalJson() throws Exception { + mockMvc.perform(get("/case-users") + .param("case_ids", CASE_ID_GOOD) + .accept(MediaTypes.HAL_JSON)) + .andExpect(status().isOk()); + } + + @Test + @DisplayName("should return 200 when Accept header is wildcard") + void getCaseUserRoles_shouldReturn200_whenAcceptHeaderIsWildcard() throws Exception { + mockMvc.perform(get("/case-users") + .param("case_ids", CASE_ID_GOOD) + .accept(MediaType.ALL)) + .andExpect(status().isOk()); + } + + @Test + @DisplayName("should return 406 when Accept header is unsupported media type") + void getCaseUserRoles_shouldReturn406_whenAcceptHeaderIsUnsupported() throws Exception { + mockMvc.perform(get("/case-users") + .param("case_ids", CASE_ID_GOOD) + .accept(MediaType.APPLICATION_XML)) + .andExpect(status().isNotAcceptable()); + } + } +}