Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
import org.breedinginsight.services.ProgramUserService;
import org.breedinginsight.services.RoleService;
import org.breedinginsight.services.exceptions.DoesNotExistException;
import org.breedinginsight.services.exceptions.UnprocessableEntityException;
import org.breedinginsight.utilities.Utilities;
import org.breedinginsight.utilities.response.mappers.ExperimentQueryMapper;

import javax.inject.Inject;
Expand Down Expand Up @@ -149,7 +147,7 @@ public HttpResponse<Response<Dataset>> createSubEntityDataset(
* @param experimentId The UUID of the experiment.
* @return An HttpResponse with a Response object containing a list of DatasetMetadata.
* @throws DoesNotExistException if the program does not exist.
* @throws ApiException if an error occurs while retrieving the datasets
* @throws ApiException if an error occurs while retrieving the datasets.
*/
@Get("/${micronaut.bi.api.version}/programs/{programId}/experiments/{experimentId}/datasets")
@ExperimentCollaboratorSecured
Expand Down Expand Up @@ -196,7 +194,7 @@ public HttpResponse<Response<ExperimentProgramUserRoleEntity>> createExperimenta
if (programUserOptional.isEmpty()) {
return HttpResponse.status(HttpStatus.NOT_FOUND, "Program user does not exist");
}

//get active user creating the collaborator
AuthenticatedUser createdByUser = securityService.getUser();
UUID programUserId = programUserOptional.get().getId();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +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.ProgramSecuredRole;
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;
Expand All @@ -42,7 +40,11 @@
import org.breedinginsight.brapi.v2.services.BrAPIStudyService;
import org.breedinginsight.brapps.importer.services.ExternalReferenceSource;
import org.breedinginsight.model.Program;
import org.breedinginsight.model.ProgramUser;
import org.breedinginsight.services.ExperimentalCollaboratorService;
import org.breedinginsight.services.ProgramService;
import org.breedinginsight.services.ProgramUserService;
import org.breedinginsight.services.exceptions.DoesNotExistException;
import org.breedinginsight.utilities.Utilities;
import org.breedinginsight.utilities.response.ResponseUtils;
import org.breedinginsight.utilities.response.mappers.StudyQueryMapper;
Expand All @@ -63,14 +65,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*}")
Expand All @@ -82,11 +96,28 @@ public HttpResponse<Response<DataResponse<List<BrAPIStudy>>>> getStudies(
@QueryValue @QueryValid(using = StudyQueryMapper.class) @Valid StudyQuery queryParams) {
try {
log.debug("fetching studies for program: " + programId);
List<BrAPIStudy> studies;

Optional<ProgramUser> experimentalCollaborator = programUserService.getIfExperimentalCollaborator(programId, securityService.getUser().getId());
// If the program user is an experimental collaborator, filter results.
if (experimentalCollaborator.isPresent()) {
Optional<Program> program = programService.getById(programId);
if (program.isEmpty()) {
return HttpResponse.notFound();
}

List<UUID> experimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(experimentalCollaborator.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());
}
Comment on lines +101 to +119
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional methods could be used to refactor the fetching logic.

Suggested change
Optional<ProgramUser> experimentalCollaborator = programUserService.getIfExperimentalCollaborator(programId, securityService.getUser().getId());
// If the program user is an experimental collaborator, filter results.
if (experimentalCollaborator.isPresent()) {
Optional<Program> program = programService.getById(programId);
if (program.isEmpty()) {
return HttpResponse.notFound();
}
List<UUID> experimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(experimentalCollaborator.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());
}
Optional<HttpResponse> httpResponse = programUserService.getIfExperimentalCollaborator(programId, securityService.getUser().getId())
.flatMap(experimentalCollaborator -> { //If the program user is an experimental collaborator, filter results.
Optional<Program> program = programService.getById(programId);
return program.map(p -> {
List<UUID> experimentIds = experimentalCollaboratorService.getAuthorizedExperimentIds(experimentalCollaborator.getId());
List<Study> studies = studyService.getStudiesByExperimentIds(p, experimentIds)
.stream()
.peek(this::setDbIds)
.collect(Collectors.toList());
return HttpResponse.ok(studies);
});
})
.orElseGet(() -> { //Otherwise, fallback to fetching studies without considering collaborator status
List<Study> studies = studyService.getStudies(programId)
.stream()
.peek(this::setDbIds)
.collect(Collectors.toList());
return HttpResponse.ok(studies);
});
return httpResponse.orElse(HttpResponse.notFound());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to test this and the BrAPITrialsController suggestions locally, they both resulted in build errors. Are there problems in my code as written that you would like me to address?


List<BrAPIStudy> studies = studyService.getStudies(programId)
.stream()
.peek(this::setDbIds)
.collect(Collectors.toList());
queryParams.setSortField(studyQueryMapper.getDefaultSortField());
queryParams.setSortOrder(studyQueryMapper.getDefaultSortOrder());
SearchRequest searchRequest = queryParams.constructSearchRequest();
Expand All @@ -97,6 +128,9 @@ public HttpResponse<Response<DataResponse<List<BrAPIStudy>>>> 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());
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -20,6 +37,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.model.ProgramUser;
import org.breedinginsight.model.Role;
import org.breedinginsight.services.exceptions.DoesNotExistException;
Expand All @@ -29,7 +51,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;

Expand All @@ -41,12 +65,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*}")
Expand All @@ -57,9 +95,22 @@ public HttpResponse<Response<DataResponse<List<BrAPITrial>>>> getExperiments(
@PathVariable("programId") UUID programId,
@QueryValue @QueryValid(using = ExperimentQueryMapper.class) @Valid ExperimentQuery queryParams) {
try {
List<BrAPITrial> experiments = new ArrayList<>();
log.debug("fetching trials for program: " + programId);

List<BrAPITrial> experiments = experimentService.getExperiments(programId).stream().peek(this::setDbIds).collect(Collectors.toList());
Optional<ProgramUser> experimentalCollaborator = programUserService.getIfExperimentalCollaborator(programId, securityService.getUser().getId());
// If the program user is an experimental collaborator, filter results.
if (experimentalCollaborator.isPresent()) {
Optional<Program> program = programService.getById(programId);
if (program.isEmpty()) {
return HttpResponse.notFound();
}
List<UUID> 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());
}

Comment on lines +101 to +113
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Optional<ProgramUser> experimentalCollaborator = programUserService.getIfExperimentalCollaborator(programId, securityService.getUser().getId());
// If the program user is an experimental collaborator, filter results.
if (experimentalCollaborator.isPresent()) {
Optional<Program> program = programService.getById(programId);
if (program.isEmpty()) {
return HttpResponse.notFound();
}
List<UUID> 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());
}
Optional<HttpResponse> httpResponse = programUserService.getIfExperimentalCollaborator(programId, securityService.getUser().getId())
.flatMap(experimentalCollaborator ->
programService.getById(programId)
.map(program ->
experimentalCollaboratorService.getAuthorizedExperimentIds(experimentalCollaborator.getId())
.stream()
.flatMap(experimentId -> experimentService.getTrialsByExperimentIds(program, List.of(experimentId)).stream())
.peek(this::setDbIds)
.collect(Collectors.toList())))
.or(() ->
Optional.of(experimentService.getExperiments(programId)
.stream()
.peek(this::setDbIds)
.collect(Collectors.toList()))
.filter(experiments -> !experiments.isEmpty())
.map(HttpResponse::ok)
.orElse(HttpResponse.notFound());
return httpResponse.orElse(HttpResponse.notFound());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment on the BrAPIStudiesController.java suggestion.

SearchRequest searchRequest = queryParams.constructSearchRequest();
return ResponseUtils.getBrapiQueryResponse(experiments, experimentQueryMapper, queryParams, searchRequest);
} catch (ApiException e) {
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,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;
Expand Down Expand Up @@ -165,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<String> request, @PathVariable Optional<String> 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.PROGRAM_SCOPED_ROLES})
Expand Down
28 changes: 25 additions & 3 deletions src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,10 @@ public List<BrAPIStudy> getStudiesByName(List<String> studyNames, Program progra
);
}

