Delete exports and files after expire time (#213)

* added cron jobs to delete old files and exports * moved cleanup retention days arg to application yml --------- Co-authored-by: Yuan Chen <[email protected]>
cph-cachet · Feb 13, 2025 · 14d4757 · 14d4757
1 parent 06a3067
commit 14d4757
Show file tree

Hide file tree

Showing 12 changed files with 285 additions and 1 deletion.
diff --git a/.env b/.env
@@ -39,3 +39,6 @@ RABBIT_PORT=5672
 
 PROMETHEUS_PORT=9090
 GRAFANA_PORT=3000
+
+FILES_RETENTION_DAYS=
+EXPORTS_RETENTION_DAYS=
diff --git a/src/main/kotlin/dk/cachet/carp/webservices/export/repository/ExportRepository.kt b/src/main/kotlin/dk/cachet/carp/webservices/export/repository/ExportRepository.kt
@@ -33,4 +33,6 @@ interface ExportRepository : JpaRepository<Export, String> {
 
     @Query(nativeQuery = true, value = "SELECT * FROM exports WHERE created_at < :timestamp")
     fun findAllCreatedBefore(timestamp: Instant): List<Export>
+
+    fun getAllByUpdatedAtIsBefore(updatedAtBefore: Instant): MutableList<Export>
 }
diff --git a/src/main/kotlin/dk/cachet/carp/webservices/export/scheduler/ExportCleanup.kt b/src/main/kotlin/dk/cachet/carp/webservices/export/scheduler/ExportCleanup.kt
@@ -0,0 +1,24 @@
+package dk.cachet.carp.webservices.export.scheduler
+
+import dk.cachet.carp.webservices.export.service.ExportService
+import org.apache.logging.log4j.LogManager
+import org.apache.logging.log4j.Logger
+import org.springframework.beans.factory.annotation.Value
+import org.springframework.scheduling.annotation.Scheduled
+import org.springframework.stereotype.Component
+
+@Component
+class ExportCleanup(
+    private val exportService: ExportService,
+    @Value("\${cleanup.exports.retention-days}") private val days: Int,
+) {
+    companion object {
+        private val LOGGER: Logger = LogManager.getLogger()
+    }
+
+    @Scheduled(cron = "0 5 9 * * ?")
+    fun cleanup() {
+        LOGGER.info("Cleaning up exports...")
+        exportService.deleteAllOlderThan(days)
+    }
+}
diff --git a/src/main/kotlin/dk/cachet/carp/webservices/export/service/ExportService.kt b/src/main/kotlin/dk/cachet/carp/webservices/export/service/ExportService.kt
@@ -19,4 +19,6 @@ interface ExportService {
         studyId: UUID,
         exportId: UUID,
     ): UUID
+
+    fun deleteAllOlderThan(days: Int)
 }
diff --git a/src/main/kotlin/dk/cachet/carp/webservices/export/service/impl/ExportServiceImpl.kt b/src/main/kotlin/dk/cachet/carp/webservices/export/service/impl/ExportServiceImpl.kt
@@ -13,7 +13,9 @@ import org.apache.logging.log4j.LogManager
 import org.apache.logging.log4j.Logger
 import org.springframework.core.io.Resource
 import org.springframework.stereotype.Service
+import java.io.IOException
 import java.nio.file.Path
+import java.time.Instant
 
 @Service
 class ExportServiceImpl(
@@ -67,6 +69,23 @@ class ExportServiceImpl(
         return studyId
     }
 
+    @Suppress("MagicNumber")
+    override fun deleteAllOlderThan(days: Int) {
+        val clockNow7DaysAgo = System.currentTimeMillis() - days * 24 * 60 * 60 * 1000
+        val exportsToDelete = exportRepository.getAllByUpdatedAtIsBefore(Instant.ofEpochMilli(clockNow7DaysAgo))
+
+        exportsToDelete.forEach { export: Export ->
+            exportRepository.delete(export)
+            try {
+                fileStorage.deleteFileAtPath(export.fileName, Path.of(export.relativePath))
+            } catch (e: IOException) {
+                LOGGER.error("Failed to delete export with id ${export.id}.", e)
+            }
+        }
+
+        LOGGER.info("Exports older than $days days have been successfully deleted.")
+    }
+
     /**
      * Get the export with the given [id] if it exists, or delete it if it failed to export and return null.
      */

diff --git a/src/main/kotlin/dk/cachet/carp/webservices/file/repository/FileRepository.kt b/src/main/kotlin/dk/cachet/carp/webservices/file/repository/FileRepository.kt
@@ -7,6 +7,7 @@ import org.springframework.data.jpa.repository.JpaRepository
 import org.springframework.data.jpa.repository.JpaSpecificationExecutor
 import org.springframework.stereotype.Repository
 import org.springframework.web.multipart.MultipartFile
+import java.time.Instant
 
 @Repository
 interface FileRepository : JpaRepository<File, Int>, JpaSpecificationExecutor<File>, FileRepositoryCustom {
@@ -16,6 +17,8 @@ interface FileRepository : JpaRepository<File, Int>, JpaSpecificationExecutor<Fi
     ): List<File>
 
     fun findByStudyId(studyId: String): List<File>
+
+    fun getAllByUpdatedAtIsBefore(updatedAtBefore: Instant): MutableList<File>
 }
 
 // TODO: This is not a repository, dont't be mislead by that, it needs to be moved to its own service somewhere else.

