Add support for external URLs in File storage and update tests

Author: BryanFauble

synapseclient/models/file.py

@@ -656,9 +656,16 @@ def _cannot_store(self) -> bool:
         return (
             not (
                 self.id is not None
-                and (self.path is not None or self.data_file_handle_id is not None)
+                and (
+                    self.path is not None
+                    or self.external_url is not None
+                    or self.data_file_handle_id is not None
+                )
+            )
+            and not (
+                (self.path is not None or self.external_url is not None)
+                and self.parent_id is not None
             )
-            and not (self.path is not None and self.parent_id is not None)
             and not (
                 self.parent_id is not None and self.data_file_handle_id is not None
             )
@@ -766,12 +773,12 @@ async def store_async(
             File object.
 
         If no Name is specified this will be derived from the file name. This is the
-        reccommended way to store a file in Synapse.
+        recommended way to store a file in Synapse.
 
         Please note:
         The file, as it appears on disk, will be the file that is downloaded from
         Synapse. The name of the actual File is different from the name of the File
-        Entity in Synapse. It is generally not reccommended to specify a different
+        Entity in Synapse. It is generally not recommended to specify a different
         name for the Entity and the file as it will cause confusion and potential
         conflicts later on.
 
@@ -787,8 +794,9 @@ async def store_async(
             The file object.
 
         Raises:
-            ValueError: If the file does not have an ID and a path, or a path and a
-                parent ID, or a data file handle ID and a parent ID.
+            ValueError: If the file does not have an ID and a (path or external_url),
+                or a (path or external_url) and a parent ID, or a data file handle ID
+                and a parent ID.
 
         Example: Using this function
             File with the ID `syn123` at path `path/to/file.txt`:
@@ -803,6 +811,10 @@ async def store_async(
 
                 file_instance = await File(path="path/to/file.txt").store_async(parent=Folder(id="syn456"))
 
+            File with an external URL and a parent folder with the ID `syn456`:
+
+                file_instance = await File(name="my_file.txt", external_url="s3://bucket/key", parent_id="syn456", synapse_store=False).store_async()
+
             File with a parent and existing file handle (This allows multiple entities to reference the underlying file):
 
                 file_instance = await File(data_file_handle_id="123", parent_id="syn456").store_async()
@@ -814,7 +826,7 @@ async def store_async(
                 await file_instance.change_metadata_async(name="my_new_name_file.txt")
 
             Rename a file, and the name of the file as downloaded
-                (Does not update the file on disk). Is is reccommended that `name` and
+                (Does not update the file on disk). Is is recommended that `name` and
                 `download_as` match to prevent confusion later on:
 
                 file_instance = await File(id="syn123", download_file=False).get_async()
@@ -825,11 +837,15 @@ async def store_async(
         self.parent_id = parent.id if parent else self.parent_id
         if self._cannot_store():
             raise ValueError(
-                "The file must have an (ID with a (path or `data_file_handle_id`)), or a "
-                "(path with a (`parent_id` or parent with an id)), or a "
+                "The file must have an (ID with a (path, external_url, or `data_file_handle_id`)), or a "
+                "((path or external_url) with a (`parent_id` or parent with an id)), or a "
                 "(data_file_handle_id with a (`parent_id` or parent with an id)) to store."
             )
-        self.name = self.name or (guess_file_name(self.path) if self.path else None)
+        self.name = self.name or (
+            guess_file_name(self.path)
+            if self.path
+            else (guess_file_name(self.external_url) if self.external_url else None)
+        )
         client = Synapse.get_client(synapse_client=synapse_client)
 
         if existing_file := await self._find_existing_file(synapse_client=client):
@@ -847,8 +863,9 @@ async def store_async(
                 }
             )
 
-        if self.path:
-            self.path = os.path.expanduser(self.path)
+        if self.path or self.external_url:
+            if self.path:
+                self.path = os.path.expanduser(self.path)
             async with client._get_parallel_file_transfer_semaphore(
                 asyncio_event_loop=asyncio.get_running_loop()
             ):

tests/integration/synapseclient/models/async/test_file_async.py

@@ -846,6 +846,64 @@ async def test_store_as_external_url_with_content_size_and_md5(
         assert file.file_handle.external_url == BOGUS_URL
         assert file.file_handle.content_md5 == BOGUS_MD5
 
+    async def test_store_external_url_then_update(
+        self, project_model: Project, file: File
+    ) -> None:
+        # GIVEN a file with an external URL
+        file.name = str(uuid.uuid4())
+        file.synapse_store = False
+        file.external_url = BOGUS_URL
+
+        # WHEN I store the file
+        file = await file.store_async(parent=project_model, synapse_client=self.syn)
+        self.schedule_for_cleanup(file.id)
+
+        # THEN I expect the file to be stored with the external URL
+        assert file.id is not None
+        assert file.external_url == BOGUS_URL
+        assert file.version_number == 1
+        assert (
+            file.file_handle.concrete_type
+            == "org.sagebionetworks.repo.model.file.ExternalFileHandle"
+        )
+        assert file.file_handle.external_url == BOGUS_URL
+        original_file_handle_id = file.file_handle.id
+
+        # WHEN I update the external URL
+        new_external_url = "https://www.example.com/updated"
+        file.external_url = new_external_url
+        file = await file.store_async(synapse_client=self.syn)
+
+        # THEN I expect the file to be updated with the new external URL
+        assert file.external_url == new_external_url
+        assert file.version_number == 2
+        assert file.file_handle.external_url == new_external_url
+        assert file.file_handle.id != original_file_handle_id
+
+    async def test_store_external_url_without_synapse_store_false(
+        self, project_model: Project, file: File
+    ) -> None:
+        # GIVEN a file with an external URL but synapse_store not explicitly set to False
+        file.name = str(uuid.uuid4())
+        file.external_url = BOGUS_URL
+
+        # WHEN I store the file
+        file = await file.store_async(parent=project_model, synapse_client=self.syn)
+        self.schedule_for_cleanup(file.id)
+
+        # THEN I expect the file to be stored with the external URL
+        # and synapse_store should be implicitly set to False
+        assert file.id is not None
+        assert file.external_url == BOGUS_URL
+        assert file.synapse_store is False
+        assert (
+            file.file_handle.concrete_type
+            == "org.sagebionetworks.repo.model.file.ExternalFileHandle"
+        )
+        assert file.file_handle.external_url == BOGUS_URL
+        assert file.file_handle.bucket_name is None
+        assert file.file_handle.key is None
+
     async def test_store_conflict_with_existing_object(
         self, project_model: Project, file: File
     ) -> None:

tests/integration/synapseclient/models/synchronous/test_file.py

@@ -819,6 +819,64 @@ def test_store_as_external_url_with_content_size_and_md5(
         assert file.file_handle.external_url == BOGUS_URL
         assert file.file_handle.content_md5 == BOGUS_MD5
 
+    def test_store_external_url_then_update(
+        self, project_model: Project, file: File
+    ) -> None:
+        # GIVEN a file with an external URL
+        file.name = str(uuid.uuid4())
+        file.synapse_store = False
+        file.external_url = BOGUS_URL
+
+        # WHEN I store the file
+        file = file.store(parent=project_model, synapse_client=self.syn)
+        self.schedule_for_cleanup(file.id)
+
+        # THEN I expect the file to be stored with the external URL
+        assert file.id is not None
+        assert file.external_url == BOGUS_URL
+        assert file.version_number == 1
+        assert (
+            file.file_handle.concrete_type
+            == "org.sagebionetworks.repo.model.file.ExternalFileHandle"
+        )
+        assert file.file_handle.external_url == BOGUS_URL
+        original_file_handle_id = file.file_handle.id
+
+        # WHEN I update the external URL
+        new_external_url = "https://www.example.com/updated"
+        file.external_url = new_external_url
+        file = file.store(synapse_client=self.syn)
+
+        # THEN I expect the file to be updated with the new external URL
+        assert file.external_url == new_external_url
+        assert file.version_number == 2
+        assert file.file_handle.external_url == new_external_url
+        assert file.file_handle.id != original_file_handle_id
+
+    def test_store_external_url_without_synapse_store_false(
+        self, project_model: Project, file: File
+    ) -> None:
+        # GIVEN a file with an external URL but synapse_store not explicitly set to False
+        file.name = str(uuid.uuid4())
+        file.external_url = BOGUS_URL
+
+        # WHEN I store the file
+        file = file.store(parent=project_model, synapse_client=self.syn)
+        self.schedule_for_cleanup(file.id)
+
+        # THEN I expect the file to be stored with the external URL
+        # and synapse_store should be implicitly set to False
+        assert file.id is not None
+        assert file.external_url == BOGUS_URL
+        assert file.synapse_store is False
+        assert (
+            file.file_handle.concrete_type
+            == "org.sagebionetworks.repo.model.file.ExternalFileHandle"
+        )
+        assert file.file_handle.external_url == BOGUS_URL
+        assert file.file_handle.bucket_name is None
+        assert file.file_handle.key is None
+
     def test_store_conflict_with_existing_object(
         self, project_model: Project, file: File
     ) -> None: