From 3acb24eae0c640f8ea8665e9daa82297b045e6a2 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Thu, 12 Sep 2024 16:07:33 -0700 Subject: [PATCH 1/3] IFC-569 generate hfid from single-attribute uniqueness constraint --- backend/infrahub/core/schema_manager.py | 18 +++++++++ .../schema_manager/test_manager_schema.py | 40 ++++++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/backend/infrahub/core/schema_manager.py b/backend/infrahub/core/schema_manager.py index 9e0f892836..9c06a3b9bf 100644 --- a/backend/infrahub/core/schema_manager.py +++ b/backend/infrahub/core/schema_manager.py @@ -1058,6 +1058,24 @@ def process_human_friendly_id(self) -> None: break continue + # if no human_friendly_id and a uniqueness_constraint with a single attribute exists + # then use that attribute as the human_friendly_id + if not node.human_friendly_id and node.uniqueness_constraints: + for constraint_paths in node.uniqueness_constraints: + if len(constraint_paths) > 1: + continue + constraint_path = constraint_paths[0] + schema_path = node.parse_schema_path(path=constraint_path, schema=node) + if ( + schema_path.is_type_attribute + and schema_path.attribute_property_name == "value" + and schema_path.attribute_schema + ): + node = self.get(name=name, duplicate=True) + node.human_friendly_id = [f"{schema_path.attribute_schema.name}__value"] + self.set(name=node.kind, schema=node) + break + if node.human_friendly_id and not node.unique_attributes and not node.uniqueness_constraints: uniqueness_constraints: list[str] = [] for item in node.human_friendly_id: diff --git a/backend/tests/unit/core/schema_manager/test_manager_schema.py b/backend/tests/unit/core/schema_manager/test_manager_schema.py index c21d74dc12..2dfd8682e3 100644 --- a/backend/tests/unit/core/schema_manager/test_manager_schema.py +++ b/backend/tests/unit/core/schema_manager/test_manager_schema.py @@ -185,7 +185,7 @@ async def test_schema_branch_process_inheritance_update_inherited_elements(anima assert dog.get_relationship(name="owner").optional is True -async def test_schema_branch_process_humain_friendly_id(animal_person_schema_dict): +async def test_schema_branch_process_human_friendly_id(animal_person_schema_dict): schema = SchemaBranch(cache={}, name="test") schema.load_schema(schema=SchemaRoot(**animal_person_schema_dict)) @@ -202,6 +202,44 @@ async def test_schema_branch_process_humain_friendly_id(animal_person_schema_dic assert dog.uniqueness_constraints == [["owner", "name__value"]] +async def test_schema_branch_infer_human_friendly_id_from_uniqueness_constraints(animal_person_schema_dict): + for node_schema_dict in animal_person_schema_dict["nodes"]: + if node_schema_dict["name"] == "Dog" and node_schema_dict["namespace"] == "Test": + node_schema_dict["uniqueness_constraints"] = [["name__value"]] + if node_schema_dict["name"] == "Cat" and node_schema_dict["namespace"] == "Test": + node_schema_dict["uniqueness_constraints"] = [["name__value", "owner"]] + node_schema_dict["human_friendly_id"] = None + if node_schema_dict["name"] == "Person" and node_schema_dict["namespace"] == "Test": + node_schema_dict["uniqueness_constraints"] = [["name__value"]] + node_schema_dict["human_friendly_id"] = ["name__value", "other_name__value"] + for generic_schema_dict in animal_person_schema_dict["generics"]: + if generic_schema_dict["name"] == "Animal" and generic_schema_dict["namespace"] == "Test": + generic_schema_dict["human_friendly_id"] = None + + schema = SchemaBranch(cache={}, name="test") + schema.load_schema(schema=SchemaRoot(**animal_person_schema_dict)) + + schema.process_inheritance() + schema.process_human_friendly_id() + + animal = schema.get(name="TestAnimal") + assert sorted(animal.used_by) == ["TestCat", "TestDog"] + + cat = schema.get(name="TestCat") + dog = schema.get(name="TestDog") + person = schema.get(name="TestPerson") + + # cat human friendly ID remains None b/c uniqueness_constraint has multiple values + assert cat.human_friendly_id is None + assert cat.uniqueness_constraints == [["name__value", "owner"]] + # dog human friendly ID is set to name__value b/c there is a uniqueness constraint with 1 attribute value + assert dog.uniqueness_constraints == [["name__value"]] + assert dog.human_friendly_id == ["name__value"] + # person human friendly ID and uniqueness_constraints remain as they were set + assert person.human_friendly_id == ["name__value", "other_name__value"] + assert person.uniqueness_constraints == [["name__value"]] + + async def test_schema_branch_process_branch_support(schema_all_in_one): schema = SchemaBranch(cache={}, name="test") schema.load_schema(schema=SchemaRoot(**schema_all_in_one)) From 1e0a87067304ea247373ec7cef1b5d2b8cd76364 Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Thu, 12 Sep 2024 16:10:34 -0700 Subject: [PATCH 2/3] add changelog --- changelog/4174.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/4174.fixed.md diff --git a/changelog/4174.fixed.md b/changelog/4174.fixed.md new file mode 100644 index 0000000000..cf97067df6 --- /dev/null +++ b/changelog/4174.fixed.md @@ -0,0 +1 @@ +Infer human-friendly ID for a schema if it includes a uniqueness constraint of a single attribute \ No newline at end of file From 8d72fe271a1c5168d4f57b636027c8f21ed1136a Mon Sep 17 00:00:00 2001 From: Aaron McCarty Date: Thu, 12 Sep 2024 18:15:49 -0700 Subject: [PATCH 3/3] IFC-581 include uniqueness_constraints when validating hfid --- backend/infrahub/core/schema_manager.py | 35 +++++----- .../schema_manager/test_manager_schema.py | 70 +++++++++++++++++++ changelog/4181.fixed.md | 1 + 3 files changed, 89 insertions(+), 17 deletions(-) create mode 100644 changelog/4181.fixed.md diff --git a/backend/infrahub/core/schema_manager.py b/backend/infrahub/core/schema_manager.py index 9c06a3b9bf..a8b43e6111 100644 --- a/backend/infrahub/core/schema_manager.py +++ b/backend/infrahub/core/schema_manager.py @@ -732,9 +732,10 @@ def validate_default_values(self): f"{node_schema.namespace}{node_schema.name}: default value {exc.message}" ) from exc - def validate_human_friendly_id(self) -> None: + def validate_human_friendly_id(self) -> None: # pylint: disable=too-many-branches for name in self.generic_names + self.node_names: node_schema = self.get(name=name, duplicate=False) + hf_attr_names = set() if not node_schema.human_friendly_id: continue @@ -759,6 +760,7 @@ def validate_human_friendly_id(self) -> None: f"{node_schema.kind} HFID is invalid at attribute '{schema_path.attribute_schema.name}', it must end with one of the " f"following properties: {', '.join(required_properties)}" ) + hf_attr_names.add(schema_path.attribute_schema.name) if schema_path.is_type_relationship and schema_path.relationship_schema: if schema_path.relationship_schema.optional and not ( @@ -771,22 +773,21 @@ def validate_human_friendly_id(self) -> None: f"{schema_path.relationship_schema.name} is not mandatory on {schema_path.relationship_schema.kind} for " f"{node_schema.kind}. ({item})" ) - # if not schema_path.attribute_schema.unique: - # raise ValueError( - # f"Only unique attribute on related node can be used used in human_friendly_id, " - # f"{schema_path.attribute_schema.name} is not unique on {schema_path.relationship_schema.kind} for " - # f"{node_schema.kind}. ({item})" - # ) - - # if ( - # schema_path.is_type_attribute - # and len(node_schema.human_friendly_id) == 1 - # and not schema_path.attribute_schema.unique - # ): - # raise ValueError( - # f"Only unique attribute can be used on their own in human_friendly_id, " - # f"{schema_path.attribute_schema.name} is not unique for {node_schema.kind}. ({item})" - # ) + + # check for uniqueness_constraint with a single attribute + if not has_unique_item and node_schema.uniqueness_constraints: + for constraint_paths in node_schema.uniqueness_constraints: + if len(constraint_paths) != 1: + continue + constraint_path = constraint_paths[0] + schema_attribute_path = node_schema.parse_schema_path(path=constraint_path, schema=self) + if ( + schema_attribute_path.is_type_attribute + and schema_attribute_path.attribute_schema + and schema_attribute_path.attribute_schema.name in hf_attr_names + ): + has_unique_item = True + break if not has_unique_item: raise ValueError( diff --git a/backend/tests/unit/core/schema_manager/test_manager_schema.py b/backend/tests/unit/core/schema_manager/test_manager_schema.py index 2dfd8682e3..0e1deda058 100644 --- a/backend/tests/unit/core/schema_manager/test_manager_schema.py +++ b/backend/tests/unit/core/schema_manager/test_manager_schema.py @@ -185,6 +185,76 @@ async def test_schema_branch_process_inheritance_update_inherited_elements(anima assert dog.get_relationship(name="owner").optional is True +@pytest.mark.parametrize( + ["uniqueness_constraints", "unique_attributes", "human_friendly_id"], + [ + (None, [], ["name__value"]), + ([["breed__value"]], [], ["name__value"]), + (None, ["breed"], ["name__value"]), + ([["name__value", "breed__value"]], ["breed"], ["name__value"]), + ], +) +async def test_validate_human_friendly_id_uniqueness_failure( + uniqueness_constraints: list[list[str]] | None, + unique_attributes: list[str], + human_friendly_id: list[str] | None, + animal_person_schema_dict, +): + schema = SchemaBranch(cache={}, name="test") + for node_schema in animal_person_schema_dict["nodes"]: + if node_schema["name"] == "Animal" and node_schema["namespace"] == "Test": + node_schema["uniqueness_constraints"] = None + node_schema["human_friendly_id"] = None + if node_schema["name"] == "Dog" and node_schema["namespace"] == "Test": + node_schema["uniqueness_constraints"] = uniqueness_constraints + node_schema["human_friendly_id"] = human_friendly_id + for attr_schema in node_schema["attributes"]: + attr_schema["unique"] = attr_schema["name"] in unique_attributes + schema.load_schema(schema=SchemaRoot(**animal_person_schema_dict)) + + schema.process_inheritance() + with pytest.raises(ValueError, match=r"At least one attribute must be unique in the human_friendly_id"): + schema.validate_human_friendly_id() + + +@pytest.mark.parametrize( + ["uniqueness_constraints", "unique_attributes", "human_friendly_id"], + [ + (None, ["name"], ["name__value"]), + (None, ["name"], ["name__value", "breed__value"]), + ([["name__value"]], [], ["name__value", "breed__value"]), + ([["name__value", "owner"], ["breed__value"]], [], ["name__value", "breed__value"]), + ], +) +async def test_validate_human_friendly_id_uniqueness_success( + uniqueness_constraints: list[list[str]] | None, + unique_attributes: list[str], + human_friendly_id: list[str] | None, + animal_person_schema_dict, +): + schema = SchemaBranch(cache={}, name="test") + for node_schema in animal_person_schema_dict["generics"]: + if node_schema["name"] == "Animal" and node_schema["namespace"] == "Test": + node_schema["uniqueness_constraints"] = None + node_schema["human_friendly_id"] = None + for attr_schema in node_schema["attributes"]: + attr_schema["unique"] = attr_schema["name"] in unique_attributes + for node_schema in animal_person_schema_dict["nodes"]: + if node_schema["name"] == "Dog" and node_schema["namespace"] == "Test": + node_schema["uniqueness_constraints"] = uniqueness_constraints + node_schema["human_friendly_id"] = human_friendly_id + for attr_schema in node_schema["attributes"]: + attr_schema["unique"] = attr_schema["name"] in unique_attributes + schema.load_schema(schema=SchemaRoot(**animal_person_schema_dict)) + + schema.process_inheritance() + schema.validate_human_friendly_id() + + dog_schema = schema.get("TestDog", duplicate=False) + assert dog_schema.uniqueness_constraints == uniqueness_constraints + assert dog_schema.human_friendly_id == human_friendly_id + + async def test_schema_branch_process_human_friendly_id(animal_person_schema_dict): schema = SchemaBranch(cache={}, name="test") schema.load_schema(schema=SchemaRoot(**animal_person_schema_dict)) diff --git a/changelog/4181.fixed.md b/changelog/4181.fixed.md new file mode 100644 index 0000000000..352273a5dd --- /dev/null +++ b/changelog/4181.fixed.md @@ -0,0 +1 @@ +Account for uniqueness constraints of a single attribute when validating human-friendly ID \ No newline at end of file