mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-16 21:02:51 +08:00
master
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 romanett
|
9b034f8291 |
Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700)
# Description Nullable reference types are now enabled once, in `common.props`, and every project in the repository builds warning-clean with them. **Configuration** - `<Nullable>enable</Nullable>` lives in `common.props`. All per-project `<Nullable>` settings are removed: 178 project files (enable, annotations, and the `disable` in the three Aot test projects). - 419 per-file `#nullable enable`/`restore` directives and the 4 `#nullable disable` directives are removed. - Kept on purpose: - The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`, `PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the project setting in generated code, so the directive is required there. - `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against the 1.5.378 packages and does not import the repository props. - `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now describe the central setting. **Migration** (the 43 unset and 13 annotations-only projects, roughly 10k diagnostics) - **Production code: annotated at the declarations.** Covers `tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the LDS sample. - Members, parameters and returns are nullable where null actually flows. - Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`, `IsMethodTypeDesign`). - `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take `params object?[]`. `Template.AddReplacement` takes `string?` (rendering already skips null replacements). - `IRegisteredServerStore.RegisterAsync` returns `Task<RegistrationEntry?>`: it returns null when a registration is removed. - Dereferences backed by an invariant (for example, the validated model) use `!`, so runtime behaviour is unchanged. - **Behavioural changes in production code** (small, deliberate): - `NodeSetToModelDesign` throws `InvalidOperationException` when the file is not a NodeSet. Before, it threw a NullReferenceException on the next line. - `ModelDesignValidator` starts its lookup tables empty instead of null. A redundant `if (m_nodes == null) return;` guard is gone. - **Test code: minimal annotations.** - `?` on values that can be null, `!` where the test asserts non-null. - `= null!` for fixture members assigned in SetUp. - `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the `IEncodeable` constraint. - A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`) folded into the adjacent `IsNull`/`Count` checks. **Notes for reviewers** - The diff is large (~1,500 files) but almost entirely mechanical: `?`, `!` and directive removal. The non-annotation changes are listed above. - Known follow-up, not done here: `IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay non-nullable even though the validator returns null before it loads a dictionary. Making them nullable would ripple through every generator call site. ## Related Issues - Fixes #4439 ## 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. - [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. The "no new warnings" box covers net10.0 and net48: full non-incremental builds of `UA.slnx`, `tools/SourceGeneration.slnx`, `tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0 warnings and 0 errors under the repository's warnings-as-errors settings. net9.0 and net8.0 also had no nullable diagnostics, but they were not rebuilt after the final fixes. Tests have not been run locally yet. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Copilot
|
c03c128384 |
Treat code analysis (CA) warnings as errors and fix all surfaced analyzer warnings (#4680)
# Description `TreatWarningsAsErrors` was already enabled, but `CodeAnalysisTreatWarningsAsErrors` was `false`, so Microsoft Code Analysis (`CAxxxx`) warnings were non-fatal. This PR sets it to `true` in `common.props` and fixes all 55 analyzer warnings the net10.0 build reported. **Production code** - `SamplingGroup.cs`: CA2213 is a false positive here because the detached `CancellationTokenSource` is disposed by `CancelSamplingLoop` once the loop completes. It is suppressed with a pragma, a justification and a TODO, following the existing pattern in `ServerBase.RequestQueue.cs`. **Tests** - CA1861: constant array arguments are moved to `static readonly` fields or replaced with `ArrayOf<T>` collection expressions. In the legacy interop server I used locals instead, because those arrays become node values that writes can change in place. - CA2025: tasks that finish before disposal (`Assert.ThrowsAsync`, or awaited before `using` scope exit) are suppressed with justification comments, following existing test patterns. - CA1812: classes created by the DI container are suppressed with a comment and a TODO. - CA2208: benchmark switch defaults now throw `InvalidOperationException`. - CA1845: `Substring(0, 8)` is replaced with `[..8]`. - CA1307: `string.Replace` is replaced with `Split`/`Join`, which also works on net48. - CA5394: `Random` is replaced with `RandomNumberGenerator`. **Docs** - `docs/DeveloperGuide.md` now says CA warnings also fail the build, and that any suppression needs a comment explaining why plus a TODO. **Validation** - I fixed the 55 warnings from a net10.0 build of `UA.slnx` (`CustomTestTarget=net10.0`). That build ran before the setting was turned on, and I have not rebuilt net10.0 since the fixes. - `CustomTestTarget=net48` build of `UA.slnx`: 0 warnings, 0 errors. - I also started a full build across all target frameworks with the setting on. Its log showed no errors or warnings while it ran, but I couldn't read its final result. - No tests have been run yet; CI should confirm the multi-TFM build and the tests. ## Related Issues - N/A ## 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. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> |
||
|
romanett
|
b03575b4e2 |
Run the interop tests against six foreign OPC UA stacks and extend feature coverage (#4644)
## Proposed changes Runs the interop fixtures added in #4630 against peers built on six other open source OPC UA stacks, so the 2.0 client and server are tested against independent implementations, not only the released 1.5.378 stack. It also extends the fixtures beyond read/write/browse to events, subscriptions, identity and more services. ### Peers (`tests/Opc.Ua.Interop.Peers`) | Peer | Stack | Toolchain | Roles | | --- | --- | --- | --- | | `node-opcua` | node-opcua 2.186.15 (MIT) | Node.js 24 | server, client | | `milo` | Eclipse Milo 1.1.7 (EPL-2.0) | Java 17+, Maven | server, client | | `asyncua` | opcua-asyncio 2.0.1 (LGPL-3.0) | Python 3.12 | server, client | | `open62541` | open62541 1.5.8 (MPL-2.0), OpenSSL backend | C, CMake | server, client, ECC | | `async-opcua` | async-opcua 0.19.0 (MPL-2.0) | Rust | server, client | | `gopcua` | gopcua 0.9.1 (MIT) | Go | client only | Every peer speaks the command line and stdout contract of `Opc.Ua.Interop.LegacyPeer` (`server` / `client --checks`, `PEER-INFO {json}`, `RESULT {json}`). The stacks are fetched at build time and run as separate processes; nothing is linked into or shipped with any package. ### Harness - **Peer selection:** `OPCUA_INTEROP_PEER_HOST` + `OPCUA_INTEROP_PEER_HOST_ARGUMENTS` choose the host (`node`, `java -jar`, `python`) for the peer named by `OPCUA_INTEROP_LEGACY_PEER`. Native peers (open62541, async-opcua, gopcua) run without a host. Unset, the 1.5.378 peer runs exactly as before, so the main CI is unchanged. - **`PEER-INFO {json}`:** stack, version, application URI, software version and, optionally, the security policies the peer implements. `LegacyServerInteropTests` compares the ApplicationUri and SoftwareVersion against it. `EccInteropTests` runs policies beyond the four 1.05 ECC policies only against a peer that declares them. - **`expected-differences.json` per peer:** confirmed defects of a stack, keyed by check name, `Fixture.Test` or a `*` prefix, each with the source file or spec section that shows it. A listed failure is reported as inconclusive with its reason; a listed test that passes produces a warning. The 2.0 client tests apply it through the fixture-level `[PeerDifferences]` action. ### New coverage (all peers, including the 1.5.378 peer) - **16 client checks**, run by each peer's client against the 2.0 server (`LegacyClientFeatureCheckAsync`, Basic256Sha256 SignAndEncrypt): EventSubscription, ConditionRefresh, AlarmAcknowledge, DeadbandFilter, QueueOverflow, Triggering, Republish, TransferSubscription, WrongPasswordRejected, X509UserToken, RegisterNodes, HistoryReadRaw, NodeManagement, IndexRange, FindServers, SessionReconnect. - **`ServerFeatureInteropTests`**: the 2.0 client against each peer server. It covers RaiseEvent, ConditionRefresh, absolute deadband, queue overflow bit, triggering, republish errors, transfer to a new session, X509 user token, RegisterNodes, IndexRange, AddNodes/DeleteNodes, FindServers and session reconnect. Every peer server gained a `RaiseEvent` method and accepts X509 user tokens. - **ECC:** with open62541's OpenSSL backend, `EccInteropTests` adds `ECC_nistP256_AesGcm` and `ECC_nistP256_ChaChaPoly` (both directions; SignAndEncrypt anonymous and username, Sign, and token renewal across two renewals). It also adds `ECC_curve25519`/`ECC_curve448` cases, which are ignored while 2.0 marks those policies unsupported. ### CI and docs - **Workflow `interop-foreign.yml`:** opt-in (nightly, `workflow_dispatch`, PRs labelled `interop`), not a required check. It builds all six peers on Windows, Linux and macOS (open62541 from source on Linux only, against the distro OpenSSL), with dependency caching. - **`tests/Opc.Ua.Interop.Peers/README.md`:** how to install the toolchains, build each peer (including Windows without Visual Studio), run the fixtures against it, maintain expected differences and add a new stack. `DeveloperGuide.md` links to it. ## Results Run locally, sequentially, net10.0, Windows. Not passed = expected differences, inconclusive by design (e.g. AddNodes on a server without NodeManagement), or ECC cases a peer or the platform does not support. | Peer | Fixtures | Total | Passed | Failed | | --- | --- | --- | --- | --- | | 1.5.378 | whole interop project | 220 | 218 | 0 | | node-opcua | server + client + feature | 145 | 143 | 0 | | Eclipse Milo | server + client + feature | 145 | 141 | 0 | | asyncua | server + client + feature | 145 | 133 | 0 | | open62541 (OpenSSL) | server + client + feature + ECC | 199 | 175 | 0 | | async-opcua | server + client + feature | 145 | 137 | 0 | | gopcua | client | 104 | 81 | 0 | After the AEAD cases were added, `EccInteropTests` against the 1.5.378 peer ran separately: 54 total, 26 passed, 0 failed. The new cases are ignored there because the peer does not declare them. ## Findings No 2.0 defects beyond the two now fixed in #4654: - Browse with a null ViewId and a Timestamp now returns `Bad_ViewTimestampInvalid`. - Subscription transfer no longer fails when the optional `Server.GetMonitoredItems` is unavailable. After #4654 merges, `TransferSubscriptionToNewSessionAsync` can drop its expected differences for asyncua. Upstream defects found, all listed in the peers' `expected-differences.json` with evidence; issues are still to be filed: - **All five non-.NET stacks:** cannot handle the TestData `ScalarStructureDataType` (fields of the abstract Enumeration/Structure/BaseDataType). Milo encodes the abstract `Enumeration` field as a Variant (Part 6 5.2.4 says Int32). - **open62541:** - **Sign-only ECC keys:** derives encryption key and IV into the HKDF length for Sign-only ECC (Part 6 6.8.1), so Sign mode fails for the four 1.05 curves. The AEAD policies are not affected. - **HMAC mismatch:** reported as `Bad_InternalError`. - **ConditionRefresh:** needs the experimental A&C option. - **Response too large:** closes the connection instead of answering `Bad_ResponseTooLarge`. - **Array limit:** no MaxArrayLength. - **gopcua:** - **RsaPss:** `RSAOAEPMinPaddingSHA256` is 130 instead of 66, which breaks `Aes256_Sha256_RsaPss`. - **Nil LocalizedText:** encoding a nil `*LocalizedText` panics. - **asyncua server:** - **Queue overflow:** no Overflow bit. - **SetTriggering:** not implemented. - **Republish:** answers Good for an unknown sequence number or subscription. - **GetMonitoredItems:** not implemented. - **IndexRange:** ignored on Read and Write. - **Reconnect:** closes a session without subscriptions on transport loss. - **Limits:** no BrowseNext; operation limits not enforced; no `Bad_ResponseTooLarge`. - **async-opcua:** - **X509 user token (client):** treats an empty token SecurityPolicyUri as None. - **Reconnect (client):** overwrites the session nonce with the channel nonce on reconnect. - **ConditionRefresh (server):** not implemented. - **Overflow bit (server):** set without the DataValue InfoType bit. - **ECC:** no ECC policies. - **Milo:** - **Wrong password:** `BadIdentityTokenInvalid` instead of `BadUserAccessDenied` / `BadIdentityTokenRejected`. - **Array limit:** accepts arrays above its advertised MaxArrayLength. - **AddNodes:** answers `BadNotSupported`. ## Related Issues - Follows #4630. Related fixes: #4654. ## Types of changes - [x] Test / CI only ## Checklist - [x] Interop fixtures pass locally against all seven peers (net10.0, Windows). - [x] `dotnet format whitespace --verify-no-changes` on the changed C# files. - [ ] `interop-foreign.yml` has not run on GitHub yet; it needs a manual dispatch or the `interop` label. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
3bb0cd8d55 |
Fix ComplexTypes interop and TCP channel dispose CI flakes (#4663)
# Description
Fixes two intermittent CI failures on master.
## Interop: `LegacyClientExtendedCheckAsync("ComplexTypes")` /
`LoadAndDecodeLegacyComplexTypesAsync`
Flaky since #4630. The node reported as "not decoded" is always
`Data_*_UserScalar_VariantValue` (ns=5;i=2340 on the 2.0 reference
server, ns=3;i=3104 on the 1.5 one). Its DataType is the
BaseDataType-derived `VariantDataType`, and `TestDataSystem` fills it
with `DataGenerator.GetRandomVariant()`. About 2.4 % of the time that
draw is an `ExtensionObject` with a random TypeId (e.g. `s=香蕉`) that no
client can decode. Both checks counted every ExtensionObject of a
variable with a non-ns0 DataType as a structure.
**Fix:** both checks now count only variables whose DataType derives
from `Structure`. They browse the `HasSubtype` closure of `Structure` to
decide this.
- In the 2.0 test, the browse-with-continuation loop moved into a shared
`BrowseAsync` helper.
- In the 1.5 peer, the existing Structure-subtype walk in
`DiagnoseEncodingNodesAsync` moved into `BrowseStructureSubtypesAsync`,
which both places now use.
Seen in master CI for
|
||
|
romanett
|
fbdd0af13b |
Add 1.5.378 <-> 2.0 interop tests; return BadAttributeIdInvalid for unset optional security attributes (#4630)
# Description
Adds an interop test suite between this stack (2.0) and the released
**1.5.378** NuGet stack, in both directions, and fixes a 2.0 bug the
suite found.
## Interop suite
- **`tests/Opc.Ua.Interop.LegacyPeer`** is a console peer built from the
`OPCFoundation.NetStandard.Opc.Ua.*` 1.5.378 NuGet packages.
- It runs as a child process, because the two stacks share assembly
names.
- It deliberately does not import the repository build settings (its own
`Directory.Build.props/targets/Packages.props`).
- Server mode hosts a small interop server with tight limits, or the 1.5
Quickstarts reference server.
- Client mode runs named checks and prints one JSON result line per
check.
- **`tests/Opc.Ua.Interop.Tests`** builds the peer into `legacy-peer/`
and runs in every CI test leg as a normal `*.Tests.csproj`. It covers:
- sessions over None, the RSA policies and the ECC policies (Sign and
SignAndEncrypt), with anonymous and user name logon, plus
read/write/browse/call/subscribe;
- the existing 2.0 `ClientTest` workers run against the 1.5 reference
server, through the new `ClientTestFramework.ExternalServerUrl`;
- custom data types (structures, unions, optional fields, enums) loaded,
decoded and written back by each side's complex type system;
- certificate trust with auto-accept off: untrusted peers, CA-issued
certificates, revoked certificates;
- multi-chunk messages, encoding limits, response size limits, operation
limits, continuation points and security token renewal (RSA and ECC).
- Every peer check is reported as its own test result.
- `-p:LegacyStackVersion=<version>` tests another 1.5 release.
- Docs: `docs/DeveloperGuide.md`.
## 2.0 fix
**Unset optional security attributes now return
`Bad_AttributeIdInvalid`** (`NodeState.ReadNonValueAttribute`).
- Since
|