From 369f421f0e72181a2cf9b9640b997973517c3d90 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Mon, 18 Nov 2024 15:05:31 -0500 Subject: [PATCH 01/16] [BI-2376] - added method signatures --- .../v1/controller/ExperimentController.java | 33 +++++++++++++++++++ .../brapi/v2/services/BrAPITrialService.java | 6 ++++ 2 files changed, 39 insertions(+) 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 a6e172c94..de40bdf46 100644 --- a/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java +++ b/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java @@ -298,4 +298,37 @@ public HttpResponse deleteExperimentalCollaborator( } } + + /** + * Deletes an experiment. + * @param programId The UUID of the program + * @param experimentId The UUID of the experiment + * @param hard Specifies hard or soft delete + * @return A Http Response + */ + @Delete("/${micronaut.bi.api.version}/programs/{programId}/experiments/{experimentId}{?hard}") + @ProgramSecured(roles = {ProgramSecuredRole.PROGRAM_ADMIN, ProgramSecuredRole.SYSTEM_ADMIN}) + @Produces(MediaType.APPLICATION_JSON) + public HttpResponse deleteExperiment( + @PathVariable("programId") UUID programId, + @PathVariable("experimentId") UUID experimentId, + @QueryValue(defaultValue = "false") Boolean hard + ) throws ApiException { + try { + Optional program = programService.getById(programId); + if(program.isEmpty()) { + return HttpResponse.notFound(); + } + + experimentService.deleteExperiment(program.get(), experimentId, hard); + + return HttpResponse.ok(); + } catch (Exception e) { + log.error("Error deleting experiment.\n\tprogramId: " + programId + "\n\texperimentId: " + experimentId + "\n\thard: " + hard); + throw e; + } + + } + + } 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 7c37f8f55..c6b55fc76 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -667,6 +667,12 @@ public BrAPITrial getExperiment(Program program, UUID experimentId) throws ApiEx return experiments.get(0); } + public boolean deleteExperiment(Program program, UUID experimentId, boolean hard) throws ApiException { + // TODO: make BrAPI request to delete experiment. + // TODO: make BrAPI request to delete list for default observation dataset. + + } + private Map createExportRow( BrAPITrial experiment, Program program, From 87d0962dcbfa649f5b136753d01f8f2e6cfebec8 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Fri, 15 Nov 2024 12:33:02 -0500 Subject: [PATCH 02/16] add delete endpoint to list controller --- .../brapi/v2/BrAPIGermplasmController.java | 37 ---- .../brapi/v2/BrAPIListController.java | 183 +++++++++++++++--- .../org/breedinginsight/daos/ListDAO.java | 102 ++++++++++ .../model/delta/DeltaEntity.java | 3 +- .../model/delta/DeltaEntityFactory.java | 55 +++++- .../delta/DeltaGermplasmListDetails.java | 62 ++++++ .../delta/DeltaGermplasmListSummary.java | 16 ++ .../model/delta/DeltaListDetails.java | 78 ++++++++ .../model/delta/DeltaListSummary.java | 20 ++ .../breedinginsight/services/ListService.java | 33 ++++ .../breedinginsight/utilities/Utilities.java | 2 + 11 files changed, 521 insertions(+), 70 deletions(-) create mode 100644 src/main/java/org/breedinginsight/daos/ListDAO.java create mode 100644 src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListDetails.java create mode 100644 src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListSummary.java create mode 100644 src/main/java/org/breedinginsight/model/delta/DeltaListDetails.java create mode 100644 src/main/java/org/breedinginsight/model/delta/DeltaListSummary.java create mode 100644 src/main/java/org/breedinginsight/services/ListService.java diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java index 948e68c20..4d67e2a00 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java @@ -221,43 +221,6 @@ public HttpResponse>>> getGermplasm( } } - @Get("/programs/{programId}/germplasm/lists/{listDbId}/records{?queryParams*}") - @Produces(MediaType.APPLICATION_JSON) - @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) - public HttpResponse>>> getGermplasmListRecords( - @PathVariable("programId") UUID programId, - @PathVariable("listDbId") String listDbId, - @QueryValue @QueryValid(using = GermplasmQueryMapper.class) @Valid GermplasmQuery queryParams) { - try { - List germplasm = germplasmService.getGermplasmByList(programId, listDbId); - SearchRequest searchRequest = queryParams.constructSearchRequest(); - return ResponseUtils.getBrapiQueryResponse(germplasm, germplasmQueryMapper, queryParams, searchRequest); - } catch (Exception e) { - log.info(e.getMessage(), e); - return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error retrieving germplasm list records"); - } - } - - @Get("/programs/{programId}/germplasm/lists/{listDbId}/export{?fileExtension}") - @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") - @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) - public HttpResponse germplasmListExport( - @PathVariable("programId") UUID programId, @PathVariable("listDbId") String listDbId, @QueryValue(defaultValue = "XLSX") String fileExtension) { - String downloadErrorMessage = "An error occurred while generating the download file. Contact the development team at bidevteam@cornell.edu."; - try { - FileType extension = Enum.valueOf(FileType.class, fileExtension); - DownloadFile germplasmListFile = germplasmService.exportGermplasmList(programId, listDbId, extension); - HttpResponse germplasmListExport = HttpResponse.ok(germplasmListFile.getStreamedFile()).header(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename="+germplasmListFile.getFileName()+extension.getExtension()); - return germplasmListExport; - } - catch (Exception e) { - log.info(e.getMessage(), e); - e.printStackTrace(); - HttpResponse response = HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, downloadErrorMessage).contentType(MediaType.TEXT_PLAIN).body(downloadErrorMessage); - return response; - } - } - @Get("/programs/{programId}/germplasm/export{?fileExtension}") @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java index 3dd6fd1f4..0eebc602d 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java @@ -1,26 +1,10 @@ -/* - * 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.http.HttpResponse; -import io.micronaut.http.HttpStatus; -import io.micronaut.http.MediaType; +import io.micronaut.core.beans.BeanIntrospection; +import io.micronaut.core.beans.BeanProperty; +import io.micronaut.http.*; import io.micronaut.http.annotation.*; +import io.micronaut.http.server.types.files.StreamedFile; import io.micronaut.security.annotation.Secured; import io.micronaut.security.rules.SecurityRule; import lombok.extern.slf4j.Slf4j; @@ -29,43 +13,55 @@ import org.brapi.v2.model.core.BrAPIListTypes; import org.breedinginsight.api.auth.ProgramSecured; import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; -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; import org.breedinginsight.api.model.v1.validators.QueryValid; import org.breedinginsight.brapi.v1.controller.BrapiVersion; +import org.breedinginsight.brapi.v1.model.request.query.BrapiQuery; import org.breedinginsight.brapi.v2.model.request.query.ListQuery; import org.breedinginsight.brapi.v2.services.BrAPIListService; +import org.breedinginsight.brapps.importer.model.exports.FileType; +import org.breedinginsight.model.DownloadFile; import org.breedinginsight.model.Program; +import org.breedinginsight.model.delta.DeltaListDetails; +import org.breedinginsight.services.ListService; import org.breedinginsight.services.ProgramService; import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.utilities.response.ResponseUtils; +import org.breedinginsight.utilities.response.mappers.AbstractQueryMapper; import org.breedinginsight.utilities.response.mappers.ListQueryMapper; +import org.breedinginsight.api.model.v1.request.query.SearchRequest; import javax.inject.Inject; +import javax.validation.ConstraintViolation; import javax.validation.Valid; +import javax.validation.Validator; import java.util.List; +import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; @Slf4j -@Controller +@Controller("/${micronaut.bi.api.version}/programs/{programId}" + BrapiVersion.BRAPI_V2) @Secured(SecurityRule.IS_AUTHENTICATED) public class BrAPIListController { - private final ProgramService programService; - private final BrAPIListService listService; + private final BrAPIListService brapiListService; + private final ListService listService; private final ListQueryMapper listQueryMapper; + private final Validator validator; @Inject - public BrAPIListController(ProgramService programService, BrAPIListService listService, - ListQueryMapper listQueryMapper) { + public BrAPIListController(ProgramService programService, BrAPIListService brapiListService, ListService listService, + ListQueryMapper listQueryMapper, Validator validator) { this.programService = programService; + this.brapiListService = brapiListService; this.listService = listService; this.listQueryMapper = listQueryMapper; + this.validator = validator; } - //@Get(BrapiVersion.BRAPI_V2 + "/lists") - @Get("/${micronaut.bi.api.version}/programs/{programId}" + BrapiVersion.BRAPI_V2 + "/lists{?queryParams*}") + @Get("/deltalists{?queryParams*}") @Produces(MediaType.APPLICATION_JSON) @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) public HttpResponse>> getLists( @@ -77,7 +73,6 @@ public HttpResponse>> getLists( .getById(programId) .orElseThrow(() -> new DoesNotExistException("Program does not exist")); - // get germplasm lists by default BrAPIListTypes type = BrAPIListTypes.fromValue(queryParams.getListType()); String source = null; String id = null; @@ -93,7 +88,7 @@ public HttpResponse>> getLists( if (dateFormatParam != null) { listQueryMapper.setDateDisplayFormat(dateFormatParam); } - List brapiLists = listService.getListSummariesByTypeAndXref(type, source, id, program); + List brapiLists = brapiListService.getListSummariesByTypeAndXref(type, source, id, program); SearchRequest searchRequest = queryParams.constructSearchRequest(); return ResponseUtils.getBrapiQueryResponse(brapiLists, listQueryMapper, queryParams, searchRequest); @@ -105,4 +100,132 @@ public HttpResponse>> getLists( throw new RuntimeException(e); } } + + @Delete("/deltalists/{listDbId}") + @Produces(MediaType.APPLICATION_JSON) + @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) + public HttpResponse>> deleteListById( + @PathVariable("programId") UUID programId, + @PathVariable("listDbId") String listDbId, + HttpRequest request + ) throws DoesNotExistException, ApiException { + boolean hardDelete = false; + if (request.getParameters().contains("hardDelete")) { + String paramValue = request.getParameters().get("hardDelete"); + hardDelete = "true".equals(paramValue); + } + try { + listService.deleteBrAPIList(listDbId, programId, hardDelete); + return HttpResponse.status(HttpStatus.NO_CONTENT); + } catch (Exception e) { + log.info(e.getMessage(), e); + return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error retrieving germplasm list records"); + } + } + + @Get("/deltalists/{listDbId}") + @Produces(MediaType.APPLICATION_JSON) + @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) + @SuppressWarnings("unchecked") + public HttpResponse>>> getListById( + @PathVariable("programId") UUID programId, + @PathVariable("listDbId") String listDbId, + HttpRequest request) { + try { + // Get the list from the BrAPI service + DeltaListDetails details = listService.getDeltaListDetails(listDbId, programId); + + // Get a new instance of BrAPI query matching the type of list contents + T queryParams = (T) details.getQuery(); + + // Bind query parameters to the object + bindQueryParams(queryParams, request); + + // Perform standard bean validation + Set> violations = validator.validate(queryParams); + if (!violations.isEmpty()) { + List errorMessages = violations.stream() + .map(ConstraintViolation::getMessage) + .collect(Collectors.toList()); + log.info(String.join(", ", errorMessages)); + return HttpResponse.status(HttpStatus.BAD_REQUEST, "Error with list contents search parameters"); + } + + // Fetch the list contents from the BrAPI service + List listContentsBrAPIObjects = (List) details.getDataObjects(); + + // Construct a search request for sorting the list contents + SearchRequest searchRequest = details.constructSearchRequest(queryParams); + + // Get the map used to connect query sorting keys to contents object values + AbstractQueryMapper contentsQueryMapper = details.getQueryMapper(); + + return ResponseUtils.getBrapiQueryResponse(listContentsBrAPIObjects, contentsQueryMapper, queryParams, searchRequest); + } catch (Exception e) { + log.info(e.getMessage(), e); + return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error retrieving list records"); + } + } + + @Get("/deltalists/{listDbId}/export{?fileExtension}") + @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") + @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) + public HttpResponse germplasmListExport( + @PathVariable("programId") UUID programId, @PathVariable("listDbId") String listDbId, @QueryValue(defaultValue = "XLSX") String fileExtension) { + String downloadErrorMessage = "An error occurred while generating the download file. Contact the development team at bidevteam@cornell.edu."; + try { + // Get the list from the BrAPI service + DeltaListDetails details = listService.getDeltaListDetails(listDbId, programId); + + FileType extension = Enum.valueOf(FileType.class, fileExtension); + DownloadFile listContentsFile = details.exportListObjects(extension); + return HttpResponse.ok(listContentsFile.getStreamedFile()).header(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename="+listContentsFile.getFileName()+extension.getExtension()); + } + catch (Exception e) { + log.info(e.getMessage(), e); + e.printStackTrace(); + HttpResponse response = HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, downloadErrorMessage).contentType(MediaType.TEXT_PLAIN).body(downloadErrorMessage); + return response; + } + } + + private void bindQueryParams(BrapiQuery queryParams, HttpRequest request) { + BeanIntrospection introspection = BeanIntrospection.getIntrospection(BrapiQuery.class); + for (BeanProperty property : introspection.getBeanProperties()) { + String paramName = property.getName(); + if (request.getParameters().contains(paramName)) { + String paramValue = request.getParameters().get(paramName); + Object convertedValue; + Class propertyType = property.getType(); + + if (propertyType.isEnum()) { + convertedValue = convertToEnum(paramValue, (Class>) propertyType); + } else { + convertedValue = convertValue(paramValue, propertyType); + } + + property.set(queryParams, convertedValue); + } + } + } + + private > T convertToEnum(String value, Class> enumClass) { + if (value == null) { + return null; + } + return Enum.valueOf((Class) enumClass, value.toUpperCase()); + } + + + // Convert, if necessary, the values of query parameters to match the type defined for the fields in the BrapiQuery class + private Object convertValue(String value, Class targetType) { + // Implement type conversion logic here + // Other list content types might need more complex logic + if (targetType == String.class) return value; + if (targetType == Integer.class) return Integer.parseInt(value); + if (targetType == Long.class) return Long.parseLong(value); + if (targetType == Boolean.class) return Boolean.parseBoolean(value); + // Add more type conversions as needed + return value; + } } diff --git a/src/main/java/org/breedinginsight/daos/ListDAO.java b/src/main/java/org/breedinginsight/daos/ListDAO.java new file mode 100644 index 000000000..f44a8757f --- /dev/null +++ b/src/main/java/org/breedinginsight/daos/ListDAO.java @@ -0,0 +1,102 @@ +package org.breedinginsight.daos; + +import io.micronaut.http.HttpStatus; +import io.micronaut.http.exceptions.HttpStatusException; +import lombok.extern.slf4j.Slf4j; +import okhttp3.*; +import org.brapi.client.v2.ApiResponse; +import org.brapi.client.v2.model.exceptions.ApiException; +import org.brapi.client.v2.modules.core.ListsApi; +import org.brapi.v2.model.core.response.BrAPIListDetails; +import org.brapi.v2.model.core.response.BrAPIListsSingleResponse; +import org.breedinginsight.brapi.v1.controller.BrapiVersion; +import org.breedinginsight.model.ProgramBrAPIEndpoints; +import org.breedinginsight.model.delta.DeltaEntityFactory; +import org.breedinginsight.model.delta.DeltaListDetails; +import org.breedinginsight.services.ProgramService; +import org.breedinginsight.services.brapi.BrAPIEndpointProvider; +import org.breedinginsight.services.exceptions.DoesNotExistException; +import org.breedinginsight.utilities.BrAPIDAOUtil; + +import javax.inject.Inject; +import javax.inject.Singleton; +import java.io.IOException; +import java.util.Objects; +import java.util.UUID; +import java.util.concurrent.TimeUnit; + +@Slf4j +@Singleton +public class ListDAO { + private final ProgramDAO programDAO; + private final BrAPIDAOUtil brAPIDAOUtil; + private final BrAPIEndpointProvider brAPIEndpointProvider; + private final DeltaEntityFactory deltaEntityFactory; + private final ProgramService programService; + + @Inject + public ListDAO(ProgramDAO programDAO, + BrAPIDAOUtil brAPIDAOUtil, + BrAPIEndpointProvider brAPIEndpointProvider, + DeltaEntityFactory deltaEntityFactory, ProgramService programService) { + this.programDAO = programDAO; + this.brAPIDAOUtil = brAPIDAOUtil; + this.brAPIEndpointProvider = brAPIEndpointProvider; + this.deltaEntityFactory = deltaEntityFactory; + this.programService = programService; + } + + public DeltaListDetails getDeltaListDetailsByDbId(String listDbId, UUID programId) throws ApiException { + ListsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ListsApi.class); + ApiResponse response = api.listsListDbIdGet(listDbId); + if (Objects.isNull(response.getBody()) || Objects.isNull(response.getBody().getResult())) + { + throw new ApiException(); + } + + BrAPIListDetails details = response.getBody().getResult(); + return deltaEntityFactory.makeDeltaListDetailsBean(details); + } + + public void deleteBrAPIList(String listDbId, UUID programId, boolean hardDelete) throws ApiException { + var programBrAPIBaseUrl = getProgramBrAPIBaseUrl(programId); + var requestUrl = HttpUrl.parse(programBrAPIBaseUrl + "/lists/" + listDbId).newBuilder(); + requestUrl.addQueryParameter("hardDelete", Boolean.toString(hardDelete)); + HttpUrl url = requestUrl.build(); + var brapiRequest = new Request.Builder().url(url) + .method("DELETE", null) + .addHeader("Content-Type", "application/json") + .build(); + + makeCall(brapiRequest); + } + + private void makeCall(Request brapiRequest) throws ApiException { + OkHttpClient client = new OkHttpClient.Builder() + .readTimeout(5, TimeUnit.MINUTES) + .build(); + try { + client.newCall(brapiRequest).execute(); + } catch (IOException e) { + log.error("Error calling BrAPI Service", e); + throw new ApiException("Error calling BrAPI Service"); + } + } + + private String getProgramBrAPIBaseUrl(UUID programId) { + ProgramBrAPIEndpoints programBrAPIEndpoints; + try { + programBrAPIEndpoints = programService.getBrapiEndpoints(programId); + } catch (DoesNotExistException e) { + throw new HttpStatusException(HttpStatus.NOT_FOUND, "Program does not exist"); + } + + if(programBrAPIEndpoints.getCoreUrl().isEmpty()) { + log.error("Program: " + programId + " is missing BrAPI URL config"); + throw new HttpStatusException(HttpStatus.INTERNAL_SERVER_ERROR, ""); + } + var programBrAPIBaseUrl = programBrAPIEndpoints.getCoreUrl().get(); + programBrAPIBaseUrl = programBrAPIBaseUrl.endsWith("/") ? programBrAPIBaseUrl.substring(0, programBrAPIBaseUrl.length() - 1) : programBrAPIBaseUrl; + return programBrAPIBaseUrl.endsWith(BrapiVersion.BRAPI_V2) ? programBrAPIBaseUrl : programBrAPIBaseUrl + BrapiVersion.BRAPI_V2; + } +} diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaEntity.java b/src/main/java/org/breedinginsight/model/delta/DeltaEntity.java index 11646f9f8..4f5b8ead9 100644 --- a/src/main/java/org/breedinginsight/model/delta/DeltaEntity.java +++ b/src/main/java/org/breedinginsight/model/delta/DeltaEntity.java @@ -7,7 +7,7 @@ public abstract class DeltaEntity { - protected final Gson gson; + protected static final Gson gson = new GsonBuilder().registerTypeAdapterFactory(new GeometryAdapterFactory()).create(); @NonNull protected final T entity; @@ -15,7 +15,6 @@ public abstract class DeltaEntity { // Note: do not use @Inject, DeltaEntity are always constructed by DeltaEntityFactory. protected DeltaEntity(@NonNull T entity) { this.entity = entity; - this.gson = new GsonBuilder().registerTypeAdapterFactory(new GeometryAdapterFactory()).create(); } protected T getEntity() { diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java b/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java index 3787c5d71..8d473ce5a 100644 --- a/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java +++ b/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java @@ -2,8 +2,14 @@ import io.micronaut.context.annotation.Bean; import io.micronaut.context.annotation.Factory; +import io.micronaut.context.annotation.Property; import io.micronaut.context.annotation.Prototype; import lombok.NonNull; +import org.brapi.v2.model.core.BrAPIListSummary; +import org.brapi.v2.model.core.BrAPIListTypes; +import org.brapi.v2.model.core.response.BrAPIListDetails; +import org.breedinginsight.brapi.v2.dao.BrAPIGermplasmDAO; +import org.breedinginsight.brapi.v2.services.BrAPIGermplasmService; import org.breedinginsight.brapps.importer.services.processors.experiment.service.ObservationUnitService; import org.brapi.v2.model.core.BrAPIStudy; @@ -13,6 +19,7 @@ import org.brapi.v2.model.pheno.BrAPIObservationUnit; import org.brapi.v2.model.pheno.BrAPIObservationVariable; import org.breedinginsight.model.ProgramLocation; +import org.breedinginsight.utilities.Utilities; import javax.inject.Inject; @@ -20,10 +27,16 @@ public class DeltaEntityFactory { private final ObservationUnitService observationUnitService; + private final BrAPIGermplasmService germplasmService; + private final String applicationReferenceSourceBase; @Inject - public DeltaEntityFactory(ObservationUnitService observationUnitService) { + public DeltaEntityFactory(ObservationUnitService observationUnitService, + BrAPIGermplasmService germplasmService, + @Property(name = "brapi.server.reference-source") String applicationReferenceSourceBase) { this.observationUnitService = observationUnitService; + this.germplasmService = germplasmService; + this.applicationReferenceSourceBase = applicationReferenceSourceBase; } private Experiment makeExperiment(BrAPITrial brAPIObject) { @@ -54,6 +67,46 @@ private DeltaObservationVariable makeDeltaObservationVariable(BrAPIObservationVa return new DeltaObservationVariable(brAPIObject); } + private DeltaGermplasmListSummary makeDeltaGermplasmListSummary(BrAPIListSummary brAPIObject) { + return new DeltaGermplasmListSummary(brAPIObject); + } + + private DeltaGermplasmListDetails makeDeltaGermplasmListDetails(BrAPIListDetails brAPIObject, String referenceSourceBase, BrAPIGermplasmService germplasmService) { + return new DeltaGermplasmListDetails(brAPIObject, referenceSourceBase, germplasmService); + } + + @Bean + @Prototype + public DeltaListSummary makeDeltaListSummaryBean(@NonNull BrAPIListSummary brAPIObject) { + BrAPIListTypes listType = brAPIObject.getListType(); + switch (listType) { + case GERMPLASM: return makeDeltaGermplasmListSummary(brAPIObject); + default: return null; + } + } + + @Bean + @Prototype + public DeltaListDetails makeDeltaListDetailsBean(@NonNull BrAPIListDetails brAPIObject) { + BrAPIListTypes listType = brAPIObject.getListType(); + switch (listType) { + case GERMPLASM: return makeDeltaGermplasmListDetailsBean(brAPIObject); + default: return null; + } + } + + @Bean + @Prototype + public DeltaGermplasmListSummary makeDeltaGermplasmListSummaryBean(@NonNull BrAPIListSummary brAPIObject) { + return makeDeltaGermplasmListSummary(brAPIObject); + } + + @Bean + @Prototype + public DeltaGermplasmListDetails makeDeltaGermplasmListDetailsBean(@NonNull BrAPIListDetails brAPIObject) { + return makeDeltaGermplasmListDetails(brAPIObject, applicationReferenceSourceBase, germplasmService); + } + @Bean @Prototype public Experiment makeExperimentBean(@NonNull BrAPITrial brAPIObject) { diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListDetails.java b/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListDetails.java new file mode 100644 index 000000000..1f8ea3d9c --- /dev/null +++ b/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListDetails.java @@ -0,0 +1,62 @@ +package org.breedinginsight.model.delta; + +import io.micronaut.context.annotation.Prototype; +import org.brapi.client.v2.model.exceptions.ApiException; +import org.brapi.v2.model.core.response.BrAPIListDetails; +import org.brapi.v2.model.germ.BrAPIGermplasm; +import org.breedinginsight.api.model.v1.request.query.SearchRequest; +import org.breedinginsight.brapi.v1.model.request.query.BrapiQuery; +import org.breedinginsight.brapi.v2.model.request.query.GermplasmQuery; +import org.breedinginsight.brapi.v2.services.BrAPIGermplasmService; +import org.breedinginsight.brapps.importer.model.exports.FileType; +import org.breedinginsight.model.DownloadFile; +import org.breedinginsight.services.exceptions.DoesNotExistException; +import org.breedinginsight.services.exceptions.UnprocessableEntityException; +import org.breedinginsight.utilities.response.mappers.AbstractQueryMapper; +import org.breedinginsight.utilities.response.mappers.GermplasmQueryMapper; + +import java.io.IOException; +import java.util.List; +import java.util.UUID; + + +@Prototype +public class DeltaGermplasmListDetails extends DeltaListDetails { + + private final BrAPIGermplasmService germplasmService; + + // Note: do not use @Inject, DeltaEntity are always constructed by DeltaEntityFactory. + DeltaGermplasmListDetails(BrAPIListDetails brAPIListDetails, + String referenceSourceBase, + BrAPIGermplasmService germplasmService) { + super(brAPIListDetails, referenceSourceBase); + this.germplasmService = germplasmService; + } + + @Override + public List getDataObjects() throws ApiException, UnprocessableEntityException { + UUID programId = getProgramId().orElseThrow(() -> new UnprocessableEntityException("Program Id not found for list " + getListName())); + return germplasmService.getGermplasmByList(programId, getListDbId()); + } + + @Override + public DownloadFile exportListObjects(FileType extension) throws IllegalArgumentException, ApiException, IOException, DoesNotExistException, UnprocessableEntityException { + UUID programId = getProgramId().orElseThrow(() -> new UnprocessableEntityException("Program Id not found for list " + getListName())); + return germplasmService.exportGermplasmList(programId, getListDbId(), extension); + } + + @Override + public AbstractQueryMapper getQueryMapper() { + return new GermplasmQueryMapper(); + } + + @Override + public BrapiQuery getQuery() { + return new GermplasmQuery(); + } + + @Override + public SearchRequest constructSearchRequest(BrapiQuery queryParams) { + return ((GermplasmQuery) queryParams).constructSearchRequest(); + } +} diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListSummary.java b/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListSummary.java new file mode 100644 index 000000000..6ddd2518b --- /dev/null +++ b/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListSummary.java @@ -0,0 +1,16 @@ +package org.breedinginsight.model.delta; + +import io.micronaut.context.annotation.Prototype; +import lombok.Getter; +import lombok.Setter; +import org.brapi.v2.model.core.BrAPIListSummary; +import org.breedinginsight.brapps.importer.model.response.ImportObjectState; + + +@Prototype +public class DeltaGermplasmListSummary extends DeltaListSummary { + + // Note: do not use @Inject, DeltaEntity are always constructed by DeltaEntityFactory. + DeltaGermplasmListSummary(BrAPIListSummary brAPIListSummary) { super(brAPIListSummary); } + +} diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaListDetails.java b/src/main/java/org/breedinginsight/model/delta/DeltaListDetails.java new file mode 100644 index 000000000..9f2c745bf --- /dev/null +++ b/src/main/java/org/breedinginsight/model/delta/DeltaListDetails.java @@ -0,0 +1,78 @@ +package org.breedinginsight.model.delta; + +import io.micronaut.context.annotation.Prototype; +import lombok.Getter; +import lombok.Setter; +import lombok.Value; +import org.brapi.client.v2.model.exceptions.ApiException; +import org.brapi.v2.model.BrAPIExternalReference; +import org.brapi.v2.model.core.BrAPIListTypes; +import org.brapi.v2.model.core.response.BrAPIListDetails; +import org.breedinginsight.api.model.v1.request.query.SearchRequest; +import org.breedinginsight.brapi.v1.model.request.query.BrapiQuery; +import org.breedinginsight.brapps.importer.model.exports.FileType; +import org.breedinginsight.brapps.importer.model.response.ImportObjectState; +import io.micronaut.context.annotation.Property; +import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; +import org.breedinginsight.model.DownloadFile; +import org.breedinginsight.services.exceptions.DoesNotExistException; +import org.breedinginsight.services.exceptions.UnprocessableEntityException; +import org.breedinginsight.utilities.Utilities; +import org.breedinginsight.utilities.response.mappers.AbstractQueryMapper; + +import java.io.IOException; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; + + +@Prototype +public abstract class DeltaListDetails extends DeltaEntity { + private final String referenceSourceBase; + @Getter + @Setter + private ImportObjectState state; + + // Note: do not use @Inject, DeltaEntity are always constructed by DeltaEntityFactory. + DeltaListDetails(BrAPIListDetails brAPIListDetails, String referenceSourceBase) { + super(brAPIListDetails); + this.referenceSourceBase = referenceSourceBase; + } + + public abstract List getDataObjects() throws ApiException, UnprocessableEntityException; + public abstract DownloadFile exportListObjects(FileType extension) throws IllegalArgumentException, ApiException, IOException, DoesNotExistException, UnprocessableEntityException; + public abstract AbstractQueryMapper getQueryMapper(); + + public abstract BrapiQuery getQuery(); + + public abstract SearchRequest constructSearchRequest(BrapiQuery queryParams); + + public BrAPIListTypes getListType() { + return entity.getListType(); + } + + public Optional getListId() { + return getXrefId(ExternalReferenceSource.LISTS); + } + + public Optional getProgramId() { + return getXrefId(ExternalReferenceSource.PROGRAMS); + } + + public String getListDbId() { + return entity.getListDbId(); + } + + public String getListName() { + return entity.getListName(); + } + + private Optional getXrefId(ExternalReferenceSource source) { + // Get the external reference if it exists + Optional xrefOptional = Utilities.getExternalReference(entity.getExternalReferences(), referenceSourceBase, source); + + // Parse the Deltabreed ID from the xref + return xrefOptional.map(BrAPIExternalReference::getReferenceId).map(UUID::fromString); + } +} diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaListSummary.java b/src/main/java/org/breedinginsight/model/delta/DeltaListSummary.java new file mode 100644 index 000000000..80569d077 --- /dev/null +++ b/src/main/java/org/breedinginsight/model/delta/DeltaListSummary.java @@ -0,0 +1,20 @@ +package org.breedinginsight.model.delta; + +import io.micronaut.context.annotation.Prototype; +import lombok.Getter; +import lombok.Setter; +import org.brapi.v2.model.core.BrAPIListSummary; +import org.breedinginsight.brapps.importer.model.response.ImportObjectState; + + +@Prototype +public class DeltaListSummary extends DeltaEntity { + + @Getter + @Setter + private ImportObjectState state; + + // Note: do not use @Inject, DeltaEntity are always constructed by DeltaEntityFactory. + DeltaListSummary(BrAPIListSummary brAPIListSummary) { super(brAPIListSummary); } + +} diff --git a/src/main/java/org/breedinginsight/services/ListService.java b/src/main/java/org/breedinginsight/services/ListService.java new file mode 100644 index 000000000..c0c18f734 --- /dev/null +++ b/src/main/java/org/breedinginsight/services/ListService.java @@ -0,0 +1,33 @@ +package org.breedinginsight.services; + +import lombok.extern.slf4j.Slf4j; +import org.brapi.client.v2.model.exceptions.ApiException; +import org.breedinginsight.brapi.v2.dao.BrAPIListDAO; +import org.breedinginsight.daos.ListDAO; +import org.breedinginsight.model.delta.DeltaListDetails; + +import javax.inject.Inject; +import javax.inject.Singleton; +import java.util.UUID; + +@Slf4j +@Singleton +public class ListService { + private final BrAPIListDAO brAPIListDAO; + private final ListDAO listDAO; + + @Inject + public ListService(BrAPIListDAO brAPIListDAO, ListDAO listDAO) { + this.brAPIListDAO = brAPIListDAO; + this.listDAO = listDAO; + } + + public DeltaListDetails getDeltaListDetails(String listDbId, UUID programId) throws ApiException { + return listDAO.getDeltaListDetailsByDbId(listDbId, programId); + } + + public void deleteBrAPIList(String listDbId, UUID programId, boolean hardDelete) throws ApiException { + listDAO.deleteBrAPIList(listDbId, programId, hardDelete); + } + +} diff --git a/src/main/java/org/breedinginsight/utilities/Utilities.java b/src/main/java/org/breedinginsight/utilities/Utilities.java index 20f3254d6..7ed72cfef 100644 --- a/src/main/java/org/breedinginsight/utilities/Utilities.java +++ b/src/main/java/org/breedinginsight/utilities/Utilities.java @@ -17,6 +17,7 @@ package org.breedinginsight.utilities; +import io.micronaut.context.annotation.Property; import org.apache.commons.lang3.StringUtils; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.v2.model.BrAPIExternalReference; @@ -24,6 +25,7 @@ import org.breedinginsight.model.Program; import org.flywaydb.core.api.migration.Context; +import javax.inject.Inject; import java.lang.reflect.Field; import java.sql.ResultSet; import java.sql.Statement; From e29d0fe873650ecbe7ffc3f6f6f3c4f22e6e71c8 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 18 Nov 2024 12:24:31 -0500 Subject: [PATCH 03/16] get germplasm list contents via germplasm controller --- .../brapi/v2/BrAPIGermplasmController.java | 3 +- .../brapi/v2/BrAPIListController.java | 21 ++-- .../brapi/v2/dao/BrAPIListDAO.java | 78 +++++++++++++- .../model/request/query/GermplasmQuery.java | 3 + .../brapi/v2/services/BrAPIListService.java | 10 ++ .../org/breedinginsight/daos/ListDAO.java | 102 ------------------ .../breedinginsight/services/ListService.java | 33 ------ 7 files changed, 97 insertions(+), 153 deletions(-) delete mode 100644 src/main/java/org/breedinginsight/daos/ListDAO.java delete mode 100644 src/main/java/org/breedinginsight/services/ListService.java diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java index 4d67e2a00..57ead5bd3 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java @@ -209,7 +209,8 @@ public HttpResponse>>> getGermplasm( germplasmQueryMapper.setDateDisplayFormat(dateFormatParam); } - List germplasm = germplasmService.getGermplasm(programId); + // Fetch all germplasm in the program unless a list id is supplied to return only germplasm in that collection + List germplasm = queryParams.getList() == null ? germplasmService.getGermplasm(programId) : germplasmService.getGermplasmByList(programId, queryParams.getList());; SearchRequest searchRequest = queryParams.constructSearchRequest(); return ResponseUtils.getBrapiQueryResponse(germplasm, germplasmQueryMapper, queryParams, searchRequest); } catch (ApiException e) { diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java index 0eebc602d..2d3324a65 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java @@ -13,6 +13,7 @@ import org.brapi.v2.model.core.BrAPIListTypes; import org.breedinginsight.api.auth.ProgramSecured; import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; +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; import org.breedinginsight.api.model.v1.validators.QueryValid; @@ -24,13 +25,11 @@ import org.breedinginsight.model.DownloadFile; import org.breedinginsight.model.Program; import org.breedinginsight.model.delta.DeltaListDetails; -import org.breedinginsight.services.ListService; import org.breedinginsight.services.ProgramService; import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.utilities.response.ResponseUtils; import org.breedinginsight.utilities.response.mappers.AbstractQueryMapper; import org.breedinginsight.utilities.response.mappers.ListQueryMapper; -import org.breedinginsight.api.model.v1.request.query.SearchRequest; import javax.inject.Inject; import javax.validation.ConstraintViolation; @@ -47,21 +46,19 @@ public class BrAPIListController { private final ProgramService programService; private final BrAPIListService brapiListService; - private final ListService listService; private final ListQueryMapper listQueryMapper; private final Validator validator; @Inject - public BrAPIListController(ProgramService programService, BrAPIListService brapiListService, ListService listService, + public BrAPIListController(ProgramService programService, BrAPIListService brapiListService, ListQueryMapper listQueryMapper, Validator validator) { this.programService = programService; this.brapiListService = brapiListService; - this.listService = listService; this.listQueryMapper = listQueryMapper; this.validator = validator; } - @Get("/deltalists{?queryParams*}") + @Get("/lists{?queryParams*}") @Produces(MediaType.APPLICATION_JSON) @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) public HttpResponse>> getLists( @@ -101,7 +98,7 @@ public HttpResponse>> getLists( } } - @Delete("/deltalists/{listDbId}") + @Delete("/lists/{listDbId}") @Produces(MediaType.APPLICATION_JSON) @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) public HttpResponse>> deleteListById( @@ -115,7 +112,7 @@ public HttpResponse>> deleteListById( hardDelete = "true".equals(paramValue); } try { - listService.deleteBrAPIList(listDbId, programId, hardDelete); + brapiListService.deleteBrAPIList(listDbId, programId, hardDelete); return HttpResponse.status(HttpStatus.NO_CONTENT); } catch (Exception e) { log.info(e.getMessage(), e); @@ -123,7 +120,7 @@ public HttpResponse>> deleteListById( } } - @Get("/deltalists/{listDbId}") + @Get("/lists/{listDbId}") @Produces(MediaType.APPLICATION_JSON) @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) @SuppressWarnings("unchecked") @@ -133,7 +130,7 @@ public HttpResponse>>> g HttpRequest request) { try { // Get the list from the BrAPI service - DeltaListDetails details = listService.getDeltaListDetails(listDbId, programId); + DeltaListDetails details = brapiListService.getDeltaListDetails(listDbId, programId); // Get a new instance of BrAPI query matching the type of list contents T queryParams = (T) details.getQuery(); @@ -167,7 +164,7 @@ public HttpResponse>>> g } } - @Get("/deltalists/{listDbId}/export{?fileExtension}") + @Get("/lists/{listDbId}/export{?fileExtension}") @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) public HttpResponse germplasmListExport( @@ -175,7 +172,7 @@ public HttpResponse germplasmListExport( String downloadErrorMessage = "An error occurred while generating the download file. Contact the development team at bidevteam@cornell.edu."; try { // Get the list from the BrAPI service - DeltaListDetails details = listService.getDeltaListDetails(listDbId, programId); + DeltaListDetails details = brapiListService.getDeltaListDetails(listDbId, programId); FileType extension = Enum.valueOf(FileType.class, fileExtension); DownloadFile listContentsFile = details.exportListObjects(extension); diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java index 801b9775c..ed7db131c 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java @@ -17,7 +17,12 @@ package org.breedinginsight.brapi.v2.dao; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.exceptions.HttpStatusException; import lombok.extern.slf4j.Slf4j; +import okhttp3.HttpUrl; +import okhttp3.OkHttpClient; +import okhttp3.Request; import org.brapi.client.v2.ApiResponse; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.client.v2.model.queryParams.core.ListQueryParams; @@ -31,19 +36,24 @@ import org.brapi.v2.model.core.request.BrAPIListSearchRequest; import org.brapi.v2.model.core.response.*; import org.brapi.v2.model.pheno.BrAPIObservation; +import org.breedinginsight.brapi.v1.controller.BrapiVersion; import org.breedinginsight.brapps.importer.daos.ImportDAO; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.daos.ProgramDAO; +import org.breedinginsight.model.ProgramBrAPIEndpoints; +import org.breedinginsight.model.delta.DeltaEntityFactory; +import org.breedinginsight.model.delta.DeltaListDetails; +import org.breedinginsight.services.ProgramService; import org.breedinginsight.services.brapi.BrAPIEndpointProvider; +import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.utilities.BrAPIDAOUtil; import org.breedinginsight.utilities.Utilities; import javax.inject.Inject; import javax.validation.constraints.NotNull; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.UUID; +import java.io.IOException; +import java.util.*; +import java.util.concurrent.TimeUnit; @Slf4j public class BrAPIListDAO { @@ -52,13 +62,17 @@ public class BrAPIListDAO { private ImportDAO importDAO; private final BrAPIDAOUtil brAPIDAOUtil; private final BrAPIEndpointProvider brAPIEndpointProvider; + private final DeltaEntityFactory deltaEntityFactory; + private final ProgramService programService; @Inject - public BrAPIListDAO(ProgramDAO programDAO, ImportDAO importDAO, BrAPIDAOUtil brAPIDAOUtil, BrAPIEndpointProvider brAPIEndpointProvider) { + public BrAPIListDAO(ProgramDAO programDAO, ImportDAO importDAO, BrAPIDAOUtil brAPIDAOUtil, BrAPIEndpointProvider brAPIEndpointProvider, DeltaEntityFactory deltaEntityFactory, ProgramService programService) { this.programDAO = programDAO; this.importDAO = importDAO; this.brAPIDAOUtil = brAPIDAOUtil; this.brAPIEndpointProvider = brAPIEndpointProvider; + this.deltaEntityFactory = deltaEntityFactory; + this.programService = programService; } public List getListByName(List listNames, UUID programId) throws ApiException { @@ -196,4 +210,58 @@ public List createBrAPILists(List brapiLi throw new ApiException("No response after creating list"); } + + public DeltaListDetails getDeltaListDetailsByDbId(String listDbId, UUID programId) throws ApiException { + ListsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ListsApi.class); + ApiResponse response = api.listsListDbIdGet(listDbId); + if (Objects.isNull(response.getBody()) || Objects.isNull(response.getBody().getResult())) + { + throw new ApiException(); + } + + BrAPIListDetails details = response.getBody().getResult(); + return deltaEntityFactory.makeDeltaListDetailsBean(details); + } + + public void deleteBrAPIList(String listDbId, UUID programId, boolean hardDelete) throws ApiException { + var programBrAPIBaseUrl = getProgramBrAPIBaseUrl(programId); + var requestUrl = HttpUrl.parse(programBrAPIBaseUrl + "/lists/" + listDbId).newBuilder(); + requestUrl.addQueryParameter("hardDelete", Boolean.toString(hardDelete)); + HttpUrl url = requestUrl.build(); + var brapiRequest = new Request.Builder().url(url) + .method("DELETE", null) + .addHeader("Content-Type", "application/json") + .build(); + + makeCall(brapiRequest); + } + + private void makeCall(Request brapiRequest) throws ApiException { + OkHttpClient client = new OkHttpClient.Builder() + .readTimeout(5, TimeUnit.MINUTES) + .build(); + try { + client.newCall(brapiRequest).execute(); + } catch (IOException e) { + log.error("Error calling BrAPI Service", e); + throw new ApiException("Error calling BrAPI Service"); + } + } + + private String getProgramBrAPIBaseUrl(UUID programId) { + ProgramBrAPIEndpoints programBrAPIEndpoints; + try { + programBrAPIEndpoints = programService.getBrapiEndpoints(programId); + } catch (DoesNotExistException e) { + throw new HttpStatusException(HttpStatus.NOT_FOUND, "Program does not exist"); + } + + if(programBrAPIEndpoints.getCoreUrl().isEmpty()) { + log.error("Program: " + programId + " is missing BrAPI URL config"); + throw new HttpStatusException(HttpStatus.INTERNAL_SERVER_ERROR, ""); + } + var programBrAPIBaseUrl = programBrAPIEndpoints.getCoreUrl().get(); + programBrAPIBaseUrl = programBrAPIBaseUrl.endsWith("/") ? programBrAPIBaseUrl.substring(0, programBrAPIBaseUrl.length() - 1) : programBrAPIBaseUrl; + return programBrAPIBaseUrl.endsWith(BrapiVersion.BRAPI_V2) ? programBrAPIBaseUrl : programBrAPIBaseUrl + BrapiVersion.BRAPI_V2; + } } diff --git a/src/main/java/org/breedinginsight/brapi/v2/model/request/query/GermplasmQuery.java b/src/main/java/org/breedinginsight/brapi/v2/model/request/query/GermplasmQuery.java index 88f6d142c..92752fad6 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/model/request/query/GermplasmQuery.java +++ b/src/main/java/org/breedinginsight/brapi/v2/model/request/query/GermplasmQuery.java @@ -27,6 +27,9 @@ public class GermplasmQuery extends BrapiQuery { // This is a meta-parameter, it describes the display format of any date fields. private String dateDisplayFormat; + // The list id used to get a collection of germplasm + private String list; + public SearchRequest constructSearchRequest() { List filters = new ArrayList<>(); if (!StringUtils.isBlank(getImportEntryNumber())) { diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java index 3353ac17c..7dd7c81c6 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java @@ -12,6 +12,7 @@ import org.breedinginsight.brapi.v2.dao.BrAPIListDAO; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.model.Program; +import org.breedinginsight.model.delta.DeltaListDetails; import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.utilities.Utilities; @@ -19,6 +20,7 @@ import javax.inject.Singleton; import java.util.List; import java.util.Optional; +import java.util.UUID; import java.util.stream.Collectors; @Slf4j @@ -88,4 +90,12 @@ public List getListSummariesByTypeAndXref( return programLists; } + + public DeltaListDetails getDeltaListDetails(String listDbId, UUID programId) throws ApiException { + return listDAO.getDeltaListDetailsByDbId(listDbId, programId); + } + + public void deleteBrAPIList(String listDbId, UUID programId, boolean hardDelete) throws ApiException { + listDAO.deleteBrAPIList(listDbId, programId, hardDelete); + } } diff --git a/src/main/java/org/breedinginsight/daos/ListDAO.java b/src/main/java/org/breedinginsight/daos/ListDAO.java deleted file mode 100644 index f44a8757f..000000000 --- a/src/main/java/org/breedinginsight/daos/ListDAO.java +++ /dev/null @@ -1,102 +0,0 @@ -package org.breedinginsight.daos; - -import io.micronaut.http.HttpStatus; -import io.micronaut.http.exceptions.HttpStatusException; -import lombok.extern.slf4j.Slf4j; -import okhttp3.*; -import org.brapi.client.v2.ApiResponse; -import org.brapi.client.v2.model.exceptions.ApiException; -import org.brapi.client.v2.modules.core.ListsApi; -import org.brapi.v2.model.core.response.BrAPIListDetails; -import org.brapi.v2.model.core.response.BrAPIListsSingleResponse; -import org.breedinginsight.brapi.v1.controller.BrapiVersion; -import org.breedinginsight.model.ProgramBrAPIEndpoints; -import org.breedinginsight.model.delta.DeltaEntityFactory; -import org.breedinginsight.model.delta.DeltaListDetails; -import org.breedinginsight.services.ProgramService; -import org.breedinginsight.services.brapi.BrAPIEndpointProvider; -import org.breedinginsight.services.exceptions.DoesNotExistException; -import org.breedinginsight.utilities.BrAPIDAOUtil; - -import javax.inject.Inject; -import javax.inject.Singleton; -import java.io.IOException; -import java.util.Objects; -import java.util.UUID; -import java.util.concurrent.TimeUnit; - -@Slf4j -@Singleton -public class ListDAO { - private final ProgramDAO programDAO; - private final BrAPIDAOUtil brAPIDAOUtil; - private final BrAPIEndpointProvider brAPIEndpointProvider; - private final DeltaEntityFactory deltaEntityFactory; - private final ProgramService programService; - - @Inject - public ListDAO(ProgramDAO programDAO, - BrAPIDAOUtil brAPIDAOUtil, - BrAPIEndpointProvider brAPIEndpointProvider, - DeltaEntityFactory deltaEntityFactory, ProgramService programService) { - this.programDAO = programDAO; - this.brAPIDAOUtil = brAPIDAOUtil; - this.brAPIEndpointProvider = brAPIEndpointProvider; - this.deltaEntityFactory = deltaEntityFactory; - this.programService = programService; - } - - public DeltaListDetails getDeltaListDetailsByDbId(String listDbId, UUID programId) throws ApiException { - ListsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ListsApi.class); - ApiResponse response = api.listsListDbIdGet(listDbId); - if (Objects.isNull(response.getBody()) || Objects.isNull(response.getBody().getResult())) - { - throw new ApiException(); - } - - BrAPIListDetails details = response.getBody().getResult(); - return deltaEntityFactory.makeDeltaListDetailsBean(details); - } - - public void deleteBrAPIList(String listDbId, UUID programId, boolean hardDelete) throws ApiException { - var programBrAPIBaseUrl = getProgramBrAPIBaseUrl(programId); - var requestUrl = HttpUrl.parse(programBrAPIBaseUrl + "/lists/" + listDbId).newBuilder(); - requestUrl.addQueryParameter("hardDelete", Boolean.toString(hardDelete)); - HttpUrl url = requestUrl.build(); - var brapiRequest = new Request.Builder().url(url) - .method("DELETE", null) - .addHeader("Content-Type", "application/json") - .build(); - - makeCall(brapiRequest); - } - - private void makeCall(Request brapiRequest) throws ApiException { - OkHttpClient client = new OkHttpClient.Builder() - .readTimeout(5, TimeUnit.MINUTES) - .build(); - try { - client.newCall(brapiRequest).execute(); - } catch (IOException e) { - log.error("Error calling BrAPI Service", e); - throw new ApiException("Error calling BrAPI Service"); - } - } - - private String getProgramBrAPIBaseUrl(UUID programId) { - ProgramBrAPIEndpoints programBrAPIEndpoints; - try { - programBrAPIEndpoints = programService.getBrapiEndpoints(programId); - } catch (DoesNotExistException e) { - throw new HttpStatusException(HttpStatus.NOT_FOUND, "Program does not exist"); - } - - if(programBrAPIEndpoints.getCoreUrl().isEmpty()) { - log.error("Program: " + programId + " is missing BrAPI URL config"); - throw new HttpStatusException(HttpStatus.INTERNAL_SERVER_ERROR, ""); - } - var programBrAPIBaseUrl = programBrAPIEndpoints.getCoreUrl().get(); - programBrAPIBaseUrl = programBrAPIBaseUrl.endsWith("/") ? programBrAPIBaseUrl.substring(0, programBrAPIBaseUrl.length() - 1) : programBrAPIBaseUrl; - return programBrAPIBaseUrl.endsWith(BrapiVersion.BRAPI_V2) ? programBrAPIBaseUrl : programBrAPIBaseUrl + BrapiVersion.BRAPI_V2; - } -} diff --git a/src/main/java/org/breedinginsight/services/ListService.java b/src/main/java/org/breedinginsight/services/ListService.java deleted file mode 100644 index c0c18f734..000000000 --- a/src/main/java/org/breedinginsight/services/ListService.java +++ /dev/null @@ -1,33 +0,0 @@ -package org.breedinginsight.services; - -import lombok.extern.slf4j.Slf4j; -import org.brapi.client.v2.model.exceptions.ApiException; -import org.breedinginsight.brapi.v2.dao.BrAPIListDAO; -import org.breedinginsight.daos.ListDAO; -import org.breedinginsight.model.delta.DeltaListDetails; - -import javax.inject.Inject; -import javax.inject.Singleton; -import java.util.UUID; - -@Slf4j -@Singleton -public class ListService { - private final BrAPIListDAO brAPIListDAO; - private final ListDAO listDAO; - - @Inject - public ListService(BrAPIListDAO brAPIListDAO, ListDAO listDAO) { - this.brAPIListDAO = brAPIListDAO; - this.listDAO = listDAO; - } - - public DeltaListDetails getDeltaListDetails(String listDbId, UUID programId) throws ApiException { - return listDAO.getDeltaListDetailsByDbId(listDbId, programId); - } - - public void deleteBrAPIList(String listDbId, UUID programId, boolean hardDelete) throws ApiException { - listDAO.deleteBrAPIList(listDbId, programId, hardDelete); - } - -} From 095342ffefcaa651bc4cae86b557eba0bbaf527c Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 20 Nov 2024 09:51:38 -0500 Subject: [PATCH 04/16] revert GET list data objects from ListController --- .../brapi/v2/BrAPIGermplasmController.java | 24 ++-- .../brapi/v2/BrAPIListController.java | 124 +----------------- .../brapi/v2/dao/BrAPIListDAO.java | 31 ++--- .../v2/services/BrAPIGermplasmService.java | 14 +- .../brapi/v2/services/BrAPIListService.java | 5 - .../model/delta/DeltaEntityFactory.java | 57 +------- .../delta/DeltaGermplasmListDetails.java | 62 --------- .../delta/DeltaGermplasmListSummary.java | 16 --- .../model/delta/DeltaListDetails.java | 78 ----------- .../model/delta/DeltaListSummary.java | 20 --- .../breedinginsight/utilities/Utilities.java | 2 - 11 files changed, 40 insertions(+), 393 deletions(-) delete mode 100644 src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListDetails.java delete mode 100644 src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListSummary.java delete mode 100644 src/main/java/org/breedinginsight/model/delta/DeltaListDetails.java delete mode 100644 src/main/java/org/breedinginsight/model/delta/DeltaListSummary.java diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java index 57ead5bd3..94cf9c9fc 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java @@ -1,5 +1,6 @@ package org.breedinginsight.brapi.v2; +import com.drew.lang.annotations.Nullable; import io.micronaut.context.annotation.Property; import io.micronaut.http.HttpHeaders; import io.micronaut.http.HttpResponse; @@ -19,10 +20,11 @@ import org.brapi.v2.model.BrAPIIndexPagination; import org.brapi.v2.model.BrAPIMetadata; import org.brapi.v2.model.BrAPIStatus; -import org.brapi.v2.model.core.BrAPITrial; import org.brapi.v2.model.germ.*; import org.brapi.v2.model.germ.request.BrAPIGermplasmSearchRequest; -import org.brapi.v2.model.germ.response.*; +import org.brapi.v2.model.germ.response.BrAPIGermplasmListResponse; +import org.brapi.v2.model.germ.response.BrAPIGermplasmPedigreeResponse; +import org.brapi.v2.model.germ.response.BrAPIGermplasmProgenyResponse; import org.breedinginsight.api.auth.ProgramSecured; import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; import org.breedinginsight.api.model.v1.request.query.SearchRequest; @@ -33,27 +35,25 @@ import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; import org.breedinginsight.brapi.v2.dao.BrAPIGermplasmDAO; import org.breedinginsight.brapi.v2.model.request.query.GermplasmQuery; -import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; -import org.breedinginsight.model.Program; -import org.breedinginsight.services.ProgramService; -import org.breedinginsight.utilities.Utilities; -import org.breedinginsight.utilities.response.mappers.GermplasmQueryMapper; import org.breedinginsight.brapi.v2.services.BrAPIGermplasmService; import org.breedinginsight.brapps.importer.model.exports.FileType; import org.breedinginsight.daos.ProgramDAO; import org.breedinginsight.model.DownloadFile; import org.breedinginsight.model.GermplasmGenotype; +import org.breedinginsight.model.Program; +import org.breedinginsight.services.ProgramService; import org.breedinginsight.services.brapi.BrAPIEndpointProvider; import org.breedinginsight.services.exceptions.AuthorizationException; import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.services.geno.GenotypeService; +import org.breedinginsight.utilities.Utilities; import org.breedinginsight.utilities.response.ResponseUtils; +import org.breedinginsight.utilities.response.mappers.GermplasmQueryMapper; import javax.inject.Inject; import javax.validation.Valid; import java.util.*; import java.util.regex.Pattern; -import java.util.stream.Collectors; @Slf4j @Controller("/${micronaut.bi.api.version}") @@ -222,15 +222,17 @@ public HttpResponse>>> getGermplasm( } } - @Get("/programs/{programId}/germplasm/export{?fileExtension}") + @Get("/programs/{programId}/germplasm/export{?fileExtension,list}") @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) public HttpResponse germplasmExport( - @PathVariable("programId") UUID programId, @QueryValue(defaultValue = "XLSX") String fileExtension) { + @PathVariable("programId") UUID programId, + @QueryValue(defaultValue = "XLSX") String fileExtension, + @QueryValue Optional list) { String downloadErrorMessage = "An error occurred while generating the download file. Contact the development team at bidevteam@cornell.edu."; try { FileType extension = Enum.valueOf(FileType.class, fileExtension); - DownloadFile germplasmListFile = germplasmService.exportGermplasm(programId, extension); + DownloadFile germplasmListFile = list.isEmpty() ? germplasmService.exportGermplasm(programId, extension) : germplasmService.exportGermplasmList(programId, list.get(), extension); HttpResponse germplasmExport = HttpResponse.ok(germplasmListFile.getStreamedFile()).header(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename="+germplasmListFile.getFileName()+extension.getExtension()); return germplasmExport; } diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java index 2d3324a65..3cb73dbae 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIListController.java @@ -1,10 +1,10 @@ package org.breedinginsight.brapi.v2; -import io.micronaut.core.beans.BeanIntrospection; -import io.micronaut.core.beans.BeanProperty; -import io.micronaut.http.*; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.MediaType; import io.micronaut.http.annotation.*; -import io.micronaut.http.server.types.files.StreamedFile; import io.micronaut.security.annotation.Secured; import io.micronaut.security.rules.SecurityRule; import lombok.extern.slf4j.Slf4j; @@ -18,27 +18,19 @@ import org.breedinginsight.api.model.v1.response.Response; import org.breedinginsight.api.model.v1.validators.QueryValid; import org.breedinginsight.brapi.v1.controller.BrapiVersion; -import org.breedinginsight.brapi.v1.model.request.query.BrapiQuery; import org.breedinginsight.brapi.v2.model.request.query.ListQuery; import org.breedinginsight.brapi.v2.services.BrAPIListService; -import org.breedinginsight.brapps.importer.model.exports.FileType; -import org.breedinginsight.model.DownloadFile; import org.breedinginsight.model.Program; -import org.breedinginsight.model.delta.DeltaListDetails; import org.breedinginsight.services.ProgramService; import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.utilities.response.ResponseUtils; -import org.breedinginsight.utilities.response.mappers.AbstractQueryMapper; import org.breedinginsight.utilities.response.mappers.ListQueryMapper; import javax.inject.Inject; -import javax.validation.ConstraintViolation; import javax.validation.Valid; import javax.validation.Validator; import java.util.List; -import java.util.Set; import java.util.UUID; -import java.util.stream.Collectors; @Slf4j @Controller("/${micronaut.bi.api.version}/programs/{programId}" + BrapiVersion.BRAPI_V2) @@ -105,7 +97,7 @@ public HttpResponse>> deleteListById( @PathVariable("programId") UUID programId, @PathVariable("listDbId") String listDbId, HttpRequest request - ) throws DoesNotExistException, ApiException { + ) { boolean hardDelete = false; if (request.getParameters().contains("hardDelete")) { String paramValue = request.getParameters().get("hardDelete"); @@ -119,110 +111,4 @@ public HttpResponse>> deleteListById( return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error retrieving germplasm list records"); } } - - @Get("/lists/{listDbId}") - @Produces(MediaType.APPLICATION_JSON) - @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) - @SuppressWarnings("unchecked") - public HttpResponse>>> getListById( - @PathVariable("programId") UUID programId, - @PathVariable("listDbId") String listDbId, - HttpRequest request) { - try { - // Get the list from the BrAPI service - DeltaListDetails details = brapiListService.getDeltaListDetails(listDbId, programId); - - // Get a new instance of BrAPI query matching the type of list contents - T queryParams = (T) details.getQuery(); - - // Bind query parameters to the object - bindQueryParams(queryParams, request); - - // Perform standard bean validation - Set> violations = validator.validate(queryParams); - if (!violations.isEmpty()) { - List errorMessages = violations.stream() - .map(ConstraintViolation::getMessage) - .collect(Collectors.toList()); - log.info(String.join(", ", errorMessages)); - return HttpResponse.status(HttpStatus.BAD_REQUEST, "Error with list contents search parameters"); - } - - // Fetch the list contents from the BrAPI service - List listContentsBrAPIObjects = (List) details.getDataObjects(); - - // Construct a search request for sorting the list contents - SearchRequest searchRequest = details.constructSearchRequest(queryParams); - - // Get the map used to connect query sorting keys to contents object values - AbstractQueryMapper contentsQueryMapper = details.getQueryMapper(); - - return ResponseUtils.getBrapiQueryResponse(listContentsBrAPIObjects, contentsQueryMapper, queryParams, searchRequest); - } catch (Exception e) { - log.info(e.getMessage(), e); - return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error retrieving list records"); - } - } - - @Get("/lists/{listDbId}/export{?fileExtension}") - @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") - @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) - public HttpResponse germplasmListExport( - @PathVariable("programId") UUID programId, @PathVariable("listDbId") String listDbId, @QueryValue(defaultValue = "XLSX") String fileExtension) { - String downloadErrorMessage = "An error occurred while generating the download file. Contact the development team at bidevteam@cornell.edu."; - try { - // Get the list from the BrAPI service - DeltaListDetails details = brapiListService.getDeltaListDetails(listDbId, programId); - - FileType extension = Enum.valueOf(FileType.class, fileExtension); - DownloadFile listContentsFile = details.exportListObjects(extension); - return HttpResponse.ok(listContentsFile.getStreamedFile()).header(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename="+listContentsFile.getFileName()+extension.getExtension()); - } - catch (Exception e) { - log.info(e.getMessage(), e); - e.printStackTrace(); - HttpResponse response = HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, downloadErrorMessage).contentType(MediaType.TEXT_PLAIN).body(downloadErrorMessage); - return response; - } - } - - private void bindQueryParams(BrapiQuery queryParams, HttpRequest request) { - BeanIntrospection introspection = BeanIntrospection.getIntrospection(BrapiQuery.class); - for (BeanProperty property : introspection.getBeanProperties()) { - String paramName = property.getName(); - if (request.getParameters().contains(paramName)) { - String paramValue = request.getParameters().get(paramName); - Object convertedValue; - Class propertyType = property.getType(); - - if (propertyType.isEnum()) { - convertedValue = convertToEnum(paramValue, (Class>) propertyType); - } else { - convertedValue = convertValue(paramValue, propertyType); - } - - property.set(queryParams, convertedValue); - } - } - } - - private > T convertToEnum(String value, Class> enumClass) { - if (value == null) { - return null; - } - return Enum.valueOf((Class) enumClass, value.toUpperCase()); - } - - - // Convert, if necessary, the values of query parameters to match the type defined for the fields in the BrapiQuery class - private Object convertValue(String value, Class targetType) { - // Implement type conversion logic here - // Other list content types might need more complex logic - if (targetType == String.class) return value; - if (targetType == Integer.class) return Integer.parseInt(value); - if (targetType == Long.class) return Long.parseLong(value); - if (targetType == Boolean.class) return Boolean.parseBoolean(value); - // Add more type conversions as needed - return value; - } } diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java index ed7db131c..55b395ea3 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java @@ -25,7 +25,6 @@ import okhttp3.Request; import org.brapi.client.v2.ApiResponse; import org.brapi.client.v2.model.exceptions.ApiException; -import org.brapi.client.v2.model.queryParams.core.ListQueryParams; import org.brapi.client.v2.modules.core.ListsApi; import org.brapi.v2.model.BrAPIExternalReference; import org.brapi.v2.model.BrAPIResponse; @@ -34,15 +33,15 @@ import org.brapi.v2.model.core.BrAPIListTypes; import org.brapi.v2.model.core.request.BrAPIListNewRequest; import org.brapi.v2.model.core.request.BrAPIListSearchRequest; -import org.brapi.v2.model.core.response.*; -import org.brapi.v2.model.pheno.BrAPIObservation; +import org.brapi.v2.model.core.response.BrAPIListDetails; +import org.brapi.v2.model.core.response.BrAPIListsListResponse; +import org.brapi.v2.model.core.response.BrAPIListsListResponseResult; +import org.brapi.v2.model.core.response.BrAPIListsSingleResponse; import org.breedinginsight.brapi.v1.controller.BrapiVersion; import org.breedinginsight.brapps.importer.daos.ImportDAO; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.daos.ProgramDAO; import org.breedinginsight.model.ProgramBrAPIEndpoints; -import org.breedinginsight.model.delta.DeltaEntityFactory; -import org.breedinginsight.model.delta.DeltaListDetails; import org.breedinginsight.services.ProgramService; import org.breedinginsight.services.brapi.BrAPIEndpointProvider; import org.breedinginsight.services.exceptions.DoesNotExistException; @@ -52,7 +51,10 @@ import javax.inject.Inject; import javax.validation.constraints.NotNull; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.UUID; import java.util.concurrent.TimeUnit; @Slf4j @@ -62,16 +64,14 @@ public class BrAPIListDAO { private ImportDAO importDAO; private final BrAPIDAOUtil brAPIDAOUtil; private final BrAPIEndpointProvider brAPIEndpointProvider; - private final DeltaEntityFactory deltaEntityFactory; private final ProgramService programService; @Inject - public BrAPIListDAO(ProgramDAO programDAO, ImportDAO importDAO, BrAPIDAOUtil brAPIDAOUtil, BrAPIEndpointProvider brAPIEndpointProvider, DeltaEntityFactory deltaEntityFactory, ProgramService programService) { + public BrAPIListDAO(ProgramDAO programDAO, ImportDAO importDAO, BrAPIDAOUtil brAPIDAOUtil, BrAPIEndpointProvider brAPIEndpointProvider, ProgramService programService) { this.programDAO = programDAO; this.importDAO = importDAO; this.brAPIDAOUtil = brAPIDAOUtil; this.brAPIEndpointProvider = brAPIEndpointProvider; - this.deltaEntityFactory = deltaEntityFactory; this.programService = programService; } @@ -211,19 +211,8 @@ public List createBrAPILists(List brapiLi throw new ApiException("No response after creating list"); } - public DeltaListDetails getDeltaListDetailsByDbId(String listDbId, UUID programId) throws ApiException { - ListsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ListsApi.class); - ApiResponse response = api.listsListDbIdGet(listDbId); - if (Objects.isNull(response.getBody()) || Objects.isNull(response.getBody().getResult())) - { - throw new ApiException(); - } - - BrAPIListDetails details = response.getBody().getResult(); - return deltaEntityFactory.makeDeltaListDetailsBean(details); - } - public void deleteBrAPIList(String listDbId, UUID programId, boolean hardDelete) throws ApiException { + // TODO: Switch to using the ListsApi from the BrAPI client library once the delete endpoints are merged into it var programBrAPIBaseUrl = getProgramBrAPIBaseUrl(programId); var requestUrl = HttpUrl.parse(programBrAPIBaseUrl + "/lists/" + listDbId).newBuilder(); requestUrl.addQueryParameter("hardDelete", Boolean.toString(hardDelete)); diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java index 8023327d7..231978368 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java @@ -82,7 +82,7 @@ public Optional getGermplasmByDBID(UUID programId, String germpl return germplasmDAO.getGermplasmByDBID(germplasmId, programId); } - public List> processListData(List germplasm, BrAPIListDetails germplasmList, Program program){ + public List> processListData(List germplasm, List listData, Program program){ Map germplasmByName = new HashMap<>(); for (BrAPIGermplasm g: germplasm) { // Use the full, unique germplasmName with programKey and accessionNumber (GID) for 2 reasons: @@ -97,14 +97,14 @@ public List> processListData(List germplasm, // This holds the BrAPI list items or all germplasm in a program if the list is null. List orderedGermplasmNames = new ArrayList<>(); - if (germplasmList == null) { + if (listData == null) { orderedGermplasmNames = germplasm.stream().sorted((left, right) -> { Integer leftAccessionNumber = Integer.parseInt(left.getAccessionNumber()); Integer rightAccessionNumber = Integer.parseInt(right.getAccessionNumber()); return leftAccessionNumber.compareTo(rightAccessionNumber); }).map(BrAPIGermplasm::getGermplasmName).collect(Collectors.toList()); } else { - orderedGermplasmNames = germplasmList.getData(); + orderedGermplasmNames = listData; } // For export, assign entry number sequentially based on BrAPI list order. @@ -124,7 +124,7 @@ public List> processListData(List germplasm, row.put("Source", source); // Use the entry number in the list map if generated - if(germplasmList == null) { + if(listData == null) { // Not downloading a real list, use GID (https://breedinginsight.atlassian.net/browse/BI-2266). row.put("Entry No", Integer.valueOf(germplasmEntry.getAccessionNumber())); } else { @@ -254,7 +254,9 @@ public DownloadFile exportGermplasm(UUID programId, FileType fileExtension) thro return new DownloadFile(fileName, downloadFile); } - public DownloadFile exportGermplasmList(UUID programId, String listId, FileType fileExtension) throws IllegalArgumentException, ApiException, IOException, DoesNotExistException { + public DownloadFile exportGermplasmList(UUID programId, + String listId, + FileType fileExtension) throws IllegalArgumentException, ApiException, IOException, DoesNotExistException { List columns = GermplasmFileColumns.getOrderedColumns(); //Retrieve germplasm list data @@ -270,7 +272,7 @@ public DownloadFile exportGermplasmList(UUID programId, String listId, FileType String fileName = createFileName(listData, listName); StreamedFile downloadFile; //Convert list data to List> data to pass into file writer - List> processedData = processListData(germplasm, listData, program); + List> processedData = processListData(germplasm, germplasmNames, program); if (fileExtension == FileType.CSV){ downloadFile = CSVWriter.writeToDownload(columns, processedData, fileExtension); diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java index 7dd7c81c6..1ee77e310 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java @@ -12,7 +12,6 @@ import org.breedinginsight.brapi.v2.dao.BrAPIListDAO; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.model.Program; -import org.breedinginsight.model.delta.DeltaListDetails; import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.utilities.Utilities; @@ -91,10 +90,6 @@ public List getListSummariesByTypeAndXref( return programLists; } - public DeltaListDetails getDeltaListDetails(String listDbId, UUID programId) throws ApiException { - return listDAO.getDeltaListDetailsByDbId(listDbId, programId); - } - public void deleteBrAPIList(String listDbId, UUID programId, boolean hardDelete) throws ApiException { listDAO.deleteBrAPIList(listDbId, programId, hardDelete); } diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java b/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java index 8d473ce5a..f1724ef12 100644 --- a/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java +++ b/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java @@ -7,19 +7,16 @@ import lombok.NonNull; import org.brapi.v2.model.core.BrAPIListSummary; import org.brapi.v2.model.core.BrAPIListTypes; -import org.brapi.v2.model.core.response.BrAPIListDetails; -import org.breedinginsight.brapi.v2.dao.BrAPIGermplasmDAO; -import org.breedinginsight.brapi.v2.services.BrAPIGermplasmService; -import org.breedinginsight.brapps.importer.services.processors.experiment.service.ObservationUnitService; - import org.brapi.v2.model.core.BrAPIStudy; import org.brapi.v2.model.core.BrAPITrial; +import org.brapi.v2.model.core.response.BrAPIListDetails; import org.brapi.v2.model.germ.BrAPIGermplasm; import org.brapi.v2.model.pheno.BrAPIObservation; import org.brapi.v2.model.pheno.BrAPIObservationUnit; import org.brapi.v2.model.pheno.BrAPIObservationVariable; +import org.breedinginsight.brapi.v2.services.BrAPIGermplasmService; +import org.breedinginsight.brapps.importer.services.processors.experiment.service.ObservationUnitService; import org.breedinginsight.model.ProgramLocation; -import org.breedinginsight.utilities.Utilities; import javax.inject.Inject; @@ -27,16 +24,10 @@ public class DeltaEntityFactory { private final ObservationUnitService observationUnitService; - private final BrAPIGermplasmService germplasmService; - private final String applicationReferenceSourceBase; @Inject - public DeltaEntityFactory(ObservationUnitService observationUnitService, - BrAPIGermplasmService germplasmService, - @Property(name = "brapi.server.reference-source") String applicationReferenceSourceBase) { + public DeltaEntityFactory(ObservationUnitService observationUnitService) { this.observationUnitService = observationUnitService; - this.germplasmService = germplasmService; - this.applicationReferenceSourceBase = applicationReferenceSourceBase; } private Experiment makeExperiment(BrAPITrial brAPIObject) { @@ -67,46 +58,6 @@ private DeltaObservationVariable makeDeltaObservationVariable(BrAPIObservationVa return new DeltaObservationVariable(brAPIObject); } - private DeltaGermplasmListSummary makeDeltaGermplasmListSummary(BrAPIListSummary brAPIObject) { - return new DeltaGermplasmListSummary(brAPIObject); - } - - private DeltaGermplasmListDetails makeDeltaGermplasmListDetails(BrAPIListDetails brAPIObject, String referenceSourceBase, BrAPIGermplasmService germplasmService) { - return new DeltaGermplasmListDetails(brAPIObject, referenceSourceBase, germplasmService); - } - - @Bean - @Prototype - public DeltaListSummary makeDeltaListSummaryBean(@NonNull BrAPIListSummary brAPIObject) { - BrAPIListTypes listType = brAPIObject.getListType(); - switch (listType) { - case GERMPLASM: return makeDeltaGermplasmListSummary(brAPIObject); - default: return null; - } - } - - @Bean - @Prototype - public DeltaListDetails makeDeltaListDetailsBean(@NonNull BrAPIListDetails brAPIObject) { - BrAPIListTypes listType = brAPIObject.getListType(); - switch (listType) { - case GERMPLASM: return makeDeltaGermplasmListDetailsBean(brAPIObject); - default: return null; - } - } - - @Bean - @Prototype - public DeltaGermplasmListSummary makeDeltaGermplasmListSummaryBean(@NonNull BrAPIListSummary brAPIObject) { - return makeDeltaGermplasmListSummary(brAPIObject); - } - - @Bean - @Prototype - public DeltaGermplasmListDetails makeDeltaGermplasmListDetailsBean(@NonNull BrAPIListDetails brAPIObject) { - return makeDeltaGermplasmListDetails(brAPIObject, applicationReferenceSourceBase, germplasmService); - } - @Bean @Prototype public Experiment makeExperimentBean(@NonNull BrAPITrial brAPIObject) { diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListDetails.java b/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListDetails.java deleted file mode 100644 index 1f8ea3d9c..000000000 --- a/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListDetails.java +++ /dev/null @@ -1,62 +0,0 @@ -package org.breedinginsight.model.delta; - -import io.micronaut.context.annotation.Prototype; -import org.brapi.client.v2.model.exceptions.ApiException; -import org.brapi.v2.model.core.response.BrAPIListDetails; -import org.brapi.v2.model.germ.BrAPIGermplasm; -import org.breedinginsight.api.model.v1.request.query.SearchRequest; -import org.breedinginsight.brapi.v1.model.request.query.BrapiQuery; -import org.breedinginsight.brapi.v2.model.request.query.GermplasmQuery; -import org.breedinginsight.brapi.v2.services.BrAPIGermplasmService; -import org.breedinginsight.brapps.importer.model.exports.FileType; -import org.breedinginsight.model.DownloadFile; -import org.breedinginsight.services.exceptions.DoesNotExistException; -import org.breedinginsight.services.exceptions.UnprocessableEntityException; -import org.breedinginsight.utilities.response.mappers.AbstractQueryMapper; -import org.breedinginsight.utilities.response.mappers.GermplasmQueryMapper; - -import java.io.IOException; -import java.util.List; -import java.util.UUID; - - -@Prototype -public class DeltaGermplasmListDetails extends DeltaListDetails { - - private final BrAPIGermplasmService germplasmService; - - // Note: do not use @Inject, DeltaEntity are always constructed by DeltaEntityFactory. - DeltaGermplasmListDetails(BrAPIListDetails brAPIListDetails, - String referenceSourceBase, - BrAPIGermplasmService germplasmService) { - super(brAPIListDetails, referenceSourceBase); - this.germplasmService = germplasmService; - } - - @Override - public List getDataObjects() throws ApiException, UnprocessableEntityException { - UUID programId = getProgramId().orElseThrow(() -> new UnprocessableEntityException("Program Id not found for list " + getListName())); - return germplasmService.getGermplasmByList(programId, getListDbId()); - } - - @Override - public DownloadFile exportListObjects(FileType extension) throws IllegalArgumentException, ApiException, IOException, DoesNotExistException, UnprocessableEntityException { - UUID programId = getProgramId().orElseThrow(() -> new UnprocessableEntityException("Program Id not found for list " + getListName())); - return germplasmService.exportGermplasmList(programId, getListDbId(), extension); - } - - @Override - public AbstractQueryMapper getQueryMapper() { - return new GermplasmQueryMapper(); - } - - @Override - public BrapiQuery getQuery() { - return new GermplasmQuery(); - } - - @Override - public SearchRequest constructSearchRequest(BrapiQuery queryParams) { - return ((GermplasmQuery) queryParams).constructSearchRequest(); - } -} diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListSummary.java b/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListSummary.java deleted file mode 100644 index 6ddd2518b..000000000 --- a/src/main/java/org/breedinginsight/model/delta/DeltaGermplasmListSummary.java +++ /dev/null @@ -1,16 +0,0 @@ -package org.breedinginsight.model.delta; - -import io.micronaut.context.annotation.Prototype; -import lombok.Getter; -import lombok.Setter; -import org.brapi.v2.model.core.BrAPIListSummary; -import org.breedinginsight.brapps.importer.model.response.ImportObjectState; - - -@Prototype -public class DeltaGermplasmListSummary extends DeltaListSummary { - - // Note: do not use @Inject, DeltaEntity are always constructed by DeltaEntityFactory. - DeltaGermplasmListSummary(BrAPIListSummary brAPIListSummary) { super(brAPIListSummary); } - -} diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaListDetails.java b/src/main/java/org/breedinginsight/model/delta/DeltaListDetails.java deleted file mode 100644 index 9f2c745bf..000000000 --- a/src/main/java/org/breedinginsight/model/delta/DeltaListDetails.java +++ /dev/null @@ -1,78 +0,0 @@ -package org.breedinginsight.model.delta; - -import io.micronaut.context.annotation.Prototype; -import lombok.Getter; -import lombok.Setter; -import lombok.Value; -import org.brapi.client.v2.model.exceptions.ApiException; -import org.brapi.v2.model.BrAPIExternalReference; -import org.brapi.v2.model.core.BrAPIListTypes; -import org.brapi.v2.model.core.response.BrAPIListDetails; -import org.breedinginsight.api.model.v1.request.query.SearchRequest; -import org.breedinginsight.brapi.v1.model.request.query.BrapiQuery; -import org.breedinginsight.brapps.importer.model.exports.FileType; -import org.breedinginsight.brapps.importer.model.response.ImportObjectState; -import io.micronaut.context.annotation.Property; -import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; -import org.breedinginsight.model.DownloadFile; -import org.breedinginsight.services.exceptions.DoesNotExistException; -import org.breedinginsight.services.exceptions.UnprocessableEntityException; -import org.breedinginsight.utilities.Utilities; -import org.breedinginsight.utilities.response.mappers.AbstractQueryMapper; - -import java.io.IOException; -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.UUID; - - -@Prototype -public abstract class DeltaListDetails extends DeltaEntity { - private final String referenceSourceBase; - @Getter - @Setter - private ImportObjectState state; - - // Note: do not use @Inject, DeltaEntity are always constructed by DeltaEntityFactory. - DeltaListDetails(BrAPIListDetails brAPIListDetails, String referenceSourceBase) { - super(brAPIListDetails); - this.referenceSourceBase = referenceSourceBase; - } - - public abstract List getDataObjects() throws ApiException, UnprocessableEntityException; - public abstract DownloadFile exportListObjects(FileType extension) throws IllegalArgumentException, ApiException, IOException, DoesNotExistException, UnprocessableEntityException; - public abstract AbstractQueryMapper getQueryMapper(); - - public abstract BrapiQuery getQuery(); - - public abstract SearchRequest constructSearchRequest(BrapiQuery queryParams); - - public BrAPIListTypes getListType() { - return entity.getListType(); - } - - public Optional getListId() { - return getXrefId(ExternalReferenceSource.LISTS); - } - - public Optional getProgramId() { - return getXrefId(ExternalReferenceSource.PROGRAMS); - } - - public String getListDbId() { - return entity.getListDbId(); - } - - public String getListName() { - return entity.getListName(); - } - - private Optional getXrefId(ExternalReferenceSource source) { - // Get the external reference if it exists - Optional xrefOptional = Utilities.getExternalReference(entity.getExternalReferences(), referenceSourceBase, source); - - // Parse the Deltabreed ID from the xref - return xrefOptional.map(BrAPIExternalReference::getReferenceId).map(UUID::fromString); - } -} diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaListSummary.java b/src/main/java/org/breedinginsight/model/delta/DeltaListSummary.java deleted file mode 100644 index 80569d077..000000000 --- a/src/main/java/org/breedinginsight/model/delta/DeltaListSummary.java +++ /dev/null @@ -1,20 +0,0 @@ -package org.breedinginsight.model.delta; - -import io.micronaut.context.annotation.Prototype; -import lombok.Getter; -import lombok.Setter; -import org.brapi.v2.model.core.BrAPIListSummary; -import org.breedinginsight.brapps.importer.model.response.ImportObjectState; - - -@Prototype -public class DeltaListSummary extends DeltaEntity { - - @Getter - @Setter - private ImportObjectState state; - - // Note: do not use @Inject, DeltaEntity are always constructed by DeltaEntityFactory. - DeltaListSummary(BrAPIListSummary brAPIListSummary) { super(brAPIListSummary); } - -} diff --git a/src/main/java/org/breedinginsight/utilities/Utilities.java b/src/main/java/org/breedinginsight/utilities/Utilities.java index 7ed72cfef..20f3254d6 100644 --- a/src/main/java/org/breedinginsight/utilities/Utilities.java +++ b/src/main/java/org/breedinginsight/utilities/Utilities.java @@ -17,7 +17,6 @@ package org.breedinginsight.utilities; -import io.micronaut.context.annotation.Property; import org.apache.commons.lang3.StringUtils; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.v2.model.BrAPIExternalReference; @@ -25,7 +24,6 @@ import org.breedinginsight.model.Program; import org.flywaydb.core.api.migration.Context; -import javax.inject.Inject; import java.lang.reflect.Field; import java.sql.ResultSet; import java.sql.Statement; From 0a3fda74ccc5236ab60ee82755e9c1bcc5056f35 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 21 Nov 2024 14:39:09 -0500 Subject: [PATCH 05/16] update tests --- .../org/breedinginsight/model/delta/DeltaEntityFactory.java | 5 ----- .../brapi/v2/GermplasmControllerIntegrationTest.java | 4 ++-- .../brapi/v2/ListControllerIntegrationTest.java | 5 +---- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java b/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java index f1724ef12..059f91e66 100644 --- a/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java +++ b/src/main/java/org/breedinginsight/model/delta/DeltaEntityFactory.java @@ -2,19 +2,14 @@ import io.micronaut.context.annotation.Bean; import io.micronaut.context.annotation.Factory; -import io.micronaut.context.annotation.Property; import io.micronaut.context.annotation.Prototype; import lombok.NonNull; -import org.brapi.v2.model.core.BrAPIListSummary; -import org.brapi.v2.model.core.BrAPIListTypes; import org.brapi.v2.model.core.BrAPIStudy; import org.brapi.v2.model.core.BrAPITrial; -import org.brapi.v2.model.core.response.BrAPIListDetails; import org.brapi.v2.model.germ.BrAPIGermplasm; import org.brapi.v2.model.pheno.BrAPIObservation; import org.brapi.v2.model.pheno.BrAPIObservationUnit; import org.brapi.v2.model.pheno.BrAPIObservationVariable; -import org.breedinginsight.brapi.v2.services.BrAPIGermplasmService; import org.breedinginsight.brapps.importer.services.processors.experiment.service.ObservationUnitService; import org.breedinginsight.model.ProgramLocation; diff --git a/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java index 23e8f54d4..7c80482d9 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java @@ -245,7 +245,7 @@ public void getAllGermplasmByListSuccess() { String germplasmListDbId = fetchGermplasmListDbId(programId); // Build the endpoint to get germplasm by germplasm list. - String endpoint = String.format("/programs/%s/germplasm/lists/%s/records", programId, germplasmListDbId); + String endpoint = String.format("/programs/%s/brapi/v2/germplasm?list=%s", programId, germplasmListDbId); // Get germplasm by list. Flowable> call = client.exchange( @@ -258,7 +258,7 @@ public void getAllGermplasmByListSuccess() { JsonObject result = JsonParser.parseString(response.body()).getAsJsonObject().getAsJsonObject("result"); JsonArray data = result.getAsJsonArray("data"); - assertEquals(data.size(), 3, "Wrong number of germplasm were returned"); + assertEquals(3, data.size(), "Wrong number of germplasm were returned"); } @Test diff --git a/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java index 9515b72a6..e29ce7e24 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java @@ -145,9 +145,7 @@ public void setup() { newExp.put(traits.get(0).getObservationVariableName(), "1"); JsonObject result = importTestUtils.uploadAndFetchWorkflow( - importTestUtils.writeExperimentDataToFile(List.of(newExp), traits), null, true, client, program, mappingId, newExperimentWorkflowId - ); - + importTestUtils.writeExperimentDataToFile(List.of(newExp), traits), null, true, client, program, mappingId, newExperimentWorkflowId); } @Test @@ -189,5 +187,4 @@ public void getAllListsSuccess() { } } } - } From f6d67453aafa968067032d2f94f1a99701be878d Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Fri, 22 Nov 2024 15:22:47 -0500 Subject: [PATCH 06/16] [BI-2376] - renamed lists methods --- .../java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java | 6 ++---- .../breedinginsight/brapi/v2/services/BrAPIListService.java | 2 +- .../brapi/v2/services/BrAPITrialService.java | 2 +- .../importer/services/processors/ExperimentProcessor.java | 4 ++-- .../steps/PopulateExistingPendingImportObjectsStep.java | 2 +- .../processors/experiment/service/DatasetService.java | 6 +----- 6 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java index 801b9775c..5f64eb5f5 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java @@ -20,7 +20,6 @@ import lombok.extern.slf4j.Slf4j; import org.brapi.client.v2.ApiResponse; import org.brapi.client.v2.model.exceptions.ApiException; -import org.brapi.client.v2.model.queryParams.core.ListQueryParams; import org.brapi.client.v2.modules.core.ListsApi; import org.brapi.v2.model.BrAPIExternalReference; import org.brapi.v2.model.BrAPIResponse; @@ -30,7 +29,6 @@ import org.brapi.v2.model.core.request.BrAPIListNewRequest; import org.brapi.v2.model.core.request.BrAPIListSearchRequest; import org.brapi.v2.model.core.response.*; -import org.brapi.v2.model.pheno.BrAPIObservation; import org.breedinginsight.brapps.importer.daos.ImportDAO; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.daos.ProgramDAO; @@ -82,7 +80,7 @@ public BrAPIListsSingleResponse getListById(String listId, UUID programId) throw return response.getBody(); } - public List getListBySearch(@NotNull BrAPIListSearchRequest searchRequest, UUID programId) throws ApiException { + public List getListsBySearch(@NotNull BrAPIListSearchRequest searchRequest, UUID programId) throws ApiException { ListsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ListsApi.class); List programLists = brAPIDAOUtil.search(api::searchListsPost, api::searchListsSearchResultsDbIdGet, searchRequest); if (searchRequest.getExternalReferenceSources() != null && searchRequest.getExternalReferenceIDs() != null) { @@ -94,7 +92,7 @@ public List getListBySearch(@NotNull BrAPIListSearchRequest se } - public List getListByTypeAndExternalRef(@NotNull BrAPIListTypes listType, UUID programId, String externalReferenceSource, UUID externalReferenceId) throws ApiException { + public List getListsByTypeAndExternalRef(@NotNull BrAPIListTypes listType, UUID programId, String externalReferenceSource, UUID externalReferenceId) throws ApiException { BrAPIListSearchRequest searchRequest = new BrAPIListSearchRequest() .externalReferenceIDs(List.of(externalReferenceId.toString())) .externalReferenceSources(List.of(externalReferenceSource)) diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java index 3353ac17c..23d37fc15 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java @@ -53,7 +53,7 @@ public List getListSummariesByTypeAndXref( if (xrefId != null && !xrefId.isEmpty()) { searchRequest.externalReferenceIDs(List.of(xrefId)); } - List lists = listDAO.getListBySearch(searchRequest, program.getId()); + List lists = listDAO.getListsBySearch(searchRequest, program.getId()); if (lists == null) { throw new DoesNotExistException("list not returned from BrAPI service"); } 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 c6b55fc76..b1c7333c3 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -638,7 +638,7 @@ private void addObsVarDataToRow( } public List getDatasetObsVars(String datasetId, Program program) throws ApiException, DoesNotExistException { - List lists = listDAO.getListByTypeAndExternalRef( + List lists = listDAO.getListsByTypeAndExternalRef( BrAPIListTypes.OBSERVATIONVARIABLES, program.getId(), String.format("%s/%s", referenceSource, ExternalReferenceSource.DATASET.getName()), diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 34030b795..0666870c6 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -2091,7 +2091,7 @@ private Map> initializeObsVarDatas try { List existingDatasets = brAPIListDAO - .getListByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, + .getListsByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, program.getId(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName()), UUID.fromString(datasetId)); @@ -2121,7 +2121,7 @@ private Map> initializeObsVarDatas ).getId().toString(); try { List existingDatasets = brAPIListDAO - .getListByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, + .getListsByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, program.getId(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName()), UUID.fromString(datasetId)); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java index ab4233b0a..b6bca19d0 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java @@ -383,7 +383,7 @@ private Map> initializeObsVarDatas try { List existingDatasets = brAPIListDAO - .getListByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, + .getListsByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, program.getId(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName()), UUID.fromString(datasetId)); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java index f85137e15..68aca9e2b 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java @@ -23,21 +23,17 @@ import org.brapi.v2.model.BrAPIExternalReference; import org.brapi.v2.model.core.BrAPIListSummary; import org.brapi.v2.model.core.BrAPIListTypes; -import org.brapi.v2.model.core.BrAPITrial; import org.brapi.v2.model.core.response.BrAPIListDetails; -import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; import org.breedinginsight.brapi.v2.dao.BrAPIListDAO; import org.breedinginsight.brapps.importer.model.response.ImportObjectState; import org.breedinginsight.brapps.importer.model.response.PendingImportObject; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.model.Program; -import org.breedinginsight.model.Trait; import org.breedinginsight.utilities.Utilities; import javax.inject.Inject; import javax.inject.Singleton; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.UUID; @@ -74,7 +70,7 @@ public Optional fetchDatasetById(String id, Program program) t // Retrieve existing dataset summaries based on program ID and external reference List existingDatasets = brAPIListDAO - .getListByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, + .getListsByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, program.getId(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName()), UUID.fromString(id)); From cd9498fe0fa0c420531d4471dcd699feed34619c Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Fri, 22 Nov 2024 15:23:42 -0500 Subject: [PATCH 07/16] [BI-2376] - added method stubs --- .../java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java | 5 +++++ .../org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java | 2 ++ .../brapi/v2/dao/impl/BrAPITrialDAOImpl.java | 6 ++++++ 3 files changed, 13 insertions(+) diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java index 5f64eb5f5..a5a7ed5c6 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java @@ -18,6 +18,7 @@ package org.breedinginsight.brapi.v2.dao; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.NotImplementedException; import org.brapi.client.v2.ApiResponse; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.client.v2.modules.core.ListsApi; @@ -194,4 +195,8 @@ public List createBrAPILists(List brapiLi throw new ApiException("No response after creating list"); } + + public void deleteBrAPIList(String listDbId, UUID id, boolean hard) { + throw new NotImplementedException(); + } } diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java index 83ee3f47b..046ca5aad 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java @@ -45,4 +45,6 @@ List createBrAPITrials(List brAPITrialList, UUID program List getTrialsByDbIds(Collection trialDbIds, Program program) throws ApiException; List getTrialsByExperimentIds(Collection experimentIds, Program program) throws ApiException; + + void deleteExperiment(UUID experimentId, boolean hard); } 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 066c7017d..0a39d29aa 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 @@ -21,6 +21,7 @@ import io.micronaut.http.server.exceptions.InternalServerException; import io.micronaut.scheduling.annotation.Scheduled; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.NotImplementedException; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.client.v2.modules.core.TrialsApi; import org.brapi.v2.model.BrAPIExternalReference; @@ -277,4 +278,9 @@ public List getTrialsByExperimentIds(Collection experimentIds, trialSearch ), program.getKey()); } + + @Override + public void deleteExperiment(UUID experimentId, boolean hard) { + throw new NotImplementedException(); + } } From 098219249048350fa770b301ea67215bc8d5cfbc Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 26 Nov 2024 11:24:18 -0500 Subject: [PATCH 08/16] add httpok helper methods to BrAPIDAOUtil --- .../brapi/v2/dao/BrAPIListDAO.java | 39 +++------------ .../utilities/BrAPIDAOUtil.java | 47 +++++++++++++++++-- .../v2/ListControllerIntegrationTest.java | 32 +++++++++++++ .../daos/BrAPIDAOUtilUnitTest.java | 10 +++- ...gwaGenotypeServiceImplIntegrationTest.java | 7 ++- 5 files changed, 96 insertions(+), 39 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java index 55b395ea3..11e079842 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java @@ -64,15 +64,16 @@ public class BrAPIListDAO { private ImportDAO importDAO; private final BrAPIDAOUtil brAPIDAOUtil; private final BrAPIEndpointProvider brAPIEndpointProvider; - private final ProgramService programService; @Inject - public BrAPIListDAO(ProgramDAO programDAO, ImportDAO importDAO, BrAPIDAOUtil brAPIDAOUtil, BrAPIEndpointProvider brAPIEndpointProvider, ProgramService programService) { + public BrAPIListDAO(ProgramDAO programDAO, + ImportDAO importDAO, + BrAPIDAOUtil brAPIDAOUtil, + BrAPIEndpointProvider brAPIEndpointProvider) { this.programDAO = programDAO; this.importDAO = importDAO; this.brAPIDAOUtil = brAPIDAOUtil; this.brAPIEndpointProvider = brAPIEndpointProvider; - this.programService = programService; } public List getListByName(List listNames, UUID programId) throws ApiException { @@ -213,7 +214,7 @@ public List createBrAPILists(List brapiLi public void deleteBrAPIList(String listDbId, UUID programId, boolean hardDelete) throws ApiException { // TODO: Switch to using the ListsApi from the BrAPI client library once the delete endpoints are merged into it - var programBrAPIBaseUrl = getProgramBrAPIBaseUrl(programId); + var programBrAPIBaseUrl = brAPIDAOUtil.getProgramBrAPIBaseUrl(programId); var requestUrl = HttpUrl.parse(programBrAPIBaseUrl + "/lists/" + listDbId).newBuilder(); requestUrl.addQueryParameter("hardDelete", Boolean.toString(hardDelete)); HttpUrl url = requestUrl.build(); @@ -222,35 +223,7 @@ public void deleteBrAPIList(String listDbId, UUID programId, boolean hardDelete) .addHeader("Content-Type", "application/json") .build(); - makeCall(brapiRequest); + brAPIDAOUtil.makeCall(brapiRequest); } - private void makeCall(Request brapiRequest) throws ApiException { - OkHttpClient client = new OkHttpClient.Builder() - .readTimeout(5, TimeUnit.MINUTES) - .build(); - try { - client.newCall(brapiRequest).execute(); - } catch (IOException e) { - log.error("Error calling BrAPI Service", e); - throw new ApiException("Error calling BrAPI Service"); - } - } - - private String getProgramBrAPIBaseUrl(UUID programId) { - ProgramBrAPIEndpoints programBrAPIEndpoints; - try { - programBrAPIEndpoints = programService.getBrapiEndpoints(programId); - } catch (DoesNotExistException e) { - throw new HttpStatusException(HttpStatus.NOT_FOUND, "Program does not exist"); - } - - if(programBrAPIEndpoints.getCoreUrl().isEmpty()) { - log.error("Program: " + programId + " is missing BrAPI URL config"); - throw new HttpStatusException(HttpStatus.INTERNAL_SERVER_ERROR, ""); - } - var programBrAPIBaseUrl = programBrAPIEndpoints.getCoreUrl().get(); - programBrAPIBaseUrl = programBrAPIBaseUrl.endsWith("/") ? programBrAPIBaseUrl.substring(0, programBrAPIBaseUrl.length() - 1) : programBrAPIBaseUrl; - return programBrAPIBaseUrl.endsWith(BrapiVersion.BRAPI_V2) ? programBrAPIBaseUrl : programBrAPIBaseUrl + BrapiVersion.BRAPI_V2; - } } diff --git a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java index e03a67017..075d6bb2b 100644 --- a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java +++ b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java @@ -18,25 +18,33 @@ package org.breedinginsight.utilities; import io.micronaut.context.annotation.Property; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.exceptions.HttpStatusException; import io.micronaut.http.server.exceptions.InternalServerException; import io.reactivex.functions.*; import lombok.extern.slf4j.Slf4j; +import okhttp3.OkHttpClient; +import okhttp3.Request; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; import org.brapi.client.v2.ApiResponse; import org.brapi.client.v2.model.exceptions.ApiException; -import org.brapi.client.v2.modules.germplasm.GermplasmApi; import org.brapi.v2.model.*; -import org.brapi.v2.model.germ.BrAPIGermplasm; -import org.brapi.v2.model.germ.response.BrAPIGermplasmSingleResponse; +import org.breedinginsight.brapi.v1.controller.BrapiVersion; import org.breedinginsight.brapps.importer.model.ImportUpload; +import org.breedinginsight.model.ProgramBrAPIEndpoints; +import org.breedinginsight.services.ProgramService; +import org.breedinginsight.services.exceptions.DoesNotExistException; import javax.inject.Inject; import javax.inject.Singleton; +import java.io.IOException; import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.UUID; +import java.util.concurrent.TimeUnit; import static org.brapi.v2.model.BrAPIWSMIMEDataTypes.APPLICATION_JSON; @@ -48,16 +56,19 @@ public class BrAPIDAOUtil { private final Duration searchTimeout; private final int pageSize; private final int postGroupSize; + private final ProgramService programService; @Inject public BrAPIDAOUtil(@Property(name = "brapi.search.wait-time") int searchWaitTime, @Property(name = "brapi.read-timeout") Duration searchTimeout, @Property(name = "brapi.page-size") int pageSize, - @Property(name = "brapi.post-group-size") int postGroupSize) { + @Property(name = "brapi.post-group-size") int postGroupSize, + ProgramService programService) { this.searchWaitTime = searchWaitTime; this.searchTimeout = searchTimeout; this.pageSize = pageSize; this.postGroupSize = postGroupSize; + this.programService = programService; } public List search(Function, Optional>>> searchMethod, @@ -366,4 +377,32 @@ public List post(List brapiObjects, return post(brapiObjects, null, postMethod, null); } + public void makeCall(Request brapiRequest) throws ApiException { + OkHttpClient client = new OkHttpClient.Builder() + .readTimeout(5, TimeUnit.MINUTES) + .build(); + try { + client.newCall(brapiRequest).execute(); + } catch (IOException e) { + log.error("Error calling BrAPI Service", e); + throw new ApiException("Error calling BrAPI Service"); + } + } + + public String getProgramBrAPIBaseUrl(UUID programId) { + ProgramBrAPIEndpoints programBrAPIEndpoints; + try { + programBrAPIEndpoints = programService.getBrapiEndpoints(programId); + } catch (DoesNotExistException e) { + throw new HttpStatusException(HttpStatus.NOT_FOUND, "Program does not exist"); + } + + if(programBrAPIEndpoints.getCoreUrl().isEmpty()) { + log.error("Program: " + programId + " is missing BrAPI URL config"); + throw new HttpStatusException(HttpStatus.INTERNAL_SERVER_ERROR, ""); + } + var programBrAPIBaseUrl = programBrAPIEndpoints.getCoreUrl().get(); + programBrAPIBaseUrl = programBrAPIBaseUrl.endsWith("/") ? programBrAPIBaseUrl.substring(0, programBrAPIBaseUrl.length() - 1) : programBrAPIBaseUrl; + return programBrAPIBaseUrl.endsWith(BrapiVersion.BRAPI_V2) ? programBrAPIBaseUrl : programBrAPIBaseUrl + BrapiVersion.BRAPI_V2; + } } diff --git a/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java index e29ce7e24..84f7385b9 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java @@ -49,6 +49,7 @@ import java.time.OffsetDateTime; import java.util.*; +import static io.micronaut.http.HttpRequest.DELETE; import static io.micronaut.http.HttpRequest.GET; import static org.junit.jupiter.api.Assertions.*; @@ -187,4 +188,35 @@ public void getAllListsSuccess() { } } } + + + @Test + @SneakyThrows + public void deleteListSuccess() { + // A GET request to the brapi/v2/lists endpoint with no query params should return all lists. + Flowable> getCall = client.exchange( + GET(String.format("/programs/%s/brapi/v2/lists", program.getId().toString())) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class + ); + + // Ensure 200 OK response for fetching lists. + HttpResponse beforeResponse = getCall.blockingFirst(); + assertEquals(HttpStatus.OK, beforeResponse.getStatus()); + + // Parse result. + JsonObject beforeResult = JsonParser.parseString(beforeResponse.body()).getAsJsonObject().getAsJsonObject("result"); + JsonArray beforeData = beforeResult.getAsJsonArray("data"); + + // A DELETE request to the brapi/v2/lists/ endpoint with no query params should delete the list. + String deleteListDbId = beforeData.get(0).getAsJsonObject().get("listDbId").getAsString(); + Flowable> deleteCall = client.exchange( + DELETE(String.format("/programs/%s/brapi/v2/lists/%s", program.getId().toString(), deleteListDbId)) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class + ); + + // Ensure 204 NO_CONTENT response for deleting a list. + HttpResponse deleteResponse = deleteCall.blockingFirst(); + assertEquals(HttpStatus.NO_CONTENT, deleteResponse.getStatus()); + + } } diff --git a/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java b/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java index 0cb51387e..427ed6d88 100644 --- a/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java +++ b/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java @@ -1,6 +1,7 @@ package org.breedinginsight.daos; import com.google.gson.JsonObject; +import io.micronaut.test.annotation.MockBean; import lombok.SneakyThrows; import org.apache.commons.lang3.tuple.Pair; import org.brapi.client.v2.ApiResponse; @@ -11,6 +12,7 @@ import org.brapi.v2.model.germ.response.BrAPIGermplasmListResponseResult; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.model.Program; +import org.breedinginsight.services.ProgramService; import org.breedinginsight.utilities.BrAPIDAOUtil; import org.breedinginsight.utilities.Utilities; import org.junit.jupiter.api.BeforeEach; @@ -21,6 +23,8 @@ import java.time.temporal.ChronoUnit; import java.util.*; +import static org.mockito.Mockito.mock; + @TestInstance(TestInstance.Lifecycle.PER_CLASS) public class BrAPIDAOUtilUnitTest { @@ -30,6 +34,10 @@ public class BrAPIDAOUtilUnitTest { private Program testProgram; private List paginatedGermplasm; private BrAPIGermplasmSearchRequest germplasmSearch; + @MockBean(ProgramService.class) + ProgramService programService() { + return mock(ProgramService.class); + } public Integer fetchPaginatedGermplasm(int page, int pageSize) { paginatedGermplasm = new ArrayList<>(); @@ -62,7 +70,7 @@ public ApiResponse, Optional Date: Tue, 26 Nov 2024 14:45:29 -0500 Subject: [PATCH 09/16] [BI-2376] - initial implementation --- .../v1/controller/ExperimentController.java | 9 ++++-- .../brapi/v2/dao/BrAPITrialDAO.java | 2 +- .../brapi/v2/dao/impl/BrAPITrialDAOImpl.java | 16 ++++++++-- .../brapi/v2/services/BrAPITrialService.java | 30 +++++++++++++++++-- 4 files changed, 47 insertions(+), 10 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 de40bdf46..059b0428e 100644 --- a/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java +++ b/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java @@ -319,9 +319,12 @@ public HttpResponse deleteExperiment( if(program.isEmpty()) { return HttpResponse.notFound(); } - - experimentService.deleteExperiment(program.get(), experimentId, hard); - + // TODO: If hard and non-zero result, return 409 Conflict. + int observationCount = experimentService.deleteExperiment(program.get(), experimentId, hard); + if (observationCount > 0 && hard) { + // 409 Conflict. https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409 + return HttpResponse.status(HttpStatus.CONFLICT); + } return HttpResponse.ok(); } catch (Exception e) { log.error("Error deleting experiment.\n\tprogramId: " + programId + "\n\texperimentId: " + experimentId + "\n\thard: " + hard); diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java index 046ca5aad..49315cb82 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java @@ -46,5 +46,5 @@ List createBrAPITrials(List brAPITrialList, UUID program List getTrialsByExperimentIds(Collection experimentIds, Program program) throws ApiException; - void deleteExperiment(UUID experimentId, boolean hard); + void deleteBrAPITrial(Program program, BrAPITrial trial, boolean hard) throws ApiException; } 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 0a39d29aa..d62d36876 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 @@ -21,7 +21,8 @@ import io.micronaut.http.server.exceptions.InternalServerException; import io.micronaut.scheduling.annotation.Scheduled; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang3.NotImplementedException; +import okhttp3.HttpUrl; +import okhttp3.Request; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.client.v2.modules.core.TrialsApi; import org.brapi.v2.model.BrAPIExternalReference; @@ -280,7 +281,16 @@ public List getTrialsByExperimentIds(Collection experimentIds, } @Override - public void deleteExperiment(UUID experimentId, boolean hard) { - throw new NotImplementedException(); + public void deleteBrAPITrial(Program program, BrAPITrial trial, boolean hard) throws ApiException { + var programBrAPIBaseUrl = getProgramBrAPIBaseUrl(program.getId()); + var requestUrl = HttpUrl.parse(programBrAPIBaseUrl + "/trials/" + trial.getTrialDbId()).newBuilder(); + requestUrl.addQueryParameter("hardDelete", Boolean.toString(hard)); + HttpUrl url = requestUrl.build(); + var brapiRequest = new Request.Builder().url(url) + .method("DELETE", null) + .addHeader("Content-Type", "application/json") + .build(); + + makeCall(brapiRequest); } } 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 b1c7333c3..a63d4632c 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -667,10 +667,34 @@ public BrAPITrial getExperiment(Program program, UUID experimentId) throws ApiEx return experiments.get(0); } - public boolean deleteExperiment(Program program, UUID experimentId, boolean hard) throws ApiException { - // TODO: make BrAPI request to delete experiment. - // TODO: make BrAPI request to delete list for default observation dataset. + // TODO: create a result type for delete requests? + // The caller could infer from the number of obs and hard whether delete succeeded. + public int deleteExperiment(Program program, UUID experimentId, boolean hard) throws ApiException { + // TODO: check for observations! + BrAPITrial trial = trialDAO.getTrialsByExperimentIds(List.of(experimentId), program).get(0); + List existingObservations = observationDAO.getObservationsByTrialDbId(List.of(trial.getTrialDbId()), program); + // If there are no observations or a soft delete is requested, proceed. + if (existingObservations.isEmpty() || !hard) { + // Make request to delete experiment. + trialDAO.deleteBrAPITrial(program, trial, hard); + // Get all lists for the trial. + List lists = listDAO + .getListsByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, + program.getId(), + Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.TRIALS), + experimentId); + // TODO: replace with a single call to a batch delete method if that becomes available. + // Iterate over lists, delete each by listDbId. + for (BrAPIListSummary list : lists) { + listDAO.deleteBrAPIList(list.getListDbId(), program.getId(), hard); // TODO: not yet implemented. + } + } else { + // Trying to hard delete a trial with existing observations, return 409 Conflict response. + // TODO: remove if unused. + } + // Successful or not, return the number of observations in this experiment. + return existingObservations.size(); } private Map createExportRow( From a0c5e9196aa41d366027f3cc9f8abc7cbad7c8b7 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:27:35 -0500 Subject: [PATCH 10/16] [BI-2376] - updated method reference --- .../breedinginsight/brapi/v2/dao/impl/BrAPITrialDAOImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d62d36876..191e48f5a 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 @@ -282,7 +282,7 @@ public List getTrialsByExperimentIds(Collection experimentIds, @Override public void deleteBrAPITrial(Program program, BrAPITrial trial, boolean hard) throws ApiException { - var programBrAPIBaseUrl = getProgramBrAPIBaseUrl(program.getId()); + var programBrAPIBaseUrl = brAPIDAOUtil.getProgramBrAPIBaseUrl(program.getId()); var requestUrl = HttpUrl.parse(programBrAPIBaseUrl + "/trials/" + trial.getTrialDbId()).newBuilder(); requestUrl.addQueryParameter("hardDelete", Boolean.toString(hard)); HttpUrl url = requestUrl.build(); @@ -291,6 +291,6 @@ public void deleteBrAPITrial(Program program, BrAPITrial trial, boolean hard) th .addHeader("Content-Type", "application/json") .build(); - makeCall(brapiRequest); + brAPIDAOUtil.makeCall(brapiRequest); } } From 7dbaf0961eb928796781abf441ad2931d0849bd2 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Thu, 5 Dec 2024 16:41:20 -0500 Subject: [PATCH 11/16] [BI-2376] - added cache invalidation --- .../brapi/v2/dao/BrAPICachedDAO.java | 16 ++++++++++++++++ .../brapi/v2/dao/BrAPIObservationDAO.java | 16 +++++++--------- .../brapi/v2/dao/BrAPIObservationUnitDAO.java | 15 ++++++--------- .../brapi/v2/dao/BrAPIStudyDAO.java | 15 ++++++--------- .../brapi/v2/dao/BrAPITrialDAO.java | 2 ++ .../brapi/v2/dao/impl/BrAPITrialDAOImpl.java | 7 +++++++ .../brapi/v2/services/BrAPITrialService.java | 6 ++++++ 7 files changed, 50 insertions(+), 27 deletions(-) create mode 100644 src/main/java/org/breedinginsight/brapi/v2/dao/BrAPICachedDAO.java diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPICachedDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPICachedDAO.java new file mode 100644 index 000000000..fb35bd448 --- /dev/null +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPICachedDAO.java @@ -0,0 +1,16 @@ +package org.breedinginsight.brapi.v2.dao; + +import org.breedinginsight.daos.cache.ProgramCache; + +import java.util.UUID; + +public abstract class BrAPICachedDAO { + + protected ProgramCache programCache; + + public void repopulateCache(UUID programId) { + // TODO: test calling populate alone (without invalidate first). + this.programCache.invalidate(programId); + this.programCache.populate(programId); + } +} diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java index e32dac6d9..fdfbd18a1 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationDAO.java @@ -36,7 +36,6 @@ import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.daos.ProgramDAO; -import org.breedinginsight.daos.cache.ProgramCache; import org.breedinginsight.daos.cache.ProgramCacheProvider; import org.breedinginsight.model.Program; import org.breedinginsight.services.brapi.BrAPIEndpointProvider; @@ -54,7 +53,7 @@ @Singleton @Slf4j -public class BrAPIObservationDAO { +public class BrAPIObservationDAO extends BrAPICachedDAO { private ProgramDAO programDAO; private ImportDAO importDAO; @@ -63,7 +62,6 @@ public class BrAPIObservationDAO { private final BrAPIEndpointProvider brAPIEndpointProvider; private final String referenceSource; private boolean runScheduledTasks; - private final ProgramCache programObservationCache; @Inject public BrAPIObservationDAO(ProgramDAO programDAO, @@ -81,7 +79,7 @@ public BrAPIObservationDAO(ProgramDAO programDAO, this.brAPIEndpointProvider = brAPIEndpointProvider; this.referenceSource = referenceSource; this.runScheduledTasks = runScheduledTasks; - this.programObservationCache = programCacheProvider.getProgramCache(this::fetchProgramObservations, BrAPIObservation.class); + this.programCache = programCacheProvider.getProgramCache(this::fetchProgramObservations, BrAPIObservation.class); } @Scheduled(initialDelay = "3s") @@ -93,7 +91,7 @@ public void setup() { log.debug("populating observation cache"); List programs = programDAO.getActive(); if (programs != null) { - programObservationCache.populate(programs.stream().map(Program::getId).collect(Collectors.toList())); + programCache.populate(programs.stream().map(Program::getId).collect(Collectors.toList())); } } @@ -163,7 +161,7 @@ private void processObservations(String programKey, List obser * Get all observations for a program from the cache. */ private Map getProgramObservations(UUID programId) throws ApiException { - return programObservationCache.get(programId); + return programCache.get(programId); } // Note: not using cache, because unique studyName (with "[ProgramKey-ExtraInfo]") is not stored directly on Observation. @@ -259,7 +257,7 @@ public List createBrAPIObservations(List brA List postResponse = brAPIDAOUtil.post(brAPIObservationList, upload, api::observationsPost, importDAO::update); return processObservationsForCache(postResponse, program.getKey()); }; - return programObservationCache.post(programId, postFunction); + return programCache.post(programId, postFunction); } return new ArrayList<>(); } catch (Exception e) { @@ -287,7 +285,7 @@ public BrAPIObservation updateBrAPIObservation(String dbId, BrAPIObservation obs } return processObservationsForCache(List.of(updatedObservation), program.getKey()); }; - return programObservationCache.post(programId, postFunction).get(0); + return programCache.post(programId, postFunction).get(0); } catch (ApiException e) { log.error(Utilities.generateApiExceptionLogMessage(e)); throw new InternalServerException("Unknown error has occurred: " + e.getMessage(), e); @@ -343,7 +341,7 @@ public List updateBrAPIObservation(Map processedObservations = processObservationsForCache(updatedObservations, program.getKey()); - return programObservationCache.postThese(programId,processedObservations); + return programCache.postThese(programId,processedObservations); } catch (ApiException e) { log.error("Error updating observation: " + Utilities.generateApiExceptionLogMessage(e), e); throw e; diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java index aeb919a2f..9ef12b371 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIObservationUnitDAO.java @@ -41,7 +41,6 @@ import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.daos.ProgramDAO; -import org.breedinginsight.daos.cache.ProgramCache; import org.breedinginsight.daos.cache.ProgramCacheProvider; import org.breedinginsight.model.Program; import org.breedinginsight.services.ProgramService; @@ -61,7 +60,7 @@ @Slf4j @Singleton -public class BrAPIObservationUnitDAO { +public class BrAPIObservationUnitDAO extends BrAPICachedDAO { private final ProgramDAO programDAO; private final ImportDAO importDAO; private final BrAPIDAOUtil brAPIDAOUtil; @@ -75,8 +74,6 @@ public class BrAPIObservationUnitDAO { private final Gson gson = new JSON().getGson(); private final Type treatmentlistType = new TypeToken>(){}.getType(); - private final ProgramCache programObservationUnitCache; - @Inject public BrAPIObservationUnitDAO(ProgramDAO programDAO, ImportDAO importDAO, @@ -95,7 +92,7 @@ public BrAPIObservationUnitDAO(ProgramDAO programDAO, this.runScheduledTasks = runScheduledTasks; this.programService = programService; this.germplasmService = germplasmService; - this.programObservationUnitCache = programCacheProvider.getProgramCache(this::fetchProgramObservationUnits, BrAPIObservationUnit.class); + this.programCache = programCacheProvider.getProgramCache(this::fetchProgramObservationUnits, BrAPIObservationUnit.class); } @Scheduled(initialDelay = "3s") @@ -107,7 +104,7 @@ public void setup() { log.debug("populating observation unit cache"); List programs = programDAO.getActive(); if(programs != null) { - programObservationUnitCache.populate(programs.stream().map(Program::getId).collect(Collectors.toList())); + programCache.populate(programs.stream().map(Program::getId).collect(Collectors.toList())); } } @@ -157,7 +154,7 @@ private Map processObservationUnitsForCache(List getProgramObservationUnits(UUID programId) throws ApiException { - return programObservationUnitCache.get(programId); + return programCache.get(programId); } public List getObservationUnitByName(List observationUnitNames, Program program) throws ApiException { @@ -183,7 +180,7 @@ public List createBrAPIObservationUnits(List ous = brAPIDAOUtil.post(brAPIObservationUnitList, upload, api::observationunitsPost, importDAO::update); return processObservationUnitsForCache(ous, program, false); }; - return programObservationUnitCache.post(programId, postFunction); + return programCache.post(programId, postFunction); } return new ArrayList<>(); } catch (Exception e) { @@ -205,7 +202,7 @@ public List createBrAPIObservationUnits(List ous = brAPIDAOUtil.post(brAPIObservationUnitList, api::observationunitsPost); return processObservationUnitsForCache(ous, program, false); }; - return programObservationUnitCache.post(programId, postFunction); + return programCache.post(programId, postFunction); } return new ArrayList<>(); } catch (Exception 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 fdd404ae3..160e8df62 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java @@ -30,7 +30,6 @@ import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.daos.ProgramDAO; -import org.breedinginsight.daos.cache.ProgramCache; import org.breedinginsight.daos.cache.ProgramCacheProvider; import org.breedinginsight.model.Program; import org.breedinginsight.services.brapi.BrAPIEndpointProvider; @@ -46,7 +45,7 @@ @Slf4j @Singleton -public class BrAPIStudyDAO { +public class BrAPIStudyDAO extends BrAPICachedDAO { @Property(name = "brapi.server.reference-source") private String referenceSource; @Property(name = "micronaut.bi.api.run-scheduled-tasks") @@ -56,8 +55,6 @@ public class BrAPIStudyDAO { private ImportDAO importDAO; private final BrAPIDAOUtil brAPIDAOUtil; private final BrAPIEndpointProvider brAPIEndpointProvider; - private final ProgramCache programStudyCache; - @Inject public BrAPIStudyDAO(ProgramDAO programDAO, ImportDAO importDAO, BrAPIDAOUtil brAPIDAOUtil, BrAPIEndpointProvider brAPIEndpointProvider, ProgramCacheProvider programCacheProvider) { @@ -65,7 +62,7 @@ public BrAPIStudyDAO(ProgramDAO programDAO, ImportDAO importDAO, BrAPIDAOUtil br this.importDAO = importDAO; this.brAPIDAOUtil = brAPIDAOUtil; this.brAPIEndpointProvider = brAPIEndpointProvider; - this.programStudyCache = programCacheProvider.getProgramCache(this::fetchProgramStudy, BrAPIStudy.class); + this.programCache = programCacheProvider.getProgramCache(this::fetchProgramStudy, BrAPIStudy.class); } @Scheduled(initialDelay = "2s") @@ -77,7 +74,7 @@ public void setup() { log.debug("populating study cache"); List programs = programDAO.getActive(); if(programs != null) { - programStudyCache.populate(programs.stream().map(Program::getId).collect(Collectors.toList())); + programCache.populate(programs.stream().map(Program::getId).collect(Collectors.toList())); } } @@ -115,7 +112,7 @@ private Map fetchProgramStudy(UUID programId) throws ApiExce * @throws ApiException */ public List getStudies(UUID programId) throws ApiException { - return new ArrayList<>(programStudyCache.get(programId).values()); + return new ArrayList<>(programCache.get(programId).values()); } public Optional getStudyByName(String studyName, Program program) throws ApiException { @@ -153,7 +150,7 @@ public List getStudiesByExperimentID(@NotNull UUID experimentId, Pro } public List getStudiesByEnvironmentIds(@NotNull Collection environmentIds, Program program) throws ApiException { - return programStudyCache.get(program.getId()) + return programCache.get(program.getId()) .entrySet() .stream() .filter(entry -> environmentIds.contains(UUID.fromString(entry.getKey()))) @@ -193,7 +190,7 @@ public List createBrAPIStudies(List brAPIStudyList, UUID .post(brAPIStudyList, upload, api::studiesPost, importDAO::update); return environmentById(postedStudies); }; - createdStudies.addAll(programStudyCache.post(programId, postCallback)); + createdStudies.addAll(programCache.post(programId, postCallback)); } return createdStudies; diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java index 49315cb82..85298c158 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPITrialDAO.java @@ -47,4 +47,6 @@ List createBrAPITrials(List brAPITrialList, UUID program List getTrialsByExperimentIds(Collection experimentIds, Program program) throws ApiException; void deleteBrAPITrial(Program program, BrAPITrial trial, boolean hard) throws ApiException; + + void repopulateCache(UUID programId); } diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/impl/BrAPITrialDAOImpl.java b/src/main/java/org/breedinginsight/brapi/v2/dao/impl/BrAPITrialDAOImpl.java index 191e48f5a..d703f97e4 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 @@ -282,6 +282,7 @@ public List getTrialsByExperimentIds(Collection experimentIds, @Override public void deleteBrAPITrial(Program program, BrAPITrial trial, boolean hard) throws ApiException { + // TODO: Switch to using the TrialsApi from the BrAPI client library once the delete endpoints are merged into it. var programBrAPIBaseUrl = brAPIDAOUtil.getProgramBrAPIBaseUrl(program.getId()); var requestUrl = HttpUrl.parse(programBrAPIBaseUrl + "/trials/" + trial.getTrialDbId()).newBuilder(); requestUrl.addQueryParameter("hardDelete", Boolean.toString(hard)); @@ -293,4 +294,10 @@ public void deleteBrAPITrial(Program program, BrAPITrial trial, boolean hard) th brAPIDAOUtil.makeCall(brapiRequest); } + + @Override + public void repopulateCache(UUID programId) { + this.programExperimentCache.invalidate(programId); + this.programExperimentCache.populate(programId); + } } diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java index a63d4632c..395471c41 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -688,6 +688,12 @@ public int deleteExperiment(Program program, UUID experimentId, boolean hard) th for (BrAPIListSummary list : lists) { listDAO.deleteBrAPIList(list.getListDbId(), program.getId(), hard); // TODO: not yet implemented. } + // TODO: if performance is poor, implement more precise invalidation, possibly using hierarchical cache keys. + // Invalidate and repopulate cache for Trial, Study, Observation, ObservationUnit. + trialDAO.repopulateCache(program.getId()); + studyDAO.repopulateCache(program.getId()); + observationDAO.repopulateCache(program.getId()); + observationUnitDAO.repopulateCache(program.getId()); } else { // Trying to hard delete a trial with existing observations, return 409 Conflict response. // TODO: remove if unused. From e5c6fea0d17dcdaa4843f07eb0ab5eec04cae610 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Mon, 16 Dec 2024 15:31:27 -0500 Subject: [PATCH 12/16] [BI-2376] - used 204 No Content --- .../api/v1/controller/ExperimentController.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 059b0428e..01a39ea92 100644 --- a/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java +++ b/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java @@ -325,7 +325,8 @@ public HttpResponse deleteExperiment( // 409 Conflict. https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409 return HttpResponse.status(HttpStatus.CONFLICT); } - return HttpResponse.ok(); + // 204 No Content indicates successful delete. https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/204 + return HttpResponse.noContent(); } catch (Exception e) { log.error("Error deleting experiment.\n\tprogramId: " + programId + "\n\texperimentId: " + experimentId + "\n\thard: " + hard); throw e; From 270b85813ae2b2bf35e90bf167469e2f316522e2 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:19:35 -0500 Subject: [PATCH 13/16] [BI-2376] - improved exception handling --- .../api/v1/controller/ExperimentController.java | 10 ++++++---- .../brapi/v2/services/BrAPITrialService.java | 13 ++++++------- 2 files changed, 12 insertions(+), 11 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 01a39ea92..8619a140a 100644 --- a/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java +++ b/src/main/java/org/breedinginsight/api/v1/controller/ExperimentController.java @@ -319,7 +319,6 @@ public HttpResponse deleteExperiment( if(program.isEmpty()) { return HttpResponse.notFound(); } - // TODO: If hard and non-zero result, return 409 Conflict. int observationCount = experimentService.deleteExperiment(program.get(), experimentId, hard); if (observationCount > 0 && hard) { // 409 Conflict. https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409 @@ -327,11 +326,14 @@ public HttpResponse deleteExperiment( } // 204 No Content indicates successful delete. https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/204 return HttpResponse.noContent(); - } catch (Exception e) { + } catch (ApiException e) { log.error("Error deleting experiment.\n\tprogramId: " + programId + "\n\texperimentId: " + experimentId + "\n\thard: " + hard); - throw e; + if (e.getCode() == 404) { + return HttpResponse.notFound(); + } else { + return HttpResponse.serverError(); + } } - } 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 395471c41..9aec86320 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -667,11 +667,13 @@ public BrAPITrial getExperiment(Program program, UUID experimentId) throws ApiEx return experiments.get(0); } - // TODO: create a result type for delete requests? - // The caller could infer from the number of obs and hard whether delete succeeded. public int deleteExperiment(Program program, UUID experimentId, boolean hard) throws ApiException { - // TODO: check for observations! - BrAPITrial trial = trialDAO.getTrialsByExperimentIds(List.of(experimentId), program).get(0); + List trials = trialDAO.getTrialsByExperimentIds(List.of(experimentId), program); + if (trials.isEmpty()) { + throw new ApiException(404, "Experiment with UUID " + experimentId + " not found"); + } + BrAPITrial trial = trials.get(0); + List existingObservations = observationDAO.getObservationsByTrialDbId(List.of(trial.getTrialDbId()), program); // If there are no observations or a soft delete is requested, proceed. if (existingObservations.isEmpty() || !hard) { @@ -694,9 +696,6 @@ public int deleteExperiment(Program program, UUID experimentId, boolean hard) th studyDAO.repopulateCache(program.getId()); observationDAO.repopulateCache(program.getId()); observationUnitDAO.repopulateCache(program.getId()); - } else { - // Trying to hard delete a trial with existing observations, return 409 Conflict response. - // TODO: remove if unused. } // Successful or not, return the number of observations in this experiment. From 3a2a1d3aad2b8a23889c177c323299d10879c14b Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Tue, 17 Dec 2024 16:03:54 -0500 Subject: [PATCH 14/16] [BI-2376] - added integration tests --- .../ExperimentControllerIntegrationTest.java | 164 +++++++++++++++++- 1 file changed, 159 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java b/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java index e2961bb9f..a3c47e573 100644 --- a/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/api/v1/controller/ExperimentControllerIntegrationTest.java @@ -73,6 +73,8 @@ public class ExperimentControllerIntegrationTest extends BrAPITest { private List traits; private User testUser; private User otherTestUser; + private String mappingId; + private final String experimentTitle = "Test Exp"; @Property(name = "brapi.server.reference-source") private String BRAPI_REFERENCE_SOURCE; @@ -145,7 +147,7 @@ void setup() throws Exception { .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class ); HttpResponse response = call.blockingFirst(); - String mappingId = JsonParser.parseString(Objects.requireNonNull(response.body())).getAsJsonObject() + mappingId = JsonParser.parseString(Objects.requireNonNull(response.body())).getAsJsonObject() .getAsJsonObject("result") .getAsJsonArray("data") .get(0).getAsJsonObject().get("id").getAsString(); @@ -171,8 +173,8 @@ void setup() throws Exception { germplasmDAO.createBrAPIGermplasm(germplasm, program.getId(), null); // Make test experiment import - Map row1 = makeExpImportRow("Env1"); - Map row2 = makeExpImportRow("Env2"); + Map row1 = makeExpImportRow(experimentTitle, "Env1"); + Map row2 = makeExpImportRow(experimentTitle, "Env2"); // Add test observation data for (Trait trait : traits) { @@ -209,6 +211,37 @@ void setup() throws Exception { envIds.add(getEnvId(importResult, 1)); } + // Create an experiment with no observations. + private String uploadExperimentWithoutObs() throws Exception { + ImportTestUtils importTestUtils = new ImportTestUtils(); + List> expRows = new ArrayList<>(); + + // Make test experiment import. + Map row1 = makeExpImportRow("Without Obs", "NewEnv1"); + Map row2 = makeExpImportRow("Without Obs", "NewEnv2"); + + expRows.add(row1); + expRows.add(row2); + + // Import test experiment, environments. + JsonObject importResult = importTestUtils.uploadAndFetchWorkflow( + writeDataToFile(expRows, null), + null, + true, + client, + program, + mappingId, + newExperimentWorkflowId); + String expId = importResult + .get("preview").getAsJsonObject() + .get("rows").getAsJsonArray() + .get(0).getAsJsonObject() + .get("trial").getAsJsonObject() + .get("id").getAsString(); + + return expId; + } + /** * Tests all 18 permutations of * 3 formats: [CSV, XLS, XLSX], @@ -690,6 +723,92 @@ public void getExperimentalCollaboratorsDeactivated(boolean active) { dsl.execute(securityFp.get("DeleteProgramUser"), otherTestUser.getId().toString()); } + /** + * A delete request with an invalid trialDbId should result in a 404 Not Found response. + */ + @Test + @SneakyThrows + public void deleteExperimentInvalid() { + // A DELETE request endpoint with an invalid experimentId. + Flowable> invalidDeleteCall = client.exchange( + DELETE(String.format("/programs/%s/experiments/%s", program.getId().toString(), "00000000-1111-2222-3333-444444444444")) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class + ); + + // Ensure 404 NOT_FOUND response for requesting to delete a non-existant trial. + HttpClientResponseException e = Assertions.assertThrows(HttpClientResponseException.class, () -> { + HttpResponse response = invalidDeleteCall.blockingFirst(); + }); + assertEquals(HttpStatus.NOT_FOUND, e.getStatus()); + } + + /* Test the following 4 Cases: + * 1. hard delete with obs - failure + * 2. soft delete with obs - success + * 3. hard delete without obs - success + * 4. soft delete without obs - success + */ + @ParameterizedTest + @CsvSource(value = {"true,true", "false,true", "true,false", "false,false"}) + @SneakyThrows + public void deleteExperimentSuccess(boolean hardDelete, boolean withObservations) { + // Set up a test trial and get the trialDbId. + String trialDbId; + if (withObservations) { + JsonArray beforeData = getProgramTrials(program.getId().toString()); + + // The trial created by setup has observations. + trialDbId = beforeData.get(0).getAsJsonObject().get("trialDbId").getAsString(); + } else { + // Create a trial without observations. + trialDbId = uploadExperimentWithoutObs(); + } + + // A DELETE request should delete an experiment with observations unless there are observations and hardDelete = true. + Flowable> deleteCall = client.exchange( + DELETE(String.format("/programs/%s/experiments/%s?hard=%s", program.getId().toString(), trialDbId, hardDelete)) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class + ); + + // Ensure 204 NO_CONTENT response after successfully deleting a trial, + // unless there are observations and hard delete is requested, then ensure 409 Conflict response. + if (hardDelete && withObservations) { + // Ensure 404 NOT_FOUND response for requesting to delete a non-existant trial. + HttpClientResponseException e = Assertions.assertThrows(HttpClientResponseException.class, () -> { + HttpResponse response = deleteCall.blockingFirst(); + }); + assertEquals(HttpStatus.CONFLICT, e.getStatus()); + + // Check that the trial was not deleted. + JsonArray trials = getProgramTrials(program.getId().toString()); + assertEquals(1, trials.size()); + + // Check that the studies were not deleted. + JsonArray studies = getProgramStudies(program.getId().toString()); + assertEquals(2, studies.size()); + + // Check that lists were not deleted. + JsonArray lists = getProgramObsVarLists(program.getId().toString()); + assertEquals(1, lists.size()); + } else { + HttpResponse deleteResponse = deleteCall.blockingFirst(); + + assertEquals(HttpStatus.NO_CONTENT, deleteResponse.getStatus()); + + // Check that the trial was deleted. + JsonArray trials = getProgramTrials(program.getId().toString()); + assertEquals(0, trials.size()); + + // Check that the studies were deleted. + JsonArray studies = getProgramStudies(program.getId().toString()); + assertEquals(0, studies.size()); + + // Check that the BrAPI lists were deleted. + JsonArray lists = getProgramObsVarLists(program.getId().toString()); + assertEquals(0, lists.size()); + } + + } private List> buildSubEntityRows(List> topLevelRows, String entityName, int repeatedMeasures) { List> plantRows = new ArrayList<>(); @@ -726,11 +845,11 @@ private File writeDataToFile(List> data, List traits) return file; } - private Map makeExpImportRow(String environment) { + private Map makeExpImportRow(String title, String environment) { 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, title); row.put(ExperimentObservation.Columns.EXP_UNIT, "Plot"); //row.put(ExperimentObservation.Columns.SUB_OBS_UNIT, ""); row.put(ExperimentObservation.Columns.EXP_TYPE, "Phenotyping"); @@ -931,4 +1050,39 @@ private String getEnvId(JsonObject result, int index) { .get("referenceId").getAsString(); } + private JsonArray getProgramTrials(String programId) { + Flowable> getCall = client.exchange( + GET(String.format("/programs/%s/brapi/v2/trials", programId)) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class + ); + HttpResponse response = getCall.blockingFirst(); + + // Parse result. + JsonObject result = JsonParser.parseString(response.body()).getAsJsonObject().getAsJsonObject("result"); + return result.getAsJsonArray("data"); + } + + private JsonArray getProgramStudies(String programId) { + Flowable> getCall = client.exchange( + GET(String.format("/programs/%s/brapi/v2/studies", programId)) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class + ); + HttpResponse response = getCall.blockingFirst(); + + // Parse result. + JsonObject result = JsonParser.parseString(response.body()).getAsJsonObject().getAsJsonObject("result"); + return result.getAsJsonArray("data"); + } + + private JsonArray getProgramObsVarLists(String programId) { + Flowable> getCall = client.exchange( + GET(String.format("/programs/%s/brapi/v2/lists?listType=observationVariables", programId)) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), String.class + ); + HttpResponse response = getCall.blockingFirst(); + + // Parse result. + JsonObject result = JsonParser.parseString(response.body()).getAsJsonObject().getAsJsonObject("result"); + return result.getAsJsonArray("data"); + } } From 2a050531cc4cacc7f7471074312e024f9504192b Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Tue, 14 Jan 2025 12:26:48 -0500 Subject: [PATCH 15/16] [BI-2376] - cleaned up comment --- .../java/org/breedinginsight/brapi/v2/dao/BrAPICachedDAO.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPICachedDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPICachedDAO.java index fb35bd448..2d623b02b 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPICachedDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPICachedDAO.java @@ -9,7 +9,6 @@ public abstract class BrAPICachedDAO { protected ProgramCache programCache; public void repopulateCache(UUID programId) { - // TODO: test calling populate alone (without invalidate first). this.programCache.invalidate(programId); this.programCache.populate(programId); } From 6ee7e4f9e51fde2af88a3f708ce04db1080b7040 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Tue, 14 Jan 2025 13:07:58 -0500 Subject: [PATCH 16/16] [BI-2376] - changes based on review --- .../breedinginsight/brapi/v2/services/BrAPITrialService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9aec86320..6b8a3f233 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java @@ -688,7 +688,7 @@ public int deleteExperiment(Program program, UUID experimentId, boolean hard) th // TODO: replace with a single call to a batch delete method if that becomes available. // Iterate over lists, delete each by listDbId. for (BrAPIListSummary list : lists) { - listDAO.deleteBrAPIList(list.getListDbId(), program.getId(), hard); // TODO: not yet implemented. + listDAO.deleteBrAPIList(list.getListDbId(), program.getId(), hard); } // TODO: if performance is poor, implement more precise invalidation, possibly using hierarchical cache keys. // Invalidate and repopulate cache for Trial, Study, Observation, ObservationUnit.