Add staged submodel composition and validation (fragments) with tests#19
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a5f3b54fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| throw new InvalidOperationException($"No base submodel with id '{submodelId}' was found for a staged fragment. Add the submodel before adding fragments."); | ||
| } | ||
|
|
||
| applyFragment(submodel); |
There was a problem hiding this comment.
Build() executes applyFragment(submodel) against the builder’s stored Submodel instances on every invocation, and _submodelFragments is never consumed or reset. This makes repeated Build() calls non-idempotent (fragment elements are appended again each time) and also mutates previously returned environments because they share the same submodel objects. Apply fragments to a fresh copy (or apply each fragment only once) before returning the environment.
Useful? React with 👍 / 👎.
| public void AddSubmodelInternal(Submodel submodel) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(submodel); | ||
| ValidateSubmodel(submodel); |
There was a problem hiding this comment.
AddSubmodelInternal now always calls ValidateSubmodel, which throws when IdShort is missing. That change also affects ShellBuilder.AddSubmodelReference, which routes through AddSubmodelInternal and previously accepted reference-only submodels created as new Submodel(id: ...). This introduces a runtime break for existing reference flows unless callers are all updated to provide IdShort.
Useful? React with 👍 / 👎.
Freezor
merged commit 9cb0629
into
12-enable-incremental-submodel-composition-via-addsubmodel-at-any-stage-of-the-build-process
#20) * Add staged submodel composition and validation (fragments) with tests (#19) * Update README for staged submodel composition and remove extra spec doc * Fix staged fragment idempotency and preserve internal submodel compatibility * 📝 Add docstrings to `12-enable-incremental-submodel-composition-via-addsubmodel-at-any-stage-of-the-build-process` (#21) Docstrings generation was requested by @Freezor. * #20 (comment) The following files were modified: * `FluentAAS/FluentAAS.Builder/AasBuilder.cs` * `FluentAAS/FluentAAS.Builder/IAasBuilder.cs` * `FluentAAS/FluentAAS.Builder/SubModel/SubmodelFragmentBuilder.cs` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * 📝 CodeRabbit Chat: Add unit tests * Fix malformed XML docs in builder interfaces (#22) * FEAT Fix docs issue * Make submodel fragment application transactional in Build (#23) --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Motivation
Submodelinstances so different modules/plugins can contribute elements beforeBuild().Description
SubmodelFragmentBuilderto stage contributions against an existingSubmodeland apply them duringBuild()viaAddSubmodelFragment(string, Action<SubmodelFragmentBuilder>)onIAasBuilder/AasBuilder.AddSubmodel(Submodel)(delegated fromAddExistingSubmodel) and centralize submodel validation inValidateSubmodelto checkIdandIdShortwhen adding.README.mdwith usage examples and notes about staged submodel composition and backward compatibility.Testing
FluentAAS.Builder.Tests/AasBuilderTests.cs:AddSubmodel_WithValidSubmodel_ShouldAddToEnvironmentAndReturnBuilder,AddSubmodel_WithInvalidIdShort_ShouldThrowArgumentException,Build_WithDuplicateSubmodelIds_ShouldThrowInvalidOperationException,AddSubmodelFragment_ShouldApplyContributionAtBuildTime, andAddSubmodelFragment_WithoutBaseSubmodel_ShouldThrowInvalidOperationException, and verified existingBuild_WhenCalledMultipleTimes_ShouldReturnNewEnvironmentInstancesWithSameContentstill behaves correctly.FluentAAS.Builder.Testssuite including the new tests and the full builder tests, and all automated tests passed successfully.Codex Task