From 694a5cfd6361dab160e5006419c45bce62d79bab Mon Sep 17 00:00:00 2001 From: Oliver Date: Fri, 27 Mar 2026 12:27:57 +0100 Subject: [PATCH] Fix malformed XML docs in builder interfaces --- .../AasBuilderTests.cs | 87 +++++++++++++++ FluentAAS/FluentAAS.Builder/AasBuilder.cs | 104 +++++++++++++----- FluentAAS/FluentAAS.Builder/IAasBuilder.cs | 23 ++-- .../SubModel/SubmodelFragmentBuilder.cs | 9 +- README.md | 2 +- 5 files changed, 172 insertions(+), 53 deletions(-) diff --git a/FluentAAS/FluentAAS.Builder.Tests/AasBuilderTests.cs b/FluentAAS/FluentAAS.Builder.Tests/AasBuilderTests.cs index 4ccb0d0..994c3ea 100644 --- a/FluentAAS/FluentAAS.Builder.Tests/AasBuilderTests.cs +++ b/FluentAAS/FluentAAS.Builder.Tests/AasBuilderTests.cs @@ -266,6 +266,93 @@ public void AddSubmodelFragment_WithoutBaseSubmodel_ShouldThrowInvalidOperationE ex.Message.ShouldContain("No base submodel"); } + [Fact] + public void Build_WithShellReferencingKnownSubmodelId_ShouldSucceed() + { + // Arrange + var builder = CreateSut(); + var submodelId = _fixture.Create()!; + builder.AddSubmodel(new Submodel(id: submodelId, idShort: "known-submodel")); + + var shell = new AssetAdministrationShell( + id: _fixture.Create()!, + assetInformation: new AssetInformation(AssetKind.Instance)) + { + IdShort = "shell-with-known-ref", + Submodels = + [ + new Reference( + ReferenceTypes.ModelReference, + [new Key(KeyTypes.Submodel, submodelId)]) + ] + }; + + builder.AddShellInternal(shell); + + // Act + var environment = builder.Build(); + + // Assert + var builtShell = environment.AssetAdministrationShells!.OfType().Single(); + builtShell.Submodels.ShouldNotBeNull(); + builtShell.Submodels.Count.ShouldBe(1); + builtShell.Submodels.Single().Keys.Single().Value.ShouldBe(submodelId); + } + + [Fact] + public void Build_WithShellReferencingUnknownSubmodelId_ShouldThrowInvalidOperationException() + { + // Arrange + var builder = CreateSut(); + var unknownSubmodelId = _fixture.Create()!; + + var shell = new AssetAdministrationShell( + id: _fixture.Create()!, + assetInformation: new AssetInformation(AssetKind.Instance)) + { + IdShort = "shell-with-unknown-ref", + Submodels = + [ + new Reference( + ReferenceTypes.ModelReference, + [new Key(KeyTypes.Submodel, unknownSubmodelId)]) + ] + }; + + builder.AddShellInternal(shell); + + // Act + var act = () => builder.Build(); + + // Assert + var ex = Should.Throw(act); + ex.Message.ShouldContain($"references unknown submodel id '{unknownSubmodelId}'"); + } + + [Fact] + public void Build_WhenFragmentBatchFails_ShouldRollbackAppliedFragmentsBeforeRetry() + { + // Arrange + var builder = CreateSut(); + var existingSubmodelId = _fixture.Create(); + var missingSubmodelId = _fixture.Create(); + + builder.AddSubmodel(new Submodel(id: existingSubmodelId, idShort: "existing")); + builder.AddSubmodelFragment(existingSubmodelId, fragment => fragment.AddProperty("temperature", "21.5")); + builder.AddSubmodelFragment(missingSubmodelId, fragment => fragment.AddProperty("pressure", "1.0")); + + // Act + Should.Throw(() => builder.Build()); + + builder.AddSubmodel(new Submodel(id: missingSubmodelId, idShort: "missing-now-added")); + var env = builder.Build(); + + // Assert + var existingSubmodel = env.Submodels!.OfType().Single(s => s.Id == existingSubmodelId); + existingSubmodel.SubmodelElements.ShouldNotBeNull(); + existingSubmodel.SubmodelElements.Count(e => e.IdShort == "temperature").ShouldBe(1); + } + [Fact] public void Build_WhenCalledMultipleTimes_ShouldReturnNewEnvironmentInstancesWithSameContent() { diff --git a/FluentAAS/FluentAAS.Builder/AasBuilder.cs b/FluentAAS/FluentAAS.Builder/AasBuilder.cs index 3ded557..da7b27d 100644 --- a/FluentAAS/FluentAAS.Builder/AasBuilder.cs +++ b/FluentAAS/FluentAAS.Builder/AasBuilder.cs @@ -41,9 +41,7 @@ internal AasBuilder() /// /// A instance for further configuration of the shell. /// - /// - /// Thrown when or is null, empty, or whitespace. - /// + /// Throws when or is null, empty, or whitespace. public IShellBuilder AddShell(string id, string idShort, AssetKind kind = AssetKind.Instance) { if (string.IsNullOrWhiteSpace(id)) @@ -75,10 +73,7 @@ public IShellBuilder AddShell(string id, string idShort, AssetKind kind = AssetK /// /// The submodel instance to add. /// The current instance for fluent chaining. - /// Thrown when is null. - /// - /// Thrown when required submodel fields are invalid. - /// + /// Throws when is null and when required submodel fields are invalid. public IAasBuilder AddSubmodel(Submodel submodel) { ValidateSubmodelForPublicAdd(submodel); @@ -91,7 +86,7 @@ public IAasBuilder AddSubmodel(Submodel submodel) /// /// The submodel instance to add. /// The current instance for fluent chaining. - /// Thrown when is null. + /// Throws when is null. public IAasBuilder AddExistingSubmodel(Submodel submodel) { AddSubmodelInternal(submodel); @@ -105,8 +100,7 @@ public IAasBuilder AddExistingSubmodel(Submodel submodel) /// The identifier of the target submodel. /// Callback that contributes submodel elements. /// The current instance for fluent chaining. - /// Thrown when is null, empty, or whitespace. - /// Thrown when is null. + /// Throws when is invalid and when is null. public IAasBuilder AddSubmodelFragment(string submodelId, Action configure) { if (string.IsNullOrWhiteSpace(submodelId)) @@ -128,13 +122,13 @@ public IAasBuilder AddSubmodelFragment(string submodelId, Action /// Builds and returns the configured instance. /// - /// - /// A new containing the configured asset administration - /// shells and submodels. - /// + /// A new containing the configured asset administration shells and submodels. public IEnvironment Build() { - var duplicateSubmodelIds = _submodels.OfType() + var workingSubmodels = CloneSubmodels(_submodels); + var workingShells = CloneShells(_shells); + + var duplicateSubmodelIds = workingSubmodels.OfType() .GroupBy(s => s.Id, StringComparer.Ordinal) .Where(g => !string.IsNullOrWhiteSpace(g.Key) && g.Count() > 1) .Select(g => g.Key) @@ -146,25 +140,28 @@ public IEnvironment Build() throw new InvalidOperationException($"Duplicate submodel id(s) detected at build-time: {duplicateIdList}. Each submodel id must be unique."); } - foreach (var (submodelId, applyFragment) in _submodelFragments) + var submodelsById = workingSubmodels.OfType() + .ToDictionary(sm => sm.Id, StringComparer.Ordinal); + + foreach (var (submodelId, _) in _submodelFragments) { - var submodel = _submodels.OfType().FirstOrDefault(sm => string.Equals(sm.Id, submodelId, StringComparison.Ordinal)); - if (submodel is null) + if (!submodelsById.ContainsKey(submodelId)) { throw new InvalidOperationException($"No base submodel with id '{submodelId}' was found for a staged fragment. Add the submodel before adding fragments."); } - - applyFragment(submodel); } - _submodelFragments.Clear(); + foreach (var (submodelId, applyFragment) in _submodelFragments) + { + applyFragment(submodelsById[submodelId]); + } - var knownSubmodelIds = _submodels.OfType() - .Select(s => s.Id) - .Where(id => !string.IsNullOrWhiteSpace(id)) - .ToHashSet(StringComparer.Ordinal); + var knownSubmodelIds = workingSubmodels.OfType() + .Select(s => s.Id) + .Where(id => !string.IsNullOrWhiteSpace(id)) + .ToHashSet(StringComparer.Ordinal); - foreach (var shell in _shells.OfType()) + foreach (var shell in workingShells.OfType()) { foreach (var reference in shell.Submodels ?? []) { @@ -181,11 +178,17 @@ public IEnvironment Build() } } + _submodels.Clear(); + _submodels.AddRange(workingSubmodels); + _shells.Clear(); + _shells.AddRange(workingShells); + _submodelFragments.Clear(); + // Return a fresh Environment each time, with copies of the internal lists return new Environment { - AssetAdministrationShells = _shells.ToList(), - Submodels = _submodels.ToList() + AssetAdministrationShells = CloneShells(_shells), + Submodels = CloneSubmodels(_submodels) }; } @@ -194,7 +197,7 @@ public IEnvironment Build() /// Intended for internal use by . /// /// The shell instance to add. - /// Thrown when is null. + /// Throws when is null. public void AddShellInternal(AssetAdministrationShell shell) { ArgumentNullException.ThrowIfNull(shell); @@ -210,7 +213,7 @@ public void AddShellInternal(AssetAdministrationShell shell) /// Intended for internal use by shell or submodel builders. /// /// The submodel instance to add. - /// Thrown when is null. + /// Throws when is null. public void AddSubmodelInternal(Submodel submodel) { ArgumentNullException.ThrowIfNull(submodel); @@ -241,4 +244,45 @@ private static void ValidateSubmodelForPublicAdd(Submodel submodel) throw new ArgumentException("Submodel idShort must not be empty.", nameof(submodel)); } } + + private static List CloneSubmodels(IEnumerable submodels) + { + return submodels.Select(submodel => submodel is Submodel concreteSubmodel + ? (ISubmodel)CloneSubmodel(concreteSubmodel) + : submodel) + .ToList(); + } + + private static Submodel CloneSubmodel(Submodel source) + { + return new Submodel(id: source.Id) + { + IdShort = source.IdShort, + SubmodelElements = source.SubmodelElements?.ToList() + }; + } + + private static List CloneShells(IEnumerable shells) + { + return shells.Select(shell => shell is AssetAdministrationShell concreteShell + ? (IAssetAdministrationShell)CloneShell(concreteShell) + : shell) + .ToList(); + } + + private static AssetAdministrationShell CloneShell(AssetAdministrationShell source) + { + return new AssetAdministrationShell(id: source.Id, assetInformation: source.AssetInformation) + { + IdShort = source.IdShort, + Submodels = source.Submodels?.Select(CloneReference).ToList() + }; + } + + private static Reference CloneReference(Reference source) + { + return new Reference( + source.Type, + source.Keys.Select(key => new Key(key.Type, key.Value)).ToList()); + } } diff --git a/FluentAAS/FluentAAS.Builder/IAasBuilder.cs b/FluentAAS/FluentAAS.Builder/IAasBuilder.cs index e3dc80b..47edaef 100644 --- a/FluentAAS/FluentAAS.Builder/IAasBuilder.cs +++ b/FluentAAS/FluentAAS.Builder/IAasBuilder.cs @@ -17,9 +17,7 @@ public interface IAasBuilder /// /// A instance for further configuration of the shell. /// - /// - /// Thrown when or is null, empty, or whitespace. - /// + /// Throws when or is null, empty, or whitespace. IShellBuilder AddShell(string id, string idShort, AssetKind kind = AssetKind.Instance); /// @@ -28,10 +26,7 @@ public interface IAasBuilder /// /// The submodel instance to add. /// The current instance for fluent chaining. - /// Thrown when is null. - /// - /// Thrown when required submodel fields (for example ) are invalid. - /// + /// Throws when is null and when required submodel fields are invalid. IAasBuilder AddSubmodel(Submodel submodel); /// @@ -39,7 +34,7 @@ public interface IAasBuilder /// /// The submodel instance to add. /// The current instance for fluent chaining. - /// Thrown when is null. + /// Throws when is null. IAasBuilder AddExistingSubmodel(Submodel submodel); /// @@ -49,17 +44,13 @@ public interface IAasBuilder /// The identifier of the target submodel. /// Callback that contributes submodel elements. /// The current instance for fluent chaining. - /// Thrown when is null, empty, or whitespace. - /// Thrown when is null. + /// Throws when is invalid and when is null. IAasBuilder AddSubmodelFragment(string submodelId, Action configure); /// /// Builds and returns the configured instance. /// - /// - /// A new containing the configured asset administration - /// shells and submodels. - /// + /// A new containing the configured asset administration shells and submodels. IEnvironment Build(); /// @@ -67,7 +58,7 @@ public interface IAasBuilder /// Intended for internal use by . /// /// The shell instance to add. - /// Thrown when is null. + /// Throws when is null. void AddShellInternal(AssetAdministrationShell shell); /// @@ -75,6 +66,6 @@ public interface IAasBuilder /// Intended for internal use by shell or submodel builders. /// /// The submodel instance to add. - /// Thrown when is null. + /// Throws when is null. void AddSubmodelInternal(Submodel submodel); } diff --git a/FluentAAS/FluentAAS.Builder/SubModel/SubmodelFragmentBuilder.cs b/FluentAAS/FluentAAS.Builder/SubModel/SubmodelFragmentBuilder.cs index df998e6..11cc0ab 100644 --- a/FluentAAS/FluentAAS.Builder/SubModel/SubmodelFragmentBuilder.cs +++ b/FluentAAS/FluentAAS.Builder/SubModel/SubmodelFragmentBuilder.cs @@ -21,9 +21,7 @@ internal SubmodelFragmentBuilder(Submodel target) /// The property value. /// The property value type. Defaults to . /// The current . - /// - /// Thrown when or is null, empty, or whitespace. - /// + /// Throws when or is null, empty, or whitespace. public SubmodelFragmentBuilder AddProperty(string idShort, string value, DataTypeDefXsd valueType = DataTypeDefXsd.String) { if (string.IsNullOrWhiteSpace(idShort)) @@ -52,8 +50,7 @@ public SubmodelFragmentBuilder AddProperty(string idShort, string value, DataTyp /// The short identifier of the multi-language property. /// Configuration callback for language strings. /// The current . - /// Thrown when is null, empty, or whitespace. - /// Thrown when is null. + /// Throws when is invalid and when is null. public SubmodelFragmentBuilder AddMultiLanguageProperty(string idShort, Action configure) { if (string.IsNullOrWhiteSpace(idShort)) @@ -75,7 +72,7 @@ public SubmodelFragmentBuilder AddMultiLanguageProperty(string idShort, Action /// The submodel element to add. /// The current . - /// Thrown when is null. + /// Throws when is null. public SubmodelFragmentBuilder AddElement(ISubmodelElement element) { ArgumentNullException.ThrowIfNull(element); diff --git a/README.md b/README.md index af2c86c..b6c2ec5 100644 --- a/README.md +++ b/README.md @@ -208,7 +208,7 @@ var environment = builder.Build(); - Fragment targets must exist. - Shell submodel references must resolve to known submodels. -> Backward compatibility note: `AddExistingSubmodel(...)` still works and delegates to `AddSubmodel(...)`. +> Backward compatibility note: `AddExistingSubmodel(...)` still works, but it delegates to `AddSubmodelInternal(...)` (not `AddSubmodel(...)`) and therefore skips the additional public `IdShort` validation performed by `AddSubmodel(...)`. ---