Fix issues where bi-generated exref id used instead of trialDbId in brapi endpoints

src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java

@@ -148,6 +148,7 @@ 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.

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

@@ -24,9 +24,11 @@
 import org.apache.commons.lang3.tuple.Pair;
 import org.brapi.client.v2.ApiResponse;
 import org.brapi.client.v2.model.exceptions.ApiException;
+import org.brapi.client.v2.model.queryParams.phenotype.ObservationQueryParams;
 import org.brapi.client.v2.modules.phenotype.ObservationsApi;
 import org.brapi.v2.model.BrAPIAcceptedSearchResponse;
 import org.brapi.v2.model.BrAPIExternalReference;
+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;
@@ -38,7 +40,6 @@
 import org.breedinginsight.daos.ProgramDAO;
 import org.breedinginsight.daos.cache.ProgramCacheProvider;
 import org.breedinginsight.model.Program;
-import org.breedinginsight.model.Trait;
 import org.breedinginsight.services.TraitService;
 import org.breedinginsight.services.brapi.BrAPIEndpointProvider;
 import org.breedinginsight.services.exceptions.DoesNotExistException;
@@ -67,6 +68,8 @@ public class BrAPIObservationDAO extends BrAPICachedDAO<BrAPIObservation> {
     private boolean runScheduledTasks;
     private final TraitService traitService;
 
+    private final int brapiMaxPageSize;
+
     @Inject
     public BrAPIObservationDAO(ProgramDAO programDAO,
                                ImportDAO importDAO,
@@ -75,6 +78,7 @@ public BrAPIObservationDAO(ProgramDAO programDAO,
                                BrAPIEndpointProvider brAPIEndpointProvider,
                                @Property(name = "brapi.server.reference-source") String referenceSource,
                                @Property(name = "micronaut.bi.api.run-scheduled-tasks") boolean runScheduledTasks,
+                               @Property(name = "brapi.cache.fetch-page-size")  int brapiFetchPageSize,
                                ProgramCacheProvider programCacheProvider, TraitService traitService) {
         this.programDAO = programDAO;
         this.importDAO = importDAO;
@@ -85,6 +89,7 @@ public BrAPIObservationDAO(ProgramDAO programDAO,
         this.runScheduledTasks = runScheduledTasks;
         this.traitService = traitService;
         this.programCache = programCacheProvider.getProgramCache(this::fetchProgramObservations, BrAPIObservation.class);
+        this.brapiMaxPageSize = brapiFetchPageSize;
     }
 
     @Scheduled(initialDelay = "${startup.delay.observation}")
@@ -162,14 +167,15 @@ private void processObservations(String programKey, List<BrAPIObservation> obser
         }
     }
 
-    /**
-     * Get all observations for a program from the cache.
-     */
-    private Map<String, BrAPIObservation> getProgramObservations(UUID programId) throws ApiException {
-        return programCache.get(programId);
+    private List<BrAPIObservation> getProgramObservations(UUID programId) throws ApiException {
+        Program program = programDAO.get(programId)
+                .stream()
+                .findFirst()
+                .orElseThrow();
+
+        return getBrAPIObservationsUsingBrAPIProgramId(program);
     }
 
-    // Note: not using cache, because unique studyName (with "[ProgramKey-ExtraInfo]") is not stored directly on Observation.
     public List<BrAPIObservation> getObservationsByStudyName(List<String> studyNames, Program program) throws ApiException {
         if(studyNames.isEmpty()) {
             return Collections.emptyList();
@@ -202,7 +208,7 @@ public List<BrAPIObservation> getObservationsByDbIds(List<String> dbIds, Program
 
         // 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()).values().stream()
+        return getProgramObservations(program.getId()).stream()
                 .filter(o -> dbIds.contains(o.getObservationDbId()))
                 .collect(Collectors.toList());
     }
@@ -212,10 +218,12 @@ public List<BrAPIObservation> getObservationsByTrialDbId(List<String> 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<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).
-        return getProgramObservations(program.getId()).values().stream()
+        // 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());
     }
@@ -224,29 +232,22 @@ public List<BrAPIObservation> getObservationsByObservationUnitsAndVariables(Coll
         if(ouDbIds.isEmpty() || variableDbIds.isEmpty()) {
             return Collections.emptyList();
         }
-        return getProgramObservations(program.getId()).values().stream()
+        // 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());
     }
 
     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()) {
             return Collections.emptyList();
         }
-        return getProgramObservations(program.getId()).values().stream()
+        return getProgramObservations(program.getId()).stream()
                 .filter(o -> ouDbIds.contains(o.getObservationUnitDbId()))
                 .collect(Collectors.toList());
     }
 
-    public List<BrAPIObservation> getObservationsByObservationUnitsAndStudies(Collection<String> ouDbIds, Collection<String> studyDbIds, Program program) throws ApiException {
-        if(ouDbIds.isEmpty()) {
-            return Collections.emptyList();
-        }
-        return getProgramObservations(program.getId()).values().stream()
-                .filter(o -> ouDbIds.contains(o.getObservationUnitDbId()) && studyDbIds.contains(o.getStudyDbId()))
-                .collect(Collectors.toList());
-    }
-
     // TODO: implement other filters in BI-2506.
     public List<BrAPIObservation> getObservationsByFilters(Program program, String studyDbId) throws ApiException, DoesNotExistException {
 
@@ -255,7 +256,7 @@ public List<BrAPIObservation> getObservationsByFilters(Program program, String s
         String observationSource = Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.OBSERVATIONS);
 
         // Get all observations for the program.
-        Collection<BrAPIObservation> observations = getProgramObservations(program.getId()).values();
+        Collection<BrAPIObservation> observations = getProgramObservations(program.getId());
         // Build a hashmap of traits for fast lookup. The key is ObservationVariableDbId, the value is the Trait Id.
         HashMap<String, String> traitIdsByObservationVariableDbId = traitService.getIdsByObservationVariableDbIds(program.getId(), observations.stream().map(BrAPIObservation::getObservationVariableDbId).collect(Collectors.toList()));
 
@@ -267,6 +268,7 @@ public List<BrAPIObservation> getObservationsByFilters(Program program, String s
                     Optional<BrAPIExternalReference> xref = Utilities.getExternalReference(o.getExternalReferences(), studySource);
                     return xref.filter(brAPIExternalReference -> studyDbId.equals(brAPIExternalReference.getReferenceId())).isPresent();
                 })
+                // Try to figure out why/how this translation is used.
                 .peek(o -> {
                     // Translate ObservationVariableDbId.
                     o.setObservationVariableDbId(traitIdsByObservationVariableDbId.get(o.getObservationVariableDbId()));
@@ -306,33 +308,35 @@ public List<BrAPIObservation> createBrAPIObservations(List<BrAPIObservation> brA
         }
     }
 
-    public BrAPIObservation updateBrAPIObservation(String dbId, BrAPIObservation observation, UUID programId) throws ApiException {
-        ObservationsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ObservationsApi.class);
-        var program = programDAO.fetchOneById(programId);
-        try {
-            Callable<Map<String, BrAPIObservation>> postFunction = () -> {
-                ApiResponse<BrAPIObservationSingleResponse> response = api.observationsObservationDbIdPut(dbId, observation);
-                    if (response == null)
-                    {
-                        throw new ApiException("Response is null", 0, null, null);
-                    }
-                    BrAPIObservationSingleResponse body = response.getBody();
-                    if (body == null) {
-                        throw new ApiException("Response is missing body", 0, response.getHeaders(), null);
-                    }
-                    BrAPIObservation updatedObservation = body.getResult();
-                    if (updatedObservation == null) {
-                        throw new ApiException("Response body is missing result", 0, response.getHeaders(), response.getBody().toString());
-                    }
-                    return processObservationsForCache(List.of(updatedObservation), program.getKey());
-            };
-            return programCache.post(programId, postFunction).get(0);
-        } catch (ApiException e) {
-            log.error(Utilities.generateApiExceptionLogMessage(e));
-            throw new InternalServerException("Unknown error has occurred: " + e.getMessage(), e);
-        } catch (Exception e) {
-            throw new InternalServerException("Unknown error has occurred: " + e.getMessage(), e);
+    private List<BrAPIObservation> getBrAPIObservationsUsingBrAPIProgramId(Program program) throws ApiException {
+
+        if (program == null || program.getId() == null) {
+            throw new InternalServerException("BI-API Program or Program ID is null");
         }
+
+        String brapiProgramDbId = Optional.of(program)
+                .map(Program::getBrapiProgram)
+                .map(BrAPIProgram::getProgramDbId)
+                .orElse(null);
+
+        if (brapiProgramDbId == null) {
+            brapiProgramDbId = programDAO.getProgramBrAPI(program).getProgramDbId();
+        }
+
+        ObservationQueryParams observationQueryParams =
+                ObservationQueryParams.builder()
+                        .programDbId(brapiProgramDbId)
+                        .pageSize(brapiMaxPageSize)
+                        .page(0)
+                        .build();
+
+        ObservationsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), ObservationsApi.class);
+
+        List<BrAPIObservation> result = brAPIDAOUtil.get(api::observationsGet, observationQueryParams);
+
+        processObservations(program.getKey(), result);
+
+        return result;
     }
 
     // This method overloads updateBrAPIObservation(String dbId, BrAPIObservation observation, UUID programId)

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

@@ -288,7 +288,7 @@ public List<BrAPIObservationUnit> getObservationUnits(Program program,
         BrAPIObservationUnitSearchRequest observationUnitSearchRequest = new BrAPIObservationUnitSearchRequest();
         observationUnitSearchRequest.programDbIds(List.of(program.getBrapiProgram()
                                                                  .getProgramDbId()));
-        //TODO add pagination support
+        //TODO add pagination support: This should be easy to implement with BrAPIDAOUtil.simpleSearch()
 //                                    .page(page)
 //                                    .pageSize(pageSize);
 
@@ -303,16 +303,15 @@ public List<BrAPIObservationUnit> getObservationUnits(Program program,
         observationUnitName.ifPresent(name -> observationUnitSearchRequest.setObservationUnitNames(List.of(Utilities.appendProgramKey(name, program.getKey()))));
         locationDbId.ifPresent(dbid -> observationUnitSearchRequest.setLocationDbIds(List.of(dbid)));
         seasonDbId.ifPresent(dbid -> observationUnitSearchRequest.setSeasonDbIds(List.of(dbid)));
+        experimentId.ifPresent(dbId -> observationUnitSearchRequest.setTrialDbIds(List.of(dbId)));
         includeObservations.ifPresent(observationUnitSearchRequest::includeObservations);
         addLevelFilter(observationUnitLevelName, observationUnitLevelOrder, observationUnitLevelCode, level, levelFilter);
         addLevelFilter(observationUnitLevelRelationshipName, observationUnitLevelRelationshipOrder, observationUnitLevelRelationshipCode, relationship, relationshipFilter);
-        experimentId.ifPresent(expId -> addXRefFilter(expId, ExternalReferenceSource.TRIALS, xrefIds, xrefSources));
         environmentId.ifPresent(envId -> addXRefFilter(envId, ExternalReferenceSource.STUDIES, xrefIds, xrefSources));
 //        germplasmId.ifPresent(germId -> {
 //            xrefIds.add(germId);
 //            xrefSources.add(Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.));
 //        });
-
         if(!xrefIds.isEmpty()) {
             observationUnitSearchRequest.externalReferenceIDs(xrefIds);
         }
@@ -326,10 +325,6 @@ public List<BrAPIObservationUnit> getObservationUnits(Program program,
                                                                                  .get()
                                                                                  .getReferenceId()))
                                                    .orElse(true);
-            matches = matches && experimentId.map(id -> id.equals(Utilities.getExternalReference(ou.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS))
-                                                                                   .get()
-                                                                                   .getReferenceId()))
-                                                     .orElse(true);
             matches = matches && environmentId.map(id -> id.equals(Utilities.getExternalReference(ou.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.STUDIES))
                                                                              .get()
                                                                              .getReferenceId()))

src/main/java/org/breedinginsight/brapi/v2/services/BrAPIObservationVariableService.java

@@ -74,10 +74,9 @@ public List<Trait> getBrAPIObservationVariablesForExperiment(
             expId = UUID.fromString(experimentId.get());
         } else {
             UUID envId = UUID.fromString(environmentId.orElseThrow(() -> new IllegalStateException("no environment id found")));
+            // TODO: Double check this (and other studyDbId bi-brapi) lookup still works with cache removal on studies [BI-2962]
             BrAPIStudy environment = trialService.getEnvironment(program.get(), envId);
-            expId = UUID.fromString(Utilities.getExternalReference(environment.getExternalReferences(),
-                    Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS))
-                    .orElseThrow(() -> new IllegalStateException("no external reference found")).getReferenceId());
+            expId = UUID.fromString(environment.getTrialDbId());
         }
 
         BrAPITrial experiment = trialService.getExperiment(program.get(), expId);

