Lock names include attributes values

Author: LucasG0

backend/infrahub/core/node/create.py

@@ -7,6 +7,7 @@
 from infrahub.core.constraint.node.runner import NodeConstraintRunner
 from infrahub.core.manager import NodeManager
 from infrahub.core.node import Node
+from infrahub.core.node.save import run_constraints_and_save
 from infrahub.core.protocols import CoreObjectTemplate
 from infrahub.dependencies.registry import get_component_registry
 
@@ -151,8 +152,15 @@ async def _do_create_node(
 ) -> Node:
     obj = await node_class.init(db=db, schema=schema, branch=branch)
     await obj.new(db=db, **data)
-    await node_constraint_runner.check(node=obj, field_filters=fields_to_validate)
-    await obj.save(db=db)
+
+    await run_constraints_and_save(
+        node=obj,
+        node_constraint_runner=node_constraint_runner,
+        fields_to_validate=fields_to_validate,
+        fields_to_save=[],
+        db=db,
+        branch=branch,
+    )
 
     object_template = await obj.get_object_template(db=db)
     if object_template:

backend/infrahub/core/node/save.py

@@ -0,0 +1,25 @@
+from infrahub import lock
+from infrahub.core.branch import Branch
+from infrahub.core.constraint.node.runner import NodeConstraintRunner
+from infrahub.core.node import Node
+from infrahub.database import InfrahubDatabase
+from infrahub.lock import InfrahubMultiLock
+from infrahub.lock_utils import _get_lock_names_on_object_mutation
+
+
+async def run_constraints_and_save(
+    node: Node,
+    node_constraint_runner: NodeConstraintRunner,
+    fields_to_validate: list[str],
+    fields_to_save: list[str],
+    db: InfrahubDatabase,
+    branch: Branch,
+    skip_uniqueness_check: bool = False,
+) -> None:
+    schema_branch = db.schema.get_schema_branch(name=branch.name)
+    lock_names = _get_lock_names_on_object_mutation(node=node, branch=branch, schema_branch=schema_branch)
+    async with InfrahubMultiLock(lock_registry=lock.registry, locks=lock_names):
+        await node_constraint_runner.check(
+            node=node, field_filters=fields_to_validate, skip_uniqueness_check=skip_uniqueness_check
+        )
+        await node.save(db=db, fields=fields_to_save)

backend/infrahub/graphql/mutations/ipam.py

@@ -118,9 +118,16 @@ async def _mutate_create_object_and_reconcile(
     ) -> Node:
         address = await cls.mutate_create_object(data=data, db=db, branch=branch)
         reconciler = IpamReconciler(db=db, branch=branch)
-        reconciled_address = await reconciler.reconcile(
-            ip_value=ip_address, namespace=namespace_id, node_uuid=address.get_id()
-        )
+
+        if lock_name := cls._get_lock_name(namespace_id, branch):
+            async with InfrahubMultiLock(lock_registry=lock.registry, locks=[lock_name]):
+                reconciled_address = await reconciler.reconcile(
+                    ip_value=ip_address, namespace=namespace_id, node_uuid=address.get_id()
+                )
+        else:
+            reconciled_address = await reconciler.reconcile(
+                ip_value=ip_address, namespace=namespace_id, node_uuid=address.get_id()
+            )
         return reconciled_address
 
     @classmethod
