From f14afe751b7ef2537bf76fb01f65ca3e3eb2b7fa Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Sat, 9 Nov 2024 11:36:17 +0300 Subject: [PATCH 1/3] Supporting invalid path in BackupStorageUtils ### What's done: - backup storage utils expects ID in s3 path after a common prefix, it leads to number exception --- .../com/saveourtool/common/storage/BackupStorageUtils.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt b/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt index eadc90f3c9..6890c976f5 100644 --- a/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt +++ b/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt @@ -96,8 +96,10 @@ fun S3Operations.deleteUnexpectedKeys( ) private fun AbstractS3KeyDatabaseManager<*, *, *>.asS3KeyValidator(): (String) -> Boolean = { s3Key -> - val id = s3Key.removePrefix(commonPrefix).toLong() - findKeyByEntityId(id) == null + val s3KeyWithoutPrefix = s3Key.removePrefix(commonPrefix) + s3KeyWithoutPrefix.toLongOrNull().let { id -> + findKeyByEntityId(id) == null + } == true } private fun S3Operations.doBackupUnexpectedKeys( From 6e5093925f65d3f7ce6c0b414fa4c89cfa4563a3 Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Sat, 9 Nov 2024 11:39:34 +0300 Subject: [PATCH 2/3] self review --- .../com/saveourtool/common/storage/BackupStorageUtils.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt b/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt index 6890c976f5..14eb242554 100644 --- a/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt +++ b/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt @@ -96,10 +96,11 @@ fun S3Operations.deleteUnexpectedKeys( ) private fun AbstractS3KeyDatabaseManager<*, *, *>.asS3KeyValidator(): (String) -> Boolean = { s3Key -> - val s3KeyWithoutPrefix = s3Key.removePrefix(commonPrefix) - s3KeyWithoutPrefix.toLongOrNull().let { id -> - findKeyByEntityId(id) == null - } == true + s3Key.removePrefix(commonPrefix) + .toLongOrNull() + ?.let { id -> + findKeyByEntityId(id) == null + } == true } private fun S3Operations.doBackupUnexpectedKeys( From a639469117247f0b941011f35f8cc024cd3b7cc8 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Mon, 11 Nov 2024 14:07:27 +0300 Subject: [PATCH 3/3] Add KDoc --- .../common/storage/BackupStorageUtils.kt | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt b/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt index 14eb242554..d05f214f92 100644 --- a/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt +++ b/common/src/jvmMain/kotlin/com/saveourtool/common/storage/BackupStorageUtils.kt @@ -26,7 +26,7 @@ private val log = getLogger {} * * @param storageName * @param commonPrefix - * @param s3KeyValidator + * @param s3KeyValidator accepts S3 keys and returns `true` for **unexpected** ones. * @return [CompletableFuture] without body */ fun S3Operations.backupUnexpectedKeys( @@ -63,7 +63,7 @@ fun S3Operations.backupUnexpectedKeys( * * @param storageName * @param commonPrefix - * @param s3KeyValidator + * @param s3KeyValidator accepts S3 keys and returns `true` for **unexpected** ones. * @return [CompletableFuture] without body */ fun S3Operations.deleteUnexpectedKeys( @@ -95,7 +95,22 @@ fun S3Operations.deleteUnexpectedKeys( s3KeyValidator = s3KeyManager.asS3KeyValidator(), ) +/** + * @return the lambda, which accepts an _S3 key_ (in the form of `path/to/data/`) + * and returns `true` if the key is _invalid_. + */ private fun AbstractS3KeyDatabaseManager<*, *, *>.asS3KeyValidator(): (String) -> Boolean = { s3Key -> + /*- + * S3 "folders", similarly to "files", also have keys. + * + * In our case, such a folder name may be the same as the prefix (e.g.: + * `path/to/data/`), that's why we use `toLongOrNull()` rather than + * `toLong()` here. + * + * The key to a folder which has the same name as the prefix (basically, + * the containing folder) is considered to be a *valid* key (the validator + * returning `false`). + */ s3Key.removePrefix(commonPrefix) .toLongOrNull() ?.let { id -> @@ -149,6 +164,9 @@ private fun S3Operations.doDeleteUnexpectedKeys( } } +/** + * @param s3KeyValidator accepts S3 keys and returns `true` for **unexpected** ones. + */ private fun S3Operations.detectUnexpectedKeys( commonPrefix: String, s3KeyValidator: (String) -> Boolean,