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..5766cae0b 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,20 @@ 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. * @@ -133,50 +147,60 @@ public List getObservationsByStudyName(List studyNames */ 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. - // 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()) { + 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()) { + + 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 346e58dfd..28a70e20a 100644 --- a/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java +++ b/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> 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..a82bb9389 --- /dev/null +++ b/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 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() { + 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.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)); + } +} \ No newline at end of file