diff --git a/src/main/kotlin/dk/cachet/carp/webservices/file/scheduler/FileCleanup.kt b/src/main/kotlin/dk/cachet/carp/webservices/file/scheduler/FileCleanup.kt
@@ -0,0 +1,24 @@
+package dk.cachet.carp.webservices.file.scheduler
+
+import dk.cachet.carp.webservices.file.service.FileService
+import org.apache.logging.log4j.LogManager
+import org.apache.logging.log4j.Logger
+import org.springframework.beans.factory.annotation.Value
+import org.springframework.scheduling.annotation.Scheduled
+import org.springframework.stereotype.Component
+
+@Component
+class FileCleanup(
+    private val fileService: FileService,
+    @Value("\${cleanup.files.retention-days}") private val days: Int,
+) {
+    companion object {
+        private val LOGGER: Logger = LogManager.getLogger()
+    }
+
+    @Scheduled(cron = "0 0 9 * * ?")
+    fun cleanup() {
+        LOGGER.info("Cleaning up files...")
+        fileService.deleteAllOlderThan(days)
+    }
+}
diff --git a/src/main/kotlin/dk/cachet/carp/webservices/file/service/FileService.kt b/src/main/kotlin/dk/cachet/carp/webservices/file/service/FileService.kt
@@ -17,6 +17,8 @@ interface FileService {
         studyId: UUID,
     )
 
+    fun deleteAllOlderThan(days: Int)
+
     suspend fun deleteAllByStudyId(studyId: String)
 
     /** The [createDEPRECATED] interface for creating a file. */

diff --git a/src/main/kotlin/dk/cachet/carp/webservices/file/service/impl/FileServiceImpl.kt b/src/main/kotlin/dk/cachet/carp/webservices/file/service/impl/FileServiceImpl.kt
@@ -30,13 +30,15 @@ import org.springframework.transaction.annotation.Transactional
 import org.springframework.util.StringUtils
 import org.springframework.web.multipart.MultipartFile
 import org.springframework.web.util.UriComponentsBuilder
+import java.io.IOException
 import java.nio.file.Files
 import java.nio.file.Path
+import java.time.Instant
 
 @Service
 @Transactional
 // Extract S3 methods into a separate service
