added additonal checks for org and schema names

Author: andrewelamb

synapseclient/models/schema_organization.py

@@ -1385,22 +1385,32 @@ def list_json_schema_organizations(
     return all_orgs
 
 
-def _check_name(name) -> None:
+def _check_name(name: str) -> None:
     """
     Checks that the input name is a valid Synapse Organization or JSONSchema name
-    - Must start with a letter
-    - Must contains only letters, numbers and periods.
+    - Length requirement of 6 ≤ x ≤ 250
+    - Names do not contain the string sagebionetworks (case insensitive)
+    - May contain periods (each part is separated by periods)
+    - Each part must start with a letter
+    - Each part contains only letters and numbers
 
     Arguments:
         name: The name of the organization to be checked
 
     Raises:
         ValueError: When the name isn't valid
     """
-    if not re.match("^([A-Za-z])([A-Za-z]|\d|\.)*$", name):
-        raise ValueError(
-            (
-                "Name must start with a letter and contain "
-                f"only letters numbers and periods: {name}"
+    if not 6 <= len(name) <= 250:
+        raise ValueError(f"The name must be of length 6 to 250 characters: {name}")
+    if re.search("sagebionetworks", name.lower()):
+        raise ValueError(f"The name must not contain 'sagebionetworks' : {name}")
+    parts = name.split(".")
+    for part in parts:
+        if not re.match("^([A-Za-z])([A-Za-z]|\d|)*$", part):
+            raise ValueError(
+                (
+                    "Name may be separated by periods, "
+                    "but each part must start with a letter and contain "
+                    f"only letters and numbers: {name}"
+                )
             )
-        )

tests/unit/synapseclient/models/synchronous/unit_test_schema_organization.py

@@ -1,37 +1,13 @@
 """Unit tests for SchemaOrganization and JSONSchema classes"""
-from typing import Any
-
 import pytest
 
 from synapseclient.models import JSONSchema, SchemaOrganization
-from synapseclient.models.schema_organization import CreateSchemaRequest
+from synapseclient.models.schema_organization import CreateSchemaRequest, _check_name
 
 
 class TestSchemaOrganization:
     """Synchronous unit tests for SchemaOrganization."""
 
-    @pytest.mark.parametrize(
-        "name",
-        ["AAAAAAA", "A12345", "A....."],
-        ids=["Just letters", "Numbers", "Periods"],
-    )
-    def test_init(self, name: str) -> None:
-        "Tests that legal names don't raise a ValueError on init"
-        assert SchemaOrganization(name)
-
-    @pytest.mark.parametrize(
-        "name",
-        ["1AAAAAA", ".AAAAAA", "AAAAAA!"],
-        ids=["Starts with a number", "Starts with a period", "Special character"],
-    )
-    def test_init_name_exceptions(self, name: str) -> None:
-        "Tests that illegal names raise a ValueError on init"
-        with pytest.raises(
-            ValueError,
-            match="Name must start with a letter and contain only letters numbers and periods",
-        ):
-            SchemaOrganization(name)
-
     def test_fill_from_dict(self) -> None:
         "Tests that fill_from_dict fills in all fields"
         organization = SchemaOrganization()
@@ -56,28 +32,6 @@ def test_fill_from_dict(self) -> None:
 class TestJSONSchema:
     """Synchronous unit tests for JSONSchema."""
 
-    @pytest.mark.parametrize(
-        "name",
-        ["AAAAAAA", "A12345", "A....."],
-        ids=["Just letters", "Numbers", "Periods"],
-    )
-    def test_init(self, name: str) -> None:
-        "Tests that legal names don't raise a ValueError on init"
-        assert JSONSchema(name, "org.name")
-
-    @pytest.mark.parametrize(
-        "name",
-        ["1AAAAAA", ".AAAAAA", "AAAAAA!"],
-        ids=["Starts with a number", "Starts with a period", "Special character"],
-    )
-    def test_init_name_exceptions(self, name: str) -> None:
-        "Tests that illegal names raise a ValueError on init"
-        with pytest.raises(
-            ValueError,
-            match="Name must start with a letter and contain only letters numbers and periods",
-        ):
-            JSONSchema(name, "org.name")
-
     @pytest.mark.parametrize(
         "uri",
         ["ORG.NAME-SCHEMA.NAME", "ORG.NAME-SCHEMA.NAME-0.0.1"],
@@ -147,74 +101,91 @@ def test_check_semantic_version_with_exceptions(self) -> None:
 
 class TestCreateSchemaRequest:
     @pytest.mark.parametrize(
-        "name",
-        ["AAAAAAA", "A12345", "A....."],
-        ids=["Just letters", "Numbers", "Periods"],
+        "version",
+        ["0.0.1", "1.0.0"],
     )
-    def test_init_name(self, name: str) -> None:
-        "Tests that legal names don't raise a ValueError on init"
-        assert CreateSchemaRequest(schema={}, name=name, organization_name="org.name")
+    def test_init_version(self, version: str) -> None:
+        "Tests that legal versions don't raise a ValueError on init"
+        assert CreateSchemaRequest(
+            schema={}, name="schema.name", organization_name="org.name", version=version
+        )
 
     @pytest.mark.parametrize(
-        "name",
-        ["1AAAAAA", ".AAAAAA", "AAAAAA!"],
-        ids=["Starts with a number", "Starts with a period", "Special character"],
+        "version",
+        ["1", "1.0", "0.0.0.1", "0.0.0"],
     )
-    def test_init_name_exceptions(self, name: str) -> None:
-        "Tests that illegal names raise a ValueError on init"
+    def test_init_version_exceptions(self, version: str) -> None:
+        "Tests that illegal versions raise a ValueError on init"
         with pytest.raises(
             ValueError,
-            match="Name must start with a letter and contain only letters numbers and periods",
+            match="Schema version must be a semantic version starting at 0.0.1",
         ):
-            CreateSchemaRequest(schema={}, name=name, organization_name="org.name")
+            CreateSchemaRequest(
+                schema={},
+                name="schema.name",
+                organization_name="org.name",
+                version=version,
+            )
+
+
+class TestCheckName:
+    """Tests for check name helper function"""
 
     @pytest.mark.parametrize(
         "name",
-        ["AAAAAAA", "A12345", "A....."],
-        ids=["Just letters", "Numbers", "Periods"],
+        ["aaaaaaa", "aaaaaa1", "aa.aa.aa", "a1.a1.a1"],
     )
-    def test_init_org_name(self, name: str) -> None:
-        "Tests that legal org names don't raise a ValueError on init"
-        assert CreateSchemaRequest(
-            schema={}, name="schema.name", organization_name=name
-        )
+    def test_check_name(self, name: str):
+        """Checks that legal names don't raise an exception"""
+        _check_name(name)
 
     @pytest.mark.parametrize(
         "name",
-        ["1AAAAAA", ".AAAAAA", "AAAAAA!"],
-        ids=["Starts with a number", "Starts with a period", "Special character"],
+        [
+            "a",
+            "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+        ],
     )
-    def test_init_org_name_exceptions(self, name: str) -> None:
-        "Tests that illegal org names raise a ValueError on init"
+    def test_check_length_exception(self, name: str):
+        """Checks that names that are too short or long raise an exception"""
         with pytest.raises(
-            ValueError,
-            match="Name must start with a letter and contain only letters numbers and periods",
+            ValueError, match="The name must be of length 6 to 250 characters"
         ):
-            CreateSchemaRequest(schema={}, name="schema.name", organization_name=name)
+            _check_name(name)
 
     @pytest.mark.parametrize(
-        "version",
-        ["0.0.1", "1.0.0"],
+        "name",
+        [
+            "sagebionetworks",
+            "asagebionetworks",
+            "sagebionetworksa",
+            "aaa.sagebionetworks.aaa",
+            "SAGEBIONETWORKS",
+            "SageBionetworks",
+        ],
     )
-    def test_init_version(self, version: str) -> None:
-        "Tests that legal versions don't raise a ValueError on init"
-        assert CreateSchemaRequest(
-            schema={}, name="schema.name", organization_name="org.name", version=version
-        )
+    def test_check_sage_exception(self, name: str):
+        """Checks that names that contain 'sagebionetworks' raise an exception"""
+        with pytest.raises(
+            ValueError, match="The name must not contain 'sagebionetworks'"
+        ):
+            _check_name(name)
 
     @pytest.mark.parametrize(
-        "version",
-        ["1", "1.0", "0.0.0.1", "0.0.0"],
+        "name",
+        ["1AAAAA", "AAA.1AAA", "AAA.AAA.1AAA", ".AAAAAAA", "AAAAAAAA!"],
+        ids=[
+            "Starts with number",
+            "Part2 starts with number",
+            "Part3 starts with number",
+            "Starts with period",
+            "Contains special characters",
+        ],
     )
-    def test_init_version_exceptions(self, version: str) -> None:
-        "Tests that illegal versions raise a ValueError on init"
+    def test_check_content_exception(self, name: str):
+        """Checks that names that contain special characters(besides periods) or have parts that start with numbers raise an exception"""
         with pytest.raises(
             ValueError,
-            match="Schema version must be a semantic version starting at 0.0.1",
+            match="Name may be separated by periods, but each part must start with a letter and contain only letters and numbers",
         ):
-            CreateSchemaRequest(
-                schema={},
-                name="schema.name",
-                organization_name="org.name",
-                version=version,
-            )
+            _check_name(name)