public List<BrAPIStudy> getStudiesByExperimentID(@NotNull UUID experimentID, Program program ) throws ApiException {
public List<BrAPIStudy> 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(
Expand All @@ -152,7 +152,7 @@ public List<BrAPIStudy> getStudiesByExperimentID(@NotNull UUID experimentID, Pro
);
}

public List<BrAPIStudy> getStudiesByEnvironmentIds(@NotNull Collection<UUID> environmentIds, Program program ) throws ApiException {
public List<BrAPIStudy> getStudiesByEnvironmentIds(@NotNull Collection<UUID> environmentIds, Program program) throws ApiException {
return programStudyCache.get(program.getId())
.entrySet()
.stream()
Expand All @@ -161,6 +161,28 @@ public List<BrAPIStudy> getStudiesByEnvironmentIds(@NotNull Collection<UUID> 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<BrAPIStudy> getStudiesByExperimentIds(@NotNull Collection<UUID> 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 new ArrayList<>(processStudyForDisplay(brAPIDAOUtil.search(
api::searchStudiesPost,
api::searchStudiesSearchResultsDbIdGet,
studySearch
), program.getKey()).values());
}

public List<BrAPIStudy> createBrAPIStudies(List<BrAPIStudy> brAPIStudyList, UUID programId, ImportUpload upload) throws ApiException {
StudiesApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), StudiesApi.class);
List<BrAPIStudy> createdStudies = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,14 +269,12 @@ public List<BrAPITrial> getTrialsByExperimentIds(Collection<UUID> 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());
}
}


Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,14 @@ public Optional<BrAPIStudy> 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<BrAPIStudy> getStudiesByExperimentIds(Program program, List<UUID> experimentIds) throws ApiException {
return studyDAO.getStudiesByExperimentIds(experimentIds, program);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ public List<BrAPITrial> 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<BrAPITrial> getTrialsByExperimentIds(Program program, List<UUID> experimentIds) throws ApiException, DoesNotExistException {
return trialDAO.getTrialsByExperimentIds(experimentIds, program);
}

public BrAPITrial getTrialDataByUUID(UUID programId, UUID trialId, boolean stats) throws DoesNotExistException {
try {
Expand Down
Loading