Skip to content

Commit eb08953

Browse files
mehabmehab
authored
Merge direct dependencies for depgraph if component with same identity repeated over multiple bom-refs (#1943)
* changes added Signed-off-by: Meha Bhargava <meha.bhargava2@gmail.com> * added change to address pipeline failure Signed-off-by: Meha Bhargava <meha.bhargava2@gmail.com> * addressed pr review comments Signed-off-by: Meha Bhargava <meha.bhargava2@gmail.com> --------- Signed-off-by: Meha Bhargava <meha.bhargava2@gmail.com>
1 parent 621aa6a commit eb08953

3 files changed

Lines changed: 147 additions & 44 deletions

File tree

‎apiserver/src/main/java/org/dependencytrack/tasks/ImportBomActivity.java‎

Lines changed: 64 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ private ConsumedBom consumeBom(final org.cyclonedx.proto.v1_6.Bom cdxBom) {
395395
);
396396
}
397397

398-
private record ProcessedBom(
398+
record ProcessedBom(
399399
Project project,
400400
Collection<Component> components,
401401
Collection<ServiceComponent> services
@@ -454,7 +454,8 @@ private ProcessedBom processBom(final ProcessingContext ctx, final ConsumedBom b
454454
processServices(qm, persistentProject, bom.services(), bom.identitiesByBomRef(), bom.bomRefsByIdentity());
455455

456456
LOGGER.info("Processing %d dependency graph entries".formatted(bom.dependencyGraph().asMap().size()));
457-
processDependencyGraph(qm, persistentProject, bom.dependencyGraph(), persistentComponentsByIdentity, bom.identitiesByBomRef());
457+
processDependencyGraph(qm, persistentProject, bom.dependencyGraph(), persistentComponentsByIdentity,
458+
bom.identitiesByBomRef(), bom.bomRefsByIdentity());
458459

459460
recordBomImport(ctx, qm, persistentProject);
460461

@@ -738,7 +739,8 @@ private void processDependencyGraph(
738739
final Project project,
739740
final MultiValuedMap<String, String> dependencyGraph,
740741
final Map<ComponentIdentity, Component> componentsByIdentity,
741-
final Map<String, ComponentIdentity> identitiesByBomRef
742+
final Map<String, ComponentIdentity> identitiesByBomRef,
743+
final MultiValuedMap<ComponentIdentity, String> bomRefsByIdentity
742744
) {
743745
assertPersistent(project, "Project must be persistent");
744746

@@ -750,7 +752,10 @@ private void processDependencyGraph(
750752
is not one of them; Graph will be incomplete because it is not possible to determine its root\
751753
""".formatted(dependencyGraph.size()));
752754
}
753-
final String directDependenciesJson = resolveDirectDependenciesJson(project.getBomRef(), directDependencyBomRefs, identitiesByBomRef);
755+
final String directDependenciesJson = resolveDependenciesJson(
756+
List.of(project.getBomRef()),
757+
sourceBomRef -> sourceBomRef.equals(project.getBomRef()) ? directDependencyBomRefs : null,
758+
identitiesByBomRef);
754759
if (!Objects.equals(directDependenciesJson, project.getDirectDependencies())) {
755760
project.setDirectDependencies(directDependenciesJson);
756761
qm.getPersistenceManager().flush();
@@ -763,25 +768,12 @@ private void processDependencyGraph(
763768
}
764769
}
765770

766-
for (final Map.Entry<String, ComponentIdentity> entry : identitiesByBomRef.entrySet()) {
767-
final String componentBomRef = entry.getKey();
768-
final Collection<String> directDependencyBomRefs = dependencyGraph.get(componentBomRef);
769-
final String directDependenciesJson = resolveDirectDependenciesJson(componentBomRef, directDependencyBomRefs, identitiesByBomRef);
770-
771-
final ComponentIdentity dependencyIdentity = identitiesByBomRef.get(entry.getKey());
772-
final Component component = componentsByIdentity.get(dependencyIdentity);
773-
// TODO: Check servicesByIdentity when persistentComponent is null
774-
// We do not currently store directDependencies for ServiceComponent
775-
if (component != null) {
776-
assertPersistent(component, "Component must be persistent");
777-
if (!Objects.equals(directDependenciesJson, component.getDirectDependencies())) {
778-
component.setDirectDependencies(directDependenciesJson);
779-
}
780-
} else {
781-
LOGGER.warn("""
782-
Unable to resolve component identity %s to a persistent component; \
783-
As a result, the dependency graph will likely be incomplete\
784-
""".formatted(dependencyIdentity.toJSON()));
771+
for (final Component component : componentsByIdentity.values()) {
772+
assertPersistent(component, "Component must be persistent");
773+
final String mergedDirectDependenciesJson = resolveMergedDirectDependenciesJson(
774+
component, dependencyGraph, identitiesByBomRef, bomRefsByIdentity);
775+
if (!Objects.equals(mergedDirectDependenciesJson, component.getDirectDependencies())) {
776+
component.setDirectDependencies(mergedDirectDependenciesJson);
785777
}
786778
}
787779

@@ -807,34 +799,63 @@ private static void recordBomImport(final ProcessingContext ctx, final QueryMana
807799
project.setLastBomImportFormat("%s %s".formatted(ctx.bomFormat.getFormatShortName(), ctx.bomSpecVersion));
808800
}
809801

810-
private String resolveDirectDependenciesJson(
811-
final String dependencyBomRef,
812-
final Collection<String> directDependencyBomRefs,
802+
/**
803+
* Builds {@code directDependencies} JSON for one component by merging direct dependency edges from every BOM ref
804+
* that maps to the same deduplicated identity.
805+
* <p>
806+
* {@link #distinctComponentsByIdentity} records those refs under {@code bomRefsByIdentity} using identities from
807+
* consumption (UUID usually unset). After persist, {@link ComponentIdentity#equals(Object)} includes UUID, so we
808+
* look up with {@code new ComponentIdentity(component, true)} — same structural key as at consumption, same pattern
809+
* as matching existing rows in {@link #processComponents}.
810+
*/
811+
private @Nullable String resolveMergedDirectDependenciesJson(
812+
final Component component,
813+
final MultiValuedMap<String, String> dependencyGraph,
814+
final Map<String, ComponentIdentity> identitiesByBomRef,
815+
final MultiValuedMap<ComponentIdentity, String> bomRefsByIdentity
816+
) {
817+
final Collection<String> sourceBomRefs = bomRefsByIdentity.get(
818+
new ComponentIdentity(component, /* excludeUuid */ true));
819+
if (sourceBomRefs == null || sourceBomRefs.isEmpty()) {
820+
return null;
821+
}
822+
823+
return resolveDependenciesJson(sourceBomRefs, dependencyGraph::get, identitiesByBomRef);
824+
}
825+
826+
private @Nullable String resolveDependenciesJson(
827+
final Collection<String> sourceBomRefs,
828+
final Function<String, Collection<String>> directDependencyBomRefsProvider,
813829
final Map<String, ComponentIdentity> identitiesByBomRef
814830
) {
815-
if (directDependencyBomRefs == null || directDependencyBomRefs.isEmpty()) {
831+
if (sourceBomRefs == null || sourceBomRefs.isEmpty()) {
816832
return null;
817833
}
818834

819835
final var jsonDependencies = Mappers.jsonMapper().createArrayNode();
820836
final var directDependencyIdentitiesSeen = new HashSet<ComponentIdentity>();
821-
for (final String directDependencyBomRef : directDependencyBomRefs) {
822-
final ComponentIdentity directDependencyIdentity = identitiesByBomRef.get(directDependencyBomRef);
823-
if (directDependencyIdentity != null) {
824-
if (!directDependencyIdentitiesSeen.add(directDependencyIdentity)) {
825-
// It's possible that multiple direct dependencies of a project or component
826-
// fall victim to de-duplication. In that case, we can ironically end up with
827-
// duplicate component identities (i.e. duplicate BOM refs).
828-
LOGGER.debug("Omitting duplicate direct dependency %s for BOM ref %s"
829-
.formatted(directDependencyBomRef, dependencyBomRef));
830-
continue;
837+
838+
for (final String sourceBomRef : sourceBomRefs) {
839+
final Collection<String> directDependencyBomRefs = directDependencyBomRefsProvider.apply(sourceBomRef);
840+
if (directDependencyBomRefs == null || directDependencyBomRefs.isEmpty()) {
841+
continue;
842+
}
843+
844+
for (final String directDependencyBomRef : directDependencyBomRefs) {
845+
final ComponentIdentity directDependencyIdentity = identitiesByBomRef.get(directDependencyBomRef);
846+
if (directDependencyIdentity != null) {
847+
if (!directDependencyIdentitiesSeen.add(directDependencyIdentity)) {
848+
LOGGER.debug("Omitting duplicate direct dependency %s for BOM ref %s"
849+
.formatted(directDependencyBomRef, sourceBomRef));
850+
continue;
851+
}
852+
jsonDependencies.add(directDependencyIdentity.toJSON());
853+
} else {
854+
LOGGER.warn("""
855+
Unable to resolve BOM ref %s to a component identity while processing direct \
856+
dependencies of BOM ref %s; As a result, the dependency graph will likely be incomplete\
857+
""".formatted(sourceBomRef, directDependencyBomRef));
831858
}
832-
jsonDependencies.add(directDependencyIdentity.toJSON());
833-
} else {
834-
LOGGER.warn("""
835-
Unable to resolve BOM ref %s to a component identity while processing direct \
836-
dependencies of BOM ref %s; As a result, the dependency graph will likely be incomplete\
837-
""".formatted(dependencyBomRef, directDependencyBomRef));
838859
}
839860
}
840861

‎apiserver/src/test/java/org/dependencytrack/notification/ProcessScheduledNotificationRuleActivityTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,9 @@ void shouldDispatchNewVulnNotification() throws InvalidProtocolBufferException {
233233

234234
qm.getPersistenceManager().evictAll();
235235
final NotificationRule updatedRule = qm.getObjectByUuid(NotificationRule.class, rule.getUuid());
236-
assertThat(updatedRule.getScheduleLastTriggeredAt()).isAfter(Date.from(ruleLastFiredAt));
236+
// DB timestamps may be stored with second precision; tolerate truncated milliseconds.
237+
assertThat(updatedRule.getScheduleLastTriggeredAt())
238+
.isAfterOrEqualTo(Date.from(ruleLastFiredAt.truncatedTo(ChronoUnit.SECONDS)));
237239
assertThat(updatedRule.getScheduleNextTriggerAt()).isAfter(new Date());
238240
}
239241

‎apiserver/src/test/java/org/dependencytrack/tasks/ImportBomActivityTest.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1327,6 +1327,86 @@ void informIssue4455Test() throws Exception {
13271327
});
13281328
}
13291329

1330+
@Test
1331+
void informWithDuplicateBomRefsMappedToSameIdentityMergesDirectDependencies() throws Exception {
1332+
final var project = new Project();
1333+
project.setName("acme-duplicate-bom-ref-app");
1334+
qm.persist(project);
1335+
1336+
final byte[] bomBytes = """
1337+
{
1338+
"bomFormat": "CycloneDX",
1339+
"specVersion": "1.6",
1340+
"version": 1,
1341+
"components": [
1342+
{
1343+
"type": "library",
1344+
"bom-ref": "a1",
1345+
"group": "com.example",
1346+
"name": "a",
1347+
"version": "1.0.0",
1348+
"purl": "pkg:maven/com.example/a@1.0.0"
1349+
},
1350+
{
1351+
"type": "library",
1352+
"bom-ref": "a2",
1353+
"group": "com.example",
1354+
"name": "a",
1355+
"version": "1.0.0",
1356+
"purl": "pkg:maven/com.example/a@1.0.0"
1357+
},
1358+
{
1359+
"type": "library",
1360+
"bom-ref": "b1",
1361+
"group": "com.example",
1362+
"name": "b",
1363+
"version": "1.0.0",
1364+
"purl": "pkg:maven/com.example/b@1.0.0"
1365+
}
1366+
],
1367+
"dependencies": [
1368+
{
1369+
"ref": "a1",
1370+
"dependsOn": ["b1"]
1371+
},
1372+
{
1373+
"ref": "a2",
1374+
"dependsOn": []
1375+
},
1376+
{
1377+
"ref": "b1",
1378+
"dependsOn": []
1379+
}
1380+
]
1381+
}
1382+
""".getBytes(StandardCharsets.UTF_8);
1383+
1384+
final var bomFileMetadata = storeBomFile(bomBytes);
1385+
final var bomUploadToken = UUID.randomUUID();
1386+
activity.execute(null, buildArg(project, bomFileMetadata, bomUploadToken));
1387+
assertBomProcessedNotification();
1388+
1389+
final List<Component> components = qm.getAllComponents(project);
1390+
assertThat(components).hasSize(2);
1391+
1392+
final Component aComponent = components.stream()
1393+
.filter(component -> component.getPurl() != null
1394+
&& "pkg:maven/com.example/a@1.0.0".equals(component.getPurl().canonicalize()))
1395+
.findFirst()
1396+
.orElseThrow();
1397+
final Component bComponent = components.stream()
1398+
.filter(component -> component.getPurl() != null
1399+
&& "pkg:maven/com.example/b@1.0.0".equals(component.getPurl().canonicalize()))
1400+
.findFirst()
1401+
.orElseThrow();
1402+
1403+
assertThat(aComponent.getDirectDependencies()).isNotNull();
1404+
final JsonArray directDependencies = Json.createReader(new StringReader(aComponent.getDirectDependencies())).readArray();
1405+
assertThat(directDependencies).hasSize(1);
1406+
assertThat(directDependencies.getJsonObject(0).getString("uuid"))
1407+
.isEqualTo(bComponent.getUuid().toString());
1408+
}
1409+
13301410
@Test
13311411
void informWithExistingDuplicateComponentPropertiesAndBomWithDuplicateComponentProperties() throws Exception {
13321412
final var project = new Project();

0 commit comments

Comments
 (0)