From 7ff118584f2f4ff2aa5c7c9aacaff62a0485e652 Mon Sep 17 00:00:00 2001 From: Teemu Hiltunen Date: Thu, 5 Dec 2024 14:45:55 +0200 Subject: [PATCH] =?UTF-8?q?HAI-752=20API=20for=20deleting=20t=C3=A4ydennys?= =?UTF-8?q?=20attachments=20(#892)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit API for deleting täydennys attachments --- .../TaydennysAttachmentControllerITest.kt | 122 ++++++++++++++---- .../TaydennysAttachmentServiceITest.kt | 48 +++++-- .../TaydennysAttachmentController.kt | 25 ++++ .../TaydennysAttachmentMetadataService.kt | 6 + .../taydennys/TaydennysAttachmentService.kt | 9 ++ 5 files changed, 174 insertions(+), 36 deletions(-) diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentControllerITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentControllerITest.kt index 23f33d7f9..2ae3f6ebd 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentControllerITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentControllerITest.kt @@ -4,8 +4,12 @@ import assertk.assertThat import assertk.assertions.isEqualTo import assertk.assertions.isNotNull import fi.hel.haitaton.hanke.ControllerTest -import fi.hel.haitaton.hanke.HankeError import fi.hel.haitaton.hanke.HankeError.HAI0001 +import fi.hel.haitaton.hanke.HankeError.HAI1001 +import fi.hel.haitaton.hanke.HankeError.HAI2001 +import fi.hel.haitaton.hanke.HankeError.HAI3002 +import fi.hel.haitaton.hanke.HankeError.HAI3004 +import fi.hel.haitaton.hanke.HankeError.HAI6001 import fi.hel.haitaton.hanke.HankeNotFoundException import fi.hel.haitaton.hanke.IntegrationTestConfiguration import fi.hel.haitaton.hanke.andReturnBody @@ -32,6 +36,7 @@ import io.mockk.checkUnnecessaryStub import io.mockk.clearAllMocks import io.mockk.confirmVerified import io.mockk.every +import io.mockk.justRun import io.mockk.verifyOrder import io.mockk.verifySequence import java.util.UUID @@ -66,6 +71,8 @@ class TaydennysAttachmentControllerITest(@Autowired override val mockMvc: MockMv @Autowired private lateinit var attachmentService: TaydennysAttachmentService @Autowired private lateinit var authorizer: TaydennysAuthorizer + private val attachmentId: UUID = UUID.fromString("df37fe12-fb36-4f61-8b07-2fb4ae8233f8") + @BeforeEach fun clearMocks() { clearAllMocks() @@ -80,11 +87,10 @@ class TaydennysAttachmentControllerITest(@Autowired override val mockMvc: MockMv @Nested inner class GetAttachmentContent { - private val attachmentId: UUID = UUID.fromString("df37fe12-fb36-4f61-8b07-2fb4ae8233f8") private val url = "/taydennykset/$DEFAULT_ID/liitteet/$attachmentId/content" @Test - fun `returns attachment file when request is valid`() { + fun `returns 200 and attachment file when request is valid`() { every { authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, VIEW.name) } returns true every { attachmentService.getContent(attachmentId) } returns @@ -105,7 +111,7 @@ class TaydennysAttachmentControllerITest(@Autowired override val mockMvc: MockMv @Test @WithAnonymousUser - fun `returns 401 and error when request is unauthorized`() { + fun `returns 401 and error when user is unauthorized`() { get(url).andExpectError(HAI0001) } @@ -114,7 +120,7 @@ class TaydennysAttachmentControllerITest(@Autowired override val mockMvc: MockMv every { authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, VIEW.name) } throws HakemusNotFoundException(APPLICATION_ID) - get(url).andExpect(status().isNotFound).andExpect(hankeError(HankeError.HAI2001)) + get(url).andExpect(status().isNotFound).andExpect(hankeError(HAI2001)) verifySequence { authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, VIEW.name) } } @@ -126,7 +132,7 @@ class TaydennysAttachmentControllerITest(@Autowired override val mockMvc: MockMv every { attachmentService.getContent(attachmentId) } throws ValtakirjaForbiddenException(attachmentId) - get(url).andExpect(status().isForbidden).andExpect(hankeError(HankeError.HAI3004)) + get(url).andExpect(status().isForbidden).andExpect(hankeError(HAI3004)) verifySequence { authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, VIEW.name) @@ -135,21 +141,21 @@ class TaydennysAttachmentControllerITest(@Autowired override val mockMvc: MockMv } @Test - fun `returns 404 and error when asking for non-existing attachment`() { + fun `returns 404 and error when attachment not found`() { every { authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, VIEW.name) } throws AttachmentNotFoundException(attachmentId) - get(url).andExpect(status().isNotFound).andExpect(hankeError(HankeError.HAI3002)) + get(url).andExpect(status().isNotFound).andExpect(hankeError(HAI3002)) verifySequence { authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, VIEW.name) } } @Test - fun `returns 404 and error when asking for non-existing taydennys`() { + fun `returns 404 and error when taydennys not found`() { every { authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, VIEW.name) } throws TaydennysNotFoundException(DEFAULT_ID) - get(url).andExpect(status().isNotFound).andExpect(hankeError(HankeError.HAI6001)) + get(url).andExpect(status().isNotFound).andExpect(hankeError(HAI6001)) verifySequence { authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, VIEW.name) } } @@ -187,9 +193,7 @@ class TaydennysAttachmentControllerITest(@Autowired override val mockMvc: MockMv every { authorizer.authorize(DEFAULT_ID, EDIT_APPLICATIONS.name) } throws HankeNotFoundException("HAI24-1") - postAttachment() - .andExpect(status().isNotFound) - .andExpect(hankeError(HankeError.HAI1001)) + postAttachment().andExpect(status().isNotFound).andExpect(hankeError(HAI1001)) verifyOrder { authorizer.authorize(DEFAULT_ID, EDIT_APPLICATIONS.name) } } @@ -199,9 +203,7 @@ class TaydennysAttachmentControllerITest(@Autowired override val mockMvc: MockMv every { authorizer.authorize(DEFAULT_ID, EDIT_APPLICATIONS.name) } throws HakemusNotFoundException(APPLICATION_ID) - postAttachment() - .andExpect(status().isNotFound) - .andExpect(hankeError(HankeError.HAI2001)) + postAttachment().andExpect(status().isNotFound).andExpect(hankeError(HAI2001)) verifyOrder { authorizer.authorize(DEFAULT_ID, EDIT_APPLICATIONS.name) } } @@ -211,18 +213,84 @@ class TaydennysAttachmentControllerITest(@Autowired override val mockMvc: MockMv fun `returns 401 and error when user is unauthorized`() { postAttachment().andExpectError(HAI0001) } + + private fun postAttachment( + taydennysId: UUID = DEFAULT_ID, + attachmentType: ApplicationAttachmentType = ApplicationAttachmentType.MUU, + file: MockMultipartFile = testFile(), + ): ResultActions { + return mockMvc.perform( + multipart("/taydennykset/$taydennysId/liitteet") + .file(file) + .param("tyyppi", attachmentType.toString()) + .with(csrf()) + ) + } } - private fun postAttachment( - taydennysId: UUID = DEFAULT_ID, - attachmentType: ApplicationAttachmentType = ApplicationAttachmentType.MUU, - file: MockMultipartFile = testFile(), - ): ResultActions { - return mockMvc.perform( - multipart("/taydennykset/$taydennysId/liitteet") - .file(file) - .param("tyyppi", attachmentType.toString()) - .with(csrf()) - ) + @Nested + inner class DeleteAttachment { + + private val url = "/taydennykset/$DEFAULT_ID/liitteet/$attachmentId" + + @Test + fun `returns 200 and empty body when deletion succeeds`() { + every { + authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, EDIT_APPLICATIONS.name) + } returns true + justRun { attachmentService.deleteAttachment(attachmentId) } + + delete(url).andExpect(status().isOk).andExpect(content().string("")) + + verifySequence { + authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, EDIT_APPLICATIONS.name) + attachmentService.deleteAttachment(attachmentId) + } + } + + @Test + @WithAnonymousUser + fun `returns 401 and error when user is unauthorized`() { + delete(url).andExpectError(HAI0001) + } + + @Test + fun `returns 404 and error when user has no rights for application`() { + every { + authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, EDIT_APPLICATIONS.name) + } throws HakemusNotFoundException(APPLICATION_ID) + + delete(url).andExpect(status().isNotFound).andExpect(hankeError(HAI2001)) + + verifySequence { + authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, EDIT_APPLICATIONS.name) + } + } + + @Test + fun `returns 404 and error when attachment not found`() { + every { + authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, EDIT_APPLICATIONS.name) + } throws AttachmentNotFoundException(attachmentId) + + delete(url).andExpect(status().isNotFound).andExpect(hankeError(HAI3002)) + + verifySequence { + authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, EDIT_APPLICATIONS.name) + } + } + + @Test + fun `returns 404 and error when taydennys not found`() { + every { + authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, EDIT_APPLICATIONS.name) + } throws TaydennysNotFoundException(DEFAULT_ID) + + delete(url).andExpect(status().isNotFound).andExpect(hankeError(HAI6001)) + + verifySequence { + authorizer.authorizeAttachment(DEFAULT_ID, attachmentId, EDIT_APPLICATIONS.name) + } + } } } diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentServiceITest.kt index 29102df0c..5f41b05bb 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentServiceITest.kt @@ -3,8 +3,10 @@ package fi.hel.haitaton.hanke.attachment.taydennys import assertk.all import assertk.assertFailure import assertk.assertThat +import assertk.assertions.containsExactly import assertk.assertions.hasClass import assertk.assertions.hasMessage +import assertk.assertions.hasSize import assertk.assertions.isEmpty import assertk.assertions.isEqualTo import assertk.assertions.isNotNull @@ -17,10 +19,9 @@ import fi.hel.haitaton.hanke.IntegrationTest import fi.hel.haitaton.hanke.attachment.DEFAULT_SIZE import fi.hel.haitaton.hanke.attachment.FILE_NAME_PDF import fi.hel.haitaton.hanke.attachment.PDF_BYTES -import fi.hel.haitaton.hanke.attachment.azure.Container +import fi.hel.haitaton.hanke.attachment.azure.Container.HAKEMUS_LIITTEET import fi.hel.haitaton.hanke.attachment.body import fi.hel.haitaton.hanke.attachment.common.ApplicationAttachmentType -import fi.hel.haitaton.hanke.attachment.common.ApplicationAttachmentType.VALTAKIRJA import fi.hel.haitaton.hanke.attachment.common.AttachmentInvalidException import fi.hel.haitaton.hanke.attachment.common.AttachmentLimitReachedException import fi.hel.haitaton.hanke.attachment.common.AttachmentNotFoundException @@ -49,7 +50,6 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.EnumSource import org.springframework.beans.factory.annotation.Autowired -import org.springframework.http.MediaType import org.springframework.http.MediaType.APPLICATION_PDF_VALUE class TaydennysAttachmentServiceITest( @@ -102,7 +102,11 @@ class TaydennysAttachmentServiceITest( @Test fun `Throws exception when trying to get valtakirja content`() { - val attachment = attachmentFactory.save(attachmentType = VALTAKIRJA).withContent().value + val attachment = + attachmentFactory + .save(attachmentType = ApplicationAttachmentType.VALTAKIRJA) + .withContent() + .value val failure = assertFailure { attachmentService.getContent(attachment.id!!) } @@ -133,7 +137,7 @@ class TaydennysAttachmentServiceITest( assertThat(result.id).isNotNull() assertThat(result.createdByUserId).isEqualTo(USERNAME) assertThat(result.fileName).isEqualTo(FILE_NAME_PDF) - assertThat(result.contentType).isEqualTo(MediaType.APPLICATION_PDF_VALUE) + assertThat(result.contentType).isEqualTo(APPLICATION_PDF_VALUE) assertThat(result.size).isEqualTo(DEFAULT_SIZE) assertThat(result.createdAt).isRecent() assertThat(result.taydennysId).isEqualTo(taydennys.id) @@ -144,8 +148,7 @@ class TaydennysAttachmentServiceITest( prop(TaydennysAttachmentEntity::id).isEqualTo(result.id) prop(TaydennysAttachmentEntity::createdByUserId).isEqualTo(USERNAME) prop(TaydennysAttachmentEntity::fileName).isEqualTo(FILE_NAME_PDF) - prop(TaydennysAttachmentEntity::contentType) - .isEqualTo(MediaType.APPLICATION_PDF_VALUE) + prop(TaydennysAttachmentEntity::contentType).isEqualTo(APPLICATION_PDF_VALUE) prop(TaydennysAttachmentEntity::size).isEqualTo(DEFAULT_SIZE) prop(TaydennysAttachmentEntity::createdAt).isRecent() prop(TaydennysAttachmentEntity::taydennysId).isEqualTo(taydennys.id) @@ -155,8 +158,7 @@ class TaydennysAttachmentServiceITest( .startsWith("${taydennys.hakemusId}/") } - val content = - fileClient.download(Container.HAKEMUS_LIITTEET, attachments.first().blobLocation) + val content = fileClient.download(HAKEMUS_LIITTEET, attachments.first().blobLocation) assertThat(content) .isNotNull() .prop(DownloadResponse::content) @@ -321,4 +323,32 @@ class TaydennysAttachmentServiceITest( assertThat(attachmentRepository.findAll()).isEmpty() } } + + @Nested + inner class DeleteAttachment { + @Test + fun `Throws exception when attachment not found`() { + val attachmentId = UUID.fromString("ab7993b7-a775-4eac-b5b7-8546332944fe") + + val failure = assertFailure { attachmentService.deleteAttachment(attachmentId) } + + failure.all { + hasClass(AttachmentNotFoundException::class) + messageContains(attachmentId.toString()) + } + } + + @Test + fun `Deletes attachment and content when attachment exists`() { + val attachment = attachmentFactory.save().withContent().value + assertThat(attachmentRepository.findAll()).hasSize(1) + assertThat(fileClient.listBlobs(HAKEMUS_LIITTEET).map { it.path }) + .containsExactly(attachment.blobLocation) + + attachmentService.deleteAttachment(attachment.id!!) + + assertThat(attachmentRepository.findAll()).isEmpty() + assertThat(fileClient.listBlobs(HAKEMUS_LIITTEET)).isEmpty() + } + } } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentController.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentController.kt index 9a2f0fe92..a245c6aa6 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentController.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentController.kt @@ -19,6 +19,7 @@ import org.springframework.http.HttpStatus import org.springframework.http.MediaType import org.springframework.http.ResponseEntity import org.springframework.security.access.prepost.PreAuthorize +import org.springframework.web.bind.annotation.DeleteMapping import org.springframework.web.bind.annotation.ExceptionHandler import org.springframework.web.bind.annotation.GetMapping import org.springframework.web.bind.annotation.PathVariable @@ -88,6 +89,30 @@ class TaydennysAttachmentController(private val attachmentService: TaydennysAtta return attachmentService.addAttachment(taydennysId, tyyppi, attachment) } + @DeleteMapping("/{attachmentId}") + @Operation( + summary = "Delete attachment from täydennys", + description = "Deletes attachment from täydennys.", + ) + @ApiResponses( + value = + [ + ApiResponse(description = "Delete attachment", responseCode = "200"), + ApiResponse( + description = "Attachment not found", + responseCode = "404", + content = [Content(schema = Schema(implementation = HankeError::class))], + ), + ] + ) + @PreAuthorize( + "@taydennysAuthorizer.authorizeAttachment(#taydennysId, #attachmentId, 'EDIT_APPLICATIONS')" + ) + fun deleteAttachment(@PathVariable taydennysId: UUID, @PathVariable attachmentId: UUID) { + logger.info { "Deleting attachment $attachmentId from täydennys $taydennysId." } + attachmentService.deleteAttachment(attachmentId) + } + @ExceptionHandler(ValtakirjaForbiddenException::class) @ResponseStatus(HttpStatus.FORBIDDEN) @Hidden diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentMetadataService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentMetadataService.kt index fef381a77..88713b941 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentMetadataService.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentMetadataService.kt @@ -80,6 +80,12 @@ class TaydennysAttachmentMetadataService( return totalAttachmentCount >= ALLOWED_ATTACHMENT_COUNT } + @Transactional + fun deleteAttachmentById(attachmentId: UUID) { + attachmentRepository.deleteById(attachmentId) + logger.info { "Deleted attachment metadata $attachmentId" } + } + /** * Delete all attachments for täydennys and return the blob locations of the deleted * attachments. diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentService.kt index 687a0a5db..415a58a08 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentService.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/attachment/taydennys/TaydennysAttachmentService.kt @@ -112,6 +112,15 @@ class TaydennysAttachmentService( return newAttachment } + fun deleteAttachment(attachmentId: UUID) { + val attachment = metadataService.findAttachment(attachmentId) + logger.info { "Deleting attachment metadata ${attachment.id}" } + metadataService.deleteAttachmentById(attachment.id) + logger.info { "Deleting attachment content at ${attachment.blobLocation}" } + attachmentContentService.delete(attachment.blobLocation) + logger.info { "Deleted attachment $attachmentId from täydennys ${attachment.taydennysId}" } + } + fun deleteAllAttachments(taydennys: TaydennysIdentifier) { logger.info { "Deleting all attachments from täydennys. ${taydennys.logString()}" } val paths = metadataService.deleteAllAttachments(taydennys)