From 7e1b2fe54e1a9e52c9542476ba8707c2f268d913 Mon Sep 17 00:00:00 2001 From: samatstarion Date: Mon, 19 Oct 2026 14:05:13 +0200 Subject: [PATCH] [Fix] AttributeValueXHTML public constructors no longer leave ExternalObjects or the logger null; fixes #98 - Initialise ExternalObjects where it is declared, so every constructor gets an empty list; QueryExternalObjects and QueryBase64PayloadsAsync of ReqIFSharp.Extensions threw for values made with the ILoggerFactory constructor. - Set a NullLogger in the parameterless constructor; reading into such a value threw an ArgumentNullException. - Correct the doc comment of the ILoggerFactory constructor. - Add tests for the constructors, reading and cancelling with the parameterless constructor, and the Extensions queries. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + ...ementWithAttributesExtensionTestFixture.cs | 15 +++++ .../AttributeValueXHTMLTestFixture.cs | 59 +++++++++++++++++++ .../AttributeValue/AttributeValueXHTML.cs | 10 +--- 4 files changed, 78 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bab06d5..3dded6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The repository ships two NuGet packages that version independently: **ReqIFSharp - A document without whitespace between its elements (minified XML) lost data without an error: every other header field (such as `CREATION-TIME` and `REQ-IF-TOOL-ID`) and every other enumeration value of an attribute value was skipped, and the spec relations of a relation group were not read or caused an `XmlException` ([#94](https://github.com/STARIONGROUP/ReqIFSharp/issues/94)). - `AttributeValueXHTML.TheOriginalValue` was not read from `THE-ORIGINAL-VALUE`, so it was always `null` after reading, and a document that was read and written again lost the original values of its simplified XHTML attribute values ([#95](https://github.com/STARIONGROUP/ReqIFSharp/issues/95)). - Attribute values did not log anything, also when an `ILoggerFactory` was passed to the deserializer: the `AttributeValue` constructors did not keep the logger factory. An enumeration attribute value no longer reports its own element and its `DEFINITION` and `VALUES` elements as not supported ([#97](https://github.com/STARIONGROUP/ReqIFSharp/issues/97)). +- `new AttributeValueXHTML(loggerFactory)` left `ExternalObjects` `null`, so `QueryExternalObjects` and `QueryBase64PayloadsAsync` of ReqIFSharp.Extensions threw for a spec element that held such a value. `new AttributeValueXHTML()` left its logger `null`, so reading into it threw ([#98](https://github.com/STARIONGROUP/ReqIFSharp/issues/98)). ## [10.1.2] - 2026-10-18 diff --git a/ReqIFSharp.Extensions.Tests/ReqIFExtensions/SpecElementWithAttributesExtensionTestFixture.cs b/ReqIFSharp.Extensions.Tests/ReqIFExtensions/SpecElementWithAttributesExtensionTestFixture.cs index 677134f..397dba0 100644 --- a/ReqIFSharp.Extensions.Tests/ReqIFExtensions/SpecElementWithAttributesExtensionTestFixture.cs +++ b/ReqIFSharp.Extensions.Tests/ReqIFExtensions/SpecElementWithAttributesExtensionTestFixture.cs @@ -26,6 +26,8 @@ namespace ReqIFSharp.Extensions.Tests.ReqIFExtensions using System.Threading; using System.Threading.Tasks; + using Microsoft.Extensions.Logging.Abstractions; + using NUnit.Framework; using ReqIFSharp; @@ -70,6 +72,19 @@ public void Verify_that_QueryExternalObjects_returns_expected_results() Assert.That(externalObjects.Count, Is.EqualTo(1)); } + [Test] + public async Task Verify_that_the_external_objects_of_xhtml_values_created_with_a_public_constructor_can_be_queried() + { + // GH-98: the ILoggerFactory constructor left ExternalObjects null + var specObject = new SpecObject(); + specObject.Values.Add(new AttributeValueXHTML { TheValue = "text" }); + specObject.Values.Add(new AttributeValueXHTML(NullLoggerFactory.Instance) { TheValue = "text" }); + specObject.Values.Add(new AttributeValueXHTML(null) { TheValue = "text" }); + + Assert.That(specObject.QueryExternalObjects(), Is.Empty); + Assert.That(await specObject.QueryBase64PayloadsAsync(this.reqIFLoaderService), Is.Empty); + } + [Test] public async Task Verify_that_QueryBase64Payloads_returns_expected_results() { diff --git a/ReqIFSharp.Tests/AttributeValueTests/AttributeValueXHTMLTestFixture.cs b/ReqIFSharp.Tests/AttributeValueTests/AttributeValueXHTMLTestFixture.cs index e12a00f..ee087da 100644 --- a/ReqIFSharp.Tests/AttributeValueTests/AttributeValueXHTMLTestFixture.cs +++ b/ReqIFSharp.Tests/AttributeValueTests/AttributeValueXHTMLTestFixture.cs @@ -24,6 +24,7 @@ namespace ReqIFSharp.Tests using System.IO; using System.Runtime.Serialization; using System.Threading; + using System.Threading.Tasks; using System.Xml; using Microsoft.Extensions.Logging; @@ -128,6 +129,64 @@ public void Verify_That_WriteXmlAsync_Without_Definition_Set_Throws_Serializatio Throws.Exception.TypeOf()); } + [Test] + public void Verify_that_every_public_constructor_creates_an_empty_list_of_external_objects() + { + // GH-98: the ILoggerFactory constructor left ExternalObjects null + Assert.Multiple(() => + { + Assert.That(new AttributeValueXHTML().ExternalObjects, Is.Not.Null.And.Empty); + Assert.That(new AttributeValueXHTML(this.loggerFactory).ExternalObjects, Is.Not.Null.And.Empty); + Assert.That(new AttributeValueXHTML(null).ExternalObjects, Is.Not.Null.And.Empty); + }); + } + + [Test] + public async Task Verify_that_a_value_created_with_the_parameterless_constructor_can_be_read() + { + // GH-98: the parameterless constructor left the logger null, so reading threw an ArgumentNullException + const string xml = "" + + "text" + + ""; + + foreach (var async in new[] { false, true }) + { + using var xmlReader = XmlReader.Create(new StringReader(xml), new XmlReaderSettings { Async = async }); + xmlReader.MoveToContent(); + + var attributeValueXhtml = new AttributeValueXHTML(); + + if (async) + { + await attributeValueXhtml.ReadXmlAsync(xmlReader, CancellationToken.None); + } + else + { + attributeValueXhtml.ReadXml(xmlReader); + } + + Assert.That(attributeValueXhtml.TheValue, Does.Contain("text"), $"async: {async}"); + Assert.That(attributeValueXhtml.IsSimplified, Is.True); + } + } + + [Test] + public void Verify_that_ReadXmlAsync_of_a_value_created_with_the_parameterless_constructor_can_be_cancelled() + { + const string xml = "" + + "text" + + ""; + + var cts = new CancellationTokenSource(); + cts.Cancel(); + + using var xmlReader = XmlReader.Create(new StringReader(xml), new XmlReaderSettings { Async = true }); + xmlReader.MoveToContent(); + + Assert.That(async () => await new AttributeValueXHTML().ReadXmlAsync(xmlReader, cts.Token), + Throws.Exception.InstanceOf()); + } + [Test] public void Verify_that_ReadXmlAsync_throws_exception_when_cancelled() { diff --git a/ReqIFSharp/AttributeValue/AttributeValueXHTML.cs b/ReqIFSharp/AttributeValue/AttributeValueXHTML.cs index e83207f..7d9c3bf 100644 --- a/ReqIFSharp/AttributeValue/AttributeValueXHTML.cs +++ b/ReqIFSharp/AttributeValue/AttributeValueXHTML.cs @@ -50,11 +50,11 @@ public class AttributeValueXHTML : AttributeValue /// public AttributeValueXHTML() { - this.ExternalObjects = new List(); + this.logger = NullLogger.Instance; } /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// /// The (injected) used to set up logging @@ -77,8 +77,6 @@ internal AttributeValueXHTML(SpecElementWithAttributes specElAt, ILoggerFactory : base(specElAt, loggerFactory) { this.logger = this.loggerFactory == null ? NullLogger.Instance : this.loggerFactory.CreateLogger(); - - this.ExternalObjects = new List(); } /// @@ -96,8 +94,6 @@ internal AttributeValueXHTML(AttributeDefinitionXHTML attributeDefinition, ILogg { this.logger = this.loggerFactory == null ? NullLogger.Instance : this.loggerFactory.CreateLogger(); - this.ExternalObjects = new List(); - this.OwningDefinition = attributeDefinition; } @@ -136,7 +132,7 @@ public override object ObjectValue /// /// Gets the that may be present as xhtml:object in the XHTML content /// - public List ExternalObjects { get; private set; } + public List ExternalObjects { get; private set; } = new List(); /// /// Gets or sets the Reference to the attribute definition that relates the value to its data type.