From 012c9dd5c24c5ade5e9f177277a3a89b3818d2e5 Mon Sep 17 00:00:00 2001 From: humsika Date: Fri, 2 Oct 2026 12:26:00 -0500 Subject: [PATCH 1/2] BI-2963: Optimized observation retrieval using BrAPI search. --- .../brapi/v2/dao/BrAPIObservationDAO.java | 54 ++++-- .../ExperimentControllerIntegrationTest.java | 22 +++ .../v2/dao/BrAPIObservationDAOUnitTest.java | 177 ++++++++++++++++++ 3 files changed, 239 insertions(+), 14 deletions(-) create mode 100644 src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java 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 16a2a73cc..9f8fe8ea6 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java @@ -123,6 +123,21 @@ public List getObservationsByStudyName(List studyNames ); } + private List searchBrapiObservations(BrAPIObservationSearchRequest observationSearchRequest, Program program) throws ApiException { + + ObservationsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), ObservationsApi.class); + + List observations = brAPIDAOUtil.search( + api::searchObservationsPost, + (brAPIWSMIMEDataTypes, searchResultsDbId, page, pageSize) -> searchObservationsSearchResultsDbIdGet(program.getId(), searchResultsDbId, page, pageSize), + observationSearchRequest + ); + + processObservations(program.getKey(), observations); + + return observations; + } + /** * Retrieves a list of observations based on their database IDs and a specific program. * @@ -149,34 +164,45 @@ public List getObservationsByTrialDbId(List trialDbIds return Collections.emptyList(); } // First, get all ObservationUnits for the given trialDbIds. - // TODO: Once OUDAO removes cache, investigate utilizing observationUnit GET param to includeObservations instead of making an extra call for the observations. This should offer performance gains. [BI-2963] List observationUnitDbIds = observationUnitDAO.getObservationUnitsForTrialDbIds(program.getId(), trialDbIds) .stream().map(BrAPIObservationUnit::getObservationUnitDbId).collect(Collectors.toList()); - // Finally, return all Observations for those ObservationUnits (Observations are linked to Trial through ObservationUnits). - // TODO: This gets all observations for the program and filters, which is extremely inefficient. If above TODO suggestion doesn't work, another improvement would be to search on OU ids directly in BrAPI instead. [BI-2963] - return getProgramObservations(program.getId()).stream() - .filter(o -> observationUnitDbIds.contains(o.getObservationUnitDbId())) - .collect(Collectors.toList()); + + if (observationUnitDbIds.isEmpty()) { + return Collections.emptyList(); + } + + BrAPIObservationSearchRequest observationSearchRequest = + new BrAPIObservationSearchRequest() + .programDbIds(List.of( + program.getBrapiProgram().getProgramDbId())) + .observationUnitDbIds(observationUnitDbIds); + + return searchBrapiObservations( + observationSearchRequest, + program); } public List getObservationsByObservationUnitsAndVariables(Collection ouDbIds, Collection variableDbIds, Program program) throws ApiException { if(ouDbIds.isEmpty() || variableDbIds.isEmpty()) { return Collections.emptyList(); } - // TODO: Once OUDAO removes cache, change to BrAPI Observation search request on program, observation var ids, and ouDbId [BI-2963] - return getProgramObservations(program.getId()).stream() - .filter(o -> ouDbIds.contains(o.getObservationUnitDbId()) && variableDbIds.contains(o.getObservationVariableDbId())) - .collect(Collectors.toList()); + + BrAPIObservationSearchRequest observationSearchRequest = new BrAPIObservationSearchRequest().programDbIds(List.of(program.getBrapiProgram().getProgramDbId())) + .observationUnitDbIds(new ArrayList<>(ouDbIds)) + .observationVariableDbIds(new ArrayList<>(variableDbIds)); + + return searchBrapiObservations(observationSearchRequest, program); } public List getObservationsByObservationUnits(Collection ouDbIds, Program program) throws ApiException { - // TODO: Once OUDAO removes cache, change to BrAPI Observation search request on program and ouDbIds [BI-2963] + if(ouDbIds.isEmpty()) { return Collections.emptyList(); } - return getProgramObservations(program.getId()).stream() - .filter(o -> ouDbIds.contains(o.getObservationUnitDbId())) - .collect(Collectors.toList()); + + BrAPIObservationSearchRequest observationSearchRequest = new BrAPIObservationSearchRequest().programDbIds(List.of(program.getBrapiProgram().getProgramDbId())).observationUnitDbIds(new ArrayList<>(ouDbIds)); + + return searchBrapiObservations(observationSearchRequest, program); } public List getObservationsByFilters(Program program, String studyDbId) throws ApiException, DoesNotExistException { diff --git a/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java b/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java index f1b5e9ce8..3db310e33 100644 --- a/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java @@ -923,6 +923,28 @@ public void deleteExperimentSuccess(boolean hardDelete, boolean withObservations } + @Test + @SneakyThrows + public void hardDeleteExperimentIgnoresObservationsFromOtherExperiment() { + Program deleteProgram = createSeededProgram("Delete Experiment Isolation"); + + String observedExperimentTitle = "Observed Experiment " + UUID.randomUUID(); + String emptyExperimentTitle = "Empty Experiment " + UUID.randomUUID(); + + uploadExperimentWithObs(deleteProgram, observedExperimentTitle, buildObservedRows(observedExperimentTitle)); + + String emptyExperimentId = uploadExperimentWithoutObs(deleteProgram, emptyExperimentTitle,"Plot"); + + Flowable> deleteCall = client.exchange(DELETE(String.format("/programs/%s/experiments/%s?hard=true", deleteProgram.getId(), emptyExperimentId)).cookie(new NettyCookie("phylo-token","test-registered-user")),String.class); + + HttpResponse deleteResponse = deleteCall.blockingFirst(); + + assertEquals(HttpStatus.NO_CONTENT, deleteResponse.getStatus()); + + JsonArray remainingTrials = getProgramTrials(deleteProgram.getId().toString()); + assertEquals(1, remainingTrials.size()); + } + private List> buildObservedRows(String title) { List> observedRows = new ArrayList<>(); String envBase = title.replaceAll("\\s+", ""); diff --git a/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java b/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java new file mode 100644 index 000000000..8df2fda10 --- /dev/null +++ b/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java @@ -0,0 +1,177 @@ +/* + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.breedinginsight.brapi.v2.dao; + +import io.reactivex.functions.Function; +import io.reactivex.functions.Function4; +import lombok.SneakyThrows; +import org.brapi.client.v2.BrAPIClient; +import org.brapi.v2.model.core.BrAPIProgram; +import org.brapi.v2.model.pheno.BrAPIObservation; +import org.brapi.v2.model.pheno.BrAPIObservationUnit; +import org.brapi.v2.model.pheno.request.BrAPIObservationSearchRequest; +import org.breedinginsight.brapps.importer.daos.ImportDAO; +import org.breedinginsight.daos.ProgramDAO; +import org.breedinginsight.model.Program; +import org.breedinginsight.services.TraitService; +import org.breedinginsight.services.brapi.BrAPIEndpointProvider; +import org.breedinginsight.utilities.BrAPIDAOUtil; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import java.util.List; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +public class BrAPIObservationDAOUnitTest { + + private static final String BRAPI_PROGRAM_DB_ID = "brapi-program-1"; + + private BrAPIObservationDAO observationDAO; + private BrAPIObservationUnitDAO observationUnitDAO; + private BrAPIDAOUtil brAPIDAOUtil; + private Program program; + + @BeforeEach + void setup() { + UUID programId = UUID.randomUUID(); + + BrAPIProgram brapiProgram = new BrAPIProgram().programDbId(BRAPI_PROGRAM_DB_ID); + + program = new Program(); + program.setId(programId); + program.setKey("TEST"); + program.setBrapiProgram(brapiProgram); + + ProgramDAO programDAO = mock(ProgramDAO.class); + observationUnitDAO = mock(BrAPIObservationUnitDAO.class); + brAPIDAOUtil = mock(BrAPIDAOUtil.class); + + when(programDAO.getCoreClient(programId)).thenReturn(mock(BrAPIClient.class)); + + observationDAO = new BrAPIObservationDAO( + programDAO, + mock(ImportDAO.class), + observationUnitDAO, + brAPIDAOUtil, + new BrAPIEndpointProvider(), + 1000, + mock(TraitService.class) + ); + } + + @Test + @SneakyThrows + void getObservationsByTrialDbIdUsesObservationSearch() { + String trialDbId = "trial-1"; + String observationUnitDbId = "ou-1"; + + BrAPIObservationUnit observationUnit = new BrAPIObservationUnit().observationUnitDbId(observationUnitDbId); + + BrAPIObservation observation = new BrAPIObservation().observationDbId("observation-1").observationUnitDbId(observationUnitDbId); + + when(observationUnitDAO.getObservationUnitsForTrialDbIds(program.getId(), List.of(trialDbId))).thenReturn(List.of(observationUnit)); + + when(brAPIDAOUtil.search(any(Function.class),any(Function4.class),any(BrAPIObservationSearchRequest.class))).thenReturn(List.of(observation)); + + List result = observationDAO.getObservationsByTrialDbId(List.of(trialDbId), program); + + assertEquals(List.of(observation), result); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(BrAPIObservationSearchRequest.class); + + verify(brAPIDAOUtil).search(any(Function.class),any(Function4.class),requestCaptor.capture()); + + BrAPIObservationSearchRequest request = requestCaptor.getValue(); + + assertEquals(List.of(BRAPI_PROGRAM_DB_ID), request.getProgramDbIds()); + + assertEquals(List.of(observationUnitDbId), request.getObservationUnitDbIds()); + } + + @Test + @SneakyThrows + void getObservationsByTrialDbIdReturnsEmptyWhenNoUnitsExist() { + String trialDbId = "trial-without-units"; + + when(observationUnitDAO.getObservationUnitsForTrialDbIds(program.getId(), List.of(trialDbId))).thenReturn(List.of()); + + List result = observationDAO.getObservationsByTrialDbId(List.of(trialDbId), program); + + assertTrue(result.isEmpty()); + + verify(brAPIDAOUtil, never()).search(any(Function.class),any(Function4.class),any(BrAPIObservationSearchRequest.class)); + } + + @Test + @SneakyThrows + void getObservationsByObservationUnitsUsesObservationSearch() { + List observationUnitDbIds = List.of("ou-1", "ou-2"); + + when(brAPIDAOUtil.search(any(Function.class),any(Function4.class),any(BrAPIObservationSearchRequest.class))).thenReturn(List.of()); + + observationDAO.getObservationsByObservationUnits(observationUnitDbIds, program); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(BrAPIObservationSearchRequest.class); + + verify(brAPIDAOUtil).search(any(Function.class),any(Function4.class),requestCaptor.capture()); + + BrAPIObservationSearchRequest request = requestCaptor.getValue(); + + assertEquals(List.of(BRAPI_PROGRAM_DB_ID), request.getProgramDbIds()); + assertEquals(observationUnitDbIds, request.getObservationUnitDbIds()); + } + + @Test + @SneakyThrows + void getObservationsByObservationUnitsAndVariablesUsesBothFilters() { + List observationUnitDbIds = List.of("ou-1", "ou-2"); + List observationVariableDbIds = List.of("variable-1", "variable-2"); + + when(brAPIDAOUtil.search(any(Function.class), any(Function4.class),any(BrAPIObservationSearchRequest.class))).thenReturn(List.of()); + + observationDAO.getObservationsByObservationUnitsAndVariables(observationUnitDbIds, observationVariableDbIds, program); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(BrAPIObservationSearchRequest.class); + + verify(brAPIDAOUtil).search(any(Function.class),any(Function4.class),requestCaptor.capture()); + + BrAPIObservationSearchRequest request = requestCaptor.getValue(); + + assertEquals(List.of(BRAPI_PROGRAM_DB_ID), request.getProgramDbIds()); + assertEquals(observationUnitDbIds, request.getObservationUnitDbIds()); + assertEquals(observationVariableDbIds, request.getObservationVariableDbIds()); + } + + @Test + @SneakyThrows + void observationSearchIsSkippedForEmptyFilters() { + assertTrue(observationDAO.getObservationsByTrialDbId(List.of(), program).isEmpty()); + + assertTrue(observationDAO.getObservationsByObservationUnits(List.of(), program).isEmpty()); + + assertTrue(observationDAO.getObservationsByObservationUnitsAndVariables(List.of("ou-1"), List.of(), program).isEmpty()); + + verifyNoInteractions(observationUnitDAO); + verify(brAPIDAOUtil, never()).search(any(Function.class),any(Function4.class),any(BrAPIObservationSearchRequest.class)); + } +} \ No newline at end of file From e9b6cd39f447e92caf50412e50fbd8e6eb708c8a Mon Sep 17 00:00:00 2001 From: humsika Date: Sun, 4 Oct 2026 19:46:36 -0500 Subject: [PATCH 2/2] BI-2963: Optimized ObservationDAO method and added test case. --- .../brapi/v2/dao/BrAPIObservationDAO.java | 40 +++++++++---------- .../v2/dao/BrAPIObservationDAOUnitTest.java | 26 ++++++++++++ 2 files changed, 45 insertions(+), 21 deletions(-) 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 9f8fe8ea6..5766cae0b 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java @@ -124,7 +124,6 @@ public List getObservationsByStudyName(List studyNames } private List searchBrapiObservations(BrAPIObservationSearchRequest observationSearchRequest, Program program) throws ApiException { - ObservationsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), ObservationsApi.class); List observations = brAPIDAOUtil.search( @@ -148,19 +147,19 @@ private List searchBrapiObservations(BrAPIObservationSearchReq */ public List getObservationsByDbIds(List dbIds, Program program) throws ApiException { // Check if the dbIds list is empty and return an empty list if so - if(dbIds.isEmpty()) { + if (dbIds.isEmpty()) { return Collections.emptyList(); } - // Filter the observations based on the provided program ID and the provided list of dbIds - // Collect the filtered observations into a List and return the result - return getProgramObservations(program.getId()).stream() - .filter(o -> dbIds.contains(o.getObservationDbId())) - .collect(Collectors.toList()); + BrAPIObservationSearchRequest observationSearchRequest = new BrAPIObservationSearchRequest() + .programDbIds(List.of(program.getBrapiProgram().getProgramDbId())) + .observationDbIds(new ArrayList<>(dbIds)); + + return searchBrapiObservations(observationSearchRequest, program); } public List getObservationsByTrialDbId(List trialDbIds, Program program) throws ApiException { - if(trialDbIds.isEmpty()) { + if (trialDbIds.isEmpty()) { return Collections.emptyList(); } // First, get all ObservationUnits for the given trialDbIds. @@ -171,36 +170,35 @@ public List getObservationsByTrialDbId(List trialDbIds return Collections.emptyList(); } - BrAPIObservationSearchRequest observationSearchRequest = - new BrAPIObservationSearchRequest() - .programDbIds(List.of( - program.getBrapiProgram().getProgramDbId())) + BrAPIObservationSearchRequest observationSearchRequest = new BrAPIObservationSearchRequest() + .programDbIds(List.of(program.getBrapiProgram().getProgramDbId())) .observationUnitDbIds(observationUnitDbIds); - return searchBrapiObservations( - observationSearchRequest, - program); + return searchBrapiObservations(observationSearchRequest, program); } public List getObservationsByObservationUnitsAndVariables(Collection ouDbIds, Collection variableDbIds, Program program) throws ApiException { - if(ouDbIds.isEmpty() || variableDbIds.isEmpty()) { + if (ouDbIds.isEmpty() || variableDbIds.isEmpty()) { return Collections.emptyList(); } - BrAPIObservationSearchRequest observationSearchRequest = new BrAPIObservationSearchRequest().programDbIds(List.of(program.getBrapiProgram().getProgramDbId())) - .observationUnitDbIds(new ArrayList<>(ouDbIds)) - .observationVariableDbIds(new ArrayList<>(variableDbIds)); + BrAPIObservationSearchRequest observationSearchRequest = new BrAPIObservationSearchRequest() + .programDbIds(List.of(program.getBrapiProgram().getProgramDbId())) + .observationUnitDbIds(new ArrayList<>(ouDbIds)) + .observationVariableDbIds(new ArrayList<>(variableDbIds)); return searchBrapiObservations(observationSearchRequest, program); } public List getObservationsByObservationUnits(Collection ouDbIds, Program program) throws ApiException { - if(ouDbIds.isEmpty()) { + if (ouDbIds.isEmpty()) { return Collections.emptyList(); } - BrAPIObservationSearchRequest observationSearchRequest = new BrAPIObservationSearchRequest().programDbIds(List.of(program.getBrapiProgram().getProgramDbId())).observationUnitDbIds(new ArrayList<>(ouDbIds)); + BrAPIObservationSearchRequest observationSearchRequest = new BrAPIObservationSearchRequest() + .programDbIds(List.of(program.getBrapiProgram().getProgramDbId())) + .observationUnitDbIds(new ArrayList<>(ouDbIds)); return searchBrapiObservations(observationSearchRequest, program); } diff --git a/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java b/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java index 8df2fda10..a82bb9389 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java @@ -79,6 +79,30 @@ void setup() { ); } + @Test + @SneakyThrows + void getObservationsByDbIdsUsesObservationSearch() { + List observationDbIds = List.of("observation-1", "observation-2"); + + BrAPIObservation observation = new BrAPIObservation().observationDbId("observation-1"); + + when(brAPIDAOUtil.search(any(Function.class), any(Function4.class), any(BrAPIObservationSearchRequest.class))).thenReturn(List.of(observation)); + + List result = observationDAO.getObservationsByDbIds(observationDbIds, program); + + assertEquals(List.of(observation), result); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(BrAPIObservationSearchRequest.class); + + verify(brAPIDAOUtil).search(any(Function.class), any(Function4.class), requestCaptor.capture()); + + BrAPIObservationSearchRequest request = requestCaptor.getValue(); + + assertEquals(List.of(BRAPI_PROGRAM_DB_ID), request.getProgramDbIds()); + + assertEquals(observationDbIds, request.getObservationDbIds()); + } + @Test @SneakyThrows void getObservationsByTrialDbIdUsesObservationSearch() { @@ -167,6 +191,8 @@ void getObservationsByObservationUnitsAndVariablesUsesBothFilters() { void observationSearchIsSkippedForEmptyFilters() { assertTrue(observationDAO.getObservationsByTrialDbId(List.of(), program).isEmpty()); + assertTrue(observationDAO.getObservationsByDbIds(List.of(),program).isEmpty()); + assertTrue(observationDAO.getObservationsByObservationUnits(List.of(), program).isEmpty()); assertTrue(observationDAO.getObservationsByObservationUnitsAndVariables(List.of("ou-1"), List.of(), program).isEmpty());