From 1fe20a7f62c3953fa4bbbece9640125500efa96d Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Wed, 26 Aug 2026 13:31:34 +0200 Subject: [PATCH 1/2] fix(discovery): keep the requested scope and configuration in the /discover/search self link /api/discover/search built its self link from a SearchConfigurationRest that never received the scope and configuration the request carried, so the link described a different request than the one it answered. The frontend then logged a self-link mismatch warning for every scoped or configured search. - DiscoverConfigurationConverter: convert() now takes the requested configuration name and scope and sets them on SearchConfigurationRest, the way DiscoverFacetConfigurationConverter already does for /discover/facets. Both fields are @JsonIgnore, so only _links.self changes -- the JSON body is untouched. The class no longer implements DSpaceConverter and getModelClass() is gone, because the 4-arg signature does not fit that interface; nothing on dtq-dev resolves this converter through ConverterService (no toRest() or getConverter() call asks for DiscoveryConfiguration), so dropping the registry entry orphans nothing. - DiscoveryRestRepository.getSearchConfiguration(): pass both values through. - DiscoverConfigurationConverterTest: adapt the existing call sites and add testRequestedConfigurationAndScopeAreKept. Port of DSpace/DSpace PR 12984 (still open upstream), commit 6477dfe30c. Upstream tracking issue: DSpace/DSpace issue 8576. Adapted, not cherry-picked: upstream is on 11.0-SNAPSHOT and its first hunk rewrites the class header, where dtq-dev keeps an @Autowired ConfigurationService for the local sort.options.filtered handling. That field, the filtering block and the commons-lang v2 imports are left as they are. SearchConfigurationRest and SearchConfigurationResourceHalLinkFactory need no change -- 7.6 already has the @JsonIgnore scope/configuration fields and the link factory already reads them. Fixes dataquest-dev/dspace-customers#934 Co-Authored-By: Claude Opus 5 (1M context) --- .../DiscoverConfigurationConverter.java | 18 ++++----- .../repository/DiscoveryRestRepository.java | 3 +- .../DiscoverConfigurationConverterTest.java | 40 ++++++++++++++----- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/DiscoverConfigurationConverter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/DiscoverConfigurationConverter.java index 2252e394824f..68ac0729d9b5 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/DiscoverConfigurationConverter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/DiscoverConfigurationConverter.java @@ -30,16 +30,21 @@ * to the convert method. */ @Component -public class DiscoverConfigurationConverter - implements DSpaceConverter { +public class DiscoverConfigurationConverter { @Autowired ConfigurationService configurationService; - @Override - public SearchConfigurationRest convert(DiscoveryConfiguration configuration, Projection projection) { + /** + * The requested configuration name and scope are kept on the REST object so that the self link can be built + * from them, the same way {@link DiscoverFacetConfigurationConverter} does it. + */ + public SearchConfigurationRest convert(final String configurationName, final String scope, + DiscoveryConfiguration configuration, Projection projection) { SearchConfigurationRest searchConfigurationRest = new SearchConfigurationRest(); searchConfigurationRest.setProjection(projection); + searchConfigurationRest.setConfiguration(configurationName); + searchConfigurationRest.setScope(scope); if (configuration != null) { addSearchFilters(searchConfigurationRest, configuration.getSearchFilters(), configuration.getSidebarFacets()); @@ -48,11 +53,6 @@ public SearchConfigurationRest convert(DiscoveryConfiguration configuration, Pro return searchConfigurationRest; } - @Override - public Class getModelClass() { - return DiscoveryConfiguration.class; - } - public void addSearchFilters(SearchConfigurationRest searchConfigurationRest, List searchFilterList, List facetList) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java index 4b9b7d764478..e0cf3f25cbe6 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java @@ -86,7 +86,8 @@ public SearchConfigurationRest getSearchConfiguration(final String dsoScope, fin DiscoveryConfiguration discoveryConfiguration = searchConfigurationService .getDiscoveryConfigurationByNameOrIndexableObject(context, configuration, scopeObject); - return discoverConfigurationConverter.convert(discoveryConfiguration, utils.obtainProjection()); + return discoverConfigurationConverter.convert(configuration, dsoScope, discoveryConfiguration, + utils.obtainProjection()); } public SearchResultsRest getSearchObjects(final String query, final List dsoTypes, final String dsoScope, diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/DiscoverConfigurationConverterTest.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/DiscoverConfigurationConverterTest.java index eb75fbfb4f15..ae948472c58a 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/DiscoverConfigurationConverterTest.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/DiscoverConfigurationConverterTest.java @@ -61,20 +61,30 @@ public void populateDiscoveryConfigurationWithEmptyList() { @Test public void testReturnType() throws Exception { populateDiscoveryConfigurationWithEmptyList(); - searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT); + searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration, + Projection.DEFAULT); assertTrue(searchConfigurationRest.getFilters().isEmpty()); assertEquals(SearchConfigurationRest.class, searchConfigurationRest.getClass()); } @Test public void testConvertWithNullParamter() throws Exception { - assertNotNull(discoverConfigurationConverter.convert(null, Projection.DEFAULT)); + assertNotNull(discoverConfigurationConverter.convert(null, null, null, Projection.DEFAULT)); + } + + @Test + public void testRequestedConfigurationAndScopeAreKept() throws Exception { + searchConfigurationRest = discoverConfigurationConverter.convert("personOrOrgunit", "a-scope-uuid", + discoveryConfiguration, Projection.DEFAULT); + assertEquals("personOrOrgunit", searchConfigurationRest.getConfiguration()); + assertEquals("a-scope-uuid", searchConfigurationRest.getScope()); } @Test public void testNoSearchSortConfigurationReturnObjectNotNull() throws Exception { discoveryConfiguration.setSearchFilters(new LinkedList<>()); - searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT); + searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration, + Projection.DEFAULT); assertTrue(discoveryConfiguration.getSearchFilters().isEmpty()); assertTrue(searchConfigurationRest.getFilters().isEmpty()); assertNotNull(searchConfigurationRest); @@ -83,7 +93,8 @@ public void testNoSearchSortConfigurationReturnObjectNotNull() throws Exception @Test public void testNoSearchFilterReturnObjectNotNull() throws Exception { discoveryConfiguration.setSearchSortConfiguration(new DiscoverySortConfiguration()); - searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT); + searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration, + Projection.DEFAULT); assertTrue(searchConfigurationRest.getFilters().isEmpty()); assertNotNull(searchConfigurationRest); } @@ -92,7 +103,8 @@ public void testNoSearchFilterReturnObjectNotNull() throws Exception { // are null @Test public void testNoSearchSortConfigurationAndNoSearchFilterReturnObjectNotNull() throws Exception { - searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT); + searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration, + Projection.DEFAULT); assertNotNull(searchConfigurationRest); } @@ -115,7 +127,8 @@ public void testCorrectSortOptionsAfterConvert() throws Exception { when(discoveryConfiguration.getSearchSortConfiguration()).thenReturn(discoverySortConfiguration); when(discoverySortConfiguration.getSortFields()).thenReturn(mockedList); - searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT); + searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration, + Projection.DEFAULT); int counter = 0; for (SearchConfigurationRest.SortOption sortOption : searchConfigurationRest.getSortOptions()) { @@ -130,7 +143,8 @@ public void testCorrectSortOptionsAfterConvert() throws Exception { @Test public void testEmptySortOptionsAfterConvertWithConfigurationWithEmptySortFields() throws Exception { populateDiscoveryConfigurationWithEmptyList(); - searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT); + searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration, + Projection.DEFAULT); assertEquals(0, searchConfigurationRest.getSortOptions().size()); } @@ -139,7 +153,8 @@ public void testEmptySortOptionsAfterConvertWithConfigurationWithEmptySortFields public void testEmptySortOptionsAfterConvertWithConfigurationWithNullSortFields() throws Exception { populateDiscoveryConfigurationWithEmptyList(); when(discoveryConfiguration.getSearchSortConfiguration()).thenReturn(null); - searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT); + searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration, + Projection.DEFAULT); assertEquals(0, searchConfigurationRest.getSortOptions().size()); } @@ -158,7 +173,8 @@ public void testCorrectSearchFiltersAfterConvert() throws Exception { mockedList.add(discoverySearchFilter1); when(discoveryConfiguration.getSearchFilters()).thenReturn(mockedList); - searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT); + searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration, + Projection.DEFAULT); int counter = 0; for (SearchConfigurationRest.Filter filter : searchConfigurationRest.getFilters()) { @@ -176,7 +192,8 @@ public void testCorrectSearchFiltersAfterConvert() throws Exception { @Test public void testEmptySearchFilterAfterConvertWithConfigurationWithEmptySearchFilters() throws Exception { populateDiscoveryConfigurationWithEmptyList(); - searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT); + searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration, + Projection.DEFAULT); assertEquals(0, searchConfigurationRest.getFilters().size()); } @@ -185,7 +202,8 @@ public void testEmptySearchFiltersAfterConvertWithConfigurationWithNullSearchFil populateDiscoveryConfigurationWithEmptyList(); when(discoveryConfiguration.getSearchFilters()).thenReturn(null); - searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT); + searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration, + Projection.DEFAULT); assertEquals(0, searchConfigurationRest.getFilters().size()); } From 6169f68863e8e8157905ddb77170bd62b0d1eeaf Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Wed, 26 Aug 2026 13:31:45 +0200 Subject: [PATCH 2/2] test(discovery): cover the /discover/search self link in the IT that actually runs The upstream patch adds discoverSearchSelfLinkKeepsScopeAndConfigurationTest to DiscoveryRestControllerIT, but that class is @Ignore'd at class level on dtq-dev (since eb40d60ffd, unrelated facet-configuration failures), so the test would never execute here. - DiscoveryRestControllerIT: add the upstream test verbatim, at the same spot, so the file stays in step with upstream and future rebases stay clean. The class-level @Ignore is left alone; lifting it is a separate job. - ClarinDiscoveryRestControllerIT: mirror the same test, in this file's continuation-indent style. This is the fork's live copy of the discovery ITs and it runs in CI, so this is where the fix is actually verified. The test asserts only on $._links.self.href, so it is independent of the fork's discovery.xml facet/sort customisations. Fixes dataquest-dev/dspace-customers#934 Co-Authored-By: Claude Opus 5 (1M context) --- .../rest/ClarinDiscoveryRestControllerIT.java | 27 +++++++++++++++++++ .../app/rest/DiscoveryRestControllerIT.java | 27 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinDiscoveryRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinDiscoveryRestControllerIT.java index 49bb211f5e08..91b5923e43fb 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinDiscoveryRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinDiscoveryRestControllerIT.java @@ -1021,6 +1021,33 @@ public void discoverSearchTest() throws Exception { ))); } + @Test + public void discoverSearchSelfLinkKeepsScopeAndConfigurationTest() throws Exception { + context.turnOffAuthorisationSystem(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + + context.restoreAuthSystemState(); + + //The self link has to describe the request it answers + getClient().perform(get("/api/discover/search") + .param("scope", parentCommunity.getID().toString()) + .param("configuration", "personOrOrgunit")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._links.self.href", + containsString("scope=" + parentCommunity.getID()))) + .andExpect(jsonPath("$._links.self.href", + containsString("configuration=personOrOrgunit"))); + + //And it may not add parameters the request did not have + getClient().perform(get("/api/discover/search")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._links.self.href", not(containsString("scope=")))) + .andExpect(jsonPath("$._links.self.href", not(containsString("configuration=")))); + } + @Test public void checkSortOrderInPersonOrOrgunitConfigurationTest() throws Exception { getClient().perform(get("/api/discover/search") diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java index 7bf4fb1d6c17..3b0c7e4643cb 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java @@ -1265,6 +1265,33 @@ public void discoverSearchTest() throws Exception { .andExpect(jsonPath("$.sortOptions", contains(allExpectedSortFields))); } + @Test + public void discoverSearchSelfLinkKeepsScopeAndConfigurationTest() throws Exception { + context.turnOffAuthorisationSystem(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + + context.restoreAuthSystemState(); + + //The self link has to describe the request it answers + getClient().perform(get("/api/discover/search") + .param("scope", parentCommunity.getID().toString()) + .param("configuration", "personOrOrgunit")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._links.self.href", + containsString("scope=" + parentCommunity.getID()))) + .andExpect(jsonPath("$._links.self.href", + containsString("configuration=personOrOrgunit"))); + + //And it may not add parameters the request did not have + getClient().perform(get("/api/discover/search")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._links.self.href", not(containsString("scope=")))) + .andExpect(jsonPath("$._links.self.href", not(containsString("configuration=")))); + } + @Test public void checkSortOrderInPersonOrOrgunitConfigurationTest() throws Exception { getClient().perform(get("/api/discover/search")