From 60ffbf0559f33eb42b6afcf63ef195afdebd6b49 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Thu, 8 Aug 2024 16:27:31 -0400 Subject: [PATCH 01/12] [BI-2258] - added authorization to datasetExport --- .../v1/controller/ExperimentController.java | 39 +++++++++++++---- .../daos/ExperimentalCollaboratorDAO.java | 42 +++++++++++++++++++ .../ExperimentalCollaboratorService.java | 13 ++++++ 3 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java b/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java index 77214c9cf..5a537de6c 100644 --- a/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java +++ b/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java @@ -10,19 +10,16 @@ import io.micronaut.security.rules.SecurityRule; import lombok.extern.slf4j.Slf4j; import org.brapi.client.v2.model.exceptions.ApiException; -import org.breedinginsight.api.auth.ProgramSecured; -import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; +import org.breedinginsight.api.auth.*; import org.breedinginsight.api.model.v1.request.SubEntityDatasetRequest; import org.breedinginsight.api.model.v1.response.Response; import org.breedinginsight.brapi.v2.model.request.query.ExperimentExportQuery; import org.breedinginsight.brapi.v2.services.BrAPITrialService; -import org.breedinginsight.model.Dataset; -import org.breedinginsight.model.DatasetMetadata; -import org.breedinginsight.model.DownloadFile; -import org.breedinginsight.model.Program; +import org.breedinginsight.model.*; +import org.breedinginsight.services.ExperimentalCollaboratorService; import org.breedinginsight.services.ProgramService; +import org.breedinginsight.services.ProgramUserService; import org.breedinginsight.services.exceptions.DoesNotExistException; -import org.breedinginsight.services.exceptions.UnprocessableEntityException; import org.breedinginsight.utilities.response.mappers.ExperimentQueryMapper; import javax.inject.Inject; @@ -38,12 +35,23 @@ public class ExperimentController { private final BrAPITrialService experimentService; private final ExperimentQueryMapper experimentQueryMapper; private final ProgramService programService; + private final SecurityService securityService; + private final ProgramUserService programUserService; + private final ExperimentalCollaboratorService experimentalCollaboratorService; @Inject - public ExperimentController(BrAPITrialService experimentService, ExperimentQueryMapper experimentQueryMapper, ProgramService programService) { + public ExperimentController(BrAPITrialService experimentService, + ExperimentQueryMapper experimentQueryMapper, + ProgramService programService, + SecurityService securityService, + ProgramUserService programUserService, + ExperimentalCollaboratorService experimentalCollaboratorService) { this.experimentService = experimentService; this.experimentQueryMapper = experimentQueryMapper; this.programService = programService; + this.securityService = securityService; + this.programUserService = programUserService; + this.experimentalCollaboratorService = experimentalCollaboratorService; } @Get("/${micronaut.bi.api.version}/programs/{programId}/experiments/{experimentId}/export{?queryParams*}") @@ -59,6 +67,21 @@ public HttpResponse datasetExport( return HttpResponse.notFound(); } + AuthenticatedUser user = securityService.getUser(); + Optional programUser = programUserService.getProgramUserbyId(programId, user.getId()); + if (programUser.isEmpty()) { + return HttpResponse.notFound(); + } + boolean isExperimentalCollaborator = programUser.get().getRoles().stream().anyMatch(x -> ProgramSecuredRole.getEnum(x.getDomain()).equals(ProgramSecuredRole.EXPERIMENTAL_COLLABORATOR)); + // If the programUser is an experimental collaborator, ensure they're authorized to access the experiment. + if (isExperimentalCollaborator) { + if (!experimentalCollaboratorService.isAuthorized(programUser.get().getId(), experimentId)){ + log.debug("Experimental Collaborator {} tried to access experiment {} - FORBIDDEN.", programUser.get().getId(), experimentId); + return HttpResponse.status(HttpStatus.FORBIDDEN); + } + log.debug("Experimental Collaborator {} tried to access experiment {} - OK.", programUser.get().getId(), experimentId); + } + // if a list of environmentIds are sent, return multiple files (zipped), // else if a single environmentId is sent, return single file (CSV/Excel), // else (if no environmentIds are sent), return a single file (CSV/Excel) including all Environments. diff --git a/src/main/java/org/breedinginsight/daos/ExperimentalCollaboratorDAO.java b/src/main/java/org/breedinginsight/daos/ExperimentalCollaboratorDAO.java index b3ac8bbaf..b0b2717d8 100644 --- a/src/main/java/org/breedinginsight/daos/ExperimentalCollaboratorDAO.java +++ b/src/main/java/org/breedinginsight/daos/ExperimentalCollaboratorDAO.java @@ -19,11 +19,17 @@ import lombok.extern.slf4j.Slf4j; import org.breedinginsight.dao.db.tables.daos.ExperimentProgramUserRoleDao; +import org.breedinginsight.dao.db.tables.pojos.ExperimentProgramUserRoleEntity; import org.jooq.Configuration; import org.jooq.DSLContext; import javax.inject.Inject; import javax.inject.Singleton; +import java.util.List; +import java.util.UUID; + +import static org.breedinginsight.dao.db.Tables.EXPERIMENT_PROGRAM_USER_ROLE; +import static org.breedinginsight.dao.db.Tables.PROGRAM_USER_ROLE; @Slf4j @Singleton @@ -36,4 +42,40 @@ public ExperimentalCollaboratorDAO(Configuration config, DSLContext dsl) { super(config); this.dsl = dsl; } + + public List fetchByProgramUserIdAndExperimentId(UUID programUserRoleId, UUID experimentId) { + // Only returns results for active program_user_role rows. + return dsl.select(EXPERIMENT_PROGRAM_USER_ROLE.fields()) + .from(EXPERIMENT_PROGRAM_USER_ROLE) + .innerJoin(PROGRAM_USER_ROLE).on(EXPERIMENT_PROGRAM_USER_ROLE.PROGRAM_USER_ROLE_ID.eq(PROGRAM_USER_ROLE.ID)) + .where(EXPERIMENT_PROGRAM_USER_ROLE.PROGRAM_USER_ROLE_ID.eq(programUserRoleId)) + .and(EXPERIMENT_PROGRAM_USER_ROLE.EXPERIMENT_ID.eq(experimentId)) + .and(PROGRAM_USER_ROLE.ACTIVE.eq(true)) + .fetchInto(ExperimentProgramUserRoleEntity.class); + } + + public List getExperimentIds(UUID programUserRoleId, boolean activeOnly) { + // If activeOnly, this will only return results if the program_user_role row is active. + if (activeOnly) + { + return getExperimentIdsIfActive(programUserRoleId); + } + return getExperimentIds(programUserRoleId); + } + + private List getExperimentIdsIfActive(UUID programUserRoleId) { + return dsl.select(EXPERIMENT_PROGRAM_USER_ROLE.EXPERIMENT_ID) + .from(EXPERIMENT_PROGRAM_USER_ROLE) + .join(PROGRAM_USER_ROLE).on(EXPERIMENT_PROGRAM_USER_ROLE.PROGRAM_USER_ROLE_ID.eq(PROGRAM_USER_ROLE.ID)) + .where(EXPERIMENT_PROGRAM_USER_ROLE.PROGRAM_USER_ROLE_ID.eq(programUserRoleId)) + .and(PROGRAM_USER_ROLE.ACTIVE.eq(true)) + .fetchInto(UUID.class); + } + + private List getExperimentIds(UUID programUserRoleId) { + return dsl.select(EXPERIMENT_PROGRAM_USER_ROLE.EXPERIMENT_ID) + .from(EXPERIMENT_PROGRAM_USER_ROLE) + .where(EXPERIMENT_PROGRAM_USER_ROLE.PROGRAM_USER_ROLE_ID.eq(programUserRoleId)) + .fetchInto(UUID.class); + } } diff --git a/src/main/java/org/breedinginsight/services/ExperimentalCollaboratorService.java b/src/main/java/org/breedinginsight/services/ExperimentalCollaboratorService.java index c8aaddf07..6f3ce29e1 100644 --- a/src/main/java/org/breedinginsight/services/ExperimentalCollaboratorService.java +++ b/src/main/java/org/breedinginsight/services/ExperimentalCollaboratorService.java @@ -21,6 +21,8 @@ import org.breedinginsight.daos.ExperimentalCollaboratorDAO; import javax.inject.Inject; +import java.util.List; +import java.util.UUID; @Slf4j public class ExperimentalCollaboratorService { @@ -31,4 +33,15 @@ public class ExperimentalCollaboratorService { public ExperimentalCollaboratorService(ExperimentalCollaboratorDAO experimentalCollaboratorDAO) { this.experimentalCollaboratorDAO = experimentalCollaboratorDAO; } + + public boolean isAuthorized(UUID programUserRoleId, UUID experimentId) { + return !experimentalCollaboratorDAO + .fetchByProgramUserIdAndExperimentId(programUserRoleId, experimentId) + .isEmpty(); + } + + public List getAuthorizedExperimentIds(UUID programUserRoleId) { + // Get the list of experimentIds associated with the programUserRoleId, empty if the programUserRole is active. + return experimentalCollaboratorDAO.getExperimentIds(programUserRoleId, true); + } } From 86e0892924feada1202f15f9b367bc50419366a7 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Fri, 9 Aug 2024 13:52:43 -0400 Subject: [PATCH 02/12] [BI-2258] - added filtering for Experimental Collaborator --- .../brapi/v2/BrAPIStudiesController.java | 49 ++++++++++++++++--- .../brapi/v2/BrAPITrialsController.java | 47 ++++++++++++++++-- .../brapi/v2/dao/BrAPIStudyDAO.java | 22 +++++++-- .../brapi/v2/services/BrAPIStudyService.java | 4 ++ .../brapi/v2/services/BrAPITrialService.java | 4 ++ 5 files changed, 112 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java index 0382ecdb0..14060280c 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java @@ -30,8 +30,7 @@ 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.ProgramSecured; -import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; +import org.breedinginsight.api.auth.*; 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; @@ -41,7 +40,10 @@ 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.utilities.Utilities; import org.breedinginsight.utilities.response.ResponseUtils; import org.breedinginsight.utilities.response.mappers.StudyQueryMapper; @@ -62,14 +64,26 @@ public class BrAPIStudiesController { private final BrAPIStudyService studyService; private final StudyQueryMapper studyQueryMapper; private final ProgramService programService; + private final SecurityService securityService; + private final ProgramUserService programUserService; + private final ExperimentalCollaboratorService experimentalCollaboratorService; @Inject - public BrAPIStudiesController(BrAPIStudyService studyService, StudyQueryMapper studyQueryMapper, @Property(name = "brapi.server.reference-source") String referenceSource, ProgramService programService) { + 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; + this.experimentalCollaboratorService = experimentalCollaboratorService; } @Get("/studies{?queryParams*}") @@ -80,11 +94,32 @@ public HttpResponse>>> getStudies( @QueryValue @QueryValid(using = StudyQueryMapper.class) @Valid StudyQuery queryParams) { try { log.debug("fetching studies for program: " + programId); + List studies; + + AuthenticatedUser user = securityService.getUser(); + Optional programUser = programUserService.getProgramUserbyId(programId, user.getId()); + if (programUser.isEmpty()) { + return HttpResponse.notFound(); + } + boolean isExperimentalCollaborator = programUser.get().getRoles().stream().anyMatch(x -> ProgramSecuredRole.getEnum(x.getDomain()).equals(ProgramSecuredRole.EXPERIMENTAL_COLLABORATOR)); + + if (isExperimentalCollaborator) { + Optional program = programService.getById(programId); + if (program.isEmpty()) { + return HttpResponse.notFound(); + } + + List experimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(programUser.get().getId()); + studies = studyService.getStudiesByExperimentIds(program.get(), experimentIds) + .stream() + .peek(this::setDbIds) + .collect(Collectors.toList()); } else { + studies = studyService.getStudies(programId) + .stream() + .peek(this::setDbIds) + .collect(Collectors.toList()); + } - List studies = studyService.getStudies(programId) - .stream() - .peek(this::setDbIds) - .collect(Collectors.toList()); queryParams.setSortField(studyQueryMapper.getDefaultSortField()); queryParams.setSortOrder(studyQueryMapper.getDefaultSortOrder()); SearchRequest searchRequest = queryParams.constructSearchRequest(); diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java index 666115efa..9d60166c3 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java @@ -11,8 +11,7 @@ import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.v2.model.core.BrAPITrial; import org.brapi.v2.model.core.response.BrAPITrialSingleResponse; -import org.breedinginsight.api.auth.ProgramSecured; -import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; +import org.breedinginsight.api.auth.*; 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; @@ -21,6 +20,11 @@ import org.breedinginsight.brapi.v2.model.request.query.ExperimentQuery; import org.breedinginsight.brapi.v2.services.BrAPITrialService; 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; @@ -28,7 +32,9 @@ import javax.inject.Inject; import javax.validation.Valid; +import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; @@ -40,12 +46,26 @@ public class BrAPITrialsController { private final String referenceSource; private final BrAPITrialService experimentService; private final ExperimentQueryMapper experimentQueryMapper; + private final SecurityService securityService; + private final ProgramUserService programUserService; + private final ExperimentalCollaboratorService experimentalCollaboratorService; + private final ProgramService programService; @Inject - public BrAPITrialsController(BrAPITrialService experimentService, ExperimentQueryMapper experimentQueryMapper, @Property(name = "brapi.server.reference-source") String referenceSource) { + public BrAPITrialsController(BrAPITrialService experimentService, + ExperimentQueryMapper experimentQueryMapper, + @Property(name = "brapi.server.reference-source") String referenceSource, + SecurityService securityService, + ProgramUserService programUserService, + ExperimentalCollaboratorService experimentalCollaboratorService, + ProgramService programService) { this.experimentService = experimentService; this.experimentQueryMapper = experimentQueryMapper; this.referenceSource = referenceSource; + this.securityService = securityService; + this.programUserService = programUserService; + this.experimentalCollaboratorService = experimentalCollaboratorService; + this.programService = programService; } @Get("/trials{?queryParams*}") @@ -55,9 +75,28 @@ public HttpResponse>>> getExperiments( @PathVariable("programId") UUID programId, @QueryValue @QueryValid(using = ExperimentQueryMapper.class) @Valid ExperimentQuery queryParams) { try { + List experiments = new ArrayList<>(); log.debug("fetching trials for program: " + programId); - List experiments = experimentService.getExperiments(programId).stream().peek(this::setDbIds).collect(Collectors.toList()); + AuthenticatedUser user = securityService.getUser(); + Optional programUser = programUserService.getProgramUserbyId(programId, user.getId()); + if (programUser.isEmpty()) { + return HttpResponse.notFound(); + } + boolean isExperimentalCollaborator = programUser.get().getRoles().stream().anyMatch(x -> ProgramSecuredRole.getEnum(x.getDomain()).equals(ProgramSecuredRole.EXPERIMENTAL_COLLABORATOR)); + + if (isExperimentalCollaborator) { + Optional program = programService.getById(programId); + if (program.isEmpty()) { + return HttpResponse.notFound(); + } + + List experimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(programUser.get().getId()); + experiments = experimentService.getTrialsByExperimentIds(program.get(), experimentIds).stream().peek(this::setDbIds).collect(Collectors.toList()); + } else { + experiments = experimentService.getExperiments(programId).stream().peek(this::setDbIds).collect(Collectors.toList()); + } + SearchRequest searchRequest = queryParams.constructSearchRequest(); return ResponseUtils.getBrapiQueryResponse(experiments, experimentQueryMapper, queryParams, searchRequest); } catch (ApiException e) { 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 870b0579b..ca8963461 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java @@ -139,10 +139,10 @@ public List getStudiesByName(List studyNames, Program progra ); } - public List getStudiesByExperimentID(@NotNull UUID experimentID, Program program ) throws ApiException { + public List getStudiesByExperimentID(@NotNull UUID experimentID, Program program) throws ApiException { BrAPIStudySearchRequest studySearch = new BrAPIStudySearchRequest(); studySearch.programDbIds(List.of(program.getBrapiProgram().getProgramDbId())); - studySearch.addExternalReferenceIDsItem(experimentID.toString()); + studySearch.addExternalReferenceIdsItem(experimentID.toString()); studySearch.addExternalReferenceSourcesItem(Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS)); StudiesApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), StudiesApi.class); return brAPIDAOUtil.search( @@ -152,7 +152,7 @@ public List getStudiesByExperimentID(@NotNull UUID experimentID, Pro ); } - public List getStudiesByEnvironmentIds(@NotNull Collection environmentIds, Program program ) throws ApiException { + public List getStudiesByEnvironmentIds(@NotNull Collection environmentIds, Program program) throws ApiException { return programStudyCache.get(program.getId()) .entrySet() .stream() @@ -161,6 +161,22 @@ public List getStudiesByEnvironmentIds(@NotNull Collection env .collect(Collectors.toList()); } + 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)); + StudiesApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), StudiesApi.class); + return brAPIDAOUtil.search( + api::searchStudiesPost, + api::searchStudiesSearchResultsDbIdGet, + studySearch + ); + } + public List createBrAPIStudies(List brAPIStudyList, UUID programId, ImportUpload upload) throws ApiException { StudiesApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), StudiesApi.class); List createdStudies = new ArrayList<>(); 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 570609d53..846d9b4cc 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIStudyService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIStudyService.java @@ -48,4 +48,8 @@ public Optional getStudyByEnvironmentId(Program program, UUID enviro return studyDAO.getStudyByEnvironmentId(environmentId, program); } + public List getStudiesByExperimentIds(Program program, List experimentIds) throws ApiException { + return studyDAO.getStudiesByExperimentIds(experimentIds, 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 333ceae60..916193ca8 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -102,6 +102,10 @@ public List getExperiments(UUID programId) throws ApiException, Does return trialDAO.getTrials(programId); } + public List getTrialsByExperimentIds(Program program, List experimentIds) throws ApiException, DoesNotExistException { + return trialDAO.getTrialsByExperimentIds(experimentIds, program); + } + public BrAPITrial getTrialDataByUUID(UUID programId, UUID trialId, boolean stats) throws DoesNotExistException { try { BrAPITrial trial = trialDAO.getTrialById(programId,trialId).orElseThrow(() -> new DoesNotExistException("Trial does not exist")); From be4ba6f27d99b320ead938e1ef5b50e7be4aa390 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Tue, 13 Aug 2024 14:57:46 -0400 Subject: [PATCH 03/12] [BI-2258] - implemented /seasons GET endpoint --- .../brapi/v2/BrAPIV2Controller.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java index b5249516e..10da73896 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java @@ -30,10 +30,7 @@ import org.brapi.v2.model.core.BrAPIServerInfo; import org.brapi.v2.model.core.BrAPIService; import org.brapi.v2.model.core.response.BrAPIServerInfoResponse; -import org.breedinginsight.api.auth.AuthenticatedUser; -import org.breedinginsight.api.auth.ProgramSecured; -import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; -import org.breedinginsight.api.auth.SecurityService; +import org.breedinginsight.api.auth.*; import org.breedinginsight.brapi.v1.controller.BrapiVersion; import org.breedinginsight.model.ProgramBrAPIEndpoints; import org.breedinginsight.services.ProgramService; @@ -42,6 +39,7 @@ import javax.inject.Inject; import java.io.IOException; import java.util.List; +import java.util.Optional; import java.util.UUID; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -168,6 +166,18 @@ private void setBrAPIServerInfo(BrAPIServerInfo serverInfo) { serverInfo.setDocumentationURL("https://brapi.org/specification"); } + // Explicit match for /seasons GET endpoint, to allow Experimental Collaborator access. + @Get("/${micronaut.bi.api.version}/programs/{programId}" + BrapiVersion.BRAPI_V2 + "/seasons{?queryParams}") + @Produces(MediaType.APPLICATION_JSON) + @ProgramSecured(roles = {ProgramSecuredRole.SYSTEM_ADMIN, ProgramSecuredRole.PROGRAM_ADMIN, ProgramSecuredRole.READ_ONLY, ProgramSecuredRole.EXPERIMENTAL_COLLABORATOR}) + public HttpResponse getSeasons(@PathVariable("programId") UUID programId, HttpRequest request, @PathVariable Optional queryParams) { + String path = "seasons"; + if (queryParams.isPresent()) { + path = path + queryParams.get(); + } + return executeRequest(path, programId, request, "GET"); + } + @Get("/${micronaut.bi.api.version}/programs/{programId}" + BrapiVersion.BRAPI_V2 + "/{+path}") @Produces(MediaType.APPLICATION_JSON) @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.ALL}) From 17c6098bac8c9cd6d654d1ef4e7f09b566449e36 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Thu, 15 Aug 2024 12:06:14 -0400 Subject: [PATCH 04/12] [BI-2258] - added integration test for studies filtering --- .../v1/controller/ExperimentController.java | 28 +------- .../brapi/v2/BrAPIStudiesController.java | 3 +- .../brapi/v2/dao/BrAPIStudyDAO.java | 8 +-- ...BrAPIStudiesControllerIntegrationTest.java | 64 ++++++++++++++++++- ...amSecuredAnnotationRuleIntegrationTest.sql | 19 ++++++ 5 files changed, 88 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java b/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java index 5a537de6c..5a579efcd 100644 --- a/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java +++ b/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java @@ -35,23 +35,12 @@ public class ExperimentController { private final BrAPITrialService experimentService; private final ExperimentQueryMapper experimentQueryMapper; private final ProgramService programService; - private final SecurityService securityService; - private final ProgramUserService programUserService; - private final ExperimentalCollaboratorService experimentalCollaboratorService; @Inject - public ExperimentController(BrAPITrialService experimentService, - ExperimentQueryMapper experimentQueryMapper, - ProgramService programService, - SecurityService securityService, - ProgramUserService programUserService, - ExperimentalCollaboratorService experimentalCollaboratorService) { + public ExperimentController(BrAPITrialService experimentService, ExperimentQueryMapper experimentQueryMapper, ProgramService programService) { this.experimentService = experimentService; this.experimentQueryMapper = experimentQueryMapper; this.programService = programService; - this.securityService = securityService; - this.programUserService = programUserService; - this.experimentalCollaboratorService = experimentalCollaboratorService; } @Get("/${micronaut.bi.api.version}/programs/{programId}/experiments/{experimentId}/export{?queryParams*}") @@ -67,21 +56,6 @@ public HttpResponse datasetExport( return HttpResponse.notFound(); } - AuthenticatedUser user = securityService.getUser(); - Optional programUser = programUserService.getProgramUserbyId(programId, user.getId()); - if (programUser.isEmpty()) { - return HttpResponse.notFound(); - } - boolean isExperimentalCollaborator = programUser.get().getRoles().stream().anyMatch(x -> ProgramSecuredRole.getEnum(x.getDomain()).equals(ProgramSecuredRole.EXPERIMENTAL_COLLABORATOR)); - // If the programUser is an experimental collaborator, ensure they're authorized to access the experiment. - if (isExperimentalCollaborator) { - if (!experimentalCollaboratorService.isAuthorized(programUser.get().getId(), experimentId)){ - log.debug("Experimental Collaborator {} tried to access experiment {} - FORBIDDEN.", programUser.get().getId(), experimentId); - return HttpResponse.status(HttpStatus.FORBIDDEN); - } - log.debug("Experimental Collaborator {} tried to access experiment {} - OK.", programUser.get().getId(), experimentId); - } - // if a list of environmentIds are sent, return multiple files (zipped), // else if a single environmentId is sent, return single file (CSV/Excel), // else (if no environmentIds are sent), return a single file (CSV/Excel) including all Environments. diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java index 14060280c..b51203099 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java @@ -113,7 +113,8 @@ public HttpResponse>>> getStudies( studies = studyService.getStudiesByExperimentIds(program.get(), experimentIds) .stream() .peek(this::setDbIds) - .collect(Collectors.toList()); } else { + .collect(Collectors.toList()); + } else { studies = studyService.getStudies(programId) .stream() .peek(this::setDbIds) 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 ca8963461..e966ecd6a 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java @@ -139,10 +139,10 @@ public List getStudiesByName(List studyNames, Program progra ); } - public List getStudiesByExperimentID(@NotNull UUID experimentID, Program program) throws ApiException { + public List getStudiesByExperimentID(@NotNull UUID experimentId, Program program) throws ApiException { BrAPIStudySearchRequest studySearch = new BrAPIStudySearchRequest(); studySearch.programDbIds(List.of(program.getBrapiProgram().getProgramDbId())); - studySearch.addExternalReferenceIdsItem(experimentID.toString()); + studySearch.addExternalReferenceIdsItem(experimentId.toString()); studySearch.addExternalReferenceSourcesItem(Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS)); StudiesApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), StudiesApi.class); return brAPIDAOUtil.search( @@ -170,11 +170,11 @@ public List getStudiesByExperimentIds(@NotNull Collection expe } studySearch.addExternalReferenceSourcesItem(Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS)); StudiesApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), StudiesApi.class); - return brAPIDAOUtil.search( + return new ArrayList<>(processStudyForDisplay(brAPIDAOUtil.search( api::searchStudiesPost, api::searchStudiesSearchResultsDbIdGet, studySearch - ); + ), program.getKey()).values()); } public List createBrAPIStudies(List brAPIStudyList, UUID programId, ImportUpload upload) throws ApiException { diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java index 1b65042d2..f3eb3348c 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java @@ -133,7 +133,6 @@ void setup() throws Exception { program = TestUtils.insertAndFetchTestProgram(gson, client, programRequest); dsl.execute(securityFp.get("InsertProgramRolesBreeder"), testUser.getId().toString(), program.getId()); - dsl.execute(securityFp.get("InsertSystemRoleAdmin"), testUser.getId().toString()); // Get experiment import map Flowable> call = client.exchange( @@ -204,8 +203,65 @@ void setup() throws Exception { // Add environmentIds. envIds.add(getEnvId(importResult, 0)); envIds.add(getEnvId(importResult, 1)); + + // Experimental Collaborator user. + User collaborator = userDAO.getUserByOrcId(TestTokenValidator.OTHER_TEST_USER_ORCID).orElseThrow(Exception::new); + dsl.execute(securityFp.get("InsertProgramRolesExperimentalCollaborator"), collaborator.getId().toString(), program.getId().toString()); + // Add collaborator to experiment. + // Make another test experiment import. + Map row3 = makeExpImportRow("Env3", "xyz"); + Map row4 = makeExpImportRow("Env4", "xyz"); + + List> newrows = new ArrayList<>(); + + newrows.add(row3); + newrows.add(row4); + + // Import test experiment, environments, and any observations + importResult = importTestUtils.uploadAndFetchWorkflow( + writeDataToFile(newrows, new ArrayList<>()), + null, + true, + client, + program, + mappingId, + newExperimentWorkflowId); + String newExperimentId = importResult + .get("preview").getAsJsonObject() + .get("rows").getAsJsonArray() + .get(0).getAsJsonObject() + .get("trial").getAsJsonObject() + .get("id").getAsString(); + + // Refetch collaborator, since we updated program_user_roles. + collaborator = userDAO.getUserByOrcId(TestTokenValidator.OTHER_TEST_USER_ORCID).orElseThrow(Exception::new); + dsl.execute(securityFp.get("AddCollaborator"), newExperimentId, collaborator.getProgramRoles().get(0).getId()); + } + + @Test + public void testGetStudiesAsExperimentalCollaborator() { + // This test ensures that studies are filtered for experimental collaborators. + Flowable> call = client.exchange( + GET(String.format("/programs/%s/brapi/v2/studies", program.getId())) + .bearerAuth("other-registered-user"), + String.class + ); + + HttpResponse response = call.blockingFirst(); + assertEquals(HttpStatus.OK, response.getStatus()); + + JsonObject responseObj = gson.fromJson(response.body(), JsonObject.class); + JsonArray studies = responseObj.getAsJsonObject("result").getAsJsonArray("data"); + assertEquals(2, studies.size()); + List studyNames = new ArrayList<>(); + for(var study : studies) { + studyNames.add(study.getAsJsonObject().get("studyName").getAsString()); + } + assertTrue(studyNames.contains("Env3")); + assertTrue(studyNames.contains("Env4")); } + @Test @SneakyThrows public void testPostGetStudiesNotFound() { @@ -395,10 +451,14 @@ private List createTraits(int numToCreate) { } private Map makeExpImportRow(String environment) { + return makeExpImportRow(environment, "Test Exp"); + } + + private Map makeExpImportRow(String environment, String expTitle) { Map row = new HashMap<>(); row.put(ExperimentObservation.Columns.GERMPLASM_GID, "1"); row.put(ExperimentObservation.Columns.TEST_CHECK, "T"); - row.put(ExperimentObservation.Columns.EXP_TITLE, "Test Exp"); + row.put(ExperimentObservation.Columns.EXP_TITLE, expTitle); row.put(ExperimentObservation.Columns.EXP_UNIT, "Plot"); row.put(ExperimentObservation.Columns.EXP_TYPE, "Phenotyping"); row.put(ExperimentObservation.Columns.ENV, environment); diff --git a/src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql b/src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql index 17f08c19d..f8c7afc28 100644 --- a/src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql +++ b/src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql @@ -34,6 +34,25 @@ insert into program (species_id, name, abbreviation, documentation_url, objectiv select species.id, 'Program4', 'test', 'localhost:8080', 'To test things', bi_user.id, bi_user.id, 'PD' from species join bi_user on bi_user.name = 'Test User' limit 1; +-- name: InsertProgramRolesExperimentalCollaborator + +insert into program_user_role (user_id, program_id, role_id, created_by, updated_by) +select +?::uuid, ?::uuid, role.id, bi_user.id, bi_user.id +from +bi_user +join role on role.domain = 'Experimental Collaborator'; + +-- name: AddCollaborator + +insert into experiment_program_user_role (experiment_id, program_user_role_id, created_by, updated_by) +select +?::uuid, ?::uuid, bi_user.id, bi_user.id +from +bi_user +where bi_user.name = 'system'; +; + -- name: InsertProgramRolesMember insert into program_user_role (user_id, program_id, role_id, created_by, updated_by) From ea4c34168a0d18580650254579772d51978a2fbf Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Fri, 16 Aug 2024 17:11:08 -0400 Subject: [PATCH 05/12] [BI-2258] - added integration test for trials filtering --- .../brapi/v2/dao/impl/BrAPITrialDAOImpl.java | 8 +- ...BrAPIStudiesControllerIntegrationTest.java | 291 +---------------- .../brapi/v2/BrAPITestUtils.java | 306 ++++++++++++++++++ .../BrAPITrialsControllerIntegrationTest.java | 135 ++++++++ 4 files changed, 453 insertions(+), 287 deletions(-) create mode 100644 src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java create mode 100644 src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java 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 a1670bf3c..066c7017d 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 @@ -269,14 +269,12 @@ public List getTrialsByExperimentIds(Collection experimentIds, BrAPITrialSearchRequest trialSearch = new BrAPITrialSearchRequest(); trialSearch.programDbIds(List.of(program.getBrapiProgram().getProgramDbId())); trialSearch.externalReferenceSources(List.of(referenceSource + "/" + ExternalReferenceSource.TRIALS.getName())); - trialSearch.externalReferenceIDs(experimentIds.stream().map(id -> id.toString()).collect(Collectors.toList())); + trialSearch.externalReferenceIds(experimentIds.stream().map(UUID::toString).collect(Collectors.toList())); TrialsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(program.getId()), TrialsApi.class); - return brAPIDAOUtil.search( + return processExperimentsForDisplay(brAPIDAOUtil.search( api::searchTrialsPost, api::searchTrialsSearchResultsDbIdGet, trialSearch - ); + ), program.getKey()); } } - - diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java index f3eb3348c..d1e6797c4 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java @@ -18,48 +18,23 @@ package org.breedinginsight.brapi.v2; import com.google.gson.*; -import io.kowalski.fannypack.FannyPack; -import io.micronaut.context.annotation.Property; +import groovy.lang.Tuple2; import io.micronaut.http.HttpResponse; import io.micronaut.http.HttpStatus; import io.micronaut.http.MediaType; import io.micronaut.http.client.RxHttpClient; import io.micronaut.http.client.annotation.Client; import io.micronaut.http.client.exceptions.HttpClientResponseException; -import io.micronaut.http.netty.cookies.NettyCookie; import io.micronaut.test.extensions.junit5.annotation.MicronautTest; import io.reactivex.Flowable; import lombok.SneakyThrows; -import org.brapi.v2.model.BrAPIExternalReference; import org.brapi.v2.model.core.BrAPIStudy; import org.brapi.v2.model.core.response.BrAPIStudySingleResponse; -import org.brapi.v2.model.germ.BrAPIGermplasm; import org.breedinginsight.BrAPITest; -import org.breedinginsight.TestUtils; -import org.breedinginsight.api.auth.AuthenticatedUser; -import org.breedinginsight.api.model.v1.request.ProgramRequest; -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.brapps.importer.ImportTestUtils; -import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation; -import org.breedinginsight.dao.db.enums.DataType; -import org.breedinginsight.dao.db.tables.pojos.SpeciesEntity; -import org.breedinginsight.daos.SpeciesDAO; -import org.breedinginsight.daos.UserDAO; import org.breedinginsight.model.*; -import org.breedinginsight.services.OntologyService; -import org.breedinginsight.services.exceptions.ValidatorException; -import org.breedinginsight.services.parsers.experiment.ExperimentFileColumns; -import org.breedinginsight.services.writers.CSVWriter; -import org.jooq.DSLContext; import org.junit.jupiter.api.*; import javax.inject.Inject; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileOutputStream; -import java.io.IOException; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.*; @@ -74,168 +49,25 @@ public class BrAPIStudiesControllerIntegrationTest extends BrAPITest { private Program program; private String experimentId; - private List envIds = new ArrayList<>(); - private final List> rows = new ArrayList<>(); - private final List columns = ExperimentFileColumns.getOrderedColumns(); - private List traits; - - @Property(name = "brapi.server.reference-source") - private String BRAPI_REFERENCE_SOURCE; - @Inject - private DSLContext dsl; - @Inject - private UserDAO userDAO; - @Inject - private SpeciesDAO speciesDAO; - @Inject - private OntologyService ontologyService; - @Inject - private BrAPIGermplasmDAO germplasmDAO; @Inject @Client("/${micronaut.bi.api.version}") private RxHttpClient client; - private String newExperimentWorkflowId; + @Inject + private BrAPITestUtils brAPITestUtils; + private final Gson gson = new GsonBuilder().registerTypeAdapter(OffsetDateTime.class, (JsonDeserializer) (json, type, context) -> OffsetDateTime.parse(json.getAsString())) .create(); @BeforeAll void setup() throws Exception { - FannyPack fp = FannyPack.fill("src/test/resources/sql/ImportControllerIntegrationTest.sql"); - ImportTestUtils importTestUtils = new ImportTestUtils(); - newExperimentWorkflowId = importTestUtils.getExperimentWorkflowId(client, 0); - FannyPack securityFp = FannyPack.fill("src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql"); - FannyPack brapiFp = FannyPack.fill("src/test/resources/sql/brapi/species.sql"); - - // Test User - User testUser = userDAO.getUserByOrcId(TestTokenValidator.TEST_USER_ORCID).orElseThrow(Exception::new); - dsl.execute(securityFp.get("InsertSystemRoleAdmin"), testUser.getId().toString()); - - // Species - super.getBrapiDsl().execute(brapiFp.get("InsertSpecies")); - SpeciesEntity validSpecies = speciesDAO.findAll().get(0); - SpeciesRequest speciesRequest = SpeciesRequest.builder() - .commonName(validSpecies.getCommonName()) - .id(validSpecies.getId()) - .build(); - - // Test Program - ProgramRequest programRequest = ProgramRequest.builder() - .name("Test Program") - .abbreviation("Test") - .documentationUrl("localhost:8080") - .objective("To test things") - .species(speciesRequest) - .key("TEST") - .build(); - program = TestUtils.insertAndFetchTestProgram(gson, client, programRequest); - - dsl.execute(securityFp.get("InsertProgramRolesBreeder"), testUser.getId().toString(), program.getId()); - - // Get experiment import map - Flowable> call = client.exchange( - GET("/import/mappings?importName=ExperimentsTemplateMap") - .contentType(MediaType.APPLICATION_JSON) - .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class - ); - HttpResponse response = call.blockingFirst(); - String mappingId = JsonParser.parseString(Objects.requireNonNull(response.body())).getAsJsonObject() - .getAsJsonObject("result") - .getAsJsonArray("data") - .get(0).getAsJsonObject().get("id").getAsString(); - - // Add traits to program - traits = createTraits(2); - AuthenticatedUser user = new AuthenticatedUser(testUser.getName(), new ArrayList<>(), testUser.getId(), new ArrayList<>()); - try { - ontologyService.createTraits(program.getId(), traits, user, false); - } catch (ValidatorException e) { - System.err.println(e.getErrors()); - throw e; - } - - // Add germplasm to program - List germplasm = createGermplasm(1); - BrAPIExternalReference newReference = new BrAPIExternalReference(); - newReference.setReferenceSource(String.format("%s/programs", BRAPI_REFERENCE_SOURCE)); - newReference.setReferenceID(program.getId().toString()); - - germplasm.forEach(germ -> germ.getExternalReferences().add(newReference)); - - germplasmDAO.createBrAPIGermplasm(germplasm, program.getId(), null); - - // Make test experiment import - Map row1 = makeExpImportRow("Env1"); - Map row2 = makeExpImportRow("Env2"); - - // Add test observation data - for (Trait trait : traits) { - Random random = new Random(); - - // TODO: test for sending obs data as double. - // A float is returned from the backend instead of double. there is a separate card to fix this. - // Double val1 = Math.random(); - - Float val1 = random.nextFloat(); - row1.put(trait.getObservationVariableName(), val1); - } - - rows.add(row1); - rows.add(row2); - - // Import test experiment, environments, and any observations - JsonObject importResult = importTestUtils.uploadAndFetchWorkflow( - writeDataToFile(rows, traits), - null, - true, - client, - program, - mappingId, - newExperimentWorkflowId); - experimentId = importResult - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(0).getAsJsonObject() - .get("trial").getAsJsonObject() - .get("id").getAsString(); - // Add environmentIds. - envIds.add(getEnvId(importResult, 0)); - envIds.add(getEnvId(importResult, 1)); - - // Experimental Collaborator user. - User collaborator = userDAO.getUserByOrcId(TestTokenValidator.OTHER_TEST_USER_ORCID).orElseThrow(Exception::new); - dsl.execute(securityFp.get("InsertProgramRolesExperimentalCollaborator"), collaborator.getId().toString(), program.getId().toString()); - // Add collaborator to experiment. - // Make another test experiment import. - Map row3 = makeExpImportRow("Env3", "xyz"); - Map row4 = makeExpImportRow("Env4", "xyz"); - - List> newrows = new ArrayList<>(); - - newrows.add(row3); - newrows.add(row4); - - // Import test experiment, environments, and any observations - importResult = importTestUtils.uploadAndFetchWorkflow( - writeDataToFile(newrows, new ArrayList<>()), - null, - true, - client, - program, - mappingId, - newExperimentWorkflowId); - String newExperimentId = importResult - .get("preview").getAsJsonObject() - .get("rows").getAsJsonArray() - .get(0).getAsJsonObject() - .get("trial").getAsJsonObject() - .get("id").getAsString(); - - // Refetch collaborator, since we updated program_user_roles. - collaborator = userDAO.getUserByOrcId(TestTokenValidator.OTHER_TEST_USER_ORCID).orElseThrow(Exception::new); - dsl.execute(securityFp.get("AddCollaborator"), newExperimentId, collaborator.getProgramRoles().get(0).getId()); + // Setup program, experiments, users, species, germplasm, traits. + Tuple2> resultTuple = brAPITestUtils.setupTestProgram(super.getBrapiDsl(), gson); + // Unpack tuple into the instance variables we need. + this.program = resultTuple.getV1(); + this.experimentId = resultTuple.getV2().get(0); } @Test @@ -261,7 +93,6 @@ public void testGetStudiesAsExperimentalCollaborator() { assertTrue(studyNames.contains("Env4")); } - @Test @SneakyThrows public void testPostGetStudiesNotFound() { @@ -367,108 +198,4 @@ public void testGetStudyById() { assertEquals(study.get("studyDbId").getAsString(), brAPIStudySingleResponse.get().getResult().getStudyDbId()); } - private File writeDataToFile(List> data, List traits) throws IOException { - File file = File.createTempFile("test", ".csv"); - - if(traits != null) { - traits.forEach(trait -> columns.add( - Column.builder() - .value(trait.getObservationVariableName()) - .dataType(Column.ColumnDataType.STRING) - .build()) - ); - } - ByteArrayOutputStream byteArrayOutputStream = CSVWriter.writeToCSV(columns, data); - FileOutputStream fos = new FileOutputStream(file); - fos.write(byteArrayOutputStream.toByteArray()); - - 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++) { - String gid = ""+(i+1); - BrAPIGermplasm testGermplasm = new BrAPIGermplasm(); - testGermplasm.setGermplasmName(String.format("Germplasm %s [TEST-%s]", gid, gid)); - testGermplasm.setSeedSource("Wild"); - testGermplasm.setAccessionNumber(gid); - testGermplasm.setDefaultDisplayName(String.format("Germplasm %s", gid)); - JsonObject additionalInfo = new JsonObject(); - additionalInfo.addProperty("importEntryNumber", gid); - additionalInfo.addProperty("breedingMethod", "Allopolyploid"); - testGermplasm.setAdditionalInfo(additionalInfo); - List externalRef = new ArrayList<>(); - BrAPIExternalReference testReference = new BrAPIExternalReference(); - testReference.setReferenceSource(BRAPI_REFERENCE_SOURCE); - testReference.setReferenceID(UUID.randomUUID().toString()); - externalRef.add(testReference); - testGermplasm.setExternalReferences(externalRef); - germplasm.add(testGermplasm); - } - - return germplasm; - } - - private List createTraits(int numToCreate) { - List traits = new ArrayList<>(); - for (int i = 0; i < numToCreate; i++) { - String varName = "tt_test_" + (i + 1); - traits.add(Trait.builder() - .observationVariableName(varName) - .fullName(varName) - .entity("Plant " + i) - .attribute("height " + i) - .traitDescription("test") - .programObservationLevel(ProgramObservationLevel.builder().name("Plot").build()) - .scale(Scale.builder() - .scaleName("test scale") - .units("test unit") - .dataType(DataType.NUMERICAL) - .validValueMin(0) - .validValueMax(100) - .build()) - .method(Method.builder() - .description("test method") - .methodClass("test method") - .build()) - .build()); - } - - return traits; - } - - private Map makeExpImportRow(String environment) { - return makeExpImportRow(environment, "Test Exp"); - } - - private Map makeExpImportRow(String environment, String expTitle) { - Map row = new HashMap<>(); - row.put(ExperimentObservation.Columns.GERMPLASM_GID, "1"); - row.put(ExperimentObservation.Columns.TEST_CHECK, "T"); - row.put(ExperimentObservation.Columns.EXP_TITLE, expTitle); - row.put(ExperimentObservation.Columns.EXP_UNIT, "Plot"); - row.put(ExperimentObservation.Columns.EXP_TYPE, "Phenotyping"); - row.put(ExperimentObservation.Columns.ENV, environment); - row.put(ExperimentObservation.Columns.ENV_LOCATION, "Location A"); - row.put(ExperimentObservation.Columns.ENV_YEAR, "2023"); - row.put(ExperimentObservation.Columns.EXP_UNIT_ID, "a-1"); - row.put(ExperimentObservation.Columns.REP_NUM, "1"); - row.put(ExperimentObservation.Columns.BLOCK_NUM, "1"); - row.put(ExperimentObservation.Columns.ROW, "1"); - row.put(ExperimentObservation.Columns.COLUMN, "1"); - return row; - } } diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java new file mode 100644 index 000000000..f59f5fab6 --- /dev/null +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java @@ -0,0 +1,306 @@ +package org.breedinginsight.brapi.v2; + +import com.google.gson.Gson; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; +import groovy.lang.Tuple2; +import io.kowalski.fannypack.FannyPack; +import io.micronaut.context.annotation.Property; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.MediaType; +import io.micronaut.http.client.RxHttpClient; +import io.micronaut.http.client.annotation.Client; +import io.micronaut.http.netty.cookies.NettyCookie; +import io.reactivex.Flowable; +import org.brapi.v2.model.BrAPIExternalReference; +import org.brapi.v2.model.germ.BrAPIGermplasm; +import org.breedinginsight.TestUtils; +import org.breedinginsight.api.auth.AuthenticatedUser; +import org.breedinginsight.api.model.v1.request.ProgramRequest; +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.brapps.importer.ImportTestUtils; +import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation; +import org.breedinginsight.dao.db.enums.DataType; +import org.breedinginsight.dao.db.tables.pojos.SpeciesEntity; +import org.breedinginsight.daos.SpeciesDAO; +import org.breedinginsight.daos.UserDAO; +import org.breedinginsight.model.*; +import org.breedinginsight.services.OntologyService; +import org.breedinginsight.services.exceptions.ValidatorException; +import org.breedinginsight.services.parsers.experiment.ExperimentFileColumns; +import org.breedinginsight.services.writers.CSVWriter; +import org.jooq.DSLContext; + +import javax.inject.Inject; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.util.*; + +import static io.micronaut.http.HttpRequest.GET; + +public class BrAPITestUtils { + + private static final List columns = ExperimentFileColumns.getOrderedColumns(); + + @Property(name = "brapi.server.reference-source") + private String BRAPI_REFERENCE_SOURCE; + @Inject + private DSLContext dsl; + @Inject + private UserDAO userDAO; + @Inject + private SpeciesDAO speciesDAO; + @Inject + private OntologyService ontologyService; + @Inject + private BrAPIGermplasmDAO germplasmDAO; + + @Inject + @Client("/${micronaut.bi.api.version}") + private RxHttpClient client; + + /** + * Create a test program and populate it with users, traits, germplasm, and experiment data. + * Note: Brittle, the exact state created by this method is depended on by multiple tests. + * @return a tuple containing the created program and the ids of the created experiments. + */ + public Tuple2> setupTestProgram(DSLContext brAPIDslContext, Gson gson) throws Exception { + + // Insert species in BrAPI server database (not bidb). + FannyPack brapiFp = FannyPack.fill("src/test/resources/sql/brapi/species.sql"); + brAPIDslContext.execute(brapiFp.get("InsertSpecies")); + + // Create program, species and test users. + FannyPack fp = FannyPack.fill("src/test/resources/sql/ImportControllerIntegrationTest.sql"); + FannyPack securityFp = FannyPack.fill("src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql"); + + // Test User + User testUser = userDAO.getUserByOrcId(TestTokenValidator.TEST_USER_ORCID).orElseThrow(Exception::new); + dsl.execute(securityFp.get("InsertSystemRoleAdmin"), testUser.getId().toString()); + + // Species + SpeciesEntity validSpecies = speciesDAO.findAll().get(0); + SpeciesRequest speciesRequest = SpeciesRequest.builder() + .commonName(validSpecies.getCommonName()) + .id(validSpecies.getId()) + .build(); + + // Test Program + ProgramRequest programRequest = ProgramRequest.builder() + .name("Test Program") + .abbreviation("Test") + .documentationUrl("localhost:8080") + .objective("To test things") + .species(speciesRequest) + .key("TEST") + .build(); + Program program = TestUtils.insertAndFetchTestProgram(gson, client, programRequest); + + // Add Program Administrator user. + dsl.execute(securityFp.get("InsertProgramRolesBreeder"), testUser.getId().toString(), program.getId()); + + // Add Experimental Collaborator user. + User collaborator = userDAO.getUserByOrcId(TestTokenValidator.OTHER_TEST_USER_ORCID).orElseThrow(Exception::new); + dsl.execute(securityFp.get("InsertProgramRolesExperimentalCollaborator"), collaborator.getId().toString(), program.getId().toString()); + + // Get experiment import map + Flowable> call = client.exchange( + GET("/import/mappings?importName=ExperimentsTemplateMap") + .contentType(MediaType.APPLICATION_JSON) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class + ); + HttpResponse response = call.blockingFirst(); + String mappingId = JsonParser.parseString(Objects.requireNonNull(response.body())).getAsJsonObject() + .getAsJsonObject("result") + .getAsJsonArray("data") + .get(0).getAsJsonObject().get("id").getAsString(); + + // Add traits to program + List traits = createTraits(2); + AuthenticatedUser user = new AuthenticatedUser(testUser.getName(), new ArrayList<>(), testUser.getId(), new ArrayList<>()); + try { + ontologyService.createTraits(program.getId(), traits, user, false); + } catch (ValidatorException e) { + System.err.println(e.getErrors()); + throw e; + } + + // Add germplasm to program + List germplasm = createGermplasm(1, BRAPI_REFERENCE_SOURCE); + BrAPIExternalReference newReference = new BrAPIExternalReference(); + newReference.setReferenceSource(String.format("%s/programs", BRAPI_REFERENCE_SOURCE)); + newReference.setReferenceId(program.getId().toString()); + + germplasm.forEach(germ -> germ.getExternalReferences().add(newReference)); + + germplasmDAO.createBrAPIGermplasm(germplasm, program.getId(), null); + + ImportTestUtils importTestUtils = new ImportTestUtils(); + String newExperimentWorkflowId = importTestUtils.getExperimentWorkflowId(client, 0); + + // Make test experiment import + Map row1 = makeExpImportRow("Env1"); + Map row2 = makeExpImportRow("Env2"); + + // Add test observation data + for (Trait trait : traits) { + Random random = new Random(); + + // TODO: test for sending obs data as double. + // A float is returned from the backend instead of double. there is a separate card to fix this. + // Double val1 = Math.random(); + + Float val1 = random.nextFloat(); + row1.put(trait.getObservationVariableName(), val1); + } + + // Import test experiment, environments, and any observations + JsonObject importResult = importTestUtils.uploadAndFetchWorkflow( + writeDataToFile(List.of(row1, row2), new ArrayList<>()), + null, + true, + client, + 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( + writeDataToFile(List.of(makeExpImportRow("Env3", "xyz"), makeExpImportRow("Env4", "xyz")), new ArrayList<>()), + null, + true, + client, + program, + mappingId, + newExperimentWorkflowId); + String experiment2Id = importResult + .get("preview").getAsJsonObject() + .get("rows").getAsJsonArray() + .get(0).getAsJsonObject() + .get("trial").getAsJsonObject() + .get("id").getAsString(); + + // Refetch collaborator, since we updated program_user_roles. + collaborator = userDAO.getUserByOrcId(TestTokenValidator.OTHER_TEST_USER_ORCID).orElseThrow(Exception::new); + // Add collaborator to experiment. + dsl.execute(securityFp.get("AddCollaborator"), experiment2Id, collaborator.getProgramRoles().get(0).getId()); + + // Return program and experimentId. + return new Tuple2<>(program, List.of(experiment1Id, experiment2Id)); + } + + public File writeDataToFile(List> data, List traits) throws IOException { + File file = File.createTempFile("test", ".csv"); + + if(traits != null) { + traits.forEach(trait -> columns.add( + Column.builder() + .value(trait.getObservationVariableName()) + .dataType(Column.ColumnDataType.STRING) + .build()) + ); + } + ByteArrayOutputStream byteArrayOutputStream = CSVWriter.writeToCSV(columns, data); + FileOutputStream fos = new FileOutputStream(file); + fos.write(byteArrayOutputStream.toByteArray()); + + return file; + } + + public 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(); + } + + public List createGermplasm(int numToCreate, String referenceSource) { + List germplasm = new ArrayList<>(); + for (int i = 0; i < numToCreate; i++) { + String gid = ""+(i+1); + BrAPIGermplasm testGermplasm = new BrAPIGermplasm(); + testGermplasm.setGermplasmName(String.format("Germplasm %s [TEST-%s]", gid, gid)); + testGermplasm.setSeedSource("Wild"); + testGermplasm.setAccessionNumber(gid); + testGermplasm.setDefaultDisplayName(String.format("Germplasm %s", gid)); + JsonObject additionalInfo = new JsonObject(); + additionalInfo.addProperty("importEntryNumber", gid); + additionalInfo.addProperty("breedingMethod", "Allopolyploid"); + testGermplasm.setAdditionalInfo(additionalInfo); + List externalRef = new ArrayList<>(); + BrAPIExternalReference testReference = new BrAPIExternalReference(); + testReference.setReferenceSource(referenceSource); + testReference.setReferenceID(UUID.randomUUID().toString()); + externalRef.add(testReference); + testGermplasm.setExternalReferences(externalRef); + germplasm.add(testGermplasm); + } + + return germplasm; + } + + public List createTraits(int numToCreate) { + List traits = new ArrayList<>(); + for (int i = 0; i < numToCreate; i++) { + String varName = "tt_test_" + (i + 1); + traits.add(Trait.builder() + .observationVariableName(varName) + .fullName(varName) + .entity("Plant " + i) + .attribute("height " + i) + .traitDescription("test") + .programObservationLevel(ProgramObservationLevel.builder().name("Plot").build()) + .scale(Scale.builder() + .scaleName("test scale") + .units("test unit") + .dataType(DataType.NUMERICAL) + .validValueMin(0) + .validValueMax(100) + .build()) + .method(Method.builder() + .description("test method") + .methodClass("test method") + .build()) + .build()); + } + + return traits; + } + + public Map makeExpImportRow(String environment) { + return makeExpImportRow(environment, "Test Exp"); + } + + public Map makeExpImportRow(String environment, String expTitle) { + Map row = new HashMap<>(); + row.put(ExperimentObservation.Columns.GERMPLASM_GID, "1"); + row.put(ExperimentObservation.Columns.TEST_CHECK, "T"); + row.put(ExperimentObservation.Columns.EXP_TITLE, expTitle); + row.put(ExperimentObservation.Columns.EXP_UNIT, "Plot"); + row.put(ExperimentObservation.Columns.EXP_TYPE, "Phenotyping"); + row.put(ExperimentObservation.Columns.ENV, environment); + row.put(ExperimentObservation.Columns.ENV_LOCATION, "Location A"); + row.put(ExperimentObservation.Columns.ENV_YEAR, "2023"); + row.put(ExperimentObservation.Columns.EXP_UNIT_ID, "a-1"); + row.put(ExperimentObservation.Columns.REP_NUM, "1"); + row.put(ExperimentObservation.Columns.BLOCK_NUM, "1"); + row.put(ExperimentObservation.Columns.ROW, "1"); + row.put(ExperimentObservation.Columns.COLUMN, "1"); + return row; + } +} diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java new file mode 100644 index 000000000..8d49a66d9 --- /dev/null +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java @@ -0,0 +1,135 @@ +/* + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.breedinginsight.brapi.v2; + +import com.google.gson.*; +import groovy.lang.Tuple2; +import io.micronaut.context.annotation.Property; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.client.RxHttpClient; +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.brapps.importer.services.ExternalReferenceSource; +import org.breedinginsight.model.*; +import org.breedinginsight.utilities.Utilities; +import org.junit.jupiter.api.*; + +import javax.inject.Inject; +import java.time.OffsetDateTime; +import java.util.*; + +import static io.micronaut.http.HttpRequest.GET; +import static org.junit.jupiter.api.Assertions.*; + + +@MicronautTest +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +public class BrAPITrialsControllerIntegrationTest extends BrAPITest { + private Program program; + private String experiment1Id; + private String experiment2Id; + + @Property(name = "brapi.server.reference-source") + private String BRAPI_REFERENCE_SOURCE; + + @Inject + @Client("/${micronaut.bi.api.version}") + private RxHttpClient client; + + @Inject + private BrAPITestUtils brAPITestUtils; + + private final Gson gson = new GsonBuilder().registerTypeAdapter(OffsetDateTime.class, (JsonDeserializer) + (json, type, context) -> OffsetDateTime.parse(json.getAsString())) + .create(); + + @BeforeAll + void setup() throws Exception { + // Setup program, experiments, users, species, germplasm, traits. + Tuple2> resultTuple = brAPITestUtils.setupTestProgram(super.getBrapiDsl(), gson); + // Unpack tuple into the instance variables we need. + this.program = resultTuple.getV1(); + this.experiment1Id = resultTuple.getV2().get(0); + this.experiment2Id = resultTuple.getV2().get(1); + } + + /** + * This test ensures that all trials in a program are returned for a system admin user. + */ + @Test + public void testGetTrials() { + Flowable> call = client.exchange( + GET(String.format("/programs/%s/brapi/v2/trials", program.getId())) + .bearerAuth("test-registered-user"), + String.class + ); + + HttpResponse response = call.blockingFirst(); + assertEquals(HttpStatus.OK, response.getStatus()); + + JsonObject responseObj = gson.fromJson(response.body(), JsonObject.class); + JsonArray trialsJson = responseObj.getAsJsonObject("result").getAsJsonArray("data"); + assertEquals(2, trialsJson.size()); + + List trials = new ArrayList<>(); + trials.add(gson.fromJson(trialsJson.get(0).getAsJsonObject(), BrAPITrial.class)); + trials.add(gson.fromJson(trialsJson.get(1).getAsJsonObject(), BrAPITrial.class)); + String source = Utilities.generateReferenceSource(BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS); + + // Check names and experimentIds (the BI-assigned UUID stored as an xref). + List expNames = new ArrayList<>(); + List expIds = new ArrayList<>(); + for (BrAPITrial trial : trials) { + String experimentId = Utilities.getExternalReference(trial.getExternalReferences(), source).get().getReferenceId(); + expIds.add(experimentId); + expNames.add(trial.getTrialName()); + } + assert(expNames.contains("xyz")); + assert(expNames.contains("Test Exp")); + assert(expIds.contains(experiment1Id)); + assert(expIds.contains(experiment2Id)); + } + + @Test + public void testGetTrialsAsExperimentalCollaborator() { + // This test ensures that trials are filtered for experimental collaborators. + Flowable> call = client.exchange( + GET(String.format("/programs/%s/brapi/v2/trials", program.getId())) + .bearerAuth("other-registered-user"), + String.class + ); + + HttpResponse response = call.blockingFirst(); + assertEquals(HttpStatus.OK, response.getStatus()); + + JsonObject responseObj = gson.fromJson(response.body(), JsonObject.class); + JsonArray trials = responseObj.getAsJsonObject("result").getAsJsonArray("data"); + 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); + } + +} From d9e9e95f40eeaa5226afc5028e8a4df1867269b9 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Mon, 19 Aug 2024 09:19:23 -0400 Subject: [PATCH 06/12] [BI-2258] - documented tests. --- .../brapi/v2/BrAPIStudiesControllerIntegrationTest.java | 4 +++- .../brapi/v2/BrAPITrialsControllerIntegrationTest.java | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java index d1e6797c4..fdd0df29b 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPIStudiesControllerIntegrationTest.java @@ -70,9 +70,11 @@ void setup() throws Exception { this.experimentId = resultTuple.getV2().get(0); } + /** + * Tests that studies are filtered for experimental collaborators. + */ @Test public void testGetStudiesAsExperimentalCollaborator() { - // This test ensures that studies are filtered for experimental collaborators. Flowable> call = client.exchange( GET(String.format("/programs/%s/brapi/v2/studies", program.getId())) .bearerAuth("other-registered-user"), diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java index 8d49a66d9..dafa9b7a9 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPITrialsControllerIntegrationTest.java @@ -74,7 +74,7 @@ void setup() throws Exception { } /** - * This test ensures that all trials in a program are returned for a system admin user. + * Tests that all trials in a program are returned for a system admin user. */ @Test public void testGetTrials() { @@ -110,9 +110,11 @@ public void testGetTrials() { assert(expIds.contains(experiment2Id)); } + /** + * Tests that trials are filtered for an experimental collaborator. + */ @Test public void testGetTrialsAsExperimentalCollaborator() { - // This test ensures that trials are filtered for experimental collaborators. Flowable> call = client.exchange( GET(String.format("/programs/%s/brapi/v2/trials", program.getId())) .bearerAuth("other-registered-user"), From b03360e06900a13d1141cfffcca1a5e95195435c Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Mon, 19 Aug 2024 09:25:03 -0400 Subject: [PATCH 07/12] [BI-2258] - added notice --- .../brapi/v2/BrAPITestUtils.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java b/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java index f59f5fab6..d0b440f79 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java +++ b/src/test/java/org/breedinginsight/brapi/v2/BrAPITestUtils.java @@ -1,3 +1,20 @@ +/* + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package org.breedinginsight.brapi.v2; import com.google.gson.Gson; From e2534b4d471bc98a24a660044e17974454d73fc8 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Mon, 19 Aug 2024 09:50:36 -0400 Subject: [PATCH 08/12] [BI-2258] - added method documentation --- .../daos/ExperimentalCollaboratorDAO.java | 12 ++++++++++++ .../services/ExperimentalCollaboratorService.java | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/main/java/org/breedinginsight/daos/ExperimentalCollaboratorDAO.java b/src/main/java/org/breedinginsight/daos/ExperimentalCollaboratorDAO.java index 431d2ae00..6f5ff9880 100644 --- a/src/main/java/org/breedinginsight/daos/ExperimentalCollaboratorDAO.java +++ b/src/main/java/org/breedinginsight/daos/ExperimentalCollaboratorDAO.java @@ -63,6 +63,12 @@ public ExperimentProgramUserRoleEntity create(UUID experimentId, UUID programUse .fetchOneInto(ExperimentProgramUserRoleEntity.class); } + /** + * Get the list (expected to have zero or one elements) of records authorizing a program user to access an experiment. + * @param programUserRoleId the primary key of a program_user_role record. + * @param experimentId the BI-assigned UUID of an experiment. + * @return a list of ExperimentProgramUserRoleEntity. + */ public List fetchByProgramUserIdAndExperimentId(UUID programUserRoleId, UUID experimentId) { // Only returns results for active program_user_role rows. return dsl.select(EXPERIMENT_PROGRAM_USER_ROLE.fields()) @@ -74,6 +80,12 @@ public List fetchByProgramUserIdAndExperimentId .fetchInto(ExperimentProgramUserRoleEntity.class); } + /** + * Get a list of BI-assigned experiment UUIDs of experiments which a program user is authorized to access. + * @param programUserRoleId the primary key of a program_user_role record. + * @param activeOnly if true, only return records for active program users. + * @return a list of BI-assigned experiment UUIDs. + */ public List getExperimentIds(UUID programUserRoleId, boolean activeOnly) { // If activeOnly, this will only return results if the program_user_role row is active. if (activeOnly) diff --git a/src/main/java/org/breedinginsight/services/ExperimentalCollaboratorService.java b/src/main/java/org/breedinginsight/services/ExperimentalCollaboratorService.java index 1f4a4a6d1..630cc4517 100644 --- a/src/main/java/org/breedinginsight/services/ExperimentalCollaboratorService.java +++ b/src/main/java/org/breedinginsight/services/ExperimentalCollaboratorService.java @@ -37,12 +37,23 @@ public ExperimentalCollaboratorService(ExperimentalCollaboratorDAO experimentalC this.experimentalCollaboratorDAO = experimentalCollaboratorDAO; } + /** + * Check if an Experimental Collaborator is authorized to access an experiment. + * @param programUserRoleId the primary key of a program_user_role record representing an experimental collaborator. + * @param experimentId the BI-assigned UUID of an experiment (stored in xref on the BrAPI trial). + * @return true if the specified program user is authorized to access the specified experiment. + */ public boolean isAuthorized(UUID programUserRoleId, UUID experimentId) { return !experimentalCollaboratorDAO .fetchByProgramUserIdAndExperimentId(programUserRoleId, experimentId) .isEmpty(); } + /** + * Get the BI-assigned UUIDs of all experiments for which an experimental collaborator has authorization. + * @param programUserRoleId the primary key of a program_user_role record representing an experimental collaborator. + * @return a list of BI-assigned experiment UUIDs. + */ public List getAuthorizedExperimentIds(UUID programUserRoleId) { // Get the list of experimentIds associated with the programUserRoleId, empty if the programUserRole is active. return experimentalCollaboratorDAO.getExperimentIds(programUserRoleId, true); From de49a177e781380248f98cf39f9b703e4d75a4dc Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Mon, 19 Aug 2024 10:01:08 -0400 Subject: [PATCH 09/12] [BI-2258] - added documentation --- .../org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java | 6 ++++++ .../brapi/v2/services/BrAPIStudyService.java | 6 ++++++ .../brapi/v2/services/BrAPITrialService.java | 6 ++++++ 3 files changed, 18 insertions(+) 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 e966ecd6a..fdd404ae3 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java @@ -161,6 +161,12 @@ public List getStudiesByEnvironmentIds(@NotNull Collection env .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. + * @param program the program. + * @return a list of BrAPIStudies. + */ public List getStudiesByExperimentIds(@NotNull Collection experimentIds, Program program) throws ApiException { BrAPIStudySearchRequest studySearch = new BrAPIStudySearchRequest(); studySearch.programDbIds(List.of(program.getBrapiProgram().getProgramDbId())); 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 846d9b4cc..104664c49 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIStudyService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIStudyService.java @@ -48,6 +48,12 @@ public Optional getStudyByEnvironmentId(Program program, UUID enviro return studyDAO.getStudyByEnvironmentId(environmentId, program); } + /** + * Get a list of studies by BI-assigned experiment UUIDs withing a program. + * @param program the program. + * @param experimentIds a list of BI-assigned experiment UUIDs. + * @return a list of BrAPIStudies. + */ public List getStudiesByExperimentIds(Program program, List experimentIds) throws ApiException { return studyDAO.getStudiesByExperimentIds(experimentIds, 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 916193ca8..7c37f8f55 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -102,6 +102,12 @@ public List getExperiments(UUID programId) throws ApiException, Does return trialDAO.getTrials(programId); } + /** + * Get a list of trials by BI-assigned experiment UUIDs withing a program (these UUIDs are xrefs on BrAPITrial objects). + * @param program the program. + * @param experimentIds a list of BI-assigned experiment UUIDs. + * @return a list of BrAPITrials. + */ public List getTrialsByExperimentIds(Program program, List experimentIds) throws ApiException, DoesNotExistException { return trialDAO.getTrialsByExperimentIds(experimentIds, program); } From 4b13e244ebbcc98bf80a87c4e7bc889ceb95b4c2 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Mon, 19 Aug 2024 16:26:14 -0400 Subject: [PATCH 10/12] [BI-2258] - reduced code duplication --- .../brapi/v2/BrAPIStudiesController.java | 17 ++++++++--------- .../brapi/v2/BrAPITrialsController.java | 14 ++++---------- .../services/ProgramUserService.java | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java index b51203099..ae9c9c4b8 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java @@ -44,6 +44,7 @@ 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; @@ -96,20 +97,15 @@ public HttpResponse>>> getStudies( log.debug("fetching studies for program: " + programId); List studies; - AuthenticatedUser user = securityService.getUser(); - Optional programUser = programUserService.getProgramUserbyId(programId, user.getId()); - if (programUser.isEmpty()) { - return HttpResponse.notFound(); - } - boolean isExperimentalCollaborator = programUser.get().getRoles().stream().anyMatch(x -> ProgramSecuredRole.getEnum(x.getDomain()).equals(ProgramSecuredRole.EXPERIMENTAL_COLLABORATOR)); - - if (isExperimentalCollaborator) { + Optional experimentalCollaborator = programUserService.getIfExperimentalCollaborator(programId, securityService.getUser().getId()); + // If the program user is an experimental collaborator, filter results. + if (experimentalCollaborator.isPresent()) { Optional program = programService.getById(programId); if (program.isEmpty()) { return HttpResponse.notFound(); } - List experimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(programUser.get().getId()); + List experimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(experimentalCollaborator.get().getId()); studies = studyService.getStudiesByExperimentIds(program.get(), experimentIds) .stream() .peek(this::setDbIds) @@ -131,6 +127,9 @@ public HttpResponse>>> getStudies( } catch (IllegalArgumentException e) { log.info(e.getMessage(), e); return HttpResponse.status(HttpStatus.UNPROCESSABLE_ENTITY, "Error parsing requested date format"); + } catch (DoesNotExistException e) { + log.info(e.getMessage(), e); + return HttpResponse.status(HttpStatus.NOT_FOUND, e.getMessage()); } } diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java index 9d60166c3..e20937573 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java @@ -78,20 +78,14 @@ public HttpResponse>>> getExperiments( List experiments = new ArrayList<>(); log.debug("fetching trials for program: " + programId); - AuthenticatedUser user = securityService.getUser(); - Optional programUser = programUserService.getProgramUserbyId(programId, user.getId()); - if (programUser.isEmpty()) { - return HttpResponse.notFound(); - } - boolean isExperimentalCollaborator = programUser.get().getRoles().stream().anyMatch(x -> ProgramSecuredRole.getEnum(x.getDomain()).equals(ProgramSecuredRole.EXPERIMENTAL_COLLABORATOR)); - - if (isExperimentalCollaborator) { + Optional experimentalCollaborator = programUserService.getIfExperimentalCollaborator(programId, securityService.getUser().getId()); + // If the program user is an experimental collaborator, filter results. + if (experimentalCollaborator.isPresent()) { Optional program = programService.getById(programId); if (program.isEmpty()) { return HttpResponse.notFound(); } - - List experimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(programUser.get().getId()); + List experimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(experimentalCollaborator.get().getId()); experiments = experimentService.getTrialsByExperimentIds(program.get(), experimentIds).stream().peek(this::setDbIds).collect(Collectors.toList()); } else { experiments = experimentService.getExperiments(programId).stream().peek(this::setDbIds).collect(Collectors.toList()); diff --git a/src/main/java/org/breedinginsight/services/ProgramUserService.java b/src/main/java/org/breedinginsight/services/ProgramUserService.java index 76afcfa1a..a07bd441c 100644 --- a/src/main/java/org/breedinginsight/services/ProgramUserService.java +++ b/src/main/java/org/breedinginsight/services/ProgramUserService.java @@ -19,6 +19,7 @@ import lombok.extern.slf4j.Slf4j; import org.breedinginsight.api.auth.AuthenticatedUser; +import org.breedinginsight.api.auth.ProgramSecuredRole; import org.breedinginsight.api.auth.SecurityService; import org.breedinginsight.api.model.v1.request.ProgramUserRequest; import org.breedinginsight.api.model.v1.request.UserRequest; @@ -274,6 +275,24 @@ public boolean existsAndActive(UUID programId, UUID userId) { return programUserDao.existsAndActive(programId, userId); } + /** + * Get a program user by programId and userId if it is in the Experimental Collaborator role. + * @param programId the program ID. + * @param userId the user ID. + * @return an Optional that unwraps to a program user if it is an Experimental Collaborator, Optional.empty() otherwise. + */ + public Optional getIfExperimentalCollaborator(UUID programId, UUID userId) throws DoesNotExistException { + Optional programUser = getProgramUserbyId(programId, userId); + if (programUser.isEmpty()) { + throw new DoesNotExistException("Program User does not exist"); + } + boolean isExperimentalCollaborator = programUser.get().getRoles().stream().anyMatch(x -> ProgramSecuredRole.getEnum(x.getDomain()).equals(ProgramSecuredRole.EXPERIMENTAL_COLLABORATOR)); + if (isExperimentalCollaborator) { + return programUser; + } + return Optional.empty(); + } + public List getProgramUsersByRole(UUID programId, UUID roleId) throws DoesNotExistException { if (!programService.exists(programId)) { throw new DoesNotExistException("Program id does not exist"); From d3d59296aae1ad0b15e236a549020f4bc14c5563 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Mon, 19 Aug 2024 16:28:30 -0400 Subject: [PATCH 11/12] [BI-2258] - added notice --- .../brapi/v2/BrAPITrialsController.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java index e20937573..f7028aefa 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java @@ -1,3 +1,20 @@ +/* + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package org.breedinginsight.brapi.v2; import io.micronaut.context.annotation.Property; From 4418f2114058abf7bd670852a96e12a3a4ad911c Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Thu, 29 Aug 2024 15:25:24 -0400 Subject: [PATCH 12/12] [BI-2258] - removed duplicate SQL in test file --- .../sql/ProgramSecuredAnnotationRuleIntegrationTest.sql | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql b/src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql index 3e1e8d9c2..2f1ca6f6b 100644 --- a/src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql +++ b/src/test/resources/sql/ProgramSecuredAnnotationRuleIntegrationTest.sql @@ -34,15 +34,6 @@ insert into program (species_id, name, abbreviation, documentation_url, objectiv select species.id, 'Program4', 'test', 'localhost:8080', 'To test things', bi_user.id, bi_user.id, 'PD' from species join bi_user on bi_user.name = 'Test User' limit 1; --- name: InsertProgramRolesExperimentalCollaborator - -insert into program_user_role (user_id, program_id, role_id, created_by, updated_by) -select -?::uuid, ?::uuid, role.id, bi_user.id, bi_user.id -from -bi_user -join role on role.domain = 'Experimental Collaborator'; - -- name: AddCollaborator insert into experiment_program_user_role (experiment_id, program_user_role_id, created_by, updated_by)