Make AasBuilder build atomic: clone working state, validate references, and add tests#22
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: 694a5cfd63
ℹ️ 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".
| return new Submodel(id: source.Id) | ||
| { | ||
| IdShort = source.IdShort, | ||
| SubmodelElements = source.SubmodelElements?.ToList() | ||
| }; |
There was a problem hiding this comment.
CloneSubmodel recreates a Submodel with only Id, IdShort, and SubmodelElements, so any other configured fields (for example SemanticId set via SubmodelBuilder.WithSemanticId, descriptions, qualifiers, or kind) are silently dropped during Build(). Because Build() then writes these clones back into _submodels, this is destructive: metadata is lost after the first successful build and cannot be recovered in later builds.
Useful? React with 👍 / 👎.
| return new AssetAdministrationShell(id: source.Id, assetInformation: source.AssetInformation) | ||
| { | ||
| IdShort = source.IdShort, | ||
| Submodels = source.Submodels?.Select(CloneReference).ToList() | ||
| }; |
There was a problem hiding this comment.
CloneShell only copies Id, AssetInformation, IdShort, and Submodels, which strips other valid AssetAdministrationShell data (e.g. DerivedFrom, administration/description fields, extensions, embedded data specs) on every Build(). Since these reduced clones are committed back into _shells, a successful build permanently erases shell metadata and subsequent environments become incomplete.
Useful? React with 👍 / 👎.
…del-at-any-stage-of-the-build-process' into codex/fix-atomicity-in-build-fragment-application # Conflicts: # FluentAAS/FluentAAS.Builder/AasBuilder.cs # FluentAAS/FluentAAS.Builder/IAasBuilder.cs # FluentAAS/FluentAAS.Builder/SubModel/SubmodelFragmentBuilder.cs
Freezor
merged commit 65dcf8b
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
AddExistingSubmodel.Description
AasBuilder.Build, applying validations and fragments against the working copies, and committing only on success; this prevents partial application on failure and enables retrying after failure.CloneSubmodels,CloneSubmodel,CloneShells,CloneShell, andCloneReferenceto produce environment copies returned byBuildand to isolate working state during build.Build_WithShellReferencingKnownSubmodelId_ShouldSucceed,Build_WithShellReferencingUnknownSubmodelId_ShouldThrowInvalidOperationException, andBuild_WhenFragmentBatchFails_ShouldRollbackAppliedFragmentsBeforeRetry) and updated API/method XML/remarks andREADMEnote aboutAddExistingSubmodelbehavior.Testing
dotnet test, including the three new tests, and all tests passed.Build_WithShellReferencingKnownSubmodelId_ShouldSucceedsucceeds when the submodel exists.Build_WithShellReferencingUnknownSubmodelId_ShouldThrowInvalidOperationExceptionandBuild_WhenFragmentBatchFails_ShouldRollbackAppliedFragmentsBeforeRetryboth surface the expectedInvalidOperationExceptionconditions.Codex Task