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..8619a140a 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,43 @@ 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(); + } + 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); + } + // 204 No Content indicates successful delete. https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/204 + return HttpResponse.noContent(); + } catch (ApiException e) { + log.error("Error deleting experiment.\n\tprogramId: " + programId + "\n\texperimentId: " + experimentId + "\n\thard: " + hard); + if (e.getCode() == 404) { + return HttpResponse.notFound(); + } else { + return HttpResponse.serverError(); + } + } + } + + } 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..2d623b02b --- /dev/null +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPICachedDAO.java @@ -0,0 +1,15 @@ +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) { + this.programCache.invalidate(programId); + this.programCache.populate(programId); + } +} 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 e73b6836a..5ed73df60 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIListDAO.java @@ -18,11 +18,8 @@ package org.breedinginsight.brapi.v2.dao; import io.micronaut.http.HttpResponse; -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; @@ -38,27 +35,19 @@ 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.api.model.v1.response.DataResponse; -import org.breedinginsight.api.model.v1.response.Response; -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.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.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.UUID; -import java.util.concurrent.TimeUnit; @Slf4j public class BrAPIListDAO { @@ -79,7 +68,7 @@ public BrAPIListDAO(ProgramDAO programDAO, this.brAPIEndpointProvider = brAPIEndpointProvider; } - public List getListByName(List listNames, UUID programId) throws ApiException { + public List getListsByName(List listNames, UUID programId) throws ApiException { if(listNames.isEmpty()) { return Collections.emptyList(); } @@ -100,7 +89,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) { @@ -112,7 +101,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/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 83ee3f47b..85298c158 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,8 @@ List createBrAPITrials(List brAPITrialList, UUID program List getTrialsByDbIds(Collection trialDbIds, Program program) throws ApiException; 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 066c7017d..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 @@ -21,6 +21,8 @@ import io.micronaut.http.server.exceptions.InternalServerException; import io.micronaut.scheduling.annotation.Scheduled; import lombok.extern.slf4j.Slf4j; +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; @@ -277,4 +279,25 @@ public List getTrialsByExperimentIds(Collection experimentIds, trialSearch ), program.getKey()); } + + @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)); + HttpUrl url = requestUrl.build(); + var brapiRequest = new Request.Builder().url(url) + .method("DELETE", null) + .addHeader("Content-Type", "application/json") + .build(); + + 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/BrAPIListService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java index 65be8f5cd..246d2991d 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIListService.java @@ -57,7 +57,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 7c37f8f55..6b8a3f233 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()), @@ -667,6 +667,41 @@ public BrAPITrial getExperiment(Program program, UUID experimentId) throws ApiEx return experiments.get(0); } + public int deleteExperiment(Program program, UUID experimentId, boolean hard) throws ApiException { + 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) { + // 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: 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()); + } + + // Successful or not, return the number of observations in this experiment. + return existingObservations.size(); + } + private Map createExportRow( BrAPITrial experiment, Program program, 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/GermplasmProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/GermplasmProcessor.java index 25555b28e..0d514ee72 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/GermplasmProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/GermplasmProcessor.java @@ -178,7 +178,7 @@ public void getExistingBrapiData(List importRows, Program program) try { Germplasm row = importRows.get(0).getGermplasm(); String listName = Germplasm.constructGermplasmListName(row.getListName(), program); - List existingLists = brAPIListDAO.getListByName(List.of(listName), program.getId()); + List existingLists = brAPIListDAO.getListsByName(List.of(listName), program.getId()); for (BrAPIListSummary existingList: existingLists) { if (existingList.getListName().equals(listName)) { listNameDup = true; 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)); 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"); + } }