Skip to content

BI-2964: Applied observation filtering and pagination on BrAPI side a… #566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jloux-brapi merged 2 commits into from
Oct 15, 2026
Merged

BI-2964: Applied observation filtering and pagination on BrAPI side a… #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f9ca55
BI-2964: Applied observation filtering and pagination on BrAPI side a…
humsika Oct 12, 2026
9108636
Update src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservatio…
jloux-brapi Oct 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 27 additions & 46 deletions src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.breedinginsight.brapi.v2;


import io.micronaut.http.HttpResponse;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.annotation.*;
Expand All @@ -28,13 +27,12 @@
import org.brapi.client.v2.ApiResponse;
import org.brapi.client.v2.model.exceptions.ApiException;
import org.brapi.client.v2.modules.phenotype.ObservationsApi;
import org.brapi.v2.model.BrAPIIndexPagination;
import org.brapi.v2.model.BrAPIMetadata;
import org.brapi.v2.model.BrAPIStatus;
import org.brapi.v2.model.BrAPIWSMIMEDataTypes;
import org.brapi.v2.model.core.BrAPIStudy;
import org.brapi.v2.model.pheno.BrAPIObservation;
import org.brapi.v2.model.pheno.response.*;
import org.brapi.v2.model.pheno.response.BrAPIObservationListResponse;
import org.brapi.v2.model.pheno.response.BrAPIObservationTableResponse;
import org.breedinginsight.api.auth.ProgramSecured;
import org.breedinginsight.api.auth.ProgramSecuredRoleGroup;
import org.breedinginsight.brapi.v1.controller.BrapiVersion;
Expand All @@ -46,7 +44,7 @@
import org.breedinginsight.services.ProgramService;
import org.breedinginsight.services.brapi.BrAPIEndpointProvider;
import org.breedinginsight.utilities.Utilities;

import org.breedinginsight.utilities.response.ResponseUtils;

import javax.annotation.Nullable;
import javax.inject.Inject;
Expand Down Expand Up @@ -144,54 +142,37 @@ public HttpResponse observationsGet(@PathVariable("programId") UUID programId,
);
}

// Get a filtered list of observations.
// TODO: Handle filtering and pagination of this request entirely on BrAPI side once study and ou cache is removed: [BI-2964]
List<BrAPIObservation> 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;
boolean pageSizeValid = pageSize == null || pageSize > 0;
boolean pageValid = page == null || page >= 0;

// Total number of records in the unpaged super set.
int totalCount = observations.size();
// 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 + " page (0-indexed): " + page + " requestedPageSize: " + requestedPageSize + " totalPages: " + totalPages);

// Determine validity of pagination query parameters.
boolean pageSizeValid = pageSize > 0;
boolean pageValid = page >= 0 && page < totalPages;

// Only paginate if valid pagination values were sent.
if (pageSizeValid && pageValid) {
int start = page * requestedPageSize;
// Account for last page, which may have fewer than requestedPageSize items, or exactly requestedPageSize items.
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 (!pageSizeValid || !pageValid) {
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.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(page)
.totalPages(totalPages)
.pageSize(observations.size())
.totalCount(totalCount)))
.result(new BrAPIObservationListResponseResult().data(observations)));
BrAPIObservationListResponse brAPIResponse = observationDAO.getObservationsByFilters(program.get(), studyDbId, page, pageSize);

