diff --git a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java index 3f2fbd8d52..bb3475cf82 100644 --- a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java @@ -516,9 +516,16 @@ public Project updateProject(Project transientProject, boolean commitIndex) { final Project project = getObjectByUuid(Project.class, transientProject.getUuid()); Project oldParent = project.getParent(); + // Resolve the new parent from DB before making scheduling decisions. + // transientProject.getParent() is a stub from the API request with only UUID populated, + // so getCollectionLogic() on it always returns NONE. + Project resolvedNewParent = null; + if (transientProject.getParent() != null && transientProject.getParent().getUuid() != null) { + resolvedNewParent = getObjectByUuid(Project.class, transientProject.getParent().getUuid()); + } boolean scheduleProjectMetricsUpdate = this.needScheduleProjectMetricsUpdate(project, transientProject); - boolean scheduleParentMetricsUpdate = this.needScheduleParentMetricsUpdate(transientProject, scheduleProjectMetricsUpdate); - boolean scheduleOldParentMetricsUpdate = this.needScheduleOldParentMetricsUpdate(oldParent, transientProject); + boolean scheduleParentMetricsUpdate = this.needScheduleParentMetricsUpdate(resolvedNewParent, scheduleProjectMetricsUpdate); + boolean scheduleOldParentMetricsUpdate = this.needScheduleOldParentMetricsUpdate(oldParent, resolvedNewParent); project.setAuthors(transientProject.getAuthors()); project.setPublisher(transientProject.getPublisher()); @@ -557,20 +564,19 @@ public Project updateProject(Project transientProject, boolean commitIndex) { } project.setIsLatest(transientProject.isLatest()); - if (transientProject.getParent() != null && transientProject.getParent().getUuid() != null) { - if (project.getUuid().equals(transientProject.getParent().getUuid())){ + if (resolvedNewParent != null) { + if (project.getUuid().equals(resolvedNewParent.getUuid())){ throw new IllegalArgumentException("A project cannot select itself as a parent"); } - Project parent = getObjectByUuid(Project.class, transientProject.getParent().getUuid()); - if (!Boolean.TRUE.equals(parent.isActive())){ + if (!Boolean.TRUE.equals(resolvedNewParent.isActive())){ throw new IllegalArgumentException("An inactive project cannot be selected as a parent"); - } else if (isChildOf(parent, transientProject.getUuid())){ + } else if (isChildOf(resolvedNewParent, transientProject.getUuid())){ throw new IllegalArgumentException("The new parent project cannot be a child of the current project."); } else { - project.setParent(parent); + project.setParent(resolvedNewParent); } - project.setParent(parent); - }else { + project.setParent(resolvedNewParent); + } else { project.setParent(null); } @@ -621,17 +627,17 @@ private boolean needScheduleProjectMetricsUpdate(Project project, Project transi /** * if parent is collection schedule an update, unless this project itself is scheduled already since that will trigger a parent update, too */ - private boolean needScheduleParentMetricsUpdate(Project transientProject, boolean scheduleProjectMetricsUpdate) { + private boolean needScheduleParentMetricsUpdate(Project newParent, boolean scheduleProjectMetricsUpdate) { return !scheduleProjectMetricsUpdate - && transientProject.getParent() != null - && transientProject.getParent().getCollectionLogic() != ProjectCollectionLogic.NONE; + && newParent != null + && newParent.getCollectionLogic() != ProjectCollectionLogic.NONE; } /** * if project gets a new parent and old parent was collection, we need to update old parent's metrics */ - private boolean needScheduleOldParentMetricsUpdate(Project oldParent, Project transientProject) { - return oldParent != transientProject.getParent() + private boolean needScheduleOldParentMetricsUpdate(Project oldParent, Project newParent) { + return oldParent != newParent && oldParent != null && oldParent.getCollectionLogic() != ProjectCollectionLogic.NONE; } @@ -949,7 +955,17 @@ private static Set parseDirectDependenciesUuids( */ @Override public void recursivelyDelete(final Project project, final boolean commitIndex) { - Project parent = project.getParent(); + // Reload the parent with collectionLogic eagerly fetched before any deletions occur. + // collectionLogic is not in the default fetch group, so accessing it after the PM + // state is modified by cascading deletes risks a silent lazy-load failure (returning NONE). + final Project parent; + if (project.getParent() != null) { + try (var ignored = new ScopedCustomization(pm).withFetchGroup(Project.FetchGroup.METRICS_UPDATE.name())) { + parent = pm.getObjectById(Project.class, project.getParent().getId()); + } + } else { + parent = null; + } if (project.getChildren() != null) { for (final Project child: project.getChildren()) { @@ -1019,6 +1035,17 @@ public void deleteProjectsByUUIDs(Collection uuids) { throw ProjectOperationException.forDeletion(errorByUUID); } + // Collect parent collection projects that need metrics updates after deletion. + // Exclude parents that are themselves being deleted. + final Set deleteUuids = uuids instanceof Set ? (Set) uuids : Set.copyOf(uuids); + final Set collectionParentUuids = projects.stream() + .map(Project::getParent) + .filter(parent -> parent != null + && parent.getCollectionLogic() != ProjectCollectionLogic.NONE + && !deleteUuids.contains(parent.getUuid())) + .map(Project::getUuid) + .collect(Collectors.toSet()); + Long[] projectIDsArray = accessibleProjectIds.toArray(Long[]::new); String commaSeparatedProjectIDs = accessibleProjectIds.stream().map(String::valueOf).collect(Collectors.joining(",")); var queryParameter = DbUtil.isMssql() ? commaSeparatedProjectIDs : projectIDsArray; @@ -1325,6 +1352,10 @@ WHERE PROJECT.ID IN (SELECT value FROM STRING_SPLIT(?, ',')) executeAndCloseWithArray(sqlQuery, queryParameter); } }); + + for (final UUID parentUuid : collectionParentUuids) { + Event.dispatch(new ProjectMetricsUpdateEvent(parentUuid)); + } } /** diff --git a/src/test/java/org/dependencytrack/persistence/ProjectQueryManagerTest.java b/src/test/java/org/dependencytrack/persistence/ProjectQueryManagerTest.java index 05cf9a51bd..d704a2209c 100644 --- a/src/test/java/org/dependencytrack/persistence/ProjectQueryManagerTest.java +++ b/src/test/java/org/dependencytrack/persistence/ProjectQueryManagerTest.java @@ -32,7 +32,9 @@ import java.util.Date; import java.util.List; +import java.util.UUID; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.Mockito.times; @@ -132,4 +134,62 @@ public void testCloneProjectMetricUpdate() throws Exception { } } + @Test + void testRecursivelyDeleteDispatchesMetricsUpdateForCollectionParent() { + final Project collectionParent = qm.createProject("Collection", null, "1.0", null, null, null, true, false); + final Project detachedParent = qm.detach(Project.class, collectionParent.getId()); + detachedParent.setCollectionLogic(ProjectCollectionLogic.AGGREGATE_DIRECT_CHILDREN); + qm.updateProject(detachedParent, false); + + final Project child = qm.createProject("Child", null, "1.0", null, collectionParent, null, true, false); + + try (MockedStatic mockedEvent = Mockito.mockStatic(Event.class)) { + qm.recursivelyDelete(child, true); + + final ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(Event.class); + mockedEvent.verify(() -> Event.dispatch(eventCaptor.capture()), Mockito.atLeastOnce()); + + final List dispatchedUuids = eventCaptor.getAllValues().stream() + .filter(e -> e instanceof ProjectMetricsUpdateEvent) + .map(e -> ((ProjectMetricsUpdateEvent) e).getUuid()) + .toList(); + assertThat(dispatchedUuids).contains(collectionParent.getUuid()); + } + } + + @Test + void testDeleteProjectsByUUIDsDispatchesMetricsUpdateForCollectionParent() { + final Project collectionParent = qm.createProject("Collection", null, "1.0", null, null, null, true, false); + final Project detachedParent = qm.detach(Project.class, collectionParent.getId()); + detachedParent.setCollectionLogic(ProjectCollectionLogic.AGGREGATE_DIRECT_CHILDREN); + qm.updateProject(detachedParent, false); + + final Project child = qm.createProject("Child", null, "1.0", null, collectionParent, null, true, false); + + try (MockedStatic mockedEvent = Mockito.mockStatic(Event.class)) { + qm.deleteProjectsByUUIDs(List.of(child.getUuid())); + + final ArgumentCaptor eventCaptor = + ArgumentCaptor.forClass(ProjectMetricsUpdateEvent.class); + mockedEvent.verify(() -> Event.dispatch(eventCaptor.capture()), times(1)); + + assertThat(eventCaptor.getValue().getUuid()).isEqualTo(collectionParent.getUuid()); + } + } + + @Test + void testDeleteProjectsByUUIDsDoesNotDispatchMetricsUpdateWhenParentAlsoDeleted() { + final Project collectionParent = qm.createProject("Collection", null, "1.0", null, null, null, true, false); + final Project detachedParent = qm.detach(Project.class, collectionParent.getId()); + detachedParent.setCollectionLogic(ProjectCollectionLogic.AGGREGATE_DIRECT_CHILDREN); + qm.updateProject(detachedParent, false); + + final Project child = qm.createProject("Child", null, "1.0", null, collectionParent, null, true, false); + + try (MockedStatic mockedEvent = Mockito.mockStatic(Event.class)) { + qm.deleteProjectsByUUIDs(List.of(collectionParent.getUuid(), child.getUuid())); + + mockedEvent.verify(() -> Event.dispatch(Mockito.any(ProjectMetricsUpdateEvent.class)), times(0)); + } + } } \ No newline at end of file