Commit Graph

3 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>
2026-10-08 08:48:13 +02:00
Marc Schier f9892020a3 Add configurable server resource isolation (#4541)
# Description

Add configurable server resource isolation on top of the merged #4522
foundation.

- Coordinate connection, handshake, retained-message,
session-establishment, queue, execution, request-cost and parked-request
limits through one server-scoped provider.
- Keep configured startup, continuity/reconnect, control and
trusted-owner reservations inside existing totals. Shared capacity
remains usable up to explicit owner ceilings.
- Use authoritative Session membership for reassembly continuity
capacity without granting unrelated resource or rate privileges.
Preserve explicit owner mappings and bounded tracking.
- Cache immutable classification evidence while retaining current
Session and classifier checks.
- Support weighted request ordering or FIFO independently of resource
accounting. Installed providers enforce decoded-request limits in both
modes.
- Preserve fail-fast reassembly, valid Abort handling, fixed deadlines,
active-work protection, reverse-connect handoff and bounded OpenAPI
processing.
- Support custom-handler parking through `IRequestParkingPolicy`,
`WithRequestParking(...)`, and `RequestLifetime.ParkSink`. Parking
releases execution capacity only, keeping retained-data and parked
charges until completion. Built-in Publish and the global opt-out remain
unchanged.
- Provide direct, fluent and DI configuration, public incoming-request
parking support, effective startup limit logs and user-facing
documentation.

Balanced remains the default. SharedOnly without an explicit provider
retains shared behavior for intentionally incompatible/unlimited
configurations. Reservations are finite and are not one full-message
allowance per Session.

## Related Issues

Builds on merged #4522. Fixes #4554.

## Review and CI corrections

All three previously missed findings in [review
5326734718](https://github.com/OPCFoundation/UA-.NETStandard/pull/4541#pullrequestreview-5326734718)
are addressed in `9e25e4ecc`:

1. A custom FIFO provider receives RequestQueue, RequestQueueBytes,
RequestExecution and ParkedRequest accounting. A blocked head cannot be
overtaken.
2. `IResourceIsolationReassemblyProvider` replaces the sessionless
threshold independently of `UseFairScheduling`. The aggregate byte
ceiling remains enforced.
3. Runtime `ReservedReassemblyBytes` includes all configured protected
stage floors, including ControlReserved.

The review's two already-resolved changes remain intact.

The CI collection race was fixed at its source in `e1a994a4b`:
`ArrayOf.ToArrayOf` treats a separately read collection count as an
initial capacity hint and grows/trims for the actual enumeration.
Deterministic tests cover growth, shrinkage and stale-zero counts.

The subsequent recovery-test race was fixed in `05753ce77`: fake time
advances only after a gated keepalive response has been processed and
reset its timer, not merely after request arrival. Recovery-order and
timeout assertions remain intact, with additional response-state
assertions. Instrumentation reproduced the original failure twice; both
fixed variants passed 400 executions per framework.

CodeQL alert 388 was verified and dismissed as a false positive. Startup
logs contain aggregate numeric capacities, not credentials or caller
identities.

## Validation

| Scope | Result |
| --- | --- |
| Latest automated checks at `05753ce7775a58801b4ba9d8f6d7781b709d0b89`
| 167 successful, one package-publication check skipped, no failures |
| Remaining external prerequisite | `license/cla` pending: Contributor
License Agreement is not signed yet |
| FIFO/weighted queues, custom parking and server-channel selection |
175 passed on each net10.0/net48 |
| Isolation, configuration and parking-hosting selection | 259 passed
net10.0; 257 passed net48 |
| ArrayOf conversion tests | 73 passed on each net10.0/net48 |
| Recovery-audit and keepalive fixtures | 40 passed on each
net10.0/net48 |
| NativeAOT smoke from preceding updates | Published and executed
successfully |

No tests, assertions, coverage thresholds, or required checks were
weakened. The remaining CLA prerequisite must be completed through [CLA
Assistant](https://cla-assistant.io/OPCFoundation/UA-.NETStandard?pullRequest=4541).

This is not CTT certification or an unconditional network-DDoS,
whole-process-memory or fleet-wide guarantee.

## 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.
- [x] 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>
2026-09-27 21:11:52 +02:00
romanett b9657e5f0d Allow passing an existing config XML to the hosted server DI AddServer (#4325)
# Description

Extends the hosted-server dependency-injection surface so applications
that already own a classic `ApplicationConfiguration` XML file
(`*.Config.xml`) have a direct migration path to DI with **all** of
their existing settings applied.

New API surface:

- `AddServer(string configurationFile, Action<ApplicationConfiguration>?
configure = null)` and the `AddServer<TServer>` variant load the
configuration from the file instead of building it from
`OpcUaServerOptions`.
- `AddServer(Stream configurationStream, ...)` (plus generic variant)
for documents that are not files on disk, e.g. embedded resources. The
stream is read once and disposed by the hosted service during startup.
- `OpcUaServerOptions.ConfigurationFile` / `ConfigurationStream` back
the overloads, so the options-callback route works too and the file path
can be bound from configuration (`OpcUa:Server:ConfigurationFile`).
- `OpcUaServerOptions.ConfigureLoadedConfiguration` runs after the
document is loaded and validated but before certificates are checked, so
individual settings can be overridden from code without editing the
file.

Behavior:

- The document is loaded through
`ApplicationInstance.LoadApplicationConfigurationAsync`, so every
setting (base addresses, security policies, certificate stores,
transport quotas, operation limits, user token policies) applies exactly
as on the classic path, including localhost fix-up.
- The supplied document is authoritative: the builder-feeding options
(`ApplicationName`, `EndpointUrls`, `PkiRoot`, policy toggles,
`ConfigureBuilder`, ...) are not applied on this path, and it takes
precedence over a shared application registered with
`ConfigureApplication(...)`. Options acting on the hosted server itself
(`Identity`, `ConfigureRateLimits`) and all fluent registrations
(`AddNodeManager`, `ConfigureRoles`, identity authenticators, ...)
compose unchanged.
- DI-registered `ICertificateManager` / `ICertificatePasswordProvider`
are honored, the "listening at" log reports the loaded base addresses,
and the unmatched user-token-policy startup warning is derived from the
policies the document advertises.
- Setting both `ConfigurationFile` and `ConfigurationStream` fails
startup with a clear `InvalidOperationException`; a missing file fails
startup with the original `ServiceResultException`.

Coverage: new `ConfigurationFileHostingTests` fixture (12 tests) with
end-to-end hosted starts from file and stream asserting the loaded
settings, custom `TServer` types, the override callback, precedence over
`ConfigureApplication`, warning derivation from the document,
missing-file and file+stream startup failures, argument validation, the
duplicate-registration guard, and configuration-section binding.

Docs: new "Migrating with an existing configuration XML file" section in
`docs/DependencyInjection.md` (plus first-class options table rows), a
pointer in the 2.0 migration guide, and a NuGet README mention.

## Related Issues

None — additive convenience overloads on the existing hosted-server
dependency-injection surface; no behavior change for existing
registrations.

## Checklist

_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._

- [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.

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-29 10:48:24 +02:00