Make NodeId assignment during a node copy explicit

docs/NodeManagers.md

@@ -1592,6 +1592,45 @@ Notes:
   `ISystemContext.NodeIdFactory`. `AsyncCustomNodeManager` supplies one
   that allocates from the manager's namespace; override `New` to derive
   ids from the parent chain instead.
+* **A node copy never assigns.** `NodeState.Create(context, source)`
+  initialises each child from its source right after creating it, which
+  overwrites any NodeId minted along the way — so minting one would only
+  consume identifiers, and leak them for factories that track outstanding
+  allocations. The copy therefore calls
+  `CreateChild(context, browseName, assignInstanceNodeIds: false)`.
+
+#### Custom node types and assignment control
+
+`NodeState` carries a second `FindChild` overload that takes
+`assignInstanceNodeIds`, plus a `SupportsInstanceNodeIdAssignmentControl`
+property that states whether a type honours it. Source generated types
+override both, so a copy of a generated node consumes nothing.
+
+Hand-written types that override only the four argument `FindChild` keep
+working: for them the copy hides the `NodeIdFactory` for its duration, which
+is the only channel that reaches an override with no such parameter. To take
+the direct path instead, override both members:
+
+```csharp
+protected override bool SupportsInstanceNodeIdAssignmentControl => true;
+
+protected override BaseInstanceState? FindChild(
+    ISystemContext context,
+    QualifiedName browseName,
+    bool createOrReplace,
+    BaseInstanceState? replacement,
+    bool assignInstanceNodeIds)
+{
+    // ... thread assignInstanceNodeIds into your CreateOrReplace<Child> calls
+    return base.FindChild(
+        context, browseName, createOrReplace, replacement, assignInstanceNodeIds);
+}
+```
+
+Override the property only together with the five argument `FindChild`. A
+hand-written type deriving from a generated one inherits `true`; if it
+overrides only the four argument `FindChild` and needs that override to run
+during a copy, it must override the property back to `false`.
 
 ### Current limitations
 

src/Opc.Ua.Types/State/BaseDataVariableState.cs

@@ -165,6 +165,9 @@ public override void GetChildren(ISystemContext context, IList<BaseInstanceState
             base.GetChildren(context, children);
         }
 
+        /// <inheritdoc/>
+        protected override bool SupportsInstanceNodeIdAssignmentControl => true;
+
         /// <summary>
         /// Finds the child with the specified browse name.
         /// </summary>
@@ -179,15 +182,54 @@ public override void GetChildren(ISystemContext context, IList<BaseInstanceState
                 return null;
             }
 
-            BaseInstanceState? instance = null;
-            switch (browseName.Name)
+            return FindDeclaredChild(context, browseName, createOrReplace, replacement, true)
+                ?? base.FindChild(context, browseName, createOrReplace, replacement);
+        }
+
+        /// <inheritdoc/>
+        protected override BaseInstanceState? FindChild(
+            ISystemContext context,
+            QualifiedName browseName,
+            bool createOrReplace,
+            BaseInstanceState? replacement,
+            bool assignInstanceNodeIds)
+        {
+            if (browseName.IsNull)
+            {
+                return null;
+            }
+
+            return FindDeclaredChild(
+                    context, browseName, createOrReplace, replacement, assignInstanceNodeIds)
+                ?? base.FindChild(
+                    context, browseName, createOrReplace, replacement, assignInstanceNodeIds);
+        }
+
+        /// <summary>
+        /// Resolves the EnumStrings property declared by this type.
+        /// </summary>
+        /// <param name="context">The system context.</param>
+        /// <param name="browseName">The browse name to resolve.</param>
+        /// <param name="createOrReplace">Whether a missing child is created.</param>
+        /// <param name="replacement">The replacement to adopt, if any.</param>
+        /// <param name="assignInstanceNodeIds">
+        /// Whether a newly created child may be given a per-instance NodeId.
+        /// </param>
+        /// <returns>The child, or <c>null</c> when this type does not declare it.</returns>
+        private PropertyState<ArrayOf<LocalizedText>>? FindDeclaredChild(
+            ISystemContext context,
+            QualifiedName browseName,
+            bool createOrReplace,
+            BaseInstanceState? replacement,
+            bool assignInstanceNodeIds)
+        {
+            if (browseName.Name == BrowseNames.EnumStrings)
             {
-                case BrowseNames.EnumStrings:
-                    instance = !createOrReplace ?
-                        EnumStrings : CreateOrReplaceEnumStrings(context, replacement);
-                    break;
+                return !createOrReplace
+                    ? EnumStrings
+                    : CreateOrReplaceEnumStrings(context, replacement, assignInstanceNodeIds);
             }
-            return instance ?? base.FindChild(context, browseName, createOrReplace, replacement);
+            return null;
         }
 
         /// <summary>