List<BrAPIObservation> observations = brAPIResponse.getResult().getData();
return ResponseUtils.getBrapiQueryResponse(observations, brAPIResponse);

} catch (ApiException e) {
log.error(Utilities.generateApiExceptionLogMessage(e), e);
return HttpResponse.serverError(new BrAPIObservationListResponse().metadata(new BrAPIMetadata().status(List.of(new BrAPIStatus().messageType(BrAPIStatus.MessageTypeEnum.ERROR)
.message("Error fetching observations")))));

BrAPIObservationListResponse errorResponse = new BrAPIObservationListResponse()
.metadata(new BrAPIMetadata()
.status(List.of(new BrAPIStatus()
.messageType(BrAPIStatus.MessageTypeEnum.ERROR)
.message("Error fetching observations"))));

if (e.getCode() == HttpStatus.BAD_REQUEST.getCode()) {
return HttpResponse.badRequest(errorResponse);
}

return HttpResponse.serverError(errorResponse);
} catch (Exception e) {
log.error("Error fetching Observations", e);
return HttpResponse.serverError(new BrAPIObservationListResponse().metadata(new BrAPIMetadata().status(List.of(new BrAPIStatus().messageType(BrAPIStatus.MessageTypeEnum.ERROR)
Expand Down
42 changes: 27 additions & 15 deletions src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,21 +203,33 @@ public List<BrAPIObservation> getObservationsByObservationUnits(Collection<Strin
return searchBrapiObservations(observationSearchRequest, program);
}

public List<BrAPIObservation> getObservationsByFilters(Program program, String studyDbId) throws ApiException, DoesNotExistException {
Collection<BrAPIObservation> observations = getProgramObservations(program.getId());

HashMap<String, String> traitIdsByObservationVariableDbId =
traitService.getIdsByObservationVariableDbIds(
program.getId(),
observations.stream()
.map(BrAPIObservation::getObservationVariableDbId)
.collect(Collectors.toList()));

return observations.stream()
.filter(o -> studyDbId == null || studyDbId.equals(o.getStudyDbId()))
.peek(o ->
o.setObservationVariableDbId(traitIdsByObservationVariableDbId.get(o.getObservationVariableDbId())))
.collect(Collectors.toList());
public BrAPIObservationListResponse getObservationsByFilters(Program program, String studyDbId, Integer page, Integer pageSize) throws ApiException, DoesNotExistException {
BrAPIObservationSearchRequest observationSearchRequest = new BrAPIObservationSearchRequest()
.programDbIds(List.of(program.getBrapiProgram().getProgramDbId()));

if (studyDbId != null) {
observationSearchRequest.setStudyDbIds(List.of(studyDbId));
}
observationSearchRequest.setPage(page == null ? 0 : page);
observationSearchRequest.setPageSize(pageSize == null ? 1000 : pageSize);
Comment on lines +213 to +214

@jloux-brapi jloux-brapi Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious why this code existed in the previous implementation, but I am glad you kept the use case working. Should be fine for now unless someone wants something different.


ObservationsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), ObservationsApi.class);

BrAPIObservationListResponse brAPIResponse = brAPIDAOUtil.simpleSearch(api::searchObservationsPost, observationSearchRequest);

List<BrAPIObservation> observations = brAPIDAOUtil.getListResult(brAPIResponse);
processObservations(program.getKey(), observations);

// TODO: Investigate if this resetting of the observationVariableDbId can be undone when trait exrefs are examined [BI-2925]
HashMap<String, String> traitIdsByObservationVariableDbId = traitService.getIdsByObservationVariableDbIds(
program.getId(), observations.stream()
.map(BrAPIObservation::getObservationVariableDbId).collect(Collectors.toList()));

observations.forEach(observation -> observation
.setObservationVariableDbId(traitIdsByObservationVariableDbId.get(observation.getObservationVariableDbId())));
brAPIResponse.getResult().setData(observations);
Comment thread
jloux-brapi marked this conversation as resolved.

return brAPIResponse;
}

@NotNull
Expand Down
55 changes: 54 additions & 1 deletion src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationsControllerIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,58 @@ public void testGetObsPagination() {
checkPagination(1, 0, HttpStatus.BAD_REQUEST, null, null, null);
checkPagination(10, 100, HttpStatus.BAD_REQUEST, null, null, null);

// Invalid negative values.
checkPagination(-1, 2, HttpStatus.BAD_REQUEST, null, null, null);
checkPagination(0, -1, HttpStatus.BAD_REQUEST, null, null, null);

// Invalid boundary when the final page is partial.
checkPagination(2, 3, HttpStatus.BAD_REQUEST, null, null, null);
}

@Test
public void testGetObsUnknownStudyReturnsEmptyFirstPage() {
Flowable<HttpResponse<String>> call = client.exchange(
GET(String.format("/programs/%s/brapi/v2/observations" + "?studyDbId=%s&page=0&pageSize=2", program.getId(), UUID.randomUUID()))
.bearerAuth("test-registered-user"), String.class);

HttpResponse<String> response = call.blockingFirst();

assertEquals(HttpStatus.OK, response.getStatus());

JsonObject responseObj = gson.fromJson(response.body(), JsonObject.class);
JsonArray observations = responseObj.getAsJsonObject("result").getAsJsonArray("data");
JsonObject pagination = responseObj.getAsJsonObject("metadata").getAsJsonObject("pagination");

assertEquals(0, observations.size());
assertEquals(0, pagination.get("currentPage").getAsInt());
assertEquals(2, pagination.get("pageSize").getAsInt());
assertEquals(0, pagination.get("totalCount").getAsInt());
assertEquals(0, pagination.get("totalPages").getAsInt());
}

@Test
public void testGetObsByStudyDbIdWithPagination() {
Flowable<HttpResponse<String>> call = client.exchange(
GET(String.format(
"/programs/%s/brapi/v2/observations?studyDbId=%s&page=0&pageSize=1",
program.getId(),
envIds.get(0)))
.bearerAuth("test-registered-user"), String.class);

HttpResponse<String> response = call.blockingFirst();
assertEquals(HttpStatus.OK, response.getStatus());

JsonObject responseObj = gson.fromJson(response.body(), JsonObject.class);
JsonArray observations = responseObj.getAsJsonObject("result").getAsJsonArray("data");
JsonObject pagination = responseObj.getAsJsonObject("metadata").getAsJsonObject("pagination");

assertEquals(1, observations.size());
assertEquals(envIds.get(0), observations.get(0).getAsJsonObject().get("studyDbId").getAsString());

assertEquals(0, pagination.get("currentPage").getAsInt());
assertEquals(1, pagination.get("pageSize").getAsInt());
assertEquals(2, pagination.get("totalCount").getAsInt());
assertEquals(2, pagination.get("totalPages").getAsInt());
}

private void checkPagination(Integer page, Integer pageSize, HttpStatus expectedStatus, Integer expectedSize, Integer expectedTotalCount, Integer expectedTotalPages) {
Expand Down Expand Up @@ -400,7 +452,8 @@ private void checkPagination(Integer page, Integer pageSize, HttpStatus expected
JsonObject responseObj = gson.fromJson(response.body(), JsonObject.class);
// Get metadata.
JsonObject pagination = responseObj.getAsJsonObject("metadata").getAsJsonObject("pagination");
assertEquals(expectedSize, pagination.get("pageSize").getAsInt());
int expectedPageSize = pageSize == null ? 1000 : pageSize;
assertEquals(expectedPageSize, pagination.get("pageSize").getAsInt());
int expectedPage = page == null ? 0 : page;
assertEquals(expectedPage, pagination.get("currentPage").getAsInt());
assertEquals(expectedTotalPages, pagination.get("totalPages").getAsInt());
Expand Down
73 changes: 72 additions & 1 deletion src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
import org.brapi.v2.model.pheno.response.BrAPIObservationListResponse;
import org.brapi.v2.model.pheno.response.BrAPIObservationListResponseResult;
import java.util.HashMap;

public class BrAPIObservationDAOUnitTest {

Expand All @@ -50,6 +53,7 @@ public class BrAPIObservationDAOUnitTest {
private BrAPIObservationUnitDAO observationUnitDAO;
private BrAPIDAOUtil brAPIDAOUtil;
private Program program;
private TraitService traitService;

@BeforeEach
void setup() {
Expand All @@ -65,6 +69,7 @@ void setup() {
ProgramDAO programDAO = mock(ProgramDAO.class);
observationUnitDAO = mock(BrAPIObservationUnitDAO.class);
brAPIDAOUtil = mock(BrAPIDAOUtil.class);
traitService = mock(TraitService.class);

when(programDAO.getCoreClient(programId)).thenReturn(mock(BrAPIClient.class));

Expand All @@ -75,7 +80,7 @@ void setup() {
brAPIDAOUtil,
new BrAPIEndpointProvider(),
1000,
mock(TraitService.class)
traitService
);
}

Expand Down Expand Up @@ -200,4 +205,70 @@ void observationSearchIsSkippedForEmptyFilters() {
verifyNoInteractions(observationUnitDAO);
verify(brAPIDAOUtil, never()).search(any(Function.class),any(Function4.class),any(BrAPIObservationSearchRequest.class));
}

@Test
@SneakyThrows
void getObservationsByFiltersUsesDefaultsWithoutStudyFilter() {
List<BrAPIObservation> observations = List.of();

BrAPIObservationListResponse brAPIResponse = new BrAPIObservationListResponse().result(new BrAPIObservationListResponseResult().data(observations));

when(brAPIDAOUtil.simpleSearch(any(Function.class), any(BrAPIObservationSearchRequest.class))).thenReturn(brAPIResponse);

doReturn(observations).when(brAPIDAOUtil).getListResult(brAPIResponse);

when(traitService.getIdsByObservationVariableDbIds(program.getId(), List.of())).thenReturn(new HashMap<>());

observationDAO.getObservationsByFilters(program, null, null, null);

ArgumentCaptor<BrAPIObservationSearchRequest> requestCaptor = ArgumentCaptor.forClass(BrAPIObservationSearchRequest.class);

verify(brAPIDAOUtil).simpleSearch(any(Function.class), requestCaptor.capture());

BrAPIObservationSearchRequest request = requestCaptor.getValue();

assertEquals(List.of(BRAPI_PROGRAM_DB_ID), request.getProgramDbIds());
assertEquals(null, request.getStudyDbIds());
assertEquals(0, request.getPage());
assertEquals(1000, request.getPageSize());
}

@Test
@SneakyThrows
void getObservationsByFiltersUsesBrAPISearchPagination() {
String brapiVariableDbId = "brapi-variable-1";
String traitId = UUID.randomUUID().toString();

BrAPIObservation observation = new BrAPIObservation()
.observationDbId("observation-1")
.studyDbId("study-1")
.observationVariableDbId(brapiVariableDbId);

List<BrAPIObservation> brapiObservations = List.of(observation);
BrAPIObservationListResponse brAPIResponse = new BrAPIObservationListResponse().result(new BrAPIObservationListResponseResult().data(brapiObservations));

when(brAPIDAOUtil.simpleSearch(any(Function.class), any(BrAPIObservationSearchRequest.class))).thenReturn(brAPIResponse);

doReturn(brapiObservations).when(brAPIDAOUtil).getListResult(brAPIResponse);

HashMap<String, String> traitIdsByObservationVariableDbId = new HashMap<>();
traitIdsByObservationVariableDbId.put(brapiVariableDbId, traitId);

when(traitService.getIdsByObservationVariableDbIds(program.getId(), List.of(brapiVariableDbId)))
.thenReturn(traitIdsByObservationVariableDbId);

BrAPIObservationListResponse result = observationDAO.getObservationsByFilters(program, "study-1", 2, 25);

ArgumentCaptor<BrAPIObservationSearchRequest> requestCaptor = ArgumentCaptor.forClass(BrAPIObservationSearchRequest.class);

verify(brAPIDAOUtil).simpleSearch(any(Function.class), requestCaptor.capture());

BrAPIObservationSearchRequest request = requestCaptor.getValue();

assertEquals(List.of(BRAPI_PROGRAM_DB_ID), request.getProgramDbIds());
assertEquals(List.of("study-1"), request.getStudyDbIds());
assertEquals(2, request.getPage());
assertEquals(25, request.getPageSize());
assertEquals(traitId, result.getResult().getData().get(0).getObservationVariableDbId());
}
}
Loading