@@ -138,15 +145,9 @@ async def mutate_create(
         namespace_id = await validate_namespace(db=db, branch=branch, data=data)
 
         async with db.start_transaction() as dbt:
-            if lock_name := cls._get_lock_name(namespace_id, branch):
-                async with InfrahubMultiLock(lock_registry=lock.registry, locks=[lock_name]):
-                    reconciled_address = await cls._mutate_create_object_and_reconcile(
-                        data=data, branch=branch, db=dbt, ip_address=ip_address, namespace_id=namespace_id
-                    )
-            else:
-                reconciled_address = await cls._mutate_create_object_and_reconcile(
-                    data=data, branch=branch, db=dbt, ip_address=ip_address, namespace_id=namespace_id
-                )
+            reconciled_address = await cls._mutate_create_object_and_reconcile(
+                data=data, branch=branch, db=dbt, ip_address=ip_address, namespace_id=namespace_id
+            )
             result = await cls.mutate_create_to_graphql(info=info, db=dbt, obj=reconciled_address)
 
         return reconciled_address, result
@@ -164,9 +165,15 @@ async def _mutate_update_object_and_reconcile(
         address = await cls.mutate_update_object(db=db, info=info, data=data, branch=branch, obj=address)
         reconciler = IpamReconciler(db=db, branch=branch)
         ip_address = ipaddress.ip_interface(address.address.value)
-        reconciled_address = await reconciler.reconcile(
-            ip_value=ip_address, node_uuid=address.get_id(), namespace=namespace_id
-        )
+        if lock_name := cls._get_lock_name(namespace_id, branch):
+            async with InfrahubMultiLock(lock_registry=lock.registry, locks=[lock_name]):
+                reconciled_address = await reconciler.reconcile(
+                    ip_value=ip_address, node_uuid=address.get_id(), namespace=namespace_id
+                )
+        else:
+            reconciled_address = await reconciler.reconcile(
+                ip_value=ip_address, node_uuid=address.get_id(), namespace=namespace_id
+            )
         return reconciled_address
 
     @classmethod
@@ -194,16 +201,9 @@ async def mutate_update(
         namespace_id = await validate_namespace(db=db, branch=branch, data=data, existing_namespace_id=namespace.id)
 
         async with db.start_transaction() as dbt:
-            if lock_name := cls._get_lock_name(namespace_id, branch):
-                async with InfrahubMultiLock(lock_registry=lock.registry, locks=[lock_name]):
-                    reconciled_address = await cls._mutate_update_object_and_reconcile(
-                        info=info, data=data, branch=branch, address=address, namespace_id=namespace_id, db=dbt
-                    )
-            else:
-                reconciled_address = await cls._mutate_update_object_and_reconcile(
-                    info=info, data=data, branch=branch, address=address, namespace_id=namespace_id, db=dbt
-                )
-
+            reconciled_address = await cls._mutate_update_object_and_reconcile(
+                info=info, data=data, branch=branch, address=address, namespace_id=namespace_id, db=dbt
+            )
             result = await cls.mutate_update_to_graphql(db=dbt, info=info, obj=reconciled_address)
 
         return address, result
@@ -289,12 +289,9 @@ async def mutate_create(
         namespace_id = await validate_namespace(db=db, branch=branch, data=data)
 
         async with db.start_transaction() as dbt:
-            lock_name = cls._get_lock_name(namespace_id)
-            async with InfrahubMultiLock(lock_registry=lock.registry, locks=[lock_name]):
-                reconciled_prefix = await cls._mutate_create_object_and_reconcile(
-                    data=data, branch=branch, db=dbt, namespace_id=namespace_id
-                )
-
+            reconciled_prefix = await cls._mutate_create_object_and_reconcile(
+                data=data, branch=branch, db=dbt, namespace_id=namespace_id
+            )
             result = await cls.mutate_create_to_graphql(info=info, db=dbt, obj=reconciled_prefix)
 
         return reconciled_prefix, result
@@ -339,11 +336,9 @@ async def mutate_update(
         namespace_id = await validate_namespace(db=db, branch=branch, data=data, existing_namespace_id=namespace.id)
 
         async with db.start_transaction() as dbt:
-            lock_name = cls._get_lock_name(namespace_id)
-            async with InfrahubMultiLock(lock_registry=lock.registry, locks=[lock_name]):
-                reconciled_prefix = await cls._mutate_update_object_and_reconcile(
-                    info=info, data=data, prefix=prefix, db=dbt, namespace_id=namespace_id, branch=branch
-                )
+            reconciled_prefix = await cls._mutate_update_object_and_reconcile(
+                info=info, data=data, prefix=prefix, db=dbt, namespace_id=namespace_id, branch=branch
+            )
             result = await cls.mutate_update_to_graphql(db=dbt, info=info, obj=reconciled_prefix)
 
         return prefix, result
@@ -378,9 +373,15 @@ async def _reconcile_prefix(
     ) -> Node:
         reconciler = IpamReconciler(db=db, branch=branch)
         ip_network = ipaddress.ip_network(prefix.prefix.value)
-        reconciled_prefix = await reconciler.reconcile(
-            ip_value=ip_network, node_uuid=prefix.get_id(), namespace=namespace_id, is_delete=is_delete
-        )
+        if lock_name := cls._get_lock_name(namespace_id):
+            async with InfrahubMultiLock(lock_registry=lock.registry, locks=[lock_name]):
+                reconciled_prefix = await reconciler.reconcile(
+                    ip_value=ip_network, node_uuid=prefix.get_id(), namespace=namespace_id, is_delete=is_delete
+                )
+        else:
+            reconciled_prefix = await reconciler.reconcile(
+                ip_value=ip_network, node_uuid=prefix.get_id(), namespace=namespace_id, is_delete=is_delete
+            )
         return reconciled_prefix
 
     @classmethod
@@ -404,11 +405,9 @@ async def mutate_delete(
         namespace_id = namespace_rels[0].peer_id
 
         async with graphql_context.db.start_transaction() as dbt:
-            lock_name = cls._get_lock_name(namespace_id)
-            async with InfrahubMultiLock(lock_registry=lock.registry, locks=[lock_name]):
-                reconciled_prefix = await cls._reconcile_prefix(
-                    branch=branch, db=dbt, prefix=prefix, namespace_id=namespace_id, is_delete=True
-                )
+            reconciled_prefix = await cls._reconcile_prefix(
+                branch=branch, db=dbt, prefix=prefix, namespace_id=namespace_id, is_delete=True
+            )
         ok = True
 
         return DeleteResult(node=reconciled_prefix, mutation=cls(ok=ok))

backend/infrahub/graphql/mutations/main.py

@@ -8,8 +8,8 @@
 from infrahub_sdk.utils import extract_fields
 from typing_extensions import Self
 
-from infrahub import config, lock
-from infrahub.core.constants import InfrahubKind, MutationAction
+from infrahub import config
+from infrahub.core.constants import MutationAction
 from infrahub.core.constraint.node.runner import NodeConstraintRunner
 from infrahub.core.manager import NodeManager
 from infrahub.core.node.create import (
@@ -27,17 +27,16 @@
 from infrahub.events.generator import generate_node_mutation_events
 from infrahub.exceptions import HFIDViolatedError, InitializationError, NodeNotFoundError
 from infrahub.graphql.context import apply_external_context
-from infrahub.lock import InfrahubMultiLock, build_object_lock_name
 from infrahub.log import get_log_data, get_logger
 
+from ...core.node.save import run_constraints_and_save
 from .node_getter.by_default_filter import MutationNodeGetterByDefaultFilter
 
 if TYPE_CHECKING:
     from graphql import GraphQLResolveInfo
 
     from infrahub.core.branch import Branch
     from infrahub.core.node import Node
-    from infrahub.core.schema.schema_branch import SchemaBranch
     from infrahub.database import InfrahubDatabase
     from infrahub.graphql.types.context import ContextInput
 
@@ -46,8 +45,6 @@
 
 log = get_logger()
 
-KINDS_CONCURRENT_MUTATIONS_NOT_ALLOWED = [InfrahubKind.GENERICGROUP]
-
 
 @dataclass
 class DeleteResult:
@@ -150,14 +147,6 @@ async def _call_mutate_create_object(cls, data: InputObjectType, db: InfrahubDat
         """
         Wrapper around mutate_create_object to potentially activate locking.
         """
-        schema_branch = db.schema.get_schema_branch(name=branch.name)
-        lock_names = _get_kind_lock_names_on_object_mutation(
-            kind=cls._meta.active_schema.kind, branch=branch, schema_branch=schema_branch
-        )
-        if lock_names:
-            async with InfrahubMultiLock(lock_registry=lock.registry, locks=lock_names):
-                return await cls.mutate_create_object(data=data, db=db, branch=branch)
-
         return await cls.mutate_create_object(data=data, db=db, branch=branch)
 
     @classmethod
@@ -214,39 +203,22 @@ async def _call_mutate_update(
         Wrapper around mutate_update to potentially activate locking and call it within a database transaction.
         """
 
-        schema_branch = db.schema.get_schema_branch(name=branch.name)
-        lock_names = _get_kind_lock_names_on_object_mutation(
-            kind=cls._meta.active_schema.kind, branch=branch, schema_branch=schema_branch
-        )
-
         if db.is_transaction:
-            if lock_names:
-                async with InfrahubMultiLock(lock_registry=lock.registry, locks=lock_names):
-                    obj = await cls.mutate_update_object(
-                        db=db, info=info, data=data, branch=branch, obj=obj, skip_uniqueness_check=skip_uniqueness_check
-                    )
-            else:
-                obj = await cls.mutate_update_object(
-                    db=db, info=info, data=data, branch=branch, obj=obj, skip_uniqueness_check=skip_uniqueness_check
-                )
+            obj = await cls.mutate_update_object(
+                db=db, info=info, data=data, branch=branch, obj=obj, skip_uniqueness_check=skip_uniqueness_check
+            )
             result = await cls.mutate_update_to_graphql(db=db, info=info, obj=obj)
             return obj, result
 
         async with db.start_transaction() as dbt:
-            if lock_names:
-                async with InfrahubMultiLock(lock_registry=lock.registry, locks=lock_names):
-                    obj = await cls.mutate_update_object(
-                        db=dbt,
-                        info=info,
-                        data=data,
-                        branch=branch,
-                        obj=obj,
-                        skip_uniqueness_check=skip_uniqueness_check,
-                    )
-            else:
-                obj = await cls.mutate_update_object(
-                    db=dbt, info=info, data=data, branch=branch, obj=obj, skip_uniqueness_check=skip_uniqueness_check
-                )
+            obj = await cls.mutate_update_object(
+                db=dbt,
+                info=info,
+                data=data,
+                branch=branch,
+                obj=obj,
+                skip_uniqueness_check=skip_uniqueness_check,
+            )
             result = await cls.mutate_update_to_graphql(db=dbt, info=info, obj=obj)
             return obj, result
 
@@ -287,16 +259,21 @@ async def mutate_update_object(
         before_mutate_profile_ids = await get_profile_ids(db=db, obj=obj)
         await obj.from_graphql(db=db, data=data)
         fields_to_validate = list(data)
-        await node_constraint_runner.check(
-            node=obj, field_filters=fields_to_validate, skip_uniqueness_check=skip_uniqueness_check
-        )
-
         fields = list(data.keys())
+
         for field_to_remove in ("id", "hfid"):
             if field_to_remove in fields:
                 fields.remove(field_to_remove)
 
-        await obj.save(db=db, fields=fields)
+        await run_constraints_and_save(
+            node=obj,
+            node_constraint_runner=node_constraint_runner,
+            fields_to_validate=fields_to_validate,
+            fields_to_save=fields,
+            db=db,
+            skip_uniqueness_check=skip_uniqueness_check,
+            branch=branch,
+        )
 
         obj = await refresh_for_profile_update(
             db=db,
@@ -465,71 +442,5 @@ def __init_subclass_with_meta__(
         super().__init_subclass_with_meta__(_meta=_meta, **options)
 
 
-def _get_kinds_to_lock_on_object_mutation(kind: str, schema_branch: SchemaBranch) -> list[str]:
-    """
-    Return kinds for which we want to lock during creating / updating an object of a given schema node.
-    Lock should be performed on schema kind and its generics having a uniqueness_constraint defined.
-    If a generic uniqueness constraint is the same as the node schema one,
-    it means node schema overrided this constraint, in which case we only need to lock on the generic.
-    """
-
-    node_schema = schema_branch.get(name=kind)
-
-    schema_uc = None
-    kinds = []
-    if node_schema.uniqueness_constraints:
-        kinds.append(node_schema.kind)
-        schema_uc = node_schema.uniqueness_constraints
-
-    if node_schema.is_generic_schema:
-        return kinds
-
-    generics_kinds = node_schema.inherit_from
-
-    node_schema_kind_removed = False
-    for generic_kind in generics_kinds:
-        generic_uc = schema_branch.get(name=generic_kind).uniqueness_constraints
-        if generic_uc:
-            kinds.append(generic_kind)
-            if not node_schema_kind_removed and generic_uc == schema_uc:
-                # Check whether we should remove original schema kind as it simply overrides uniqueness_constraint
-                # of a generic
-                kinds.pop(0)
-                node_schema_kind_removed = True
-    return kinds
-
-
-def _should_kind_be_locked_on_any_branch(kind: str, schema_branch: SchemaBranch) -> bool:
-    """
-    Check whether kind or any kind generic is in KINDS_TO_LOCK_ON_ANY_BRANCH.
-    """
-
-    if kind in KINDS_CONCURRENT_MUTATIONS_NOT_ALLOWED:
-        return True
-
-    node_schema = schema_branch.get(name=kind)
-    if node_schema.is_generic_schema:
-        return False
-
-    for generic_kind in node_schema.inherit_from:
-        if generic_kind in KINDS_CONCURRENT_MUTATIONS_NOT_ALLOWED:
-            return True
-    return False
-
-
-def _get_kind_lock_names_on_object_mutation(kind: str, branch: Branch, schema_branch: SchemaBranch) -> list[str]:
-    """
-    Return objects kind for which we want to avoid concurrent mutation (create/update). Except for some specific kinds,
-    concurrent mutations are only allowed on non-main branch as objects validations will be performed at least when merging in main branch.
-    """
-
-    if not branch.is_default and not _should_kind_be_locked_on_any_branch(kind, schema_branch):
-        return []
-
-    lock_kinds = _get_kinds_to_lock_on_object_mutation(kind, schema_branch)
-    lock_names = [build_object_lock_name(kind) for kind in lock_kinds]
-    return lock_names
-
-
 def _get_data_fields(data: InputObjectType) -> list[str]:
     return [field for field in data.keys() if field not in ["id", "hfid"]]