Some .NET 9 maintenance and JSON encoder improvements #2922

mregen · 2025-01-03T10:03:24Z

Proposed changes

This pull request includes various updates and improvements across multiple files. The changes focus on updating package references, enhancing debugging capabilities, and improving code efficiency and readability.

Reduce memory allocations in JSON encoder by using stackalloc/ArrayPool
Fix some new warnings in .NET9 code analyzers
Fix fuzzing error in JsonDecoder
do not use .NET9 libraries with .NET8 LTS builds
Bump langversion to 9 (max VS2019)
Use official ASP.Net 2.3. libraries for .NET Framework (rel. Jan 15th)

Fixes #2940
Fixes #2931

Package Reference Updates:

Updated NUnit to version 4.3.2 and NUnit.Console to version 3.19.0 in Fuzzing/Encoders/Fuzz.Tests/Opc.Ua.Encoders.Fuzz.Tests.csproj.
Updated System.Diagnostics.DiagnosticSource to version 6.0.2 in Stack/Opc.Ua.Core/Opc.Ua.Core.csproj.
Added conditional package references for System.Formats.Asn1 and Microsoft.Bcl.Cryptography in Libraries/Opc.Ua.Security.Certificates/Opc.Ua.Security.Certificates.csproj. [1] [2]

Debugging Enhancements:

Added #if DEBUG directive to validate publish time and reject old values in Session.cs.

Code Efficiency and Readability:

Simplified SaveMessageInCache method by removing redundant parameters and using object initializers in Subscription.cs. [1] [2]
Replaced string interpolation with ServiceResultException.Create for consistency in error messages in NodeId.cs. [1] [2] [3]

New Features:

Added MoreNotifications property to NotificationMessage and NotificationData classes to indicate if more notifications are expected. [1] [2]

Miscellaneous:

Removed unnecessary package reference conditions in Opc.Ua.Client.csproj.
Added support for stack allocation and array pooling in ReadString method for performance improvement in BinaryDecoder.cs.

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply. You can also fill these out after creating the PR.

Bugfix (non-breaking change which fixes an issue)
Enhancement (non-breaking change which adds functionality)
Test enhancement (non-breaking change to increase test coverage)
Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages)
Documentation Update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

I have read the CONTRIBUTING doc.
I have signed the CLA.
I ran tests locally with my changes, all passed.
I fixed all failing tests in the CI pipelines.
I fixed all introduced issues with CodeQL and LGTM.
I have added tests that prove my fix is effective or that my feature works and increased code coverage.
I have added necessary documentation (if appropriate).
Any dependent changes have been merged and published in downstream modules.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

codecov · 2025-01-03T10:03:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (a0b12d7) to head (d868a6d).
Report is 1 commits behind head on develop/main374.

Additional details and impacted files

@@                 Coverage Diff                 @@
##           develop/main374   #2922       +/-   ##
===================================================
- Coverage            54.89%       0   -54.90%     
===================================================
  Files                  349       0      -349     
  Lines                66377       0    -66377     
  Branches             13624       0    -13624     
===================================================
- Hits                 36437       0    -36437     
+ Misses               25957       0    -25957     
+ Partials              3983       0     -3983

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copilot reviewed 5 out of 12 changed files in this pull request and generated no comments.

Files not reviewed (7)

Libraries/Opc.Ua.Security.Certificates/Opc.Ua.Security.Certificates.csproj: Language not supported
targets.props: Language not supported
Libraries/Opc.Ua.Security.Certificates/X509Certificate/X509CertificateLoader.cs: Evaluated as low risk
Tests/Opc.Ua.Core.Tests/Types/Encoders/JsonEncoderEscapeStringBenchmarks.cs: Evaluated as low risk
Tests/Opc.Ua.Gds.Tests/Common.cs: Evaluated as low risk
Tests/Opc.Ua.Client.Tests/SubscriptionTest.cs: Evaluated as low risk
Tests/Opc.Ua.Core.Tests/Security/Certificates/ApplicationTestDataGenerator.cs: Evaluated as low risk

