diff --git a/backend/infrahub/core/migrations/query/attribute_add.py b/backend/infrahub/core/migrations/query/attribute_add.py index d34a8e285d..f55dc84b2f 100644 --- a/backend/infrahub/core/migrations/query/attribute_add.py +++ b/backend/infrahub/core/migrations/query/attribute_add.py @@ -37,6 +37,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> No self.params["attr_name"] = self.attribute_name self.params["attr_type"] = self.attribute_kind self.params["branch_support"] = self.branch_support + self.params["current_time"] = self.at.to_string() if self.default_value is not None: self.params["attr_value"] = self.default_value @@ -55,28 +56,34 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> No query = """ MATCH p = (n:%(node_kind)s) - WHERE NOT exists((n)-[:HAS_ATTRIBUTE]-(:Attribute { name: $attr_name })) CALL { WITH n - MATCH (root:Root)<-[r:IS_PART_OF]-(n) - WHERE %(branch_filter)s - RETURN n as n1, r as r1 - ORDER BY r.branch_level DESC, r.from DESC + MATCH (root:Root)<-[r1:IS_PART_OF]-(n) + OPTIONAL MATCH (n)-[r2:HAS_ATTRIBUTE]-(:Attribute { name: $attr_name }) + WHERE all(r in [r1, r2] WHERE (%(branch_filter)s)) + RETURN n as n1, r1 as r11, r2 as r12 + ORDER BY r2.branch_level DESC, r2.from ASC, r1.branch_level DESC, r1.from ASC LIMIT 1 } - WITH n1 as n, r1 as rb - WHERE rb.status = "active" + WITH n1 as n, r11 as r1, r12 as r2 + WHERE r1.status = "active" AND (r2 IS NULL OR r2.status = "deleted") MERGE (av:AttributeValue { value: $attr_value, is_default: true }) MERGE (is_protected_value:Boolean { value: $is_protected_default }) MERGE (is_visible_value:Boolean { value: $is_visible_default }) - WITH n, av, is_protected_value, is_visible_value + WITH n, av, is_protected_value, is_visible_value, r2 CREATE (a:Attribute { name: $attr_name, branch_support: $branch_support }) CREATE (n)-[:HAS_ATTRIBUTE $rel_props ]->(a) CREATE (a)-[:HAS_VALUE $rel_props ]->(av) CREATE (a)-[:IS_PROTECTED $rel_props]->(is_protected_value) CREATE (a)-[:IS_VISIBLE $rel_props]->(is_visible_value) - """ % {"branch_filter": branch_filter, "node_kind": self.node_kind} + %(uuid_generation)s + FOREACH (i in CASE WHEN r2.status = "deleted" THEN [1] ELSE [] END | + SET r2.to = $current_time + ) + """ % { + "branch_filter": branch_filter, + "node_kind": self.node_kind, + "uuid_generation": db.render_uuid_generation(node_label="a", node_attr="uuid"), + } self.add_to_query(query) self.return_labels = ["n.uuid", "a.uuid"] - - self.add_to_query(db.render_uuid_generation(node_label="a", node_attr="uuid")) diff --git a/backend/tests/integration/schema_lifecycle/shared.py b/backend/tests/integration/schema_lifecycle/shared.py index ca43eb61de..12b8c08887 100644 --- a/backend/tests/integration/schema_lifecycle/shared.py +++ b/backend/tests/integration/schema_lifecycle/shared.py @@ -1,3 +1,4 @@ +import copy from typing import Any, Dict import pytest @@ -40,7 +41,7 @@ def schema_person_02_first_last(self, schema_person_base) -> Dict[str, Any]: @pytest.fixture(scope="class") def schema_person_03_no_height(self, schema_person_02_first_last) -> Dict[str, Any]: """Remove the attribute height.""" - person = schema_person_02_first_last + person = copy.deepcopy(schema_person_02_first_last) assert person["attributes"][2]["name"] == "height" person["attributes"][2]["state"] = "absent" return person diff --git a/backend/tests/integration/schema_lifecycle/test_schema_attribute_remove_add.py b/backend/tests/integration/schema_lifecycle/test_schema_attribute_remove_add.py new file mode 100644 index 0000000000..07d14e3e0d --- /dev/null +++ b/backend/tests/integration/schema_lifecycle/test_schema_attribute_remove_add.py @@ -0,0 +1,211 @@ +from typing import Any, Optional + +import pytest +from infrahub_sdk import InfrahubClient + +from infrahub.core import registry +from infrahub.core.branch import Branch +from infrahub.core.node import Node +from infrahub.database import InfrahubDatabase +from infrahub.exceptions import InitializationError + +from ..shared import load_schema +from .shared import ( + CAR_KIND, + MANUFACTURER_KIND_01, + PERSON_KIND, + TAG_KIND, + TestSchemaLifecycleBase, +) + +# pylint: disable=unused-argument + + +class BranchState: + def __init__(self) -> None: + self._branch: Optional[Branch] = None + + @property + def branch(self) -> Branch: + if self._branch: + return self._branch + raise InitializationError + + @branch.setter + def branch(self, value: Branch) -> None: + self._branch = value + + +state = BranchState() + + +# --------------------------------- +# This test was initially written to troubleshoot and fix https://github.com/opsmill/infrahub/issues/4727 +# The issue was primarily happening in Main +# --------------------------------- +class TestSchemaLifecycleAttributeRemoveAddMain(TestSchemaLifecycleBase): + @property + def branch1(self) -> Branch: + return state.branch + + @pytest.fixture(scope="class") + async def initial_dataset(self, db: InfrahubDatabase, initialize_registry, schema_step01): + await load_schema(db=db, schema=schema_step01) + + # Load data in the MAIN branch first + john = await Node.init(schema=PERSON_KIND, db=db) + await john.new(db=db, firstname="John", lastname="Doe", height=175, description="The famous Joe Doe") + await john.save(db=db) + + renault = await Node.init(schema=MANUFACTURER_KIND_01, db=db) + await renault.new( + db=db, name="renault", description="Groupe Renault is a French multinational automobile manufacturer" + ) + await renault.save(db=db) + + megane = await Node.init(schema=CAR_KIND, db=db) + await megane.new( + db=db, name="Megane", description="Renault Megane", color="#c93420", manufacturer=renault, owner=john + ) + await megane.save(db=db) + + clio = await Node.init(schema=CAR_KIND, db=db) + await clio.new( + db=db, name="Clio", description="Renault Clio", color="#ff3420", manufacturer=renault, owner=john + ) + await clio.save(db=db) + + red = await Node.init(schema=TAG_KIND, db=db) + await red.new(db=db, name="red", persons=[john]) + await red.save(db=db) + + objs = { + "john": john.id, + "renault": renault.id, + "megane": megane.id, + "clio": clio.id, + "red": red.id, + } + + return objs + + @pytest.fixture(scope="class") + def schema_step01( + self, schema_car_base, schema_person_02_first_last, schema_manufacturer_base, schema_tag_base + ) -> dict[str, Any]: + return { + "version": "1.0", + "nodes": [schema_person_02_first_last, schema_car_base, schema_manufacturer_base, schema_tag_base], + } + + @pytest.fixture(scope="class") + def schema_step02( + self, schema_car_base, schema_person_03_no_height, schema_manufacturer_base, schema_tag_base + ) -> dict[str, Any]: + return { + "version": "1.0", + "nodes": [schema_person_03_no_height, schema_car_base, schema_manufacturer_base, schema_tag_base], + } + + @pytest.fixture(scope="class") + def schema_step03( + self, schema_car_base, schema_person_02_first_last, schema_manufacturer_base, schema_tag_base + ) -> dict[str, Any]: + return { + "version": "1.0", + "nodes": [ + schema_person_02_first_last, + schema_car_base, + schema_manufacturer_base, + schema_tag_base, + ], + } + + async def test_step01_baseline_backend(self, db: InfrahubDatabase, initial_dataset): + persons = await registry.manager.query(db=db, schema=PERSON_KIND) + assert len(persons) == 1 + + async def test_step02_check_attr_add_rename( + self, db: InfrahubDatabase, client: InfrahubClient, initial_dataset, schema_step02 + ): + success, response = await client.schema.check(schemas=[schema_step02]) + assert success + assert response == { + "diff": { + "added": {}, + "changed": { + "TestingPerson": { + "added": {}, + "changed": { + "attributes": { + "added": {}, + "changed": {}, + "removed": {"height": None}, + }, + }, + "removed": {}, + }, + }, + "removed": {}, + }, + } + + async def test_step02_load(self, db: InfrahubDatabase, client: InfrahubClient, initial_dataset, schema_step02): + response = await client.schema.load(schemas=[schema_step02]) + assert not response.errors + + # Ensure that we can query the nodes with the new schema in BRANCH1 + persons = await registry.manager.query( + db=db, + schema=PERSON_KIND, + filters={"firstname__value": "John"}, # , branch=self.branch1 + ) + assert len(persons) == 1 + john = persons[0] + assert john.firstname.value == "John" # type: ignore[attr-defined] + assert not hasattr(john, "height") + + async def test_step03_check(self, db: InfrahubDatabase, client: InfrahubClient, initial_dataset, schema_step03): + success, response = await client.schema.check(schemas=[schema_step03]) + assert response == { + "diff": { + "added": {}, + "changed": { + "TestingPerson": { + "added": {}, + "changed": { + "attributes": {"added": {"height": None}, "changed": {}, "removed": {}}, + }, + "removed": {}, + }, + }, + "removed": {}, + }, + } + assert success + + async def test_step03_load(self, db: InfrahubDatabase, client: InfrahubClient, initial_dataset, schema_step03): + response = await client.schema.load(schemas=[schema_step03]) + assert not response.errors + + # Modify the value for Height in the database + persons = await registry.manager.query( + db=db, + schema=PERSON_KIND, + filters={"firstname__value": "John"}, + ) + assert len(persons) == 1 + john = persons[0] + assert john.height.value is None + john.height.value = 200 + await john.save(db=db) + + # Validate that the new value has been properly saved + persons2 = await registry.manager.query( + db=db, + schema=PERSON_KIND, + filters={"firstname__value": "John"}, + ) + assert len(persons2) == 1 + john2 = persons2[0] + assert john2.height.value == 200 diff --git a/backend/tests/unit/core/migrations/schema/test_node_attribute_add.py b/backend/tests/unit/core/migrations/schema/test_node_attribute_add.py index 34ef2a0964..91addd687c 100644 --- a/backend/tests/unit/core/migrations/schema/test_node_attribute_add.py +++ b/backend/tests/unit/core/migrations/schema/test_node_attribute_add.py @@ -4,11 +4,17 @@ from infrahub_sdk import InfrahubClient from infrahub_sdk.uuidt import UUIDT -from infrahub.core.constants import SchemaPathType +from infrahub.core import registry +from infrahub.core.branch import Branch +from infrahub.core.constants import HashableModelState, SchemaPathType from infrahub.core.migrations.schema.node_attribute_add import ( NodeAttributeAddMigration, NodeAttributeAddMigrationQuery01, ) +from infrahub.core.migrations.schema.node_attribute_remove import ( + NodeAttributeRemoveMigration, + NodeAttributeRemoveMigrationQuery01, +) from infrahub.core.path import SchemaPath from infrahub.core.schema import NodeSchema from infrahub.core.timestamp import Timestamp @@ -88,6 +94,54 @@ async def test_query01(db: InfrahubDatabase, default_branch, init_database, sche assert await count_nodes(db=db, label="Attribute") == 5 +async def test_query01_re_add(db: InfrahubDatabase, default_branch: Branch, car_accord_main, car_camry_main): + schema = registry.schema.get_schema_branch(name=default_branch.name) + + assert await count_nodes(db=db, label="TestCar") == 2 + assert await count_nodes(db=db, label="Attribute") == 14 + + # ------------------------------------------ + # Delete the attribute Color + # ------------------------------------------ + candidate_schema = schema.duplicate() + car_schema = candidate_schema.get_node(name="TestCar") + attr = car_schema.get_attribute(name="color") + attr.state = HashableModelState.ABSENT + + migration_remove = NodeAttributeRemoveMigration( + previous_node_schema=schema.get_node(name="TestCar"), + new_node_schema=car_schema, + schema_path=SchemaPath(path_type=SchemaPathType.ATTRIBUTE, schema_kind="TestCar", field_name="color"), + ) + query = await NodeAttributeRemoveMigrationQuery01.init(db=db, branch=default_branch, migration=migration_remove) + await query.execute(db=db) + assert query.get_nbr_migrations_executed() == 2 + + # ------------------------------------------ + # Add the attribute Color back + # ------------------------------------------ + migration_add = NodeAttributeAddMigration( + new_node_schema=schema.get_node(name="TestCar"), + previous_node_schema=car_schema, + schema_path=SchemaPath(path_type=SchemaPathType.ATTRIBUTE, schema_kind="TestCar", field_name="color"), + ) + query = await NodeAttributeAddMigrationQuery01.init(db=db, branch=default_branch, migration=migration_add) + await query.execute(db=db) + + assert query.get_nbr_migrations_executed() == 2 + + assert await count_nodes(db=db, label="TestCar") == 2 + assert await count_nodes(db=db, label="Attribute") == 16 + + # Re-execute the query once to ensure that it won't recreate the attribute twice + query = await NodeAttributeAddMigrationQuery01.init(db=db, branch=default_branch, migration=migration_add) + await query.execute(db=db) + + assert query.get_nbr_migrations_executed() == 0 + assert await count_nodes(db=db, label="TestCar") == 2 + assert await count_nodes(db=db, label="Attribute") == 16 + + async def test_migration(db: InfrahubDatabase, default_branch, init_database, schema_aware): node = schema_aware migration = NodeAttributeAddMigration(