Skip to content

Reconcile bare/rich component identities in ComponentsFound #1760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AMaini503 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Reconcile bare/rich component identities in ComponentsFound #1760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 119 additions & 33 deletions src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,56 +34,142 @@ public TypedComponent GetComponent(string componentId)

public IEnumerable<DetectedComponent> GetDetectedComponents()
{
IEnumerable<DetectedComponent> detectedComponents;
if (this.singleFileRecorders == null)
{
return [];
}

detectedComponents = this.singleFileRecorders.Values
.SelectMany(singleFileRecorder => singleFileRecorder.GetDetectedComponents().Values)
.GroupBy(x => x.Component.Id)
.Select(grouping =>
{
// We pick a winner here -- any stateful props could get lost at this point.
var winningDetectedComponent = grouping.First();
var allComponents = this.singleFileRecorders.Values
.SelectMany(singleFileRecorder => singleFileRecorder.GetDetectedComponents().Values);

HashSet<string> mergedLicenses = null;
HashSet<ActorInfo> mergedSuppliers = null;
// When both rich and bare entries exist for the same BaseId, rich entries are used as merge targets for bare entries.
var reconciledComponents = new List<DetectedComponent>();

foreach (var component in grouping.Skip(1))
{
winningDetectedComponent.ContainerDetailIds.UnionWith(component.ContainerDetailIds);
foreach (var baseIdGroup in allComponents.GroupBy(x => x.Component.BaseId))
{
var richEntries = new List<DetectedComponent>();
var bareEntries = new List<DetectedComponent>();

// Defensive: merge in case different file recorders set different values for the same component.
if (component.LicensesConcluded != null)
{
mergedLicenses ??= new HashSet<string>(winningDetectedComponent.LicensesConcluded ?? [], StringComparer.OrdinalIgnoreCase);
mergedLicenses.UnionWith(component.LicensesConcluded);
}
// Sub-group by full Id first: merge duplicates of the same Id (existing behavior).
foreach (var idGroup in baseIdGroup.GroupBy(x => x.Component.Id))
{
var merged = MergeDetectedComponentGroup(idGroup);

if (component.Suppliers != null)
{
mergedSuppliers ??= new HashSet<ActorInfo>(winningDetectedComponent.Suppliers ?? []);
mergedSuppliers.UnionWith(component.Suppliers);
}
if (merged.Component.Id == merged.Component.BaseId)
{
bareEntries.Add(merged);
}

if (mergedLicenses != null)
else
{
winningDetectedComponent.LicensesConcluded = mergedLicenses.Where(x => x != null).OrderBy(x => x, StringComparer.OrdinalIgnoreCase).ToList();
richEntries.Add(merged);
}
}

if (mergedSuppliers != null)
if (richEntries.Count > 0 && bareEntries.Count > 0)
{
// Merge each bare entry's metadata into every rich entry, then drop the bare.
foreach (var bare in bareEntries)
{
winningDetectedComponent.Suppliers = mergedSuppliers.Where(s => s != null).OrderBy(s => s.Name).ThenBy(s => s.Type).ToList();
foreach (var rich in richEntries)
{
MergeComponentMetadata(source: bare, target: rich);
}
}

return winningDetectedComponent;
})
.ToArray();
reconciledComponents.AddRange(richEntries);
}
else
{
// No conflict: either all rich (different Ids kept separate) or all bare.
reconciledComponents.AddRange(richEntries);
reconciledComponents.AddRange(bareEntries);
}
}

return reconciledComponents.ToArray();
}

/// <summary>
/// Merges component-level metadata from <paramref name="source"/> into <paramref name="target"/>.
/// </summary>
private static void MergeComponentMetadata(DetectedComponent source, DetectedComponent target)
{
target.ContainerDetailIds.UnionWith(source.ContainerDetailIds);

foreach (var kvp in source.ContainerLayerIds)
{
if (target.ContainerLayerIds.TryGetValue(kvp.Key, out var existingLayers))
{
target.ContainerLayerIds[kvp.Key] = existingLayers.Union(kvp.Value).Distinct().ToList();
}
else
{
target.ContainerLayerIds[kvp.Key] = kvp.Value.ToList();
}
}

target.LicensesConcluded = MergeAndNormalizeLicenses(target.LicensesConcluded, source.LicensesConcluded);
target.Suppliers = MergeAndNormalizeSuppliers(target.Suppliers, source.Suppliers);
}

Comment on lines +110 to +114
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When subsuming/merging DetectedComponent instances, MergeComponentMetadata currently merges container metadata + LicensesConcluded/Suppliers, but does not merge FilePaths (custom locations added by detectors via AddComponentFilePath) or TargetFrameworks (added in RegisterUsage). Because bare entries (and duplicate Id entries) can be dropped during reconciliation, those locations/frameworks can be lost from ComponentsFound. Consider unioning source.FilePaths into target.FilePaths and merging TargetFrameworks similarly to the existing MergeTargetFrameworks logic so this metadata survives reconciliation.

Suggested change
target.LicensesConcluded = MergeAndNormalizeLicenses(target.LicensesConcluded, source.LicensesConcluded);
target.Suppliers = MergeAndNormalizeSuppliers(target.Suppliers, source.Suppliers);
}
target.FilePaths = MergeAndNormalizeStrings(target.FilePaths, source.FilePaths, StringComparer.Ordinal);
target.TargetFrameworks = MergeAndNormalizeStrings(target.TargetFrameworks, source.TargetFrameworks, StringComparer.OrdinalIgnoreCase);
target.LicensesConcluded = MergeAndNormalizeLicenses(target.LicensesConcluded, source.LicensesConcluded);
target.Suppliers = MergeAndNormalizeSuppliers(target.Suppliers, source.Suppliers);
}
private static IList<string> MergeAndNormalizeStrings(IList<string> target, IList<string> source, StringComparer comparer)
{
if (target == null && source == null)
{
return null;
}
var merged = new HashSet<string>(comparer);
if (target != null)
{
merged.UnionWith(target.Where(x => x != null));
}
if (source != null)
{
merged.UnionWith(source.Where(x => x != null));
}
return merged.OrderBy(x => x, comparer).ToList();
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@AMaini503 AMaini503 Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not applicable here. FilePaths and TargetFrameworks are not populated at this stage - they get filled downstream in GatherSetOfDetectedComponentsUnmerged from graph data. The original GetDetectedComponents code also only merged licenses, suppliers, and containers, so this is consistent with existing behavior.

private static IList<string> MergeAndNormalizeLicenses(IList<string> target, IList<string> source)
{
if (target == null && source == null)
{
return null;
}

var merged = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

if (target != null)
{
merged.UnionWith(target.Where(x => x != null));
}

if (source != null)
{
merged.UnionWith(source.Where(x => x != null));
}

return merged.OrderBy(x => x, StringComparer.OrdinalIgnoreCase).ToList();
}

private static IList<ActorInfo> MergeAndNormalizeSuppliers(IList<ActorInfo> target, IList<ActorInfo> source)
{
if (target == null && source == null)
{
return null;
}

var merged = new HashSet<ActorInfo>();

if (target != null)
{
merged.UnionWith(target.Where(s => s != null));
}

if (source != null)
{
merged.UnionWith(source.Where(s => s != null));
}

return merged.OrderBy(s => s.Name).ThenBy(s => s.Type).ToList();
}

/// <summary>
/// Merges a group of <see cref="DetectedComponent"/>s that share the same <see cref="TypedComponent.Id"/>
/// into a single entry.
/// </summary>
private static DetectedComponent MergeDetectedComponentGroup(IEnumerable<DetectedComponent> grouping)
{
var winner = grouping.First();

foreach (var component in grouping.Skip(1))
{
MergeComponentMetadata(source: component, target: winner);
}

return detectedComponents;
return winner;
}

public IEnumerable<string> GetSkippedComponents()
Expand Down
33 changes: 24 additions & 9 deletions ...mponentDetection.Orchestrator/Services/GraphTranslation/DefaultGraphTranslationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ private static ConcurrentHashSet<string> MergeTargetFrameworks(ConcurrentHashSet
return left;
}

/// <summary>
/// Checks whether the graph contains the component by its full Id, or by its BaseId
/// when the component is a rich entry (Id != BaseId) whose bare counterpart was registered in the graph.
/// </summary>
private static bool GraphContainsComponent(IDependencyGraph graph, TypedComponent component)
{
return graph.Contains(component.Id) ||
(component.Id != component.BaseId && graph.Contains(component.BaseId));
}

private void LogComponentScopeTelemetry(List<DetectedComponent> components)
{
using var record = new DetectedComponentScopeRecord();
Expand Down Expand Up @@ -126,25 +136,30 @@ private IEnumerable<DetectedComponent> GatherSetOfDetectedComponentsUnmerged(IEn
}

// Information about each component is relative to all of the graphs it is present in, so we take all graphs containing a given component and apply the graph data.
foreach (var graphKvp in dependencyGraphsByLocation.Where(x => x.Value.Contains(component.Component.Id)))
foreach (var graphKvp in dependencyGraphsByLocation.Where(x => GraphContainsComponent(x.Value, component.Component)))
{
var location = graphKvp.Key;
var dependencyGraph = graphKvp.Value;

// Determine the Id stored in this graph — may be the rich Id or the bare BaseId.
var graphComponentId = dependencyGraph.Contains(component.Component.Id)
? component.Component.Id
: component.Component.BaseId;

// Calculate roots of the component
var rootStartTime = DateTime.UtcNow;
this.AddRootsToDetectedComponent(component, dependencyGraph, componentRecorder);
this.AddRootsToDetectedComponent(component, graphComponentId, dependencyGraph, componentRecorder);
var rootEndTime = DateTime.UtcNow;
totalTimeToAddRoots += rootEndTime - rootStartTime;

// Calculate Ancestors of the component
var ancestorStartTime = DateTime.UtcNow;
this.AddAncestorsToDetectedComponent(component, dependencyGraph, componentRecorder);
this.AddAncestorsToDetectedComponent(component, graphComponentId, dependencyGraph, componentRecorder);
var ancestorEndTime = DateTime.UtcNow;
totalTimeToAddAncestors += ancestorEndTime - ancestorStartTime;

component.DevelopmentDependency = this.MergeDevDependency(component.DevelopmentDependency, dependencyGraph.IsDevelopmentDependency(component.Component.Id));
component.DependencyScope = DependencyScopeComparer.GetMergedDependencyScope(component.DependencyScope, dependencyGraph.GetDependencyScope(component.Component.Id));
component.DevelopmentDependency = this.MergeDevDependency(component.DevelopmentDependency, dependencyGraph.IsDevelopmentDependency(graphComponentId));
component.DependencyScope = DependencyScopeComparer.GetMergedDependencyScope(component.DependencyScope, dependencyGraph.GetDependencyScope(graphComponentId));
component.DetectedBy = detector;

// Experiments uses this service to build the dependency graph for analysis. In this case, we do not want to update the locations of the component.
Expand Down Expand Up @@ -269,26 +284,26 @@ private DetectedComponent MergeComponents(IEnumerable<DetectedComponent> enumera
return firstComponent;
}

private void AddRootsToDetectedComponent(DetectedComponent detectedComponent, IDependencyGraph dependencyGraph, IComponentRecorder componentRecorder)
private void AddRootsToDetectedComponent(DetectedComponent detectedComponent, string graphComponentId, IDependencyGraph dependencyGraph, IComponentRecorder componentRecorder)
{
detectedComponent.DependencyRoots ??= new HashSet<TypedComponent>(new ComponentComparer());
if (dependencyGraph == null)
{
return;
}

detectedComponent.DependencyRoots.UnionWith(dependencyGraph.GetRootsAsTypedComponents(detectedComponent.Component.Id, componentRecorder.GetComponent));
detectedComponent.DependencyRoots.UnionWith(dependencyGraph.GetRootsAsTypedComponents(graphComponentId, componentRecorder.GetComponent));
}

private void AddAncestorsToDetectedComponent(DetectedComponent detectedComponent, IDependencyGraph dependencyGraph, IComponentRecorder componentRecorder)
private void AddAncestorsToDetectedComponent(DetectedComponent detectedComponent, string graphComponentId, IDependencyGraph dependencyGraph, IComponentRecorder componentRecorder)
{
detectedComponent.AncestralDependencyRoots ??= new HashSet<TypedComponent>(new ComponentComparer());
if (dependencyGraph == null)
{
return;
}

detectedComponent.AncestralDependencyRoots.UnionWith(dependencyGraph.GetAncestorsAsTypedComponents(detectedComponent.Component.Id, componentRecorder.GetComponent));
detectedComponent.AncestralDependencyRoots.UnionWith(dependencyGraph.GetAncestorsAsTypedComponents(graphComponentId, componentRecorder.GetComponent));
}

private HashSet<string> MakeFilePathsRelative(ILogger logger, DirectoryInfo rootDirectory, HashSet<string> filePaths)
Expand Down
132 changes: 132 additions & 0 deletions test/Microsoft.ComponentDetection.Common.Tests/ComponentRecorderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,4 +428,136 @@ public void GetAllDependencyGraphs_ReturnedGraphsAreImmutable()
Action attemptedAdd = () => asCollection.Add("should't work");
attemptedAdd.Should().Throw<NotSupportedException>();
}

[TestMethod]
public void GetDetectedComponents_BareAndRichAcrossFiles_BareSubsumedIntoRich()
{
var recorder1 = this.componentRecorder.CreateSingleFileComponentRecorder("package.json");
var recorder2 = this.componentRecorder.CreateSingleFileComponentRecorder("package-lock.json");

var bareComponent = new DetectedComponent(new NpmComponent("lodash", "4.17.23"));
var richComponent = new DetectedComponent(new NpmComponent("lodash", "4.17.23") { DownloadUrl = new Uri("https://registry.npmjs.org/lodash/-/lodash-4.17.23.tgz") });

recorder1.RegisterUsage(bareComponent);
recorder2.RegisterUsage(richComponent);

var results = this.componentRecorder.GetDetectedComponents().ToList();

// Only the rich entry should remain
results.Should().ContainSingle();
results[0].Component.Id.Should().Be(richComponent.Component.Id);
results[0].Component.Id.Should().NotBe(bareComponent.Component.Id);
}

[TestMethod]
public void GetDetectedComponents_BareAndMultipleRichAcrossFiles_BareMergesIntoAllRich()
{
var recorder1 = this.componentRecorder.CreateSingleFileComponentRecorder("package.json");
var recorder2 = this.componentRecorder.CreateSingleFileComponentRecorder("lockfile-a.json");
var recorder3 = this.componentRecorder.CreateSingleFileComponentRecorder("lockfile-b.json");

var bareComponent = new DetectedComponent(new NpmComponent("lodash", "4.17.23"))
{
LicensesConcluded = ["MIT"],
};

var richA = new DetectedComponent(new NpmComponent("lodash", "4.17.23") { DownloadUrl = new Uri("https://registry-a.example.com/lodash-4.17.23.tgz") });
var richB = new DetectedComponent(new NpmComponent("lodash", "4.17.23") { DownloadUrl = new Uri("https://registry-b.example.com/lodash-4.17.23.tgz") });

recorder1.RegisterUsage(bareComponent);
recorder2.RegisterUsage(richA);
recorder3.RegisterUsage(richB);

var results = this.componentRecorder.GetDetectedComponents().ToList();

// Two rich entries, bare dropped
results.Should().HaveCount(2);
results.Should().NotContain(c => c.Component.Id == bareComponent.Component.Id);

// Bare's license merged into both rich entries
results.Should().OnlyContain(c => c.LicensesConcluded != null && c.LicensesConcluded.Contains("MIT"));
}

[TestMethod]
public void GetDetectedComponents_TwoRichDifferentUrls_BothKeptSeparate()
{
var recorder1 = this.componentRecorder.CreateSingleFileComponentRecorder("lockfile-a.json");
var recorder2 = this.componentRecorder.CreateSingleFileComponentRecorder("lockfile-b.json");

var richA = new DetectedComponent(new NpmComponent("lodash", "4.17.23") { DownloadUrl = new Uri("https://registry-a.example.com/lodash-4.17.23.tgz") });
var richB = new DetectedComponent(new NpmComponent("lodash", "4.17.23") { DownloadUrl = new Uri("https://registry-b.example.com/lodash-4.17.23.tgz") });

recorder1.RegisterUsage(richA);
recorder2.RegisterUsage(richB);

var results = this.componentRecorder.GetDetectedComponents().ToList();

results.Should().HaveCount(2);
results.Should().Contain(c => c.Component.Id == richA.Component.Id);
results.Should().Contain(c => c.Component.Id == richB.Component.Id);
}

[TestMethod]
public void GetDetectedComponents_BareOnlyAcrossFiles_MergesIntoSingleBare()
{
var recorder1 = this.componentRecorder.CreateSingleFileComponentRecorder("package.json");
var recorder2 = this.componentRecorder.CreateSingleFileComponentRecorder("other-package.json");

var bare1 = new DetectedComponent(new NpmComponent("lodash", "4.17.23"))
{
LicensesConcluded = ["MIT"],
};

var bare2 = new DetectedComponent(new NpmComponent("lodash", "4.17.23"))
{
LicensesConcluded = ["Apache-2.0"],
};

recorder1.RegisterUsage(bare1);
recorder2.RegisterUsage(bare2);

var results = this.componentRecorder.GetDetectedComponents().ToList();

// Same Id → merged into one
results.Should().ContainSingle();
results[0].LicensesConcluded.Should().Contain("MIT");
results[0].LicensesConcluded.Should().Contain("Apache-2.0");
}

[TestMethod]
public void GetDetectedComponents_BareAndRich_MetadataMergedCorrectly()
{
var recorder1 = this.componentRecorder.CreateSingleFileComponentRecorder("package.json");
var recorder2 = this.componentRecorder.CreateSingleFileComponentRecorder("package-lock.json");

var bareComponent = new DetectedComponent(new NpmComponent("lodash", "4.17.23"))
{
LicensesConcluded = ["MIT"],
Suppliers = [new ActorInfo { Name = "Lodash Team", Type = "Organization" }],
};

var richComponent = new DetectedComponent(new NpmComponent("lodash", "4.17.23") { DownloadUrl = new Uri("https://registry.npmjs.org/lodash/-/lodash-4.17.23.tgz") })
{
LicensesConcluded = ["Apache-2.0"],
Suppliers = [new ActorInfo { Name = "Contoso", Type = "Organization" }],
};

recorder1.RegisterUsage(bareComponent);
recorder2.RegisterUsage(richComponent);

var results = this.componentRecorder.GetDetectedComponents().ToList();

results.Should().ContainSingle();
var result = results[0];

// Licenses from both bare and rich should be merged
result.LicensesConcluded.Should().HaveCount(2);
result.LicensesConcluded.Should().Contain("Apache-2.0");
result.LicensesConcluded.Should().Contain("MIT");

// Suppliers from both should be merged
result.Suppliers.Should().HaveCount(2);
result.Suppliers.Should().Contain(s => s.Name == "Lodash Team");
result.Suppliers.Should().Contain(s => s.Name == "Contoso");
}
}
Loading
Loading