src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitControllerIntegrationTest.java

@@ -31,6 +31,7 @@
 import io.reactivex.Flowable;
 import lombok.SneakyThrows;
 import org.brapi.v2.model.BrAPIExternalReference;
+import org.brapi.v2.model.core.BrAPITrial;
 import org.brapi.v2.model.germ.BrAPIGermplasm;
 import org.brapi.v2.model.pheno.BrAPIObservationUnit;
 import org.brapi.v2.model.pheno.response.BrAPIObservationUnitSingleResponse;
@@ -41,8 +42,10 @@
 import org.breedinginsight.api.model.v1.request.SpeciesRequest;
 import org.breedinginsight.api.v1.controller.TestTokenValidator;
 import org.breedinginsight.brapi.v2.dao.BrAPIGermplasmDAO;
+import org.breedinginsight.brapi.v2.dao.BrAPITrialDAO;
 import org.breedinginsight.brapps.importer.ImportTestUtils;
 import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation;
+import org.breedinginsight.brapps.importer.services.processors.experiment.service.TrialService;
 import org.breedinginsight.dao.db.enums.DataType;
 import org.breedinginsight.dao.db.tables.pojos.SpeciesEntity;
 import org.breedinginsight.daos.SpeciesDAO;
@@ -91,6 +94,8 @@ public class BrAPIObservationUnitControllerIntegrationTest extends BrAPITest {
     private OntologyService ontologyService;
     @Inject
     private BrAPIGermplasmDAO germplasmDAO;
+    @Inject
+    private BrAPITrialDAO brapiTrialDAO;
 
     @Inject
     @Client("/${micronaut.bi.api.version}")
@@ -195,12 +200,8 @@ void setup() throws Exception {
                 program,
                 mappingId,
                 newExperimentWorkflowId);
-        experimentId = importResult
-                .get("preview").getAsJsonObject()
-                .get("rows").getAsJsonArray()
-                .get(0).getAsJsonObject()
-                .get("trial").getAsJsonObject()
-                .get("id").getAsString();
+        List<BrAPITrial> trials = brapiTrialDAO.getTrials(program.getId());
+        experimentId = trials.get(0).getTrialDbId();
         // Add environmentIds.
         envIds.add(getEnvId(importResult, 0));
         envIds.add(getEnvId(importResult, 1));