src/Opc.Ua.Types/State/MethodState.cs

@@ -517,6 +517,9 @@ public override void GetChildren(ISystemContext context, IList<BaseInstanceState
             base.GetChildren(context, children);
         }
 
+        /// <inheritdoc/>
+        protected override bool SupportsInstanceNodeIdAssignmentControl => true;
+
         /// <inheritdoc/>
         protected override BaseInstanceState? FindChild(
             ISystemContext context,
@@ -528,19 +531,61 @@ public override void GetChildren(ISystemContext context, IList<BaseInstanceState
             {
                 return null;
             }
-            BaseInstanceState? instance = null;
+            return FindDeclaredChild(context, browseName, createOrReplace, replacement, true)
+                ?? base.FindChild(context, browseName, createOrReplace, replacement);
+        }
+
+        /// <inheritdoc/>
+        protected override BaseInstanceState? FindChild(
+            ISystemContext context,
+            QualifiedName browseName,
+            bool createOrReplace,
+            BaseInstanceState? replacement,
+            bool assignInstanceNodeIds)
+        {
+            if (browseName.IsNull)
+            {
+                return null;
+            }
+            return FindDeclaredChild(
+                    context, browseName, createOrReplace, replacement, assignInstanceNodeIds)
+                ?? base.FindChild(
+                    context, browseName, createOrReplace, replacement, assignInstanceNodeIds);
+        }
+
+        /// <summary>
+        /// Resolves one of the arguments properties declared by this type.
+        /// </summary>
+        /// <param name="context">The system context.</param>
+        /// <param name="browseName">The browse name to resolve.</param>
+        /// <param name="createOrReplace">Whether a missing child is created.</param>
+        /// <param name="replacement">The replacement to adopt, if any.</param>
+        /// <param name="assignInstanceNodeIds">
+        /// Whether a newly created child may be given a per-instance NodeId.
+        /// </param>
+        /// <returns>The child, or <c>null</c> when this type does not declare it.</returns>
+        private PropertyState<ArrayOf<Argument>>? FindDeclaredChild(
+            ISystemContext context,
+            QualifiedName browseName,
+            bool createOrReplace,
+            BaseInstanceState? replacement,
+            bool assignInstanceNodeIds)
+        {
             switch (browseName.Name)
             {
                 case BrowseNames.InputArguments:
-                    instance = !createOrReplace ?
-                        OutputArguments : CreateOrReplaceInputArguments(context, replacement);
-                    break;
+                    return !createOrReplace
+                        ? InputArguments
+                        : CreateOrReplaceInputArguments(
+                            context, replacement, assignInstanceNodeIds);
                 case BrowseNames.OutputArguments:
-                    instance = !createOrReplace ?
-                        OutputArguments : CreateOrReplaceOutputArguments(context, replacement);
-                    break;
+                    return !createOrReplace
+                        ? OutputArguments
+                        : CreateOrReplaceOutputArguments(
+                            context, replacement, assignInstanceNodeIds);
+                default:
+                    return null;
             }
-            return instance ?? base.FindChild(context, browseName, createOrReplace, replacement);
         }
 
         /// <summary>

src/Opc.Ua.Types/State/NodeIdFactorySuppressedContext.cs

@@ -36,17 +36,20 @@ namespace Opc.Ua
     /// <see cref="ISystemContext.NodeIdFactory"/>, which is reported as absent.
     /// </summary>
     /// <remarks>
-    /// A node copy materialises its children through
-    /// <see cref="NodeState.CreateChild"/>, and the
-    /// <c>CreateOrReplace&lt;Child&gt;</c> plumbing behind it assigns a
-    /// per-instance NodeId whenever the context carries a factory. In a copy
-    /// every child is initialised from its source immediately afterwards, which
-    /// overwrites that NodeId, so the assignment only consumes identifiers -
-    /// and permanently leaks them for factories that track outstanding
-    /// allocations. Hiding the factory for the duration of the copy leaves
-    /// those identifiers unused; the <c>assignInstanceNodeIds</c> flag cannot
-    /// serve here because it is not part of the virtual <c>FindChild</c>
-    /// contract the copy goes through.
+    /// Compatibility fallback for the node copy in
+    /// <see cref="NodeState.Initialize(ISystemContext, NodeState)"/>. A copy
+    /// materialises its children and then initialises each one from its source,
+    /// which overwrites whatever NodeId was assigned along the way, so assigning
+    /// one only consumes identifiers - and permanently leaks them for factories
+    /// that track outstanding allocations.
+    /// <para>
+    /// A node type that reports
+    /// <c>NodeState.SupportsInstanceNodeIdAssignmentControl</c> is simply asked
+    /// not to assign, and never sees this wrapper. It exists only for types that
+    /// override the four argument <c>FindChild</c> and therefore have no way of
+    /// being told - hiding the factory is the one channel that reaches them.
+    /// Remove it once that overload is no longer supported.
+    /// </para>
     /// </remarks>
     internal sealed class NodeIdFactorySuppressedContext : ISystemContext
     {

src/Opc.Ua.Types/State/NodeState.cs

@@ -337,15 +337,23 @@ protected virtual void Initialize(ISystemContext context, NodeState source)
 
             // Every child created below is initialized from its source right
             // afterwards, which overwrites the NodeId a factory would hand out
-            // here, so the copy must not reach the factory at all.
-            ISystemContext childContext = children.Count > 0 && context.NodeIdFactory != null
-                ? new NodeIdFactorySuppressedContext(context)
-                : context;
+            // here, so the copy must not consume identifiers for them. Types
+            // that understand the request are told not to assign; the rest have
+            // the factory hidden from them for the duration of the copy because
+            // their FindChild override has no way to be told.
+            bool suppressViaContext = !SupportsInstanceNodeIdAssignmentControl;
+            ISystemContext childContext =
+                suppressViaContext && children.Count > 0 && context.NodeIdFactory != null
+                    ? new NodeIdFactorySuppressedContext(context)
+                    : context;
 
             for (int ii = 0; ii < children.Count; ii++)
             {
                 BaseInstanceState sourceChild = children[ii];
-                BaseInstanceState? child = CreateChild(childContext, sourceChild.BrowseName);
+                BaseInstanceState? child = CreateChild(
+                    childContext,
+                    sourceChild.BrowseName,
+                    assignInstanceNodeIds: false);
 
                 if (child == null)
                 {
@@ -637,6 +645,31 @@ public AccessRestrictionType? AccessRestrictions
         /// </summary>
         public bool DesignToolOnly { get; set; }
 
+        /// <summary>
+        /// Whether this type honours the <c>assignInstanceNodeIds</c> argument of
+        /// <see cref="FindChild(ISystemContext, QualifiedName, bool, BaseInstanceState, bool)"/>.
+        /// </summary>
+        /// <remarks>
+        /// Source generated node types override this to <c>true</c> because they
+        /// thread the argument through to their <c>CreateOrReplace&lt;Child&gt;</c>
+        /// helpers. A hand written type that overrides only the four argument
+        /// <c>FindChild</c> leaves it <c>false</c>, and a node copy then keeps
+        /// dispatching through
+        /// <see cref="CreateChild(ISystemContext, QualifiedName)"/> and hides the
+        /// <see cref="ISystemContext.NodeIdFactory"/> from it instead, so
+        /// identifiers are still not consumed for children whose NodeId is about
+        /// to be overwritten. Override this together with the five argument
+        /// <c>FindChild</c>, never on its own.
+        /// <para>
+        /// A hand written type deriving from a source generated one inherits
+        /// <c>true</c>. That is safe - the factory is still hidden from any four
+        /// argument override reached from here - but such a type only sees the
+        /// children its generated base does not itself declare. Override this
+        /// back to <c>false</c> if the four argument override must run first.
+        /// </para>
+        /// </remarks>
+        protected virtual bool SupportsInstanceNodeIdAssignmentControl => false;
+
         /// <summary>
         /// Exports a copy of the node to a node table.
         /// </summary>
@@ -4702,6 +4735,45 @@ protected virtual ServiceResult WriteValueAttribute(
             return FindChild(context, browseName, true, null);
         }
 
+        /// <summary>
+        /// Finds or creates the child with the specified browse name, stating
+        /// whether the child should be given a per-instance NodeId.
+        /// </summary>
+        /// <remarks>
+        /// A caller that overwrites the child's NodeId immediately afterwards -
+        /// a node copy is the canonical case - passes <c>false</c> so the
+        /// <see cref="ISystemContext.NodeIdFactory"/> is never asked for an
+        /// identifier that is about to be discarded. Only node types that report
+        /// <see cref="SupportsInstanceNodeIdAssignmentControl"/> honour the
+        /// request; for every other type this behaves exactly like
+        /// <see cref="CreateChild(ISystemContext, QualifiedName)"/>.
+        /// </remarks>
+        /// <param name="context">The context to use.</param>
+        /// <param name="browseName">The browse name.</param>
+        /// <param name="assignInstanceNodeIds">
+        /// Whether a newly created child may be given a per-instance NodeId.
+        /// </param>
+        /// <returns>The child if available. Null otherwise.</returns>
+        public virtual BaseInstanceState? CreateChild(
+            ISystemContext context,
+            QualifiedName browseName,
+            bool assignInstanceNodeIds)
+        {
+            if (browseName.IsNull)
+            {
+                return null;
+            }
+
+            if (!SupportsInstanceNodeIdAssignmentControl)
+            {
+                // Keep dispatching through the original virtual so a type that
+                // overrides only that one is still the thing that runs.
+                return CreateChild(context, browseName);
+            }
+
+            return FindChild(context, browseName, true, null, assignInstanceNodeIds);
+        }
+
         /// <summary>
         /// Creates or replaces the child with the same browse name.
         /// </summary>
@@ -5352,6 +5424,55 @@ public virtual void GetReferences(
             }
         }
 
+        /// <summary>
+        /// Finds the child with the specified browse name, stating whether a
+        /// newly created child should be given a per-instance NodeId.
+        /// </summary>
+        /// <remarks>
+        /// The base implementation forwards to
+        /// <see cref="FindChild(ISystemContext, QualifiedName, bool, BaseInstanceState)"/>,
+        /// so a type that overrides only that method keeps working unchanged. It
+        /// hides the <see cref="ISystemContext.NodeIdFactory"/> while doing so
+        /// when assignment was declined, because an override with no such
+        /// parameter cannot be told any other way.
+        /// <para>
+        /// A type that honours the request overrides this method and reports
+        /// <see cref="SupportsInstanceNodeIdAssignmentControl"/> as <c>true</c>,
+        /// which is what lets a node copy decline assignment outright while
+        /// still showing that type the real context.
+        /// </para>
+        /// </remarks>
+        /// <param name="context">The context for the system being accessed.</param>
+        /// <param name="browseName">The browse name of the children to add.</param>
+        /// <param name="createOrReplace">if set to <c>true</c> and the child does
+        /// not exist then the child is created or replaced with the provided
+        /// replacement.</param>
+        /// <param name="replacement">The replacement to use if createOrReplace is
+        /// true.</param>
+        /// <param name="assignInstanceNodeIds">
+        /// Whether a newly created child may be given a per-instance NodeId.
+        /// </param>
+        /// <returns>The child.</returns>
+        protected virtual BaseInstanceState? FindChild(
+            ISystemContext context,
+            QualifiedName browseName,
+            bool createOrReplace,
+            BaseInstanceState? replacement,
+            bool assignInstanceNodeIds)
+        {
+            // Control passes here to hand off to an override that has no way of
+            // being told not to assign - either because the type predates this
+            // overload, or because a derived type only overrides the four
+            // argument one. Hiding the factory is the only channel that reaches
+            // it, so a request not to assign is honoured even then.
+            ISystemContext childContext =
+                !assignInstanceNodeIds && context.NodeIdFactory != null
+                    ? new NodeIdFactorySuppressedContext(context)
+                    : context;
+
+            return FindChild(childContext, browseName, createOrReplace, replacement);
+        }
+
         /// <summary>
         /// Finds the child with the specified browse name.
         /// </summary>