Repository: Sage-Bionetworks/synapsePythonClient

Issue:
[Feature request] Add `is_synapse_id` to the client
# Feature Request

Add a helper function to the client that will allow checking whether a given string is a valid Synapse ID.  A "valid" synID must:

* have the expected format of `syn[0-9]+`
* be linked to an actual Synapse entity, that is, "syn123" is not a real Synapse entity

### Expected behavior

Return `True`/`False` whether a given synID is valid.  Something like this:

```python
>>> is_synapse_id("test")
False
>>> is_synapse_id("123")
False
>>> is_synapse_id("syn123")
False
>>> is_synapse_id("syn123abc")
False
>>> is_synapse_id("syn28590455")
True
>>> is_synapse_id("syn22392179")
True
```

### Request Motivation

One of our collaborators for a challenge wants to implement a check in their workflow to ensure given IDs are valid Synapse IDs. I suggested the utils function, `is_synapse_id`, but after some digging, realize that this function is more for testing the client, rather than for users to use.



Discussion:

Patch:
diff --git a/synapseclient/client.py b/synapseclient/client.py
index 1f7a4c0d..0034732a 100644
--- a/synapseclient/client.py
+++ b/synapseclient/client.py
@@ -626,6 +626,24 @@ class Synapse(object):
                 return False
             raise
 
+    def is_synapse_id(self, syn_id: str) -> bool:
+        """Checks if given synID is valid (attached to actual entity?)"""
+        if isinstance(syn_id, str):
+            try:
+                self.get(syn_id, downloadFile=False)
+            except SynapseFileNotFoundError:
+                return False
+            except (SynapseHTTPError, SynapseAuthenticationError, ) as err:
+                status = err.__context__.response.status_code or err.response.status_code
+                if status in (400, 404):
+                    return False
+                # Valid ID but user lacks permission or is not logged in
+                elif status == 403:
+                    return True
+            return True
+        self.logger.warn("synID must be a string")
+        return False
+
     def onweb(self, entity, subpageId=None):
         """Opens up a browser window to the entity page or wiki-subpage.
 
@@ -1448,7 +1466,7 @@ class Synapse(object):
         downloaded_files = []
         new_manifest_path = f'manifest_{time.time_ns()}.csv'
         with open(dl_list_path) as manifest_f, \
-             open(new_manifest_path, 'w') as write_obj:
+                open(new_manifest_path, 'w') as write_obj:
 
             reader = csv.DictReader(manifest_f)
             columns = reader.fieldnames


Test patch:
diff --git a/tests/unit/synapseclient/unit_test_client.py b/tests/unit/synapseclient/unit_test_client.py
index 32cdb224..de06cb73 100644
--- a/tests/unit/synapseclient/unit_test_client.py
+++ b/tests/unit/synapseclient/unit_test_client.py
@@ -962,7 +962,7 @@ def test_check_entity_restrictions__no_unmet_restriction(syn):
             'parentId': 'syn12345'},
             'restrictionInformation': {
                 'hasUnmetAccessRequirement': False
-            }
+        }
         }
         entity = 'syn123'
         syn._check_entity_restrictions(bundle, entity, True)
@@ -979,7 +979,7 @@ def test_check_entity_restrictions__unmet_restriction_entity_file_with_downloadF
             'entityType': 'file',
             'restrictionInformation': {
                 'hasUnmetAccessRequirement': True
-            }
+        }
         }
         entity = 'syn123'
         pytest.raises(SynapseUnmetAccessRestrictions, syn._check_entity_restrictions, bundle, entity, True)
@@ -996,7 +996,7 @@ def test_check_entity_restrictions__unmet_restriction_entity_project_with_downlo
             'entityType': 'project',
             'restrictionInformation': {
                 'hasUnmetAccessRequirement': True
-            }
+        }
         }
         entity = 'syn123'
         syn._check_entity_restrictions(bundle, entity, True)
@@ -1016,7 +1016,7 @@ def test_check_entity_restrictions__unmet_restriction_entity_folder_with_downloa
             'entityType': 'folder',
             'restrictionInformation': {
                 'hasUnmetAccessRequirement': True
-            }
+        }
         }
         entity = 'syn123'
         syn._check_entity_restrictions(bundle, entity, True)
@@ -2056,12 +2056,12 @@ def test_store__needsUploadFalse__fileHandleId_not_in_local_state(syn):
                        'annotations': {'id': synapse_id, 'etag': etag, 'annotations': {}},
                        }
     with patch.object(syn, '_getEntityBundle', return_value=returned_bundle), \
-         patch.object(synapseclient.client, 'upload_file_handle', return_value=returned_file_handle), \
-         patch.object(syn.cache, 'contains', return_value=True), \
-         patch.object(syn, '_updateEntity'), \
-         patch.object(syn, 'set_annotations'), \
-         patch.object(Entity, 'create'), \
-         patch.object(syn, 'get'):
+            patch.object(synapseclient.client, 'upload_file_handle', return_value=returned_file_handle), \
+            patch.object(syn.cache, 'contains', return_value=True), \
+            patch.object(syn, '_updateEntity'), \
+            patch.object(syn, 'set_annotations'), \
+            patch.object(Entity, 'create'), \
+            patch.object(syn, 'get'):
 
         f = File('/fake_file.txt', parent=parent_id)
         syn.store(f)
@@ -2136,14 +2136,14 @@ def test_store__existing_processed_as_update(syn):
     }
 
     with patch.object(syn, '_getEntityBundle') as mock_get_entity_bundle, \
-         patch.object(synapseclient.client, 'upload_file_handle', return_value=returned_file_handle), \
-         patch.object(syn.cache, 'contains', return_value=True), \
-         patch.object(syn, '_createEntity') as mock_createEntity, \
-         patch.object(syn, '_updateEntity') as mock_updateEntity, \
-         patch.object(syn, 'findEntityId') as mock_findEntityId, \
-         patch.object(syn, 'set_annotations') as mock_set_annotations, \
-         patch.object(Entity, 'create'), \
-         patch.object(syn, 'get'):
+            patch.object(synapseclient.client, 'upload_file_handle', return_value=returned_file_handle), \
+            patch.object(syn.cache, 'contains', return_value=True), \
+            patch.object(syn, '_createEntity') as mock_createEntity, \
+            patch.object(syn, '_updateEntity') as mock_updateEntity, \
+            patch.object(syn, 'findEntityId') as mock_findEntityId, \
+            patch.object(syn, 'set_annotations') as mock_set_annotations, \
+            patch.object(Entity, 'create'), \
+            patch.object(syn, 'get'):
 
         mock_get_entity_bundle.return_value = returned_bundle
 
@@ -2233,14 +2233,14 @@ def test_store__409_processed_as_update(syn):
     }
 
     with patch.object(syn, '_getEntityBundle') as mock_get_entity_bundle, \
-         patch.object(synapseclient.client, 'upload_file_handle', return_value=returned_file_handle), \
-         patch.object(syn.cache, 'contains', return_value=True), \
-         patch.object(syn, '_createEntity') as mock_createEntity, \
-         patch.object(syn, '_updateEntity') as mock_updateEntity, \
-         patch.object(syn, 'findEntityId') as mock_findEntityId, \
-         patch.object(syn, 'set_annotations') as mock_set_annotations, \
-         patch.object(Entity, 'create'), \
-         patch.object(syn, 'get'):
+            patch.object(synapseclient.client, 'upload_file_handle', return_value=returned_file_handle), \
+            patch.object(syn.cache, 'contains', return_value=True), \
+            patch.object(syn, '_createEntity') as mock_createEntity, \
+            patch.object(syn, '_updateEntity') as mock_updateEntity, \
+            patch.object(syn, 'findEntityId') as mock_findEntityId, \
+            patch.object(syn, 'set_annotations') as mock_set_annotations, \
+            patch.object(Entity, 'create'), \
+            patch.object(syn, 'get'):
 
         mock_get_entity_bundle.side_effect = [None, returned_bundle]
         mock_createEntity.side_effect = SynapseHTTPError(response=DictObject({'status_code': 409}))
@@ -2487,11 +2487,11 @@ def test_store__existing_no_update(syn):
     }
 
     with patch.object(syn, '_getEntityBundle') as mock_get_entity_bundle, \
-         patch.object(synapseclient.client, 'upload_file_handle', return_value=returned_file_handle), \
-         patch.object(syn.cache, 'contains', return_value=True), \
-         patch.object(syn, '_createEntity') as mock_createEntity, \
-         patch.object(syn, '_updateEntity') as mock_updatentity, \
-         patch.object(syn, 'get'):
+            patch.object(synapseclient.client, 'upload_file_handle', return_value=returned_file_handle), \
+            patch.object(syn.cache, 'contains', return_value=True), \
+            patch.object(syn, '_createEntity') as mock_createEntity, \
+            patch.object(syn, '_updateEntity') as mock_updatentity, \
+            patch.object(syn, 'get'):
 
         mock_get_entity_bundle.return_value = returned_bundle
         mock_createEntity.side_effect = SynapseHTTPError(response=DictObject({'status_code': 409}))
@@ -2713,7 +2713,7 @@ class TestTableSnapshot:
         """Raise error if entity view or table not passed in"""
         wrong_type = Mock()
         with patch.object(syn, 'get', return_value=wrong_type),\
-             pytest.raises(ValueError, match="This function only accepts Synapse ids of Tables or Views"):
+                pytest.raises(ValueError, match="This function only accepts Synapse ids of Tables or Views"):
             syn.create_snapshot_version("syn1234")
 
 
@@ -2983,9 +2983,9 @@ def test__get_certified_passing_record(userid, syn):
 def test_is_certified(response, syn):
     with patch.object(syn, "getUserProfile",
                       return_value={"ownerId": "foobar"}) as patch_get_user,\
-         patch.object(syn,
-                      "_get_certified_passing_record",
-                      return_value={'passed': response}) as patch_get_cert:
+        patch.object(syn,
+                     "_get_certified_passing_record",
+                     return_value={'passed': response}) as patch_get_cert:
         is_certified = syn.is_certified("test")
         patch_get_user.assert_called_once_with("test")
         patch_get_cert.assert_called_once_with("foobar")
@@ -2999,15 +2999,27 @@ def test_is_certified__no_quiz_results(syn):
     response.status_code = 404
     with patch.object(syn, "getUserProfile",
                       return_value={"ownerId": "foobar"}) as patch_get_user,\
-         patch.object(syn,
-                      "_get_certified_passing_record",
-                      side_effect=SynapseHTTPError(response=response)) as patch_get_cert:
+        patch.object(syn,
+                     "_get_certified_passing_record",
+                     side_effect=SynapseHTTPError(response=response)) as patch_get_cert:
         is_certified = syn.is_certified("test")
     patch_get_user.assert_called_once_with("test")
     patch_get_cert.assert_called_once_with("foobar")
     assert is_certified is False
 
 
+def test_is_synapse_id(syn):
+    # Invalid IDs
+    assert not syn.is_synapse_id("test")
+    assert not syn.is_synapse_id("123")
+    assert not syn.is_synapse_id([])
+    assert not syn.is_synapse_id(["syn123"])
+
+    # Valid ID; must use Mock call to test
+    with patch.object(syn, 'get'):
+        assert syn.is_synapse_id("syn28590455")
+
+
 def test_init_change_cache_path():
     """
     Verify that the user can customize the cache path.