Remove hardcoded Retain=true in ConditionState.UpdateStateAfterEnable (#3370)

* Initial plan

* Add EvaluateRetainStateOnEnable virtual method for ConditionState

- Removed hardcoded `Retain.Value = true` in UpdateStateAfterEnable
- Added virtual EvaluateRetainStateOnEnable method that derived classes can override
- Default implementation calls UpdateRetainState() which uses GetRetainState()
- This allows concrete classes to customize Retain evaluation when enabling conditions
- Added comprehensive tests for the new functionality
- All existing tests pass (7846 tests)

Co-authored-by: romanett <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: romanett <[email protected]>

Author: Copilot

Stack/Opc.Ua.Core/Stack/State/ConditionState.cs

@@ -698,6 +698,21 @@ protected virtual ServiceResult ProcessBeforeEnableDisable(
             return ServiceResult.Good;
         }
 
+        /// <summary>
+        /// Evaluates and updates the Retain state when the condition is enabled.
+        /// </summary>
+        /// <param name="context">The system context.</param>
+        /// <remarks>
+        /// This method is called by UpdateStateAfterEnable to determine the Retain value.
+        /// The default implementation calls UpdateRetainState() which uses GetRetainState().
+        /// Derived classes can override this method to provide custom logic for determining
+        /// the Retain value when the condition is enabled.
+        /// </remarks>
+        protected virtual void EvaluateRetainStateOnEnable(ISystemContext context)
+        {
+            UpdateRetainState();
+        }
+
         /// <summary>
         /// Updates the condition state after enabling.
         /// </summary>
@@ -709,7 +724,6 @@ protected virtual void UpdateStateAfterEnable(ISystemContext context)
                 "en-US",
                 ConditionStateNames.Enabled);
 
-            Retain.Value = true;
             EnabledState.Value = new LocalizedText(state);
             EnabledState.Id.Value = true;
 
@@ -718,6 +732,8 @@ protected virtual void UpdateStateAfterEnable(ISystemContext context)
                 EnabledState.TransitionTime.Value = DateTime.UtcNow;
             }
 
+            EvaluateRetainStateOnEnable(context);
+
             UpdateEffectiveState(context);
         }
 

Tests/Opc.Ua.Core.Tests/Stack/State/ConditionStateTests.cs

@@ -0,0 +1,171 @@
+/* ========================================================================
+ * Copyright (c) 2005-2018 The OPC Foundation, Inc. All rights reserved.
+ *
+ * OPC Foundation MIT License 1.00
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * The complete license agreement can be found here:
+ * http://opcfoundation.org/License/MIT/1.00/
+ * ======================================================================*/
+
+using NUnit.Framework;
+using Opc.Ua.Tests;
+
+namespace Opc.Ua.Core.Tests.Stack.State
+{
+    /// <summary>
+    /// Tests for the ConditionState class.
+    /// </summary>
+    [TestFixture]
+    [Category("ConditionState")]
+    [SetCulture("en-us")]
+    [SetUICulture("en-us")]
+    [Parallelizable]
+    public class ConditionStateTests
+    {
+        private ISystemContext m_context;
+        private ITelemetryContext m_telemetry;
+
+        [OneTimeSetUp]
+        protected void OneTimeSetUp()
+        {
+            m_telemetry = NUnitTelemetryContext.Create();
+            var messageContext = new ServiceMessageContext(m_telemetry);
+            // Add OPC UA namespace
+            messageContext.NamespaceUris.GetIndexOrAppend(Namespaces.OpcUa);
+            m_context = new SystemContext(m_telemetry)
+            {
+                NamespaceUris = messageContext.NamespaceUris
+            };
+        }
+
+        [OneTimeTearDown]
+        protected void OneTimeTearDown()
+        {
+            Utils.SilentDispose(m_context);
+        }
+
+        /// <summary>
+        /// Test that UpdateStateAfterEnable calls EvaluateRetainStateOnEnable
+        /// and that the default implementation sets Retain based on GetRetainState.
+        /// </summary>
+        [Test]
+        public void UpdateStateAfterEnableCallsEvaluateRetainStateOnEnable()
+        {
+            // Arrange
+            var condition = new ConditionState(null);
+            condition.Create(m_context, null, new QualifiedName("TestCondition"), null, true);
+
+            // Initially disabled
+            condition.SetEnableState(m_context, false);
+            Assert.That(condition.EnabledState.Id.Value, Is.False);
+            Assert.That(condition.Retain.Value, Is.False);
+
+            // Act - Enable the condition
+            condition.SetEnableState(m_context, true);
+
+            // Assert - Enabled state should be true
+            Assert.That(condition.EnabledState.Id.Value, Is.True);
+            
+            // The default implementation should call UpdateRetainState which uses GetRetainState
+            // For base ConditionState with no branches, GetRetainState returns false when enabled
+            Assert.That(condition.Retain.Value, Is.False);
+        }
+
+        /// <summary>
+        /// Test that UpdateStateAfterDisable sets Retain to false as per specification.
+        /// </summary>
+        [Test]
+        public void UpdateStateAfterDisableSetsRetainToFalse()
+        {
+            // Arrange
+            var condition = new TestConditionStateWithRetain(null);
+            condition.Create(m_context, null, new QualifiedName("TestCondition"), null, true);
+
+            // Enable the condition and set retain to true
+            condition.SetEnableState(m_context, true);
+            condition.ForceRetain(true);
+            Assert.That(condition.Retain.Value, Is.True);
+
+            // Act - Disable the condition
+            condition.SetEnableState(m_context, false);
+
+            // Assert - Retain should be false per specification
+            Assert.That(condition.EnabledState.Id.Value, Is.False);
+            Assert.That(condition.Retain.Value, Is.False);
+        }
+
+        /// <summary>
+        /// Test that a derived class can override EvaluateRetainStateOnEnable
+        /// to provide custom logic for determining the Retain value.
+        /// </summary>
+        [Test]
+        public void DerivedClassCanOverrideEvaluateRetainStateOnEnable()
+        {
+            // Arrange
+            var condition = new TestConditionStateWithCustomRetain(null);
+            condition.Create(m_context, null, new QualifiedName("TestCondition"), null, true);
+
+            // Initially disabled
+            condition.SetEnableState(m_context, false);
+            Assert.That(condition.Retain.Value, Is.False);
+
+            // Act - Enable the condition
+            condition.SetEnableState(m_context, true);
+
+            // Assert - The custom implementation should have been called
+            Assert.That(condition.EnabledState.Id.Value, Is.True);
+            Assert.That(condition.EvaluateCalled, Is.True);
+            Assert.That(condition.Retain.Value, Is.True);  // Custom logic always sets to true
+        }
+
+        /// <summary>
+        /// Test condition that exposes method to force Retain value for testing.
+        /// </summary>
+        private sealed class TestConditionStateWithRetain : ConditionState
+        {
+            public TestConditionStateWithRetain(NodeState parent) : base(parent) { }
+
+            public void ForceRetain(bool value)
+            {
+                Retain.Value = value;
+            }
+        }
+
+        /// <summary>
+        /// Test condition that overrides EvaluateRetainStateOnEnable.
+        /// </summary>
+        private sealed class TestConditionStateWithCustomRetain : ConditionState
+        {
+            public bool EvaluateCalled { get; private set; }
+
+            public TestConditionStateWithCustomRetain(NodeState parent) : base(parent) { }
+
+            protected override void EvaluateRetainStateOnEnable(ISystemContext context)
+            {
+                EvaluateCalled = true;
+                // Custom logic: always set Retain to true when enabled
+                Retain.Value = true;
+            }
+        }
+    }
+}