Skip to content

Commit f27d932

Browse files
milanmajchrakclaude
milanmajchrak
and
claude
authored
[Port to dtq-dev] fix: Keep the requested scope and configuration in the /discover/search self link (#1421)
* 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 6477dfe. 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) <noreply@anthropic.com> * 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 eb40d60, 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 23f3786 commit f27d932

5 files changed

Lines changed: 94 additions & 21 deletions

File tree

‎dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/DiscoverConfigurationConverter.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,21 @@
3030
* to the convert method.
3131
*/
3232
@Component
33-
public class DiscoverConfigurationConverter
34-
implements DSpaceConverter<DiscoveryConfiguration, SearchConfigurationRest> {
33+
public class DiscoverConfigurationConverter {
3534

3635
@Autowired
3736
ConfigurationService configurationService;
3837

39-
@Override
40-
public SearchConfigurationRest convert(DiscoveryConfiguration configuration, Projection projection) {
38+
/**
39+
* The requested configuration name and scope are kept on the REST object so that the self link can be built
40+
* from them, the same way {@link DiscoverFacetConfigurationConverter} does it.
41+
*/
42+
public SearchConfigurationRest convert(final String configurationName, final String scope,
43+
DiscoveryConfiguration configuration, Projection projection) {
4144
SearchConfigurationRest searchConfigurationRest = new SearchConfigurationRest();
4245
searchConfigurationRest.setProjection(projection);
46+
searchConfigurationRest.setConfiguration(configurationName);
47+
searchConfigurationRest.setScope(scope);
4348
if (configuration != null) {
4449
addSearchFilters(searchConfigurationRest,
4550
configuration.getSearchFilters(), configuration.getSidebarFacets());
@@ -48,11 +53,6 @@ public SearchConfigurationRest convert(DiscoveryConfiguration configuration, Pro
4853
return searchConfigurationRest;
4954
}
5055

51-
@Override
52-
public Class<DiscoveryConfiguration> getModelClass() {
53-
return DiscoveryConfiguration.class;
54-
}
55-
5656
public void addSearchFilters(SearchConfigurationRest searchConfigurationRest,
5757
List<DiscoverySearchFilter> searchFilterList,
5858
List<DiscoverySearchFilterFacet> facetList) {

‎dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ public SearchConfigurationRest getSearchConfiguration(final String dsoScope, fin
8686
DiscoveryConfiguration discoveryConfiguration = searchConfigurationService
8787
.getDiscoveryConfigurationByNameOrIndexableObject(context, configuration, scopeObject);
8888

89-
return discoverConfigurationConverter.convert(discoveryConfiguration, utils.obtainProjection());
89+
return discoverConfigurationConverter.convert(configuration, dsoScope, discoveryConfiguration,
90+
utils.obtainProjection());
9091
}
9192

9293
public SearchResultsRest getSearchObjects(final String query, final List<String> dsoTypes, final String dsoScope,

‎dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinDiscoveryRestControllerIT.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,6 +1021,33 @@ public void discoverSearchTest() throws Exception {
10211021
)));
10221022
}
10231023

1024+
@Test
1025+
public void discoverSearchSelfLinkKeepsScopeAndConfigurationTest() throws Exception {
1026+
context.turnOffAuthorisationSystem();
1027+
1028+
parentCommunity = CommunityBuilder.createCommunity(context)
1029+
.withName("Parent Community")
1030+
.build();
1031+
1032+
context.restoreAuthSystemState();
1033+
1034+
//The self link has to describe the request it answers
1035+
getClient().perform(get("/api/discover/search")
1036+
.param("scope", parentCommunity.getID().toString())
1037+
.param("configuration", "personOrOrgunit"))
1038+
.andExpect(status().isOk())
1039+
.andExpect(jsonPath("$._links.self.href",
1040+
containsString("scope=" + parentCommunity.getID())))
1041+
.andExpect(jsonPath("$._links.self.href",
1042+
containsString("configuration=personOrOrgunit")));
1043+
1044+
//And it may not add parameters the request did not have
1045+
getClient().perform(get("/api/discover/search"))
1046+
.andExpect(status().isOk())
1047+
.andExpect(jsonPath("$._links.self.href", not(containsString("scope="))))
1048+
.andExpect(jsonPath("$._links.self.href", not(containsString("configuration="))));
1049+
}
1050+
10241051
@Test
10251052
public void checkSortOrderInPersonOrOrgunitConfigurationTest() throws Exception {
10261053
getClient().perform(get("/api/discover/search")

‎dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,6 +1265,33 @@ public void discoverSearchTest() throws Exception {
12651265
.andExpect(jsonPath("$.sortOptions", contains(allExpectedSortFields)));
12661266
}
12671267

1268+
@Test
1269+
public void discoverSearchSelfLinkKeepsScopeAndConfigurationTest() throws Exception {
1270+
context.turnOffAuthorisationSystem();
1271+
1272+
parentCommunity = CommunityBuilder.createCommunity(context)
1273+
.withName("Parent Community")
1274+
.build();
1275+
1276+
context.restoreAuthSystemState();
1277+
1278+
//The self link has to describe the request it answers
1279+
getClient().perform(get("/api/discover/search")
1280+
.param("scope", parentCommunity.getID().toString())
1281+
.param("configuration", "personOrOrgunit"))
1282+
.andExpect(status().isOk())
1283+
.andExpect(jsonPath("$._links.self.href",
1284+
containsString("scope=" + parentCommunity.getID())))
1285+
.andExpect(jsonPath("$._links.self.href",
1286+
containsString("configuration=personOrOrgunit")));
1287+
1288+
//And it may not add parameters the request did not have
1289+
getClient().perform(get("/api/discover/search"))
1290+
.andExpect(status().isOk())
1291+
.andExpect(jsonPath("$._links.self.href", not(containsString("scope="))))
1292+
.andExpect(jsonPath("$._links.self.href", not(containsString("configuration="))));
1293+
}
1294+
12681295
@Test
12691296
public void checkSortOrderInPersonOrOrgunitConfigurationTest() throws Exception {
12701297
getClient().perform(get("/api/discover/search")

‎dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/DiscoverConfigurationConverterTest.java‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,20 +61,30 @@ public void populateDiscoveryConfigurationWithEmptyList() {
6161
@Test
6262
public void testReturnType() throws Exception {
6363
populateDiscoveryConfigurationWithEmptyList();
64-
searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT);
64+
searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration,
65+
Projection.DEFAULT);
6566
assertTrue(searchConfigurationRest.getFilters().isEmpty());
6667
assertEquals(SearchConfigurationRest.class, searchConfigurationRest.getClass());
6768
}
6869

6970
@Test
7071
public void testConvertWithNullParamter() throws Exception {
71-
assertNotNull(discoverConfigurationConverter.convert(null, Projection.DEFAULT));
72+
assertNotNull(discoverConfigurationConverter.convert(null, null, null, Projection.DEFAULT));
73+
}
74+
75+
@Test
76+
public void testRequestedConfigurationAndScopeAreKept() throws Exception {
77+
searchConfigurationRest = discoverConfigurationConverter.convert("personOrOrgunit", "a-scope-uuid",
78+
discoveryConfiguration, Projection.DEFAULT);
79+
assertEquals("personOrOrgunit", searchConfigurationRest.getConfiguration());
80+
assertEquals("a-scope-uuid", searchConfigurationRest.getScope());
7281
}
7382

7483
@Test
7584
public void testNoSearchSortConfigurationReturnObjectNotNull() throws Exception {
7685
discoveryConfiguration.setSearchFilters(new LinkedList<>());
77-
searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT);
86+
searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration,
87+
Projection.DEFAULT);
7888
assertTrue(discoveryConfiguration.getSearchFilters().isEmpty());
7989
assertTrue(searchConfigurationRest.getFilters().isEmpty());
8090
assertNotNull(searchConfigurationRest);
@@ -83,7 +93,8 @@ public void testNoSearchSortConfigurationReturnObjectNotNull() throws Exception
8393
@Test
8494
public void testNoSearchFilterReturnObjectNotNull() throws Exception {
8595
discoveryConfiguration.setSearchSortConfiguration(new DiscoverySortConfiguration());
86-
searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT);
96+
searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration,
97+
Projection.DEFAULT);
8798
assertTrue(searchConfigurationRest.getFilters().isEmpty());
8899
assertNotNull(searchConfigurationRest);
89100
}
@@ -92,7 +103,8 @@ public void testNoSearchFilterReturnObjectNotNull() throws Exception {
92103
// are null
93104
@Test
94105
public void testNoSearchSortConfigurationAndNoSearchFilterReturnObjectNotNull() throws Exception {
95-
searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT);
106+
searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration,
107+
Projection.DEFAULT);
96108
assertNotNull(searchConfigurationRest);
97109
}
98110

@@ -115,7 +127,8 @@ public void testCorrectSortOptionsAfterConvert() throws Exception {
115127
when(discoveryConfiguration.getSearchSortConfiguration()).thenReturn(discoverySortConfiguration);
116128
when(discoverySortConfiguration.getSortFields()).thenReturn(mockedList);
117129

118-
searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT);
130+
searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration,
131+
Projection.DEFAULT);
119132

120133
int counter = 0;
121134
for (SearchConfigurationRest.SortOption sortOption : searchConfigurationRest.getSortOptions()) {
@@ -130,7 +143,8 @@ public void testCorrectSortOptionsAfterConvert() throws Exception {
130143
@Test
131144
public void testEmptySortOptionsAfterConvertWithConfigurationWithEmptySortFields() throws Exception {
132145
populateDiscoveryConfigurationWithEmptyList();
133-
searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT);
146+
searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration,
147+
Projection.DEFAULT);
134148
assertEquals(0, searchConfigurationRest.getSortOptions().size());
135149

136150
}
@@ -139,7 +153,8 @@ public void testEmptySortOptionsAfterConvertWithConfigurationWithEmptySortFields
139153
public void testEmptySortOptionsAfterConvertWithConfigurationWithNullSortFields() throws Exception {
140154
populateDiscoveryConfigurationWithEmptyList();
141155
when(discoveryConfiguration.getSearchSortConfiguration()).thenReturn(null);
142-
searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT);
156+
searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration,
157+
Projection.DEFAULT);
143158

144159
assertEquals(0, searchConfigurationRest.getSortOptions().size());
145160
}
@@ -158,7 +173,8 @@ public void testCorrectSearchFiltersAfterConvert() throws Exception {
158173
mockedList.add(discoverySearchFilter1);
159174
when(discoveryConfiguration.getSearchFilters()).thenReturn(mockedList);
160175

161-
searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT);
176+
searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration,
177+
Projection.DEFAULT);
162178

