🐛 Storage: Wrong file destination path when copying a project (ITISFoundation#3700)

Author: sanderegg

services/storage/src/simcore_service_storage/models.py

@@ -99,7 +99,7 @@ def from_simcore_node(
             "location": location_name,
             "bucket_name": bucket,
             "object_name": file_id,
-            "file_name": parts[2],
+            "file_name": parts[-1],
             "user_id": user_id,
             "project_id": parse_obj_as(ProjectID, parts[0])
             if is_uuid(parts[0])

services/storage/src/simcore_service_storage/simcore_s3_dsm.py

@@ -470,7 +470,7 @@ async def deep_copy_project_simcore_s3(
                         user_id,
                         src_fmd,
                         SimcoreS3FileID(
-                            f"{dst_project_uuid}/{new_node_id}/{src_fmd.object_name.split('/')[-1]}"
+                            f"{dst_project_uuid}/{new_node_id}/{src_fmd.object_name.split('/', maxsplit=2)[-1]}"
                         ),
                         bytes_transfered_cb=s3_transfered_data_cb.copy_transfer_cb,
                     )

services/storage/tests/conftest.py

@@ -543,10 +543,19 @@ async def _uploader(
 
 
 @pytest.fixture
-def create_simcore_file_id() -> Callable[[ProjectID, NodeID, str], SimcoreS3FileID]:
+def create_simcore_file_id(
+    faker: Faker,
+) -> Callable[[ProjectID, NodeID, str, Optional[Path]], SimcoreS3FileID]:
     def _creator(
-        project_id: ProjectID, node_id: NodeID, file_name: str
+        project_id: ProjectID,
+        node_id: NodeID,
+        file_name: str,
+        file_base_path: Optional[Path] = None,
     ) -> SimcoreS3FileID:
-        return parse_obj_as(SimcoreS3FileID, f"{project_id}/{node_id}/{file_name}")
+        s3_file_name = file_name
+        if file_base_path:
+            s3_file_name = f"{file_base_path / file_name}"
+        clean_path = Path(f"{project_id}/{node_id}/{s3_file_name}")
+        return SimcoreS3FileID(f"{clean_path}")
 
     return _creator

services/storage/tests/fixtures/data_models.py

@@ -3,7 +3,7 @@
 # pylint:disable=redefined-outer-name
 
 
-from typing import Any, AsyncIterator, Awaitable, Callable
+from typing import Any, AsyncIterator, Awaitable, Callable, Optional
 
 import pytest
 import sqlalchemy as sa
@@ -79,7 +79,9 @@ async def project_id(
 async def create_project_node(
     user_id: UserID, aiopg_engine: Engine, faker: Faker
 ) -> AsyncIterator[Callable[..., Awaitable[NodeID]]]:
-    async def _creator(project_id: ProjectID, **kwargs) -> NodeID:
+    async def _creator(
+        project_id: ProjectID, node_id: Optional[NodeID] = None, **kwargs
+    ) -> NodeID:
         async with aiopg_engine.acquire() as conn:
             result = await conn.execute(
                 sa.select([projects.c.workbench]).where(
@@ -89,7 +91,7 @@ async def _creator(project_id: ProjectID, **kwargs) -> NodeID:
             row = await result.fetchone()
             assert row
             project_workbench: dict[str, Any] = row[projects.c.workbench]
-            new_node_id = NodeID(faker.uuid4())
+            new_node_id = node_id or NodeID(faker.uuid4())
             node_data = {
                 "key": "simcore/services/frontend/file-picker",
                 "version": "1.0.0",

services/storage/tests/helpers/utils_file_meta_data.py

@@ -24,7 +24,11 @@ async def assert_file_meta_data_in_db(
         )
         db_data = await result.fetchall()
         assert db_data is not None
-        assert len(db_data) == (1 if expected_entry_exists else 0)
+        assert len(db_data) == (1 if expected_entry_exists else 0), (
+            f"{file_id} was not found!"
+            if expected_entry_exists
+            else f"{file_id} should not exist"
+        )
         upload_id = None
         if expected_entry_exists:
             row = db_data[0]

services/storage/tests/unit/test_handlers_simcore_s3.py

@@ -186,7 +186,9 @@ async def random_project_with_files(
     aiopg_engine: Engine,
     create_project: Callable[[], Awaitable[dict[str, Any]]],
     create_project_node: Callable[..., Awaitable[NodeID]],
-    create_simcore_file_id: Callable[[ProjectID, NodeID, str], SimcoreS3FileID],
+    create_simcore_file_id: Callable[
+        [ProjectID, NodeID, str, Optional[Path]], SimcoreS3FileID
+    ],
     upload_file: Callable[
         [ByteSize, str, str], Awaitable[tuple[Path, SimcoreS3FileID]]
     ],
@@ -207,23 +209,43 @@ async def _creator(
         src_projects_list: dict[NodeID, dict[SimcoreS3FileID, Path]] = {}
         upload_tasks: deque[Awaitable] = deque()
         for _node_index in range(num_nodes):
-            # NOTE: we put some more outputs in there to simuate a real case better
+            # NOTE: we put some more outputs in there to simulate a real case better
+            new_node_id = NodeID(faker.uuid4())
+            output3_file_id = create_simcore_file_id(
+                ProjectID(project["uuid"]),
+                new_node_id,
+                faker.file_name(),
+                Path("outputs/output3"),
+            )
             src_node_id = await create_project_node(
                 ProjectID(project["uuid"]),
-                outputs={"output_1": faker.pyint(), "output_2": faker.pystr()},
+                new_node_id,
+                outputs={
+                    "output_1": faker.pyint(),
+                    "output_2": faker.pystr(),
+                    "output_3": f"{output3_file_id}",
+                },
             )
+            assert src_node_id == new_node_id
+
+            # upload the output 3 and some random other files at the root of each node
             src_projects_list[src_node_id] = {}
+            src_file, _ = await upload_file(
+                choice(file_sizes), Path(output3_file_id).name, output3_file_id
+            )
+            src_projects_list[src_node_id][output3_file_id] = src_file
 
             async def _upload_file_and_update_project(project, src_node_id):
                 src_file_name = faker.file_name()
                 src_file_uuid = create_simcore_file_id(
-                    ProjectID(project["uuid"]), src_node_id, src_file_name
+                    ProjectID(project["uuid"]), src_node_id, src_file_name, None
                 )
                 src_file, _ = await upload_file(
                     choice(file_sizes), src_file_name, src_file_uuid
                 )
                 src_projects_list[src_node_id][src_file_uuid] = src_file
 
+            # add a few random files in the node storage
             upload_tasks.extend(
                 [
                     _upload_file_and_update_project(project, src_node_id)
@@ -277,11 +299,14 @@ async def test_copy_folders_from_valid_project_with_one_large_file(
     for src_node_id in src_projects_list:
         dst_node_id = nodes_map.get(NodeIDStr(f"{src_node_id}"))
         assert dst_node_id
-        for src_file in src_projects_list[src_node_id].values():
+        for src_file_id, src_file in src_projects_list[src_node_id].items():
             await assert_file_meta_data_in_db(
                 aiopg_engine,
-                file_id=create_simcore_file_id(
-                    ProjectID(dst_project["uuid"]), NodeID(dst_node_id), src_file.name
+                file_id=parse_obj_as(
+                    SimcoreS3FileID,
+                    f"{src_file_id}".replace(
+                        src_project["uuid"], dst_project["uuid"]
+                    ).replace(f"{src_node_id}", f"{dst_node_id}"),
                 ),
                 expected_entry_exists=True,
                 expected_file_size=src_file.stat().st_size,
@@ -317,15 +342,19 @@ async def test_copy_folders_from_valid_project(
     assert data == jsonable_encoder(
         await _get_updated_project(aiopg_engine, dst_project["uuid"])
     )
+
     # check that file meta data was effectively copied
     for src_node_id in src_projects_list:
         dst_node_id = nodes_map.get(NodeIDStr(f"{src_node_id}"))
         assert dst_node_id
-        for src_file in src_projects_list[src_node_id].values():
+        for src_file_id, src_file in src_projects_list[src_node_id].items():
             await assert_file_meta_data_in_db(
                 aiopg_engine,
-                file_id=create_simcore_file_id(
-                    ProjectID(dst_project["uuid"]), NodeID(dst_node_id), src_file.name
+                file_id=parse_obj_as(
+                    SimcoreS3FileID,
+                    f"{src_file_id}".replace(
+                        src_project["uuid"], dst_project["uuid"]
+                    ).replace(f"{src_node_id}", f"{dst_node_id}"),
                 ),
                 expected_entry_exists=True,
                 expected_file_size=src_file.stat().st_size,