diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java index daadea133..f9536ebdd 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java @@ -114,6 +114,7 @@ public HttpResponse>>> getStudies( return ResponseUtils.getBrapiQueryResponse(authorizedStudies, studyQueryMapper, queryParams, searchRequest); } + // TODO: Instead of getting all studies for a program and filtering, doing the filtering on brapi side List studies = studyService.getStudies(programId) .stream() .peek(this::setDbIds) @@ -172,10 +173,6 @@ private void setDbIds(BrAPIStudy study) { .orElseThrow(() -> new IllegalStateException("No BI external reference found")) .getReferenceID()); - study.trialDbId(Utilities.getExternalReference(study.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS)) - .orElseThrow(() -> new IllegalStateException("No BI external reference found")) - .getReferenceID()); - //TODO update locationDbId } } diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java index 1ad489d91..195270184 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java @@ -26,6 +26,7 @@ import io.micronaut.security.rules.SecurityRule; import lombok.extern.slf4j.Slf4j; import org.brapi.client.v2.model.exceptions.ApiException; +import org.brapi.v2.model.BrAPIExternalReference; import org.brapi.v2.model.core.BrAPITrial; import org.brapi.v2.model.core.response.BrAPITrialSingleResponse; import org.breedinginsight.api.auth.*; @@ -158,10 +159,8 @@ public HttpResponse trialsTrialDbIdPut(@PathVariable("programId") UUID progra return HttpResponse.notFound(); } + // TODO: Remove for trialDbId once cache is removed for that entity private void setDbIds(BrAPITrial trial) { - trial.trialDbId(Utilities.getExternalReference(trial.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS)) - .orElseThrow(() -> new IllegalStateException("No BI external reference found")) - .getReferenceID()); trial.programDbId(Utilities.getExternalReference(trial.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.PROGRAMS)) .orElseThrow(() -> new IllegalStateException("No BI external reference found")) .getReferenceID()); diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java index 217d10216..7bc893f10 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java @@ -137,6 +137,7 @@ public List getStudiesByName(List studyNames, Program progra } public List getStudiesByExperimentID(@NotNull UUID experimentId, Program program) throws ApiException { + // TODO: This should look up on trialDbId, and the trialDbId should be passed thru BrAPIStudySearchRequest studySearch = new BrAPIStudySearchRequest(); studySearch.programDbIds(List.of(program.getBrapiProgram().getProgramDbId())); studySearch.addExternalReferenceIdsItem(experimentId.toString()); @@ -167,11 +168,7 @@ public List getStudiesByEnvironmentIds(@NotNull Collection env public List getStudiesByExperimentIds(@NotNull Collection experimentIds, Program program) throws ApiException { BrAPIStudySearchRequest studySearch = new BrAPIStudySearchRequest(); studySearch.programDbIds(List.of(program.getBrapiProgram().getProgramDbId())); - // Add all experimentIds as xref ID search terms. - for (UUID experimentId : experimentIds) { - studySearch.addExternalReferenceIdsItem(experimentId.toString()); - } - studySearch.addExternalReferenceSourcesItem(Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS)); + studySearch.trialDbIds(experimentIds.stream().map(UUID::toString).collect(Collectors.toList())); StudiesApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), StudiesApi.class); return new ArrayList<>(processStudyForDisplay(brAPIDAOUtil.search( api::searchStudiesPost, diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java index 85298c158..bbc3ceb56 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java @@ -40,13 +40,9 @@ List createBrAPITrials(List brAPITrialList, UUID program Optional getTrialById(UUID programId, UUID trialId) throws ApiException, DoesNotExistException; - Optional getTrialByDbId(String trialDbId, Program program) throws ApiException; - List getTrialsByDbIds(Collection trialDbIds, Program program) throws ApiException; List getTrialsByExperimentIds(Collection experimentIds, Program program) throws ApiException; void deleteBrAPITrial(Program program, BrAPITrial trial, boolean hard) throws ApiException; - - void repopulateCache(UUID programId); } diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/impl/BrAPITrialDAOImpl.java b/src/main/java/org/breedinginsight/brapi/v2/dao/impl/BrAPITrialDAOImpl.java index b0c771f6d..29b01ed44 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/impl/BrAPITrialDAOImpl.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/impl/BrAPITrialDAOImpl.java @@ -19,22 +19,22 @@ import io.micronaut.context.annotation.Context; import io.micronaut.context.annotation.Property; import io.micronaut.http.server.exceptions.InternalServerException; -import io.micronaut.scheduling.annotation.Scheduled; import lombok.extern.slf4j.Slf4j; import okhttp3.HttpUrl; import okhttp3.Request; +import org.brapi.client.v2.ApiResponse; import org.brapi.client.v2.model.exceptions.ApiException; +import org.brapi.client.v2.model.queryParams.core.TrialQueryParams; import org.brapi.client.v2.modules.core.TrialsApi; -import org.brapi.v2.model.BrAPIExternalReference; +import org.brapi.v2.model.core.BrAPIProgram; import org.brapi.v2.model.core.BrAPITrial; import org.brapi.v2.model.core.request.BrAPITrialSearchRequest; +import org.brapi.v2.model.core.response.BrAPITrialListResponse; import org.breedinginsight.brapi.v2.dao.BrAPITrialDAO; import org.breedinginsight.brapps.importer.daos.ImportDAO; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.daos.ProgramDAO; -import org.breedinginsight.daos.cache.ProgramCache; -import org.breedinginsight.daos.cache.ProgramCacheProvider; import org.breedinginsight.model.Program; import org.breedinginsight.services.ProgramService; import org.breedinginsight.services.brapi.BrAPIEndpointProvider; @@ -45,14 +45,12 @@ import javax.inject.Inject; import javax.inject.Singleton; import java.util.*; -import java.util.concurrent.Callable; import java.util.stream.Collectors; @Slf4j @Context @Singleton public class BrAPITrialDAOImpl implements BrAPITrialDAO { - private final ProgramCache programExperimentCache; private final ProgramDAO programDAO; private final ImportDAO importDAO; private final BrAPIDAOUtil brAPIDAOUtil; @@ -62,15 +60,13 @@ public class BrAPITrialDAOImpl implements BrAPITrialDAO { private final boolean runScheduledTasks; @Inject - public BrAPITrialDAOImpl(ProgramCacheProvider programCacheProvider, - ProgramDAO programDAO, + public BrAPITrialDAOImpl(ProgramDAO programDAO, ImportDAO importDAO, BrAPIDAOUtil brAPIDAOUtil, ProgramService programService, @Property(name = "brapi.server.reference-source") String referenceSource, BrAPIEndpointProvider brAPIEndpointProvider, @Property(name = "micronaut.bi.api.run-scheduled-tasks") boolean runScheduledTasks) { - this.programExperimentCache = programCacheProvider.getProgramCache(this::fetchProgramExperiments, BrAPITrial.class); this.programDAO = programDAO; this.importDAO = importDAO; this.brAPIDAOUtil = brAPIDAOUtil; @@ -80,136 +76,97 @@ public BrAPITrialDAOImpl(ProgramCacheProvider programCacheProvider, this.runScheduledTasks = runScheduledTasks; } + /** + * This method requires a BI-API program. If the BrAPIProgram inside this data model is not set, + * this method will retrieve it. + */ + private List getBrAPITrialsUsingBrAPIProgram(Program program) throws ApiException { - @Scheduled(initialDelay = "${startup.delay.trial}") - public void setup() { - if(!runScheduledTasks) { - return; + if (program == null || program.getId() == null) { + throw new InternalServerException("BI-API Program or Program ID is null"); } - // Populate the experiment cache for all programs on startup - log.debug("populating experiment cache"); - List programs = programDAO.getActive(); - if (programs != null) { - programExperimentCache.populate(programs.stream().map(Program::getId).collect(Collectors.toList())); + String brapiProgramDbId = Optional.of(program) + .map(Program::getBrapiProgram) + .map(BrAPIProgram::getProgramDbId) + .orElse(null); + + if (brapiProgramDbId == null) { + brapiProgramDbId = programDAO.getProgramBrAPI(program).getProgramDbId(); } + + // TODO: Configurable max amount of trials per program, or paginate. + + TrialQueryParams trialQueryParams = + TrialQueryParams.builder() + .programDbId(brapiProgramDbId) + .pageSize(1000) + .page(0) + .build(); + + return getTrialsFromBrAPI(program, trialQueryParams); } - private Map fetchProgramExperiments(UUID programId) throws ApiException { - TrialsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), TrialsApi.class); + private List getTrialsFromBrAPI(Program program, TrialQueryParams trialQueryParams) throws ApiException { + TrialsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), TrialsApi.class); - // Get the program - List programs = programDAO.get(programId); - if (programs.size() != 1) { - throw new InternalServerException("Program was not found for given key"); + ApiResponse response; + + try { + response = api.trialsGet(trialQueryParams); + } catch (ApiException e) { + log.warn(Utilities.generateApiExceptionLogMessage(e)); + throw new InternalServerException("Error making BrAPI call", e); } - Program program = programs.get(0); - // Get the program experiments - BrAPITrialSearchRequest trialSearch = new BrAPITrialSearchRequest(); - trialSearch.externalReferenceIDs(List.of(programId.toString())); - trialSearch.externalReferenceSources( - List.of(Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.PROGRAMS)) - ); - List programExperiments = brAPIDAOUtil.search( - api::searchTrialsPost, - api::searchTrialsSearchResultsDbIdGet, - trialSearch - ); + if (response.getBody().getMetadata().getPagination().getTotalCount() > trialQueryParams.pageSize()) { + throw new InternalServerException(String.format("More trials exist than requested [%s]", trialQueryParams)); + } + + List trialsFromResponse = response.getBody().getResult().getData(); - return experimentById(processExperimentsForDisplay(programExperiments, program.getKey())); + return processExperimentsForDisplay(trialsFromResponse, program.getKey()); } private Map experimentById(List trials) { Map experimentById = new HashMap<>(); for (BrAPITrial experiment: trials) { - BrAPIExternalReference xref = experiment - .getExternalReferences() - .stream() - .filter(reference -> String.format("%s/%s", referenceSource, ExternalReferenceSource.TRIALS.getName()).equals(reference.getReferenceSource())) - .findFirst().orElseThrow(() -> new IllegalStateException("No BI external reference found")); - experimentById.put(xref.getReferenceID(), experiment); + experimentById.put(experiment.getTrialDbId(), experiment); } return experimentById; } @Override public List getTrialsByName(List trialNames, Program program) throws ApiException { - Map cache = programExperimentCache.get(program.getId()); - List trials = new ArrayList<>(); - if (cache != null) { + List allTrialsForProgram = getBrAPITrialsUsingBrAPIProgram(program); - // TODO: replace with more performant cache search, e.g. RediSearch - trials.addAll(cache - .values() - .stream() - .filter(t -> trialNames.contains(t.getTrialName())) - .collect(Collectors.toList())); - } - - return trials; - } - - private List getTrialsByExRef(String referenceSource, String referenceId, Program program) throws ApiException { - Map cache = programExperimentCache.get(program.getId()); List trials = new ArrayList<>(); - if (cache != null) { - trials.addAll(cache - .values() + if (allTrialsForProgram != null) { + trials.addAll(allTrialsForProgram .stream() - .filter(t -> { - BrAPIExternalReference xref = t - .getExternalReferences() - .stream() - .filter(reference -> String.format("%s/%s", referenceSource, ExternalReferenceSource.TRIALS) - .equalsIgnoreCase(reference.getReferenceSource())) - .findFirst().orElseThrow(() -> new IllegalStateException("No BI trial external reference found")); - return referenceId.equals(xref.getReferenceID()); - }) + .filter(t -> trialNames.contains(t.getTrialName())) .collect(Collectors.toList())); } return trials; } + // TODO: Fix by using only code of inner callback and returning result @Override - public List createBrAPITrials(List brAPITrialList, UUID programId, ImportUpload upload) - throws ApiException { + public List createBrAPITrials(List brAPITrialList, UUID programId, ImportUpload upload) { TrialsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), TrialsApi.class); - List createdTrials = new ArrayList<>(); try { - if (!brAPITrialList.isEmpty()) { - Callable> postCallback = () -> { - List postedTrials = brAPIDAOUtil - .post(brAPITrialList, upload, api::trialsPost, importDAO::update); - return experimentById(postedTrials); - }; - createdTrials.addAll(programExperimentCache.post(programId, postCallback)); - } - - return createdTrials; + return brAPIDAOUtil.post(brAPITrialList, upload, api::trialsPost, importDAO::update); } catch (Exception e) { throw new InternalServerException("Unknown error has occurred: " + e.getMessage(), e); } } + @Override - public BrAPITrial updateBrAPITrial(String trialDbId, BrAPITrial trial, UUID programId) throws ApiException { + public BrAPITrial updateBrAPITrial(String trialDbId, BrAPITrial trial, UUID programId) { TrialsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), TrialsApi.class); - BrAPITrial updatedTrial = null; try { - if (trial != null && !trialDbId.isBlank()) { - Callable> putCallback = () -> { - BrAPITrial putTrial = brAPIDAOUtil.put(trialDbId, trial, api::trialsTrialDbIdPut); - return experimentById(List.of(putTrial)); - }; - List cachedUpdates = programExperimentCache.post(programId, putCallback); - if (cachedUpdates.isEmpty()) { - throw new Exception(); - } - updatedTrial = cachedUpdates.get(0); - } - - return updatedTrial; + return brAPIDAOUtil.put(trialDbId, trial, api::trialsTrialDbIdPut); } catch (Exception e) { throw new InternalServerException("Unknown error has occurred: " + e.getMessage(), e); } @@ -217,7 +174,10 @@ public BrAPITrial updateBrAPITrial(String trialDbId, BrAPITrial trial, UUID prog @Override public List getTrials(UUID programId) throws ApiException { - return new ArrayList<>(programExperimentCache.get(programId).values()); + Program program = programDAO.get(programId).get(0); + program.setBrapiProgram(programDAO.getProgramBrAPI(program)); + + return getBrAPITrialsUsingBrAPIProgram(program); } //Removes program key from trial name @@ -234,44 +194,37 @@ private List processExperimentsForDisplay( @Override public Optional getTrialById(UUID programId, UUID trialId) throws ApiException, DoesNotExistException { - Map cache = programExperimentCache.get(programId); - BrAPITrial trial = null; - if (cache != null) { - trial = cache.get(trialId.toString()); - } + Program program = programDAO.get(programId).get(0); - return Optional.ofNullable(trial); - } + TrialQueryParams params = TrialQueryParams.builder() + .trialDbId(trialId.toString()) + .pageSize(1) + .page(0) + .build(); - @Override - public Optional getTrialByDbId(String trialDbId, Program program) throws ApiException { - List trials = getTrialsByDbIds(List.of(trialDbId), program); - return Utilities.getSingleOptional(trials); + + return getTrialsFromBrAPI(program, params).stream().findFirst(); } @Override public List getTrialsByDbIds(Collection trialDbIds, Program program) throws ApiException { - Map cache = programExperimentCache.get(program.getId()); - List trials = new ArrayList<>(); - if (cache != null) { - trials.addAll(cache - .values() - .stream() - .filter(t -> trialDbIds.contains(t.getTrialDbId())) - .collect(Collectors.toList())); - } + Collection trialDbUUIDs = trialDbIds.stream().map(UUID::fromString).collect(Collectors.toList()); - return trials; + return getTrialsByExperimentIds(trialDbUUIDs, program); } + + // TODO: ExperimentIds will = trialDbIds once cache is updated. Update this method to get trials on dbId directly from brapi. @Override public List getTrialsByExperimentIds(Collection experimentIds, Program program) throws ApiException { if(experimentIds.isEmpty()) { return Collections.emptyList(); } + + List experimentIdsAsStrings = experimentIds.stream().map(UUID::toString).collect(Collectors.toList()); + BrAPITrialSearchRequest trialSearch = new BrAPITrialSearchRequest(); trialSearch.programDbIds(List.of(program.getBrapiProgram().getProgramDbId())); - trialSearch.externalReferenceSources(List.of(referenceSource + "/" + ExternalReferenceSource.TRIALS.getName())); - trialSearch.externalReferenceIds(experimentIds.stream().map(UUID::toString).collect(Collectors.toList())); + trialSearch.trialDbIds(experimentIdsAsStrings); TrialsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), TrialsApi.class); return processExperimentsForDisplay(brAPIDAOUtil.search( api::searchTrialsPost, @@ -294,10 +247,4 @@ public void deleteBrAPITrial(Program program, BrAPITrial trial, boolean hard) th brAPIDAOUtil.makeCall(brapiRequest); } - - @Override - public void repopulateCache(UUID programId) { - this.programExperimentCache.invalidate(programId); - this.programExperimentCache.populate(programId); - } } diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java index bc1ce1ce0..34312233b 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -121,6 +121,7 @@ public BrAPITrialService(@Property(name = "brapi.server.reference-source") Strin } public List getExperiments(UUID programId) throws ApiException, DoesNotExistException { + // TODO: Edit this to filter/paginate trials return trialDAO.getTrials(programId); } @@ -185,10 +186,17 @@ public DownloadFile exportObservations( // add columns for requested dataset obsvars and timestamps log.debug(logHash + ": fetching experiment for export"); BrAPITrial experiment = getExperiment(program, experimentId); + String expExRefId = Utilities.getExternalReference(experiment.getExternalReferences(), this.referenceSource, ExternalReferenceSource.TRIALS) + .map(BrAPIExternalReference::getReferenceId) + .orElse(null); + + if (expExRefId == null) { + throw new InternalServerException(String.format("Trials exref does not exist for experiment [%s]", experimentId)); + } // get requested environments for the experiment log.debug(logHash + ": fetching environments for export"); - List expStudies = studyDAO.getStudiesByExperimentID(experimentId, program); + List expStudies = studyDAO.getStudiesByExperimentID(UUID.fromString(expExRefId), program); if (!requestedEnvIds.isEmpty()) { expStudies = expStudies .stream() @@ -788,25 +796,33 @@ public int deleteExperiment(Program program, UUID experimentId, boolean hard) th } BrAPITrial trial = trials.get(0); + UUID exRefId = Utilities.getExternalReference(trial.getExternalReferences(), referenceSource, ExternalReferenceSource.TRIALS) + .map(BrAPIExternalReference::getReferenceId) + .map(UUID::fromString) + .orElse(null); + + if (exRefId == null) { + throw new InternalServerException(String.format("Experiment with UUID [%s] does not have TRIALS exref", experimentId)); + } + List existingObservations = observationDAO.getObservationsByTrialDbId(List.of(trial.getTrialDbId()), program); // If there are no observations or a soft delete is requested, proceed. if (existingObservations.isEmpty() || !hard) { // Make request to delete experiment. trialDAO.deleteBrAPITrial(program, trial, hard); // Get all lists for the trial. + // TODO: Get lists by trialDbId if trials get decoupled from datasets. List lists = listDAO .getListsByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, program.getId(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS), - experimentId); + exRefId); // TODO: replace with a single call to a batch delete method if that becomes available. // Iterate over lists, delete each by listDbId. for (BrAPIListSummary list : lists) { listDAO.deleteBrAPIList(list.getListDbId(), program.getId(), hard); } // TODO: if performance is poor, implement more precise invalidation, possibly using hierarchical cache keys. - // Invalidate and repopulate cache for Trial, Study, Observation, ObservationUnit. - trialDAO.repopulateCache(program.getId()); studyDAO.repopulateCache(program.getId()); observationDAO.repopulateCache(program.getId()); observationUnitDAO.repopulateCache(program.getId()); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java index bdcc8956f..d4f3e2198 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java @@ -208,9 +208,14 @@ public void process(ProcessContext processContext, ProcessedData processedData) // set the DbId to the for each newly created trial for (BrAPITrial createdTrial : createdTrials) { String createdTrialName = Utilities.removeProgramKey(createdTrial.getTrialName(), program.getKey()); + + trialByNameNoScope.get(createdTrialName) .getBrAPIObject() .setTrialDbId(createdTrial.getTrialDbId()); + // TODO: May need to set the main ID for pending objects with the BrAPI ID for downstream usage once cache is removed. + trialByNameNoScope.get(createdTrialName) + .setId(UUID.fromString(createdTrial.getTrialDbId())); } List createdLocations = new ArrayList<>(locationService.create(actingUser, program.getId(), newLocations)); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java index f30512d2b..06c7c14c9 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java @@ -198,9 +198,21 @@ private Map> initializeStudyByNameNoScop List existingStudies; Optional> trial = getTrialPIO(experimentImportRows, trialByNameNoScope); + String expExRefId = Utilities.getExternalReference(trial.get() + .getBrAPIObject() + .getExternalReferences(), BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS) + .map(BrAPIExternalReference::getReferenceId) + .orElse(null); + + if (expExRefId == null) { + String logMessage = "No exref found in trial to link trial to study"; + log.error(logMessage); + throw new InternalServerException(logMessage); + } + try { // the 'trial' variable will never be "null". - UUID experimentId = trial.get().getId(); + UUID experimentId = UUID.fromString(expExRefId); existingStudies = brAPIStudyDAO.getStudiesByExperimentID(experimentId, program); for (BrAPIStudy existingStudy : existingStudies) { experimentStudyService.processAndCacheStudy(existingStudy, program, BrAPIStudy::getStudyName, studyByName); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/TrialService.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/TrialService.java index 1a9a7f02f..a8e1d0adf 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/TrialService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/TrialService.java @@ -273,11 +273,7 @@ private void processAndCacheTrial( Program program, Map> trialByNameNoScope) { - //get TrialId from existingTrial - BrAPIExternalReference experimentIDRef = Utilities.getExternalReference(existingTrial.getExternalReferences(), - String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())) - .orElseThrow(() -> new InternalServerException("An Experiment ID was not found in any of the external references")); - UUID experimentId = UUID.fromString(experimentIDRef.getReferenceId()); + UUID experimentId = UUID.fromString(existingTrial.getTrialDbId()); trialByNameNoScope.put( Utilities.removeProgramKey(existingTrial.getTrialName(), program.getKey()), @@ -288,20 +284,14 @@ private void processAndCacheTrial( * Constructs a PendingImportObject containing a BrAPITrial object based on the input BrAPITrial. * * This function takes a BrAPITrial object as input and constructs a PendingImportObject which - * encapsulates the trial along with its associated experiment ID. The experiment ID is retrieved - * from the external references of the trial object using utility method getExternalReference. - * If the experiment ID is not found in the external references, an InternalServerException is thrown. + * encapsulates the trial along with its associated experiment ID, which is the trialDbId from the BrAPI DB. * * @param trial the BrAPITrial object for which the PendingImportObject is to be constructed * @return a PendingImportObject containing the BrAPITrial object and its associated experiment ID - * @throws InternalServerException if the experiment ID is not found in the external references of the trial */ - public PendingImportObject constructPIOFromBrapiTrial(BrAPITrial trial) throws InternalServerException { - PendingImportObject pio = null; - BrAPIExternalReference experimentIDRef = Utilities.getExternalReference(trial.getExternalReferences(), - String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())) - .orElseThrow(() -> new InternalServerException("An Experiment ID was not found in any of the external references")); - UUID experimentId = UUID.fromString(experimentIDRef.getReferenceId()); + public PendingImportObject constructPIOFromBrapiTrial(BrAPITrial trial) { + PendingImportObject pio; + UUID experimentId = UUID.fromString(trial.getTrialDbId()); pio = new PendingImportObject<>(ImportObjectState.EXISTING, trial, experimentId); return pio; } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/services/ExperimentTrialService.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/services/ExperimentTrialService.java index 2e3a2ec33..2be781952 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/services/ExperimentTrialService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/services/ExperimentTrialService.java @@ -149,11 +149,7 @@ private void processAndCacheTrial( Program program, Map> trialByNameNoScope) { - //get TrialId from existingTrial - BrAPIExternalReference experimentIDRef = Utilities.getExternalReference(existingTrial.getExternalReferences(), - String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())) - .orElseThrow(() -> new InternalServerException("An Experiment ID was not found in any of the external references")); - UUID experimentId = UUID.fromString(experimentIDRef.getReferenceId()); + UUID experimentId = UUID.fromString(existingTrial.getTrialDbId()); trialByNameNoScope.put( Utilities.removeProgramKey(existingTrial.getTrialName(), program.getKey()), 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 7c5c2a872..bf8f6ba52 100644 --- a/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java @@ -95,6 +95,8 @@ public class ExperimentControllerIntegrationTest extends BrAPITest { private ProgramUserDAO programUserDAO; @Inject private RoleDao roleDao; + @Inject + private BrAPITrialService brAPITrialService; @Inject @Client("/${micronaut.bi.api.version}") @@ -201,12 +203,11 @@ void setup() throws Exception { program, mappingId, newExperimentWorkflowId); - experimentId = importResult - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(0).getAsJsonObject() - .get("trial").getAsJsonObject() - .get("id").getAsString(); + + BrAPITrial trial = brAPITrialService.getExperiments(program.getId()).get(0); + + experimentId = trial.getTrialDbId(); + // Add environmentIds. envIds.add(getEnvId(importResult, 0)); envIds.add(getEnvId(importResult, 1)); @@ -234,7 +235,7 @@ private String uploadExperimentWithoutObs(Program targetProgram, String title, S expRows.add(row2); // Import test experiment, environments. - JsonObject importResult = importTestUtils.uploadAndFetchWorkflow( + importTestUtils.uploadAndFetchWorkflow( writeDataToFile(expRows, null), null, true, @@ -242,14 +243,14 @@ private String uploadExperimentWithoutObs(Program targetProgram, String title, S targetProgram, mappingId, newExperimentWorkflowId); - String expId = importResult - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(0).getAsJsonObject() - .get("trial").getAsJsonObject() - .get("id").getAsString(); - return expId; + BrAPITrial trial = brAPITrialService.getExperiments(targetProgram.getId()) + .stream() + .filter(t -> t.getTrialName().equals(title)) + .findFirst() + .orElseThrow(); + + return trial.getTrialDbId(); } /** @@ -929,7 +930,7 @@ private List> buildObservedRows(String title) { private String uploadExperimentWithObs(Program targetProgram, String title, List> expRows) throws Exception { ImportTestUtils importTestUtils = new ImportTestUtils(); - JsonObject importResult = importTestUtils.uploadAndFetchWorkflow( + importTestUtils.uploadAndFetchWorkflow( importTestUtils.writeExperimentDataToFile(expRows, traits, false, false, null), null, true, @@ -938,12 +939,13 @@ private String uploadExperimentWithObs(Program targetProgram, String title, List mappingId, newExperimentWorkflowId); - return importResult - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(0).getAsJsonObject() - .get("trial").getAsJsonObject() - .get("id").getAsString(); + BrAPITrial trial = brAPITrialService.getExperiments(targetProgram.getId()) + .stream() + .filter(t -> t.getTrialName().equals(title)) + .findFirst() + .orElseThrow(); + + return trial.getTrialDbId(); } private List> buildSubEntityRows(List> topLevelRows, String entityName, int repeatedMeasures) { diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java index 936bd401c..44caec1fa 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java @@ -30,6 +30,7 @@ import io.micronaut.http.netty.cookies.NettyCookie; import io.reactivex.Flowable; import org.brapi.v2.model.BrAPIExternalReference; +import org.brapi.v2.model.core.BrAPITrial; import org.brapi.v2.model.germ.BrAPIGermplasm; import org.breedinginsight.TestUtils; import org.breedinginsight.api.auth.AuthenticatedUser; @@ -37,6 +38,7 @@ 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.services.BrAPITrialService; import org.breedinginsight.brapps.importer.ImportTestUtils; import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation; import org.breedinginsight.dao.db.enums.DataType; @@ -56,6 +58,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.util.*; +import java.util.stream.Collectors; import static io.micronaut.http.HttpRequest.GET; @@ -75,6 +78,8 @@ public class BrAPITestUtils { private OntologyService ontologyService; @Inject private BrAPIGermplasmDAO germplasmDAO; + @Inject + private BrAPITrialService brAPITrialService; @Inject @Client("/${micronaut.bi.api.version}") @@ -184,12 +189,6 @@ public Tuple2> setupTestProgram(DSLContext brAPIDslContext program, mappingId, newExperimentWorkflowId); - String experiment1Id = importResult - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(0).getAsJsonObject() - .get("trial").getAsJsonObject() - .get("id").getAsString(); // Import test experiment, environments, and any observations importResult = importTestUtils.uploadAndFetchWorkflow( @@ -200,12 +199,15 @@ public Tuple2> setupTestProgram(DSLContext brAPIDslContext program, mappingId, newExperimentWorkflowId); - String experiment2Id = importResult - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(0).getAsJsonObject() - .get("trial").getAsJsonObject() - .get("id").getAsString(); + + List trials = brAPITrialService.getExperiments(program.getId()) + .stream() + // Ensure trial with title xyz is always second + .sorted(Comparator.comparing(BrAPITrial::getTrialName)) + .collect(Collectors.toList()); + + String experiment1Id = trials.get(0).getTrialDbId(); + String experiment2Id = trials.get(1).getTrialDbId(); // Refetch collaborator, since we updated program_user_roles. collaborator = userDAO.getUserByOAuthId(TestTokenValidator.OTHER_TEST_USER_ORCID).orElseThrow(Exception::new); diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java index dafa9b7a9..17888d69e 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java @@ -28,6 +28,7 @@ import io.reactivex.Flowable; import org.brapi.v2.model.core.BrAPITrial; import org.breedinginsight.BrAPITest; +import org.breedinginsight.brapi.v2.services.BrAPITrialService; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.model.*; import org.breedinginsight.utilities.Utilities; @@ -59,6 +60,9 @@ public class BrAPITrialsControllerIntegrationTest extends BrAPITest { @Inject private BrAPITestUtils brAPITestUtils; + @Inject + private BrAPITrialService brAPIITrialService; + private final Gson gson = new GsonBuilder().registerTypeAdapter(OffsetDateTime.class, (JsonDeserializer) (json, type, context) -> OffsetDateTime.parse(json.getAsString())) .create(); @@ -98,16 +102,17 @@ public void testGetTrials() { // Check names and experimentIds (the BI-assigned UUID stored as an xref). List expNames = new ArrayList<>(); - List expIds = new ArrayList<>(); + List trialExRefIds = new ArrayList<>(); for (BrAPITrial trial : trials) { String experimentId = Utilities.getExternalReference(trial.getExternalReferences(), source).get().getReferenceId(); - expIds.add(experimentId); + trialExRefIds.add(experimentId); expNames.add(trial.getTrialName()); } assert(expNames.contains("xyz")); assert(expNames.contains("Test Exp")); - assert(expIds.contains(experiment1Id)); - assert(expIds.contains(experiment2Id)); + // Verify two different bi-api ex ref ids were generated for each trial + assertEquals(2 , trialExRefIds.size()); + assertNotEquals(trialExRefIds.get(0), trialExRefIds.get(1)); } /** @@ -129,9 +134,7 @@ public void testGetTrialsAsExperimentalCollaborator() { assertEquals(1, trials.size()); BrAPITrial trial = gson.fromJson(trials.get(0).getAsJsonObject(), BrAPITrial.class); assertEquals("xyz", trial.getTrialName()); - String source = Utilities.generateReferenceSource(BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS); - String experimentId = Utilities.getExternalReference(trial.getExternalReferences(), source).get().getReferenceId(); - assertEquals(this.experiment2Id, experimentId); + assertEquals(this.experiment2Id, trial.getTrialDbId()); } } diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIV2ObservationVariableControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIV2ObservationVariableControllerIntegrationTest.java index 9fc0aac90..17f91ea0c 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIV2ObservationVariableControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPIV2ObservationVariableControllerIntegrationTest.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.BrAPIObservationVariable; import org.breedinginsight.BrAPITest; @@ -40,6 +41,7 @@ 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.services.BrAPITrialService; import org.breedinginsight.brapps.importer.ImportTestUtils; import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation; import org.breedinginsight.dao.db.enums.DataType; @@ -88,6 +90,8 @@ public class BrAPIV2ObservationVariableControllerIntegrationTest extends BrAPITe private OntologyService ontologyService; @Inject private BrAPIGermplasmDAO germplasmDAO; + @Inject + BrAPITrialService brAPITrialService; @Inject @Client("/${micronaut.bi.api.version}") @@ -195,12 +199,15 @@ void setup() throws Exception { program, mappingId, newExperimentWorkflowId); - experimentId = importResult - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(0).getAsJsonObject() - .get("trial").getAsJsonObject() - .get("id").getAsString(); + + BrAPITrial trial = brAPITrialService.getExperiments(program.getId()) + .stream() + .findFirst() + .orElseThrow(); + + experimentId = trial.getTrialDbId(); + + // Add environmentIds. envIds.add(getEnvId(importResult, 0)); envIds.add(getEnvId(importResult, 1)); diff --git a/src/test/java/org/breedinginsight/brapi/v2/SubEntityDatasetLockIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/SubEntityDatasetLockIntegrationTest.java index d958d0b44..1e1f3c110 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/SubEntityDatasetLockIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/SubEntityDatasetLockIntegrationTest.java @@ -10,7 +10,9 @@ import io.micronaut.http.client.annotation.Client; import io.micronaut.test.extensions.junit5.annotation.MicronautTest; import io.reactivex.Flowable; +import org.brapi.v2.model.core.BrAPITrial; import org.breedinginsight.BrAPITest; +import org.breedinginsight.brapi.v2.services.BrAPITrialService; import org.breedinginsight.model.Program; import org.junit.jupiter.api.*; @@ -34,6 +36,9 @@ public class SubEntityDatasetLockIntegrationTest extends BrAPITest { @Inject private BrAPITestUtils brAPITestUtils; + @Inject + BrAPITrialService brAPIITrialService; + @Inject @Client("/${micronaut.bi.api.version}") private RxHttpClient client; @@ -46,7 +51,10 @@ public class SubEntityDatasetLockIntegrationTest extends BrAPITest { void setup() throws Exception { var setup = brAPITestUtils.setupTestProgram(super.getBrapiDsl(), gson); program = setup.getV1(); - experimentId = setup.getV2().get(0); + experimentId = brAPIITrialService.getExperiments(program.getId()).stream() + .map(BrAPITrial::getTrialDbId) + .findFirst() + .orElseThrow(); } @Test diff --git a/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java b/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java index 5a055bdd0..866866195 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java @@ -35,6 +35,7 @@ import org.breedinginsight.services.TraitService; import org.breedinginsight.services.lock.DistributedLockService; import org.breedinginsight.utilities.FileUtil; +import org.breedinginsight.utilities.Utilities; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import tech.tablesaw.api.Table; @@ -121,6 +122,12 @@ void setup() { experimentInfo.addProperty(BrAPIAdditionalInfoFields.EXPERIMENT_TYPE, "Phenotyping"); experiment.setAdditionalInfo(experimentInfo); + BrAPIExternalReference externalReference = new BrAPIExternalReference(); + externalReference.setReferenceSource(Utilities.generateReferenceSource(REFERENCE_SOURCE, ExternalReferenceSource.TRIALS)); + externalReference.setReferenceId("11111111-1111-1111-1111-111111111111"); + + experiment.setExternalReferences(List.of(externalReference)); + study = new BrAPIStudy(); study.setStudyDbId("study-1"); study.setStudyName("Environment 1"); diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 802bf6889..abd55e9b0 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -327,12 +327,7 @@ public void appendExperimentMultipleDatasets() { newExp.put(Columns.COLUMN, "1"); JsonObject importResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); - String expId = importResponse - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(0).getAsJsonObject() - .get("trial").getAsJsonObject() - .get("id").getAsString(); + String expId = brAPITrialDAO.getTrialsByName(List.of(newExp.get(Columns.EXP_TITLE).toString()), program).get(0).getTrialDbId(); // Create two sub-entity datasets that have two different sub entity units Flowable> sub1PostCall = client.exchange( diff --git a/src/test/java/org/breedinginsight/brapps/importer/SampleSubmissionFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/SampleSubmissionFileImportTest.java index f19803127..b59c41900 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/SampleSubmissionFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/SampleSubmissionFileImportTest.java @@ -45,6 +45,7 @@ import org.breedinginsight.api.model.v1.request.SpeciesRequest; import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; import org.breedinginsight.brapi.v2.dao.*; +import org.breedinginsight.brapi.v2.services.BrAPITrialService; import org.breedinginsight.brapps.importer.daos.*; import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation; import org.breedinginsight.brapps.importer.model.imports.sample.SampleSubmissionImport.Columns; @@ -143,6 +144,9 @@ public class SampleSubmissionFileImportTest extends BrAPITest { @Inject private SampleSubmissionService sampleSubmissionService; + @Inject + BrAPITrialService brAPITrialService; + private Gson gson = new GsonBuilder().registerTypeAdapter(OffsetDateTime.class, (JsonDeserializer) (json, type, context) -> OffsetDateTime.parse(json.getAsString())) .registerTypeAdapter(BrAPIPagination.class, new PaginationTypeAdapter()) @@ -567,7 +571,7 @@ private String createExperiment(Program program) throws IOException, Interrupted .getAsJsonArray("data") .get(0).getAsJsonObject().get("id").getAsString(); - JsonObject importResult = importTestUtils.uploadAndFetchWorkflow( + importTestUtils.uploadAndFetchWorkflow( importTestUtils.writeExperimentDataToFile(List.of(makeExpImportRow("Env1")), null, false, false, null), null, true, @@ -575,12 +579,23 @@ private String createExperiment(Program program) throws IOException, Interrupted program, expMappingId, newExperimentWorkflowId); - return importResult - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(0).getAsJsonObject() - .get("trial").getAsJsonObject() - .get("id").getAsString(); + + + String result; + + try { + // Assumes only one trial/experiment exists + BrAPITrial trial = brAPITrialService.getExperiments(program.getId()) + .stream() + .findFirst() + .orElseThrow(); + + result = trial.getTrialDbId(); + } catch (Exception e) { + throw new RuntimeException(e); + } + + return result; } private Map makeExpImportRow(String environment) {