Refactor AttributeAddQuery to support node with an attribute of the s…

…ame name previously active

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"))

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

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

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(