Comments suppressed due to low confidence (1)

Tests/Opc.Ua.Core.Tests/Types/Encoders/EncoderTests.cs:497

The assertion Assert.IsTrue(nullspan == null) is incorrect because ReadOnlySpan cannot be null.

Assert.IsTrue(nullspan == null);

reduce string binarydecoder memory allocations

Copilot reviewed 22 out of 35 changed files in this pull request and generated no comments.

Files not reviewed (13)

Fuzzing/Encoders/Fuzz.Tests/Opc.Ua.Encoders.Fuzz.Tests.csproj: Language not supported
Libraries/Opc.Ua.Client/Opc.Ua.Client.csproj: Language not supported
Libraries/Opc.Ua.Security.Certificates/Opc.Ua.Security.Certificates.csproj: Language not supported
Stack/Opc.Ua.Core/Opc.Ua.Core.csproj: Language not supported
Tests/Opc.Ua.Client.ComplexTypes.Tests/Opc.Ua.Client.ComplexTypes.Tests.csproj: Language not supported
Tests/Opc.Ua.Client.Tests/Opc.Ua.Client.Tests.csproj: Language not supported
Tests/Opc.Ua.Configuration.Tests/Opc.Ua.Configuration.Tests.csproj: Language not supported
Tests/Opc.Ua.Core.Tests/Opc.Ua.Core.Tests.csproj: Language not supported
Libraries/Opc.Ua.Client/Session/Session.cs: Evaluated as low risk
Libraries/Opc.Ua.Client/Subscription/Subscription.cs: Evaluated as low risk
Stack/Opc.Ua.Core/Types/Encoders/IEncoder.cs: Evaluated as low risk
Libraries/Opc.Ua.Security.Certificates/X509Certificate/X509CertificateLoader.cs: Evaluated as low risk
Stack/Opc.Ua.Core/Stack/Types/NotificationMessage.cs: Evaluated as low risk

Copilot

Copilot reviewed 21 out of 34 changed files in this pull request and generated 1 comment.

Files not reviewed (13)

Fuzzing/Encoders/Fuzz.Tests/Opc.Ua.Encoders.Fuzz.Tests.csproj: Language not supported
Libraries/Opc.Ua.Client/Opc.Ua.Client.csproj: Language not supported
Libraries/Opc.Ua.Security.Certificates/Opc.Ua.Security.Certificates.csproj: Language not supported
Stack/Opc.Ua.Core/Opc.Ua.Core.csproj: Language not supported
Tests/Opc.Ua.Client.ComplexTypes.Tests/Opc.Ua.Client.ComplexTypes.Tests.csproj: Language not supported
Tests/Opc.Ua.Client.Tests/Opc.Ua.Client.Tests.csproj: Language not supported
Tests/Opc.Ua.Configuration.Tests/Opc.Ua.Configuration.Tests.csproj: Language not supported
Tests/Opc.Ua.Core.Tests/Opc.Ua.Core.Tests.csproj: Language not supported
Tests/Opc.Ua.Client.Tests/SubscriptionTest.cs: Evaluated as low risk
Tests/Opc.Ua.Core.Tests/Security/Certificates/ApplicationTestDataGenerator.cs: Evaluated as low risk
Libraries/Opc.Ua.Client/Session/Session.cs: Evaluated as low risk
Libraries/Opc.Ua.Client/Subscription/Subscription.cs: Evaluated as low risk
Stack/Opc.Ua.Core/Types/Encoders/IEncoder.cs: Evaluated as low risk

Comments suppressed due to low confidence (8)

Stack/Opc.Ua.Core/Types/Encoders/JsonEncoder.cs:705

The condition should be 'if (!string.IsNullOrEmpty(fieldName))' to correctly handle null fields.

if (string.IsNullOrEmpty(fieldName))

Stack/Opc.Ua.Core/Types/Encoders/BinaryDecoder.cs:2549

The error message string formatting should use ServiceResultException.Create for consistency.

