🐛 fixed storage broken dsm cleaner was removing files when uploading to directories (#4732)

Author: GitHK

services/storage/src/simcore_service_storage/simcore_s3_dsm.py

@@ -839,6 +839,7 @@ async def _clean_dangling_multipart_uploads(self):
         )
         if not current_multipart_uploads:
             return
+        _logger.debug("found %s", f"{current_multipart_uploads=}")
 
         # there are some multipart uploads, checking if
         # there is a counterpart in file_meta_data
@@ -861,6 +862,8 @@ async def _clean_dangling_multipart_uploads(self):
             ] = await db_file_meta_data.list_fmds(
                 conn, file_ids=list(set(directory_and_file_ids))
             )
+            _logger.debug("metadata entries %s", f"{list_of_known_metadata_entries=}")
+
         # known uploads do have an expiry date (regardless of upload ID that we do not always know)
         list_of_known_uploads = [
             fmd for fmd in list_of_known_metadata_entries if fmd.upload_expires_at
@@ -885,6 +888,7 @@ async def _clean_dangling_multipart_uploads(self):
             ):
                 list_of_valid_upload_ids.append(upload_id)
 
+        _logger.debug("found the following %s", f"{list_of_valid_upload_ids=}")
         list_of_invalid_uploads = [
             (
                 upload_id,

services/storage/src/simcore_service_storage/simcore_s3_dsm_utils.py

@@ -1,4 +1,5 @@
 from contextlib import suppress
+from pathlib import Path
 
 from aiohttp import web
 from aiopg.sa.connection import SAConnection
@@ -66,7 +67,7 @@ def get_simcore_directory(file_id: SimcoreS3FileID) -> str:
         directory_id = SimcoreS3DirectoryID.from_simcore_s3_object(file_id)
     except ValueError:
         return ""
-    return f"{directory_id}"
+    return f"{Path(directory_id)}"
 
 
 async def get_directory_file_id(
@@ -96,7 +97,7 @@ async def _get_fmd(
         # could not extract a directory name from the provided path
         return None
 
-    directory_file_id = parse_obj_as(SimcoreS3FileID, directory_file_id_str.rstrip("/"))
+    directory_file_id = parse_obj_as(SimcoreS3FileID, directory_file_id_str)
     directory_file_id_fmd = await _get_fmd(conn, directory_file_id)
 
     return directory_file_id if directory_file_id_fmd else None

services/storage/tests/unit/test_dsm_dsmcleaner.py

@@ -43,7 +43,9 @@ def disabled_dsm_cleaner_task(monkeypatch: pytest.MonkeyPatch):
 
 @pytest.fixture
 def simcore_directory_id(simcore_file_id: SimcoreS3FileID) -> SimcoreS3FileID:
-    return SimcoreS3FileID(SimcoreS3DirectoryID.from_simcore_s3_object(simcore_file_id))
+    return SimcoreS3FileID(
+        Path(SimcoreS3DirectoryID.from_simcore_s3_object(simcore_file_id))
+    )
 
 
 async def test_clean_expired_uploads_aborts_dangling_multipart_uploads(
@@ -83,8 +85,15 @@ async def test_clean_expired_uploads_aborts_dangling_multipart_uploads(
     [ByteSize(0), parse_obj_as(ByteSize, "10Mib"), parse_obj_as(ByteSize, "100Mib")],
     ids=byte_size_ids,
 )
-@pytest.mark.parametrize("link_type", [LinkType.S3, LinkType.PRESIGNED])
-@pytest.mark.parametrize("is_directory", [True, False])
+@pytest.mark.parametrize(
+    "link_type, is_directory",
+    [
+        # NOTE: directories are handled only as LinkType.S3
+        (LinkType.S3, True),
+        (LinkType.S3, False),
+        (LinkType.PRESIGNED, False),
+    ],
+)
 async def test_clean_expired_uploads_deletes_expired_pending_uploads(
     disabled_dsm_cleaner_task,
     aiopg_engine: Engine,
@@ -281,7 +290,10 @@ async def test_clean_expired_uploads_does_not_clean_multipart_upload_on_creation
     FILES_IN_DIR: Final[int] = 5
 
     file_ids_to_upload: set[SimcoreS3FileID] = (
-        {SimcoreS3FileID(f"{file_or_directory_id}file{x}") for x in range(FILES_IN_DIR)}
+        {
+            SimcoreS3FileID(f"{file_or_directory_id}/file{x}")
+            for x in range(FILES_IN_DIR)
+        }
         if is_directory
         else {simcore_file_id}
     )