Skip to content

Commit 1778714

Browse files
kuchtiak-ufalMilan Kuchtiak
kuchtiak-ufal
and
Milan Kuchtiak
authored
Issue 1165: Handle PaginantionException in Clarin licences search requests (#1184)
* Issue 1165: Handle PaginantionException in Clarin licences search requests * resolve MR comments --------- Co-authored-by: Milan Kuchtiak <mkuchtiak@Milans-MacBook-Pro.local>
1 parent f51de74 commit 1778714

2 files changed

Lines changed: 77 additions & 27 deletions

File tree

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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.dspace.app.rest.Parameter;
2626
import org.dspace.app.rest.SearchRestMethod;
2727
import org.dspace.app.rest.exception.DSpaceBadRequestException;
28+
import org.dspace.app.rest.exception.PaginationException;
2829
import org.dspace.app.rest.exception.UnprocessableEntityException;
2930
import org.dspace.app.rest.model.ClarinLicenseLabelRest;
3031
import org.dspace.app.rest.model.ClarinLicenseRest;
@@ -43,6 +44,7 @@
4344
import org.dspace.core.Context;
4445
import org.springframework.beans.factory.annotation.Autowired;
4546
import org.springframework.data.domain.Page;
47+
import org.springframework.data.domain.PageImpl;
4648
import org.springframework.data.domain.Pageable;
4749
import org.springframework.data.rest.webmvc.ResourceNotFoundException;
4850
import org.springframework.security.access.prepost.PreAuthorize;
@@ -103,33 +105,44 @@ public Page<ClarinLicenseRest> findByName(@Parameter(value = "name", required =
103105
throw new RuntimeException(e.getMessage(), e);
104106
}
105107
clarinLicenseList.add(clarinLicense);
106-
return converter.toRestPage(clarinLicenseList, pageable, utils.obtainProjection());
108+
try {
109+
return converter.toRestPage(clarinLicenseList, pageable, utils.obtainProjection());
110+
} catch (PaginationException pe) {
111+
return getEmptyPageForIncorrectPageable(pageable, clarinLicenseList.size());
112+
}
107113
}
108114

109115
@SearchRestMethod(name = "byNameLike")
110116
public Page<ClarinLicenseRest> findByNameLike(@Parameter(value = "name", required = true) String name,
111-
Pageable pageable) {
112-
List<ClarinLicense> clarinLicenseList;
117+
Pageable pageable) {
118+
List<ClarinLicense> clarinLicenseList = new ArrayList<>();
113119
try {
114120
Context context = obtainContext();
115-
clarinLicenseList = clarinLicenseService.findByNameLike(context, name);
116-
if (CollectionUtils.isEmpty(clarinLicenseList)) {
121+
clarinLicenseList.addAll(clarinLicenseService.findByNameLike(context, name));
122+
if (clarinLicenseList.isEmpty()) {
117123
return null;
118124
}
119125
} catch (SQLException e) {
120126
throw new RuntimeException(e.getMessage(), e);
121127
}
122-
return converter.toRestPage(clarinLicenseList, pageable, utils.obtainProjection());
128+
try {
129+
return converter.toRestPage(clarinLicenseList, pageable, utils.obtainProjection());
130+
} catch (PaginationException pe) {
131+
return getEmptyPageForIncorrectPageable(pageable, clarinLicenseList.size());
132+
}
123133
}
124134

125135
@Override
126136
@PreAuthorize("permitAll()")
127137
public Page<ClarinLicenseRest> findAll(Context context, Pageable pageable) {
138+
List<ClarinLicense> clarinLicenseList = new ArrayList<>();
128139
try {
129-
List<ClarinLicense> clarinLicenseList = clarinLicenseService.findAll(context);
140+
clarinLicenseList.addAll(clarinLicenseService.findAll(context));
130141
return converter.toRestPage(clarinLicenseList, pageable, utils.obtainProjection());
131142
} catch (SQLException | AuthorizeException e) {
132143
throw new RuntimeException(e.getMessage(), e);
144+
} catch (PaginationException pe) {
145+
return getEmptyPageForIncorrectPageable(pageable, clarinLicenseList.size());
133146
}
134147
}
135148

@@ -273,4 +286,8 @@ private ClarinLicenseLabel getClarinLicenseLabelFromRest(ClarinLicenseLabelRest
273286
return clarinLicenseLabel;
274287
}
275288

289+
private <R> Page<R> getEmptyPageForIncorrectPageable(Pageable pageable, int totalElements) {
290+
return new PageImpl<>(List.of(), pageable, totalElements);
291+
}
292+
276293
}

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

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import static com.jayway.jsonpath.JsonPath.read;
1111
import static org.apache.commons.codec.CharEncoding.UTF_8;
1212
import static org.apache.commons.io.IOUtils.toInputStream;
13+
import static org.dspace.app.rest.utils.Utils.DEFAULT_PAGE_SIZE;
1314
import static org.hamcrest.Matchers.is;
1415
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
1516
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
@@ -54,6 +55,7 @@
5455
import org.junit.Before;
5556
import org.junit.Test;
5657
import org.springframework.beans.factory.annotation.Autowired;
58+
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
5759

5860
/**
5961
* Integration tests for the Clarin License Rest Repository
@@ -62,6 +64,8 @@
6264
*/
6365
public class ClarinLicenseRestRepositoryIT extends AbstractControllerIntegrationTest {
6466

67+
private static final String BASE_URI = "/api/core/clarinlicenses";
68+
6569
@Autowired
6670
ClarinLicenseService clarinLicenseService;
6771

@@ -188,7 +192,7 @@ public void clarinLicensesAndLicenseLabelsAreInitialized() throws Exception {
188192
@Test
189193
public void findAll() throws Exception {
190194
String authTokenAdmin = getAuthToken(admin.getEmail(), password);
191-
getClient(authTokenAdmin).perform(get("/api/core/clarinlicenses"))
195+
getClient(authTokenAdmin).perform(get(BASE_URI))
192196
.andExpect(status().isOk())
193197
.andExpect(content().contentType(contentType))
194198
.andExpect(jsonPath("$._embedded.clarinlicenses", Matchers.hasItem(
@@ -207,8 +211,7 @@ public void findAll() throws Exception {
207211
Objects.requireNonNull(getExtendedLicenseLabels(
208212
firstCLicense.getLicenseLabels())))
209213
)))
210-
.andExpect(jsonPath("$._links.self.href",
211-
Matchers.containsString("/api/core/clarinlicenses")));
214+
.andExpect(jsonPath("$._links.self.href", Matchers.containsString(BASE_URI)));
212215
}
213216

214217
/**
@@ -217,7 +220,7 @@ public void findAll() throws Exception {
217220
@Test
218221
public void searchBy() throws Exception {
219222
String authTokenAdmin = getAuthToken(admin.getEmail(), password);
220-
getClient(authTokenAdmin).perform(get("/api/core/clarinlicenses/search/byName")
223+
getClient(authTokenAdmin).perform(get(BASE_URI + "/search/byName")
221224
.param("name", "CL Name1"))
222225
.andExpect(status().isOk())
223226
.andExpect(content().contentType(contentType))
@@ -232,7 +235,7 @@ public void searchBy() throws Exception {
232235
@Test
233236
public void searchByLike() throws Exception {
234237
String authTokenAdmin = getAuthToken(admin.getEmail(), password);
235-
getClient(authTokenAdmin).perform(get("/api/core/clarinlicenses/search/byNameLike")
238+
getClient(authTokenAdmin).perform(get(BASE_URI + "/search/byNameLike")
236239
.param("name", "Name"))
237240
.andExpect(status().isOk())
238241
.andExpect(content().contentType(contentType))
@@ -252,8 +255,17 @@ public void searchByLike() throws Exception {
252255
Objects.requireNonNull(getExtendedLicenseLabels(
253256
firstCLicense.getLicenseLabels())))
254257
)))
255-
.andExpect(jsonPath("$._links.self.href",
256-
Matchers.containsString("/api/core/clarinlicenses")));
258+
.andExpect(jsonPath("$._links.self.href", Matchers.containsString(BASE_URI)));
259+
}
260+
261+
/**
262+
* This method tests requests for incorrect page parameter (e.g. page=5).
263+
*/
264+
@Test
265+
public void searchWithIncorrectPageParameter() throws Exception {
266+
testRequestWithIncorrectPageable("", null, 2, 10);
267+
testRequestWithIncorrectPageable("/search/byName", "CL Name1", 1, 20);
268+
testRequestWithIncorrectPageable("/search/byNameLike", "Name", 2, 30);
257269
}
258270

259271
@Test
@@ -276,7 +288,7 @@ public void create() throws Exception {
276288
AtomicReference<Integer> idRef = new AtomicReference<>();
277289
String authTokenAdmin = getAuthToken(admin.getEmail(), password);
278290
try {
279-
getClient(authTokenAdmin).perform(post("/api/core/clarinlicenses")
291+
getClient(authTokenAdmin).perform(post(BASE_URI)
280292
.content(new ObjectMapper().writeValueAsBytes(clarinLicenseRest))
281293
.contentType(org.springframework.http.MediaType.APPLICATION_JSON))
282294
.andExpect(status().isCreated())
@@ -352,15 +364,15 @@ public void update() throws Exception {
352364
ClarinLicenseRest clarinLicenseRest = clarinLicenseConverter.convert(clarinLicenseUpdated, Projection.DEFAULT);
353365

354366
String authTokenAdmin = getAuthToken(admin.getEmail(), password);
355-
getClient(authTokenAdmin).perform(get("/api/core/clarinlicenses/" + clarinLicense.getID()))
367+
getClient(authTokenAdmin).perform(get(BASE_URI + "/" + clarinLicense.getID()))
356368
.andExpect(status().isOk());
357369

358-
getClient(authTokenAdmin).perform(put("/api/core/clarinlicenses/" + clarinLicense.getID())
370+
getClient(authTokenAdmin).perform(put(BASE_URI + "/" + clarinLicense.getID())
359371
.content(new ObjectMapper().writeValueAsBytes(clarinLicenseRest))
360372
.contentType(org.springframework.http.MediaType.APPLICATION_JSON))
361373
.andExpect(status().isOk());
362374

363-
getClient(authTokenAdmin).perform(get("/api/core/clarinlicenses/" + clarinLicense.getID()))
375+
getClient(authTokenAdmin).perform(get(BASE_URI + "/" + clarinLicense.getID()))
364376
.andExpect(status().isOk())
365377
.andExpect(jsonPath("$", Matchers.is(
366378
ClarinLicenseMatcher.matchClarinLicenseWithoutId(clarinLicenseUpdated))
@@ -387,7 +399,7 @@ public void forbiddenUpdateClarinLicense() throws Exception {
387399

388400
ClarinLicenseRest clarinLicenseRest = clarinLicenseConverter.convert(clarinLicense, Projection.DEFAULT);
389401
String authTokenUser = getAuthToken(eperson.getEmail(), password);
390-
getClient(authTokenUser).perform(delete("/api/core/clarinlicenses/" + clarinLicense.getID())
402+
getClient(authTokenUser).perform(delete(BASE_URI + "/" + clarinLicense.getID())
391403
.content(new ObjectMapper().writeValueAsBytes(clarinLicenseRest))
392404
.contentType(org.springframework.http.MediaType.APPLICATION_JSON))
393405
.andExpect(status().isForbidden())
@@ -415,7 +427,7 @@ public void notFoundUpdateClarinLicense() throws Exception {
415427
ClarinLicenseRest clarinLicenseRest = clarinLicenseConverter.convert(clarinLicense, Projection.DEFAULT);
416428

417429
String authTokenAdmin = getAuthToken(admin.getEmail(), password);
418-
getClient(authTokenAdmin).perform(put("/api/core/clarinlicenses/" + clarinLicense.getID() + "124679")
430+
getClient(authTokenAdmin).perform(put(BASE_URI + "/" + clarinLicense.getID() + "124679")
419431
.content(new ObjectMapper().writeValueAsBytes(clarinLicenseRest))
420432
.contentType(org.springframework.http.MediaType.APPLICATION_JSON))
421433
.andExpect(status().isNotFound())
@@ -430,13 +442,13 @@ public void deleteClarinLicense() throws Exception {
430442
context.restoreAuthSystemState();
431443

432444
String authTokenAdmin = getAuthToken(admin.getEmail(), password);
433-
getClient(authTokenAdmin).perform(get("/api/core/clarinlicenses/" + clarinLicense.getID()))
445+
getClient(authTokenAdmin).perform(get(BASE_URI + "/" + clarinLicense.getID()))
434446
.andExpect(status().isOk());
435447

436-
getClient(authTokenAdmin).perform(delete("/api/core/clarinlicenses/" + clarinLicense.getID()))
448+
getClient(authTokenAdmin).perform(delete(BASE_URI + "/" + clarinLicense.getID()))
437449
.andExpect(status().isNoContent());
438450

439-
getClient(authTokenAdmin).perform(get("/api/core/clarinlicenses/" + clarinLicense.getID()))
451+
getClient(authTokenAdmin).perform(get(BASE_URI + "/" + clarinLicense.getID()))
440452
.andExpect(status().isNotFound());
441453
}
442454

@@ -447,7 +459,7 @@ public void unauthorizedDeleteClarinLicense() throws Exception {
447459
ClarinLicense clarinLicense = ClarinLicenseBuilder.createClarinLicense(context).build();
448460
context.restoreAuthSystemState();
449461

450-
getClient().perform(delete("/api/core/clarinlicenses/" + clarinLicense.getID()))
462+
getClient().perform(delete(BASE_URI + "/" + clarinLicense.getID()))
451463
.andExpect(status().isUnauthorized())
452464
;
453465
}
@@ -460,7 +472,7 @@ public void forbiddenDeleteClarinLicense() throws Exception {
460472
context.restoreAuthSystemState();
461473

462474
String authTokenUser = getAuthToken(eperson.getEmail(), password);
463-
getClient(authTokenUser).perform(delete("/api/core/clarinlicenses/" + clarinLicense.getID()))
475+
getClient(authTokenUser).perform(delete(BASE_URI + "/" + clarinLicense.getID()))
464476
.andExpect(status().isForbidden())
465477
;
466478
}
@@ -469,7 +481,7 @@ public void forbiddenDeleteClarinLicense() throws Exception {
469481
@Test
470482
public void notFoundDeleteClarinLicense() throws Exception {
471483
String authTokenAdmin = getAuthToken(admin.getEmail(), password);
472-
getClient(authTokenAdmin).perform(delete("/api/core/clarinlicenses/" + 1239990))
484+
getClient(authTokenAdmin).perform(delete(BASE_URI + "/" + 1239990))
473485
.andExpect(status().isNotFound())
474486
;
475487
}
@@ -489,7 +501,7 @@ public void findAllBitstreamsAttachedToLicense() throws Exception {
489501

490502
String tokenAdmin = getAuthToken(admin.getEmail(), password);
491503
// Check if the Clarin License was attached to the Bitstreams
492-
getClient(tokenAdmin).perform(get("/api/core/clarinlicenses/" + firstCLicense.getID()))
504+
getClient(tokenAdmin).perform(get(BASE_URI + "/" + firstCLicense.getID()))
493505
.andExpect(status().isOk())
494506
.andExpect(jsonPath("$.bitstreams", is(2)));
495507
}
@@ -514,5 +526,26 @@ private ClarinLicenseLabel getExtendedLicenseLabels(List<ClarinLicenseLabel> cla
514526
return null;
515527
}
516528

529+
private void testRequestWithIncorrectPageable(String uri,
530+
String nameValue,
531+
int expectedTotal,
532+
int pageNumber) throws Exception {
533+
String authTokenAdmin = getAuthToken(admin.getEmail(), password);
534+
MockHttpServletRequestBuilder mockBuilder = get(BASE_URI + uri)
535+
.param("page", String.valueOf(pageNumber));
536+
if (nameValue != null) {
537+
mockBuilder = mockBuilder.param("name", nameValue);
538+
}
539+
540+
getClient(authTokenAdmin).perform(mockBuilder)
541+
.andExpect(status().isOk())
542+
.andExpect(content().contentType(contentType))
543+
.andExpect(jsonPath("$._embedded").doesNotExist())
544+
.andExpect(jsonPath("$._links.self.href", Matchers.containsString(BASE_URI)))
545+
.andExpect(jsonPath("$.page.size", Matchers.equalTo(DEFAULT_PAGE_SIZE)))
546+
.andExpect(jsonPath("$.page.totalElements", Matchers.equalTo(expectedTotal)))
547+
.andExpect(jsonPath("$.page.totalPages", Matchers.equalTo(1)))
548+
.andExpect(jsonPath("$.page.number", Matchers.equalTo(pageNumber)));
549+
}
517550

518551
}

0 commit comments

Comments
 (0)