diff --git a/synapseclient/models/schema_organization.py b/synapseclient/models/schema_organization.py index a6fc10566..91922a4d5 100644 --- a/synapseclient/models/schema_organization.py +++ b/synapseclient/models/schema_organization.py @@ -254,7 +254,7 @@ class SchemaOrganization(SchemaOrganizationProtocol): def __post_init__(self) -> None: if self.name: - _check_name(self.name) + _check_org_name(self.name) async def get_async( self, *, synapse_client: Optional["Synapse"] = None @@ -826,9 +826,9 @@ class JSONSchema(JSONSchemaProtocol): def __post_init__(self) -> None: if self.name: - _check_name(self.name) + _check_schema_name(self.name) if self.organization_name: - _check_name(self.organization_name) + _check_org_name(self.organization_name) if self.name and self.organization_name: self.uri = f"{self.organization_name}-{self.name}" else: @@ -1313,8 +1313,8 @@ class CreateSchemaRequest(AsynchronousCommunicator): def __post_init__(self) -> None: self.concrete_type = CREATE_SCHEMA_REQUEST - _check_name(self.name) - _check_name(self.organization_name) + _check_schema_name(self.name) + _check_org_name(self.organization_name) uri = f"{self.organization_name}-{self.name}" if self.version: self._check_semantic_version(self.version) @@ -1428,9 +1428,9 @@ def list_json_schema_organizations( return all_orgs -def _check_name(name: str) -> None: +def _check_org_name(name: str) -> None: """ - Checks that the input name is a valid Synapse Organization or JSONSchema name + Checks that the input name is a valid Synapse Organization - Length requirement of 6 ≤ x ≤ 250 - Names do not contain the string sagebionetworks (case insensitive) - May contain periods (each part is separated by periods) @@ -1449,11 +1449,42 @@ def _check_name(name: str) -> None: raise ValueError(f"The name must not contain 'sagebionetworks' : {name}") parts = name.split(".") for part in parts: - if not re.match(r"^([A-Za-z])([A-Za-z]|\d|)*$", part): + if len(part) == 0: + raise ValueError(f"Organization name sections must not be empty: {name}") + if not re.match(r"^([A-Za-z])([A-Za-z0-9])*$", 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}" + f"Un-allowed characters in organization name: {name}" + "Organization name may separated into sections by periods. " + "For example 'my.organization.name'. " + "Each section must start with a letter and contain only letters and numbers." + ) + ) + + +def _check_schema_name(name: str) -> None: + """ + Checks that the input name is a valid Synapse JSONSchema name + - 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 schema to be checked + + Raises: + ValueError: When the name isn't valid + """ + parts = name.split(".") + for part in parts: + if len(part) == 0: + raise ValueError(f"Schema name sections must not be empty: {name}") + if not re.match(r"^([A-Za-z])([A-Za-z0-9])*$", part): + raise ValueError( + ( + f"Un-allowed characters in schema name: {name}" + "Schema name may separated into sections by periods. " + "For example 'my.schema.name'. " + "Each section must start with a letter and contain only letters and numbers." ) ) diff --git a/tests/unit/synapseclient/models/async/unit_test_schema_organization_async.py b/tests/unit/synapseclient/models/async/unit_test_schema_organization_async.py index 494e064ba..24f9cd7fe 100644 --- a/tests/unit/synapseclient/models/async/unit_test_schema_organization_async.py +++ b/tests/unit/synapseclient/models/async/unit_test_schema_organization_async.py @@ -10,7 +10,8 @@ CreateSchemaRequest, JSONSchema, SchemaOrganization, - _check_name, + _check_org_name, + _check_schema_name, ) ORG_NAME = "mytest.organization" @@ -97,42 +98,59 @@ def _get_acl_response(): } -class TestCheckName: - """Tests for the _check_name validation function.""" +class TestCheckOrgName: + """Tests for the _check_org_name validation function.""" def test_valid_name(self) -> None: # GIVEN a valid name # WHEN I check the name # THEN no exception should be raised - _check_name("mytest.organization") + _check_org_name("mytest.organization") def test_name_too_short(self) -> None: # GIVEN a name that is too short # WHEN I check the name # THEN it should raise ValueError with pytest.raises(ValueError, match="length 6 to 250"): - _check_name("abc") + _check_org_name("abc") def test_name_too_long(self) -> None: # GIVEN a name that is too long # WHEN I check the name # THEN it should raise ValueError with pytest.raises(ValueError, match="length 6 to 250"): - _check_name("a" * 251) + _check_org_name("a" * 251) def test_name_contains_sagebionetworks(self) -> None: # GIVEN a name containing 'sagebionetworks' # WHEN I check the name # THEN it should raise ValueError with pytest.raises(ValueError, match="sagebionetworks"): - _check_name("my.sagebionetworks.test") + _check_org_name("my.sagebionetworks.test") def test_name_part_starts_with_number(self) -> None: # GIVEN a name where a part starts with a number # WHEN I check the name # THEN it should raise ValueError with pytest.raises(ValueError, match="must start with a letter"): - _check_name("mytest.1invalid") + _check_org_name("mytest.1invalid") + + +class TestCheckSchemaName: + """Tests for the _check_schema_name validation function.""" + + def test_valid_name(self) -> None: + # GIVEN a valid name + # WHEN I check the name + # THEN no exception should be raised + _check_schema_name("mytest.schema") + + def test_name_part_starts_with_number(self) -> None: + # GIVEN a name where a part starts with a number + # WHEN I check the name + # THEN it should raise ValueError + with pytest.raises(ValueError, match="must start with a letter"): + _check_schema_name("mytest.1invalid") class TestSchemaOrganization: