mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-28 22:55:49 +08:00
# Description Adds an opt-in `SubscriptionRecoveryPolicy` for both the Classic V1 and V2 subscription engines that lets the client recover from server-emitted `Good_SubscriptionTransferred` notifications which arrive against a freshly created subscription (observed against Kepware after a server re-initialise, very likely a leak from the pre-restart session's pending status notifications onto the new session — abetted by Kepware re-using `SubscriptionId = 1` across generations). Per [OPC UA Part 4 §5.14.7](https://reference.opcfoundation.org/Core/Part4/v105/docs/5.14.7), `Good_SubscriptionTransferred` is supposed to be delivered to the *old* Session whose subscription was transferred away via `TransferSubscriptions`. The existing V1 behaviour of stopping the per-subscription publish loop on receipt is therefore **spec-compliant**; the issue is that some servers deliver the notification on a Session that is still actively using the subscription, leaving the per-subscription dispatch silently disabled. The fix is **opt-in** and **non-breaking**: - New `SubscriptionRecoveryPolicy` enum (`ReportOnly` default, `RecreateOnUnsolicitedTransfer`). - V1 Classic `Subscription.RecoveryPolicy` property; V2 `Subscriptions.SubscriptionOptions.RecoveryPolicy` init property. - When the policy is `RecreateOnUnsolicitedTransfer`, the SDK drops queued acks for the dead subscription id (avoiding `BadSubscriptionIdInvalid` ack errors on servers that re-use ids) and recreates the subscription in place on the same Session. ## What changed **Public API additions (non-breaking, defaults preserve current behaviour):** - `SubscriptionRecoveryPolicy` enum (`ReportOnly`, `RecreateOnUnsolicitedTransfer`). - V1 `Subscription.RecoveryPolicy` property. - V2 `Subscriptions.SubscriptionOptions.RecoveryPolicy` init property. - Internal `IMessageAckQueue.DropPendingForSubscription(uint)` and protected `MessageProcessor.AckQueue` accessor. **V1 Classic — `Libraries/Opc.Ua.Client/Subscription/Classic/Subscription.cs`:** - Replaces the inline `Good_SubscriptionTransferred` branch with `HandleGoodSubscriptionTransferred`. `ReportOnly` preserves the original behaviour (stop publish timer/worker); `RecreateOnUnsolicitedTransfer` triggers `TriggerUnsolicitedTransferRecovery`, guarded by `m_recreateAfterTransferInProgress` for idempotency. - Recovery drains pending acks via the new `ClassicSubscriptionEngine.RemoveAcknowledgementsForSubscription` **before** resetting the subscription, then runs `DeleteSubscription()` + `CreateAsync()` on the same session so the publish worker restarts via `StartKeepAliveTimer`. - Discriminator (`Session != null && !Session.Reconnecting && Created`) defers to the reconnect pipeline if it is already running. **V2 — `Libraries/Opc.Ua.Client/Subscription/Subscription.cs`:** - Completes the `OnStatusChangeNotificationAsync` TODO: now always fires `OnPublishStateChanged(mask)` so `PublishState.Transferred` actually reaches handlers (previously the no-op override silently swallowed the flag). - Under `RecreateOnUnsolicitedTransfer`, calls `AckQueue.DropPendingForSubscription(deadId)` then existing `ResetToRecreateAsync` on a worker task — no service calls from the dispatch path. - V2 `SubscriptionManager` implements `DropPendingForSubscription` via drain-and-refill on the prioritised ack channel. **Tests — `Tests/Opc.Ua.Client.Tests/Subscription`:** - V1 in `Classic/SubscriptionUnitTests.cs`: `ReportOnlyPolicyDoesNotRecreateOnUnsolicitedTransferAsync`, `RecreatePolicyRecreatesSubscriptionOnUnsolicitedTransferAsync`, `RecreatePolicyCollapsesConcurrentNotificationsAsync`, `RecreatePolicyDoesNotRecreateWhileSessionReconnectingAsync`. - V2 in `SubscriptionTests.cs`: `GoodSubscriptionTransferredRaisesPublishStateTransferredAsync`, `GoodSubscriptionTransferredRecreatesSubscriptionWhenPolicyEnabledAsync`, `GoodSubscriptionTransferredCollapsesConcurrentRecreatesAsync`. - `FakeMessageAckQueue` gained a `DroppedSubscriptions` recorder so the V2 test can assert ack pruning ran. **Docs — `Docs/TransferSubscription.md`:** - New "Recovering from unsolicited `Good_SubscriptionTransferred`" section with spec context, usage examples for both engines, and the caveat that recovery is recreate-based — **not** lossless failover; the server-side retransmission queue and triggering relationships tied to the invalidated subscription id are not preserved. ## Validation | Scope | Result | | --- | --- | | `Libraries/Opc.Ua.Client` build (`net10.0`, `net48`) | 0 warnings, 0 errors | | `Tests/Opc.Ua.Client.Tests` build (`net10.0`, `net472`) | 0 warnings, 0 errors | | New tests on `net10.0` and `net472` | 7/7 pass on each | | Full subscription unit-test suite (`SubscriptionUnitTests` + `SubscriptionTests` + `MessageProcessorTests` + `SubscriptionManagerTests`) | 110/110 pass on each TFM | | Entire `Opc.Ua.Client.Tests` suite (`net10.0`) | 1268/1268 pass | | Classic transfer integration tests (`Tests/Opc.Ua.Subscriptions.Classic.Tests`, `net10.0`) | 20/20 pass | | V2 transfer compliance tests (`Tests/Opc.Ua.Subscriptions.Tests`, `net10.0`) | 9/9 pass | > Marked as **draft** because the maintainer wanted to first confirm whether the stack is spec-compliant before deciding which knobs to ship; this PR documents the investigation (stack is compliant), and surfaces an opt-in recovery policy that addresses the field symptom without changing the default behaviour. Convert to ready-for-review once the design intent is acknowledged. ## Related Issues - Fixes #3540 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: marcschier <marc.schier@github.com>
356 lines
13 KiB
C#
356 lines
13 KiB
C#
/* ========================================================================
|
|
* Copyright (c) 2005-2025 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 System;
|
|
using System.Threading;
|
|
using System.Threading.Tasks;
|
|
using Moq;
|
|
using NUnit.Framework;
|
|
using Opc.Ua.Tests;
|
|
|
|
namespace Opc.Ua.Client.Tests
|
|
{
|
|
[TestFixture]
|
|
[Parallelizable]
|
|
[Category("Client")]
|
|
[Category("ClassicSubscriptionEngine")]
|
|
public class ClassicSubscriptionEngineTests
|
|
{
|
|
private ITelemetryContext m_telemetry;
|
|
private Mock<ISubscriptionEngineContext> m_mockContext;
|
|
private SemaphoreSlim m_lock;
|
|
|
|
[SetUp]
|
|
public void SetUp()
|
|
{
|
|
m_telemetry = NUnitTelemetryContext.Create();
|
|
m_lock = new SemaphoreSlim(1, 1);
|
|
m_mockContext = new Mock<ISubscriptionEngineContext>();
|
|
m_mockContext.Setup(c => c.SessionId)
|
|
.Returns(new NodeId(1));
|
|
m_mockContext.Setup(c => c.Disposed)
|
|
.Returns(false);
|
|
m_mockContext.Setup(c => c.Reconnecting)
|
|
.Returns(false);
|
|
m_mockContext.Setup(c => c.Connected)
|
|
.Returns(false);
|
|
m_mockContext.Setup(c => c.Closing)
|
|
.Returns(false);
|
|
m_mockContext.Setup(c => c.Telemetry)
|
|
.Returns(m_telemetry);
|
|
m_mockContext.Setup(c => c.Subscriptions)
|
|
.Returns([]);
|
|
m_mockContext.Setup(c => c.OperationTimeout)
|
|
.Returns(60000);
|
|
m_mockContext.Setup(c => c.ReturnDiagnostics)
|
|
.Returns(DiagnosticsMasks.None);
|
|
m_mockContext.Setup(c => c.ReconnectLock)
|
|
.Returns(m_lock);
|
|
m_mockContext.Setup(c => c.GoodPublishRequestCount)
|
|
.Returns(0);
|
|
}
|
|
|
|
[TearDown]
|
|
public void TearDown()
|
|
{
|
|
m_lock.Dispose();
|
|
}
|
|
|
|
[Test]
|
|
public void FactoryCreateReturnsClassicEngine()
|
|
{
|
|
ISubscriptionEngine engine =
|
|
ClassicSubscriptionEngineFactory.Instance.Create(
|
|
m_mockContext.Object);
|
|
|
|
Assert.That(engine, Is.Not.Null);
|
|
Assert.That(
|
|
engine,
|
|
Is.InstanceOf<ClassicSubscriptionEngine>());
|
|
|
|
engine.Dispose();
|
|
}
|
|
|
|
[Test]
|
|
public void StartPublishingWithNoSubscriptionsDoesNothing()
|
|
{
|
|
m_mockContext.Setup(c => c.Subscriptions)
|
|
.Returns([]);
|
|
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
engine.StartPublishing(timeout: 5000, fullQueue: false);
|
|
|
|
m_mockContext.Verify(
|
|
c => c.PublishAsync(
|
|
It.IsAny<RequestHeader>(),
|
|
It.IsAny<ArrayOf<SubscriptionAcknowledgement>>(),
|
|
It.IsAny<CancellationToken>()),
|
|
Times.Never);
|
|
}
|
|
|
|
[Test]
|
|
public void NotifySubscriptionsChangedTriggersPublishReEvaluation()
|
|
{
|
|
m_mockContext.Setup(c => c.Subscriptions)
|
|
.Returns([]);
|
|
m_mockContext.Setup(c => c.Disposed)
|
|
.Returns(false);
|
|
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
// With no subscriptions, NotifySubscriptionsChanged
|
|
// should not trigger a publish request.
|
|
engine.NotifySubscriptionsChanged();
|
|
|
|
m_mockContext.Verify(
|
|
c => c.PublishAsync(
|
|
It.IsAny<RequestHeader>(),
|
|
It.IsAny<ArrayOf<SubscriptionAcknowledgement>>(),
|
|
It.IsAny<CancellationToken>()),
|
|
Times.Never);
|
|
}
|
|
|
|
[Test]
|
|
public void MinPublishRequestCountDefaultsCorrectly()
|
|
{
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
Assert.That(
|
|
engine.MinPublishRequestCount, Is.EqualTo(1));
|
|
Assert.That(
|
|
engine.MaxPublishRequestCount,
|
|
Is.EqualTo(ushort.MaxValue));
|
|
}
|
|
|
|
[Test]
|
|
public void MinMaxPublishRequestCountCanBeSet()
|
|
{
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
engine.MinPublishRequestCount = 5;
|
|
engine.MaxPublishRequestCount = 50;
|
|
|
|
Assert.That(
|
|
engine.MinPublishRequestCount, Is.EqualTo(5));
|
|
Assert.That(
|
|
engine.MaxPublishRequestCount, Is.EqualTo(50));
|
|
}
|
|
|
|
[Test]
|
|
public void MinPublishRequestCountRejectsInvalidValues()
|
|
{
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
Assert.That(
|
|
() => engine.MinPublishRequestCount = 0,
|
|
Throws.TypeOf<ArgumentOutOfRangeException>());
|
|
Assert.That(
|
|
() => engine.MinPublishRequestCount = 101,
|
|
Throws.TypeOf<ArgumentOutOfRangeException>());
|
|
}
|
|
|
|
[Test]
|
|
public void MaxPublishRequestCountRejectsInvalidValues()
|
|
{
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
Assert.That(
|
|
() => engine.MaxPublishRequestCount = 0,
|
|
Throws.TypeOf<ArgumentOutOfRangeException>());
|
|
}
|
|
|
|
[Test]
|
|
public void PauseAndResumePublishing()
|
|
{
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
Assert.That(engine.PausePublishing, Throws.Nothing);
|
|
Assert.That(engine.ResumePublishing, Throws.Nothing);
|
|
}
|
|
|
|
[Test]
|
|
public async Task StopPublishingAsyncDisposesCleanlyAsync()
|
|
{
|
|
using var engine = new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
await engine.StopPublishingAsync().ConfigureAwait(false);
|
|
}
|
|
|
|
[Test]
|
|
public void DisposeIsIdempotent()
|
|
{
|
|
#pragma warning disable CA2000 // Dispose objects before losing scope
|
|
var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
#pragma warning restore CA2000 // Dispose objects before losing scope
|
|
|
|
Assert.That(engine.Dispose, Throws.Nothing);
|
|
Assert.That(engine.Dispose, Throws.Nothing);
|
|
}
|
|
|
|
[Test]
|
|
public void ConstructorThrowsOnNullContext()
|
|
{
|
|
Assert.That(
|
|
() => new ClassicSubscriptionEngine(null),
|
|
Throws.TypeOf<ArgumentNullException>());
|
|
}
|
|
|
|
[Test]
|
|
public void GoodPublishRequestCountDelegatesToContext()
|
|
{
|
|
m_mockContext.Setup(c => c.GoodPublishRequestCount)
|
|
.Returns(42);
|
|
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
Assert.That(
|
|
engine.GoodPublishRequestCount, Is.EqualTo(42));
|
|
}
|
|
|
|
[Test]
|
|
public void BadPublishRequestCountIsZero()
|
|
{
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
Assert.That(
|
|
engine.BadPublishRequestCount, Is.Zero);
|
|
}
|
|
|
|
[Test]
|
|
public void BeginPublishSkipsWhenSessionKeepAliveStopped()
|
|
{
|
|
// Arrange — session looks healthy except KeepAliveStopped is true.
|
|
m_mockContext.Setup(c => c.Connected).Returns(true);
|
|
m_mockContext.Setup(c => c.Reconnecting).Returns(false);
|
|
m_mockContext.Setup(c => c.Closing).Returns(false);
|
|
m_mockContext.Setup(c => c.KeepAliveStopped).Returns(true);
|
|
m_mockContext.Setup(c => c.LastKeepAliveTime)
|
|
.Returns(DateTime.UtcNow.AddSeconds(-30));
|
|
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
// Act
|
|
bool result = engine.BeginPublish(timeout: 5000);
|
|
|
|
// Assert — guard short-circuited; no PublishAsync was invoked.
|
|
Assert.That(result, Is.False);
|
|
m_mockContext.Verify(
|
|
c => c.PublishAsync(
|
|
It.IsAny<RequestHeader>(),
|
|
It.IsAny<ArrayOf<SubscriptionAcknowledgement>>(),
|
|
It.IsAny<CancellationToken>()),
|
|
Times.Never);
|
|
}
|
|
|
|
[Test]
|
|
public void MaxPublishRequestCountRespectsMinimum()
|
|
{
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
engine.MinPublishRequestCount = 10;
|
|
engine.MaxPublishRequestCount = 5;
|
|
|
|
// MaxPublishRequestCount getter returns the larger of
|
|
// min and max to ensure min is always respected.
|
|
Assert.That(
|
|
engine.MaxPublishRequestCount,
|
|
Is.GreaterThanOrEqualTo(
|
|
engine.MinPublishRequestCount));
|
|
}
|
|
|
|
/// <summary>
|
|
/// Regression coverage for the V1 ack-pruning helper added by
|
|
/// #3540. <see cref="ClassicSubscriptionEngine.RemoveAcknowledgementsForSubscription"/>
|
|
/// drops every queued acknowledgement targeting the given
|
|
/// subscription id while leaving acks for other subscriptions
|
|
/// untouched. The recovery path relies on this before recreating
|
|
/// a subscription so stale acks for the dead id cannot produce
|
|
/// <c>BadSubscriptionIdInvalid</c> responses on servers that
|
|
/// re-use subscription identifiers (e.g. Kepware always
|
|
/// starting at <c>1</c>).
|
|
/// </summary>
|
|
[Test]
|
|
public void RemoveAcknowledgementsForSubscriptionDropsOnlyMatchingAcks()
|
|
{
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
engine.AddPendingAcknowledgement(1u, 10u);
|
|
engine.AddPendingAcknowledgement(2u, 11u);
|
|
engine.AddPendingAcknowledgement(1u, 12u);
|
|
engine.AddPendingAcknowledgement(1u, 13u);
|
|
engine.AddPendingAcknowledgement(3u, 14u);
|
|
|
|
int dropped = engine.RemoveAcknowledgementsForSubscription(1u);
|
|
|
|
Assert.That(dropped, Is.EqualTo(3),
|
|
"All queued acks for the dead subscription id must be dropped.");
|
|
|
|
int droppedAgain = engine.RemoveAcknowledgementsForSubscription(1u);
|
|
Assert.That(droppedAgain, Is.Zero,
|
|
"Second call must drop nothing (idempotent).");
|
|
|
|
int droppedSecond = engine.RemoveAcknowledgementsForSubscription(2u);
|
|
Assert.That(droppedSecond, Is.EqualTo(1),
|
|
"Survivors must still be in the queue and drainable per-id.");
|
|
|
|
int droppedThird = engine.RemoveAcknowledgementsForSubscription(3u);
|
|
Assert.That(droppedThird, Is.EqualTo(1));
|
|
}
|
|
|
|
/// <summary>
|
|
/// Calling
|
|
/// <see cref="ClassicSubscriptionEngine.RemoveAcknowledgementsForSubscription"/>
|
|
/// on an empty pending-ack list is a no-op and must not throw.
|
|
/// </summary>
|
|
[Test]
|
|
public void RemoveAcknowledgementsForSubscriptionOnEmptyQueueReturnsZero()
|
|
{
|
|
using var engine =
|
|
new ClassicSubscriptionEngine(m_mockContext.Object);
|
|
|
|
int dropped = engine.RemoveAcknowledgementsForSubscription(42u);
|
|
Assert.That(dropped, Is.Zero);
|
|
}
|
|
}
|
|
}
|