mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-16 21:02:51 +08:00
Marc Schier
e78d36ce4b
# Description Layer 2 of the upstream non-UaLens extraction, with core/types prerequisite #4801 (`marcschier-extract-core-and-types`). The initial extraction commit `86ff10d3994f132f7343a929abdae209bc2e69b7` started from `8881e096fb5e4eaa36f3c13f8caa57733373bd19` and copied exactly 13 client/server paths from #4462 at frozen head `390c4a7a51b71da40caec2d22ef74518bbe670ce`, using its original base `5b03e9afa492cdb36049ee52ae844f630f89d8db`. Those initial blobs were source-identical; the core/types layer was not duplicated or rewritten. - Add bounded temporary-file copying with byte/request limits, session ownership checks, and independent bounded cleanup. - Refresh revised monitored-item parameters after snapshot transfer and notify the target handler after successful handle synchronization. - Apply WSS opening deadlines while preserving caller cancellation, and cover the wire envelope and request timeout behavior. - Make hosted server presets honor role configuration without replacing an explicitly registered role manager; include the corresponding tests and documentation. **Review follow-up:** commit `a5c8fe0884ad3346106529deba71d93c5df20a9a` intentionally corrects three defects in the extracted bounded-copy implementation. It captures one `SessionRequestContext` for managed transfers, calls the standard `FileType.Close` declaration without metadata discovery, and retains valid generation handles before rejecting non-Good status or malformed metadata. Read/Close remain strictly validated, and transfer/cleanup failures remain independently visible. The follow-up also adds 21 direct-client regression cases and updates `docs/FileSystemClient.md`, bringing this PR to 15 changed paths. The current bounded-copy implementation is therefore intentionally no longer identical to the frozen source. No UaLens projects/UI, AMB or Machinery implementations, samples, CI/native renderer changes, release changes, or generic infrastructure files are included. The follow-up does not merge/rebase a lower or upper stack layer. **Current review-fix validation:** normal analyzers and warnings-as-errors remained enabled, using the existing SDK **10.0.401** without changing `global.json`, installing an SDK, or adding suppressions. The final file-system and session-context selection passed **241 tests on net10.0 and 241 tests on net48**, with **0 failed and 0 skipped** on each. Both selections include all 21 new regressions. Before the production fix, the first 20-case regression suite had 13 failures and 7 passes, reproducing the lost cleanup, cancellation, session-recreation and Close-discovery defects. An additional deterministic recovery-gate race case was then added and passed on both frameworks. Informational-severity formatting/style/analyzer checks passed for the changed C# files. | Review requirement | Direct regression evidence | | --- | --- | | Pin the managed generation through copying and cleanup | `CopyRejectsRecreatedManagedGenerationAsync`, `CopyRejectsRecoveryBetweenGenerationAndReadDispatchAsync` | | Preserve a generation response that returns after cancellation | `ManagedCopyClosesHandleReturnedAfterCancellationAsync` with immediate and delayed responses through a real managed transport lease | | Do not spend the Close deadline discovering metadata | `CopyUsesDeclarationCloseWithoutDiscoveryAsync` for ordinary and managed sessions | | Retain valid handles before rejecting generation status/metadata | `CopyClosesHandleBeforeRejectingGenerationStatusAsync`, `CopyClosesHandleFromMalformedGenerationResponseAsync` | | Preserve error semantics and strict Read/Close validation | `CopyRejectsGenerationWithoutInventingHandleAsync`, `CopyPreservesGenerationAndCleanupFailuresAsync`, `CopyKeepsReadAndCloseStatusValidationAsync` | <details> <summary>Exact review-fix validation commands and results</summary> Commands ran from this layer's repository root. The following variables expand the actual executable, filters and results paths. Console logs and TRX reports were retained in the session artifacts. ```powershell $dotnet = 'D:\git\UA-.NETStandard_master\TestResults\ualens-528-deep-repro\sdk\dotnet.exe' $env:DOTNET_ROOT = 'D:\git\UA-.NETStandard_master\TestResults\ualens-528-deep-repro\sdk' $env:DOTNET_ROOT_X64 = $env:DOTNET_ROOT $env:DOTNET_CLI_TELEMETRY_OPTOUT = '1' $client = 'tests\Opc.Ua.Client.Tests\Opc.Ua.Client.Tests.csproj' $filter = 'FullyQualifiedName~Opc.Ua.Client.Tests.FileSystem|FullyQualifiedName~Opc.Ua.Client.Tests.SessionContexts' $r10 = 'C:\Users\mschier\.copilot\session-state\9b51b89f-b5ed-4ed7-a097-cca67efe7b0e\files\feedback-net10' $r48 = 'C:\Users\mschier\.copilot\session-state\9b51b89f-b5ed-4ed7-a097-cca67efe7b0e\files\feedback-net48' # Before modifying production code, after the new tests compiled: & $dotnet test $client --no-restore -c Release -f net10.0 -p:CustomTestTarget=net10.0 --filter 'FullyQualifiedName~TemporaryFileTransferClientBoundedReadTests' --logger 'trx;LogFileName=bounded-copy-feedback-red.trx' --results-directory $r10 --verbosity quiet # Expected regression failure: Passed: 7, Failed: 13, Skipped: 0. # After the production fix, before adding the explicit queued-recovery case: & $dotnet test $client --no-restore -c Release -f net10.0 -p:CustomTestTarget=net10.0 --filter 'FullyQualifiedName~TemporaryFileTransferClientBoundedReadTests' --logger 'trx;LogFileName=bounded-copy-feedback-green.trx' --results-directory $r10 --verbosity quiet # Passed: 20, Failed: 0, Skipped: 0. # Final complete affected selections, including all 21 new cases: & $dotnet test $client --no-restore -c Release -f net48 -p:CustomTestTarget=net48 --filter $filter --logger 'trx;LogFileName=file-system-context-feedback-final.trx' --results-directory $r48 --verbosity quiet # Passed: 241, Failed: 0, Skipped: 0. & $dotnet test $client -c Release -f net10.0 -p:CustomTestTarget=net10.0 --filter $filter --logger 'trx;LogFileName=file-system-context-feedback-final.trx' --results-directory $r10 --verbosity quiet # Passed: 241, Failed: 0, Skipped: 0. $sourceFile = 'D:\git\copilot-worktrees\UA-wot-d3-final\marcschier-improved-invention\src\Opc.Ua.Client\FileSystem\TemporaryFileTransferClient.BoundedRead.cs' $testFile = 'D:\git\copilot-worktrees\UA-wot-d3-final\marcschier-improved-invention\tests\Opc.Ua.Client.Tests\FileSystem\TemporaryFileTransferClientBoundedReadTests.cs' $env:CustomTestTarget = 'net48' & $dotnet format 'src\Opc.Ua.Client\Opc.Ua.Client.csproj' --include $sourceFile --no-restore --verify-no-changes --severity info --verbosity quiet # Passed, exit 0. $env:CustomTestTarget = 'net10.0' & $dotnet format $client --include $testFile --no-restore --verify-no-changes --severity info --verbosity quiet # Passed, exit 0. git diff --check # Passed. CRLF, final newlines and the 120-column C# limit were also checked. ``` Initial no-restore test attempts after switching frameworks exited without executing tests and were not counted as passes. Diagnostic `dotnet build ... --no-restore` runs reported `NETSDK1005` for the missing framework assets; normal restore was then allowed. Test-authoring compile errors were corrected before recording the red run, without suppressions. Intermediate broader selections passed 240/240 on each framework before the queued-recovery case increased the final count to 241. </details> **Initial-extraction validation (commit `86ff10d3994f132f7343a929abdae209bc2e69b7`):** these results are historical, not a claim that the broader initial selections were rerun after the review fix. The server test assembly was built as a dependency of the client project before its `--no-build` runs. | Initial targeted scope | net10.0 | net48 | | --- | --- | --- | | Client subscriptions and file system | Passed: 546, Failed: 0, Skipped: 0 | Passed: 542, Failed: 0, Skipped: 0 | | Hosted roles, identity, composition, fluent APIs and builder coverage | Passed: 164, Failed: 0, Skipped: 0 | Passed: 164, Failed: 0, Skipped: 0 | | WebAPI WSS transport | Passed: 37, Failed: 0, Skipped: 0 | Not supported: the repository excludes this .NET 8+ test project on net48 | | V2 transfer and snapshot integration | Passed: 6, Failed: 0, Skipped: 0 | Passed: 6, Failed: 0, Skipped: 0 | The first broader net48 hosting run had **9 startup timeouts and certificate leaks** (155 passed). A single startup case reproduced the failure. With only a shorter `NUnit.WorkDirectory`, the isolated case and then all 164 hosting cases passed, with certificate leak tracking enabled and no teardown leaks. This is consistent with the fixture's documented .NET Framework PKI path-length constraint; no source or diagnostic suppression was changed. Both failed runs and successful reruns are recorded below. **Deferred coverage:** the broader domain-level `MachineryResultTransferTests*.cs` fixtures remain in Machinery layer #4804. They were not imported or run here. This review follow-up adds direct-client coverage for the three defects instead. The Machinery owner has been informed that the old expectations for failed Close discovery and zero cleanup after a returned non-Good generation response must be reconciled when the lower fixes are propagated. **Not run:** complete `UA.slnx` suites on net48/net10.0, a coverage measurement, CI/CodeQL completion, native/fuzz qualifications, or package publishing. This remains a draft pending full-stack integration and CI evidence. <details> <summary>Initial-extraction validation command arguments and outcomes</summary> All commands ran from this layer's repository root. Variables below expand the executable, project paths, filters and results directories used by the actual commands; test output was also retained in session-local log files. ```powershell $dotnet = 'D:\git\UA-.NETStandard_master\TestResults\ualens-528-deep-repro\sdk\dotnet.exe' $env:DOTNET_ROOT = 'D:\git\UA-.NETStandard_master\TestResults\ualens-528-deep-repro\sdk' $env:DOTNET_ROOT_X64 = $env:DOTNET_ROOT $env:DOTNET_CLI_TELEMETRY_OPTOUT = '1' $r10 = 'C:\Users\mschier\.copilot\session-state\9b51b89f-b5ed-4ed7-a097-cca67efe7b0e\files\validation-net10' $r48 = 'C:\Users\mschier\.copilot\session-state\9b51b89f-b5ed-4ed7-a097-cca67efe7b0e\files\validation-net48' $client = 'tests\Opc.Ua.Client.Tests\Opc.Ua.Client.Tests.csproj' $server = 'tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj' $webapi = 'tests\Opc.Ua.Bindings.Https.WebApi.Tests\Opc.Ua.Bindings.Https.WebApi.Tests.csproj' $subscriptions = 'tests\Opc.Ua.Subscriptions.Tests\Opc.Ua.Subscriptions.Tests.csproj' $clientFilter = 'FullyQualifiedName~Opc.Ua.Client.Subscriptions|FullyQualifiedName~Opc.Ua.Client.Tests.FileSystem' $serverFilter = 'FullyQualifiedName~HostedRolePresetTests|FullyQualifiedName~IdentityHostingTests|FullyQualifiedName~ServerFluentApiHostingTests|FullyQualifiedName~ServerCompositionHostingTests|FullyQualifiedName~OpcUaServerBuilderExtensionsCoverageTests' $subscriptionFilter = 'FullyQualifiedName~Opc.Ua.Subscriptions.Tests.TransferSubscriptionTests|FullyQualifiedName~Opc.Ua.Subscriptions.Tests.SubscriptionSnapshotTests' & $dotnet --version # Passed: 10.0.401. & $dotnet test $client --no-restore -c Release -f net10.0 -p:CustomTestTarget=net10.0 --filter $clientFilter --logger 'trx;LogFileName=client-net10.trx' --results-directory $r10 --verbosity quiet # Exit 0 but no tests executed: missing restore metadata. NOT counted as validation. & $dotnet build $client --no-restore -c Release -f net10.0 -p:CustomTestTarget=net10.0 --verbosity quiet # Failed: NETSDK1004, missing obj\project.assets.json. Normal restore was then allowed in test commands. & $dotnet test $client -c Release -f net10.0 -p:CustomTestTarget=net10.0 --filter $clientFilter --logger 'trx;LogFileName=client-net10.trx' --results-directory $r10 --verbosity quiet # Passed: 546, Failed: 0, Skipped: 0. & $dotnet test $server --no-build --no-restore -c Release -f net10.0 -p:CustomTestTarget=net10.0 --filter $serverFilter --logger 'trx;LogFileName=server-net10.trx' --results-directory $r10 --verbosity quiet # Passed: 164, Failed: 0, Skipped: 0. & $dotnet test $webapi -c Release -f net10.0 -p:CustomTestTarget=net10.0 --filter 'FullyQualifiedName~WebApiWssTransportChannelTests' --logger 'trx;LogFileName=webapi-net10.trx' --results-directory $r10 --verbosity quiet # Passed: 37, Failed: 0, Skipped: 0. & $dotnet test $client -c Release -f net48 -p:CustomTestTarget=net48 --filter $clientFilter --logger 'trx;LogFileName=client-net48.trx' --results-directory $r48 --verbosity quiet # Passed: 542, Failed: 0, Skipped: 0. & $dotnet test $server --no-build --no-restore -c Release -f net48 -p:CustomTestTarget=net48 --filter $serverFilter --logger 'trx;LogFileName=server-net48.trx' --results-directory $r48 --verbosity quiet # Failed: Passed: 155, Failed: 9, Skipped: 0; startup timeouts and 9 undisposed certificates at teardown. & $dotnet test $subscriptions -c Release -f net48 -p:CustomTestTarget=net48 --filter $subscriptionFilter --logger 'trx;LogFileName=subscriptions-net48.trx' --results-directory $r48 --verbosity quiet # Passed: 6, Failed: 0, Skipped: 0. & $dotnet test $subscriptions -c Release -f net10.0 -p:CustomTestTarget=net10.0 --filter $subscriptionFilter --logger 'trx;LogFileName=subscriptions-net10.trx' --results-directory $r10 --verbosity quiet # Passed: 6, Failed: 0, Skipped: 0. & $dotnet msbuild $webapi -p:CustomTestTarget=net48 -p:TargetFramework=net48 -getProperty:TargetFramework,RestrictForLegacyTfm,_RestrictedToLegacyTfm,IsTestProject,EnableDefaultCompileItems # Passed: net48 / true / true / false / false. Confirms the net48 project is an empty non-test shell. # Each following diagnostic invocation had certificate allocation tracking enabled. $env:OPCUA_CERTIFICATE_LEAK_TRACKING = '1' & $dotnet test $server --no-build --no-restore -c Release -f net48 -p:CustomTestTarget=net48 --filter 'FullyQualifiedName~HostedRolePresetTests' --logger 'trx;LogFileName=hosted-role-net48.trx' --results-directory $r48 --verbosity quiet # Passed: 10, Failed: 0, Skipped: 0. & $dotnet test $server --no-build --no-restore -c Release -f net48 -p:CustomTestTarget=net48 --filter 'FullyQualifiedName~ServerFluentApiHostingTests.AddServerWithCustomServerStartsCustomServerAsync' --logger 'trx;LogFileName=hosting-startup-net48.trx' --results-directory $r48 --verbosity quiet # Failed: Passed: 0, Failed: 1, Skipped: 0; 30-second startup timeout and one certificate leak. # A fresh, dedicated short working directory was created for the following two runs. & $dotnet test $server --no-build --no-restore -c Release -f net48 -p:CustomTestTarget=net48 --filter 'FullyQualifiedName~ServerFluentApiHostingTests.AddServerWithCustomServerStartsCustomServerAsync' --logger 'trx;LogFileName=hosting-startup-net48-shortpath.trx' --results-directory $r48 --verbosity quiet -- 'NUnit.WorkDirectory=D:\Temp\opc-l2-6caf4556' # Passed: 1, Failed: 0, Skipped: 0; no certificate leak. & $dotnet test $server --no-build --no-restore -c Release -f net48 -p:CustomTestTarget=net48 --filter $serverFilter --logger 'trx;LogFileName=server-net48-shortpath.trx' --results-directory $r48 --verbosity quiet -- 'NUnit.WorkDirectory=D:\Temp\opc-l2-6caf4556' # Passed: 164, Failed: 0, Skipped: 0; no certificate leak or assembly-level teardown error. # The dedicated short working directory and its generated PKI contents were removed afterward. ``` TRX inspection confirmed all initially added client (3), WSS (4), and hosted-role (10) regression cases executed on supported frameworks. The initial 13-path set and source blobs were verified before the extraction commit. A separate mistyped SDK availability probe (`uals-528-deep-repro` instead of `ualens-528-deep-repro`) failed before launching any process; all actual builds/tests used the correct verified SDK above. </details> ## Related Issues Related to #4461; extracted from source PR #4462. Neither is closed by this PR. Prerequisite: #4801, the core/types layer. _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] 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. - [x] I have addressed **all** PR feedback received.