Merge pull request #2444 from 4Science/DS-4269

DS-4269 Incorrect hal pagination links in the browses endpoint
This commit is contained in:
Andrea Bollini
2019-06-20 17:06:46 +02:00
committed by GitHub
3 changed files with 75 additions and 3 deletions

View File

@@ -832,7 +832,7 @@ public class RestResourceController implements InitializingBean {
link = linkTo(this.getClass(), apiCategory, model).slash(uuid)
.slash(subpath + '?' + querystring).withSelfRel();
} else {
link = linkTo(this.getClass(), apiCategory, model).slash(uuid).withSelfRel();
link = linkTo(this.getClass(), apiCategory, model).slash(uuid).slash(subpath).withSelfRel();
}
Page<HALResource> halResources = pageResult.map(linkRepository::wrapResource);

View File

@@ -54,13 +54,13 @@ public class BrowseEntryHalLinkFactory extends HalLinkFactory<BrowseEntryResourc
return BrowseEntryResource.class;
}
// TODO use the reflaction to discover the link repository and additional information on the link annotation to
// TODO use the reflection to discover the link repository and additional information on the link annotation to
// build the parameters?
private UriComponentsBuilder addFilterParams(UriComponentsBuilder uriComponentsBuilder,
final BrowseEntryRest data) {
UriComponentsBuilder result;
if (data.getAuthority() != null) {
result = uriComponentsBuilder.queryParam("filterValue", data.getAuthority());
result = uriComponentsBuilder.queryParam("filterAuthority", data.getAuthority());
} else {
result = uriComponentsBuilder.queryParam("filterValue", data.getValue());
}

View File

@@ -387,6 +387,78 @@ public class BrowsesResourceControllerIT extends AbstractControllerIntegrationTe
not(matchMetadata("dc.title", "Internal publication")))));
}
@Test
/**
* This test was introduced to reproduce the bug DS-4269 Pagination links must be consistent also when there is not
* explicit pagination parameters in the request (i.e. defaults apply)
*
* @throws Exception
*/
public void browsePaginationWithoutExplicitParams() throws Exception {
context.turnOffAuthorisationSystem();
//** GIVEN **
//1. A community-collection structure with one parent community and one collection.
parentCommunity = CommunityBuilder.createCommunity(context)
.withName("Parent Community")
.build();
Collection col1 = CollectionBuilder.createCollection(context, parentCommunity).withName("Collection 1").build();
//2. Twenty-one public items that are readable by Anonymous
for (int i = 0; i <= 20; i++) {
ItemBuilder.createItem(context, col1)
.withTitle("Public item " + String.format("%02d", i))
.withIssueDate("2017-10-17")
.withAuthor("Test, Author" + String.format("%02d", i))
.withSubject("Java").withSubject("Unit Testing")
.build();
}
context.restoreAuthSystemState();
//** WHEN **
//An anonymous user browses the items in the Browse by item endpoint
getClient().perform(get("/api/discover/browses/title/items"))
//** THEN **
//The status has to be 200 OK
.andExpect(status().isOk())
//We expect the content type to be "application/hal+json;charset=UTF-8"
.andExpect(content().contentType(contentType))
//We expect 21 public items
.andExpect(jsonPath("$.page.size", is(20)))
.andExpect(jsonPath("$.page.totalElements", is(21)))
.andExpect(jsonPath("$.page.totalPages", is(2)))
.andExpect(jsonPath("$.page.number", is(0)))
// embedded items are already checked by other test, we focus on links here
.andExpect(jsonPath("$._links.next.href", Matchers.containsString("/api/discover/browses/title/items?")))
.andExpect(jsonPath("$._links.last.href", Matchers.containsString("/api/discover/browses/title/items?")))
.andExpect(
jsonPath("$._links.first.href", Matchers.containsString("/api/discover/browses/title/items?")))
.andExpect(jsonPath("$._links.self.href", Matchers.endsWith("/api/discover/browses/title/items")));
//** WHEN **
//An anonymous user browses the items in the Browse by item endpoint
getClient().perform(get("/api/discover/browses/author/entries"))
//** THEN **
//The status has to be 200 OK
.andExpect(status().isOk())
//We expect the content type to be "application/hal+json;charset=UTF-8"
.andExpect(content().contentType(contentType))
//We expect 21 public items
.andExpect(jsonPath("$.page.size", is(20)))
.andExpect(jsonPath("$.page.totalElements", is(21)))
.andExpect(jsonPath("$.page.totalPages", is(2)))
.andExpect(jsonPath("$.page.number", is(0)))
// embedded items are already checked by other test, we focus on links here
.andExpect(jsonPath("$._links.next.href",
Matchers.containsString("/api/discover/browses/author/entries?")))
.andExpect(jsonPath("$._links.last.href",
Matchers.containsString("/api/discover/browses/author/entries?")))
.andExpect(jsonPath("$._links.first.href",
Matchers.containsString("/api/discover/browses/author/entries?")))
.andExpect(jsonPath("$._links.self.href", Matchers.endsWith("/api/discover/browses/author/entries")));
}
@Test
public void testPaginationBrowseByDateIssuedItems() throws Exception {
context.turnOffAuthorisationSystem();