From 8b89125621abb271b143279a82e11d09548cdecc Mon Sep 17 00:00:00 2001 From: Gertjan van Oosten Date: Wed, 9 Aug 2023 14:36:10 +0200 Subject: [PATCH 1/3] Add extra assertions Also fix the query response from createAttachment. --- .../dds/anet/test/resources/AttachmentResourceTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/java/mil/dds/anet/test/resources/AttachmentResourceTest.java b/src/test/java/mil/dds/anet/test/resources/AttachmentResourceTest.java index 292a55ac76..05927cbf71 100644 --- a/src/test/java/mil/dds/anet/test/resources/AttachmentResourceTest.java +++ b/src/test/java/mil/dds/anet/test/resources/AttachmentResourceTest.java @@ -83,6 +83,7 @@ public void testCreateAttachment() final Report updatedReport = adminQueryExecutor.report(REPORT_FIELDS, testReport.getUuid()); assertThat(updatedReport.getAttachments()).hasSize(1); final Attachment reportAttachment = updatedReport.getAttachments().get(0); + assertThat(reportAttachment.getUuid()).isEqualTo(createdAttachmentUuid); assertThat(reportAttachment.getAttachmentRelatedObjects()).hasSize(1); assertThat(reportAttachment.getDescription()).isEqualTo(testAttachmentInput.getDescription()); assertThat(reportAttachment.getClassification()) @@ -122,13 +123,14 @@ public void testDeleteAttachment() final Report updatedReport = adminQueryExecutor.report(REPORT_FIELDS, testReport.getUuid()); assertThat(updatedReport.getAttachments()).hasSize(1); final Attachment reportAttachment = updatedReport.getAttachments().get(0); + assertThat(reportAttachment.getUuid()).isEqualTo(createdAttachmentUuid); assertThat(reportAttachment.getAttachmentRelatedObjects()).hasSize(1); assertThat(reportAttachment.getDescription()).isEqualTo(testAttachmentInput.getDescription()); assertThat(reportAttachment.getClassification()) .isEqualTo(testAttachmentInput.getClassification()); assertThat(reportAttachment.getFileName()).isEqualTo(testAttachmentInput.getFileName()); - // F - delete attachment classification as someone else + // F - delete attachment as someone else final MutationExecutor erinMutationExecutor = getMutationExecutor(getRegularUser().getDomainUsername()); failAttachmentDelete(erinMutationExecutor, reportAttachment.getUuid()); @@ -221,8 +223,7 @@ private AttachmentRelatedObjectInput createAttachmentRelatedObject(final String private String succeedAttachmentCreate(final MutationExecutor mutationExecutor, final AttachmentInput attachmentInput) throws GraphQLRequestPreparationException, GraphQLRequestExecutionException { - final String createdAttachmentUuid = - mutationExecutor.createAttachment(attachmentInput.getUuid(), attachmentInput); + final String createdAttachmentUuid = mutationExecutor.createAttachment("", attachmentInput); assertThat(createdAttachmentUuid).isNotNull(); return createdAttachmentUuid; } @@ -230,7 +231,7 @@ private String succeedAttachmentCreate(final MutationExecutor mutationExecutor, private void failAttachmentCreate(final MutationExecutor mutationExecutor, final AttachmentInput attachmentInput) { try { - mutationExecutor.createAttachment(attachmentInput.getUuid(), attachmentInput); + mutationExecutor.createAttachment("", attachmentInput); fail("Expected exception creating attachment"); } catch (Exception expected) { // OK From 3b18bcef727e38ab7d940ebf1c860893abe95488 Mon Sep 17 00:00:00 2001 From: Gertjan van Oosten Date: Wed, 9 Aug 2023 14:39:25 +0200 Subject: [PATCH 2/3] AB#912 Add test for attachment deletion upon report delete Check that report attachments are deleted when a report is deleted. --- .../test/resources/ReportResourceTest.java | 42 +++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/src/test/java/mil/dds/anet/test/resources/ReportResourceTest.java b/src/test/java/mil/dds/anet/test/resources/ReportResourceTest.java index f2ee44758c..96dbb30d2f 100644 --- a/src/test/java/mil/dds/anet/test/resources/ReportResourceTest.java +++ b/src/test/java/mil/dds/anet/test/resources/ReportResourceTest.java @@ -15,6 +15,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -23,6 +24,8 @@ import javax.ws.rs.ForbiddenException; import javax.ws.rs.NotFoundException; import mil.dds.anet.AnetObjectEngine; +import mil.dds.anet.database.ReportDao; +import mil.dds.anet.resources.AttachmentResource; import mil.dds.anet.test.TestData; import mil.dds.anet.test.client.AdvisorReportsEntry; import mil.dds.anet.test.client.AnetBeanList_Location; @@ -34,6 +37,9 @@ import mil.dds.anet.test.client.ApprovalStepInput; import mil.dds.anet.test.client.ApprovalStepType; import mil.dds.anet.test.client.Atmosphere; +import mil.dds.anet.test.client.Attachment; +import mil.dds.anet.test.client.AttachmentInput; +import mil.dds.anet.test.client.AttachmentRelatedObjectInput; import mil.dds.anet.test.client.Comment; import mil.dds.anet.test.client.Location; import mil.dds.anet.test.client.LocationSearchQueryInput; @@ -108,9 +114,10 @@ public class ReportResourceTest extends AbstractResourceTest { + " reportPeople %3$s tasks %4$s approvalStep { uuid relatedObjectUuid } location %5$s" + " comments %6$s notes %7$s authorizationGroups { uuid name }" + " workflow { step { uuid relatedObjectUuid approvers { uuid person { uuid } } }" - + " person { uuid } type createdAt } reportSensitiveInformation { uuid text } }", + + " person { uuid } type createdAt } reportSensitiveInformation { uuid text } " + + " attachments %8$s }", REPORT_FIELDS, ORGANIZATION_FIELDS, REPORT_PEOPLE_FIELDS, TASK_FIELDS, LOCATION_FIELDS, - COMMENT_FIELDS, NoteResourceTest.NOTE_FIELDS); + COMMENT_FIELDS, NoteResourceTest.NOTE_FIELDS, AttachmentResourceTest.ATTACHMENT_FIELDS); @Test public void createReport() @@ -1412,8 +1419,9 @@ public void searchAttendeePosition() } @Test - public void reportDeleteTest() + void reportDeleteTest() throws GraphQLRequestExecutionException, GraphQLRequestPreparationException { + final Map attachmentSettings = AttachmentResource.getAttachmentSettings(); final Person elizabeth = getElizabethElizawell(); final MutationExecutor elizabethMutationExecutor = getMutationExecutor(elizabeth.getDomainUsername()); @@ -1434,6 +1442,26 @@ public void reportDeleteTest() assertThat(r).isNotNull(); assertThat(r.getUuid()).isNotNull(); + // Attach attachment to test report + final var allowedMimeTypes = (List) attachmentSettings.get("mimeTypes"); + final String mimeType = allowedMimeTypes.get(0); + + final AttachmentRelatedObjectInput testAroInput = AttachmentRelatedObjectInput.builder() + .withRelatedObjectType(ReportDao.TABLE_NAME).withRelatedObjectUuid(r.getUuid()).build(); + final AttachmentInput testAttachmentInput = + AttachmentInput.builder().withFileName("testDeleteAttachment.jpg").withMimeType(mimeType) + .withDescription("a test attachment created by testDeleteAttachment") + .withAttachmentRelatedObjects(Collections.singletonList(testAroInput)).build(); + final String createdAttachmentUuid = + adminMutationExecutor.createAttachment("", testAttachmentInput); + assertThat(createdAttachmentUuid).isNotNull(); + + final Report updatedReport = adminQueryExecutor.report(FIELDS, r.getUuid()); + assertThat(updatedReport.getAttachments()).hasSize(1); + final Attachment reportAttachment = updatedReport.getAttachments().get(0); + assertThat(reportAttachment.getUuid()).isEqualTo(createdAttachmentUuid); + assertThat(reportAttachment.getAttachmentRelatedObjects()).hasSize(1); + // Try to delete by jack, this should fail. try { jackMutationExecutor.deleteReport("", r.getUuid()); @@ -1451,6 +1479,14 @@ public void reportDeleteTest() fail("Expected NotFoundException"); } catch (NotFoundException expectedException) { } + + // Assert that the attachment is gone + try { + adminQueryExecutor.attachment(AttachmentResourceTest.ATTACHMENT_FIELDS, + reportAttachment.getUuid()); + fail("Expected NotFoundException"); + } catch (NotFoundException expectedException) { + } } @Test From 156fb4780f3c8a70c98873e4fb4488013fd6867a Mon Sep 17 00:00:00 2001 From: Gertjan van Oosten Date: Wed, 9 Aug 2023 14:41:42 +0200 Subject: [PATCH 3/3] AB#912 Delete report attachments when report is deleted --- .../mil/dds/anet/database/AttachmentDao.java | 46 +++++++++++++++++-- .../java/mil/dds/anet/database/ReportDao.java | 4 ++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/main/java/mil/dds/anet/database/AttachmentDao.java b/src/main/java/mil/dds/anet/database/AttachmentDao.java index 5d0d9fe0e8..20399abb3f 100644 --- a/src/main/java/mil/dds/anet/database/AttachmentDao.java +++ b/src/main/java/mil/dds/anet/database/AttachmentDao.java @@ -1,5 +1,7 @@ package mil.dds.anet.database; +import static org.jdbi.v3.core.statement.EmptyHandling.NULL_KEYWORD; + import io.leangen.graphql.annotations.GraphQLRootContext; import java.io.IOException; import java.io.InputStream; @@ -8,6 +10,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; import mil.dds.anet.AnetObjectEngine; @@ -21,6 +24,7 @@ import mil.dds.anet.views.ForeignKeyFetcher; import org.apache.commons.io.IOUtils; import org.eclipse.jetty.io.EofException; +import org.jdbi.v3.core.result.ResultIterable; import org.jdbi.v3.sqlobject.customizer.Bind; import org.jdbi.v3.sqlobject.customizer.BindBean; import org.jdbi.v3.sqlobject.statement.SqlBatch; @@ -197,9 +201,43 @@ private void insertAttachmentRelatedObjects(String uuid, ab.insertAttachmentRelatedObjects(uuid, attachmentRelatedObjects); } - private void deleteAttachmentRelatedObjects(String uuid) { - getDbHandle().execute( - "/* deleteAttachmentRelatedObjects */ DELETE FROM \"attachmentRelatedObjects\" WHERE \"attachmentUuid\" = ?", - uuid); + private void deleteAttachmentRelatedObjects(String attachmentUuid) { + getDbHandle() + .createUpdate( + "/* deleteAttachmentRelatedObjects */ DELETE FROM \"attachmentRelatedObjects\"" + + " WHERE \"attachmentUuid\" = :attachmentUuid") + .bind("attachmentUuid", attachmentUuid).execute(); + } + + public void deleteAttachments(String relatedObjectType, String relatedObjectUuid) { + final String relatedObjectTypeParam = "relatedObjectType"; + final String relatedObjectUuidParam = "relatedObjectUuid"; + final String fromClause = "FROM \"attachmentRelatedObjects\" WHERE \"" + relatedObjectTypeParam + + "\" = :relatedObjectType AND \"" + relatedObjectUuidParam + "\" = :relatedObjectUuid"; + final String selectAttachmentUuids = "SELECT \"attachmentUuid\" " + fromClause; + + // get uuid's of attachments linked to related object + final Set attachmentUuids = getDbHandle() + .createQuery("/* selectAttachmentUuidsForRelatedObject */ " + selectAttachmentUuids) + .bind(relatedObjectTypeParam, relatedObjectType) + .bind(relatedObjectUuidParam, relatedObjectUuid).mapTo(String.class) + .collect(Collectors.toSet()); + if (attachmentUuids.isEmpty()) { + // nothing to delete + return; + } + + // delete attachmentRelatedObjects for the related object + getDbHandle().createUpdate("/* deleteAttachmentRelatedObjects */ DELETE " + fromClause) + .bind(relatedObjectTypeParam, relatedObjectType) + .bind(relatedObjectUuidParam, relatedObjectUuid).execute(); + + // delete attachments for the related object if they no longer have any links + getDbHandle() + .createUpdate("/* deleteAttachments */ DELETE FROM attachments" + + " WHERE uuid in () AND uuid NOT IN (" + selectAttachmentUuids + ")") + .bindList(NULL_KEYWORD, "attachmentUuids", attachmentUuids) + .bind(relatedObjectTypeParam, relatedObjectType) + .bind(relatedObjectUuidParam, relatedObjectUuid).execute(); } } diff --git a/src/main/java/mil/dds/anet/database/ReportDao.java b/src/main/java/mil/dds/anet/database/ReportDao.java index 20ad44535d..b7f2b250d7 100644 --- a/src/main/java/mil/dds/anet/database/ReportDao.java +++ b/src/main/java/mil/dds/anet/database/ReportDao.java @@ -404,6 +404,10 @@ public int deleteInternal(String reportUuid) { // Delete orphan notes noteDao.deleteOrphanNotes(); + final AttachmentDao attachmentDao = instance.getAttachmentDao(); + // Delete attachments + attachmentDao.deleteAttachments(TABLE_NAME, reportUuid); + // Delete report // GraphQL mutations *have* to return something, so we return the number of deleted report rows return getDbHandle()