throw ServiceResultException.Create(StatusCodes.BadDecodingError, "Reading {0} bytes of {1} reached end of stream after {2} bytes.", length, functionName, bytes.Length);

Stack/Opc.Ua.Core/Types/Encoders/BinaryDecoder.cs:2531

The error message string formatting should use ServiceResultException.Create for consistency.

throw ServiceResultException.Create(StatusCodes.BadDecodingError, "Reading {0} bytes of {1} reached end of stream after {2} bytes.", length, functionName, bytes.Length);

Stack/Opc.Ua.Core/Types/BuiltIn/NodeId.cs:315

[nitpick] The error message string should be clear and consistent. Consider rephrasing to 'Invalid NodeId: {0}'.

throw ServiceResultException.Create(StatusCodes.BadNodeIdInvalid, "Invalid NodeId ({0}).", originalText);

Stack/Opc.Ua.Core/Types/BuiltIn/NodeId.cs:323

[nitpick] The error message string should be clear and consistent. Consider rephrasing to 'No mapping to NamespaceIndex for NamespaceUri: {0}'.

throw ServiceResultException.Create(StatusCodes.BadNodeIdInvalid, "No mapping to NamespaceIndex for NamespaceUri ({0}).", namespaceUri);

Stack/Opc.Ua.Core/Types/BuiltIn/NodeId.cs:335

[nitpick] The error message string should be clear and consistent. Consider rephrasing to 'Invalid ExpandedNodeId: {0}'.

throw ServiceResultException.Create(StatusCodes.BadNodeIdInvalid, "Invalid ExpandedNodeId ({0}).", originalText);

Stack/Opc.Ua.Core/Types/BuiltIn/NodeId.cs:405

[nitpick] The error message string should be clear and consistent. Consider rephrasing to 'Invalid NodeId Identifier: {0}'.

throw ServiceResultException.Create(StatusCodes.BadNodeIdInvalid, "Invalid NodeId Identifier ({0}).", originalText);

Stack/Opc.Ua.Core/Types/Encoders/JsonEncoder.cs

romanett

Looks good to me

mregen · 2025-01-20T05:30:39Z

Thanks @romanett

…TStandard into net9updates

romanett · 2025-01-20T06:32:53Z

common.props

@@ -15,7 +16,7 @@
    <PlatformTarget>AnyCPU</PlatformTarget>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
    <!-- Highest language version supported by .NET Standard 2.0 and .NET Framework -->
-    <LangVersion>7.3</LangVersion>
+    <LangVersion>12</LangVersion>


looks like VS2019 only supports 9

Copilot reviewed 38 out of 51 changed files in this pull request and generated no comments.

Files not reviewed (13)

Fuzzing/Encoders/Fuzz.Tests/Opc.Ua.Encoders.Fuzz.Tests.csproj: Language not supported
Libraries/Opc.Ua.Client.ComplexTypes/Opc.Ua.Client.ComplexTypes.csproj: Language not supported
Libraries/Opc.Ua.Client/Opc.Ua.Client.csproj: Language not supported
Libraries/Opc.Ua.Configuration/Opc.Ua.Configuration.csproj: Language not supported
Libraries/Opc.Ua.Gds.Client.Common/Opc.Ua.Gds.Client.Common.csproj: Language not supported
Libraries/Opc.Ua.Gds.Server.Common/Opc.Ua.Gds.Server.Common.csproj: Language not supported
Libraries/Opc.Ua.PubSub/Opc.Ua.PubSub.csproj: Language not supported
Libraries/Opc.Ua.Security.Certificates/Opc.Ua.Security.Certificates.csproj: Language not supported
Libraries/Opc.Ua.Server/Opc.Ua.Server.csproj: Language not supported
Stack/Opc.Ua.Bindings.Https/Opc.Ua.Bindings.Https.csproj: Language not supported
Stack/Opc.Ua.Core/Opc.Ua.Core.csproj: Language not supported
Applications/Quickstarts.Servers/Boiler/BoilerNodeManager.cs: Evaluated as low risk
Libraries/Opc.Ua.Client/Session/Session.cs: Evaluated as low risk

