mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-18 14:43:08 +08:00
fix/encoder-stack-depth-json-flake
36 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 romanett
|
78e66f3473 |
Aggregates: TreatUncertainAsBad never applies to bounding values; DurationGood/Bad count Good/Bad regions only (#4702)
# Description Server-side aggregate fixes that follow three answers from the OPC Foundation on Mantis: - [11462](https://mantis.opcfoundation.org/view.php?id=11462) (~0025840): TreatUncertainAsBad does not apply to bounding values. - [11425](https://mantis.opcfoundation.org/view.php?id=11425) (~0025847): DurationGood counts only Good regions and DurationBad only Bad regions, and TreatUncertainAsBad does not apply. - [11426](https://mantis.opcfoundation.org/view.php?id=11426) (~0025852): Minimum, Maximum, the ActualTime variants and Range look at Uncertain values as if they are Good. The principle in 11425 and 11426 is that the aggregate definition wins over TreatUncertainAsBad. The CTT-side defects are filed as [11487](https://mantis.opcfoundation.org/view.php?id=11487). ### Bounding values (`AggregateCalculator`, `StartEndAggregateCalculator`) - Simple (§3.1.9) and Interpolated (§3.1.8) Bounding Values are formed from **non-Bad** raw values (`IsBoundCandidate`), whatever TreatUncertainAsBad is. - An Uncertain raw value before or after the timestamp makes the bound `UncertainDataSubNormal`. - A raw value exactly at the timestamp is the bound and keeps its own status. - Only a **Bad** value after a Simple bound reverts to stepped interpolation. - StartBound/EndBound keep an Uncertain bound instead of `Bad_NoData` (Tables 76/77). - WorstQuality2 now includes an Uncertain start bound, as §5.4.3.36 requires ("the server shall ignore the TreatUncertainAsBad for this aggregate"). ### How aggregates use the values (unchanged rule, two gaps closed) TreatUncertainAsBad still governs how an aggregate uses the values inside the interval, including an Uncertain bound. - `GetRegionsInValueSet` now also converts an Uncertain **first** region to Bad for Simple-bound aggregates. TimeAverage2/Total2 therefore omit it like any other Uncertain region (§5.4.3.7, Table 55 "Bound Uncertain"). - TimeAverage/Total (Interpolated bounds) still use an Uncertain bound and report Uncertain (Table 54). - TimeAverage2/Total2 return the PercentTime status `Bad` instead of `Bad_NoData` for an interval that has data but only omitted data (§5.4.3.2.1). - Minimum2/Maximum2 are unchanged: an Uncertain bound is a candidate only when TreatUncertainAsBad is false (§5.4.3.15). ### NumberOfTransitions (`CountAggregateCalculator`) §5.4.3.24 speaks only of "non-Bad" values, and Table 72 says "Bound Uncertain: Use as value". So: - Uncertain values are counted and can be the previous value, whatever TreatUncertainAsBad is. - The previous value is the nearest non-Bad value before the interval (the early bound). This reverses an earlier change that made NumberOfTransitions honour TreatUncertainAsBad. ### Minimum, Maximum, MinimumActualTime, MaximumActualTime, Range (`MinMaxAggregateCalculator`) Per the 11426 answer: - Every non-Bad value is a candidate, whatever TreatUncertainAsBad is. - An Uncertain extremum is returned with `UncertainDataSubNormal` plus Calculated. ActualTime keeps the timestamp of the first such value. - Otherwise the status is Good unless the interval contains Bad values (then `UncertainDataSubNormal`); it is `BadNoData` if all values are Bad. - Range is max − min over the non-Bad values. It is Uncertain if either extremum is Uncertain or Bad values exist. Before this change the server ignored an Uncertain value beyond the Good extremum and returned the Good one with an Uncertain status. Minimum2/Maximum2/Range2 are unchanged, because §5.4.3.15 ties them to TreatUncertainAsBad explicitly. ### DurationGood/DurationBad/PercentGood/PercentBad (`StatusAggregateCalculator`) - A region is Good only if its data point is Good, and Bad only if it is Bad. - Uncertain counts for neither (§5.4.3.31–34). - The first region still takes the status of the raw value at or before the start. ### Tests - New `Aggregates/BoundingValueTreatUncertainAsBadTests`: DeltaBounds, StartBound/EndBound, TimeAverage2/Total2, Interpolative, NumberOfTransitions (forward and reverse), DurationGood/Bad/PercentGood/Bad. - New Min/Max test for Bad-only and Bad-next-to-Good intervals. - Updated tests that encoded the old interpretation: - Interpolative with an Uncertain raw value at the start: 2 tests. - Duration/Percent Good/Bad: 2 tests. - NumberOfTransitions with TreatUncertainAsBad: 2 tests. - Minimum/Maximum/Range with an Uncertain extremum or only Uncertain values: 2 tests in Server.Tests, 1 in History.Tests. - `ProcessedReadUsesAdvertisedNodeAggregateDefaultsAsync`: it now probes the TreatUncertainAsBad default with Count, because PercentGood no longer depends on it. ### CTT verification CTT 1.05.513 with the patched scripts (c3674ff, which encode the 11462/11425/11426 answers), Aggregates part (p06): | Server | Errors | Warnings | Pass | NotImpl | | --- | --- | --- | --- | --- | | master |
||
|
romanett
|
f32259a1a7 |
GDS: pin SecurityAdmin-only delivery of ApplicationRegistrationChangedAuditEvents; document CTT defect C57 (#4693)
## Description CTT 1.05.513 (patched scripts) GDS Application Directory `011.js`, `018.js` and `028.js` fail on master ( |
||
|
romanett
|
cce2004fdd |
CTT: keep GeneratesEvent off Object instances and Views off non-hierarchical references; classify Deadband Err-006 as CTT defect (#4667)
# Description Three findings from the 2026-10-03 CTT run with the patched 1.05.513 scripts. Two are server/model defects and are fixed here. The third is a CTT script defect, recorded as C56 in `plans/ctt-issues.md`. ## 1. Monitored Items Deadband Filter `Err-006.js`: CTT defect (C56, no server change) The script modifies a MonitoredItem on the first static numeric scalar (a plain BaseDataVariable with no EURange) to use a PercentDeadband. It accepts only `Good` or `Bad_MonitoredItemFilterUnsupported`. Since #4629 the server returns `Bad_DeadbandFilterInvalid`, and that is the code the spec defines for this case: - OPC 10000-8 §7.2: PercentDeadband "applies only to AnalogItems with an EURange Property". - OPC 10000-8 §7.3.2 Table 61: `Bad_DeadbandFilterInvalid` is returned when "a PercentDeadband is not supported, since an EURange is not configured". CreateMonitoredItems and ModifyMonitoredItems both go through `ValidateMonitoringFilterAsync` and return the same code. Monitor Basic `Err-022.js` already accepts `Bad_FilterNotAllowed` for non-analog items, so the scripts disagree with each other here. Draft Mantis note (not filed yet): > **Monitored Items Deadband Filter Err-006.js rejects Bad_DeadbandFilterInvalid for a PercentDeadband on a node without EURange** > Err-006.js picks the first node of `/Server Test/NodeIds/Static/All Profiles/Scalar/NumericSettings`, which is normally a BaseDataVariable without an EURange. It then calls ModifyMonitoredItems with a PercentDeadband (10 %) and accepts only Good or Bad_MonitoredItemFilterUnsupported. OPC 10000-8 §7.2 limits PercentDeadband to AnalogItems with an EURange. §7.3.2 Table 61 defines Bad_DeadbandFilterInvalid for "a PercentDeadband is not supported, since an EURange is not configured", so a compliant server fails this test. Monitor Basic Err-022.js already accepts Bad_FilterNotAllowed for the non-analog fallback. > Proposed fix: add StatusCode.BadDeadbandFilterInvalid (and, consistent with Err-022.js, StatusCode.BadFilterNotAllowed) to the ExpectedAndAcceptedResults of the ModifyMonitoredItems call. ## 2. GeneratesEvent on Objects (Base Info Core Structure 2 `001.js`): source generator defect The CTT reported `ns=9;i=234` (the `<ServiceName>` placeholder of AuthorizationServicesFolderType) and `ns=9;s=AuthorizationServices/Default` as Objects that are the source of GeneratesEvent references. OPC 10000-3 §7.15 says: "The SourceNode of References of this type shall be an ObjectType, a VariableType or a Method". `OpcUaGdsModel.xml` declares both references (AccessTokenRequested/Issued audit events) correctly, on `AuthorizationServiceType`. The source generator copied the type's own non-hierarchical references onto every Object or Variable built from the type: onto instance declarations (the placeholder) and into `CreateInstanceOf<Type>`, which `AuthorizationServiceState` initialises from, and that is how the `Default` service gets them. UA-ModelCompiler does the same thing, so the official `Opc.Ua.Gds.NodeSet2.xml` in UA-Nodeset has the same two references on `ns=1;i=234`. Fix: - `TranslateReferences` now marks references declared on a type (`HierarchyReference.DefinedOnType`, which already existed but was never set). - `NodeStateGenerator.GetReferences` drops forward GeneratesEvent references (including subtypes such as AlwaysGeneratesEvent) for Object and Variable nodes when the reference came from the type. - ObjectTypes, VariableTypes and Methods keep these references. For example, the RegisterApplication methods and the Acknowledge/AddComment methods of the alarm types are unchanged. - References that a model declares explicitly on an instance are emitted as before. ## 3. View targeted by HasCause/HasEffect (Base Info Core Structure 2 `001.js`): sample model defect #4485 added `Views_Operations HasCause/HasEffect Views_Engineering` so that View Minimum Continuation Point 01 `012.js` and View Basic 2 `018.js` have at least two references to View nodes on the configured View (`/Server Test/NodeIds/NodeClasses/View` = `Views_Operations`). OPC 10000-3 §5.4 says Views "can be the SourceNode of any hierarchical Reference. They shall not be the SourceNode of any NonHierarchical Reference." `Views_Operations` now Organizes `Views_Engineering` and a new View, `Views_Maintenance`, which organizes `DataAccess`. This keeps two View-class references and adds no loops (ContainsNoLoops stays true). ## Tests - `NodeStateGeneratorTests.TypeGeneratesEventReferencesAreNotCopiedToObjectInstances`: covers the type, the placeholder, the instance, `CreateInstanceOf`, Methods and an explicit instance reference. It fails without the fix. - `ReferenceServerTest.SampleViewsHaveNoNonHierarchicalReferencesAsync`. The existing `NodeClassSampleNodesReferenceTheirOwnNodeClassAsync` still passes. - Results with net10.0: - SourceGeneration.Core.Tests: 4267 passed, 0 failed - InformationModel.Tests: 1078 passed, 0 failed - Di.Tests, including RobotOpenUsdE2e: 438 passed, 0 failed - OpenUsd.Tests: 921 passed, 0 failed - Server.Tests `ReferenceServerTest`: 56 passed, 0 failed - `UA.slnx` builds with net10.0. The changed test projects build with net48. ## CTT (patched 1.05.513 scripts, compared with the 2026-10-03 patched baseline) | Part | Change | | --- | --- | | p02-base-info | Base Info Core Structure 2 `001.js`: Error → Skipped. The GeneratesEvent and View errors are gone; the remaining skips are the CTT's own "type not defined within the test-script" notes. No other change. | | p03-base-view-method | No change. View Basic 2 `018.js` and View Minimum Continuation Point 01 `012.js` still pass. | | p13-monitored-items | Deadband `Err-006.js` still Error (C56). With the script fix it passes. Monitor Basic `007.js` gave a response-timestamp delay warning (>200 ms) caused by machine load; it also appears in the 2026-09-29 base run. | 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
2871355d18 |
GDS: implement the AliasName Server facet (OPC 10000-17 Annex C) (#4668)
## Summary Implements the **GDS AliasName Server facet** (OPC 10000-17 Annex C) in `Opc.Ua.Gds.Server`, so that the CTT conformance unit *GDS AliasName Discovery* passes apart from two CTT script defects (documented, Mantis drafts in `plans/ctt-issues.md`). Spec requirements: - **C.1:** aggregate the AliasNames of registered Servers that expose the `ALIAS` capability (OPC 10000-12 Annex D). Pull all AliasNameCategory instances and merge those whose BrowseNames have the same Name. Provide the ExpandedNodeId of every referenced node and the ServerUri of the Server that contains it. - **C.2:** on registration, merge the Server's AliasNames into a single tree for TagVariables/Topics, combine aliases that share a name, and add the ServerUri to `ServerArray`. - **C.3:** on unregistration, remove that Server's aliases and categories (but keep those other Servers still provide), remove its AliasFor references, and remove its ServerUri from `ServerArray`. ### Design - **`AliasNameAggregator`** (`Opc.Ua.Gds.Server.AliasNames`) holds one snapshot per registered application and builds the merged view: - Categories are keyed by their BrowseName path below `Aliases`. Aliases are merged by name within a category; their targets are the union over all sources. - Targets are stored as namespace-URI ExpandedNodeIds. The ServerIndex is resolved against the server's `ServerArray` each time the view is built, because removing a ServerUri renumbers the entries after it. - Only ServerUris the aggregator added itself are removed again. - Also implements `FindAlias`/`FindAliasVerbose`, recursive over sub-categories. - **`SessionAliasNameSourceReader`** reads a source over a client Session by browsing the `Aliases` (i=23470) hierarchy and the AliasFor references. - It browses instead of calling `FindAlias` because the category structure has to be aggregated too, and a source whose aliases come from a NodeSet (like the CTT's embedded server) does not implement `FindAlias`. - It connects anonymously, to the most secure endpoint first, falling back to SecurityMode None. It runs GetEndpoints once per DiscoveryUrl. - **`ApplicationsNodeManager`** (new partial `ApplicationsNodeManager.AliasNames.cs`): - Hooks RegisterApplication (now `OnCallAsync`), UpdateApplication (background re-read) and UnregisterApplication. - Materializes the merged view as `AliasNameType` and `AliasNameCategoryType` nodes in the GDS application-record namespace, organized by the well-known TagVariables/Topics/Aliases objects. Each category gets a `FindAlias` answered from the merged list. - Changes and node updates are serialized, so a read that finishes after a concurrent unregister cannot re-add the source. - RegisterApplication waits at most `AliasNameRegistrationWait` (5 s) for the read; a slower or unreachable source is finished in the background and never blocks clients past their call timeout. - Optional periodic re-read via `AliasNameRefreshInterval`; it also retries sources that could not be read. - **`IAliasNameStoreRegistry.RegisterContributor`/`UnregisterContributor`** (Opc.Ua.Server): a read-only contributor's aliases are added to the `FindAlias`/`FindAliasVerbose` results of the categories it reports, next to the owning store's. This lets the aggregated aliases share the well-known categories with the server's own aliases, which matters for the reference server, an AliasName Server and a GDS at once. Contributors never receive Add/Delete calls. - **Opt-in:** `GlobalDiscoveryServerConfiguration.EnableAliasNameAggregation` is off by default, because the GDS then opens client connections to the DiscoveryUrls of registered applications. `Ctt.ReferenceServer.Config.xml` enables it. - `Opc.Ua.Gds.Server` now references `Opc.Ua.Client`; the `Opc.Ua` meta-package already depends on both. ### CTT (1.05.513 with the patched scripts), p14-gds, reference server `--ctt`, embedded CTT source + 2 extra AliasName sources | GDS AliasName Discovery | before | after | after, with the C54/C55 script fixes | | --- | --- | --- | --- | | initialize | Skipped (no sources) | Pass | Pass | | 001 | Error | Error (C55) | Pass | | 002 | Error | Error (C54) | Warning (RegisterApplication > 100 ms) | | 003 | Pass | Pass | Pass | | 004 | Error | Error (C54) | Pass | | 005 | Skipped | Error (C55) | Pass | | 006, 007, 009, 011, 012, 014, 015 | Skipped | Error (C54) | Pass | | 008 | Skipped | Pass | Pass | | 010, 013 | Skipped | Error (C55) | Pass | - **C54:** the script compares the GDS's AliasFor target (ServerIndex > 0, namespace URI, as Annex C.1 and Part 4 §7.16 require) with the source's local NodeId using `equals()`, which can never match. - **C55:** "no remote aliases" is checked as "no aliases", so it fails on the reference server's own aliases. Both are documented with draft Mantis texts (not filed). The remaining script checks pass in 15/15 test cases. No regressions: - Application Directory and Query Applications are unchanged against the baseline. The only difference is one ActivateSession timing warning in one run. - p09 (AliasName CUs) passes 32/32, identical to the baseline. ## Tests - `AliasNameAggregatorTests` (unit) covers: - merging across Servers; - categories kept while still referenced and removed after the last source; - ServerArray add, remove and renumbering; - pre-existing ServerUris kept; - local targets; - FindAlias/FindAliasVerbose scope, pattern and reference-type filter. - `GdsAliasNameAggregationTests` (end-to-end) registers a second in-process reference server with `ALIAS` and reads it with the real session reader. It checks: - aggregated nodes and remote AliasFor targets; - the ServerArray entry; - FindAlias on TagVariables and on the aggregated category; - that unregistering removes everything; - that an unreachable source still registers; - that a Server without `ALIAS` is ignored. - Registry contributor tests in `AliasNameStoreRegistryTests`. - Opc.Ua.Gds.Tests: 1246 passed, 0 failed, 56 skipped. Opc.Ua.Server.Tests AliasNames: 198/198. InformationModel and Features alias tests pass. net48 compiles. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
4c1c3fd244 |
Aggregates: fix remaining CTT aggregate server defects and seed Bad data for the CTT (#4655)
Fixes the remaining CTT Aggregate failures of the reference server that are server defects or missing test data, using the patched CTT 1.05.513 scripts (all known Mantis defects fixed) as the oracle. ## Server fixes (`src/Opc.Ua.Server`) - **Real-valued bounds for TimeAverage/TimeAverage2/Total/Total2.** `SlopedInterpolate` converted every interpolated bound to the raw data type, so on Int32 nodes these Double aggregates used rounded bounds (ramp 0..23.8: 11.9034 instead of 11.9). The conversion (Part 4 §7.7.3) is now only applied for aggregates that return the raw data type (Interpolative, Start/EndBound, DeltaBounds, Minimum2/Maximum2/Range2, ActualTime2); `AverageAggregateCalculator` keeps the real bound (Part 13 §3.1.8, Table 50). - **Non-numeric values are interpolated stepped.** Boolean, String and other non-numeric values cannot be interpolated with a sloped line (Part 13 §5.4.2.3, Table 50). The calculator now uses stepped interpolation for them whatever the Stepped property says: a String EndBound between "23" and "24" is "23" (was "23.8"), a Boolean bound between false and true is false, and Boolean DurationInState no longer counts the region before the first value as true. - **TreatUncertainAsBad = false in Average and Count.** Both used only strictly Good values for the result but counted Uncertain values as Good for the status. They now include Uncertain values when TreatUncertainAsBad is false (Part 13 §4.2.1.2), like StandardDeviation and NumberOfTransitions already did. - **Interpolative honors TreatUncertainAsBad for a raw value at the interval start**, as the calculator already did for the raw values around a bound. - **NumberOfTransitions**: a Bad result carries no aggregate bits, like Count. - **Processed reads get the bounding values they need** (`HistorianDispatcher`). The raw read only returned the raw value at or beyond each end of the request. When that value is Bad (or Uncertain with TreatUncertainAsBad), the interpolated bound needs the nearest Good value further out, so an interval starting on a Bad value returned `BadNoData` and the next interval lost its first region. The dispatcher now reads outwards to the nearest Good values (reusing the ReadAtTime bound search), only when the end value is not Good. ## Reference server test data and CTT template - Every scalar history node also gets 400 values at a fixed date (`2026-01-01T00:00:00Z`) with a block of five Bad values, laid out for the CTT's Bad-data search (`HAAggregateHelper.GetBadData`). StartOfArchive is the fixed date. The values relative to the server start are unchanged. - `samples/UAReferenceServer.ctt.xml`: all ten `.../Aggregates/StartOfBadData*` settings point at the block (`2026-01-01T00:25:01.234Z`), and `RequestedNodeId` is enabled with namespace 2 (needs the C34 fix). - `plans/ctt-issues.md` / `plans/ctt-testing.md`: updated configuration notes, the oracle differences, U4, and new U5 (TreatUncertainAsBad = false). Also documents a CTT defect: the Bad-data search's "reverse" read sends EndTime = 0001-01-01 (null), which a server reads forward. ## CTT results (p06 Aggregates, 1267 test cases) | | Error cases | | --- | --- | | Before (patched scripts, origin/master) | 253 | | This PR, patched scripts | 125 | | This PR, patched scripts + proposed Int32 oracle corrections for TimeAverage/Total(2) and Range2 | 51 | The 125 include 78 TimeAverage/TimeAverage2/Total/Total2 and 4 Range2 test cases on Int32 nodes where the server now returns the Part 13 value but the patched oracle still truncates the bounds. The remaining 51 are spec questions: U4 (DeltaBounds, 9; TimeAverage2/Total2 with an Uncertain value after the start bound, 8) and U5 (TreatUncertainAsBad = false, 34). p07 Historical Access: no change (0 errors). ## Tests - New `AggregateBoundDataTypeTests` (real bounds on Int32, Interpolative still rounds, Boolean/String stepped bounds, Boolean DurationInState, Average with TreatUncertainAsBad). - `AggregateCttRegressionTests`: direct and live (dispatcher) interval starting on a Bad value, Interpolative with TreatUncertainAsBad, NumberOfTransitions Bad result; the Count test now expects Uncertain values to be counted with TreatUncertainAsBad = false. - `AggregateCalculatorCoverageTests`: an Uncertain raw value at the start with TreatUncertainAsBad is interpolated over. - `Opc.Ua.Server.Tests` and `Opc.Ua.History.Tests` pass on net10.0; net48 builds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
3442950647 |
Return Bad_IndexRangeNoData for writes to a range beyond the target (CTT regression from #4573) (#4593)
## Description Fixes a CTT regression introduced by #4573, found in a full CTT run of master after the merge. **Attribute Write Index `Err-001.js` went Pass → Error.** The test writes a range that starts beyond the end of each array (`1073741824:1073741824+len`), with data whose size differs from the range by one element. It expects `Bad_IndexRangeNoData` or `Bad_WriteNotSupported`. #4573 checked the size of the written data before it checked whether the range lies within the target, so the server returned `Bad_IndexRangeDataMismatch`. Part 4 §7.27 says no data exists within such a range, so `Bad_IndexRangeNoData` is the correct result. `NumericRange.UpdateRange` now checks the lower bounds against the target first, for arrays, Strings, ByteStrings and matrices. A size mismatch within the target still returns `Bad_IndexRangeDataMismatch` (Part 4 §5.11.4.4), and the existing mismatch tests are unchanged. Also in this PR: `plans/ctt-issues.md` C1 now lists Aggregate – NumberOfTransitions `003-02.js` and `004-02.js`. Since #4545, NumberOfTransitions honours TreatUncertainAsBad, and the harness always sends `UseServerCapabilitiesDefaults = TRUE` (Mantis 11420). ## CTT run (scripts 1.05.513, all 23 parts) The run compared master with #4573 merged (`20c2d77d5`) against the last recorded full run (2026-09-24, `22d07a31e`) at test-case level. It also compared against master just before the merge (`87c35b84b`). | Change vs recorded run | Cause | | --- | --- | | Attribute Write Index `Err-001.js` Pass → Error | This PR fixes it; with the fix: Pass, 0 errors in the group | | NumberOfTransitions: 11 cases Error → Pass; `003-02`/`004-02` Pass → Error | #4545 (intentional), CTT defect C1 | | Subscription Basic `Err-002.js` | Local project still claimed 100 subscriptions while #4545 raised the server to 200; passes with the checked-in template value | | A & C Refresh2/Shelving/Confirm: 9 cases improved | — | Every other part is identical at test-case level, apart from the usual "Timestamp shows a delay" warnings. All remaining errors are documented CTT defects. ## Tests - 3 new `NumericRangeTests` (array, String, matrix: range beyond the target with a wrong-size slice → `Bad_IndexRangeNoData`) - `Opc.Ua.Types.Tests` (net10.0): 11006 passed, 0 failed 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
d6356766c5 |
Resolve the open CTT findings: aggregate fixes, de-DE ApplicationName and CTT configuration (#4545)
# Description Resolves the open items of `plans/ctt-issues.md` (*Needs clarification*, *Open server findings*, *CTT project configuration notes*). Each item was checked against the OPC 10000 spec text. A server change was made only where the server was wrong, and every server change has a regression test. CTT script and oracle defects are filed in Mantis instead. ### Server changes (`Opc.Ua.Server`) - **NumberOfTransitions honours TreatUncertainAsBad.** - Part 13 §5.4.3.24 does not count Bad values. §4.2.1.2 makes Uncertain values "equivalent to Bad" when TreatUncertainAsBad is True, and the aggregate definition doesn't override that. - Uncertain values are therefore no longer counted, and no longer used as the previous value, when the setting is True. The now unused `TimeSlice.NonBadEarlyBound` is removed. - **DurationInStateZero/NonZero status follows the variable's interpolation.** - §5.4.3.2.2: "A region is Uncertain if a region ends in a Bad or Uncertain value and SlopedInterpolation is used." - The calculator always built stepped status regions. So on a sloped variable, the last region stayed Good while TimeAverage2 and Total2 over the same interval were already Uncertain. - The duration value doesn't change. - **AnnotationCount returns `Bad_NoData` and sets Partial.** - Table 68 requires `Bad_NoData` for intervals before the start or after the end of data. §5.3.3.2 requires the Partial bit for incomplete intervals. - `HistorianDispatcher` now derives the start and end of data from the raw history (read with bounds) plus the annotation timestamps. It passes them to a new `CalculateAnnotationCounts` overload; the old overload is unchanged. - Three existing dispatcher tests change their expectations. For example, a node without any data now returns `Bad_NoData` instead of Good zeros. ### Reference server sample and CTT configuration - **`de-DE` ApplicationName.** The server advertises `de-DE` in LocaleIdArray, but FindServers and GetEndpoints always returned the en-US name. The sample now keys the ApplicationName and registers en-US and de-DE translations (Part 4 §5.5.2.2 / §5.5.4.2 localeIds). - **Subscription limit.** `Max Supported Subscriptions` in `samples/UAReferenceServer.ctt.xml` and `MaxSubscriptionCount` in `Ctt.ReferenceServer.Config.xml` go from 100 to 200. Subscription Publish Min 05 `003.js` needs 190 subscriptions. - **Alarm Cycle Time.** The CTT template's Alarm Cycle Time is 30 s. At 60 s it equalled `RequestedSessionTimeout`, so a Limit/Level CU that starts a run finds its session expired (C40). ### Tests only - **AddNodes duplicate BrowseName test:** `AddNodeAsync_DuplicateBrowseNameUnderParent_ReturnsBadBrowseNameDuplicatedAsync` never ran. Its items had no TypeDefinition, so its `Assume` always made it inconclusive. The finding it belongs to (AddNodes after a rename returning `BadNodeIdExists`) was already fixed by #4477. - **RequestHandle echo:** a new test covers the ServiceFault for an oversized argument in a chunked request (already fixed by #4488). - **Value-based status:** a new test covers the aggregate status with TreatUncertainAsBad = false (already fixed by #4503). ### Not changed - **GDS QueryApplications/QueryServers still return records with ServerCapability `NA`.** OPC 10000-12 §6.5.10/§6.5.11 says they "shall not", but the CTT GDS scripts expect those records. Implementing the exclusion turned 40 GDS test cases from pass to error, so it waits for the script fix ([Mantis 11458](https://mantis.opcfoundation.org/view.php?id=11458)). - **Idle SecureChannels are still closed after `ChannelLifetime`.** Keeping them open would be security relevant. The CTT configuration's `ChannelLifetime` of 120000 (#4502) covers the CTT case. ### Mantis issues filed | Id | Issue | | --- | --- | | C51 | [11458](https://mantis.opcfoundation.org/view.php?id=11458) GDS scripts expect NA records in query results | | C52 | [11459](https://mantis.opcfoundation.org/view.php?id=11459) A & C Refresh scripts leak subscriptions on the shared alarm session | | C53 | [11460](https://mantis.opcfoundation.org/view.php?id=11460) `StartThreadPublish.js` defaults misspelled properties | | U2 | [11461](https://mantis.opcfoundation.org/view.php?id=11461) Minimum2 oracle on reverse reads | | U4 | [11462](https://mantis.opcfoundation.org/view.php?id=11462) Part 13 clarification: TreatUncertainAsBad and simple bounds (DeltaBounds) | The A & C `BadSubscriptionIdInvalid` failures (Refresh, Refresh2, Shelving) come from CPU load: with other processes at 100 % CPU, all subscriptions of the CTT alarm session expire within one second. This happens with and without this PR (3 of 3 loaded runs, 0 of 2 idle runs). ### CTT verification CTT 1.05.06, scripts 1.05.513, compared at test-case level against a build of the starting commit: | Group | Before | After | | --- | --- | --- | | Aggregates | 4,562 error messages | 4,186. NumberOfTransitions: 12 test cases go from error to pass. AnnotationCount `005-01.js`/`005-02.js` pass. NumberOfTransitions `003-02.js`/`004-02.js` now fail because of C1 ([Mantis 11420](https://mantis.opcfoundation.org/view.php?id=11420)): they request TreatUncertainAsBad = false, but the harness makes the server use its default of true. | | Discovery | 10 warnings | 0. Find Servers Filter `003.js`/`006.js` and Get Endpoints `002.js` pass. | | Subscription | – | Publish Min 05 `003.js` goes from warning to pass. | | Historical Access, GDS, Node Management | – | no new failures | `plans/ctt-issues.md` is updated: resolved items leave the open sections, and the new Mantis issues are recorded. ## Related Issues - Related to #4479 (CTT configuration notes) - Follows #4502 ## 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. - [x] I have added all necessary documentation. - [ ] 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. Tests run locally on net10.0 after merging master: | Project | Filter | Passed | | --- | --- | --- | | `Opc.Ua.Server.Tests` | Aggregate, Historian, NodeManagement, Publish, Subscription | 1,419 | | `Opc.Ua.Core.Tests` | TcpServerChannelBuffer | 65 | | `Opc.Ua.History.Tests` | Aggregate, Annotation | 67 | | `Opc.Ua.Sessions.Tests` | discovery locale tests | 8 | Before the merge, the whole `Opc.Ua.Sessions.Tests` ClientTest fixture passed (372 passed, 0 failed). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
romanett
|
13f96fd4ba |
Raise ChannelLifetime for the CTT reference server and record the full CTT run of the merged CTT PRs (#4502)
# Description Findings of a full CTT run (23 parts, 3,752 test cases, CTT 1.05.06 / scripts 1.05.513) against the merge of #4477, #4482, #4485 and #4486, which are all on master now, plus the CTT defects filed in Mantis. No server code changes: the PR changes only the CTT configuration of the reference server sample and the `plans/` documentation. ## CTT configuration: raise `ChannelLifetime` to 120 s CTT Security None `007.js` and Security Basic256Sha256 `005.js` failed (also on master): they keep SecureChannels without a Session idle for about 51 s (60 s token, stretched by the CTT's CloseSession delay, C50) and expect the newest one to close with Good. `TcpTransportListener.DetectInactiveChannels` had already closed it after `ChannelLifetime` (30 s) without traffic (`CloseSecureChannel().Result received BadInvalidState`). `samples/Reference/ConsoleReferenceServer/Ctt.ReferenceServer.Config.xml` now sets `ChannelLifetime` to 120000. The default of 30000 and the server's inactivity cleanup are unchanged. An earlier revision of this PR changed the inactivity cleanup instead, keeping idle Session-less channels open until their token expired (Part 4 §5.6.2.1). That change was withdrawn because it lets an unauthenticated client hold a channel for up to the `SecurityTokenLifetime` (one hour by default) instead of 30 s. The difference to Part 4 §5.6.2.1 is recorded as an open server finding in `plans/ctt-issues.md`. Validated on 2026-09-24 with the reverted code and the new configuration (full run, compared with the run of 2026-09-22 at test-case level): - Security None `007.js` passes and Security Basic256Sha256 `005.js` has only the C50 CloseSession delay warnings (both failed with 30 s). - All other parts have the same error signatures as before; the remaining differences are *"Response Timestamp shows a delay"* warnings from a loaded machine (a back-to-back Session Services run with 30 s and 120 s on an idle machine is identical). - A & C Refresh2 `Err_003.js`/`Err_004.js` and Shelving `initialize.js` report `BadSubscriptionIdInvalid` in today's runs with both 30 s and 120 s on the same build, so they are unrelated to the configuration (both passed again in a run on the idle machine with 120 s). ## Mantis 32 CTT defects filed on 2026-09-24 (11420–11454, script issues in *CTT UA Scripts*, native aggregate oracle issues in *CTT UA Binary*); 12 drafts were already covered by existing reports (including GDS issues 11406–11413 by node-opcua and 11102 for the `opc.https`/`opc.wss` endpoint selection); 5 are held back for further investigation. `plans/ctt-issues.md` links every entry. ## CTT documentation (`plans/ctt-issues.md`, `plans/ctt-testing.md`) - `ctt-issues.md` lists every failing CTT test case with a short abstract and its Mantis issue; the details live in Mantis. It keeps the defects not filed yet, the open questions (U1–U4), the open server findings and the CTT project configuration notes, and no longer lists findings the server has fixed. - Notable CTT findings of this run: the aggregate oracle lacks the Part 13 Uncertain status and Calculated bit rules that #4477 implements (C48/C49, 24 test cases); the CloseSession "delay in excess of 600ms" warnings are a CTT client artifact (C50: the server answers in 2–5 ms, the CTT stops the Session's SessionThread between stamping and sending); the CTT's `FindTokenType` rewrites its cached endpoints, so Security User Name Password 2 `015.js` reports duplicate PolicyIds (C36); A & C Comment passes or skips depending on the date (C39). - Open server finding: AddNodes after a child rename returns `BadNodeIdExists` (#4477 × #4486 interaction, not covered by the CTT). - `ctt-testing.md` is a runbook for agents: setup, headless runs, selection generation, reading and comparing results, triage, recipes per conformance group (A & C, Session/Subscription, Monitored Items, security groups with an isolated PKI) and a full partitioned run of every group (about 80 minutes). It points to `ctt-issues.md` for the failures to expect. ## Related Issues - Follow-up to #4477, #4482, #4485, #4486 ## 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. - [ ] 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. - [ ] 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. Validated locally with a full partitioned CTT run of the Release net10.0 reference server (reverted code, `ChannelLifetime` 120000). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
romanett
|
2415503ab9 |
Close CTT skips in the reference server sample address space, diagnostics EnabledFlag and project settings (#4485)
Closes #4479 The CTT skipped a number of reference-server test cases because the sample address space, the diagnostics EnabledFlag or the CTT project settings did not provide what the scripts need. This PR fixes the sample data and the server behaviour behind those skips, updates `samples/UAReferenceServer.ctt.xml`, and records the CTT script defects that the newly running cases expose. ## Changes ### Sample address space (`ReferenceServer.NodeSet2.xml`, `ReferenceNodeManager`) | # | Skip | Fix | | --- | --- | --- | | 1 | Base Info Engineering Units `004.js`: ArrayItemType `EngineeringUnits` have an empty NamespaceUri | UNECE `mV` (12890) EUInformation, and the axis definitions use `s` (5457219) instead of UnitId 0. | | 2 | Data Access PercentDeadband `006/016/017`: `DataAccess_AnalogType_Float` EURange 0..0 | EURange 0..100 like the other analog items. | | 3 | Attribute Read `033.js` (19 nodes): second dimension of 3 | All `Scalar_Static_Arrays2D_*` are 5x5 (generated values; the Variant matrix holds scalar Int32 elements). The code-seeded Integer/Number/UInteger matrices are 5x5 too. | | 4 | View Minimum Continuation Point 01 `012.js`, View Basic 2 `018.js`: Method/View nodes need two references of their own NodeClass | `Methods_Void` HasCause/HasEffect to two methods, `Views_Operations` HasCause/HasEffect to `Views_Engineering`. | | 5 | Base Info Server Capabilities 2 `002.js`: no node without MinimumSamplingInterval | `DataAccess_DataItem_String` reports `Bad_AttributeIdInvalid` for the optional attribute (Part 3 §5.6.2). A DA item is used because Address Space WriteMask writes every attribute of the static scalars back. | | 6 | Historical Access Read Raw `Err-025.js`: no non-historizing Static Scalar | New `Scalar_Static_NonHistorizing_Boolean` (HistoryRead access, Historizing = false). See the historian change below. | | 10 | Address Space Atomicity `001.js`: no NonatomicRead node found | New `AccessRights_AccessAll_NonatomicReadWrite`. The script examines only the first 10000 variables sorted by NodeId string; the 5000 mass variables sort before `Scalar_Static_NonatomicReadWrite`. | ### Server library - **Diagnostics EnabledFlag (item 7, Base Info Diagnostics `018-1/2/3`).** `Server.ServerDiagnostics.EnabledFlag` is writable for SecurityAdmin or ConfigureAdmin over SignAndEncrypt (UserAccessLevel hook, asynchronous write handler); other users get `Bad_UserAccessDenied`. Transitions are serialized, including the node removal. Disabling returns `Bad_NotReadable` for ServerDiagnosticsSummary, its children and the diagnostics arrays, and removes the session and subscription diagnostics nodes (Part 5 §6.3.3). The server keeps the registrations: enabling restores the nodes of the live sessions and subscriptions with the same NodeIds (sessions created while disabled included) and resets the arrays to Good. Counters stay cumulative (required by `018-1.js`). Subscriptions created while diagnostics are disabled still get no diagnostics node. - `DiagnosticsNodeManager.IsDiagnosticsStructureNode` had an inverted NodeId check, so monitored diagnostics nodes never started the scan timer; `OnMonitoringModeChangedAsync` counted every node and replaced a running timer. Both fixed. - **Non-historized nodes (item 6).** `HistorianDispatcher` raw/modified, processed and at-time reads ask the resolved provider `IsHistorizingAsync` and return `Bad_HistoryOperationUnsupported` for a node it does not historize, instead of the empty result the in-memory historian gave for any node. - **Browse order (item 8).** `ReferenceDictionary.Find` returned references of a type from hash-based target maps, so a Browse filtered by ReferenceType returned references in a different order than an unfiltered Browse. It now returns them in insertion order (no extra allocation; a single match keeps the fast path). View Basic 2 `015.js` compares both lists by index. The characterization test in `NodeStateReferenceStorageTests` is updated accordingly. ### CTT project settings (`samples/UAReferenceServer.ctt.xml`) | Setting | Old | New | | --- | --- | --- | | `/Server Test/NodeIds/Static/All Profiles/Scalar/Bool` | `ns=2;s=Scalar_Static_Boolean` | `ns=2;s=Scalar_Static_NonHistorizing_Boolean` | | `/Server Test/NodeIds/References/Has References of a ReferenceType and SubType` | `ns=2;s=References_HasReferenceTypeAndSubType` | `i=2253` | | `/Server Test/NodeIds/NodeClasses/Object` | `ns=2;s=Scalar` | `i=2253` | `References_HasReferenceTypeAndSubType` loses its HasComponent/HasOrderedComponent references in the source generator; that is tracked in #4484 and not changed here. The Server object has HasComponent and HasAddIn references. ### Decisions - **Item 11, Attribute Write StatusCode & Timestamp `003/006/008` (writing ServerTimestamp): unchanged.** The stack rejects writes that contain a ServerTimestamp (`NodeState`/`BaseVariableState`), which Part 4 §5.11.4.2 allows (`Bad_WriteNotSupported`); the CTT reports it as "Not Supported", not as an error. Side finding, not changed: the StatusWrite/TimestampWrite AccessLevel bits are not enforced. - **Item 6:** Part 4 §5.11.3.4 (Table 52) specifies `Bad_HistoryOperationUnsupported`; `Err-025.js` accepts only `Bad_NotSupported`, so the case now runs and reports that as an error. Recorded as a CTT defect in `plans/ctt-issues.md`. - **Item 12:** removed the stale Historical Access `Err-012.js` entry (fixed by #4026). `plans/ctt-issues.md` adds C32 (`Err-025.js` status code), C33 (Atomicity `001.js`: 10000 cap, operator precedence, substring matching of `UaNodesToIgnore`) and C34 (View Basic 2 `015.js` index comparison), and replaces the Err-025/Err-012 configuration notes with the new settings and the EnabledFlag access requirement. Monitor Basic `039.js` is already C17. The run procedure in `plans/ctt-testing.md` is unchanged. ## Verification CTT 1.05.06, scripts 1.05.513, `ConsoleReferenceServer --ctt -a`, fresh server per conformance unit/group. "master" = |
||
|
romanett
|
93559d06d3 |
Fix sample A&C refresh and alarm timing found by slow CTT Alarms and Conditions runs (#4482)
## Why the CTT *Alarms and Conditions* group was so slow
I measured every A&C CU with a fresh `ConsoleReferenceServer --ctt` (CTT
1.05.06, scripts 1.05.513), read the per-test `ResultNode` timestamps,
and added `addLog` timing/counter instrumentation to a copy of the CTT
project.
| Cause | Effect |
| --- | --- |
| **CertificateExpiration** opens a modal dialog in `initialize.js`
("adjust the clock on the server"), and it also appears in `--hidden`
runs | The CTT never exits. This is why the whole group "did not finish
in 40 minutes". |
| **Limit/Level CUs:** `initialize.js` idles the CU session for one
Alarm Cycle Time (60 s) during the initial capture.
`RequestedSessionTimeout` is also 60 s. | The server correctly closes
the session. `LimitHelper` then gets `BadSessionIdInvalid` for its 5
filter subscriptions, and Test_003–006 each run 180 s, about 14 min per
CU. |
| Alarm `Test_002` never gives the AcknowledgeableCondition type a
result | Always runs 3 × cycle (180 s). |
| Enable `Test_003` needs every alarm type to go active within cycle/10
| Timed out in 6 of 7 runs, because boolean and analog alarms changed
state on different ticks. |
| Enable `Err_004` answers each event with Disable/Disable/Enable |
Feedback burst, after which the CTT alarm thread sometimes returns no
events. Details below. |
| Comment `Test_001`–`004` compare against
`CallHelper.ServerTimeOfCall`, which the CTT never sets (`0001-01-01`) |
Every comment event counts as unrelated, and the tests skip after 3
retries. The server's events carry the expected comment. |
Measured CU by CU, the group took **about 87 min plus the hang**. With
the fixes below, running the whole group in one CTT process without
CertificateExpiration takes **18:45** at Alarm Cycle Time 30. The per-CU
table is in `plans/ctt-testing.md` §6.
## Server changes
- **Refresh one subscription per request, not one per Call**
(`AlarmNodeManager.CallAsync`). The sample answered every
ConditionRefresh/ConditionRefresh2 after the first one in a Call with
`Bad_RefreshInProgress`, even for other subscriptions, and never
forwarded them. So CTT Refresh `Err_004` (10 subscriptions per Call)
only ever refreshed one of them. The check is now keyed by
SubscriptionId and MonitoredItemId.
- Regression tests:
`AlarmsAndConditionsRefreshTests.ConditionRefresh[2]OfDifferentSubscriptionsInOneCallSucceedsAsync`.
Both fail without the fix.
- **Boolean alarm source now uses the analog source's steps and limits**
(`AlarmController`, `ConditionTypeHolder.GetSeverity`). OffNormal,
Discrete and SystemOffNormal alarms now change state on the same tick as
the limit alarms (5 per second, 70/90/30/10), and the shortest state
still lasts 4 s.
- With this, Enable `Test_003` passed in 4 s instead of a timeout.
Confirm `Test_001` for the boolean types also stopped failing in both
group runs.
- Test:
`AlarmControllerTests.BooleanAndAnalogSourcesChangeStateOnTheSameStep`
(fails without the change).
- **Alarm controllers step on shared interval boundaries**
(`AlarmController.SetNextInterval`). Scheduling relative to the last
step let the two controllers drift up to one 100 ms simulation tick
apart, so the boolean events sometimes landed in a later publish and
Enable `Test_003` still timed out in 3 of 5 runs. With shared boundaries
it passed in 6 of 6 runs.
- Test:
`AlarmControllerTests.ControllersScheduleTheirStepsOnSharedIntervalBoundaries`.
I did not make the simulation faster. Acknowledge, Confirm and Comment
tests call methods with the EventId of the event they just received, and
a shorter state would make them race the next transition.
## Not reproduced / not a server defect
- **Refresh `Err_004` `BadSubscriptionIdInvalid` cascade.** In the
original run the alarm-thread session's subscriptions (15.5 s lifetime)
were gone after the script's 30 s wait. Nine further CTT runs passed all
28 Refresh/Refresh2 cases. An in-process replay with a sequential
publisher kept every subscription; that replay is what exposed the
refresh defect above.
- **Enable event stall (C43) is inside the CTT.** A server build that
logs every returned notification message showed that during a stall the
server sent 435 and 420 events to the two CTT event items in 173
messages, and the CTT acknowledged all of them, while its alarm-thread
buffers returned nothing. It only happens after `Err_004`'s
Disable/Enable feedback burst, in 8 of 28 runs.
## Docs
- `plans/ctt-testing.md`: new §6 *Alarms and Conditions*, covering the
recommended run, where the time goes, how to instrument a run and the
expected result. Also a note that a hidden CTT run can block on dialogs,
so always use a timeout.
- `plans/ctt-issues.md`: the A&C "open server findings" are replaced
with these results, and there are new CTT script defects C39–C44 plus
configuration notes (Shelving chattering alarms, Alarm Cycle Time vs
session timeout).
## Tests
- `Opc.Ua.History.Tests` A&C + AlarmClient: 155 passed.
- `Opc.Ua.Server.Tests` `AlarmControllerTests` /
`QuickstartsServerUtilitiesTests`: 4 passed.
- CTT whole-group runs (27 CUs, 128 cases): 18:45 at cycle 30 and 29:23
at cycle 60 on this branch; 23:43 and 24:21 with only the refresh fix.
The spread comes from the Enable stall (C43).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
|
||
|
romanett
|
162fc5ebb3 |
Fix AddNodes scaling and OpenSecureChannel revocation errors found by CTT Monitored Item, Node Management and security runs (#4486)
# Description Findings from running the CTT conformance groups **Monitored Item Services**, **Node Management Services**, **Security General** and **Security User Token** against `ConsoleReferenceServer --ctt` (CTT 1.05.06, scripts 1.05.513). The security groups ran **without `-a`**, against an isolated server PKI built from the CTT's `copyToServer` folders, so the negative certificate tests are meaningful. ## Server fixes ### AddNodes below one parent was quadratic (Node Management Delete Node `Err-002.js`) The CTT adds 15,000 variables below `ns=2;s=CTT` in batches of 5,000 and warned that AddNodes responses took 400–800 ms. For every added node the server searched all children of the parent twice: once in the BrowseName duplicate check in `AsyncCustomNodeManager.AddNodeCoreAsync`, which also copied the children list, and once in the NodeVersion lookup of `FilterChangesByNodeVersion`. - `NodeState.FindChild` now searches children lists with ≥ 64 entries through a BrowseName index kept in a `ConditionalWeakTable`, so nodes with few children get no extra field. - It keeps first-match semantics and is maintained by `AddChild`/`RemoveChild`. - It is dropped on replacement and invalidated when a child is renamed or moves to another parent. - Lists whose children point at another parent (clones) are still searched linearly. - The duplicate check uses `FindChild` instead of copying the list. | 5,000-node AddNodes batch (in-process, Debug) | 1st | 2nd | 3rd | | --- | --- | --- | --- | | before | 769 ms | 2,083 ms | 4,011 ms | | after | 141 ms | 166 ms | 172 ms | In the CTT, AddNodes of 5,000 items now answers in 51–72 ms and the warning is gone. ### OpenSecureChannel leaked revocation errors (Security Certificate Validation `002.js`) Part 4 §6.1.3, Table 106: - For the revocation check (`Bad_CertificateRevoked`, `Bad_CertificateIssuerRevoked`), `Bad_SecurityChecksFailed` **shall** be reported to the client. - For a missing revocation list (`Bad_CertificateRevocationUnknown`, `Bad_CertificateIssuerRevocationUnknown`), it **should** be. `TcpServerChannel` masked only `Bad_CertificateRevoked`, and `StandardServer.OnApplicationCertificateError` (CreateSession) had its own list that missed `Bad_CertificatePolicyCheckFailed`. Both now use one helper, `CertificateErrorReporting` in Opc.Ua.Core, which also inspects the nested errors of the certificate validator (both paths now throw the validation result with `ThrowIfInvalid`). Only validity period, host name, URI and usage errors keep their own code, and audit events still carry the specific result. The CTT warning is gone. ### Sample data - **Monitor Value Change V2 `020.js`** skipped because `Scalar_Static_Arrays_ByteString` had a 1-byte and a 3-byte element. The script checks every element, and the simulation only padded elements 0–2. The seed value now has no element shorter than four bytes, and the simulation pads every element. - **Monitor Value Change V2 `042.js`** got 18 of 19 notifications. The missing item is the Float matrix: its element [1,1] was `-1.03179664E+33`, where adding 1 is below Float precision, so the value really did not change. The server did not drop a notification. The element is now `-1031.79664`. ## CTT results after the fixes | Group | Result | | --- | --- | | Monitored Item Services (208) | Monitor Value Change V2 44/44. Remaining: Monitor Basic `039.js` (C17, missing include), `038.js` warning (C32), and 1–2 sporadic timestamp-tolerance warnings that move between test cases from run to run. | | Node Management Services (15) | Only Add Node `Err-008.js` (Mantis 11266). | | Security General (314) | Certificate Validation: 23 pass, 24 not implemented, 3 not-applicable skips. None CreateSession/ActivateSession: all pass. The other 50 CUs are *Not Implemented* in scripts 1.05.513. | | Security User Token (66) | Anonymous `002.js` (C35) and User Name Password 2 `015.js` (C36) fail because of CTT defects. X509: 18/18 automated cases pass. | ## CTT defects documented (`plans/ctt-issues.md`) - **C32** `038.js` compares a RevisedSamplingInterval of 0 with a project setting instead of the node's MinimumSamplingInterval or the server's MinSupportedSampleRate. - **C33** `020.js` requires every ByteString element to be ≥ 4 bytes, but its message says elements 0–2. - **C34** `RequestedNewNodeId()` always sends `ns=1`, regardless of `RequestedNodeId_Namespace`, so client NodeIds can't be enabled. - **C35** Security User Anonymous picks the `opc.wss` endpoint, which the CTT cannot open (`BadNotSupported`). - **C36** `UaEndpointDescription.FindTokenType` rewrites the cached endpoints' token policies, so `015.js` reports PolicyIds as reused. The server's GetEndpoints output was verified compliant (PolicyIds unique per configuration, Mantis 11258). - Evidence added to **C9** (042.js) and **issue 17** (015.js). `plans/ctt-testing.md` gains how to run the Monitored Item/Node Management groups and a recommended setup for the security groups: isolated PKI from `copyToServer`, no `-a`, PKI restore per part, and waiting for `Server started`. Other findings recorded there: - the Create/DeleteMonitoredItems timing warnings are jitter; - `LoginNameAccessDenied` is not a real user; - `samples\UAReferenceServer.ctt.xml` needs no change (enabling `RequestedNodeId` breaks six Add Node cases because of C34). **Note:** `plans/ctt-testing.md` and the refreshed `plans/ctt-issues.md` are introduced by #4482. The plans files here are based on #4482's versions, and new entries start at C32 because #4481 uses C19–C31. Please merge after #4482 (or rebase the plans files onto it). ## Tests - `NodeStateChildNameIndexTests` (Opc.Ua.Types.Tests): index semantics (duplicates, remove, rename, replace, re-parent, clone, shrink/grow, 2,000 random operations against a linear search). Also a scan guard: 5,000 misses against 50,000 children take 97 ms with the index and 7,083 ms with it disabled. - `AsyncCustomNodeManagerNodeManagementTests.AddNodeAsync_DuplicateBrowseNameUnderParentWithManyChildren…` - `TcpServerChannelCertificateErrorTests` (Opc.Ua.Core.Tests): masked vs reported certificate errors. - Opc.Ua.Server.Tests 5,553 passed, Opc.Ua.Core.Security.Tests 556 passed, Opc.Ua.Core.Tests (Security + Stack) 3,561 passed, InformationModel NodeManagement 26 passed (net10.0). - Opc.Ua.Types.Tests 10,404 passed and 1 failed: `WotSpecFixtureManifestTests.EveryEmbeddedExampleMatchesItsRecordedHash`. That test hashes WoT assets which are `eol=lf` in the index but CRLF in this worktree's checkout; no WoT file is touched by this PR. ## Checklist - [x] I have signed the CLA and read the CONTRIBUTING doc. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added all necessary documentation. - [ ] I ran all tests locally using UA.slnx against .NET Framework and .NET 10 (net10.0 suites listed above only). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> |
||
|
romanett
|
484d6c8870 |
Fix ActivateSession deadlock on expired sessions and keep-alive after durable subscription transfer found by CTT Session/Subscription runs (#4487)
Fixes two server defects found by running the CTT (UA 1.05.06, scripts 1.05.513) conformance groups **Session Services** and **Subscription Services** against `ConsoleReferenceServer --ctt -a`, and documents every remaining failure. ## Server fixes ### ActivateSession on an expired session deadlocked the session manager `SessionManager.ActivateSessionAsync` detected `session.HasExpired` while holding the session-manager `SemaphoreSlim` and then called `IServerInternal.CloseSessionAsync`, which ends in `SessionManager.CloseSessionAsync` waiting for the same non-reentrant semaphore. The activation never returned and every later CreateSession/ActivateSession blocked, so the server stopped accepting sessions. - CTT: Session Base `002.js` (stall a session past its timeout, then activate it) hung for the CTT's 20 s request timeout. After it, 19 Session Base test cases, all of Session Change User, and the `initialize.js` of Session Cancel and Session Multiple failed with `BadTimeout` on CreateSession. The group took 9:03. - Fix: detect expiry under the lock, release it, then count `SessionTimeoutCount`, report the audit event, close the session and return `Bad_SessionClosed` (Part 4 §5.7.2). - Test: `SessionManagerExpiryTests` uses a `SessionManager` without the monitor loop and a `FakeTimeProvider`, so the expired-session path is deterministic. It fails without the fix (the 30 s deadlock guard trips). ### First Publish after TransferSubscriptions of an abandoned subscription returned a keep-alive While a (durable) subscription has no Session, the publish timer keeps counting its keep-alive but moves ready monitored items to the publish list only when `Session != null`. After the transfer, the first Publish found the keep-alive due and nothing scheduled, and returned an empty keep-alive although buffered values were ready (Part 4 §5.14.1.1). The data only arrived one Publish later. - CTT: Subscription Durable `004.js`, *"Didn't receive the data from the transferred subscription"*. It passed when the CU ran alone and failed in the whole-group run. - Fix: `Subscription.InnerPublish` moves ready items from the check list to the publish list before it would send a keep-alive. - Test: `SubscriptionTests.FirstPublishAfterTransferOfAbandonedSubscriptionReturnsQueuedDataAsync` fails without the fix (empty `NotificationData`). ## CTT results (fresh server per run) | Group | Before | After | | --- | --- | --- | | Session Services (4 CUs, 32 cases) | ~25 errors (deadlock cascade), 9:03 | 3 errors, all C32 (CTT defect), 0:45 | | Subscription Services (14 CUs, 215 cases) | `020.js` (known, Mantis 11260), `012.js`, and `004.js` intermittently | `020.js` + `012.js` (C33, CTT defect); `004.js` passed 3 of 3 runs | Transfer, Minimum 05, Publish Min 10 and Basic have no errors. Warnings and skips are classified in `plans/ctt-issues.md`. ## Documentation (`plans/`) - The first docs commit takes `plans/ctt-testing.md` and `plans/ctt-issues.md` unchanged from #4482, so identical content merges cleanly with the open CTT PRs. The second docs commit contains only this PR's additions. - **C32**: Session Base `Err-002.js`/`Err-005.js`/`Err-022.js` pick the `opc.wss` SignAndEncrypt endpoint in `initialize.js` and send it as the EndpointUrl over a UA TCP channel; `HostnameFromUrl` (`UaH.js:64`) cannot parse `opc.wss`. - **C33**: Subscription Durable `012.js` ignores `Bad_UserAccessDenied` when browsing `SubscriptionDiagnosticsArray`. The server restricts it to SecurityAdmin over SignAndEncrypt. The script then reads an empty list (`Bad_NothingToDo`), and line 37 is missing braces. - Issue 18 (`008.js`) is still misspelled in 1.05.513 but passes against this server. - CloseSession latency warning (600–700 ms in the CTT; 0–19 ms in-process), Publish Min 05 `003.js` subscription-limit configuration note, Session/Subscription coverage (manual and no-test-case CUs). - `ctt-testing.md` section 7: how to run both groups (one CTT process each, 20 min timeout, short project path — a deep project copy breaks the CTT's relative certificate paths), per-CU timings and expected results; pitfall about several automated CTT runs sharing port 62541. C-entry numbers start at C32 because #4481 already uses C19–C31. ## Tests - `Opc.Ua.Server.Tests`: 5554 passed, 0 failed - `Opc.Ua.Sessions.Tests`: 778 passed, 0 failed, 379 skipped (same counts as a clean tree) - `Opc.Ua.Subscriptions.Tests`: 619 passed, 0 failed, 6 skipped 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
romanett
|
37d8716a9b |
Fix Like filtering (GDS queries, ContentFilter), QueryServers paging and FindApplications found by the CTT GDS run (#4481)
Triage of the CTT **GDS** conformance group (CTT 1.05.06, scripts 1.05.513) against `ConsoleReferenceServer --ctt -a`, which hosts the GDS node manager. The baseline run had 60 errors in 4 CUs. This PR fixes the three server defects behind them and documents the rest in `plans/ctt-issues.md`. ## Server defects fixed | Defect | Spec | CTT tests fixed | | --- | --- | --- | | QueryServers/QueryApplications Like filters: the hand-written tokenizer in `ApplicationsDatabaseBase.Match` returned no records or all records for `%_erver%`, `%e_`, `%\_%`, `%\%%`, `%[q-s]`, `%[^q-s]`, `%_ompliance%`, and accepted the malformed `%[a^j-l]%` | Part 4 §7.7.3 Table 120 (whole-string match; "The ^ shall be the first character inside on the []"), Part 12 §6.5.10/§6.5.11 | Application Directory 062, 066, 068, 071, 073 (078 now returns `Bad_InvalidArgument`, see C22); Query Applications 013, 017, 018, 019, 022, 024 | | QueryServers used the application id as the `RecordId` of every DiscoveryUrl record, so `StartingRecordId` paging skipped the remaining DiscoveryUrls of an application | Part 12 §6.5.11 Table 15: one record per DiscoveryUrl | Application Directory 045, 075 | | FindApplications with an empty ApplicationUri returned every registered application | Part 12 §6.5.4: result size 0 or 1; `Bad_InvalidArgument` for an invalid URI | Application Directory 004 | Changes: - `Opc.Ua.LikePattern` (new, `Opc.Ua.Types`) parses, validates and matches Like patterns: `%`, `_`, `\`, `[list]`, ranges, `[^list]` and the legacy `[!list]`. The GDS database and `AliasNameWildcardMatcher` (FindAlias) both use it now. The alias matcher's previous regex translation also accepted `[a^j-l]`. - QueryServers/QueryApplications reject a malformed filter with `Bad_InvalidArgument` and parse each filter once per call (`ValidateQueryServersArguments` / `ValidateQueryApplicationsArguments`). - FindApplications returns `Bad_InvalidArgument` for an empty or blank ApplicationUri. Other unknown strings, including non-URIs, return an empty array: CTT `003.js` calls it with up to MaxStringLength `X` characters and expects Good, and §6.5.4 allows both readings. - `QueryApplications` compared `(int)startingRecordId`, so a StartingRecordId above `int.MaxValue` returned every record. - Every DiscoveryUrl record (`ServerEndpoint`) now has its own monotonically increasing ID, persisted in the JSON database. `JsonApplicationsDatabase.Load` assigns and saves IDs for databases written before this change. A note on 074.js and Query Applications 025.js: both passed before and fail now. Their patterns `%[^f-h]%` / `%[^w-y]%` expect "contains none of f–h", which no Like pattern can express. The old matcher special-cased `[^`, and that quirk is why they passed (C21). ## ContentFilter Like operator `FilterEvaluator` had its own regex translation of Like patterns with the same class of defects: the regex was not anchored (`abc` matched `xxabcxx`), `^` was escaped so the 1.05 negated list `[^13-5]` never worked, and malformed patterns threw from `Regex`. - The Like operator now uses `LikePattern`. An invalid search string matches nothing, so Like resolves to FALSE, as Part 4 §7.7.3 specifies for an operand that cannot be resolved to a string. - `ContentFilterElement.Validate` rejects a malformed **literal** Like pattern with `Bad_FilterOperandInvalid` for that operand (`Bad_ContentFilterInvalid` for the element), so an event filter with such a pattern is refused when the monitored item is created. Patterns from other operands (for example an AttributeOperand) are only known at evaluation time and evaluate to FALSE. - The regex helpers are removed. No other code in the repository uses `FilterOperator.Like`, and the existing Like tests already assumed whole-string matching. ## Classification of the remaining failures | Test | Classification | Entry | | --- | --- | --- | | Application Directory 060, 067, 069 | CTT: registers an ApplicationUri containing a raw `%` / `\`, which is not a valid URI | C19 | | Application Directory 065, 060; Query Applications 011, 016 | CTT: `[_]` / `[%]` evaluated as "contains" | C20 | | Application Directory 074; Query Applications 025 | CTT: `%[^x-y]%` evaluated as "contains none of" | C21 | | Application Directory 079, 078 | CTT: `MethodCalls.js:286` reads the output arguments of a failed call (078 now gets the correct `Bad_InvalidArgument` and hits this) | C22 | | Application Directory 018 | CTT: selects `ActionTimestamp` (BrowseName is `ActionTimeStamp`); the missing event is the queue size issue fixed by #4480 | C23 | | Application Directory 019 | CTT client: the batched RegisterApplication Call never reaches the server (no server log entry, no service fault); new test shows the server handles the batch | C24 | | Application Directory 010 | CTT: dereferences the ApplicationId of a `Bad_EntryExists` result | C25 | | Application Directory 012 step 5, 032 | CTT: requires ServerCapabilities for a Server; Part 12 has no such rule | C26 | | Application Directory 027 | CTT: updates a Server with an `opc.tcp` DiscoveryUrl to a Client (invalid per Part 12 §6.5.5, Part 4 §7.2) | C27 | | Application Directory 029, 038, 039 | CTT: expects `Bad_InvalidArgument` where Part 12 defines `Bad_NotFound` | C28 | | Application Directory 005 | CTT: expects `Bad_InvalidArgument` for a string above MaxStringLength, which cannot be decoded | C29 | | Query Applications 036 | CTT: expects `rcp+` URLs it never registered | C30 | | Query Applications 038 step 5 | CTT: applicationType is a mask, `3` = Servers \| Clients is valid (step 6, max UInt32, already returns `Bad_InvalidArgument`) | C31 | | AliasName Discovery 001, 002, 004 | Not applicable: GDS AliasName Server Facet (Part 17 Annex C) is not implemented; 001 also sees the reference server's own aliases | config notes | | AliasName Discovery 005–015, LDS-ME Connectivity | Configuration (AliasName sources / registered LDS) | config notes | Which server to target: Application Directory and Query Applications run the same `Opc.Ua.Gds.Server` code on the reference server (in-memory database, clean on every start) and on a dedicated GDS. The dedicated GDS sample keeps a JSON database, and leftover records of earlier runs change every expected record count. The GDS AliasName Discovery CU needs a GDS that implements the facet and should be deselected on both. Spec conflict, server unchanged: Part 12 §6.5.10/§6.5.11 say QueryApplications/QueryServers *"shall not return records with a ServerCapabilities that includes NA"*, but the CTT registers its reference Servers with `NA` and expects them in the results. ## Tests - `LikePatternTests` (Types): Part 4 Table 120 examples, CTT patterns, invalid patterns (93 cases). - `LinqApplicationsDatabaseTests`: paging over multi-URL applications, RecordId after update, JSON reload and legacy databases, CTT filters for QueryServers and QueryApplications, StartingRecordId above `int.MaxValue`, empty FindApplications. - `ApplicationsDatabaseBaseTests`: CTT patterns, invalid filters rejected. - `GdsApplicationDirectoryTests` (live): invalid pattern → `Bad_InvalidArgument` with no output arguments; four RegisterApplication operations in one Call. - `GdsDepthTests.AppDirFindApplicationsEmptyUriAsync` expects `Bad_InvalidArgument`; `AppDirFindApplicationsUnknownNonUriReturnsEmptyAsync` covers the 003.js strings. - `FilterEvaluatorTests` (Core): the Part 4 Table 120 examples through the ContentFilter, whole-string and case-sensitive matching, invalid patterns evaluating to FALSE, and validation of literal patterns (44 cases). ## CTT rerun Same GDS selection, fresh `ConsoleReferenceServer --ctt -a` built from this branch: **47 errors (baseline 60)**. - Now pass: Application Directory 004, 045, 062, 066, 068, 071, 073, 075; Query Applications 013, 017, 018, 019, 022, 024 (15 errors gone). - Newly fail as expected: Application Directory 074 and Query Applications 025 (C21). - 078 changed from "Good instead of BadInvalidArgument" to the C22 helper abort. - All remaining errors match C19–C31 or the AliasName Discovery note. - Rerun on `4e4fb7f7c` (review fixes, ContentFilter change, lenient FindApplications for non-URI strings): 48 errors. It matches the 47-error run exactly, plus `028.js` missing its audit event (QueueSize 1 flakiness, see C23 / #4480). An intermediate build that rejected every non-URI in FindApplications broke `003.js`, which is why that check was relaxed again. - Patched-script run (recommended fixes of C19, C20, C22, C23, C25, C29 applied to a scratch copy of the CTT project): 41 errors. `010`, `060`, `065`, `067`, `078`, `079`, QA `011`, `016` pass; `018` receives a correct audit event (the validator then trips over another script bug); `005` gets the expected `Bad_EncodingLimitsExceeded` ServiceFault. No further GDS server defect showed up. That ServiceFault carries RequestHandle 0 (Part 4 §7.33 says it *should* be echoed), a transport-level item left for a separate change. - AliasName group (FindAlias uses `LikePattern` now): 1 error, the known CTT defect in `AliasName Hierarchy/002.js`, unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
romanett
|
6313e38a06 |
Fix Part 13 aggregate status and FindAlias validation defects found by CTT runs (#4478)
# Description Headless CTT runs (UA 1.05.06, scripts 1.05.513) against `ConsoleReferenceServer --ctt` surfaced server-side defects in the Part 13 aggregate calculators and in the Part 17 FindAlias argument validation. This PR fixes them and documents the CTT workflow and findings. ## Aggregate fixes (`src/Opc.Ua.Server/Aggregates`) - **Partial bit on reverse reads.** `m_startOfData`/`m_endOfData` followed queue order, which is newest-first when `StartTime > EndTime`, so the start-of-data check tested the wrong end. - Both are now tracked chronologically, and only for values that pass the queue-order check. - The end-of-data check, previously disabled for backward reads for the same reason, now applies in both directions (Part 13 §5.3.3.2). - **Time-based status with TreatUncertainAsBad.** `GetTimeBasedStatusCode` counted Uncertain regions as neither Good nor Bad when `TreatUncertainAsBad = true`. - They now count as Bad, and the Bad ratio is evaluated before the Good ratio. - A Bad result keeps its aggregate bits, like the Good and Uncertain results (§5.4.3.2.1). - Affects TimeAverage2, Total2, Minimum2, Maximum2, Range2 and the ActualTime2 variants. - **DurationGood / DurationBad / PercentGood / PercentBad first region.** The first region inherited the status of the interpolated simple bound (e.g. Uncertain when Bad data follows). It now takes the status of the raw value at or before the interval start, or Bad if there is none (§5.4.3.31/32). - **WorstQuality2 on backward reads.** The start bound was taken at the late time and values were scanned newest-first. A backward interval is now calculated like the forward interval over the same range, with the start bound at the early time (§5.4.2.2, §5.4.3.36). - **Behavior change:** this reverses the reverse-direction semantics added in #3978. Part 11 §6.5.4.2 can be read as making the late time the interval start, but Part 13 §5.4.2.2 and the CTT oracle agree on the forward-equivalent calculation. The two `StatusAggregateCalculatorTests` cases that encoded the old behavior were updated. - The WorstQuality and WorstQuality2 loop is shared, so backward **WorstQuality** now also reports the chronologically first of equally severe statuses, like the forward calculation. This is covered by `DirectAndLiveWorstQualityBackwardSelectsChronologicallyFirstStatusAsync`. ## FindAlias argument validation (`src/Opc.Ua.Server/AliasNames`) - **Found by:** CTT AliasName Base `Err-001.js`–`Err-004.js`. FindAlias returned Good for malformed search strings (`A[`, `A\`, `A\\\`) and for a `HasComponent` ReferenceTypeFilter. - **Spec:** Part 17 §6.3.2 Table 4 requires `Bad_InvalidArgument` for an invalid search string, and the ReferenceTypeFilter shall be `AliasFor` or one of its subtypes. - **Matcher:** `AliasNameWildcardMatcher.IsValidPattern` rejects a trailing escape character, an unterminated or empty `[..]` list, and list contents that are not a valid character class (these previously threw). `IsMatch` no longer matches malformed patterns. - **Dispatcher:** `AliasNameMethodDispatcher` returns `Bad_InvalidArgument` for FindAlias and FindAliasVerbose on an invalid pattern or a non-AliasFor filter. A null filter and `References` still match every alias. ## Tests - **Aggregates:** - `AggregateCttRegressionTests` gains 18 cases. Each runs the calculator directly and through `HistorianDispatcher` with the in-memory historian. - Covered: the Partial bit forward and backward at the start and end of data; rejected out-of-order values; Minimum2 status with TreatUncertainAsBad true and false; the Duration and Percent first-region case; WorstQuality2 and WorstQuality backward vs forward. - With the aggregate source fixes reverted, every bug-targeted case fails and the controls pass. - **FindAlias:** new matcher and dispatcher unit tests. The InformationModel integration tests for these cases now require `Bad_InvalidArgument` instead of accepting an empty result. - **Local results (net10.0):** - `Opc.Ua.Server.Tests` aggregate/historian filter: 848/848. - `Opc.Ua.Server.Tests` AliasName: 147/147. - `Opc.Ua.History.Tests` aggregate/historical access/history client: 263/263. - `Opc.Ua.InformationModel.Tests` Alias: 54 passed, 5 skipped. - `Opc.Ua.Features.Tests` Alias: 11/11. - `Opc.Ua.Client.Tests` AliasName: 28/28. - **CTT reruns against the fixed server:** - Aggregate errors: 4,560 → 4,226. Server-vs-oracle DataValue mismatches: 10,067 → 9,415. - Every targeted aggregate mismatch pattern is gone, except the new DurationGood/PercentGood rows. Those are a CTT oracle defect, documented as C6. - AliasName Base `Err-001.js`–`Err-004.js` now pass. The remaining Hierarchy `002.js` error is a known CTT defect, Mantis 11262. ## Documentation (`plans/`) - **`ctt-testing.md` (new):** running the CTT headless against the reference server, generating selection files, reading results, capturing the aggregate values behind failures, and triaging CTT vs server defects. - **`ctt-issues.md`:** an issue register with the current Mantis status of every filed issue. It adds: - the not-yet-filed CTT defects (C1–C18); - clarification items; - open server findings to investigate (Auditing, A&C Refresh and Comment, GDS Like filters and AliasName Discovery); - CTT project configuration and coverage notes. - Skipped tests caused by sample-data gaps are tracked in #4479. ## Related Issues - Relates to #3978 (earlier CTT aggregate conformance work) and #4479 (CTT coverage gaps). ## 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. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
romanett
|
1af191ec26 |
Drop the .Common suffix from the three GDS projects (#4438)
# Description Every other companion specification in the stack ships as `<Family>`, `<Family>.Client` and `<Family>.Server` — `Opc.Ua.Di`, `Opc.Ua.ISA95`, `Opc.Ua.Positioning`, `Opc.Ua.Robotics`, `Opc.Ua.Vision`, `Opc.Ua.WotCon`, `Opc.Ua.XRegistry`, and `Opc.Ua.Lds.Server` without a family package. GDS was the one exception, carrying a `.Common` suffix that named nothing. | Before | After | | --- | --- | | `src/Opc.Ua.Gds.Common` | `src/Opc.Ua.Gds` | | `src/Opc.Ua.Gds.Client.Common` | `src/Opc.Ua.Gds.Client` | | `src/Opc.Ua.Gds.Server.Common` | `src/Opc.Ua.Gds.Server` | ## No source changes were needed `RootNamespace` was already the final name in all three projects — the namespaces have always been `Opc.Ua.Gds`, `Opc.Ua.Gds.Client` and `Opc.Ua.Gds.Server`, and only `AssemblyName` and `PackageId` disagreed. Nothing that compiles against these assemblies changes a `using` directive; the diff outside the moves is entirely ids, paths and prose. Directories were moved with `git mv`, and every one of the 73 file moves is detected as a rename at 100% similarity (bar the handful that also had a name in their text), so `git log --follow` and blame survive. ## Consumer impact The package ids change, which is visible to anyone consuming the NuGet packages: | 1.5.378 package | 2.0 package | | --- | --- | | `OPCFoundation.NetStandard.Opc.Ua.Gds.Common` | `OPCFoundation.NetStandard.Opc.Ua.Gds` | | `OPCFoundation.NetStandard.Opc.Ua.Gds.Client.Common` | `OPCFoundation.NetStandard.Opc.Ua.Gds.Client` | | `OPCFoundation.NetStandard.Opc.Ua.Gds.Server.Common` | `OPCFoundation.NetStandard.Opc.Ua.Gds.Server` | `docs/migrate/2.0.x/packages.md` gains a renamed-packages section — it is the document that undertakes to cover NuGet renames, and had no GDS entry — and the migration skill's package table gains the two rows a 1.5.378 consumer needs. That is the only hand-written content in this PR; everything else is mechanical. Assembly names follow the package ids, so the section also notes that binding redirects, trimming descriptors and signing manifests naming the files directly have to be updated. ## Build infrastructure Everything naming the projects or their output moves with them: - `UA.slnx` and `tools/MigrationAnalyzer.slnx` (both re-sorted — the project lists are alphabetical, and `Gds` now sorts before `Gds.Client`). - `.azurepipelines/expected-packages.txt` — 3 ids, also re-sorted for the same reason. - `.azurepipelines/signlistDebug.txt` and `signlistRelease.txt` — 18 paths each. Their internal ordering is left alone: that group was already unsorted, and reordering a signing manifest is diff noise. - `samples/Reference/ConsoleReferenceServer/Dockerfile`, the two metapackage nuspecs, the analyzer nuspec's package dependencies, and 8 `.csproj` references across tests, samples and tools. `preview-pack.slnx` is generated from `UA.slnx` by `generate-slnx.ps1`, so there is nothing static to edit. Nothing under `.github/workflows`, and no `.ps1`/`.yml` in `.azurepipelines`, names these projects. The migration analyzer's shim tree mirrors the source project layout by its own documented convention, so `Gds.Client.Common/` becomes `Gds.Client/` and its readme table follows. ## Deliberately not changed Prose naming past work: `plans/23` still records that its deferral happened during the "GDS Client.Common modernization", which is the name that effort had. File *paths* in the same document were updated. ## Validation `net10.0`: - `UA.slnx`: 0 errors. 14 warnings, all pre-existing CA1861/CA1850/CA1307 in `XRegistry`, `Types.Tests` and `WotCon.Tests` — none in anything this PR touches. - Assemblies build as `Opc.Ua.Gds.dll`, `Opc.Ua.Gds.Client.dll`, `Opc.Ua.Gds.Server.dll`, with no `*.Common.dll` produced anywhere. - `Opc.Ua.Gds.Tests`: 1,116 passed, 0 failed, 54 skipped. - `Opc.Ua.MigrationAnalyzer.Core.Tests`: 10 passed, 3 skipped. - A repo-wide grep for the old names returns only the intentional old → new mappings in the migration docs. ## Checklist - [x] Existing tests pass - [x] No new tests — this is a rename with no behaviour change - [x] Consumer-visible package rename documented in the migration guide 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
romanett
|
414957c88a |
Reduce ISubscription: internalize the publish pipeline, delete the dead members (#4349)
# 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> |
||
|
Copilot
|
d525bc3f28 |
Bound reconnect cleanup and organize samples by domain (#4276)
# Description This PR fixes bounded ManagedSession reconnect cleanup and reorganizes the sample applications into domain-focused folders. ## Reconnect cleanup - Bounds reconnect integration cleanup with independent 30-second close and dispose limits. - Sends `CloseSession` through the awaited transport request path during `Session.DisposeAsync`. - Preserves explicit `Session.CloseAsync` behavior that waits for or cancels outstanding publish requests. - Covers the disposal request path with a regression test. ## Sample organization - `samples/OpenUsd`: `GeneratorServer`, `SiteCompositionServer` - `samples/Redundancy`: `RedundantClient`, `RedundantServer`, `RedundantPubSub` - `samples/Isa95`: `MinimalIsa95Server` - `samples/MinimalApi`: `MinimalBoilerServer`, `MinimalCalcServer`, `MinimalClient` - `samples/DI`: `PumpDeviceIntegrationServer` - `samples/Lds`: `ConsoleLdsServer` - `samples/Reference`: `ConsoleReferenceClient`, `ConsoleReferenceServer` - `samples/PubSub`: `ConsoleReferencePubSubClient` `UA.slnx` now exposes the same requested domain groups. Project references, tests, runtime sample probes, CI workflows, Dockerfiles, Compose files, helper scripts, and documentation were updated to the new paths. `samples/Robotics/MinimalRobotServer` remains under `Robotics`. ## Validation - `dotnet build UA.slnx` - 3 Redundancy short-haul sample tests - 70 DI sample tests - 914 OpenUSD tests - 1 WotCon model-materialization test - 137 AOT tests - `docker compose config` for all 6 moved Redundancy Compose files ## Related Issues - Fixes #4273 ## 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. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce new build or analyzer warnings. - [x] I built the complete `UA.slnx` solution locally. - [ ] I ran all tests locally against at least .NET Framework and .NET 10. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <mschier@microsoft.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Copilot-Session: a2c2838f-74ae-4997-a1d4-50574d9e0636 |
||
|
Marc Schier
|
aad8350bc1 |
Pluggable symmetric cryptography, an awaitable asymmetric path, registrable security policies, and PubSub crypto offboarding (#4206, #4207, #4208, #4210) (#4232)
# Description Closes the three crypto-offboarding follow-ups split out of #4192. Each was recorded there as knowingly *not* done, with the reason and what would justify revisiting it. Those conditions are met, and where one of them turned out to be unachievable that is written down rather than worked around. The safety property throughout: **every seam is opt-in and null-checked, and in the default software configuration the added code is observably inert.** No interface dispatch on the per-message path, and the asynchronous paths complete synchronously — so channel locking order, buffer ownership and error sequencing are unchanged for a deployment that configures nothing. ## #4207 — symmetric, key-derivation and RNG provider seams `ISymmetricCryptoProvider`, `IKeyDerivationProvider` and `ISecureRandomSource`, added on the terms #4207 set: only once a concrete requirement arrived (a validated module that must perform *every* operation, not just the asymmetric ones), behind a fast path, with `SymmetricChannelCryptoBenchmarks` as the gate. They are **facets discovered by type test**, not members of `ICryptoProvider`, so providers written against the shipped interface still compile. `PlatformCryptoProvider` implements all three, which answers the "an interface nothing implements" objection recorded in the issue. `CryptoProviderFacets` returns `null` both when no registry is configured *and* when resolution lands on the platform, because the platform facets are the inline code. Resolution happens once, in `CalculateSymmetricKeySizes`, and is held for the life of the channel. `CryptoCompliance.GetUnservedOperationPurposes` closes the failure the seam would otherwise introduce: a provider bound to `ChannelSymmetric` without implementing the facet is silently replaced by the platform, so a deployment could believe its validated module performed the per-message cryptography while the platform did. Under `FipsOnly` that now refuses to start. ## #4208 — the asymmetric path is awaitable `RSA` and `ECDsa` are .NET's contracts and cannot be replaced without making every ready-made hardware and cloud implementation unusable. Instead an implementation may **also** declare `IAsyncRsaKey` / `IAsyncEcdsaKey`, which the stack finds by type test. Now asynchronous: the secure channel open and renew path, user identity token signing and decryption, and session activation. A software key declares neither facet, so those paths complete synchronously and nothing about their ordering changes. `ReadAsymmetricMessage` is refactored into shared helpers rather than duplicated — adding an `await` was not worth two hundred cloned lines of signature, padding and endpoint-selection logic. `AsymmetricMessage` / `AsymmetricWriteResult` exist because an asynchronous method cannot have `out` parameters. The channel no longer serialises its state on a monitor. A monitor cannot be held across an `await`, and a plain `SemaphoreSlim` was not an option at the time: the code relied on re-entrancy in eight places (`HandleIncomingMessage` holds the lock and calls `ForceChannelFault`, which takes it again; `ForceReconnect` and `Shutdown` do the same). `ChannelGate` replaces it at 32 sites and `DataLock` is `[Obsolete]`. That reliance has since been removed as well — see [Removing the gate's re-entrancy](#removing-the-gates-re-entrancy) below. ### Defects this surfaced, all fixed and regression-tested **Detached work started inline.** Writes were started with `_ = WriteBuffersAsync(...)`. An async method runs its prologue on the caller's stack, so `LeaveInheritedContext` — which exists to stop *detached* work inheriting the right to re-enter the gate — was stripping the **caller's own** entitlement; when the send then completed synchronously, the completion blocked on a gate that very thread was holding. The client sent its OpenSecureChannel request and never processed the response. Writes are now queued. **A synchronous gate handle held across an `await`.** `Enter()` records the acquiring thread so an inline completion callback can re-enter — `ChannelAsyncOperation` invokes its callback both inline and detached. That record is only sound while the holder is synchronously on that thread; the token renewal path held it across an await, so a reused pool thread was recognised as the holder. **`SignAsync` rejected the policy that signs nothing.** It validated the certificate before the algorithm, so `SecurityPolicies.None` — which carries no certificate — threw where the synchronous `Sign` returns `null`. **`EnterAsync` lost the holder when contended** *(found in review)*. The `AsyncLocal` recording ownership was written inside the awaiting method, and .NET restores the caller's execution context when an async method completes, so the write was discarded. The caller returned owning the semaphore while `IsHeldByCurrentContext` reported `false` — and any nested `Enter()` then blocked forever on a gate its own context held. Reachable remotely: `HandleIncomingMessageAsync` → error path → `ForceChannelFault`. The holder is now published from the caller's frame before the wait, and only its depth is raised in the continuation. **Queued writes lost their order** *(found in review)*. Sequence numbers are assigned under the gate, but independently queued work items reach the transport's FIFO send lock in arbitrary order, so a peer could reject a chunk with `BadSequenceNumberInvalid`. Writes are now appended to a chain: still off the caller's stack, but a write cannot start before its predecessor finishes. ### Four more this surfaced in CI, on paths only the integration suites reach Queuing the writes to fix the deadlock above turned out to have moved three implicit guarantees, and the gate had one more liveness bug. Each was reproduced locally against a clean baseline before it was changed. **A faulting channel's error message never reached the peer.** A listener channel sends a UASC `Error` describing why it faulted and closes the transport as its next statement. That worked only because the write started on the caller's stack, so the bytes reached the socket first; queued, the close discarded them and the client saw `BadConnectionClosed` instead of the certificate status it was told to expect. Nine `SecurityCertValidationTests` cases assert exactly that status. The terminal error is written inline again, on a path that touches the gate only to disclaim what it inherited and reports nothing — a channel that is already faulted is owed nothing beyond its buffer. **The certificate of a rejected sender leaked.** An asynchronous method cannot have `out` parameters, so the asymmetric read returns a record instead. That silently dropped an ownership guarantee: an `out` parameter reaches the caller even when the method goes on to throw, so the caller's `catch` disposed the sender certificate and reported it to the audit. A return value does not — nine leaked certificates, one per rejection case, which the leak detector fails the run on. The parsed certificate is now handed to the caller before anything that can reject the message, restoring both the disposal and the audit. **The gate could strand its own permit.** It released only when the holder's depth returned to zero, but that depth lives in an `AsyncLocal` and is therefore shared with every context forked from the owner's — a timer callback whose timer was created inside the region, for instance. Such a context entering the gate raised the shared depth, so the owner's own exit saw a non-zero depth and returned without releasing, and nothing afterwards could: the nested exit is not the owner. Every later entry on that channel blocked forever. `Opc.Ua.Features.Tests` hung after its tests had finished, with a receive loop waiting on the gate and the server unable to stop. The owner now releases when it leaves; depth still gates re-entrancy, but liveness no longer depends on a count another context can move. **Write completion throttled the writer.** Reporting a completed write sat inside the ordering chain, and the client channel implements that callback by entering the gate — so every write queued behind a gate the receive loop contends for on each incoming chunk. A session with fifty subscriptions publishing continuously had its publish requests throttled enough that the client kept topping them up past its own limit, which `Subscriptions.Classic` asserts against. Completion is now reported off the chain, which covers only what ordering actually requires. ### Still synchronous, and why Certificate, certificate request and revocation list signing cannot be made asynchronous at all: `X509SignatureGenerator.SignData` is invoked by .NET's own builders. Service faults and the `Reconnect` override are reached from synchronous call sites (`SendServiceFault` alone has eight) and are left as they are behind scoped, commented suppressions. All three are documented rather than left to be rediscovered. ## #4210 — PubSub This one produced a finding rather than the API the issue asked for, and the finding is the point: > **With a standard Security Key Service the key necessarily exists in process memory.** `GetSecurityKeys` (Part 14 §8.3.2) returns raw key bytes over the wire, so the "key never leaves the device" property #4192 achieved for client and server **cannot** be achieved for PubSub through the SKS pull profile. That is a property of the specification, not of this stack. A wrapped-key envelope would change what is on the wire and break interoperability with third-party key services and publishers, so it was rejected rather than built. What is achievable is delivered: the per-message AES-CTR and HMAC route through `ISymmetricCryptoProvider`, so a validated module performs them. `IPubSubSecurityPolicy` is **not** modified — new members would break every implementation of it; the provider is supplied through the policy constructor. `PlatformSymmetricCryptoProvider` gained AES counter mode, which the seam should have covered anyway since the algorithm enum names it. Key lifetime is also tightened: unpacking an SKS response copied key material into intermediates that `ByteString.Create` then copied again, leaving plain keys in the heap for the lifetime of the process. Those are now zeroed. ### Two wiring gaps this depended on, both found in review The registrations above were **inert**, so this section originally claimed more than the code did: - `CryptoProviderConfiguration` was applied nowhere outside tests, so `AddCryptoProvider(configure)` never reached the registry in a real host. The registry now applies every registered configuration when it is first resolved — which also affects #4207's DI story. - `PubSubSecurityWrapperResolver` selected `PubSubAes256CtrPolicy.Instance`, the provider-less static singleton, ignoring the provider-backed bundles registered for it. A `FipsOnly` deployment therefore certified while the platform did the work. The resolver now selects from the registered policies; the container and `PubSubApplicationBuilder` both supply theirs. A third, smaller one: `CryptoProviderBuilder.Use` published a **policy-scoped** random source as the process-wide nonce source, so `.For(purpose, policyUri).Use(hsm)` redirected nonce generation for every other policy too. Only an unscoped binding does that now. ## #4206 — a provider can contribute a security policy Added in review: the policy set was fixed at compile time, so a provider could contribute key custody but not a *profile*. It is now table-driven and open, and the blockers #4206 listed are gone. ```csharp using IDisposable registration = SecurityPolicies.Register( new SecurityPolicyInfo("urn:vendor:SecurityPolicy#CustomProfile", "CustomProfile") { PlatformSupport = () => true, SupportedCertificateTypes = [ObjectTypeIds.RsaSha256ApplicationCertificateType], IsDefault = true }); services.AddOpcUa() .AddCryptoProvider(crypto => crypto .For(CryptoPurpose.ChannelSymmetric, "urn:vendor:SecurityPolicy#CustomProfile").Use(module)) .AddSecurityPolicy(customPolicy); ``` - **The reflection is gone.** Four lookup tables were built by reflecting over the fields of `SecurityPolicies` and `SecurityPolicyInfo`. They are one explicit table now, which removes the **last reflection in `Security/Constants`** — so this is a trimming and Native AOT improvement as much as a feature. - **`SecurityPolicyInfo` is constructible from another assembly** and carries the metadata the helpers need, so `IsPlatformSupportedName`, `GetDefaultUris`, `MapSecurityPolicyToCertificateTypes` and `GetCurveFromCertificateTypeId` read that table instead of each being hand-written. Adding a policy is one entry rather than five edits. - **The dead `#if CURVE25519` conditional is deleted** from `Security/Constants`. - Registration is copy-on-write, so the read path stays lock-free. Shadowing a built-in policy requires `replaceExisting: true` and is reversible by disposing the registration. `RegisterLightsUpCurvePoliciesFromOutsideCore` is the acceptance test the issue asked for: it registers `ECC_curve25519` and `ECC_curve448` from the test project and asserts they become discoverable through the public API. **Registering a policy makes it advertised and resolvable; it does not supply the cryptography behind it.** For those two curve profiles the in-tree key agreement is still behind a compile symbol no project defines plus a BouncyCastle dependency, so a deployment lighting them up provides the operations through a provider. That division is the point: the policy set says *what* is offered, the provider says *who* performs it. ## Removing the gate's re-entrancy Folded in from #4242, which was merged into this branch. `ChannelGate` was re-entrant, and that forced the design behind most of the defects above. A holder that awaits releases its thread, so ownership could not be tracked by thread identity — it had to live in an `AsyncLocal<Holder?>`. A logical context is *inherited*, so anything started while the gate was held inherited the right to re-enter and ran inside the guarded region alongside whatever started it, unless it opted out by hand via `LeaveInheritedContext()` at each of eight sites, enforced by review alone. Five of the defects listed above trace to that mechanism, including both CI hangs. So the *requirement* was removed rather than the mechanism, and the `AsyncLocal` then deleted itself. Every path that re-entered now calls a lock-free `Core` method (`ForceChannelFaultCore`, `ShutdownCore`, `ForceReconnectCore`, `CompleteConnectCore`), and three regions stopped running foreign code under the lock: the `RequestReceived` dispatch is deferred until the gate is released, `OnTransportError` no longer wraps the `HandleSocketError` virtual, and the callback setters and `EndpointDescription` publish a single reference with a volatile write instead of locking — the last is load-bearing, because `AsyncResultBase.OperationCompleted` invokes the reverse-hello callback inline while the gate is held. `ChannelGate` is now a `SemaphoreSlim(1,1)` behind a `Releaser`; `Holder`, the thread-identity fallback, `LeaveInheritedContext`, `IsHeldByCurrentContext` and all eight disclaimer calls are gone, with the call shape at all 32 sites unchanged. `SaveIntermediateChunk`, `GetSavedChunks` and `DoMessageLimitsExceeded` gained a `gateHeld` argument, because the client reaches them both with and without the gate and must pick the matching teardown — without it an oversized `OpenSecureChannel` response deadlocked the receive loop against itself. The remaining paths were not found by reading the code, since that is what missed them originally. A temporary strict mode made every re-entrant acquisition throw with its stack, the suite was run under it until clean, and it was deleted with the re-entrancy it measured. It caught two sites review had missed: the `EndpointDescription` **property getter** taking the gate, and the `RequestReceived` dispatch to application code happening inside the guarded region. Two deliberate residuals are documented for reviewers: `Dispose` no longer takes the gate (unavoidable — it is reached from `ChannelFaulted` with the gate held, where the old re-entrant acquisition excluded nothing anyway), and the deferred request dispatch builds `SecureChannelContext` just after gate release rather than under it. ## Testing New: the three seams and their platform implementations, facet discovery and resolution precedence, `FipsOnly` refusing a provider that cannot serve what it was bound to, AES counter mode (including a non-block-aligned round trip and a check that the counter carries, since one that did not would silently reuse key stream), `ChannelGate` (exclusion, re-entrancy across an await, re-entrancy on a reused thread, and the deadlocks above), the asynchronous channel paths, and the PubSub policy provider. Each review finding got a test that fails without its fix: - `ContendedEntryStillRecordsTheHolderAsync` — established contention deterministically, then re-entered; failed on the pre-fix gate. Removed with the re-entrancy it covered; `ChannelGateTests` now asserts the non-re-entrant contract instead. - `SecureClientAndTcpListenerKeepChunkOrderUnderConcurrentRequestsAsync` — 60 concurrent multi-chunk `SignAndEncrypt` requests over a real loopback listener; failed 3 runs out of 3 with the previous dispatch, passes 3 out of 3 with the chain. - `DependencyInjectionRoutesPubSubCryptoThroughTheRegisteredProviderAsync` — builds a container, registers a counting provider, wraps a message and asserts the provider actually performed the encryption and the signature. This is the assertion the PubSub claim above now rests on. - `PolicyScopedRandomBindingDoesNotEscapeItsPolicy`. Two further assertions are the ones that would catch a silent defect: a provider and the platform must produce **byte-identical** output — a validated module that disagreed could not interoperate — and with a software key the returned `ValueTask` must report `IsCompletedSuccessfully`, which is what makes "ordering is unchanged" checkable rather than merely asserted. `SymmetricChannelCryptoBenchmarks` gains a provider-registered variant beside the existing baseline, so the cost of the indirection is measured rather than assumed. Verified locally, every suite at or above its recorded baseline: `dotnet build UA.slnx -c Release` **0 warnings, 0 errors on all six target frameworks**; Core **4413** (net10.0); Core.Security **617**, with its certificate-leak detector clean; PubSub **1350** plus PubSub.Diagnostics **54** and PubSub.Udp **233**; Features **156**; Subscriptions.Classic **32**; Server integration **4053**; Client integration **2123**; MigrationAnalyzer **151** and MigrationAnalyzer.Core **13**. Zero failures in each. Core moved from 4388 to 4381 when the gate change landed — eight re-entrancy tests were removed along with the behaviour they covered, and one deadlock regression test added — and has since grown to 4413 with the registry isolation, provider-output and security-review tests. `ChannelGateTests` was rewritten around the new contract — exclusion across both entry modes, an uncontended asynchronous entry not suspending, re-entry blocking by contract, and the property that used to be the hazard: work started while the gate is held is excluded without having to opt out. Every CI failure this PR produced was reproduced locally, diagnosed against a clean `origin/master` baseline in a second worktree, and fixed at the source. Two hangs were found from `--blame-hang-timeout` dumps read with `dotnet-dump`. One failure that looked like a candidate was confirmed **not** to be: an intermittent `Opc.Ua.Aot.Tests` connect failure, pre-existing on `origin/master` (5 of 8 baseline runs failed, versus 3 of 14 here). ### The publish request overshoot `Subscriptions.Classic.PublishRequestCountAsync` failed on the Windows net10.0 leg with **207** outstanding publish requests against a cap of `max(maxServerPublishRequest, subscriptions)` = 50. It failed in 2 of 6 pipeline builds on this branch, never on `master` in 13 sampled builds, and never locally in 12 attempts including under deliberate CPU contention. The mechanism is a genuine race. `ClassicSubscriptionEngine.QueueBeginPublish` was a **check-then-act**: it read `GoodPublishRequestCount`, compared it against `GetDesiredPublishRequestCount(false)`, and issued a publish if it was below. It runs on every publish completion, so *n* completions that overlap each read the same value, each conclude they are below the cap, and each issue — overshooting by up to *n* per round. The read also *lags*: a request only becomes visible to `GoodPublishRequestCount` once `AsyncRequestStarted` has recorded it, which happens after the request has already been sent. That code is pre-existing and was untouched by this PR. What this PR changed is how hard the race is hit: the server used to hold the channel gate across request processing, which serialised every request on a channel, so publish completions arrived one at a time. Dispatching `RequestReceived` outside the gate — an intended improvement, since a channel should not serialise on application request-processing time — lets them complete concurrently. Rather than change the publish pipeline blind, the race was first made reproducible. `ConcurrentPublishReEvaluationDoesNotExceedDesiredRequestCount` drives 64 barrier-released completions against a mocked context whose count lags the send exactly the way the session's does. It issued **64 requests where 5 were desired** — the same defect the CI leg hit, now deterministic and running in milliseconds. The engine now tracks the requests it has in flight and reserves a slot with a compare-exchange before sending, so the decision and the send are one atomic step. The reservation is released when the request completes, when the send is declined, or when it throws. The repro test then issues at most the desired count. `StartPublishing` is deliberately left **uncapped**. It is the recovery valve that refills a pipeline whose requests are outstanding but no longer expected to return, so gating it on the in-flight count would remove the only path out of a stalled pipeline. An earlier revision of this fix did gate it, and that is the one change here that could not be justified on the evidence — it was reverted. The assertion was **not** loosened, retried away, or marked flaky: it caught a real throttling regression earlier in this PR and is worth keeping sharp. Validated: the repro test red before the fix and green after; `Subscriptions.Classic` 32/32 **five** consecutive runs on net10.0 and once on net48; `Opc.Ua.Client.Tests` 2124 on net10.0 across four consecutive runs and 2127 on net48; `Opc.Ua.Subscriptions.Tests` 604 passed with this fix in place. ### The SSH.NET advisory that turned CI red repo-wide Partway through this work every build job began failing with `NU1903: Package 'SSH.NET' 2025.1.0 has a known high severity vulnerability` ([GHSA-q939-rpr3-3284](https://github.com/advisories/GHSA-q939-rpr3-3284), `<= 2025.1.0` affected, first patched `2026.0.0`). Nothing in the dependency graph changed — the advisory was published. Build 17026 failed on it where build 17000, two hours earlier on an identical graph, had passed every build job. SSH.NET is not referenced anywhere in the repository; it arrives transitively through `Testcontainers.Kafka`. `master` landed the remediation independently while this was being investigated (a central transitive pin to `2026.0.0`, which `CentralPackageTransitivePinningEnabled` exists for). This branch briefly carried its own identical pin, which made the merge commit hold **two** `PackageVersion` entries for SSH.NET and fail restore with `NU1506`. The duplicate is removed and `master` is merged, so the pin now comes from one place only. The audit was **not** suppressed: `NuGetAudit` stays enabled and `NU1903` stays an error. Verified after the merge — `dotnet restore UA.slnx` reports neither NU1903 nor NU1506, and `dotnet build UA.slnx -c Release` is **0 warnings, 0 errors** with the audit live. The remaining red check in build 17026 was not from this change either. `aot-ubuntu-latest` is the intermittent `Opc.Ua.Aot.Tests` failure already noted above — `ReconnectSessionAsync` timed out at 30s with 135 of 136 passing. It remains below the `origin/master` baseline rate (4 of 15 sampled here against 5 of 8 on the baseline), and the managed reconnect paths are green throughout: `Opc.Ua.Client.Tests` 2124 across four consecutive runs and `Opc.Ua.Subscriptions.Tests` 604. ## Documentation `docs/CryptoProvider.md` gains sections on substituting the symmetric primitives, using a key served over a network, and PubSub — including what cannot be claimed and why, the scope rule for a random-source binding, and how to supply a provider-backed policy outside the container. `docs/MigrationGuide.md` covers the `[Obsolete]` channel virtuals and `DataLock`, and the non-re-entrant gate's contract for channel subclasses. `docs/WhatsNewIn2.0.md` is updated. `plans/cryptooffboard.md` is deleted now that the work it planned has landed. ## Note for reviewers This touches the secure channel handshake, which is the most security-critical code in the stack. Nine distinct concurrency and lifetime defects were found and fixed in it during this work — three during development, two in review, four from CI — so `ChannelGate`, the write path and the certificate ownership across the new suspension points deserve the closest reading. Five of those nine traced to the gate's `AsyncLocal`, which is why the re-entrancy was subsequently removed and the `AsyncLocal` deleted. The wrapper/`Core` split and the two documented residuals in that section are the parts most worth a second reading. The #4206 work removes the last reflection from `Security/Constants` and rewrites the policy lookup tables, so `SecurityConfiguration.SupportedSecurityPolicies` and the endpoint set it feeds are worth a second look: an earlier revision of it changed that set and broke six server tests, which is now covered by `SecurityConfigurationBuildsExpectedSupportedPolicySet`. ## Related Issues - Fixes #4206 - Fixes #4207 - Fixes #4208 - Fixes #4210 - Follow-up #4250 — this PR makes the security policy set injectable, but nothing in the stack resolves `ISecurityPolicyRegistry` from DI yet, so every production path still lands on the `Default` fallback. #4250 tracks threading it through `Session` and `UaSCBinaryChannel`, which is what makes the injection points already added here load-bearing. ## 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. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Copilot-Session: 7a68ca72-83e8-496c-b2f5-3df4c718320b Copilot-Session: 3f5ab76a-c0ba-4f47-87ac-54d1f4182c6b |
||
|
Marc Schier
|
e05a820c91 |
Removal of the exposed diagnostics and node locks (#4188)
## What this is Design analysis for `Opc.Ua.Server`, the two rounds of design exploration behind it, and the first two implementation chunks it recommends. Draft: the analyses are ready to read, but the *conclusions* deserve discussion before the larger phases proceed. Later chunks stack onto this PR as they land. | File | Kind | Lines | |---|---|---:| | `plans/NodeManagerAnalysis.md` | Evidence — the node-manager plugin surface | 488 | | `plans/ServerRuntimeAnalysis.md` | Evidence — the server runtime cluster | 576 | | `plans/ServerDesignExploration.md` | Design round 2 — delivery strategy | 1,394 | | `plans/ServerDesignExplorationAlt.md` | Design round 1 — four interface designs | 2,119 | | `src/Opc.Ua.Types/State/NodeState.cs` | **Code** — node self-guards attributes | | | `src/Opc.Ua.Types/State/BaseVariableState.cs` | **Code** — value accessors take the lock | | | `src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs` | **Code** — 6 external locks removed | | | `tests/…/NodeStateAttributeConcurrencyTests.cs` | **Code** — concurrency safety net | 449 | `docs/` is untouched — it holds shipped feature documentation, and none of this is shipped API. ## The analyses Counted mechanically over `src`, `samples` and `tests`; **every figure is reproducible** from commands printed in each document. **Node manager surface.** `AsyncCustomNodeManager` declares **93 `virtual`/`abstract` members** (80 distinct names). Across the repository: **112 override sites, 25 distinct names ever overridden, 55 of 80 never**. Six carry nearly all real use. For scale, `ReferenceNodeManager` is 6,373 lines and overrides **8**. **Server runtime.** Seven findings, **two of which recommend no change**. Findings 1–5 are filed as #4183–#4187. The headline is an `object`-typed sync root published on three public interfaces and taken in **88 `lock` statements across 7 files**, including a shipped sample. **What is deliberately not flagged.** `IStandardServer` — 9 members over 5,575 lines, 16 of 37 virtuals used — is identified as the reference shape to preserve. `IApplicationInstance` and the already-`[Obsolete]` `Stop()` are examined and left alone. ## The design exploration **Round 1** explored four alternative interfaces in parallel under the current back-compat constraint. Its value is the convergence: four designers who never saw each other's work independently arrived at a 6–9 member `IServerContext` replacing the 57-member locator, removal of the `object` node-manager handle, elimination of the `Processed` protocol, and `IHistorianProvider` as the pattern to copy. **Round 2** re-runs it with back-compat lifted and asks how to deliver: full rewrite versus side-by-side fork. **Both option designers independently declined to recommend their own assignment.** Three structural facts were verified: 1. **The fork line already sits at `IAsyncNodeManager`** — old and new node managers can coexist with zero runtime changes, so a fork duplicates ~10% of the assembly, not all of it. 2. **`[OpcUaShim]` cannot shim an inheritance surface** — honest auto-fix ceiling for the authoring surface is **~10%**, against ~70% for the shipped analyzers. 3. **A second lock leak exists** — `NodeState` takes `lock (this)` under a `CA2002` suppression whose own comment documents the contract. Recommendation: **neither pure option**, but a staged path reaching the rewrite end state incrementally, gated on migrating the five framework-internal node managers that today make it impossible for the old base to ever freeze. ## Implementation: NodeState now guards its own attributes This removes the prose-only contract in which `NodeState` took `lock(this)` and every external caller was required to take `lock(source)` on the node instance. The code already carried a TODO describing exactly this fix and noting it required updating every external site. - The node self-guards rather than relying on callers. `BaseVariableState` gains a **fully private** `System.Threading.Lock` covering its value fields; `NodeState`'s other collections keep their existing private locks. - All **6** `lock(this)` sites and both `CA2002`/`RCS1059` suppressions are gone. - `Value`, `Timestamp` and `StatusCode` on `BaseVariableState` take the lock too — without that, a direct property write raced an attribute read once the external lock was gone. - **6 of 9** external sites removed. Five were already redundant (`GetReferences`, `GetChildren`, `ReferenceExists` and `SetAreEventsMonitored` guard themselves; `FindMethod` reaches its state via `GetChildren`); the sixth was an attribute read now covered by the node's own lock. - Attribute read/write deliberately take **no** node-wide lock: both dispatch to caller-supplied handlers (`OnReadValue` and siblings) that may take locks of their own, and holding a lock on the node across one gives the node no fixed place in the lock order. - **3 deliberately retained and annotated in code**: two `CreateBrowser` calls, which are not internally synchronized (tracked in #4216), and one in `CustomNodeManager`, whose class is slated for deletion so refactoring its lock would be wasted work. ### No lock is exposed anywhere, to anyone An earlier revision put the attribute lock on `NodeState` as `private protected`, which still published a lock-sharing contract on the inheritance surface. Review pushed back and it is gone: `BaseVariableState` was the only user, so the lock moved there as a plain `private readonly Lock`. There is no member for a derived type to call, in this assembly or any other. `BaseVariableValue` was the last public leak — it handed out the lock guarding its value through a `public Lock` property, so any caller could hold it across a call back into the stack. That property is removed: - Derived value classes (what the source generator emits for every structure variable) synchronize through a `protected EnterLock()` / `ExitLock()` pair. Callback helpers were the first choice, but the generated bodies pass `ref Variant` / `ref StatusCode` / `ref DateTimeUtc`, and `ref` arguments cannot be captured by a lambda. The lock *object* stays private either way. - `NodeStateTemplates` emits the new form. - `ServerInternalData`, the one cross-assembly caller, was reading the lock back off a value it had itself constructed with that lock. It now takes `m_diagnosticsLock` directly. `NodeBrowser.DataLock` remains `protected` and is tracked in #4215: a subclass extending `Next()` legitimately needs its enumeration state atomic with the base browser's, so the fix needs a base-owned acquisition rather than a deletion. ### Migration coverage for the removed lock members None of the removed lock members get an `[Obsolete]` shim, and the migration guide now says why: a shim would have to be `object`-typed to keep old call sites compiling, and on .NET 9+ `lock (o)` on a statically-`object` reference is `Monitor.Enter`, which does **not** interlock with `Lock.Enter()`. It would compile, run, and silently synchronize nothing — strictly worse than a compile error. Re-adding a member to a downstream-implemented interface is also a break in its own right on `net48` / `netstandard2.0`, where there are no default interface methods. Instead, every removed member has a named replacement on the migration path: | Rule | Covers | Replacement it names | |---|---|---| | `UA0024` | `IServerInternal` / `ISession` / `ISubscription` `DiagnosticsLock`, `DiagnosticsWriteLock` | `UpdateServerDiagnostics(...)` / `UpdateDiagnostics(...)`, `ReadServerDiagnostics` / `ReadDiagnostics` | | `UA0025` | `ILocalNode.DataLock`, `Node.DataLock` | a lock the caller owns; the node guards its own state | | `UA0026` | `BaseVariableValue.Lock` | a lock passed to the `BaseVariableValue` constructor | Each rule fires on sources still written against 1.5.378, where the compiler only says a member is missing. `docs/MigrationGuide.md` gains a section per removal with before/after. ### The tests earned their keep The four concurrency tests were landed **first**, against the unmodified code, because removing a synchronization contract cannot be verified by the compiler and the existing `NodeStateHandlerConcurrencyTests` covers a different concern (racing a handler assignment against a write, not reader/writer exclusion). They now run with **no external locking**, which is what demonstrates the node guards itself. They caught two real problems, and one of them was mine: - The `Value`/`StatusCode`/`Timestamp` setters bypassed the new lock — a genuine race. - One test asserted an invariant the design does not offer. Two property setters are two lock acquisitions, so that pair was **never** atomic; it only passed originally because the test held a lock across both. It now uses a single attribute write carrying a whole `DataValue`, which genuinely is atomic and is the path the server uses. A third issue was found and fixed before it shipped: the reader loops gated on `!writer.IsCompleted`, so a reader scheduled after the writer finished performed zero reads — failing two runs in three on `net48`. Converted to `do…while`; now five consecutive green runs. **Verified:** `Stack.State` 116 passed on `net10.0` *and* `net48`; the concurrency category green 5/5 consecutive on `net48`; `Opc.Ua.Types.Tests` 8,456 passed; the server node-manager suite 1,205 passed, including `NodeManagerLifecycleTests` and `AsyncCustomNodeManagerTests` which exercise the changed paths. ## Two corrections to my own published figures Both were found while starting implementation, and both are recorded on #4183. **`ForceDiagnosticsScan()` was overstated.** The analysis claimed the `DiagnosticsWriteLock` getter *"triggers a full address-space diagnostics rescan"*. It is in fact a single flag assignment (`DiagnosticsNodeManager.cs:700-703`) consumed by the read paths and the periodic timer. The defect stands — a side-effecting property getter with a racy write on a synchronisation path — but its cost was misstated. **The contract was 15 sites, not 19.** A naive search matched ten comment and `#pragma` lines that *mention* `lock(source)` while explaining it. The documents now carry a counting command that excludes comment matches. A third finding expanded scope the other way: the `object`-typed lock leak is **repo-wide**, with ten such members across `Opc.Ua.Core`, `Opc.Ua.Types` and `Opc.Ua.Server` — including `Node.DataLock => this` and `ApplicationConfiguration.PropertiesLock => m_properties`, the same defect class as `Subscription.DiagnosticsLock`. ## Caveats stated in the documents - **Nothing in the designs is implemented or benchmarked.** Every performance claim across all seven designs is a design intention, not a measurement. - The larger phases are a **major undertaking against a live plugin API**; the documents say so rather than letting the neatness of the proposals imply otherwise. - The recommended sequencing puts low-risk, self-contained steps first and notes they are **worth doing regardless** of which strategy is eventually chosen. - Round 2 notes the two option designers' cost estimates are **not independent of their assignments**. ## Review guidance The two analyses make factual claims checkable by running the included commands. The design explorations are exploratory and need not be agreed with to merge. The code change is small, test-backed, and independent of which delivery strategy is eventually chosen. Reasonable outcomes include merging as-is, merging only the analyses and the lock fix, or disagreeing with the recommendation while keeping the evidence. --- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041 Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63 |
||
|
Marc Schier
|
0612a43fe4 |
Pluggable cryptography and hardware-held private keys (#4192)
Makes the stack's cryptographic operations replaceable - by another library, an offboard service, or hardware (TPM 2.0 / HSM / PKCS#11 token / cloud KMS) - with no performance cost in the default configuration, and extends this to certificates so a private key need never exist in process memory. Also fixes three pre-existing defects found while validating: STACKGEN001 erroring for every compilation the stack generator was merely loaded into, certificates failing to load from paths past MAX_PATH on Windows, and the MonitoredNode2 tests starving the thread pool. Fixes #4190 |
||
|
romanett
|
537b10c8b3 |
Fix History User Access Levels and add CTT coverage nodes (#4026)
# Description Improve Reference Server coverage for OPC UA Compliance Test Tool scenarios and document the latest CTT findings. ## Reference Server coverage - Historize the configured AccessRights nodes and seed them with the same deterministic mixed-quality history as the other test nodes while preserving their access controls. - Add a historian node that intentionally does not support ServerTimestamp. - Add a SelectionListType String instance whose Selections property is String[] as required by Part 5, with localized descriptions and RestrictToList enforcement. - Add an EUR CurrencyUnit property and verify its structured value and binary encoding. - Add a node with explicit read/write base access bits plus NonatomicRead and NonatomicWrite AccessLevelEx flags. - Ensure the first ByteString array elements meet the CTT minimum-length prerequisite. ## Latest CTT findings - Confirm run-23 A&C residuals are CTT re-entrant Read and branch-unaware Retain checks rather than server regressions. - Confirm NumberOfTransitions includes Uncertain values as required by Part 13. - Document the CTT CurrencyUnit decoder dropping a correctly encoded Exponent. ## Related Issues - Follow-up to #3960 and #4002. ## 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. - [x] I have addressed **all** PR feedback received. Focused affected tests pass on .NET 10 and .NET Framework 4.8. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@hotmail.com> |
||
|
Marc Schier
|
84fa8e8906 |
CTT follow-up: monitored items, alarms, codecs, history, and model validation (#4002)
# Description Follow-up to #3978 based on the latest OPC UA CTT aggregate, matrix, Historical Access, Attribute, and Node Management runs. ## Aggregate run 16 - Ignore synthetic `BadBoundNotFound` raw-bound markers when feeding aggregate calculators. - Add direct and live historian regression coverage. ## Multi-dimensional Variant matrices - Fix `Variant.Copy()` so rank-2 Variant matrices preserve their payload and dimensions. - Enforce valid Variant matrix dimensions consistently across Binary, JSON, XML stream, and XML parser codecs. - Encoders return `BadEncodingError`; decoders return `BadDecodingError` for malformed matrix dimensions. - Add cross-codec valid/null/empty/malformed tests and a batched ReferenceServer Read regression. - Correct the previous CTT issue attribution: the invalid `Scalar_Static_Arrays2D_Variant` server encoding poisoned the batched CTT Read. ## Node Management AddNodes - Return `BadNodeIdRejected` for unsupported requested-NodeId namespaces. - Validate hierarchical ReferenceType source/target NodeClass constraints. - Document run-18 confirmation that all remaining 15 AddNodes errors are CTT project/script configuration issues. ## Historical Access and Attribute Services run 18 - Reject whitespace in `NumericRange` syntax with `BadIndexRangeInvalid`. - Add HistoryRead tests for operation-level invalid syntax and per-DataValue `BadIndexRangeNoData`. - Document CTT defects: - `012.js` expects `BadIndexRangeNoData` at the operation level instead of on returned DataValues; - `Err-012.js` uses a non-historizing node for an access-denied test; - generic Attribute array helpers omit NodeId[] and StatusCode[] conversion paths. ## Base Information model validation run 19 - Run the CTT main session as `sysadmin` over Basic256Sha256 SignAndEncrypt so SecurityAdmin RolePermissions and EncryptionRequired restrictions are both satisfied without weakening the server model. - Preserve predefined well-known NodeIds while making generated dynamic type and optional-child factories allocate compatible per-instance NodeIds, remap internal references, and update references from the owning subtree. - Materialize dynamic RoleType properties through generated helpers and keep role-child string NodeIds disjoint from numeric RoleManager root IDs. - Add net48/net10 regressions for generator output, NodeId allocation/reference remapping, DI mandatory children, dynamic roles, and low-privilege versus SecurityAdmin Browse visibility. - Document CTT defects for permission-unaware mandatory-child validation, the UA 1.04 reference model used against a UA 1.05 server, stale ServerDiagnostics cache, and the `ConformanceUnits` scalar/array mismatch. ## Related Issues - Follow-up to #3960 and #3978. ## Checklist - [x] I have signed the CLA and read the CONTRIBUTING document. - [x] Tests prove the fixes and improve coverage. - [x] Required CTT documentation is updated. - [x] Focused builds/tests introduce no warnings. - [x] Affected tests pass on .NET Framework and .NET 10. - [ ] CI and CodeQL are green. - [ ] All PR feedback is addressed. ## Latest CTT full run and A&C run 20 - Fix Monitor Value Change V2 `036.js`: an ambiguous `DataValue(StatusCode)` call encoded `BadIndexRangeNoData` as a Good value. - Prove all 19 configured matrix monitored items notify when the selected value actually changes; document the independent `042.js` script defects. - Rebase dynamic `CertificateExpired` / `TrustListOutOfDate` alarm roots and descendants, restore standard type declarations and ModellingRules, and expose the cross-node-manager references. - Deliver method-triggered events to every authorized Session, fixing the two-Session Confirm scenario. - Preserve locale-only `LocalizedText` values consistently across Binary, JSON, and XML and align Acknowledge/Confirm comment behavior and tests. - Document the remaining proven CTT defects for TransactionDiagnostics, SemanticChange, History `013`, PolicyId scope, A&C, durable subscriptions, and Subscription Minimum events. ## Base synchronization Merged `master` through `007cfaf90` (`#4010`, repository layout consolidation) and retained the matrix regression tests at their new `tests/` locations. ## Latest validation - Focused merged-tree tests pass on net10 and net48. - `Opc.Ua.Server` and `Quickstarts.Servers` Release builds complete with 0 warnings and 0 errors on net10 and net48. - The existing `DisableEnableViaTypeAndInstanceMethodNoEventsDuringDisabledAsync` timing test took its documented inconclusive timeout path after the layout merge; no test failure was reported. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <romanett98@gmail.com> |
||
|
Marc Schier
|
8538f531e7 | Update paths for the consolidated repository layout | ||
|
Marc Schier
|
0da85b9d89 |
CTT conformance: access rights, X509, aggregates, and matrix diagnostics (#3978)
# Description Investigates successive OPC UA CTT runs and fixes the proven server conformance defects while documenting proven CTT script/codec defects in `plans/ctt-issues.md`. ## Server fixes - Grant `WriteAttribute` on `Scalar_Static_Int32` so attributes advertised by `WriteMask` are writable. - Disable the client-certificate-keyed authentication lockout in the dedicated CTT configuration so negative token tests do not mask later results with `BadUserAccessDenied`. - Map an X509 user-token signature algorithm mismatch to token-level `BadIdentityTokenInvalid`. - Persist rejected X509 user certificates to `pki/rejectedUser` to simplify trusted-user provisioning. - Populate `HistoricalDataConfiguration.AggregateConfiguration` with the server's actual defaults. - Return per-node `BadInvalidArgument` for processed reads with equal StartTime/EndTime. - Feed reverse processed reads in request order and preserve exact non-Bad interpolation boundaries. - Correct PercentGood/PercentBad uncertain classification. - Correct WorstQuality/WorstQuality2 eligible bounds, StatusCode preservation, and `MultipleValues`. - Correct reverse Min/Max Raw-vs-Calculated semantics. - Fix `HistoryClient.GetConfigurationAsync` traversal and aggregate-configuration reads. ## Tests and CTT evidence - Added direct calculator and live historian Part 11/13 oracle tests covering forward/reverse intervals, equal-time validation, bounds, status aggregates, DurationInState, and Min/Max. - Added exact Part 6 Table 26 binary golden-vector coverage for a 2×3 Int32 Matrix Variant. - Added in-process whole-matrix read/write and multidimensional `NumericRange` coverage. - Updated `plans/ctt-issues.md` with exact evidence and recommended fixes for CTT aggregate, X509, multidimensional-array, A&C, and AliasName script defects. No CTT failure is suppressed or accepted without a spec-based classification. ## Related Issues - Relates to #3960. ## Checklist - [x] Tests proving the server fixes are included. - [x] CTT-side defects are documented with script locations and specification references. - [x] Targeted aggregate/routing/matrix suites pass on net48 and net10. - [x] History aggregate/client suites pass on net48 and net10. - [x] Modified lines are analyzer-clean. - [ ] CI and CodeQL complete. - [ ] Maintainer review complete. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Roman Ettlinger <romanett98@gmail.com> |
||
|
Marc Schier
|
d7cb170b86 |
CTT compliance (#3960) + HA/Aggregate facets (#3875) + server-side pcap capture (#3962)
## Summary This PR bundles two independent bodies of work on the branch: 1. **CTT compliance & Historical Access** — improves OPC UA Compliance Test Tool (CTT) conformance of the Console Reference Server (issue #3960) and advertises the correct Historical Raw Data + Aggregate server facets (issue #3875). 2. **Server-side in-process packet capture (pcap)** — lets a hosted server capture its inbound client→server traffic in-process, with a documented non-DI wiring path. The CTT work was driven by an actual CTT run (`NewCTT2`, script v1.05.513) whose 4757 error results were parsed and clustered by conformance unit. Fixes are verified live via MCP against the running `--ctt` server and covered by unit/integration tests that are green on **net48 + net10**. ## Server-side in-process packet capture (pcap) Adds the ability to capture a hosted server's **inbound** (client→server) `opc.tcp` traffic in-process, symmetric to the existing client-channel capture in `Opc.Ua.Core.Diagnostics`. Previously `PcapBindings.Install(server.Server.TransportBindings)` only installed a client *channel* decorator — a server accepts connections through *listeners*, which never consult channel factories — so nothing was captured, and `InProcessServerCaptureSource` was a placeholder. Installing a binding also records nothing on its own until a `CaptureSessionManager` session is started (which is now documented). - **Core seam** (`Opc.Ua.Core`, opt-in, zero cost when unused): `TransportListenerSettings.AcceptedTransportDecorator` + `OnAcceptedChannel`, applied in `TcpListenerChannel.Attach(socket)` and the reverse-connect path; `TcpTransportListener` wires them per accepted channel. Default `null` → no behavior change for non-capturing servers. - **`PcapTransportListenerBinding`** — an `ITransportListenerFactory` **listener decorator** (the server-side mirror of the client `PcapTransportChannelBinding`) that injects the capture seam via `OpenAsync`, wraps each accepted channel's byte transport in `CapturingByteTransport`, and forwards channel-token key material to the active observer. - **`PcapBindings`** — `Install` now wires **both** client and server; new `InstallClient` / `InstallServer` helpers. `AddPcap` covers both sides too. `InProcessServerCaptureSource` is now functional. - **Non-DI usability** — new public `ClientChannelManager.DefaultChannelBindings` accessor so non-DI clients can install the binding into the registry the client actually uses, plus a new "Enable pcap capture without dependency injection" guide in `Docs/Diagnostics.md` (server / client / both recipes, install-before-start timing, the two-registries nuance) and `NugetREADME` updates. - **Tests** — Core settings-seam unit tests (net48 + net10), `PcapTransportListenerBinding` / `PcapBindings` unit tests, and a real server↔client integration test asserting a non-empty pcap of server-accepted frames. ## Fixes (CTT compliance / Historical Access) ### Historical Access — Read Raw (~1996 CTT errors) The new historian returned `Good_MoreData` when handing back a `ContinuationPoint`. Per OPC UA Part 11 §6.5.3.2 (and the CTT HA Read Raw tests 008/009, which assert `Good` **and** a `ContinuationPoint` together) the operation status must be **`Good`** — the non-empty ContinuationPoint alone signals more data. Fixed all four continuation sites (raw / modified / annotation / processed-event) in `HistorianDispatcher`, and additionally report **`Good_NoData`** for a raw/modified read of an interval that contains no data and requests no bounds (CTT HA Read Raw 007). Adds regression tests. ### Historical Access — HA Configuration (~282 CTT errors) Historizing reference-server nodes had no `HasHistoricalConfiguration` reference, so History Access aggregate clients (and the CTT) could not discover the archive/aggregate configuration. The reference server now attaches a populated `HistoricalDataConfigurationType` companion object ("HA Configuration") to every historizing node. This also fixes a latent bug in `HistoricalDataConfigurationInstaller`: a freshly created configuration instance (and its mandatory `Stepped` / `AggregateConfiguration` children) retained the type's shared NodeIds, so the reference targeted the type node and multiple nodes collided — the installer now assigns instance NodeIds to the whole subtree via the node manager's `INodeIdFactory`. Adds a regression test. ### Historical Access — Read Raw status codes Additional CTT HA Read Raw conformance fixes: - Insufficient details (<2 of startTime/endTime/numValuesPerNode) → `Bad_HistoryOperationInvalid` (Err-027). - IndexRange on scalar history → `Bad_IndexRangeNoData` at the operation level (Err-009). - Unknown/foreign ContinuationPoint (e.g. a Browse CP) → `Bad_ContinuationPointInvalid` (Err-014/Err-024). - No spurious ContinuationPoint on the final page of a one-value-at-a-time read (008/009). - `TimestampsToReturn.Neither` → `Bad_TimestampsToReturnInvalid` (Err-002). All validated against the actual CTT run (`NewCTT2` Run2, an HA-only re-run built with the base fixes) which confirms the 1999 "Good_MoreData vs Good" failures are gone. Covered by tests. ### Earlier commits in this PR Variant/BaseDataType array write `BadTypeMismatch`, DiagnosticInfos gating on `DiagnosticsMask`, multi-dimensional array values; strengthened Attribute Write conformance tests; Condition comment retention on null Acknowledge/Confirm; historized the full scalar type set + paired aggregate nodes; populated `ServerCapabilities.ConformanceUnits`; advertised `HistoricalRawData2022` + `AggregateHistorical2022` facets (#3875) with `Docs/Profiles.md` updates. ## Diagnosed (not server bugs — CTT connection configuration) ### Auditing Connections (~1684 CTT errors) All 1684 failures are identical ("Unable to Find Entry for ClientAuditEntryId"). Audit events are, per spec, delivered only to sessions holding the `SecurityAdmin` role: the server validates `PermissionType.ReceiveEvents` per event-monitored-item, and the integration test `AuditingOperationTests` connects as `sysadmin`/`demo` for exactly this reason. The CTT run collected zero audit events because it connected **anonymously**. Documented in `Docs/Diagnostics.md`: configure the CTT to authenticate as `sysadmin`/`demo` (mapped to `[SecurityAdmin, AuthenticatedUser]` in `ReferenceServer.cs`). ### Address Space WriteMask (~126 CTT errors) `revertToOriginalNodeAttributes` writes back all node attributes and gets `BadUserAccessDenied` (a permission error) rather than Good/BadNotWritable/BadAttributeIdInvalid — the same anonymous-connection cause; expected to clear when the CTT connects as `sysadmin`/`demo`. ## Still open Alarms & Conditions Retain/ConditionDisabled/InputNode (logic is spec-aligned on inspection; needs a CTT-faithful reproduction to separate a genuine bug from alarm-simulation timing), Aggregate function values (now unblocked by the HA Configuration fix), GDS self-registration, residual Attribute IndexRange/type-mismatch. CTT-script errors (`ArrayItems` / `ItemsHistoryCountExceeds` undefined) are not server-fixable. ## Testing `Opc.Ua.Server.Tests` (Historian) and `Opc.Ua.History.Tests` green on net48 + net10; historian and HA-configuration behaviour verified live against the running `--ctt` server. Server-side pcap capture: Core builds clean on net48 + net10, and the new unit + server↔client integration tests pass (the only failing `Opc.Ua.Core.Diagnostics.Tests` are 3 pre-existing, unrelated `OfflineSecureChannelSmokeTests`). The single most decisive remaining CTT step is a re-run authenticated as `sysadmin`/`demo`, which clears the permission-based clusters and isolates the true remaining server bugs. |
||
|
Marc Schier
|
76bceb51e2 |
Env-var driven Pcap auto-capture + rename to Opc.Ua.Core.Diagnostics + Docs/Diagnostics.md (#3888)
# Description
This PR also renames the project itself from `Opc.Ua.Bindings.Pcap`
(assembly / NuGet / folder / root namespace) to
`Opc.Ua.Core.Diagnostics`. `Pcap`-prefixed type names (`PcapFileWriter`,
`PcapOptions`, `IPcapAuditSink`, ...) and the `AddPcap*` extension
methods are preserved — only the project shell broadens to
`Core.Diagnostics`. The new NuGet id is
`OPCFoundation.NetStandard.Opc.Ua.Core.Diagnostics`. Folders were moved
with `git mv` so blame and log are preserved.
This PR also renames the **C# namespace** from
`Opc.Ua.Core.Diagnostics.*` to `Opc.Ua.Pcap.*` (commit `efc03691c`). The
**assembly identity** stays `Opc.Ua.Core.Diagnostics` (assembly name,
NuGet id `OPCFoundation.NetStandard.Opc.Ua.Core.Diagnostics`, folder,
csproj filename, `InternalsVisibleTo`, .slnx project paths, sign-lists
are all unchanged). Result: one `Opc.Ua.Core.Diagnostics.dll` assembly
hosting types in the `Opc.Ua.Pcap.*` namespace — standard .NET pattern
(cf. `System.Net.Http.dll` hosting `System.Web.*`). Future diagnostic
features can add other `Opc.Ua.*` namespaces to the same assembly
without another rename. 149 files changed (+411/-411 net); 422/422 tests
pass on net10.0.
This PR also merges `Docs/PacketCapture.md` and `Docs/Observability.md`
into a new `Docs/Diagnostics.md`. `ITelemetryContext` leads the doc with
expanded usage and extensibility guidance (custom contexts,
OpenTelemetry exporter wiring, ActivitySource / Meter naming
conventions, testing patterns). New sections cover OPC UA server audit
events and the built-in `Server.ServerDiagnostics` info-model nodes. The
1.4 / 1.5 evolution narrative has been dropped; the still-relevant
migration content has moved into `Docs/MigrationGuide.md`. All 8 inbound
doc / slnx / XML-doc references are retargeted.
This PR also adds a new public DI extension `AddPcapFromEnvironment` to
`OPCFoundation.NetStandard.Opc.Ua.Core.Diagnostics` that activates
packet capture and/or symmetric-key logging at host start based on two
environment variables. The variables are evaluated **once** during the
`AddXXX` call (matching the existing `OPCUA_PCAP_ENABLE_DIAGNOSTICS`
opt-in semantics):
| Variable | Effect when set |
|---|---|
| `OPCUA_PCAP_FILE` | Auto-start an in-process capture session on host
start. Frames are written verbatim to the literal path; the parent
directory becomes `PcapOptions.BaseFolder` so the existing
`SessionFolder` path-traversal validation still runs. |
| `OPCUA_KEYLOGFILE` | When set without `OPCUA_PCAP_FILE`: install a
stand-alone keylog observer (SSLKEYLOGFILE-style) directly into
`IChannelCaptureRegistry`. When set together with `OPCUA_PCAP_FILE`:
keylog records go to this path instead of the conventional
`*.uakeys.json` sibling of the pcap. |
When neither variable is set the call behaves exactly like `AddPcap`
— binding registered, no auto-start. When the auto-start path is
active the host emits a `Warning`-level log line naming the env var(s)
consumed and the resolved path(s); key material is never logged.
## Public surface
The new public API is intentionally narrow:
```csharp
namespace Opc.Ua.Core.Diagnostics.DependencyInjection
{
public static class PcapEnvironmentVariableNames
{
public const string OpcuaPcapFile = "OPCUA_PCAP_FILE";
public const string OpcuaKeyLogFile = "OPCUA_KEYLOGFILE";
}
public static partial class PcapServiceCollectionExtensions
{
public static IServiceCollection AddPcapFromEnvironment(this IServiceCollection services);
public static IServiceCollection AddPcapFromEnvironment(this IServiceCollection services, Action<PcapOptions> configure);
}
}
```
`StandaloneKeyLogObserver`, `PcapEnvironmentDefaults`, and
`PcapEnvironmentAutoStartHostedService` are internal — the env-var
names and DI methods are the only public surface, which keeps it
AOT-clean.
## Behaviour-impacting code change
`InProcessCaptureSourceBase` now honors
`StartCaptureRequest.PcapFilePath` and
`StartCaptureRequest.KeyLogFilePath` for in-process capture (previously
these properties were respected only by the Replay source, despite their
generic XML docs). Behaviour is **opt-in**: callers that pass `null`
continue to get the conventional `capture.pcap` / `keys.uakeys.json` /
`keys.uakeys.txt` filenames inside `SessionFolder`. Path-traversal
validation in `CaptureSessionManager.StartAsync` is unchanged.
## Operator example
```yaml
# Kubernetes manifest snippet
env:
- name: OPCUA_PCAP_FILE
value: /var/log/opcua/cap.pcap
- name: OPCUA_KEYLOGFILE
value: /var/log/opcua/keys.uakeys.json
```
```csharp
// Program.cs
HostApplicationBuilder builder = Host.CreateApplicationBuilder();
builder.Services.AddOpcUa().AddClient(_ => { });
builder.Services.AddPcapFromEnvironment();
await builder.Build().RunAsync();
```
For SSLKEYLOGFILE-style keylog only (no pcap):
```bash
export OPCUA_KEYLOGFILE=$HOME/opcua-keys.uakeys.json
```
## Security model
Anyone (process, container runtime, host operator, sidecar) who can set
these env vars can divert OPC UA channel keys and traffic to
attacker-controlled paths. `Docs/PacketCapture.md` gains a security note
and an operator-checklist item that ties this to the existing security
guidance: the destination path / directory must be operator-controlled
and protected with file-system permissions in addition to the binding's
own `0600`-on-Unix file mode.
|
||
|
Marc Schier
|
f358078b1c |
[Client] Central channel manager: ref-counted shared channels, coalesced reconnect, IRetryBudget (#3288) (#3852)
# Description Introduces a central `IClientChannelManager` that owns client-side transport channels with reference counting, sharing, coalesced reconnect, and asynchronous participant notification. Sessions, discovery clients, and registration clients targeting the same `ConfiguredEndpoint` (with the same reverse-connect identity) now share a single underlying `ITransportChannel`; reconnect is transparent to callers; and the previous `AttachChannel`/`DetachChannel` + `SessionReconnectHandler` patterns are obsoleted. ## What lands ### Core types (`Stack/Opc.Ua.Core/Stack/Client/`) - `ManagedChannelKey`, `ChannelState`, `ChannelStateChange`, `ParticipantReconnectResult` - `IReconnectParticipant`, `IManagedTransportChannel`, `IClientChannelManager` - `IChannelReconnectPolicy` + `ExponentialBackoffChannelReconnectPolicy` (default mirrors historical `SessionReconnectHandler`: 500 ms → 30 s exponential) - `IRetryBudget` + `RetryBudget` — shared retry deadline so the two-level retry no longer compounds multiplicatively - `ClientChannelManager` — single non-partial sealed class with SOLID-extracted internal types under `Stack/Opc.Ua.Core/Stack/Client/Channels/Internal/`: `ChannelEntry` (behind `IChannelEntryHost`), `ManagedTransportChannelLease`, `ClientChannel`, `ClientChannelManagerMetrics` (IMeter), `ClientChannelManagerCertRotation` (auto `ReconnectAllAsync` on `CertificateManager` rotation), `ClientChannelManagerDiagnostics` (ActivitySource + EventSource). Refcount, coalesced reconnect, and transparent faulted-entry recovery via `SwapFaultedEntryAsync` (swaps a lease's underlying entry on subsequent `ReconnectAsync` when the prior cycle Faulted/Closed, preserves participant, back-off via policy `GetDelay(SwapCount)`) ### Three-state lifecycle gate `Disconnected` → `TransportConnecting` / `TransportReconnecting` → `TransportConnectedSessionReactivating` → `Ready` (or `Faulted` / `Closed`). Only `Ready` releases the service-call gate; the manager bypasses the gate internally for participant `ActivateSession` traffic during reactivation via an `AsyncLocal` scope. ### Session integration (`Libraries/Opc.Ua.Client/Session/`) - New `Session.CreateAsync(IClientChannelManager, …)` overload - `Session` implements `IReconnectParticipant`; reactivation handler distinguishes `Reactivated` / `RequiresSessionRecreate` / `TransientFailure` / `FatalForParticipant` / `FatalForChannel` - `Session.ReconnectAsync(ct)` automatically delegates to the channel manager when wired - `Session.RecreateInPlaceAsync` swaps managed-channel leases for failover-to-different-endpoint via a new atomic `IClientChannelManager.GetAsync(endpoint, participantFactory, …)` overload; same-key recreates delegate to manager reconnect; explicit-channel/connection callers retain legacy behavior - KA failure routes through `IClientChannelManager.ReconnectAsync(channel)` ### ManagedSession integration - `WithChannelManager(...)` builder + ctor param; DI-resolved automatically - Two-level retry: channel-mgr handles transparent reconnect; outer `ConnectionStateMachine` + `IReconnectPolicy` only triggers on terminal channel `Faulted`. Outer-state churn suppressed during channel-mgr cycles via `_channelReconnectInProgress` flag wired through `IManagedTransportChannel.StateChanged` - New `ManagedSession.ChannelStateChanged` event surfaces transparent reconnects - `ConnectionStateChangedEventArgs.UnderlyingChannelState` populated when outer reconnect was triggered by a channel fault - Shared `IRetryBudget` propagated so the layers enforce a single max-total-time ### Discovery / Registration / GDS Channel-manager-aware `CreateAsync` overloads on `DiscoveryClient`, `RegistrationClient`, `ServerPushConfigurationClient`, `LocalDiscoveryServerClient`, `GlobalDiscoveryServerClient`. Each registers a minimal `IReconnectParticipant`; co-located sessions and discovery probes targeting the same endpoint share a channel. ### HTTPS transport (`Stack/Opc.Ua.Bindings.Https/`) - New `IOpcUaHttpClientFactory` abstraction - HTTPS / OPC-HTTPS channels migrated to `IHttpClientFactory` + `Microsoft.Extensions.Http.Resilience` (10.6.0; AOT-compatible on net10 via source generators; legacy direct-HTTP path retained for older TFMs / no-DI consumers) ### DI `AddOpcUa().AddClient(...)` registers `IClientChannelManager` and the named HTTPS `HttpClient` + standard resilience handler automatically. `ManagedSessionBuilder` resolves the manager from DI when present. ### Obsoletions (functional, `[Obsolete]` with guidance) - `IClientBase.AttachChannel` / `DetachChannel` - `SessionReconnectHandler` - `SessionExtensions.ReconnectAsync(connection, ct)` and `(channel, ct)` legacy overloads ### Tests - Core: `ClientChannelManagerManagedTests`, `ClientChannelManagerCertRotationTests`, `RetryBudgetTests`, `OpcUaHttpClientFactoryTests` — **56 / 56 pass** - Client: `ManagedSession*Tests` updated; legacy `SessionReconnectHandlerTests` + `SessionExtensionsTests` preserved with `#pragma warning disable CS0618` — **129 / 129 pass** - Sessions integration: `ChannelManagerSharing` / `TransparentReconnect` / `SessionLifecycle` / `CertRotation` — **7 pass, 1 skipped** (`ChannelManagerExhaustionEscalatesAndRecoversWhenServerReturns` un-`[Ignore]`d and implemented in this PR via `SwapFaultedEntryAsync`; remaining skip is end-to-end `RequiresSessionRecreate` plumbing — carry-forward) - Stress (consolidated `Tests/Opc.Ua.Stress.Tests/Subscriptions/` + `Channels/`): chaos / contract / soak / lifecycle / leak-accuracy / gap / participant-result-aggregation tests merged from the previously-separate `Opc.Ua.Channels.Stress.Tests` project; nightly run via `.github/workflows/stress-test.yml` Builds clean (0 warnings / 0 errors) across all six TFMs: `net472`, `net48`, `netstandard2.1`, `net8.0`, `net9.0`, `net10.0`. ### Docs - `Docs/Sessions.md` new § 4 documents the channel manager, three-state gating, two-level retry with shared `IRetryBudget`, HTTPS resilience layering - `Docs/MigrationGuide.md` migration recipes for the four obsoleted API groups - `Docs/DependencyInjection.md` channel-manager and HTTPS factory registration ## Carry-forward (separate PRs) - End-to-end `ParticipantReconnectResult.RequiresSessionRecreate` → `Session.RecreateAsync` lifecycle wiring (currently incomplete — flagged in the 1 remaining skipped integration test) ## Related Issues - Fixes #3288 - Builds on #3282 (PR that introduced the initial `ITransportChannelManager` skeleton) ## 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. *(Channel-manager-specific tests + ManagedSession/client tests verified across all 6 TFMs; full UA.slnx run is pending CI.)* - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. |
||
|
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. |
||
|
Marc Schier
|
4756e7c245 |
DI (OPC 10000-100) with samples (Pump, SW Update), fluent API gap closures and model dependency implementation (#3808)
This PR implements the **OPC UA Device Integration (DI, OPC 10000-100)** companion-spec library trio (`Opc.Ua.Di` / `Opc.Ua.Di.Server` / `Opc.Ua.Di.Client`) plus a worked example (`Applications/MinimalPumpServer`) demonstrating the OPC 40223 Pumps companion specification end-to-end with full simulation. It also closes 11 fluent-API gaps in `Opc.Ua.Server.Fluent`, three source-generator bugs, integrates the DI layer into the unified `services.AddOpcUa()` Microsoft.Extensions DI hosting pattern, and adds 250+ new tests. **Worked example — `Applications/MinimalPumpServer/`** - AOT-compatible console host using `Host.CreateApplicationBuilder` + `AddOpcUa()`. - `PumpNodeManager` inherits from `DiNodeManager`; multi-namespace (DI + Machinery + Pumps) via runtime NodeSet2 loading and `ModelLoaderBuilder`. - Demonstrates declarative second-pump creation via `.ConfigureDevicesFor<PumpNodeManager>(...)` and software-package seeding from the same configurator. - Full simulation: pressure, temperatures, power, flow, efficiency, level, cavitation/overheat fault injection, alarm setup, NAMUR-style supervision. **Documentation** - `Docs/Opc.Ua.Di/` — README, DeviceBuilder, Hosting, LockService, SoftwareUpdate, ClientHelpers (6 new developer guides). - `Docs/Opc.Ua.Di/ComplianceMatrix.md` — every DI declaration mapped to In-v1 / Generated / Deferred / Excluded. - `Docs/CompanionSpecLibraries.md` — packaging guide for new companion-spec libraries. - `Docs/FluentApiGaps.md` — historical record of G1–G11. - `Docs/SourceGeneratedNodeManagers.md` updated for richer node managers. - `Docs/plans/StateMachineBuilder.md` — deferred state-machine builder design. |
||
|
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. |
||
|
Marc Schier
|
01d7162313 |
Part 9 Alarms & Conditions: complete server + client implementation (#3805)
Complete implementation of **OPC UA Part 9 ΓÇö Alarms & Conditions** for
both server and client, **OPC UA Part 16 ΓÇö State Machines** (generic
extensible client + server API), and the supporting infrastructure
(source-generated event records, source-generated ObjectType proxies,
`IAsyncEnumerable` streaming subscriptions, address-space model change
tracking, per-node `INodeCache` invalidation, unified DI integration).
### Server-side (Part 9 complete)
- **`AlarmConditionState`** ΓÇö every Part 9 method wired automatically
in `OnAfterCreate`: `Silence`, `Suppress`/`Suppress2`,
`Unsuppress`/`Unsuppress2`, `RemoveFromService`/`RemoveFromService2`,
`PlaceInService`/`PlaceInService2`, `Reset`/`Reset2`,
`GetGroupMemberships`. Each method generates the spec-mandated audit
event and supports an optional veto-delegate hook.
- **Latched alarms** ΓÇö `ActiveState` reflects real process state;
`LatchedState` persists until explicit `Reset` (with full precondition
validation).
- **Re-alarming** ΓÇö
`ProcessReAlarm`/`ResetReAlarmRepeatCount`/`IsReAlarmEnabled` helpers
(opt-in, no hidden timer).
- **Audible alarms** ΓÇö `UpdateAudibleState` for
`AudibleEnabled`/`AudibleSound`/`SilenceState` lifecycle.
- **Suppression + groups** ΓÇö `AlarmGroup`, `AlarmSuppressionEngine`
(with first-in-group), `AlarmRateTracker` (for `AlarmMetricsType`).
- **Spec-compliance fixes** ΓÇö `SuppressedOrShelved` now includes
`OutOfServiceState`; `GetRetainState` includes `LatchedState`;
`EffectiveDisplayName` composition includes `OutOfServiceState` +
`LatchedState`.
### Client-side Part 9 API
- **`AlarmClient`** ΓÇö typed Part 9 operations. Each method delegates
to the matching source-generated `*TypeClient` proxy
(`ConditionTypeClient`, `AcknowledgeableConditionTypeClient`,
`AlarmConditionTypeClient`, `DialogConditionTypeClient`,
`ShelvedStateMachineTypeClient`) ΓÇö one source of truth for every
method NodeId.
- **Interface hierarchy** — `IConditionOperations` →
`IAcknowledgeableConditionOperations` → `IAlarmOperations` +
`IDialogConditionOperations`.
- **Source-generated event records** ΓÇö new `EventRecordGenerator`
(alongside `ObjectTypeProxyGenerator`) emits `{Type}TypeRecord` per OPC
UA event type: `ConditionTypeRecord` →
`AcknowledgeableConditionTypeRecord` → `AlarmConditionTypeRecord` →
`LimitAlarmTypeRecord`/`ExclusiveLimitAlarmTypeRecord`/… all the way
to vendor subtypes. **Vendor alarm types get records automatically.**
- **`AlarmEventDecoder`** ΓÇö decodes `EventNotification.Fields` into
the most-specific generated record.
- **`AlarmEventFilterBuilder`** ΓÇö fluent builder for Part 9
`EventFilter`s.
- **`ISession.GetAlarmClient(telemetry)`** convenience extension.
### Part 16 state machines (generic, extensible)
- **Client**: extension methods on the source-generated
`StateMachineTypeClient` / `FiniteStateMachineTypeClient` ΓÇö
`GetCurrentStateAsync`, `ObserveFiniteTransitionsAsync`
(`IAsyncEnumerable<FiniteStateSnapshot>`), `WaitForStateAsync`,
`GetAvailableStatesAsync` / `GetAvailableTransitionsAsync`. **Vendor
proxies inherit the API automatically** through the generated
inheritance chain.
- **Server**: `StateMachineBuilder` (fluent: `AddState` /
`AddTransition` / `OnCause` / validation) +
`FluentFiniteStateMachineState` ΓÇö declare a Part 16 state machine
without subclassing the abstract base.
- **Alarms alignment**: `AlarmClient.GetShelvingStateAsync` /
`ObserveShelvingTransitionsAsync` surface the API for every alarm's
`ShelvingState` child.
### IAsyncEnumerable streaming subscription
- **`IStreamingSubscription`** with
`SubscribeDataChangesAsync`/`SubscribeEventsAsync` returning
`IAsyncEnumerable<T>` (Channel-backed).
- **`ManagedSession.DefaultStreaming`** ΓÇö lazy shared subscription;
disposed with the session.
- **Helpers** ΓÇö `TakeUntilAsync`, `WithTimeoutAsync`, `TakeAsync`,
`BufferedAsync` for state-machine waits and bounded observation windows.
- **`AlarmStreamExtensions`** ΓÇö
`SubscribeAlarmsAsync`/`SubscribeConditionsAsync`/`SubscribeDialogsAsync`
returning the source-generated record types.
### Address-space model change tracking
- **Server**: `RaiseGeneralModelChangeEvent` on both `CustomNodeManager`
and `AsyncCustomNodeManager`; `ModelChangeAggregator` (in
`Opc.Ua.Server.NodeManager`, used by every node manager) batches changes
per service call; **auto-emit** from `CreateNode`/`DeleteNode` (opt-out
via `ModelChangeEmissionEnabled = false`).
- **Client**: `IModelChangeTracker` subscribes to
`BaseModelChangeEventType` via the streaming subscription, decodes
`ModelChangeStructureDataType` payloads, invalidates the `INodeCache`
per-node.
- **`INodeCache.InvalidateNode(NodeId)`** ΓÇö new interface member (true
per-node eviction in `NodeCache`).
- **`ManagedSession.EnableModelChangeTrackingAsync()`** +
`ManagedSessionOptions.ModelChangeTracking` +
`ManagedSessionBuilder.WithModelChangeTracking()` fluent method.
### Dependency Injection integration
- **`services.AddOpcUa().AddClient(...).AddAlarms()`** ΓÇö registers
`AlarmClientFactory` so DI-hosted apps can resolve a Part 9 client per
session without `new`-ing one manually.
- Cross-link added to the unified DI surface (`AddServer` / `AddClient`
/ `AddComplexTypes` / `AddGdsServer` / `AddLdsServer` /
`AddWotConServer`).
### Quickstart server modernization
- `Quickstarts.Servers/Alarms/AlarmNodeManager.cs` ported to
`AsyncCustomNodeManager` and demonstrates the new `AlarmGroup` +
`AlarmSuppressionEngine` end-to-end via a writable `MaintenanceMode`
boolean that suppresses every member alarm.
### Documentation
- New: `Docs/AlarmsAndConditions.md`, `Docs/StreamingSubscription.md`,
`Docs/ModelChangeTracking.md`, `Docs/StateMachines.md` ΓÇö full
developer guides.
- Updated: `Docs/DependencyInjection.md` (`AddAlarms()` row + section),
`Docs/README.md` index, `Docs/MigrationGuide.md` (Part 9 / Part 16 /
model change tracking sections covering behavior changes + breaking API
surface).
- Sample: `Applications/ConsoleReferenceClient/AlarmClientSample.cs`.
|
||
|
Marc Schier
|
889572472f |
Convert DataValue from class to readonly struct for further GC relief (#3796)
Convert `DataValue` from a mutable reference type (`class`) to a `readonly struct` to eliminate per-allocation GC pressure on every attribute read, subscription publish, and history sample. ### Key API changes (user-visible) | Before | After | Pattern | |---|---|---| | `class DataValue` | `readonly struct DataValue : INullable` | Foundation | | `Equals(DataValue?)` / `operator ==(DataValue?, DataValue?)` | `Equals(DataValue)` / `operator ==(DataValue, DataValue)` | Value semantics | | `IEncoder.WriteDataValue(string?, DataValue?)` | `IEncoder.WriteDataValue(string?, DataValue)` | Non-nullable wire API | | `IDecoder.ReadDataValue(...) -> DataValue?` | `IDecoder.ReadDataValue(...) -> DataValue` | Same | | `IDataChangeMonitoredItemQueue.PeekLast/Oldest()` | `TryPeekLastValue/Oldest(out DataValue)` | TryGet pattern | | `IAggregateCalculator.GetProcessedValue(bool)` | `TryGetProcessedValue(bool, out DataValue)` | Same | | `IUaPubSubDataStore.ReadPublishedDataItem(...)` | `TryReadPublishedDataItem(NodeId, uint, out DataValue)` | Same | | `Dictionary<uint, DataValue?>` (NodeCacheContext) | `Dictionary<uint, DataValue>` | Default + IsNull | | `IHistoryDataSource.FirstRaw/NextRaw` | `TryFirstRaw/TryNextRaw(out DataValue)` | TryGet pattern | | `NodeState.ReadAttribute(... DataValue)` | `NodeState.ReadAttribute(... ref DataValue)` | Sync mutation via ref | | `NodeState.ReadAttributeAsync(... DataValue)` | `NodeState.ReadAttributeAsync(...) -> ValueTask<(ServiceResult, DataValue)>` | Async return tuple | | `obj as DataValue` (heterogeneous Dictionary<string, object>) | `obj is DataValue dv ? dv : default(DataValue)` | Safe struct cast | | `Assert.That(dv, Is.Not.Null)` (tests) | `Assert.That(dv.IsNull, Is.False)` | Struct test pattern | | `[Obsolete] Value` setter, all property setters | Removed; use ctor or `With*()` | Immutability | | `[DataContract]` + `[DataMember]` | Removed; surrogate via `SerializableDataValue` + `DataValueSurrogateProvider` | Serialization | ### Wire format Wire format is **byte-identical** to the previous class-based implementation across binary / JSON / XML encoders. Default DataValue (`.IsNull == true`) is treated as the equivalent of the old null reference (writes JSON `null`, binary `byte 0`, XML omitted element). ### Why TryGet pattern (instead of leaving `DataValue?`) For methods that previously returned `DataValue?` to signal "no value available" (queue peek, aggregate result, data store lookup, history navigation), the conversion to `bool TryGet(out DataValue)` matches BCL conventions (`Dictionary.TryGetValue`, `Queue.TryPeek`, `Int32.TryParse`) and gives the caller an explicit "may be absent" affordance without `Nullable<DataValue>` overhead. ### Breaking changes summary - `DataValue` is now a `readonly struct` - boxing/unboxing semantics change for callers using `object`-typed APIs. - `IEncoder.WriteDataValue` / `IDecoder.ReadDataValue` / `ReadDataValueArray` interface signatures lose the `Nullable` wrapper. - `IDataChangeMonitoredItemQueue.PeekLastValue` / `PeekOldestValue` -> `TryPeekLastValue` / `TryPeekOldestValue` (out parameter). - `IAggregateCalculator.GetProcessedValue` -> `TryGetProcessedValue` (out parameter). - `IUaPubSubDataStore.ReadPublishedDataItem` -> `TryReadPublishedDataItem` (out parameter). - `NodeState.ReadAttribute` takes `ref DataValue`; `ReadAttributeAsync` returns `ValueTask<(ServiceResult, DataValue)>`. - `DataValue` property setters removed (use constructor + `With*` mutators). |
||
|
Marc Schier
|
3e5f129c6f | Enable nullable in Stack, Libraries (9), and Applications projects (#3732) | ||
|
Marc Schier
|
96f8964704 |
Fix erroneous debug.assert in SessionManager (#3759)
* Erroneous debug.assert in SessionManager * Improve build pipelines |
||
|
Marc Schier
|
d14cd10a86 |
Model dependency annotations using source generator + flaky test fixes (#3687)
* Fix publish/dispose race that hangs Client tests
Subscription.Dispose pre-disposed m_publishTimer and m_messageWorkerCts
before calling ResetPublishTimerAndWorkerState, causing Cancel() to throw
ObjectDisposedException and leaving the publish worker stuck. Removed the
duplicate disposal so the reset method is the single owner of those resources,
and guarded Cancel/Dispose with try/catch ObjectDisposedException for defense
in depth.
Session.OnPublishComplete could run after Session.Dispose disposed
m_reconnectLock, throwing ObjectDisposedException from SemaphoreSlim.Wait()
and re-arming via the throttle path indefinitely. Added an early Disposed
guard, wrapped Wait/Release in try/catch ObjectDisposedException, and added
a Disposed guard in QueueBeginPublish so the throttle cannot reschedule
publishes after disposal.
Fixes the macOS net10.0 hang seen in Azure pipeline build 13257
(TransferSubscriptionAsync, ReconnectWithSavedSessionSecretsAsync).
* fix lockout test and use UserName token for SuccessfulAuthenticationClearsFailedAttempts test
After
|
||
|
Marc Schier
|
09e44453a4 |
Generate type safe (method) proxies for ObjectTypes inside models (#3685)
Add interop test agent The new stack-pass ObjectMethodProxyGenerator emits proxies that derive from Opc.Ua.ObjectTypeClient and reference Session.MessageContext.NamespaceUris. Neither type exists in the in-memory test compilation, which only references Opc.Ua.Types.dll. Moving ObjectTypeClient to Opc.Ua.Types would cascade ISessionClient, IClientBase, ITransportChannel and the generated service interfaces with it, so the type stays in Opc.Ua.Core and we add a minimal stub to OpcUaCoreStubs instead. Wire Microsoft.Extensions.Options 10.0.5 + DependencyInjection.Abstractions 10.0.5 into Directory.Packages.props and the Gds.Client.Common project to enable upcoming options-driven configuration work. Modernize Opc.Ua.Gds.Client.Common (DI, options, interfaces, async dispose) |