mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-20 00:31:32 +08:00
romanett
414957c88a
# Description Reduces the server subscription API surface per the measurements from issue #4185. ## ISubscription: 42 → 30 members (#4185) A call-site census of the 11 "publish-pipeline protocol" members plus `Publish` (all 68 `src/` projects, samples, tools, fuzzing, tests) found **zero callers outside `Opc.Ua.Server`** and split the members three ways: - **Deleted (4):** `ItemReadyToPublish` and `ItemNotificationsAvailable` were dead — zero callers repo-wide and implementation bodies commented out since before 1.5.378 — the obsolete parameterless `SessionClosed()` had zero callers, and `TransferSessionAsync` (the one-shot direct transfer, which bypassed the reservation protecting a transfer against a concurrently closing source session) lost its last caller once the manager always uses the claim/prepare/commit protocol. - **Internalized (8):** `PublishTimerExpired`, `Acknowledge`, `PublishTimeout`, `SubscriptionTransferred`, `AvailableSequenceNumbersForRetransmission`, `QueueOverflowHandler`, `SessionClosed(ISession)` and `Publish` move to `internal interface ISubscriptionPublishPipeline : ISubscription`, implemented **explicitly** by `Subscription` (the `ISubscriptionMonitoredItemLifecycle` shape) so they are unreachable through the public concrete class too. `Publish` goes with them deliberately — it is the same hazard class (consumes notifications, advances sequence numbers) with the same single production caller. - **Kept public (1):** `ResendData` drops out of the issue's list — its only caller is the `Server_ResendData` UA-method handler in `DiagnosticsNodeManager`, resolving via `TryGetSubscription` like `GetMonitoredItems` does. It is service-facing, not pipeline. ## The pipeline contract is complete: SessionPublishQueue traffics only in it `SessionPublishQueue` becomes `internal sealed` (zero consumers outside the assembly; `SentMessageQueue` already was) and works **solely with `ISubscriptionPublishPipeline`** — no `ISubscription` and no concrete `Subscription` anywhere in the file. To get there, the internal transfer-claim protocol (`TryBeginTransfer`, `PrepareSessionTransferAsync`, `CompleteTransfer`, `AbortTransfer`, `TryRestoreSessionAfterFailedTransfer`) moved from `internal` methods on the concrete class onto the internal interface, implemented explicitly. That completes the contract, so `TransferSubscriptionsAsync` loses its two-tier shape: the ~6 `subscription as Subscription` null-guard branches and the degraded `TransferSessionAsync` fallback for non-`Subscription` implementations are gone — one code path, always the claim/prepare/commit/rollback protocol, and `ISubscription.TransferSessionAsync` is deleted with it. Subscription creation fails with `Bad_InternalError` when a `CreateSubscription` override returns a type not derived from `Subscription`, instead of publishing partially — such implementations were already second-class (transfer required the concrete type before this change). This is the headline behavior change for exotic extenders and is documented prominently in `docs/migrate/2.0.x/sessions-subscriptions.md`. The durable-subscription open question in the issue is answered **no**: `Subscription/Persistence/` never references `ISubscription` (the store deals in `IStoredSubscription` DTOs; restore constructs the concrete type via `Subscription.RestoreAsync` inside the manager), so the internalization cannot break a store implementer. ## #4186: measured, prototyped, and deliberately kept as-is Body inspection showed only 4 of the 10 `ISubscriptionManager` wrappers are pure lookup-and-delegate (`Republish`, `SetTriggering`, `ModifyMonitoredItemsAsync`, `SetMonitoringModeAsync` — one caller each); the other 6 carry logic with no `ISubscription` substitute. A collapse of the four was implemented and then reverted on this branch: it traded ten uniform id-addressed members for a mixed dispatch shape in `StandardServer` without reducing what a reader must understand. **Every subscription service keeps going through `ISubscriptionManager`**; the wrapper bodies now resolve against the pipeline-typed registry. The decision is recorded in `plans/ServerRuntimeAnalysis.md` (open question 5). ## Compatibility treatment Removals are outright, per the documented no-`[Obsolete]`-shim policy for these interfaces (`docs/MigrationGuide.md` — re-adding an interface member would break every implementer; net48/netstandard2.0 have no default interface methods). In its place: - Migration analyzer **UA0030** (pipeline members + `SessionPublishQueue`, distinguishing deleted no-ops from internalized protocol), with tests, following the UA0024 syntax-based pattern. The transfer-claim members were never public in 1.5.378, so they need no migration story. - `docs/MigrationGuide.md#ua0030` section with before/after code; the `docs/migrate/2.0.x/sessions-subscriptions.md` section that told implementers to *add* `SessionClosed(ISession)` is replaced with the derive-from-`Subscription` story; the 2.0.x README symptom table routes the rule. - All 11 removed/internalized members existed in 1.5.378 (verified against `master378`), so each is documented as a 2.0 break. ## Tests Converted, not deleted. `Mock<ISubscription>` setups of pipeline members become `Mock<ISubscriptionPublishPipeline>` — Moq proxies the internal interface via a new `InternalsVisibleTo("DynamicProxyGenAssembly2")` (keyless; the SDK appends `$(PublicKey)` under `SignAssembly`, and signed pipelines do not run tests, matching the existing `Opc.Ua.Redundancy.Kubernetes` precedent). A test-side extension shim forwards the original member names (pipeline + transfer protocol) so the direct call sites on concrete `Subscription` instances compile unchanged. The transfer-race tests' typed reflection probes into `m_abandonedSubscriptions` follow the field's new type. - `Opc.Ua.Server.Tests` (net10.0): 4795 tests, 0 failures - `Opc.Ua.Subscriptions.Tests`, `Opc.Ua.Subscriptions.Durable.Tests`, `Opc.Ua.Subscriptions.Classic.Tests` (net10.0): green - `Opc.Ua.MigrationAnalyzer.Tests`: green incl. the new UA0030 tests - net48 gate: solution build green; subscription-area fixtures green (the known pre-existing local hosting-test environment failures reproduce identically on unmodified master) ## Related Issues - Fixes #4185 - #4186 was measured and prototyped on this branch and deliberately **not** taken (see the section above); the decision is recorded in `plans/ServerRuntimeAnalysis.md`. ## 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. _(net10 suites fully green locally; net48 build green and subscription-area fixtures green — the remaining net48 hosting-test failures on my machine reproduce identically on unmodified master.)_ - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>