Files
UA-.NETStandard/Tests/Opc.Ua.Client.Tests/Session/ClassicSubscriptionEngineTests.cs
Marc Schierandmarcschier 0fbc900919 Add opt-in SubscriptionRecoveryPolicy for unsolicited Good_SubscriptionTransferred (#3540) (#3847)
# 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>
2026-06-07 17:38:58 +02:00

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);
}
}
}