From edb94f5234e8eb7a1c7b4f25096fec283cdaba39 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Tue, 18 Feb 2025 10:28:27 -0500 Subject: [PATCH] [BI-2047] - added correct defaults --- .../brapi/v2/BrAPIObservationsController.java | 26 +++++++++++-------- .../brapi/v2/dao/BrAPIObservationDAO.java | 2 +- ...ObservationsControllerIntegrationTest.java | 12 ++++++--- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java index 4c7127528..4611fd34f 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java @@ -150,38 +150,42 @@ public HttpResponse observationsGet(@PathVariable("programId") UUID programId, // Get a filtered list of observations. List observations = observationDAO.getObservationsByFilters(program.get(), studyDbId); + // If page is not provided, set it to the default value 0. + if (page == null) page = 0; + // If pageSize is not provided, set it to the default value 1000. + if (pageSize == null) pageSize = 1000; + // Total number of records in the unpaged super set. int totalCount = observations.size(); - // Zero-indexed page, default to zero. - int actualPage = page != null ? page : 0; - // The least of pageSize and totalCount, unless pageSize is null or zero, in which case use totalCount. - int requestedPageSize = (pageSize != null && pageSize > 0) ? Math.min(pageSize, totalCount) : totalCount; + // The least of pageSize and totalCount, unless pageSize is zero, in which case use totalCount. + int requestedPageSize = pageSize > 0 ? Math.min(pageSize, totalCount) : totalCount; // Integer division and round up. int totalPages = totalCount / requestedPageSize + ((totalCount % requestedPageSize == 0) ? 0 : 1); - log.info("(Pagination) totalCount: " + totalCount + " actualPage (0-indexed): " + actualPage + " requestedPageSize: " + requestedPageSize + " totalPages: " + totalPages); + log.info("(Pagination) totalCount: " + totalCount + " page (0-indexed): " + page + " requestedPageSize: " + requestedPageSize + " totalPages: " + totalPages); // Determine validity of pagination query parameters. - boolean pageSizeValid = pageSize != null && pageSize > 0; - boolean pageValid = page != null && page >= 0 && page < totalPages; + boolean pageSizeValid = pageSize > 0; + boolean pageValid = page >= 0 && page < totalPages; // Only paginate if valid pagination values were sent. if (pageSizeValid && pageValid) { - int start = actualPage * requestedPageSize; + int start = page * requestedPageSize; // Account for last page, which may have fewer than requestedPageSize items, or exactly requestedPageSize items. - int end = (actualPage == (totalPages - 1) && totalCount % requestedPageSize != 0) ? (start + (totalCount % requestedPageSize)) : Math.min(((actualPage + 1) * requestedPageSize), totalCount); + int end = (page == (totalPages - 1) && totalCount % requestedPageSize != 0) ? (start + (totalCount % requestedPageSize)) : Math.min(((page + 1) * requestedPageSize), totalCount); log.info("(Pagination) start " + start + " end " + end); // Sort observations so that paging is consistent and coherent. observations.sort(Comparator.comparing(BrAPIObservation::getObservationDbId)); // Paginate response. observations = observations.subList(start, end); - } else if (pageSize != null || page != null) { + } else { // If one or more of the pagination query parameters are not null, both must be present and valid. String errorMessage = "Invalid query parameters: page, pageSize"; return HttpResponse.badRequest(new BrAPIObservationListResponse().metadata(new BrAPIMetadata().status(List.of(new BrAPIStatus().messageType(BrAPIStatus.MessageTypeEnum.ERROR) .message(errorMessage))))); } - return HttpResponse.ok(new BrAPIObservationListResponse().metadata(new BrAPIMetadata().pagination(new BrAPIIndexPagination().currentPage(actualPage) + return HttpResponse.ok(new BrAPIObservationListResponse().metadata(new BrAPIMetadata().pagination(new BrAPIIndexPagination() + .currentPage(page) .totalPages(totalPages) .pageSize(observations.size()) .totalCount(totalCount))) diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java index f68024184..e9d2f788f 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java @@ -259,7 +259,7 @@ public List getObservationsByFilters(Program program, String s // Build a hashmap of traits for fast lookup. The key is ObservationVariableDbId, the value is the Trait Id. HashMap traitIdsByObservationVariableDbId = traitService.getIdsByObservationVariableDbIds(program.getId(), observations.stream().map(BrAPIObservation::getObservationVariableDbId).collect(Collectors.toList())); - // Lookup studyDbId + // Lookup studyDbId. return observations.stream() .filter(o -> { // Short circuit if filter is null. diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationsControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationsControllerIntegrationTest.java index 12269df3e..27cf6defd 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationsControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationsControllerIntegrationTest.java @@ -341,6 +341,9 @@ public void testGetObsPagination() { // Check no pagination. checkPagination(null, null, HttpStatus.OK, 4, 4, 1); + // Check page and pageSize defaults. + checkPagination(null, 2, HttpStatus.OK, 2, 4, 2); + checkPagination(0, null, HttpStatus.OK, 4, 4, 1); // Check valid pagination, including last page edge case. checkPagination(0, 1, HttpStatus.OK, 1, 4, 4); checkPagination(1, 2, HttpStatus.OK, 2, 4, 2); @@ -360,10 +363,10 @@ private void checkPagination(Integer page, Integer pageSize, HttpStatus expected String requestURL = String.format("/programs/%s/brapi/v2/observations", program.getId()); if (page != null) { requestURL = requestURL + "?page=" + page; - } else { - page = 0; } - if (pageSize != null) { + if (pageSize != null && page == null) { + requestURL = requestURL + "?pageSize=" + pageSize; + } else if (pageSize != null) { requestURL = requestURL + "&pageSize=" + pageSize; } @@ -386,7 +389,8 @@ private void checkPagination(Integer page, Integer pageSize, HttpStatus expected // Get metadata. JsonObject pagination = responseObj.getAsJsonObject("metadata").getAsJsonObject("pagination"); assertEquals(expectedSize, pagination.get("pageSize").getAsInt()); - assertEquals(page, pagination.get("currentPage").getAsInt()); + int expectedPage = page == null ? 0 : page; + assertEquals(expectedPage, pagination.get("currentPage").getAsInt()); assertEquals(expectedTotalPages, pagination.get("totalPages").getAsInt()); assertEquals(expectedTotalCount, pagination.get("totalCount").getAsInt()); // Get observations.