-@Suppress("LongParameterList")
+@Suppress("LongParameterList", "TooManyFunctions")
 class FileServiceImpl(
     private val fileRepository: FileRepository,
     private val fileStorage: FileStorage,
@@ -166,6 +168,23 @@ class FileServiceImpl(
         LOGGER.info("File deleted, id = $id")
     }
 
+    @Suppress("MagicNumber")
+    override fun deleteAllOlderThan(days: Int) {
+        val clockNow7DaysAgo = System.currentTimeMillis() - days * 24 * 60 * 60 * 1000
+        val filesToDelete = fileRepository.getAllByUpdatedAtIsBefore(Instant.ofEpochMilli(clockNow7DaysAgo))
+
+        filesToDelete.forEach { file ->
+            fileRepository.delete(file)
+            try {
+                fileStorage.deleteFileAtPath(file.fileName, Path.of(file.relativePath))
+            } catch (e: IOException) {
+                LOGGER.error("Failed to delete file with id = ${file.id}", e)
+            }
+        }
+
+        LOGGER.info("All files older than $days days deleted")
+    }
+
     override suspend fun deleteAllByStudyId(studyId: String) {
         val files =
             withContext(Dispatchers.IO) {

diff --git a/src/main/resources/config/application-local.yml b/src/main/resources/config/application-local.yml
@@ -257,6 +257,12 @@ storage:
       enabled: true
       days: 90
 
+cleanup:
+  files:
+    retention-days: "${FILES_RETENTION_DAYS:7}"
+  exports:
+    retention-days: "${EXPORTS_RETENTION_DAYS:7}"
+
 environment:
   url: "${CAWS_URL}"
   portalUrl: "${CAWS_PORTAL_URL}"

diff --git a/src/test/kotlin/dk/cachet/carp/webservices/export/service/ExportServiceTest.kt b/src/test/kotlin/dk/cachet/carp/webservices/export/service/ExportServiceTest.kt
@@ -13,6 +13,8 @@ import io.mockk.every
 import io.mockk.mockk
 import io.mockk.verify
 import org.junit.jupiter.api.Nested
+import org.junit.jupiter.api.assertThrows
+import java.io.IOException
 import java.nio.file.Path
 import kotlin.test.Test
 import kotlin.test.assertFailsWith
@@ -137,4 +139,77 @@ class ExportServiceTest {
             verify { fileStorage.deleteFileAtPath(entry.fileName, any<Path>()) }
         }
     }
+
+    @Nested
+    inner class DeleteAllOlderThan {
+        @Test
+        fun `should delete all exports older than specified days`() {
+            val days = 7
+            val clockNow7DaysAgo = System.currentTimeMillis() - days * 24 * 60 * 60 * 1000
+            val exportsToDelete =
+                mutableListOf(
+                    Export(
+                        id = "1",
+                        fileName = "file1",
+                        relativePath = "path1",
+                    ),
+                    Export(
+                        id = "2",
+                        fileName = "file2",
+                        relativePath = "path2",
+                    ),
+                )
+
+            every { repository.getAllByUpdatedAtIsBefore(any()) } returns exportsToDelete
+            every { repository.delete(any()) } answers { nothing }
+            every { fileStorage.deleteFileAtPath(any(), any()) } returns true
+
+            val sut = ExportServiceImpl(repository, invoker, fileStorage)
+            sut.deleteAllOlderThan(days)
+
+            verify(exactly = exportsToDelete.size) { repository.delete(any()) }
+            verify(exactly = exportsToDelete.size) { fileStorage.deleteFileAtPath(any(), any()) }
+
+            verify {
+                repository.getAllByUpdatedAtIsBefore(
+                    match {
+                        val tolerance = 5000
+                        Math.abs(it.toEpochMilli() - clockNow7DaysAgo) <= tolerance
+                    },
+                )
+            }
+        }
+
+        @Test
+        fun `should continue deletion if something throws`() {
+            val days = 7
+            val exportsToDelete =
+                mutableListOf(
+                    Export(
+                        id = "1",
+                        fileName = "file1",
+                        relativePath = "path1",
+                    ),
+                    Export(
+                        id = "2",
+                        fileName = "file2",
+                        relativePath = "path2",
+                    ),
+                )
+
+            every { repository.getAllByUpdatedAtIsBefore(any()) } returns exportsToDelete
+            every { repository.delete(any()) } answers { nothing }
+            every { fileStorage.deleteFileAtPath("file1", Path.of("path1")) } returns true
+            every { fileStorage.deleteFileAtPath("file2", Path.of("path2")) } throws IOException(";(")
+
+            val sut = ExportServiceImpl(repository, invoker, fileStorage)
+            sut.deleteAllOlderThan(days)
+
+            verify(exactly = exportsToDelete.size) { repository.delete(any()) }
+            verify(exactly = exportsToDelete.size) { fileStorage.deleteFileAtPath(any(), any()) }
+            assertThrows<IOException> {
+                fileStorage.deleteFileAtPath("file2", Path.of("path2"))
+            }
+        }
+    }
 }
diff --git a/src/test/kotlin/dk/cachet/carp/webservices/files/service/FileServiceTest.kt b/src/test/kotlin/dk/cachet/carp/webservices/files/service/FileServiceTest.kt
@@ -13,10 +13,13 @@ import dk.cachet.carp.webservices.security.authorization.Claim
 import dk.cachet.carp.webservices.security.authorization.service.AuthorizationService
 import io.mockk.*
 import kotlinx.coroutines.test.runTest
+import kotlinx.datetime.Instant
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
 import org.springframework.mock.web.MockMultipartFile
 import org.springframework.util.StringUtils
+import java.io.IOException
 import java.util.*
 import kotlin.test.assertEquals
 import kotlin.test.assertNotEquals
@@ -192,4 +195,106 @@ class FileServiceTest {
             assertEquals(expectedKey, actualKey)
         }
     }
