diff --git a/src/main/java/org/broadinstitute/consent/http/db/StudyDAO.java b/src/main/java/org/broadinstitute/consent/http/db/StudyDAO.java index 2944f7ba3b..6e679a7dcc 100644 --- a/src/main/java/org/broadinstitute/consent/http/db/StudyDAO.java +++ b/src/main/java/org/broadinstitute/consent/http/db/StudyDAO.java @@ -133,6 +133,12 @@ DELETE FROM study WHERE study_id in (select study_id from property_deletes) """) void deleteStudyByStudyId(@Bind("studyId") Integer studyId); + @SqlUpdate( + """ + DELETE FROM study_property where study_id = :studyId + """) + void deleteStudyPropertiesByStudyId(@Bind("studyId") Integer studyId); + @UseRowReducer(StudyReducer.class) @SqlQuery( """ diff --git a/src/main/java/org/broadinstitute/consent/http/service/dao/DatasetServiceDAO.java b/src/main/java/org/broadinstitute/consent/http/service/dao/DatasetServiceDAO.java index a98141c205..487a82aced 100644 --- a/src/main/java/org/broadinstitute/consent/http/service/dao/DatasetServiceDAO.java +++ b/src/main/java/org/broadinstitute/consent/http/service/dao/DatasetServiceDAO.java @@ -247,7 +247,7 @@ public Study updateStudy( jdbi.useHandle( handle -> { handle.getConnection().setAutoCommit(false); - executeUpdateStudy(handle, studyUpdate); + executeUpdateStudyReplaceProps(handle, studyUpdate); for (DatasetUpdate datasetUpdate : datasetUpdates) { executeUpdateDatasetWithFiles( handle, @@ -275,7 +275,15 @@ public Study updateStudy( return studyDAO.findStudyById(studyUpdate.studyId); } - private void executeUpdateStudy(Handle handle, StudyUpdate update) { + private void executeUpdateStudyReplaceProps(Handle handle, StudyUpdate update) { + executeUpdateStudy(handle, update, true); + } + + private void executeUpdateStudyKeepProps(Handle handle, StudyUpdate update) { + executeUpdateStudy(handle, update, false); + } + + private void executeUpdateStudy(Handle handle, StudyUpdate update, boolean replaceProps) { StudyDAO studyDAO = handle.attach(StudyDAO.class); Study study = studyDAO.findStudyById(update.studyId); studyDAO.updateStudy( @@ -288,26 +296,34 @@ private void executeUpdateStudy(Handle handle, StudyUpdate update) { update.userId, Instant.now()); - // Handle property inserts and updates - Set existingStudyProperties = - studyDAO.findStudyById(update.studyId).getProperties(); - update.props.forEach( - p -> { - Optional existingProp = - existingStudyProperties.stream() - .filter(ep -> p.getKey().equals(ep.getKey()) && p.getType().equals(ep.getType())) - .findFirst(); - if (existingProp.isPresent()) { - // Update existing study prop: - studyDAO.updateStudyProperty( - update.studyId, p.getKey(), p.getType().toString(), p.getValue().toString()); - } else { - // Add new study prop: - studyDAO.insertStudyProperty( - update.studyId, p.getKey(), p.getType().toString(), p.getValue().toString()); - } - }); - + if (replaceProps) { + studyDAO.deleteStudyPropertiesByStudyId(update.studyId); + update.props.forEach( + p -> + studyDAO.insertStudyProperty( + update.studyId, p.getKey(), p.getType().toString(), p.getValue().toString())); + } else { + // Handle property inserts and updates + Set existingStudyProperties = + studyDAO.findStudyById(update.studyId).getProperties(); + update.props.forEach( + p -> { + Optional existingProp = + existingStudyProperties.stream() + .filter( + ep -> p.getKey().equals(ep.getKey()) && p.getType().equals(ep.getType())) + .findFirst(); + if (existingProp.isPresent()) { + // Update existing study prop: + studyDAO.updateStudyProperty( + update.studyId, p.getKey(), p.getType().toString(), p.getValue().toString()); + } else { + // Add new study prop: + studyDAO.insertStudyProperty( + update.studyId, p.getKey(), p.getType().toString(), p.getValue().toString()); + } + }); + } executeInsertFiles(handle, update.files, update.userId, study.getUuid().toString()); } @@ -423,7 +439,7 @@ public Study patchStudy(Study study, User user, StudyPatch patch) throws SQLExce // Convert the patch to a StudyUpdate for reuse of existing methods StudyUpdate studyUpdate = convertToStudyUpdate(study, user, patch); try { - executeUpdateStudy(handle, studyUpdate); + executeUpdateStudyKeepProps(handle, studyUpdate); } catch (Exception e) { handle.rollback(); logException(e); diff --git a/src/test/java/org/broadinstitute/consent/http/db/StudyDAOTest.java b/src/test/java/org/broadinstitute/consent/http/db/StudyDAOTest.java index 6602959589..a5c432e6f9 100644 --- a/src/test/java/org/broadinstitute/consent/http/db/StudyDAOTest.java +++ b/src/test/java/org/broadinstitute/consent/http/db/StudyDAOTest.java @@ -343,6 +343,19 @@ void testDeleteStudyById() { assertNull(deletedStudy); } + @Test + void testDeleteStudyPropertiesById() { + Study study = insertStudyWithProperties(); + Integer id = study.getStudyId(); + int propertyCount = study.getProperties().size(); + assertTrue(propertyCount > 0); + + studyDAO.deleteStudyPropertiesByStudyId(id); + + Study foundStudy = studyDAO.findStudyById(id); + assertEquals(0, foundStudy.getProperties().size()); + } + @Test void testFindStudyByName() { Study study = insertStudyWithProperties(); diff --git a/src/test/java/org/broadinstitute/consent/http/service/dao/DatasetServiceDAOTest.java b/src/test/java/org/broadinstitute/consent/http/service/dao/DatasetServiceDAOTest.java index 88db2f8d34..6600f1c495 100644 --- a/src/test/java/org/broadinstitute/consent/http/service/dao/DatasetServiceDAOTest.java +++ b/src/test/java/org/broadinstitute/consent/http/service/dao/DatasetServiceDAOTest.java @@ -586,7 +586,7 @@ void testUpdateStudyWithPropUpdates() throws Exception { // Updated prop Optional updatedProp1 = updatedStudy.getProperties().stream() - .filter(p -> p.getStudyPropertyId().equals(prop1.getStudyPropertyId())) + .filter(p -> p.getKey().equals(prop1.getKey())) .findFirst(); assertTrue(updatedProp1.isPresent()); assertEquals(newPropValue, updatedProp1.get().getValue());