From 004179ed244f74f4716bf2ede1757f76de6dc24b Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 18 Sep 2024 19:09:07 -0400 Subject: [PATCH 1/2] fix service method for fetching program user collaborators to allow for system admins --- .../brapi/v2/BrAPIStudiesController.java | 3 --- .../brapi/v2/BrAPITrialsController.java | 18 ++++++++---------- .../services/ProgramUserService.java | 17 +++++++---------- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java index a08e01764..e49e1560f 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java @@ -128,9 +128,6 @@ 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 f63515b77..1ad489d91 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPITrialsController.java @@ -95,23 +95,21 @@ public HttpResponse>>> getExperiments( @PathVariable("programId") UUID programId, @QueryValue @QueryValid(using = ExperimentQueryMapper.class) @Valid ExperimentQuery queryParams) { try { - List experiments = new ArrayList<>(); + Optional program = programService.getById(programId); + if (program.isEmpty()) { return HttpResponse.notFound(); } + + SearchRequest searchRequest = queryParams.constructSearchRequest(); log.debug("fetching trials for program: " + programId); + // If the program user is an experimental collaborator, filter results for only authorized experiments. 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(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()); + List authorizedExperiments = experimentService.getTrialsByExperimentIds(program.get(), experimentIds).stream().peek(this::setDbIds).collect(Collectors.toList()); + return ResponseUtils.getBrapiQueryResponse(authorizedExperiments, experimentQueryMapper, queryParams, searchRequest); } - SearchRequest searchRequest = queryParams.constructSearchRequest(); + List experiments = experimentService.getExperiments(programId).stream().peek(this::setDbIds).collect(Collectors.toList()); return ResponseUtils.getBrapiQueryResponse(experiments, experimentQueryMapper, queryParams, searchRequest); } catch (ApiException e) { log.info(e.getMessage(), e); diff --git a/src/main/java/org/breedinginsight/services/ProgramUserService.java b/src/main/java/org/breedinginsight/services/ProgramUserService.java index a07bd441c..2ecd751e7 100644 --- a/src/main/java/org/breedinginsight/services/ProgramUserService.java +++ b/src/main/java/org/breedinginsight/services/ProgramUserService.java @@ -281,16 +281,13 @@ public boolean existsAndActive(UUID programId, UUID userId) { * @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 Optional getIfExperimentalCollaborator(UUID programId, UUID userId) { + return getProgramUserbyId(programId, userId) + .flatMap(user -> user.getRoles().stream() + .anyMatch(role -> ProgramSecuredRole.getEnum(role.getDomain()).equals(ProgramSecuredRole.EXPERIMENTAL_COLLABORATOR)) + ? Optional.of(user) + : Optional.empty() + ); } public List getProgramUsersByRole(UUID programId, UUID roleId) throws DoesNotExistException { From 899576c55380758544ef126eab27d6c2dc24093a Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 19 Sep 2024 11:19:25 -0400 Subject: [PATCH 2/2] refactor for clarity --- .../brapi/v2/BrAPIStudiesController.java | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java index e49e1560f..daadea133 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIStudiesController.java @@ -95,32 +95,29 @@ public HttpResponse>>> getStudies( @PathVariable("programId") UUID programId, @QueryValue @QueryValid(using = StudyQueryMapper.class) @Valid StudyQuery queryParams) { try { + Optional program = programService.getById(programId); + if (program.isEmpty()) { return HttpResponse.notFound(); } + + queryParams.setSortField(studyQueryMapper.getDefaultSortField()); + queryParams.setSortOrder(studyQueryMapper.getDefaultSortOrder()); + SearchRequest searchRequest = queryParams.constructSearchRequest(); log.debug("fetching studies for program: " + programId); - List studies; + // If the program user is an experimental collaborator, filter results for only authorized studies. 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(experimentalCollaborator.get().getId()); - studies = studyService.getStudiesByExperimentIds(program.get(), experimentIds) + List authorizedExperimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(experimentalCollaborator.get().getId()); + List authorizedStudies = studyService.getStudiesByExperimentIds(program.get(), authorizedExperimentIds) .stream() .peek(this::setDbIds) .collect(Collectors.toList()); - } else { - studies = studyService.getStudies(programId) + return ResponseUtils.getBrapiQueryResponse(authorizedStudies, studyQueryMapper, queryParams, searchRequest); + } + + List studies = studyService.getStudies(programId) .stream() .peek(this::setDbIds) .collect(Collectors.toList()); - } - - queryParams.setSortField(studyQueryMapper.getDefaultSortField()); - queryParams.setSortOrder(studyQueryMapper.getDefaultSortOrder()); - SearchRequest searchRequest = queryParams.constructSearchRequest(); return ResponseUtils.getBrapiQueryResponse(studies, studyQueryMapper, queryParams, searchRequest); } catch (ApiException e) { log.info(e.getMessage(), e);