Conversation
…eStrategy Foundational types for a strategy-driven BOM merge engine: a marker interface (IBomEntity) plus IMergeable<T>/IEquivalent<T>, each with a default interface method body that falls back to the type's own IEquatable<T> equality. Most model classes will need nothing more than declaring the interface (zero-body opt-in); only types with real reconciliation logic override the defaults. Compiled for net8.0+/net10.0 only (default interface methods require an ABI this library's netstandard2.0 target cannot provide); netstandard2.0 consumers keep today's merge behavior unchanged. MergeStrategy configures the merge: independent on/off toggles for subset-dependency merging, dependency-as-extra-property treatment, and metadata refresh, plus a ComponentConflictResolution enum (rather than more booleans) so new resolution algorithms can be added as new cases without reshaping this type or any call site that reads it. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
This was referenced Sep 15, 2026
Open
Declares the new interfaces on every model type the merge engine needs to process generically as list elements (Tool, Service, ExternalReference, Dependency, Composition, Vulnerability, Annotation, Standard, Assessor, Attestation, Claim, OrganizationalEntity, Component, plus nested-list element types Hash, Property, OrganizationalContact, LicenseChoice, PatentAssertion). For types that already implement IEquatable<T>, this is a one-line, zero-body change (the interface defaults handle it). Property, OrganizationalContact, LicenseChoice and PatentAssertion had no equality implementation at all; they gain the same JSON-hash-based Equals(object)/ Equals(T)/GetHashCode pattern already used by Component/Annotation/etc. elsewhere in this codebase (all three together, not just Equals(T)/ GetHashCode, to keep the object.Equals/GetHashCode contract consistent for callers that only see the non-generic Equals), so the interface defaults have something real to fall back on. Hash already had Equals(T)/GetHashCode but was missing the same Equals(object) override; added here too since this commit already touches the file. Hash gets a real (non-default) Equivalent/MergeWith: two hashes of the same algorithm should carry the same content; if one side is missing content, fill it in from the other, and treat a genuine content mismatch for the same algorithm as an unmergeable conflict. Component's real merge logic follows in a separate commit. Signed-off-by: Jim Klimov <jimklimov@gmail.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Generic-constrained (T : IEquatable<T>, IEquivalent<T>, IMergeable<T>) list merge: exact-equal items dedupe as before, equivalent-but-unequal items attempt MergeWith, everything else is kept as a separate entry. Falls back to the existing exact-match ListMergeHelper<T> when a strategy disables entity merging. One implementation serves every mergeable list field in a Bom (components, services, hashes, external references, authors, ...) via real interface dispatch -- no reflection, no per-type branching. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
Implements Component's merge logic as explicit per-field code instead of a PropertyInfo/switch(Type) walk: Equivalent() checks type/name required-equal, version/group/purl equal-if-both-present (bom-ref deliberately excluded -- that's the merge orchestration's concern, not a per-field check). MergeWith() folds scalar fields via ??, list fields via the new MergeableListHelper.Merge (lives in CycloneDX.Core rather than CycloneDX.Utils so Component can call it directly -- Utils depends on Core, not the reverse), and Scope via TryMergeScope, which handles the both-Excluded case explicitly: two components that are both Excluded-scope but differ in some unrelated field should still merge, not be treated as an unresolvable scope conflict just because neither side is Optional. MergeableListHelper also gains small single-value/string-list/ nullable-bool merge helpers used by Component's scalar and simple-list fields. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
BomRefWalker.RewriteRefs(bom, rewrite) generalizes the per-type ref-
rewriting CycloneDXUtils.HierarchicalMerge already hand-rolls for
bom-ref namespacing (NamespaceComponentBomRefs, NamespaceDependencyBomRefs,
NamespaceCompositions, NamespaceVulnerabilitiesRefs,
NamespaceAnnotationsBomRefs) into one reusable entry point parameterized
on an arbitrary rewrite function instead of a fixed namespace prefix.
Namespacing becomes RewriteRefs(bom, r => $"{ns}:{r}"); a single manual
rename becomes RewriteRefs(bom, r => r == oldRef ? newRef : r).
Covers Metadata.Component, Components, Services, Dependencies,
Compositions, Vulnerabilities, and Annotations; the newer (CycloneDX
1.6) Declarations/Definitions sections are not yet walked -- a
mechanical follow-up, not an architectural one.
Bom.RenameRef builds on the walker to rename a bom-ref and every
back-reference to it throughout a document in one pass.
BomMetadataReferThisToolkit/BomMetadataUpdate stamp this library's
(and the entry assembly's) Tool reference into Metadata.Tools, and
refresh Version/SerialNumber/Timestamp.
Signed-off-by: Jim Klimov <jimklimov@gmail.com>
New FlatMerge(bom1, bom2, MergeStrategy) and HierarchicalMerge(boms, bomSubject, MergeStrategy) overloads sit alongside the existing fixed-behavior ones (which are untouched and keep today's exact behavior for existing callers). FlatMerge's overload swaps every ListMergeHelper<T> call for MergeableListHelper.Merge(..., strategy), so Components/Services/Tools/Hashes/etc. attempt real reconciliation (Component's Scope-aware squash, Hash's content fill-in) instead of only deduping exact matches, and applies RenameConflictingComponents (pre-merge bom-ref collision detection via BomRefWalker) and the metadata-update toggles. Known gaps, called out rather than silently dropped: MergeSubsetDependencies isn't wired to real subset-detection logic yet (Dependency still merges via its IMergeable<T> default), and HierarchicalMerge's strategy overload only adds the metadata-update toggles so far (namespacing already avoids the collisions FlatMerge's squash logic exists to resolve, so it needed less new behavior here). Signed-off-by: Jim Klimov <jimklimov@gmail.com>
Unit tests (not full-BOM snapshots, for faster feedback) covering: Component.Equivalent's type/name/version-if-both-present rules and its deliberate bom-ref exclusion; Component.MergeWith's scope squash, excluded-vs-required conflict refusal, and the both-Excluded case (two components that are both Excluded-scope but differ in some unrelated field still merge); Hash.MergeWith's content fill-in/mismatch cases; FlatMerge(bom1, bom2, strategy) actually squashing equivalent components across two BOMs; and Bom.RenameRef rewriting both an identifier and its back-reference (plus the not-found no-op case). Signed-off-by: Jim Klimov <jimklimov@gmail.com>
The earlier commit only added the 2-BOM FlatMerge(bom1, bom2, strategy) overload; the CLI's merge command (and anyone merging more than two documents) needs the IEnumerable<Bom> form too. Also fixes a now- ambiguous overload resolution: the existing single-arg FlatMerge(boms) delegated via `FlatMerge(boms, null)`, which became ambiguous between the Component and MergeStrategy overloads once the latter existed -- disambiguated with an explicit (Component)null cast. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
jimklimov
force-pushed
the
merge-strategy-core
branch
from
September 17, 2026 09:27
4a8a659 to
139c3b0
Compare
Contributor
Author
|
Fixed: |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Introduces a generic, opt-in merge engine for
Bomand its component model classes:IBomEntity/IMergeable<T>/IEquivalent<T>(Models/Interfaces/IBomEntity.cs,Models/MergeStrategy.cs) — a small interface family a model class can implement to define its ownEquivalent()/MergeWith()behavior, plus aMergeStrategyoptions object to control it.Component,Dependency,Hash,Property,Tool,Vulnerability, etc.) retargeted to these interfaces.MergeableListHelper.Merge<T>(), a strategy-aware list-merge primitive.Component: real, field-by-fieldEquivalent()/MergeWith()(no reflection).BomRefWalker+Bom.RenameRef()/BomMetadataUpdate()/ReferThisToolkit(): a general bom-refrewriting primitive (covers
Metadata.Component,Components,Services,Dependencies,Compositions,Vulnerabilities,Annotations), used both to rename a single bom-ref and itsback-references, and to stamp a merged/updated
Bom's ownMetadata(tool reference, timestamp,serial number / version).
FlatMerge/HierarchicalMergeoverloads, including a multi-BOMFlatMerge(IEnumerable<Bom>, ...)form, alongside the existing plain overloads (opt-in, no behaviorchange for existing callers).
MergeStrategyTests.cs.Why
This supersedes #245 and #256, both of which stalled in review. #245 proposed the same idea (dedup
"equivalent" merge entries instead of blindly concatenating/renaming bom-refs) but touched ~70 files —
effectively every model class — via a large reflection-based base class, which made it hard to review.
This PR keeps the same goal but only retargets the ~18 classes that actually need custom merge
semantics, with straightforward field-by-field code instead of reflection. #256 added
Bomself-metadatainitialization (
BomMetadataUpdate/ReferThisToolkit); that lands here bundled withBomRefWalker/RenameRefsince they shareBom.csand were developed together — splitting them further wasn't worththe surgery.
This is PR 1 of a 5-PR stack that together replace #245/#256 with smaller, reviewable pieces: this PR
(interfaces + core merge engine), then #447 (scope-conflict/
Squash_RenameByScoperefinements), then#448 (
Merge.csmetadata/empty-list cleanup), then #449 (AttachDanglingComponents), then #450 (arecursive empty-list pruner) — each based on the previous one's tip. A matching
cyclonedx-cliPR stack(#509-#516, starting with a
rename-entitycommand and amergecommand wired toMergeStrategy.Default()) depends on this landing first.Fixes #219 (cyclonedx-cli), fixes #188 (cyclonedx-cli), fixes #82.