Comments suppressed due to low confidence (1)

Applications/Quickstarts.Servers/Alarms/AlarmHolders/ConditionTypeHolder.cs:168

[nitpick] The variable name 'alarm' is ambiguous. It should be renamed to something more descriptive like 'conditionState'.

public void ReportEvent(ConditionState alarm = null)

some .NET9 maintenance and perf improvements

7be5218

mregen requested a review from Copilot January 3, 2025 10:03

Copilot AI reviewed Jan 3, 2025

View reviewed changes

mregen added 6 commits January 3, 2025 12:31

bump nunit and null string handling

188db24

fix net60 builds

462f104

support morenotification flag

28f5426

reduce string binarydecoder memory allocations

fix morenotifications

03ab6ff

bump more updates

0891688

typo

c44dd02

mregen added this to the 1.5.374 Januar update milestone Jan 7, 2025

mregen added 3 commits January 16, 2025 22:51

bump dependenies, use logging abstraction 8 for net80

201a607

merge

6d2af00

fix fuzzing error

536567d

mregen requested a review from Copilot January 16, 2025 22:11

mregen marked this pull request as ready for review January 16, 2025 22:12

Copilot AI reviewed Jan 16, 2025

View reviewed changes

mregen added 2 commits January 17, 2025 06:20

fix net9 dependencies

dd1e3fe

bump more dependencies

0ea6a25

This was linked to issues Jan 17, 2025

Restore .NET 8 LTS Support for UA-.NETStandard Stack #2940

Closed

Conflict with Microsoft.Extensions.Logging.Abstractions in Opc.Ua.Core and ASP.NET Core Applications #2931

Closed

mregen requested review from Copilot, romanett and mrsuciu January 18, 2025 06:30

Copilot AI reviewed Jan 18, 2025

View reviewed changes

Stack/Opc.Ua.Core/Types/Encoders/JsonEncoder.cs Show resolved Hide resolved

romanett approved these changes Jan 18, 2025

View reviewed changes

mregen added 3 commits January 20, 2025 07:17

sync with local changes

f4cd776

Merge branch 'net9updates' of https://github.com/OPCFoundation/UA-.NE…

57373dd

…TStandard into net9updates

fix netstandard2.0 tests

9f4eec9

romanett reviewed Jan 20, 2025

View reviewed changes

mregen added 2 commits January 20, 2025 07:34

fix langversion

3b15c1e

use official asp.net packages for ,NET Framework

d868a6d

mregen requested a review from Copilot January 20, 2025 07:41

Copilot AI reviewed Jan 20, 2025

View reviewed changes

mregen merged commit 61056d0 into develop/main374 Jan 20, 2025
27 of 51 checks passed

mregen deleted the net9updates branch January 20, 2025 07:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Some .NET 9 maintenance and JSON encoder improvements #2922

Some .NET 9 maintenance and JSON encoder improvements #2922

mregen commented Jan 3, 2025 •

edited

Loading

codecov bot commented Jan 3, 2025 •

edited

Loading

Copilot AI left a comment

romanett left a comment

mregen commented Jan 20, 2025

romanett Jan 20, 2025

mregen Jan 20, 2025

Some .NET 9 maintenance and JSON encoder improvements #2922

Some .NET 9 maintenance and JSON encoder improvements #2922

Conversation

mregen commented Jan 3, 2025 • edited Loading

Proposed changes

Package Reference Updates:

Debugging Enhancements:

Code Efficiency and Readability:

New Features:

Miscellaneous:

Types of changes

Checklist

Further comments

codecov bot commented Jan 3, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

romanett left a comment

Choose a reason for hiding this comment

mregen commented Jan 20, 2025

romanett Jan 20, 2025

Choose a reason for hiding this comment

mregen Jan 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mregen commented Jan 3, 2025 •

edited

Loading

codecov bot commented Jan 3, 2025 •

edited

Loading