Commit Graph

4 Commits

Author SHA1 Message Date
romanett e639932b95 Fix source generator defects from the Client/Types/SourceGeneration audit (#4473)
# Description

Fixes the SourceGeneration findings of a read-only bug audit of
`a05a36055` — sections 15 (host generators, Roslyn side), 16
(SourceGeneration.Core schema / ModelDesign loader) and 17
(SourceGeneration.Core generators). 36 of the 39 findings are fixed;
each has a regression test.

### Generated-code correctness

- **State machine NodeId constants** were emitted as `const uint` even
when the aliased `Objects` constant is a `string`, so every model using
string NodeIds with a `FiniteStateMachineType` subtype failed with
CS0029. The constant type now follows the aliased node, looked up from
the same source `NodeIdGenerator` emits from — top-level nodes **plus
each node's instance hierarchy**, since a state or transition only ever
exists as a hierarchy entry.
- `NodeIdGenerator` emitted string identifiers into C# string literals
without escaping them.
- Structure field names are authored data, so they can be C# keywords or
collide with the members every generated data type carries. Properties
and `{Type}Fields` members now go through a sanitizing,
collision-breaking mapping (including two siblings that sanitize to the
same identifier). **Wire names are untouched** — OPC 10000-3 §8.51
requires the StructureField name to match the BrowseName, and the
`DataMember`/encode/decode names still come from `Parameter.Name`.
- `NodeStateGenerator`'s copy-on-write cast now uses the fully qualified
data type name, which a cross-model structure needs.
- `GetScalarValueAsCode` emitted `(double)NaN` and the infinities, which
do not compile.
- `TemplateWriter` formatted with the current culture, so a machine with
a comma decimal separator emitted invalid generated source.

### Served schema documents

- The binary schema described a **different wire layout than the
generated `Encode`/`Decode`**: unions carried no
`SwitchField`/`SwitchValue`, and structures with optional fields carried
no encoding-mask presence bits. Both are now emitted per **OPC 10000-6
§5.2.7**, matching the layout the runtime `BsdSchemaGenerator` produces.
A structure with more than 32 optional fields now fails generation
instead of silently wrapping `1 << 32` back onto the first field's bit.
- Documentation text and attribute values are XML-escaped.

### ModelDesign loading and validation

- Identifiers pinned in a CSV were dropped whenever any other loaded
namespace declared the same symbolic name. Removal is now scoped to the
file's target namespace, and explicit `NumericId`s are keyed by
`(namespace, id)` so the same number in two namespaces is legal.
- Declaration order inside a design file no longer matters — base types
are imported and validated ahead of their subtypes.
- OptionSet `OptionSetValues` were truncated at bit 31 (signed shift)
and bits 32–63 of a 64-bit option set were never emitted.
- An authored `ValueRank` is no longer overwritten by the shape of the
default value (OPC 10000-3 §6.2.8: a ValueRank may only be further
restricted), and a VariableType data type may be narrowed any number of
levels deep rather than exactly one.
- NodeSet import: `AccessRestrictions="0"` means "no restrictions", not
`EncryptionRequired` (OPC 10000-3 §8.56) — an unrecognised mask stays
**fail-closed**; a NodeSet without `<Models>` reports what is wrong; an
optional `<References>` no longer throws; the parent/child reference
scan looks at the parent's references; and `IsTypeOf` terminates on the
self-referential root reference types NodeSets routinely declare.
- Version comparison is numeric per component (ordinally `"1.05.9"`
sorts above `"1.05.10"`) and now lives on `SemVer`, so the NodeSet
collection and the dependency tie-break cannot pick different winners; a
publication-date version no longer outranks a real one.

### Host generator

- `[DataType]` on an `enum` is matched again (an enum is not a
`TypeDeclarationSyntax`), MODELGEN003 renders its arguments instead of
the raw template, and the Types hint name keeps the namespace's dots so
`A.BC` and `AB.C` cannot claim the same one.
- Compilation-derived pipeline stages carry value comparers, so editing
a file no longer re-runs the entire model generation on every keystroke.
- A NodeSet that fails to load skips that file rather than every
remaining one; a file listed twice as an `AdditionalFile` is deduped;
superseded NodeSet versions are still recognised as NodeSet inputs and
not generated a second time by the ModelDesign pass.
- `LogLevel.Critical` maps to a diagnostic instead of being silently
dropped.

### Client

- `EventRecordFieldReaders` gained the readers the event record
generator needs (including `Variant`, enumerations and the array forms).
Fields whose type had no reader were declared on the record but never
populated, so they read as `null`/`default` for every event.

## Not fixed (deliberately)

- **Forward same-namespace hierarchical references in
`NodeSetToModelDesign.ImportReferences`** — the audit flags the
inverse-only handling as asymmetric. Making it symmetric compiles and
passes every generator unit test, but regresses the address space: all
17 `RobotOpenUsdE2eTests` fail in `OneTimeSetUpAsync` because the hosted
server never finishes starting. Reverted, with a comment at the site
recording why.
- **§17.10 and §15.10** (both low + PLAUSIBLE) — each needs a wider
refactor than this PR: a consistent NodeId-constant rename across ~6
cross-reference sites, and replacing `Location` with a serializable
equivalent for incremental cache equality.

## Spec cross-check

The encoding/schema changes were checked against the specification: OPC
10000-6 §5.2.7 (32-bit EncodingMask, one bit per optional field in
order, padded), the union `SwitchField` encoding, OPC 10000-3 §8.56
(AccessRestrictionType bit mask), §6.2.8 and §5.6.2/§5.6.5 (ValueRank),
§8.51 (StructureField name == BrowseName).
`GenerateStackTests.OptionalFieldBitsAgreeBetweenBsdAndGeneratedEncoder`
pins the BSD presence-bit order against the generated encoder's
`LogRecordFields` so the two cannot drift apart again.

## Related Issues

_No tracking issue — this PR implements the SourceGeneration sections of
a bug audit. Happy to open one if maintainers prefer._

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [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.

> Note on the test checkbox: verified on **net10.0 only** — `UA.slnx`
builds clean (0 warnings, 0 errors) and
`Opc.Ua.SourceGeneration.Core.Tests` (3915),
`Opc.Ua.SourceGeneration.Tests` (184), `Opc.Ua.Di.Tests` (425, including
the `RobotOpenUsdE2eTests` end-to-end fixture that catches address-space
content changes) and the `EventRecordFieldReaders` tests (16) all pass.
.NET Framework has **not** been run locally; CI needs to confirm it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
2026-09-13 19:07:55 +02:00
Marc Schier b97fd8d4b5 Fix OPC 10000-21 onboarding interoperability (#4385)
# Description

Fix the four blockers that prevented the OPC 10000-21 onboarding
registrar and client from working end to end:

- generate every `ModelDesign` target independently, including multiple
models in one directory, while preserving stable identifier CSV matching
and cross-model dependencies;
- ship the generated `Opc.Ua.Onboarding` constants, node states,
encodeables, and typed clients from `Opc.Ua.Gds.Common`;
- correct the standard `EncodedTicket` datatype to derive from
`ByteString`;
- align the new 2.0 `OnboardingClient` API with the generated
`ArrayOf<ByteString>` / `ArrayOf<StatusCode>` contract;
- make `BindToTicketStore` fully asynchronous, cancellation-aware, and
assign the declared output slot instead of appending a second output;
- add generated-model, client fallback, server binding, and in-process
end-to-end regressions.

The change also adds `samples/Gds/OnboardingRegistrar`,
`samples/Gds/OnboardingClient`, and `run-onboarding-demo.ps1`. The demo
loads the generated GDS and Onboarding models, restricts ticket
administration to an ephemeral `RegistrarAdmin` user, proves anonymous
access is denied, and completes a secure register/unregister round trip.

The History CI follow-up rejects negative and non-finite
processed-history intervals before provider dispatch and enforces the
buffered aggregate output cap while calculator results are flushed. This
prevents malformed requests from running until the client timeout,
exhausting memory, delaying server teardown, and causing later
alarm-event tests to fail.

## Related Issues

- Fixes #4378
- Unblocks OPCFoundation/UA-.NETStandard-Samples#819
- Tracks the canonical model correction in OPCFoundation/UA-Nodeset#122

## Validation

- Full `net10.0` test projects:
  - `Opc.Ua.SourceGeneration.Core.Tests`: 3803 passed, 8 skipped
  - `Opc.Ua.SourceGeneration.Tests`: 143 passed
  - `Opc.Ua.Gds.Tests`: 1114 passed, 42 skipped
- `Opc.Ua.History.Tests` with the CI coverage settings: 506 passed, 27
skipped
- `HistorianDispatcher` tests: 88 passed on `net10.0`.
- Invalid processed-history interval regressions passed on `net10.0` and
`net48`.
- Focused onboarding/source-generation regressions passed on `net48`.
- `dotnet build UA.slnx -p:CustomTestTarget=net10.0 --no-restore`
succeeded; the reported analyzer warnings are pre-existing in unchanged
XRegistry/WoT files.
- Packed all three GDS packages and verified the packaged
`Opc.Ua.Gds.Common` assembly contains the generated Onboarding model
surface.
- `pwsh samples/Gds/run-onboarding-demo.ps1` verified anonymous denial
and the authenticated secure ticket lifecycle.

## 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.
- [ ] 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>
Copilot-Session: 2d2c52c7-7040-4bd6-98de-aaf82ba7846b
2026-09-02 11:39:19 +02:00
Marc Schier 5b9bcedc62 Add OPC UA ISA-95 Common Model and Job Control support (#4089)
# Description

Adds first-class support for the OPC UA ISA-95 companion specifications:

- OPC-10030 Common Model.
- OPC-10031-4 Job Control V1.
- OPC-10031-4 Job Control V2.

The change introduces `Opc.Ua.ISA95`, `Opc.Ua.ISA95.Client`, and
`Opc.Ua.ISA95.Server`, together with a NativeAOT-compatible minimal
server sample and comprehensive tests.

## Highlights

- Compiles the three canonical NodeSets and identifier mapping CSV files
through the source generator.
- Adds NodeSet identifier mapping validation, typed method
arguments/results, companion event records, and generated method
argument nodes.
- Provides typed Common Model builders with normative relationships and
an injectable `GeoSpatialLocationType` provider.
- Implements lossless V1/V2 Job Control providers over a shared
version-neutral store, including state paths, status events, comments,
response queries, catalog changes, and cohesive DI registration.
- Adds subtype-aware typed clients, standalone encodeable registration,
`IIsa95ClientFactory`, and V2 status-event streaming.
- Adds the `MinimalIsa95Server` sample, package documentation, and an
ISA-95 guide with conformance and HA guidance.
- Tracks typed RSL/GPOS integration as follow-up issue #4095.

## Validation

- `Opc.Ua.ISA95.Tests`: 137/137 passed on `net10.0` and `net48`.
- Source-generator suites: 81/81 model, 90/90 stack, 3718 passed / 8
skipped core.
- `EventRecordFieldReaders` feedback tests: 6/6 on `net10.0` and
`net48`.
- `EventRecordFieldReaders` changed-method patch coverage: 100% line
coverage (36/36) and 100% branch coverage (22/22).
- `Opc.Ua.ISA95`, `.Client`, and `.Server` build across all configured
target frameworks without new diagnostics.
- Cobertura line coverage: 88.08% model, 90.04% client, 91.19% server.
- NativeAOT sample publishes and passes a live typed V2
Store/status-event validation.
- The ISA-95 NativeAOT smoke test passes in the source-generated native
test executable.
- OPC UA KB NodeSet validation reports zero errors for Common, V1, and
V2.

The broader NativeAOT executable still reports unrelated existing GDS
and complex-type test failures; the ISA-95-specific native test passes.

## Node state generator fixes

Generating method arguments inline for imported nodesets regressed every
model that already emits `InputArguments` / `OutputArguments` child node
factories:

- The inline block ran before the child factories.
`MethodState.CreateOrReplaceInputArguments` keeps the property that was
assigned first, so the child carrying the well-known NodeId was dropped
and a NodeId-less placeholder remained in the address space. This broke
browsing (reference descriptions without a target) and made method
arguments unreachable at their well-known NodeIds.
- The `MethodDeclarationId` fallback keyed on an autogenerated display
name redirected every declared method of the standard address space to
its own NodeId, which made method calls fail.

The inline block is now emitted after the child factories and reduced to
a value assignment when a child factory exists. The value is only
re-emitted when an argument references a namespace other than OPC UA,
because the value of an imported property is the verbatim XML of the
source nodeset whose namespace indices do not survive the import. The
`MethodDeclarationId` fallback is restricted to declarations without a
resolvable NodeId. The generated OPC UA address space is byte-identical
to `master` again.

## Related Issues

- Fixes #4088
- Follow-up: #4095
2026-07-28 12:01:57 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00