Commit Graph
3 Commits
Author SHA1 Message Date
Marc Schier 4e6c3303dd Add V2 subscription triggering (SetTriggering) with N:M support and recreate replay (#3854)
# Description

Adds **triggered-item (SetTriggering) support to the V2 (modern)
subscription engine** with a hybrid API, batched apply pipeline, and
automatic replay on recreate/reconnect — closing the gap from #3834 and
adding store/load round-trip for triggering relationships.

## Public API

| Concern | Surface |
|---|---|
| Declarative | `MonitoredItemOptions.TriggeredByNames`
(`IReadOnlyList<string>`) — set at item-add time |
| Imperative | `ISubscription.SetTriggeringAsync(IMonitoredItem
triggeringItem, IReadOnlyCollection<IMonitoredItem>? linksToAdd,
IReadOnlyCollection<IMonitoredItem>? linksToRemove, CancellationToken
ct)` returning `SetTriggeringResult` with per-link statuses |
| Fluent helpers | name-based overloads `SetTriggeringAsync(this
ISubscription, string triggeringItemName, params string[]
triggeredItemNames)` (and `add`/`remove`/`ct` variant) |
| N:M navigation | `IMonitoredItem.TriggeringItems` (items that trigger
this) and `IMonitoredItem.TriggeredItems` (items I trigger), both
on-demand projections |
| Result type | `public sealed record SetTriggeringResult(IMonitoredItem
TriggeringItem, IReadOnlyList<(IMonitoredItem Item, StatusCode Status)>
AddResults, IReadOnlyList<(IMonitoredItem Item, StatusCode Status)>
RemoveResults, StatusCode ServiceResult)` |

No client handles surface on the public API. Validation in the
imperative path is by **reference identity** under the manager lock —
items from a different subscription with a matching name cannot sneak
through, and a removed-then-re-added item with the same name does not
silently re-target.

## Engine — batched apply

A new manager-level operations queue + a batched apply step in
`MonitoredItemManager.ApplyMonitoredItemChangesAsync`:

1. After Create / Modify / SetMonitoringMode, drain the operations
queue.
2. Group by triggering item (reference identity).
3. Re-queue ops whose triggering item is not yet `Created` (bounded by
`MaxTriggeringRetryCount`; permanently failed items propagate the
create-failure status to the operation TCS).
4. Fold per-edge across operations targeting the same triggering item —
last-intent wins; same-edge add+remove resolves to one net operation.
5. Pre-resolve: triggered items with `Created=false` resolve to
`Bad_MonitoredItemIdInvalid` (adds) or `Good` no-op (removes, per Part 4
§5.13.1.6 server auto-cleanup).
6. Issue **one `SetTriggering` RPC per triggering item** carrying both
`linksToAdd` and `linksToRemove` (the server processes remove before add
per §5.13.5.2 — atomic swap semantics).
7. Map per-link results back to contributing operations; complete each
TCS with its own `SetTriggeringResult` subset.

Imperative writes mutate the per-item runtime `DesiredTriggeredByNames`
synchronously so navigation (`TriggeringItems`/`TriggeredItems`) and
`Snapshot()` reflect intent before the RPC fires. Per-link failures roll
back the optimistic mutation so the client view matches server reality.

## State ownership

`MonitoredItemOptions.TriggeredByNames` is the **initial declarative
input**. The canonical runtime source of truth is a per-item
`DesiredTriggeredByNames` field, initialized from options and mutated by
both the imperative API and subsequent options changes. The field is
preserved across `Reset` (recreate path), which is what enables
automatic SetTriggering replay after recreate.

## Save / load (closes #3834)

- `MonitoredItemStateSnapshot.TriggeredByNames` (`ArrayOf<string>`)
replaces the old `TriggeringItemClientHandle` field (V2-only — not yet
shipped, so a clean break; classic snapshot format unchanged).
- `Snapshot()` reads from runtime `DesiredTriggeredByNames`;
`ApplyLoadState` writes loaded names into both options and runtime state
without leaking through the OnChange tracker.
- **TransferSubscriptions** success path: only the local view is
restored — trusting the spec (`§5.13.5`) that servers preserve
triggering links across subscription transfer. No SetTriggering RPC is
issued.
- **Recreate-fallback** (transfer rejected), **in-session
`RecreateAsync`**, and **Reconnect** paths: `Reset` preserves
`DesiredTriggeredByNames` and enqueues an add-delta per preserved
triggering name; the batched apply pass replays SetTriggering after
items finish (re)creating.

## Error handling

Per OPC UA Part 4 §5.13.5.3/§5.13.5.4 (Tables 73/74):

| Condition | Engine response |
|---|---|
| Per-link `Bad_MonitoredItemIdInvalid` | Surfaced in
`SetTriggeringResult.AddResults`/`RemoveResults`; optimistic
`DesiredTriggeredByNames` mutation rolled back |
| Service-level `Bad_SubscriptionIdInvalid` | Operations re-queued, not
terminal — replays after the subscription state machine recreates |
| Service-level `Bad_TooManyOperations` | Surfaced as
`ServiceResultException` to all contributing TCSs (no auto-chunking in
this version) |
| Triggering item never reaches `Created` | After
`MaxTriggeringRetryCount` re-queues, op completes with the triggering
item's propagated error (no infinite loop) |
| Communication errors | Routed through the existing retry policy with
bounded retry count |

## Spec consultation

Cross-checked against **OPC UA Part 4 v1.05.07 §5.13.5** (verified Table
73 / Table 74 directly via the OPC Foundation spec site) and §5.13.1.6
(deletion side-effects, monitoring-mode interactions). The only per-link
spec-specific status code is `Bad_MonitoredItemIdInvalid`;
`linksToRemove` is processed server-side before `linksToAdd` (driving
the single-RPC-per-trigger batch shape).

## Tests

- **9 new unit tests** in `MonitoredItemManagerTriggeringTests.cs`
(single op, multi-op merge into one RPC, per-trig grouping,
requeue-when-not-Created, per-link rollback, `Bad_SubscriptionIdInvalid`
recovery, deletion races, last-intent same-edge conflict resolution).
- **Updated** integration tests `SetTriggeringTrackingV2Async` and
`SnapshotCapturesTriggeringForReplayAsync` to use the new API.
- **2 new integration tests** in `SubscriptionTests.cs`:
`DeclarativeTriggeredByNamesV2Async` (options-driven path end-to-end)
and `NToMTriggeringV2Async` (one triggered item linked to two trigger
items, per-link removal).
- **AOT smoke test** in `SubscriptionAotTests.cs` for the
`MonitoredItemStateSnapshot.TriggeredByNames` binary encoder/decoder
round-trip.
- All **344 V2 subscription/monitored-item unit tests** and **71 V2
subscription integration tests** pass locally (net10).

## Documentation

- New: `Docs/Subscriptions.md` — *Subscriptions and Monitored Items
Service Set*. Merges the previous `StreamingSubscription.md` and
`SubscriptionTriggering.md` into a single guide; leads with a
Classic-vs-V2 engine overview (recommending V2 for new code) and links
to `Sessions.md` §4 for the full engine comparison. Covers the V2
callback API, declarative + imperative SetTriggering with N:M,
save/load/restore behavior, error handling, and the
`IStreamingSubscription` async-enumerable surface.
- Updated: `Docs/README.md`, `Docs/AlarmsAndConditions.md`,
`Docs/StateMachines.md`, `Docs/Profiles.md` — all point to the new
merged doc.
- Updated: `plans/26-v2-subscription-parity.md` — rows 49
(`SetTriggeringAsync`) and 111 (`TriggeringItems` plural /
`TriggeredItems`).

## Related Issues

- Fixes #3834 — `TransferSubscription does not automatically
restore/retain links between trigger and monitored items`. The fix is
two-pronged: (a) transfer preserves links server-side per spec and the
client snapshot captures `TriggeredByNames` so the local view restores
automatically; (b) when transfer fails and the engine falls back to
recreate (or when `RecreateAsync` is invoked explicitly), `Reset`
enqueues per-triggering-name add-deltas from the preserved runtime state
and the batched apply pass replays `SetTriggering` after re-creation.

## 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.
- [ ] 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.
2026-06-12 10:38:41 +02:00
Marc Schierandagent a81d981755 dotnet format sweep (whitespace, IDE, RCS) + code-style enforcer agent fix (#3865)
# Description

Pure code-style sweep across the repo — no behavioral changes. Drives
`dotnet format` info-level diagnostics down on a large slice of the
codebase (Stack, Libraries, Tests, Tools).

Six commits, applied bottom-up in safety order so each batch leaves the
build green:

1. **`style: fix whitespace and final-newline formatting (WHITESPACE,
FINALNEWLINE)`**
   197 hits across ~30 files; pure mechanical cleanup.

2. **`style: apply dotnet format style fixes (IDE rules)`**
IDE-rule batch via `dotnet format style --severity info` —
`var`/explicit type, simplify names, collection expressions, expression
bodies, unused usings, etc.

3. **`style: apply dotnet format analyzers fixes (RCS blank-line
formatting)`**
   Roslynator formatting (RCS0xxx) blank-line and punctuation rules.

4. **`style: convert comments to doc comments and inline local vars
(RCS1181, RCS1124)`**
24 files. RCS1181 promotes `// summary`-style comments above
public/internal members to `/// <summary>` doc comments; RCS1124 inlines
locals used exactly once. Generated `Tools/` files were reverted to keep
the source-generator outputs deterministic.

5. **`docs(agent): correct --diagnostics flag form and verify Roslynator
4.x code fixers`**
Updates `.github/agents/opc-ua-codestyle-enforcer.agent.md` with two
corrections discovered while running this sweep:
- `dotnet format --diagnostics "RULE1 RULE2"` (single quoted,
space-separated) silently fixes nothing on `dotnet format` 10.x — it
parses as one unknown ID and exits 0. The correct form is `--diagnostics
RULE1 --diagnostics RULE2 …`, repeated per rule.
- Roslynator 4.15.0 **does** ship code fixers for
RCS1078/1118/1124/1140/1142/1181/1221/1260 (the previous "no auto-fixer"
list was outdated). RCS1140/1142 default to `Hidden` so need `--severity
info` to surface and fire; RCS1078 requires
`roslynator_empty_string_style = field` (already set in
`.editorconfig`). Only NUnit4002/2046/2010 genuinely lack auto-fixers in
this stack.

6. **`style: format new code from master merge (dotnet format sweep)`**
After merging master into the branch (commit `e3ebc8742`), re-ran the
same three-phase format sweep on the 151 .cs files brought in by the
merge, scoped per project via repeated `--include`. 81 files changed;
same rule families as commits 1–4.
- Hand-fixed `Tests/Opc.Ua.MigrationAnalyzer.Tests/AnalyzerHarness.cs`
for `RCS1174` (redundant `async`/`await` removal) because the auto-fixer
didn't apply for that specific case.
   - Reverted files that the formatter would break:
- `Libraries/Opc.Ua.Server/Server/ServerInternalData.cs` — multi-TFM
analysis disagreement injected `<<<<<<<` / `=======` / `>>>>>>>`
markers.
-
`Tools/Opc.Ua.MigrationAnalyzer.Core/{Core/Stack/Client/ChannelBase.cs,
Core/Stack/Types/IUserIdentityTokenHandler.cs,
Client/Session/Session.cs}` — IDE0005 removed `using
Opc.Ua.Security.Certificates` that IS used (the files are 1.5.378
reference fixtures for the analyzer).
- `Tests/Opc.Ua.Gds.Tests/PushTest.cs` — IDE0002 over-simplified
`OpcUa.ObjectTypeIds` to `ObjectTypeIds`, which then resolves to a
conflicting type.

### What this PR does **not** touch

- No behavioral or semantic logic changes.
- No `.editorconfig` rule promotions (rules stay at their current
severities; this is a backlog-burndown pass, not a rule tightening).
- Generated source under `Tools/` is intentionally excluded.
- Higher-risk rule families (`RCS1090`/`CA2007` `ConfigureAwait`,
`RCS1249` null-forgiving cleanup, `IDE0270`/`IDE0059`) are left for
follow-up sweeps because they need per-TFM build verification or
semantic review.

## Related Issues

_No tracking issue — this is a non-functional style sweep and the
agent-doc correction is a follow-up to the same work. Happy to open one
if the maintainers prefer._

## Checklist

- [ ] 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. _(N/A — style-only, no
behavioral change)_
- [x] I have added all necessary documentation. _(agent doc updated with
the two corrections)_
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings. _(verified clean on `net10.0` and `net48`; only
pre-existing `NETSDK1005`/`NETSDK1207` failures on Tools/AOT projects)_
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. _(build
verified on net10 and net48 per batch; full test suite not yet run
locally — relying on CI)_
- [ ] 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: agent <agent@local>
2026-06-10 08:35:44 +02:00
Marc Schier 49305d9085 V2 subscription engine close gap for test and feature parity with classic engine (#3824)
# Description

V2 subscription engine feature parity round to make the classic engine
eligible for deprecation. Splits the test surface so the classic and V2
engines can evolve independently, ports the gaps the classic-engine
integration tests had been exercising, and lands the public V2 API
additions needed to close them.

- Splits `Tests/Opc.Ua.Subscriptions.Tests` into a classic
(`Opc.Ua.Subscriptions.Classic.Tests`) and a V2
(`Opc.Ua.Subscriptions.Tests`) project to shrink CI time and let the two
engines diverge cleanly.
- Adds the V2 public-surface gaps required by the test ports
(`Subscription.ServerId` on the class, `SetSubscriptionDurableAsync`,
`SendInitialValuesOnTransfer`, unified `OnSubscriptionStateChangedAsync`
handler callback, per-item `ConditionRefreshAsync`, fluent
`LoadSubscriptionsAsync(Stream, …)` and `Subscription.Snapshot()` /
`RestoreAsync()` for save/load + transfer).
- Lands handler-centric design (no `PublishingStopped` property handlers
maintain derived state); always-sequential per-subscription delivery
documented + tested.
- **Adds short and long haul stability tests for the V2 `ManagedSession`
+ subscription engine. Closes #3744** (see "Issue #3744 Haul tests"
below).
- **Migrates SubscriptionStateSnapshot / MonitoredItemStateSnapshot to
`[DataType]` source-generator codecs**.
- `SubscriptionDurableV2Tests.cs` 5 V2 ports of the classic durable test
patterns.
- `V2FollowUpCoverageTests.cs` handler state-change callback, fluent
stream-based `LoadSubscriptionsAsync`, `SendInitialValuesOnTransfer`,
snapshot edge cases (empty / `DataChangeFilter` round-trip / concurrent
  mutation).
- `SubscriptionFailoverV2Tests.cs` forced channel-break with and without
`WithTransferSubscriptionsOnRecreate`.
- `MonitoredItemConditionRefreshLiveV2Tests.cs` ΓÇö live
`ConditionRefresh` against the reference server's event source; asserts
`RefreshStart` / `RefreshEnd` events flow through the handler.

## Fixes

- #3744 Haul tests (V2 ManagedSession + subscription engine)
`Tests/Opc.Ua.Sessions.Tests/ManagedSessionStabilityTest.cs` adds the
short/long haul coverage requested by
[#3744](https://github.com/OPCFoundation/UA-.NETStandard/issues/3744)
plus the bonus fault-injection variant:

| Test | Duration | Category | Purpose |
|---|---|---|---|
| `ShortHaulManagedSessionV2Async` | 2 minutes | `ManagedSessionHaul`
(`[Explicit]`) | Quick local sanity check of the V2 monotonicity
contract. |
| `LongHaulManagedSessionV2Async` | 90 minutes (configurable via
`TEST_DURATION_MINUTES`) | `ManagedSessionHaul` (`[Explicit]`) |
Long-running V2 stability + monotonicity. |
| `LongHaulManagedSessionWithFaultInjectionV2Async` | 90 minutes
(configurable) + fault every `FAULT_INJECTION_INTERVAL_SECONDS` (default
60s) | `ManagedSessionHaul` (`[Explicit]`) | **Bonus** ΓÇö force-closes
the subscriber transport channel periodically; verifies V2 manager
transfers/recreates the subscription and continues monotonic delivery
across reconnects. |

**Pattern.** A writer `ManagedSession` increments a `uint32` counter and
writes it into `Scalar_Static_Mass_UInt32_UInt32_00` every 250ms. A
subscriber `ManagedSession` runs a V2 subscription with a single
monitored item on the same node. `MonotonicCounterHandler` asserts each
received sample is *strictly greater than* the previous one. V2
`ISubscriptionNotificationHandler` per-subscription ordering guarantee.
Skipped values (sampling rate < write rate) are tolerated; duplicates
and reorderings are violations.
2026-06-03 04:39:08 +02:00