diff --git a/Tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/NodeStateGeneratorTests.cs b/Tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/NodeStateGeneratorTests.cs index e792673d7e..11a1a6abb5 100644 --- a/Tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/NodeStateGeneratorTests.cs +++ b/Tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/NodeStateGeneratorTests.cs @@ -28,9 +28,12 @@ * ======================================================================*/ using System; +using System.Collections.Generic; +using System.Collections.Immutable; using System.IO; using System.Linq; using System.Text; +using System.Text.RegularExpressions; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.Extensions.Logging; @@ -353,6 +356,136 @@ namespace Opc.Ua.SourceGeneration.Generator.Tests }); } + /// + /// Regression test for issue #3964. A concrete predefined instance + /// (AnalogDevice) declares a BaseAnalogType variable + /// (Measurement) whose EURange / EngineeringUnits + /// property children are explicitly present on the instance but only + /// resolve to Optional (inherited from BaseAnalogType, + /// not restated as Mandatory on the instance). Before the fix the + /// generated instance-level factory emitted the creation of those + /// children inside the if (!forInstance) type-template block, + /// so — because the factory is always invoked with + /// forInstance: true — the properties were dropped from the + /// actual instance. They must instead be created unconditionally, + /// exactly like the type-level factory does. + /// + [Test] + public void ConcreteInstanceAnalogChildren_AreMaterializedUnconditionally() + { + // Arrange + ITelemetryContext telemetry = NUnitTelemetryContext.Create(logLevel: LogLevel.Error); + + // Act + Dictionary files = GenerateFromNodeSet( + "AnalogInstance.NodeSet2.xml", telemetry); + + // Assert + string ex = files.Single( + kv => kv.Key.EndsWith(".NodeStates.ex.g.cs", StringComparison.Ordinal)).Value; + + // The concrete-instance factory for the analog variable + // (Create_) is distinct from the type-level + // factory (Create_). Only the instance one regressed. + string instanceFactory = ExtractMethodBody(ex, "CreateAnalogDevice_Measurement"); + + // The variable must be recognised as an analog state so that the + // typed EngineeringUnits / EURange children (and the fluent + // WithEURange / WithEngineeringUnits helpers) are available. + Assert.That(instanceFactory, + Does.Contain("global::Opc.Ua.BaseAnalogState"), + "Measurement should be generated as a BaseAnalogState."); + + Assert.Multiple(() => + { + Assert.That(instanceFactory, Does.Contain("CreateOrReplaceEURange"), + "Instance factory must create the EURange child."); + Assert.That(instanceFactory, Does.Contain("CreateOrReplaceEngineeringUnits"), + "Instance factory must create the EngineeringUnits child."); + }); + + int idxGate = instanceFactory.IndexOf( + "if (!forInstance)", StringComparison.Ordinal); + int idxEuRange = instanceFactory.IndexOf( + "CreateOrReplaceEURange", StringComparison.Ordinal); + int idxEngineeringUnits = instanceFactory.IndexOf( + "CreateOrReplaceEngineeringUnits", StringComparison.Ordinal); + + if (idxGate >= 0) + { + Assert.Multiple(() => + { + Assert.That(idxEuRange, Is.LessThan(idxGate), + "EURange must be created before the `if (!forInstance)` gate " + + "so it materialises on the actual instance (issue #3964)."); + Assert.That(idxEngineeringUnits, Is.LessThan(idxGate), + "EngineeringUnits must be created before the `if (!forInstance)` " + + "gate so it materialises on the actual instance (issue #3964)."); + }); + } + } + + private static Dictionary GenerateFromNodeSet( + string nodeSetResource, + ITelemetryContext telemetry) + { + using var fileSystem = new VirtualFileSystem(); + string path = Path.Combine( + Directory.GetCurrentDirectory(), "Resources", nodeSetResource); + + var nodesets = new global::Opc.Ua.SourceGeneration.NodesetFileCollection( + ImmutableArray.Create( + (path, new global::Opc.Ua.SourceGeneration.NodesetFileOptions())), + fileSystem, + telemetry); + + // The test compilation has no Opc.Ua.Server reference, so + // suppress fluent-builder emission (mirrors model-only projects). + nodesets.GenerateCode( + fileSystem, + string.Empty, + telemetry, + new GeneratorOptions { OmitFluentApi = true }); + + return fileSystem.CreatedFiles + .Where(c => Path.GetExtension(c) == ".cs") + .ToDictionary(c => c, c => Encoding.UTF8.GetString(fileSystem.Get(c))); + } + + /// + /// Extracts the source of the factory method whose signature begins + /// with internal static … {methodName}(. Returns the body from + /// the method definition up to (but not including) the next + /// static-method definition, so callers can assert on ordering within + /// a single method without matching call sites elsewhere in the file. + /// + private static string ExtractMethodBody(string code, string methodName) + { + System.Text.RegularExpressions.Match match = Regex.Match( + code, + @"internal static [^\r\n]*\b" + Regex.Escape(methodName) + @"\("); + Assert.That(match.Success, Is.True, + $"Method definition '{methodName}' not found in generated code."); + + int start = match.Index; + int end = code.Length; + foreach (string marker in new[] + { + "\n internal static", + "\n public static", + "\n private static" + }) + { + int idx = code.IndexOf( + marker, start + match.Length, StringComparison.Ordinal); + if (idx >= 0 && idx < end) + { + end = idx; + } + } + return code.Substring(start, end - start); + } + private Mock m_mockFileSystem; private Mock m_mockModelDesign; private Mock m_mockTelemetry; diff --git a/Tests/Opc.Ua.SourceGeneration.Core.Tests/Resources/AnalogInstance.NodeSet2.xml b/Tests/Opc.Ua.SourceGeneration.Core.Tests/Resources/AnalogInstance.NodeSet2.xml new file mode 100644 index 0000000000..af79bcd1d1 --- /dev/null +++ b/Tests/Opc.Ua.SourceGeneration.Core.Tests/Resources/AnalogInstance.NodeSet2.xml @@ -0,0 +1,134 @@ + + + + + http://test.org/UA/AnalogInstance/ + + + + + + + + i=11 + i=47 + i=46 + i=35 + i=45 + i=40 + i=37 + + + + AnalogDeviceType + + i=58 + ns=1;i=1001 + + + + + Measurement + + i=15318 + i=78 + ns=1;i=1002 + ns=1;i=1003 + + + + + EURange + + i=68 + i=78 + + + + + EngineeringUnits + + i=68 + i=78 + + + + + AnalogDevice + + i=85 + ns=1;i=1000 + + + + + Measurement + + ns=1;i=2000 + i=15318 + ns=1;i=2002 + ns=1;i=2003 + + + + + EURange + + ns=1;i=2001 + i=68 + + + + + i=885 + + + + -5 + 95 + + + + + + + + EngineeringUnits + + ns=1;i=2001 + i=68 + + + + + i=888 + + + + http://www.opcfoundation.org/UA/units/un/cefact + 4408652 + + °C + + + degree Celsius + + + + + + + diff --git a/Tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateGenerator.cs b/Tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateGenerator.cs index 6803c64cd6..18778c0488 100644 --- a/Tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateGenerator.cs +++ b/Tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateGenerator.cs @@ -1579,6 +1579,32 @@ namespace Opc.Ua.SourceGeneration (node.Parent.Design is not InstanceDesign || !m_context.Options.UseTypeDefinitionModellingRules); + // Explicitly-declared descendants of a concrete predefined instance + // (a user-modelled / declared under + // Objects, e.g. a device instance) must be materialised on the + // actual instance regardless of their (possibly type-inherited) + // modelling rule: they are real nodes present in the model, not + // modelling-rule-driven optional children the SDK adds on demand. + // The topLevelInstanceException above only covers the instance's + // DIRECT children (node.Parent.Parent == null); deeper descendants + // — e.g. the EURange / EngineeringUnits properties of a + // BaseAnalogType / AnalogUnitType variable nested under the + // instance (issue #3964) — inherit IsUnderSingletonInstance and + // would otherwise land in the `if (!forInstance)` type-template + // block and be dropped from the instance. SDK-managed singletons + // (Server, ServerConfiguration, …) are generated with + // UseTypeDefinitionModellingRules on and keep the #3768 + // optional-child suppression, so they are excluded here. + bool concreteInstanceDescendant = + node.IsUnderSingletonInstance && + !node.RootIsTypeDefinition && + node.InstanceOf == null && + !node.IsNotExplicitlyDefined && + !m_context.Options.UseTypeDefinitionModellingRules; + + bool alwaysEmitInstanceChild = + topLevelInstanceException || concreteInstanceDescendant; + if (context.Token == Tokens.ListOfOptionalChildNodeStates) { if (effectiveRule == ModellingRule.Mandatory) @@ -1588,7 +1614,7 @@ namespace Opc.Ua.SourceGeneration // Real instance children of a top-level (non-typed) parent belong in // the always-emitted list so they materialize for the actual instance, // not only when the node is treated as a type template. - if (topLevelInstanceException) + if (alwaysEmitInstanceChild) { return null; } @@ -1598,7 +1624,7 @@ namespace Opc.Ua.SourceGeneration else if (effectiveRule != ModellingRule.Mandatory) { // Exception: real instance children of a top-level (non-typed) parent. - if (!topLevelInstanceException) + if (!alwaysEmitInstanceChild) { return null; }