BI-2963: ObservationDAO Optimization: Utilize observation/UnitSearch requests to optimize GET calls in DAO

src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java

@@ -123,6 +123,20 @@ public List<BrAPIObservation> getObservationsByStudyName(List<String> studyNames
         );
     }
 
+    private List<BrAPIObservation> searchBrapiObservations(BrAPIObservationSearchRequest observationSearchRequest, Program program) throws ApiException {
+        ObservationsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), ObservationsApi.class);
+
+        List<BrAPIObservation> 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.
      *
@@ -133,50 +147,60 @@ public List<BrAPIObservation> getObservationsByStudyName(List<String> studyNames
      */
     public List<BrAPIObservation> getObservationsByDbIds(List<String> 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<BrAPIObservation> getObservationsByTrialDbId(List<String> trialDbIds, Program program) throws ApiException {
-        if(trialDbIds.isEmpty()) {
+        if (trialDbIds.isEmpty()) {
             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<String> 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<BrAPIObservation> getObservationsByObservationUnitsAndVariables(Collection<String> ouDbIds, Collection<String> variableDbIds, Program program) throws ApiException {
-        if(ouDbIds.isEmpty() || variableDbIds.isEmpty()) {
+        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<BrAPIObservation> getObservationsByObservationUnits(Collection<String> 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()) {
+
+        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<BrAPIObservation> getObservationsByFilters(Program program, String studyDbId) throws ApiException, DoesNotExistException {

src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java

@@ -928,6 +928,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<HttpResponse<String>> 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<String> deleteResponse = deleteCall.blockingFirst();
+
+        assertEquals(HttpStatus.NO_CONTENT, deleteResponse.getStatus());
+
+        JsonArray remainingTrials = getProgramTrials(deleteProgram.getId().toString());
+        assertEquals(1, remainingTrials.size());
+    }
+
     private List<Map<String, Object>> buildObservedRows(String title) {
         List<Map<String, Object>> observedRows = new ArrayList<>();
         String envBase = title.replaceAll("\\s+", "");

src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAOUnitTest.java

@@ -0,0 +1,203 @@
+/*
+ * 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 getObservationsByDbIdsUsesObservationSearch() {
+        List<String> 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<BrAPIObservation> result = observationDAO.getObservationsByDbIds(observationDbIds, program);
+
+        assertEquals(List.of(observation), result);
+
+        ArgumentCaptor<BrAPIObservationSearchRequest> 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() {
+        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<BrAPIObservation> result = observationDAO.getObservationsByTrialDbId(List.of(trialDbId), program);
+
+        assertEquals(List.of(observation), result);
+
+        ArgumentCaptor<BrAPIObservationSearchRequest> 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<BrAPIObservation> 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<String> 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<BrAPIObservationSearchRequest> 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<String> observationUnitDbIds = List.of("ou-1", "ou-2");
+        List<String> 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<BrAPIObservationSearchRequest> 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.getObservationsByDbIds(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));
+    }
+}