diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationLevelsController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationLevelsController.java index ed2ef1884..65fa2c278 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationLevelsController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationLevelsController.java @@ -31,7 +31,6 @@ import org.brapi.client.v2.modules.phenotype.ObservationUnitsApi; import org.brapi.v2.model.BrAPIIndexPagination; import org.brapi.v2.model.BrAPIMetadata; -import org.brapi.v2.model.core.BrAPIStudy; import org.brapi.v2.model.core.BrAPITrial; import org.brapi.v2.model.pheno.BrAPIObservationUnitHierarchyLevel; import org.brapi.v2.model.pheno.response.BrAPIObservationLevelListResponse; @@ -39,7 +38,6 @@ import org.breedinginsight.api.auth.ProgramSecured; import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; import org.breedinginsight.brapi.v1.controller.BrapiVersion; -import org.breedinginsight.brapi.v2.dao.BrAPIStudyDAO; import org.breedinginsight.brapi.v2.dao.BrAPITrialDAO; import org.breedinginsight.daos.ProgramDAO; import org.breedinginsight.model.BrAPIConstants; @@ -65,15 +63,13 @@ public class BrAPIObservationLevelsController { private final ProgramDAO programDAO; private final ProgramService programService; private final BrAPITrialDAO trialDAO; - private final BrAPIStudyDAO studyDAO; @Inject - public BrAPIObservationLevelsController(BrAPIEndpointProvider brAPIEndpointProvider, ProgramDAO programDAO, ProgramService programService, BrAPITrialDAO trialDAO, BrAPIStudyDAO studyDAO) { + public BrAPIObservationLevelsController(BrAPIEndpointProvider brAPIEndpointProvider, ProgramDAO programDAO, ProgramService programService, BrAPITrialDAO trialDAO) { this.brAPIEndpointProvider = brAPIEndpointProvider; this.programDAO = programDAO; this.programService = programService; this.trialDAO = trialDAO; - this.studyDAO = studyDAO; } @Get("/observationlevels") @@ -93,25 +89,13 @@ public HttpResponse observationlevelsGet(@Pat } String programDbId = program.get().getBrapiProgram().getProgramDbId(); - String studyDbId = null; + String studyDbId = environmentId; String trialDbId = null; - if(environmentId != null) { - try { - Optional study = studyDAO.getStudyByEnvironmentId(UUID.fromString(environmentId), program.get()); - if(study.isPresent()) { - studyDbId = study.get().getStudyDbId(); - } else { - studyDbId = environmentId; - } - } catch (ApiException e) { - log.error(Utilities.generateApiExceptionLogMessage(e), "Error fetching environment"); - return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error finding observation levels"); - } - } else if(experimentId != null) { + if(environmentId == null && experimentId != null) { try { List trial = trialDAO.getTrialsByExperimentIds(List.of(UUID.fromString(experimentId)), program.get()); - if(trial.size() == 1) { + if (trial.size() == 1) { trialDbId = trial.get(0).getTrialDbId(); } else { trialDbId = experimentId; diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitController.java index 2be39af43..659e79406 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitController.java @@ -220,9 +220,6 @@ public HttpResponse observationunitsTableGet(@PathVariable("programId") UUID } private void setDbIds(BrAPIObservationUnit ou) { - ou.studyDbId(Utilities.getExternalReference(ou.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.STUDIES)) - .orElseThrow(() -> new IllegalStateException("No BI external reference found")) - .getReferenceID()); ou.programDbId(Utilities.getExternalReference(ou.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/BrAPIObservationsController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java index 5f4fcc7c8..7c1a18897 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIObservationsController.java @@ -61,20 +61,17 @@ public class BrAPIObservationsController { private final ProgramService programService; private final ProgramDAO programDAO; - private final BrAPIStudyDAO brAPIStudyDAO; private final BrAPIEndpointProvider brAPIEndpointProvider; private final BrAPIObservationDAO observationDAO; @Inject public BrAPIObservationsController(ProgramService programService, - ProgramDAO programDAO, ProgramDAO programDAO1, BrAPIStudyDAO brAPIStudyDAO, BrAPIEndpointProvider brAPIEndpointProvider, BrAPIObservationDAO brAPIObservationDAO) { this.programService = programService; this.programDAO = programDAO1; - this.brAPIStudyDAO = brAPIStudyDAO; this.brAPIEndpointProvider = brAPIEndpointProvider; this.observationDAO = brAPIObservationDAO; } @@ -254,16 +251,6 @@ public HttpResponse observationsTableGet( } try { - // Translate studyDbId if provided. - if (queryParams.getStudyDbId() != null) { - Optional study = brAPIStudyDAO.getStudyByEnvironmentId(UUID.fromString(queryParams.getStudyDbId()), program.get()); - if (study.isEmpty()) { - return HttpResponse.notFound(); - } - queryParams.setStudyDbId(study.get().getStudyDbId()); - } - // TODO: Translate other DbIds if provided as well (but studyDbId is sufficient for Mr. Bean). - ObservationsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ObservationsApi.class); ApiResponse response = api.observationsTableGet(BrAPIWSMIMEDataTypes.APPLICATION_JSON, queryParams.toBrAPIQueryParams()); diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java index 52027436d..66e21c055 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java @@ -17,7 +17,6 @@ package org.breedinginsight.brapi.v2; -import io.micronaut.context.annotation.Property; import io.micronaut.http.HttpResponse; import io.micronaut.http.HttpStatus; import io.micronaut.http.MediaType; @@ -30,7 +29,10 @@ import org.brapi.v2.model.BrAPIStatus; import org.brapi.v2.model.core.BrAPIStudy; import org.brapi.v2.model.core.response.BrAPIStudySingleResponse; -import org.breedinginsight.api.auth.*; +import org.breedinginsight.api.auth.ProgramSecured; +import org.breedinginsight.api.auth.ProgramSecuredRole; +import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; +import org.breedinginsight.api.auth.SecurityService; import org.breedinginsight.api.model.v1.request.query.SearchRequest; import org.breedinginsight.api.model.v1.response.DataResponse; import org.breedinginsight.api.model.v1.response.Response; @@ -38,13 +40,11 @@ import org.breedinginsight.brapi.v1.controller.BrapiVersion; import org.breedinginsight.brapi.v2.model.request.query.StudyQuery; import org.breedinginsight.brapi.v2.services.BrAPIStudyService; -import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.model.Program; import org.breedinginsight.model.ProgramUser; import org.breedinginsight.services.ExperimentalCollaboratorService; import org.breedinginsight.services.ProgramService; import org.breedinginsight.services.ProgramUserService; -import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.utilities.Utilities; import org.breedinginsight.utilities.response.ResponseUtils; import org.breedinginsight.utilities.response.mappers.StudyQueryMapper; @@ -54,14 +54,12 @@ import java.util.List; import java.util.Optional; import java.util.UUID; -import java.util.stream.Collectors; @Slf4j @Controller("/${micronaut.bi.api.version}/programs/{programId}" + BrapiVersion.BRAPI_V2) @Secured(SecurityRule.IS_AUTHENTICATED) public class BrAPIStudiesController { - private final String referenceSource; private final BrAPIStudyService studyService; private final StudyQueryMapper studyQueryMapper; private final ProgramService programService; @@ -73,14 +71,12 @@ public class BrAPIStudiesController { @Inject public BrAPIStudiesController(BrAPIStudyService studyService, StudyQueryMapper studyQueryMapper, - @Property(name = "brapi.server.reference-source") String referenceSource, ProgramService programService, SecurityService securityService, ProgramUserService programUserService, ExperimentalCollaboratorService experimentalCollaboratorService) { this.studyService = studyService; this.studyQueryMapper = studyQueryMapper; - this.referenceSource = referenceSource; this.programService = programService; this.securityService = securityService; this.programUserService = programUserService; @@ -107,18 +103,14 @@ public HttpResponse>>> getStudies( Optional experimentalCollaborator = programUserService.getIfExperimentalCollaborator(programId, securityService.getUser().getId()); if (experimentalCollaborator.isPresent()) { List authorizedExperimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(experimentalCollaborator.get().getId()); - List authorizedStudies = studyService.getStudiesByExperimentIds(program.get(), authorizedExperimentIds) - .stream() - .peek(this::setDbIds) - .collect(Collectors.toList()); + List authorizedStudies = studyService.getStudiesByExperimentIds( + program.get(), + authorizedExperimentIds); return ResponseUtils.getBrapiQueryResponse(authorizedStudies, studyQueryMapper, queryParams, searchRequest); } // TODO: Instead of getting all studies for a program and filtering, doing the filtering on brapi side [BI-2922] - List studies = studyService.getStudies(programId) - .stream() - .peek(this::setDbIds) - .collect(Collectors.toList()); + List studies = studyService.getStudies(programId); return ResponseUtils.getBrapiQueryResponse(studies, studyQueryMapper, queryParams, searchRequest); } catch (ApiException e) { log.info(e.getMessage(), e); @@ -138,25 +130,31 @@ public HttpResponse studiesPost(@PathVariable("programId") UUID programId, @Body @Get("/studies/{studyDbId}") @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) - public HttpResponse studiesStudyDbIdGet(@PathVariable("programId") UUID programId, @PathVariable("studyDbId") String environmentId) { + public HttpResponse studiesStudyDbIdGet(@PathVariable("programId") UUID programId, @PathVariable("studyDbId") String studyDbId) { Optional program = programService.getById(programId); - if(program.isEmpty()) { + if (program.isEmpty()) { log.warn("program id: " + programId + " not found"); return HttpResponse.notFound(); } + try { - Optional study = studyService.getStudyByEnvironmentId(program.get(), UUID.fromString(environmentId)); - if(study.isPresent()) { - setDbIds(study.get()); - return HttpResponse.ok(new BrAPIStudySingleResponse().result(study.get())); + Optional study = studyService.getStudyByDbId(program.get(), studyDbId); + + if (study.isPresent()) { + return HttpResponse.ok( + new BrAPIStudySingleResponse() + .result(study.get())); } else { - log.warn("studyDbId: " + environmentId + " not found"); + log.warn("studyDbId: " + studyDbId + " not found"); return HttpResponse.notFound(); } } catch (ApiException e) { log.error(Utilities.generateApiExceptionLogMessage(e), e); - return HttpResponse.serverError(new BrAPIStudySingleResponse().metadata(new BrAPIMetadata().addStatusItem(new BrAPIStatus().message("Error fetching study") - .messageType(BrAPIStatus.MessageTypeEnum.ERROR)))); + return HttpResponse.serverError(new BrAPIStudySingleResponse() + .metadata(new BrAPIMetadata() + .addStatusItem(new BrAPIStatus() + .message("Error fetching study") + .messageType(BrAPIStatus.MessageTypeEnum.ERROR)))); } } @@ -168,11 +166,4 @@ public HttpResponse studiesStudyDbIdPut(@PathVariable("programId") UUID programI return HttpResponse.notFound(); } - private void setDbIds(BrAPIStudy study) { - study.studyDbId(Utilities.getExternalReference(study.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.STUDIES)) - .orElseThrow(() -> new IllegalStateException("No BI external reference found")) - .getReferenceID()); - - //TODO update locationDbId - } } 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 d592687aa..16a2a73cc 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java @@ -26,7 +26,6 @@ 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; @@ -34,7 +33,6 @@ import org.brapi.v2.model.pheno.response.BrAPIObservationListResponse; 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.model.Program; import org.breedinginsight.services.TraitService; @@ -60,7 +58,6 @@ public class BrAPIObservationDAO { private BrAPIObservationUnitDAO observationUnitDAO; private final BrAPIDAOUtil brAPIDAOUtil; private final BrAPIEndpointProvider brAPIEndpointProvider; - private final String referenceSource; private final TraitService traitService; private final int brapiMaxPageSize; @@ -71,7 +68,6 @@ public BrAPIObservationDAO(ProgramDAO programDAO, BrAPIObservationUnitDAO observationUnitDAO, BrAPIDAOUtil brAPIDAOUtil, BrAPIEndpointProvider brAPIEndpointProvider, - @Property(name = "brapi.server.reference-source") String referenceSource, @Property(name = "brapi.cache.fetch-page-size") int brapiFetchPageSize, TraitService traitService) { this.programDAO = programDAO; @@ -79,7 +75,6 @@ public BrAPIObservationDAO(ProgramDAO programDAO, this.observationUnitDAO = observationUnitDAO; this.brAPIDAOUtil = brAPIDAOUtil; this.brAPIEndpointProvider = brAPIEndpointProvider; - this.referenceSource = referenceSource; this.traitService = traitService; this.brapiMaxPageSize = brapiFetchPageSize; } @@ -184,32 +179,21 @@ public List getObservationsByObservationUnits(Collection getObservationsByFilters(Program program, String studyDbId) throws ApiException, DoesNotExistException { - - String studySource = Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.STUDIES); - - // Get all observations for the program. 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())); - // Lookup studyDbId. + HashMap traitIdsByObservationVariableDbId = + traitService.getIdsByObservationVariableDbIds( + program.getId(), + observations.stream() + .map(BrAPIObservation::getObservationVariableDbId) + .collect(Collectors.toList())); + return observations.stream() - .filter(o -> { - // Short circuit if filter is null. - if (studyDbId == null) return true; - Optional xref = Utilities.getExternalReference(o.getExternalReferences(), studySource); - return xref.filter(brAPIExternalReference -> studyDbId.equals(brAPIExternalReference.getReferenceId())).isPresent(); - }) - .peek(o -> { - // Translate ObservationVariableDbId. - o.setObservationVariableDbId(traitIdsByObservationVariableDbId.get(o.getObservationVariableDbId())); - // Translate StudyDbId. - o.setStudyDbId(Utilities.getExternalReference(o.getExternalReferences(), studySource) - .orElseThrow(() -> new RuntimeException("study xref not found on observation")).getReferenceId()); - // TODO: consider translating germplasmDbId in BI-2506. - }).collect(Collectors.toList()); + .filter(o -> studyDbId == null || studyDbId.equals(o.getStudyDbId())) + .peek(o -> + o.setObservationVariableDbId(traitIdsByObservationVariableDbId.get(o.getObservationVariableDbId()))) + .collect(Collectors.toList()); } @NotNull 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 550dc9b56..5179435d5 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java @@ -237,18 +237,16 @@ public List getObservationUnitsForDataset(@NotNull String public List getObservationUnitsForDatasetAndEnvs(@NotNull String datasetId, Collection envIds, @NotNull Program program) throws ApiException { String datasetReferenceSource = Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.DATASET); - String studyReferenceSource = Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.STUDIES); - // TODO: Optimize and change this to search/get on both dataset id and studyDbId once a solution is in place to relate ous to datasets and study cache is removed [BI-2961] - return getProgramObservationUnits(program.getId()).stream() + + return getProgramObservationUnits(program.getId()) + .stream() .filter(ou -> { - Optional datasetExRef = Utilities.getExternalReference(ou.getExternalReferences(), datasetReferenceSource); - Optional studyExRef = Utilities.getExternalReference(ou.getExternalReferences(), studyReferenceSource); - return Boolean.logicalAnd( - datasetExRef.map(x -> x.getReferenceId().equals(datasetId)).orElse(false), - studyExRef.map(x -> envIds.contains(x.getReferenceId())).orElse(false) - ); - }) - .collect(Collectors.toList()); + Optional datasetExRef = + Utilities.getExternalReference(ou.getExternalReferences(), datasetReferenceSource); + + return Boolean.logicalAnd(datasetExRef.map(x -> x.getReferenceId().equals(datasetId)).orElse(false), + envIds.contains(ou.getStudyDbId())); + }).collect(Collectors.toList()); } // Note: does not use cache, impractical to implement all search parameters client-side. @@ -279,8 +277,7 @@ public List getObservationUnits(Program program, // .page(page) // .pageSize(pageSize); - List xrefIds = new ArrayList<>(); - List xrefSources = new ArrayList<>(); + BrAPIObservationUnitLevelRelationship level = new BrAPIObservationUnitLevelRelationship(); AtomicBoolean levelFilter = new AtomicBoolean(false); BrAPIObservationUnitLevelRelationship relationship = new BrAPIObservationUnitLevelRelationship(); @@ -296,34 +293,15 @@ public List getObservationUnits(Program program, addLevelFilter(observationUnitLevelName, observationUnitLevelOrder, observationUnitLevelCode, level, levelFilter); addLevelFilter(observationUnitLevelRelationshipName, observationUnitLevelRelationshipOrder, observationUnitLevelRelationshipCode, relationship, relationshipFilter); // TODO: Use observationUnitSearchRequest.setStudyDbIds() instead of xrefs [BI-2919] - 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); - } - if(!xrefSources.isEmpty()) { - observationUnitSearchRequest.externalReferenceSources(xrefSources); - } - - return searchObservationUnitsAndProcess(observationUnitSearchRequest, program, true).stream().filter(ou -> { - //xref search does an OR, so we need to convert the searching for expId/envId to be an AND - boolean matches = environmentId.map(id -> id.equals(Utilities.getExternalReference(ou.getExternalReferences(), Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.STUDIES)) - .get() - .getReferenceId())) - .orElse(true); - - //adding filter for germplasmDbId because we can't easily search that in the stored data object - // TODO: Add search on accessionNumber once it's been added to prod server and brapi client [BI-2978] - return matches && germplasmId.map(id -> id.equals(ou.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_UUID).getAsString())).orElse(true); - }).collect(Collectors.toList()); - } + environmentId.ifPresent(envId -> observationUnitSearchRequest.setStudyDbIds(List.of(envId))); - private void addXRefFilter(String ouId, ExternalReferenceSource externalReferenceSource, List xrefIds, List xrefSources) { - xrefIds.add(ouId); - xrefSources.add(Utilities.generateReferenceSource(referenceSource, externalReferenceSource)); + return searchObservationUnitsAndProcess(observationUnitSearchRequest, program, true) + .stream() + .filter(ou -> germplasmId.map(id -> id.equals(ou.getAdditionalInfo() + .get(BrAPIAdditionalInfoFields.GERMPLASM_UUID) + .getAsString())) + .orElse(true)) + .collect(Collectors.toList()); } private void addLevelFilter(Optional observationUnitLevelName, Optional observationUnitLevelOrder, Optional observationUnitLevelCode, BrAPIObservationUnitLevelRelationship level, AtomicBoolean levelFilter) { 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 adbdb5821..2f2b808e0 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java @@ -181,17 +181,6 @@ public List getStudiesByBrAPITrialExRefId(@NotNull UUID brapiTrialEx ); } - public List getStudiesByEnvironmentIds(@NotNull Collection environmentIds, Program program) throws ApiException { - // TODO: Optimize and change to a BrAPI search or get on external reference IDs [BI-3029] - String refSource = Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.STUDIES); - - return getBrAPIStudiesUsingBrAPIProgramId(program).stream() - .filter(study -> Utilities.getExternalReference(study.getExternalReferences(), refSource) - .map(ref -> environmentIds.contains(UUID.fromString(ref.getReferenceId()))) - .orElse(false)) - .collect(Collectors.toList()); - } - /** * Get a list of studies by a list of BI-assigned experiment UUIDs within a program. * @param experimentIds a list of BI-assigned experiment UUIDs. @@ -267,13 +256,6 @@ public Optional getStudyByDbId(String studyDbId, Program program) th return Utilities.getSingleOptional(studies); } - public Optional getStudyByEnvironmentId(UUID environmentId, Program program) throws ApiException { - List studies = getStudiesByEnvironmentIds(List.of(environmentId), program); - - return Utilities.getSingleOptional(studies); - } - - /** * Process study into a format for display * @param programStudy 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 1359a760d..4b579c0f1 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIObservationVariableService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIObservationVariableService.java @@ -17,7 +17,6 @@ package org.breedinginsight.brapi.v2.services; -import io.micronaut.context.annotation.Property; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; @@ -27,12 +26,11 @@ import org.brapi.v2.model.core.BrAPITrial; import org.brapi.v2.model.pheno.*; import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; -import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; -import org.breedinginsight.model.*; +import org.breedinginsight.model.Program; +import org.breedinginsight.model.Trait; import org.breedinginsight.services.ProgramService; import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.utilities.DatasetUtil; -import org.breedinginsight.utilities.Utilities; import org.jetbrains.annotations.NotNull; import javax.inject.Inject; @@ -45,15 +43,12 @@ public class BrAPIObservationVariableService { private final ProgramService programService; private final BrAPITrialService trialService; - private final String referenceSource; @Inject public BrAPIObservationVariableService( - ProgramService programService, BrAPITrialService trialService, - @Property(name = "brapi.server.reference-source") String referenceSource) { + ProgramService programService, BrAPITrialService trialService){ this.programService = programService; this.trialService = trialService; - this.referenceSource = referenceSource; } // TODO: support sub-entity datasets. @@ -73,9 +68,8 @@ public List getBrAPIObservationVariablesForExperiment( if(experimentId.isPresent()) { 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); + String studyDbId = environmentId.orElseThrow(() -> new IllegalStateException("no study db id found")); + BrAPIStudy environment = trialService.getEnvironment(program.get(), studyDbId); expId = UUID.fromString(environment.getTrialDbId()); } diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIStudyService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIStudyService.java index 104664c49..5c9a2e8f0 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIStudyService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIStudyService.java @@ -44,8 +44,8 @@ public List getStudies(UUID programId) throws ApiException { return studyDAO.getStudies(programId); } - public Optional getStudyByEnvironmentId(Program program, UUID environmentId) throws ApiException { - return studyDAO.getStudyByEnvironmentId(environmentId, program); + public Optional getStudyByDbId(Program program, String studyDbId) throws ApiException { + return studyDAO.getStudyByDbId(studyDbId, program); } /** 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 54693404d..e698df2e6 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -1,9 +1,9 @@ package org.breedinginsight.brapi.v2.services; -import com.google.gson.JsonArray; -import com.google.gson.JsonElement; import com.github.filosganga.geogson.model.Coordinates; import com.github.filosganga.geogson.model.positions.SinglePosition; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; import com.google.gson.JsonObject; import io.micronaut.context.annotation.Property; import io.micronaut.http.MediaType; @@ -17,7 +17,6 @@ import org.brapi.v2.model.core.response.BrAPIListsSingleResponse; import org.brapi.v2.model.core.response.BrAPITrialListResponse; import org.brapi.v2.model.germ.BrAPIGermplasm; - import org.brapi.v2.model.pheno.*; import org.breedinginsight.api.model.v1.request.SubEntityDatasetRequest; import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; @@ -31,21 +30,20 @@ import org.breedinginsight.brapps.importer.services.FileMappingUtil; import org.breedinginsight.brapps.importer.services.processors.experiment.service.DatasetService; import org.breedinginsight.dao.db.enums.DataType; -import org.breedinginsight.model.BrAPIConstants; -import org.breedinginsight.model.Column; -import org.breedinginsight.model.DownloadFile; -import org.breedinginsight.model.Program; import org.breedinginsight.model.*; import org.breedinginsight.model.delta.DeltaEntityFactory; import org.breedinginsight.model.delta.Experiment; import org.breedinginsight.services.TraitService; import org.breedinginsight.services.exceptions.AlreadyExistsException; -import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.services.exceptions.CreationBusyException; +import org.breedinginsight.services.exceptions.DoesNotExistException; +import org.breedinginsight.services.lock.DistributedLockService; import org.breedinginsight.services.parsers.experiment.ExperimentFileColumns; -import org.breedinginsight.utilities.*; +import org.breedinginsight.utilities.DatasetUtil; +import org.breedinginsight.utilities.FileUtil; +import org.breedinginsight.utilities.IntOrderComparator; +import org.breedinginsight.utilities.Utilities; import org.jetbrains.annotations.NotNull; -import org.breedinginsight.services.lock.DistributedLockService; import javax.inject.Inject; import javax.inject.Singleton; @@ -56,11 +54,11 @@ import java.time.OffsetDateTime; import java.time.format.DateTimeFormatter; import java.util.*; +import java.util.concurrent.TimeoutException; import java.util.function.Function; import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; -import java.util.concurrent.TimeoutException; import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.OBSERVATION_UNIT_ID_SUFFIX; @@ -213,7 +211,7 @@ public DownloadFile exportObservations( if (!requestedEnvIds.isEmpty()) { expStudies = expStudies .stream() - .filter(study -> requestedEnvIds.contains(getStudyId(study))) + .filter(study -> requestedEnvIds.contains(study.getStudyDbId())) .collect(Collectors.toList()); } expStudies.forEach(study -> studyByDbId.putIfAbsent(study.getStudyDbId(), study)); @@ -733,16 +731,6 @@ private void addBrAPIObsToRecords( } } - private String getStudyId(BrAPIStudy study) { - // HACK: avoid null reference exceptions. - if (study == null) return null; - BrAPIExternalReference studyXref = Utilities.getExternalReference( - study.getExternalReferences(), - String.format("%s/%s", referenceSource, ExternalReferenceSource.STUDIES.getName())) - .orElseThrow(() -> new RuntimeException("study id not found")); - return studyXref.getReferenceID(); - } - private void addObsVarDataToRow( Map row, BrAPIObservation obs, @@ -1103,16 +1091,6 @@ private String makeZipFileName(BrAPITrial experiment, Program program, String da return Utilities.makePortableFilename(unsafeName); } - private List filterDatasetByEnvironment( - List dataset, - List envIds, - Map studyByDbId) { - return dataset - .stream() - .filter(obs -> envIds.contains(getStudyId(studyByDbId.get(obs.getStudyDbId())))) - .collect(Collectors.toList()); - } - private boolean isSubEntityDataset(List ous){ return (ous.get(0).getObservationUnitPosition().getObservationLevelRelationships().size() > 2); } @@ -1141,12 +1119,8 @@ private void sortDefaultForExportRows(@NotNull List> exportR exportRows.sort(envComparator.thenComparing(expUnitIdComparator).thenComparing(subUnitIdComparator)); } - public BrAPIStudy getEnvironment(Program program, UUID envId) throws ApiException { - List environments = studyDAO.getStudiesByEnvironmentIds(List.of(envId), program); - if (environments.isEmpty()) { - throw new RuntimeException("A study with given experiment id was not returned"); + public BrAPIStudy getEnvironment(Program program, String studyDbId) throws ApiException { + return studyDAO.getStudyByDbId(studyDbId, program) + .orElseThrow(() -> new RuntimeException("A study with given study db id was not returned")); } - - return environments.get(0); - } } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateNewPendingImportObjectsStep.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateNewPendingImportObjectsStep.java index f2b6ba133..a02054a69 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateNewPendingImportObjectsStep.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateNewPendingImportObjectsStep.java @@ -1,4 +1,4 @@ - /* +/* * See the NOTICE file distributed with this work for additional information * regarding copyright ownership. * @@ -45,17 +45,12 @@ import org.breedinginsight.brapps.importer.services.processors.experiment.create.model.ProcessedPhenotypeData; import org.breedinginsight.brapps.importer.services.processors.experiment.service.DatasetService; import org.breedinginsight.brapps.importer.services.processors.experiment.services.ExperimentSeasonService; -import org.breedinginsight.model.Program; -import org.breedinginsight.model.ProgramLocation; -import org.breedinginsight.model.User; +import org.breedinginsight.model.*; import org.breedinginsight.services.exceptions.MissingRequiredInfoException; import org.breedinginsight.services.exceptions.UnprocessableEntityException; import org.breedinginsight.utilities.DatasetUtil; import org.breedinginsight.utilities.Utilities; -import org.breedinginsight.model.DatasetMetadata; -import org.breedinginsight.model.DatasetLevel; import org.jooq.DSLContext; -import org.breedinginsight.model.Trait; import tech.tablesaw.columns.Column; import javax.inject.Inject; @@ -635,8 +630,6 @@ private void fetchOrCreateObservationPIO(ProcessedPhenotypeData phenotypeData, newObservation.setObservationTimeStamp(OffsetDateTime.parse(timeStampValue)); } - newObservation.setStudyDbId(studyPIO.getId().toString()); //set as the BI ID to facilitate looking up studies when saving new observations - pio = new PendingImportObject<>(ImportObjectState.NEW, newObservation); observationByHash.put(key, pio); } 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 d95270d27..36055121a 100644 --- a/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java @@ -24,7 +24,7 @@ import org.breedinginsight.api.model.v1.request.ProgramRequest; import org.breedinginsight.api.model.v1.request.SpeciesRequest; import org.breedinginsight.brapi.v2.dao.BrAPIGermplasmDAO; -import org.breedinginsight.brapi.v2.model.request.query.ExperimentQuery; +import org.breedinginsight.brapi.v2.dao.BrAPIStudyDAO; import org.breedinginsight.brapi.v2.services.BrAPITrialService; import org.breedinginsight.brapps.importer.ImportTestUtils; import org.breedinginsight.brapps.importer.model.exports.FileType; @@ -46,19 +46,24 @@ import org.breedinginsight.utilities.DatasetUtil; import org.breedinginsight.utilities.FileUtil; import org.jooq.DSLContext; -import org.junit.jupiter.api.*; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; import tech.tablesaw.api.ColumnType; import tech.tablesaw.api.Row; import tech.tablesaw.api.Table; + import javax.inject.Inject; import java.io.*; import java.math.BigDecimal; import java.time.OffsetDateTime; import java.util.*; import java.util.stream.Collectors; + import static io.micronaut.http.HttpRequest.*; import static org.junit.Assert.assertNotEquals; import static org.junit.jupiter.api.Assertions.*; @@ -98,6 +103,8 @@ public class ExperimentControllerIntegrationTest extends BrAPITest { private RoleDao roleDao; @Inject private BrAPITrialService brAPITrialService; + @Inject + private BrAPIStudyDAO brAPIStudyDAO; @Inject @Client("/${micronaut.bi.api.version}") @@ -196,7 +203,7 @@ void setup() throws Exception { rows.add(row2); // Import test experiment, environments, and any observations - JsonObject importResult = importTestUtils.uploadAndFetchWorkflow( + importTestUtils.uploadAndFetchWorkflow( writeDataToFile(rows, traits), null, true, @@ -209,9 +216,9 @@ void setup() throws Exception { experimentId = trial.getTrialDbId(); - // Add environmentIds. - envIds.add(getEnvId(importResult, 0)); - envIds.add(getEnvId(importResult, 1)); + envIds.clear(); + brAPIStudyDAO.getStudies(program.getId()) + .forEach(study -> envIds.add(study.getStudyDbId())); } // Create an experiment with no observations. @@ -1291,18 +1298,6 @@ private BigDecimal toBigDecimal(Object value) { return new BigDecimal(value.toString()); } - private String getEnvId(JsonObject result, int index) { - return result - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(index).getAsJsonObject() - .get("study").getAsJsonObject() - .get("brAPIObject").getAsJsonObject() - .get("externalReferences").getAsJsonArray() - .get(2).getAsJsonObject() - .get("referenceId").getAsString(); - } - private JsonArray getProgramTrials(String programId) { Flowable> getCall = client.exchange( GET(String.format("/programs/%s/brapi/v2/trials", programId)) diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationLevelsControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationLevelsControllerIntegrationTest.java index 4f15c553d..ef79cd3e3 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationLevelsControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationLevelsControllerIntegrationTest.java @@ -37,6 +37,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.dao.BrAPIStudyDAO; import org.breedinginsight.brapps.importer.ImportTestUtils; import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation; import org.breedinginsight.dao.db.enums.DataType; @@ -60,8 +61,7 @@ import java.util.*; import static io.micronaut.http.HttpRequest.GET; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; @MicronautTest @TestInstance(TestInstance.Lifecycle.PER_CLASS) @@ -87,6 +87,8 @@ public class BrAPIObservationLevelsControllerIntegrationTest extends BrAPITest { private OntologyService ontologyService; @Inject private BrAPIGermplasmDAO germplasmDAO; + @Inject + private BrAPIStudyDAO brAPIStudyDAO; @Inject @Client("/${micronaut.bi.api.version}") @@ -199,9 +201,10 @@ void setup() throws Exception { .get(0).getAsJsonObject() .get("trial").getAsJsonObject() .get("id").getAsString(); - // Add environmentIds. - envIds.add(getEnvId(importResult, 0)); - envIds.add(getEnvId(importResult, 1)); + + envIds.clear(); + brAPIStudyDAO.getStudies(program.getId()) + .forEach(study -> envIds.add(study.getStudyDbId())); } @Test @@ -225,6 +228,25 @@ public void testGetObservationLevels() { assertTrue(levelNames.contains("plot")); } + @Test + public void testGetObservationLevelsByStudyDbId() { + Flowable> call = client.exchange( + GET(String.format( + "/programs/%s/brapi/v2/observationlevels?studyDbId=%s", + program.getId(), envIds.get(0))) + .bearerAuth("test-registered-user"), String.class); + + HttpResponse response = call.blockingFirst(); + + assertEquals(HttpStatus.OK, response.getStatus()); + + JsonObject responseObj = gson.fromJson( + response.body(), + JsonObject.class); + + assertNotNull(responseObj.getAsJsonObject("result")); + } + private File writeDataToFile(List> data, List traits) throws IOException { File file = File.createTempFile("test", ".csv"); @@ -243,18 +265,6 @@ private File writeDataToFile(List> data, List traits) return file; } - private String getEnvId(JsonObject result, int index) { - return result - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(index).getAsJsonObject() - .get("study").getAsJsonObject() - .get("brAPIObject").getAsJsonObject() - .get("externalReferences").getAsJsonArray() - .get(2).getAsJsonObject() - .get("referenceId").getAsString(); - } - private List createGermplasm(int numToCreate) { List germplasm = new ArrayList<>(); for (int i = 0; i < numToCreate; i++) { diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitControllerIntegrationTest.java index de876c626..0eb64a74d 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationUnitControllerIntegrationTest.java @@ -42,10 +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.BrAPIStudyDAO; 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; @@ -96,6 +96,8 @@ public class BrAPIObservationUnitControllerIntegrationTest extends BrAPITest { private BrAPIGermplasmDAO germplasmDAO; @Inject private BrAPITrialDAO brapiTrialDAO; + @Inject + private BrAPIStudyDAO brAPIStudyDAO; @Inject @Client("/${micronaut.bi.api.version}") @@ -192,7 +194,7 @@ void setup() throws Exception { rows.add(row2); // Import test experiment, environments, and any observations - JsonObject importResult = importTestUtils.uploadAndFetchWorkflow( + importTestUtils.uploadAndFetchWorkflow( writeDataToFile(rows, traits), null, true, @@ -202,9 +204,10 @@ void setup() throws Exception { newExperimentWorkflowId); List trials = brapiTrialDAO.getTrials(program.getId()); experimentId = trials.get(0).getTrialDbId(); - // Add environmentIds. - envIds.add(getEnvId(importResult, 0)); - envIds.add(getEnvId(importResult, 1)); + + envIds.clear(); + brAPIStudyDAO.getStudies(program.getId()) + .forEach(study -> envIds.add(study.getStudyDbId())); } @Test @@ -358,6 +361,44 @@ public void testGetOUById() { assertEquals(ou.get("observationUnitDbId").getAsString(), brAPIObservationUnitSingleResponse.get().getResult().getObservationUnitDbId()); } + @Test + public void testGetOUListByStudyDbId() { + String studyDbId = envIds.get(0); + + Flowable> call = + client.exchange( + GET(String.format( + "/programs/%s/brapi/v2/observationunits?studyDbId=%s", + program.getId(), + studyDbId)) + .bearerAuth("test-registered-user"), + String.class); + + HttpResponse response = call.blockingFirst(); + + assertEquals(HttpStatus.OK, response.getStatus()); + + JsonObject responseObj = + gson.fromJson( + response.body(), + JsonObject.class); + + JsonArray observationUnits = + responseObj + .getAsJsonObject("result") + .getAsJsonArray("data"); + + assertEquals(1, observationUnits.size()); + + assertEquals( + studyDbId, + observationUnits + .get(0) + .getAsJsonObject() + .get("studyDbId") + .getAsString()); // NEW: returned ID remains real studyDbId + } + private File writeDataToFile(List> data, List traits) throws IOException { File file = File.createTempFile("test", ".csv"); @@ -376,18 +417,6 @@ private File writeDataToFile(List> data, List traits) return file; } - private String getEnvId(JsonObject result, int index) { - return result - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(index).getAsJsonObject() - .get("study").getAsJsonObject() - .get("brAPIObject").getAsJsonObject() - .get("externalReferences").getAsJsonArray() - .get(2).getAsJsonObject() - .get("referenceId").getAsString(); - } - private List createGermplasm(int numToCreate) { List germplasm = new ArrayList<>(); for (int i = 0; i < numToCreate; i++) { diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationsControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationsControllerIntegrationTest.java index 59cd33630..41fe4b7f6 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationsControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPIObservationsControllerIntegrationTest.java @@ -40,6 +40,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.dao.BrAPIStudyDAO; import org.breedinginsight.brapps.importer.ImportTestUtils; import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation; import org.breedinginsight.dao.db.enums.DataType; @@ -92,6 +93,8 @@ public class BrAPIObservationsControllerIntegrationTest extends BrAPITest { private OntologyService ontologyService; @Inject private BrAPIGermplasmDAO germplasmDAO; + @Inject + private BrAPIStudyDAO brAPIStudyDAO; @Inject @Client("/${micronaut.bi.api.version}") @@ -199,9 +202,10 @@ void setup() throws Exception { .get(0).getAsJsonObject() .get("trial").getAsJsonObject() .get("id").getAsString(); - // Add environmentIds. - envIds.add(getEnvId(importResult, 0)); - envIds.add(getEnvId(importResult, 1)); + + envIds.clear(); + brAPIStudyDAO.getStudies(program.getId()) + .forEach(study -> envIds.add(study.getStudyDbId())); } @Test @@ -329,6 +333,10 @@ public void testGetObsByStudyDbId() { JsonArray observations = responseObj.getAsJsonObject("result").getAsJsonArray("data"); assertEquals(2, observations.size()); + for (JsonElement observation : observations) { + assertEquals(envIds.get(0), observation.getAsJsonObject().get("studyDbId").getAsString()); + } + // Check the observation values, keep in mind the order of results is not guaranteed. Float value1 = observations.get(0).getAsJsonObject().get("value").getAsFloat(); Float value2 = observations.get(1).getAsJsonObject().get("value").getAsFloat(); @@ -422,18 +430,6 @@ private File writeDataToFile(List> data, List traits) return file; } - private String getEnvId(JsonObject result, int index) { - return result - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(index).getAsJsonObject() - .get("study").getAsJsonObject() - .get("brAPIObject").getAsJsonObject() - .get("externalReferences").getAsJsonArray() - .get(2).getAsJsonObject() - .get("referenceId").getAsString(); - } - private List createGermplasm(int numToCreate) { List germplasm = new ArrayList<>(); for (int i = 0; i < numToCreate; i++) { diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java index fdd0df29b..b4d4ea906 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java @@ -181,6 +181,21 @@ public void testGetStudyById() { assertEquals(2, studies.size()); JsonObject study = studies.get(0).getAsJsonObject(); + String studyExternalReferenceId = null; + + for (JsonElement reference : study.getAsJsonArray("externalReferences")) { + JsonObject externalReference = reference.getAsJsonObject(); + + if (externalReference.get("referenceSource").getAsString().endsWith("/studies")) { + studyExternalReferenceId = externalReference.get("referenceId").getAsString(); + break; + } + } + + assertNotNull(studyExternalReferenceId); + + assertNotEquals(studyExternalReferenceId, study.get("studyDbId").getAsString()); + Flowable> studyCall = client.exchange( GET(String.format("/programs/%s/brapi/v2/studies/%s", program.getId(), study.get("studyDbId").getAsString())) .bearerAuth("test-registered-user"), diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIV2ObservationVariableControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIV2ObservationVariableControllerIntegrationTest.java index f8a99ac35..80b1dde56 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIV2ObservationVariableControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPIV2ObservationVariableControllerIntegrationTest.java @@ -41,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.dao.BrAPIStudyDAO; import org.breedinginsight.brapi.v2.model.request.query.ExperimentQuery; import org.breedinginsight.brapi.v2.services.BrAPITrialService; import org.breedinginsight.brapps.importer.ImportTestUtils; @@ -92,7 +93,9 @@ public class BrAPIV2ObservationVariableControllerIntegrationTest extends BrAPITe @Inject private BrAPIGermplasmDAO germplasmDAO; @Inject - BrAPITrialService brAPITrialService; + private BrAPITrialService brAPITrialService; + @Inject + private BrAPIStudyDAO brAPIStudyDAO; @Inject @Client("/${micronaut.bi.api.version}") @@ -192,7 +195,7 @@ void setup() throws Exception { rows.add(row2); // Import test experiment, environments, and any observations - JsonObject importResult = importTestUtils.uploadAndFetchWorkflow( + importTestUtils.uploadAndFetchWorkflow( writeDataToFile(rows, expTraits), null, true, @@ -208,10 +211,9 @@ void setup() throws Exception { experimentId = trial.getTrialDbId(); - - // Add environmentIds. - envIds.add(getEnvId(importResult, 0)); - envIds.add(getEnvId(importResult, 1)); + envIds.clear(); + brAPIStudyDAO.getStudies(program.getId()) + .forEach(study -> envIds.add(study.getStudyDbId())); } @Test @@ -329,6 +331,25 @@ public void testGetVariableById() { assertEquals(traits.get(0).getScale().getValidValueMax(), scale.getAsJsonObject("validValues").get("max").getAsInt()); } + @Test + public void testGetVariablesByStudyDbId() { + Flowable> call = client.exchange( + GET(String.format( + "/programs/%s/brapi/v2/variables?studyDbId=%s", + program.getId(), envIds.get(0))) + .bearerAuth("test-registered-user"), String.class); + + HttpResponse response = call.blockingFirst(); + + assertEquals(HttpStatus.OK, response.getStatus()); + + JsonObject responseObj = gson.fromJson(response.body(), JsonObject.class); + + JsonArray variables = responseObj.getAsJsonObject("result").getAsJsonArray("data"); + + assertEquals(2, variables.size()); + } + private File writeDataToFile(List> data, List traits) throws IOException { File file = File.createTempFile("test", ".csv"); @@ -347,18 +368,6 @@ private File writeDataToFile(List> data, List traits) return file; } - private String getEnvId(JsonObject result, int index) { - return result - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(index).getAsJsonObject() - .get("study").getAsJsonObject() - .get("brAPIObject").getAsJsonObject() - .get("externalReferences").getAsJsonArray() - .get(2).getAsJsonObject() - .get("referenceId").getAsString(); - } - private List createGermplasm(int numToCreate) { List germplasm = new ArrayList<>(); for (int i = 0; i < numToCreate; i++) { diff --git a/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAOUnitTest.java b/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAOUnitTest.java index 90b8d90b1..3dda7b8e3 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAOUnitTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAOUnitTest.java @@ -17,12 +17,14 @@ package org.breedinginsight.brapi.v2.dao; import io.reactivex.functions.Function; +import io.reactivex.functions.Function3; import lombok.SneakyThrows; import org.brapi.client.v2.BrAPIClient; import org.brapi.client.v2.model.queryParams.core.StudyQueryParams; import org.brapi.v2.model.BrAPIExternalReference; import org.brapi.v2.model.core.BrAPIProgram; import org.brapi.v2.model.core.BrAPIStudy; +import org.brapi.v2.model.core.request.BrAPIStudySearchRequest; import org.breedinginsight.brapps.importer.daos.ImportDAO; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.daos.ProgramDAO; @@ -39,9 +41,11 @@ import java.lang.reflect.Field; import java.util.List; +import java.util.Optional; 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.*; @@ -120,49 +124,38 @@ void getStudiesUsesDirectBrAPIGetInsteadOfProgramCache() { @Test @SneakyThrows - void getStudiesByEnvironmentIdsUsesDirectBrAPIGetInsteadOfProgramCache() { - BrAPIStudy matchingStudy = study(environmentId, "Env1 [TEST-1]"); - BrAPIStudy otherStudy = study(UUID.randomUUID(), "Env2 [TEST-2]"); + void getStudiesByStudyDbIdUsesDirectBrAPISearch() { + String studyDbId = UUID.randomUUID().toString(); + BrAPIStudy study = new BrAPIStudy().studyDbId(studyDbId).studyName("Env1"); - when(brAPIDAOUtil.get(any(Function.class), any(StudyQueryParams.class))) - .thenReturn(List.of(matchingStudy, otherStudy)); + when(brAPIDAOUtil.search(any(Function.class), any(Function3.class), any(BrAPIStudySearchRequest.class))) + .thenReturn(List.of(study)); - List result = studyDAO.getStudiesByEnvironmentIds(List.of(environmentId), program); + List result = studyDAO.getStudiesByStudyDbId( + List.of(studyDbId), program); assertEquals(1, result.size()); - assertEquals("Env1", result.get(0).getStudyName()); - ArgumentCaptor queryParamsCaptor = ArgumentCaptor.forClass(StudyQueryParams.class); - verify(brAPIDAOUtil).get(any(Function.class), queryParamsCaptor.capture()); - assertEquals("brapi-program-1", queryParamsCaptor.getValue().programDbId()); - assertEquals(0, queryParamsCaptor.getValue().page()); - assertEquals(1000, queryParamsCaptor.getValue().pageSize()); - verify(programCache, never()).get(any(UUID.class)); - } + assertEquals(studyDbId, result.get(0).getStudyDbId()); - @Test - @SneakyThrows - void getStudiesByEnvironmentIdsReturnsEmptyListWhenStudyIsNotFoundInBrAPI() { - when(brAPIDAOUtil.get(any(Function.class), any(StudyQueryParams.class))) - .thenReturn(List.of()); + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(BrAPIStudySearchRequest.class); + + verify(brAPIDAOUtil).search(any(Function.class), any(Function3.class), requestCaptor.capture()); - List result = studyDAO.getStudiesByEnvironmentIds(List.of(environmentId), program); + assertEquals(List.of("brapi-program-1"), requestCaptor.getValue().getProgramDbIds()); + assertEquals(List.of(studyDbId), requestCaptor.getValue().getStudyDbIds()); - assertEquals(0, result.size()); verify(programCache, never()).get(any(UUID.class)); } @Test @SneakyThrows - void getStudiesByEnvironmentIdsReturnsEmptyListWhenNoStudyExternalReferenceMatches() { - BrAPIStudy study = study(UUID.randomUUID(), "Env2 [TEST-2]"); - - when(brAPIDAOUtil.get(any(Function.class), any(StudyQueryParams.class))) - .thenReturn(List.of(study)); + void getStudyByDbIdReturnsEmptyWhenBrAPIDoesNotFindStudy() { + when(brAPIDAOUtil.search(any(Function.class), any(Function3.class), any(BrAPIStudySearchRequest.class))) + .thenReturn(List.of()); - List result = studyDAO.getStudiesByEnvironmentIds(List.of(environmentId), program); + Optional result = studyDAO.getStudyByDbId(UUID.randomUUID().toString(), program); - assertEquals(0, result.size()); - verify(programCache, never()).get(any(UUID.class)); + assertTrue(result.isEmpty()); } private BrAPIStudy study(UUID environmentId, String studyName) { 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 c10778b35..a4afcedb1 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/services/BrAPITrialServiceUnitTest.java @@ -247,6 +247,53 @@ void getDatasetDataThrowsWhenSeasonYearIsNull() throws Exception { assertEquals("Env Year not found for Study DbId = 'study-1'.", exception.getMessage()); } + @Test + void exportObservationsFiltersByStudyDbId() throws Exception { + ExperimentExportQuery params = exportQuery(EXPORT_DATASET_ID); + + setField(params, "environments", "study-1"); + + BrAPIStudy secondStudy = new BrAPIStudy(); + secondStudy.setStudyDbId("study-2"); + secondStudy.setStudyName("Environment 2"); + secondStudy.setLocationName("Location 2"); + secondStudy.setSeasons(List.of("season-2")); + + BrAPIObservationUnit requestedObservationUnit = createObservationUnit("ou-db-1", "plot-1"); + + when(trialDAO.getTrialsByExperimentIds(anyCollection(), eq(program))) + .thenReturn(List.of(experiment)); + + when(studyDAO.getStudiesByBrAPITrialExRefId(any(UUID.class), eq(program))) + .thenReturn(List.of(study, secondStudy)); + + when(observationUnitDAO.getObservationUnitsForDatasetAndEnvs(EXPORT_DATASET_ID, List.of("study-1"), program)) + .thenReturn(List.of(requestedObservationUnit)); + + when(seasonDAO.getSeasonById("season-1", program.getId())).thenReturn(season); + + when(listDAO.getListsByTypeAndExternalRef(any(), eq(program.getId()), any(), any())) + .thenReturn(Collections.emptyList()); + + when(observationDAO.getObservationsByObservationUnits(anyCollection(), eq(program))) + .thenReturn(Collections.emptyList()); + + when(germplasmDAO.getGermplasmsByDBID(anyList(), eq(program.getId()))) + .thenReturn(List.of(germplasm)); + + DownloadFile downloadFile = service.exportObservations(program, UUID.fromString("11111111-1111-1111-1111-111111111111"), params); + + Table exportTable = FileUtil.parseTableFromCsv(new ByteArrayInputStream(downloadFile.getStreamedFile() + .getInputStream() + .readAllBytes())); + + assertEquals(1, exportTable.rowCount()); + + assertEquals("Environment 1", exportTable.stringColumn(Columns.ENV).get(0)); + + verify(observationUnitDAO).getObservationUnitsForDatasetAndEnvs(EXPORT_DATASET_ID, List.of("study-1"), program); + } + private ExperimentExportQuery exportQuery(String datasetId) throws Exception { ExperimentExportQuery params = new ExperimentExportQuery(); setField(params, "datasetId", datasetId);