163179
int counter = 0;
164180
for (SearchConfigurationRest.Filter filter : searchConfigurationRest.getFilters()) {
@@ -176,7 +192,8 @@ public void testCorrectSearchFiltersAfterConvert() throws Exception {
176192
@Test
177193
public void testEmptySearchFilterAfterConvertWithConfigurationWithEmptySearchFilters() throws Exception {
178194
populateDiscoveryConfigurationWithEmptyList();
179-
searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT);
195+
searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration,
196+
Projection.DEFAULT);
180197
assertEquals(0, searchConfigurationRest.getFilters().size());
181198
}
182199

@@ -185,7 +202,8 @@ public void testEmptySearchFiltersAfterConvertWithConfigurationWithNullSearchFil
185202
populateDiscoveryConfigurationWithEmptyList();
186203

187204
when(discoveryConfiguration.getSearchFilters()).thenReturn(null);
188-
searchConfigurationRest = discoverConfigurationConverter.convert(discoveryConfiguration, Projection.DEFAULT);
205+
searchConfigurationRest = discoverConfigurationConverter.convert(null, null, discoveryConfiguration,
206+
Projection.DEFAULT);
189207

190208
assertEquals(0, searchConfigurationRest.getFilters().size());
191209
}

0 commit comments

Comments
 (0)