enhance impl

Author: giancarloromeo

services/storage/src/simcore_service_storage/db_file_meta_data.py

@@ -72,7 +72,7 @@ def _list_filter_with_partial_file_id_stmt(
     file_id_prefix: str | None,
     partial_file_id: str | None,
     sha256_checksum: SHA256Str | None,
-    only_files: bool,
+    is_directory: bool | None = None,
     limit: int | None = None,
     offset: int | None = None,
 ):
@@ -98,8 +98,8 @@ def _list_filter_with_partial_file_id_stmt(
         conditions.append(file_meta_data.c.file_id.startswith(file_id_prefix))
     if partial_file_id:
         conditions.append(file_meta_data.c.file_id.ilike(f"%{partial_file_id}%"))
-    if only_files:
-        conditions.append(file_meta_data.c.is_directory.is_(False))
+    if is_directory is not None:
+        conditions.append(file_meta_data.c.is_directory.is_(is_directory))
     if sha256_checksum:
         conditions.append(file_meta_data.c.sha256_checksum == sha256_checksum)
 
@@ -119,7 +119,7 @@ async def list_filter_with_partial_file_id(
     file_id_prefix: str | None,
     partial_file_id: str | None,
     sha256_checksum: SHA256Str | None,
-    only_files: bool,
+    is_directory: bool | None = None,
     limit: int | None = None,
     offset: int | None = None,
 ) -> list[FileMetaDataAtDB]:
@@ -129,7 +129,7 @@ async def list_filter_with_partial_file_id(
         file_id_prefix=file_id_prefix,
         partial_file_id=partial_file_id,
         sha256_checksum=sha256_checksum,
-        only_files=only_files,
+        is_directory=is_directory,
         limit=limit,
         offset=offset,
     )

services/storage/src/simcore_service_storage/simcore_s3_dsm.py

@@ -80,7 +80,7 @@
 from .s3 import get_s3_client
 from .s3_utils import S3TransferDataCB, update_task_progress
 from .settings import Settings
-from .simcore_s3_dsm_utils import expand_directory, get_directory_file_id, try_get_fmd
+from .simcore_s3_dsm_utils import expand_directory, get_directory_file_id
 from .utils import (
     convert_db_to_model,
     download_to_file_or_raise,
@@ -182,7 +182,6 @@ async def list_files(  # noqa C901
                 ),
                 file_id_prefix=None,
                 partial_file_id=uuid_filter,
-                only_files=False,
                 sha256_checksum=None,
             )
 
@@ -518,6 +517,11 @@ async def delete_file(
         # Only use this in those circumstances where a collaborator requires to delete a file (the current
         # permissions model will not allow him to do so, even though this is a legitimate action)
         # SEE https://github.com/ITISFoundation/osparc-simcore/issues/5159
+        def _get_subpath(path: str, levels: int):
+            components = path.strip("/").split("/")
+            subpath = "/".join(components[:levels])
+            return "/" + subpath if subpath else "/"
+
         async with self.engine.acquire() as conn:
             if enforce_access_rights:
                 can: AccessRights = await get_file_access_rights(conn, user_id, file_id)
@@ -530,10 +534,23 @@ async def delete_file(
         )
 
         async with self.engine.acquire() as conn:
-            if await try_get_fmd(conn, file_id):
-                await db_file_meta_data.delete(conn, [file_id])
-            if parent_dir_file_id := await get_directory_file_id(conn, file_id):
-                parent_dir_fmd = await db_file_meta_data.get(conn, parent_dir_file_id)
+            try:
+                if await db_file_meta_data.get(conn, file_id):
+                    await db_file_meta_data.delete(conn, [file_id])
+            except FileMetaDataNotFoundError:
+                _logger.warning("File %s not found in database", file_id)
+
+            if parent_dir_fmds := await db_file_meta_data.list_filter_with_partial_file_id(
+                conn,
+                user_or_project_filter=UserOrProjectFilter(
+                    user_id=user_id, project_ids=[]
+                ),
+                file_id_prefix=_get_subpath(file_id, 2),
+                partial_file_id=None,
+                is_directory=True,
+                sha256_checksum=None,
+            ):
+                parent_dir_fmd = parent_dir_fmds[0]
                 parent_dir_fmd.file_size = UNDEFINED_SIZE
                 await db_file_meta_data.upsert(conn, parent_dir_fmd)
 

services/storage/src/simcore_service_storage/simcore_s3_dsm_utils.py

@@ -78,16 +78,6 @@ async def expand_directory(
     return result
 
 
-async def try_get_fmd(
-    conn: SAConnection, s3_file_id: StorageFileID
-) -> FileMetaDataAtDB | None:
-    with suppress(FileMetaDataNotFoundError):
-        return await db_file_meta_data.get(
-            conn, TypeAdapter(SimcoreS3FileID).validate_python(s3_file_id)
-        )
-    return None
-
-
 def get_simcore_directory(file_id: SimcoreS3FileID) -> str:
     try:
         directory_id = SimcoreS3DirectoryID.from_simcore_s3_object(file_id)
@@ -104,7 +94,16 @@ async def get_directory_file_id(
     in the `file_meta_data` table
     """
 
-    provided_file_id_fmd = await try_get_fmd(conn, file_id)
+    async def _get_fmd(
+        conn: SAConnection, s3_file_id: StorageFileID
+    ) -> FileMetaDataAtDB | None:
+        with suppress(FileMetaDataNotFoundError):
+            return await db_file_meta_data.get(
+                conn, TypeAdapter(SimcoreS3FileID).validate_python(s3_file_id)
+            )
+        return None
+
+    provided_file_id_fmd = await _get_fmd(conn, file_id)
     if provided_file_id_fmd:
         # file_meta_data exists it is not a directory
         return None
@@ -117,6 +116,6 @@ async def get_directory_file_id(
     directory_file_id = TypeAdapter(SimcoreS3FileID).validate_python(
         directory_file_id_str
     )
-    directory_file_id_fmd = await try_get_fmd(conn, directory_file_id)
+    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_db_file_meta_data.py

@@ -31,7 +31,7 @@ def _check(func_smt, **kwargs):
         file_id_prefix=None,
         partial_file_id=None,
         sha256_checksum=None,
-        only_files=True,
+        is_directory=False,
     )
     # WHERE file_meta_data.is_directory IS false ORDER BY file_meta_data.created_at ASC
 
@@ -41,7 +41,7 @@ def _check(func_smt, **kwargs):
         file_id_prefix=None,
         partial_file_id=None,
         sha256_checksum=None,
-        only_files=True,
+        is_directory=False,
     )
     # WHERE file_meta_data.user_id = '42' AND file_meta_data.is_directory IS false ORDER BY file_meta_data.created_at ASC
 
@@ -53,7 +53,7 @@ def _check(func_smt, **kwargs):
         file_id_prefix=None,
         partial_file_id=None,
         sha256_checksum=None,
-        only_files=True,
+        is_directory=False,
     )
     # WHERE (file_meta_data.user_id = '42' OR file_meta_data.project_id IN ('18d5'..., )) AND file_meta_data.is_directory IS false ORDER BY file_meta_data.created_at ASC
 
@@ -65,7 +65,7 @@ def _check(func_smt, **kwargs):
         file_id_prefix=None,
         partial_file_id=None,
         sha256_checksum=None,
-        only_files=True,
+        is_directory=False,
         limit=10,
         offset=1,
     )
@@ -78,7 +78,6 @@ def _check(func_smt, **kwargs):
         file_id_prefix=None,
         partial_file_id="{project_id}/",
         sha256_checksum=None,
-        only_files=False,
     )
 
     # As used in SimcoreS3DataManager.search_owned_files
@@ -88,7 +87,7 @@ def _check(func_smt, **kwargs):
         file_id_prefix="api/",
         partial_file_id=None,
         sha256_checksum=faker.sha256(),
-        only_files=True,
+        is_directory=False,
         limit=10,
         offset=0,
     )

services/storage/tests/unit/test_handlers_files.py

@@ -1345,6 +1345,7 @@ async def test_upload_file_is_directory_and_remove_content(
     client: TestClient,
     location_id: LocationID,
     user_id: UserID,
+    faker: Faker,
 ):
     FILE_SIZE_IN_DIR = TypeAdapter(ByteSize).validate_python("1Mib")
     DIR_NAME = "some-dir"
@@ -1407,6 +1408,26 @@ async def test_upload_file_is_directory_and_remove_content(
 
     assert len(list_of_files) == SUBDIR_COUNT * FILE_COUNT - 1
 
+    # DELETE NOT EXISTING
+    delete_url = (
+        client.app.router["delete_file"]
+        .url_for(
+            location_id=f"{location_id}",
+            file_id=urllib.parse.quote(
+                "/".join(list_of_files[0].file_id.split("/")[:2]) + "/does_not_exist",
+                safe="",
+            ),
+        )
+        .with_query(user_id=user_id)
+    )
+    response = await client.delete(f"{delete_url}")
+    _, error = await assert_status(response, status.HTTP_204_NO_CONTENT)
+    assert error is None
+
+    list_of_files: list[FileMetaDataGet] = await _list_files_legacy(
+        client, user_id, location_id, directory_file_upload
+    )
+
     # DIRECTORY REMOVAL
 
     await delete_directory(directory_file_upload=directory_file_upload)