+
+    @Nested
+    inner class DeleteAllOlderThan {
+        @Test
+        fun `deletes older than 7 days`() {
+            val fileMock1 =
+                mockk<File> {
+                    every { id } returns 1
+                    every { fileName } returns "file1"
+                    every { relativePath } returns "foo/bar/baz"
+                }
+            val fileMock2 =
+                mockk<File> {
+                    every { id } returns 2
+                    every { fileName } returns "file2"
+                    every { relativePath } returns "foo/bar/qux"
+                }
+            every { fileRepository.getAllByUpdatedAtIsBefore(any()) } returns mutableListOf(fileMock1, fileMock2)
+            every { fileRepository.delete(any<File>()) } just runs
+            every { fileStorage.deleteFileAtPath(any(), any()) } returns true
+            val sut =
+                FileServiceImpl(
+                    fileRepository,
+                    fileStorage,
+                    messageBase,
+                    s3Client,
+                    authenticationService,
+                    s3SpaceBucket,
+                    s3SpaceEndpoint,
+                )
+            val sevenDaysAgo = System.currentTimeMillis() - 7 * 24 * 60 * 60 * 1000
+
+            sut.deleteAllOlderThan(7)
+
+            verify { fileRepository.getAllByUpdatedAtIsBefore(java.time.Instant.ofEpochMilli(sevenDaysAgo)) }
+            verify { fileRepository.delete(fileMock1) }
+            verify { fileRepository.delete(fileMock2) }
+            verify { fileStorage.deleteFileAtPath("file1", java.nio.file.Path.of("foo", "bar", "baz")) }
+            verify { fileStorage.deleteFileAtPath("file2", java.nio.file.Path.of("foo", "bar", "qux")) }
+        }
+
+        @Test
+        fun `should keep deleting even if one of the files could not be deleted from storage`() {
+            val fileMock1 =
+                mockk<File> {
+                    every { id } returns 1
+                    every { fileName } returns "file1"
+                    every { relativePath } returns "foo/bar/baz"
+                }
+            val fileMock2 =
+                mockk<File> {
+                    every { id } returns 2
+                    every { fileName } returns "file2"
+                    every { relativePath } returns "foo/bar/qux"
+                }
+            every { fileRepository.getAllByUpdatedAtIsBefore(any()) } returns mutableListOf(fileMock1, fileMock2)
+            every { fileRepository.delete(any<File>()) } just runs
+            every {
+                fileStorage.deleteFileAtPath(
+                    "file1",
+                    java.nio.file.Path.of("foo", "bar", "baz"),
+                )
+            } returns true
+            every {
+                fileStorage.deleteFileAtPath(
+                    "file2",
+                    java.nio.file.Path.of("foo", "bar", "qux"),
+                )
+            } throws IOException(":(")
+            val sut =
+                FileServiceImpl(
+                    fileRepository,
+                    fileStorage,
+                    messageBase,
+                    s3Client,
+                    authenticationService,
+                    s3SpaceBucket,
+                    s3SpaceEndpoint,
+                )
+            val sevenDaysAgo = System.currentTimeMillis() - 7 * 24 * 60 * 60 * 1000
+
+            sut.deleteAllOlderThan(7)
+
+            verify { fileRepository.getAllByUpdatedAtIsBefore(any()) }
+            verify { fileRepository.delete(fileMock1) }
+            verify { fileRepository.delete(fileMock2) }
+            verify { fileStorage.deleteFileAtPath("file1", java.nio.file.Path.of("foo", "bar", "baz")) }
+            verify { fileStorage.deleteFileAtPath("file2", java.nio.file.Path.of("foo", "bar", "qux")) }
+            assertThrows<IOException> {
+                fileStorage.deleteFileAtPath("file2", java.nio.file.Path.of("foo", "bar", "qux"))
+            }
+
+            verify {
+                fileRepository.getAllByUpdatedAtIsBefore(
+                    match {
+                        val tolerance = 5000
+                        Math.abs(it.toEpochMilli() - sevenDaysAgo) <= tolerance
+                    },
+                )
+            }
+        }
+    }
 }