diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java index 27ac4e34c..5f4fcc7c8 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java +++ b/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 observations = observationDAO.getObservationsByFilters(program.get(), studyDbId); // If page is not provided, set it to the default value 0. 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 10048564c..7d42f0479 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java +++ b/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 { 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 obser } } - /** - * Get all observations for a program from the cache. - */ - private Map getProgramObservations(UUID programId) throws ApiException { - return programCache.get(programId); + private List 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 getObservationsByStudyName(List studyNames, Program program) throws ApiException { if(studyNames.isEmpty()) { return Collections.emptyList(); @@ -202,7 +208,7 @@ public List getObservationsByDbIds(List 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 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). - 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 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 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()).values().stream() + return getProgramObservations(program.getId()).stream() .filter(o -> ouDbIds.contains(o.getObservationUnitDbId())) .collect(Collectors.toList()); } - public List getObservationsByObservationUnitsAndStudies(Collection ouDbIds, Collection 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 getObservationsByFilters(Program program, String studyDbId) throws ApiException, DoesNotExistException { @@ -255,7 +256,7 @@ public List getObservationsByFilters(Program program, String s String observationSource = Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.OBSERVATIONS); // Get all observations for the program. - Collection observations = getProgramObservations(program.getId()).values(); + Collection observations = getProgramObservations(program.getId()); // Build a hashmap of traits for fast lookup. The key is ObservationVariableDbId, the value is the Trait Id. HashMap traitIdsByObservationVariableDbId = traitService.getIdsByObservationVariableDbIds(program.getId(), observations.stream().map(BrAPIObservation::getObservationVariableDbId).collect(Collectors.toList())); @@ -267,6 +268,7 @@ public List getObservationsByFilters(Program program, String s Optional 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 createBrAPIObservations(List 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> postFunction = () -> { - ApiResponse 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 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 result = brAPIDAOUtil.get(api::observationsGet, observationQueryParams); + + processObservations(program.getKey(), result); + + return result; } // This method overloads updateBrAPIObservation(String dbId, BrAPIObservation observation, UUID programId) diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java index 76acb8076..370d398d5 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java @@ -288,7 +288,7 @@ public List 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 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 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())) diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIObservationVariableService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIObservationVariableService.java index 96a21cd14..1359a760d 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIObservationVariableService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIObservationVariableService.java @@ -74,10 +74,9 @@ public List 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); diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitControllerIntegrationTest.java index 44792ea64..de876c626 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitControllerIntegrationTest.java +++ b/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 trials = brapiTrialDAO.getTrials(program.getId()); + experimentId = trials.get(0).getTrialDbId(); // Add environmentIds. envIds.add(getEnvId(importResult, 0)); envIds.add(getEnvId(importResult, 1));