From 164847a87ad747c3128793c7b0fb01d4b06067cf Mon Sep 17 00:00:00 2001 From: nscuro Date: Mon, 15 Jul 2024 20:54:27 +0200 Subject: [PATCH] Ensure no unique constraint violation for `ProjectMetadata` Adds regression test for #3895. The behavior itself does not reproduce on `master`, but does with `4.11.5`. Wraps the project cloning in a transaction to make it more reliable. Adds MDC variables for `CloneProjectTask` (#3234). Fixes #3895 Signed-off-by: nscuro --- .../org/dependencytrack/common/MdcKeys.java | 1 + .../persistence/ProjectQueryManager.java | 255 +++++++++--------- .../tasks/CloneProjectTask.java | 31 ++- .../tasks/BomUploadProcessingTaskTest.java | 77 ++++++ 4 files changed, 233 insertions(+), 131 deletions(-) diff --git a/src/main/java/org/dependencytrack/common/MdcKeys.java b/src/main/java/org/dependencytrack/common/MdcKeys.java index 7c7254baae..2dd46bb231 100644 --- a/src/main/java/org/dependencytrack/common/MdcKeys.java +++ b/src/main/java/org/dependencytrack/common/MdcKeys.java @@ -30,6 +30,7 @@ public final class MdcKeys { public static final String MDC_BOM_SPEC_VERSION = "bomSpecVersion"; public static final String MDC_BOM_UPLOAD_TOKEN = "bomUploadToken"; public static final String MDC_BOM_VERSION = "bomVersion"; + public static final String MDC_EVENT_TOKEN = "eventToken"; public static final String MDC_PROJECT_NAME = "projectName"; public static final String MDC_PROJECT_UUID = "projectUuid"; public static final String MDC_PROJECT_VERSION = "projectVersion"; diff --git a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java index d6f81cdd2a..1dfdb5ab27 100644 --- a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java @@ -578,151 +578,160 @@ public Project updateProject(Project transientProject, boolean commitIndex) { } @Override - public Project clone(UUID from, String newVersion, boolean includeTags, boolean includeProperties, - boolean includeComponents, boolean includeServices, boolean includeAuditHistory, - boolean includeACL, boolean includePolicyViolations) { - final Project source = getObjectByUuid(Project.class, from, Project.FetchGroup.ALL.name()); - if (source == null) { - LOGGER.warn("Project with UUID %s was supposed to be cloned, but it does not exist anymore".formatted(from)); - return null; - } - if (doesProjectExist(source.getName(), newVersion)) { - // Project cloning is an asynchronous process. When receiving the clone request, we already perform - // this check. It is possible though that a project with the new version is created synchronously - // between the clone event being dispatched, and it being processed. - LOGGER.warn("Project %s was supposed to be cloned to version %s, but that version already exists" - .formatted(source, newVersion)); - return null; - } - Project project = new Project(); - project.setAuthor(source.getAuthor()); - project.setManufacturer(source.getManufacturer()); - project.setSupplier(source.getSupplier()); - project.setPublisher(source.getPublisher()); - project.setGroup(source.getGroup()); - project.setName(source.getName()); - project.setDescription(source.getDescription()); - project.setVersion(newVersion); - project.setClassifier(source.getClassifier()); - project.setActive(source.isActive()); - project.setCpe(source.getCpe()); - project.setPurl(source.getPurl()); - project.setSwidTagId(source.getSwidTagId()); - if (includeComponents && includeServices) { - project.setDirectDependencies(source.getDirectDependencies()); - } - project.setParent(source.getParent()); - project = persist(project); - - if (source.getMetadata() != null) { - final var metadata = new ProjectMetadata(); - metadata.setProject(project); - metadata.setAuthors(source.getMetadata().getAuthors()); - metadata.setSupplier(source.getMetadata().getSupplier()); - persist(metadata); - } + public Project clone( + final UUID from, + final String newVersion, + final boolean includeTags, + final boolean includeProperties, + final boolean includeComponents, + final boolean includeServices, + final boolean includeAuditHistory, + final boolean includeACL, + final boolean includePolicyViolations + ) { + final Project clonedProject = callInTransaction(() -> { + final Project source = getObjectByUuid(Project.class, from, Project.FetchGroup.ALL.name()); + if (source == null) { + LOGGER.warn("Project was supposed to be cloned, but it does not exist anymore"); + return null; + } + if (doesProjectExist(source.getName(), newVersion)) { + // Project cloning is an asynchronous process. When receiving the clone request, we already perform + // this check. It is possible though that a project with the new version is created synchronously + // between the clone event being dispatched, and it being processed. + LOGGER.warn("Project was supposed to be cloned to version %s, but that version already exists".formatted(newVersion)); + return null; + } + Project project = new Project(); + project.setAuthor(source.getAuthor()); + project.setManufacturer(source.getManufacturer()); + project.setSupplier(source.getSupplier()); + project.setPublisher(source.getPublisher()); + project.setGroup(source.getGroup()); + project.setName(source.getName()); + project.setDescription(source.getDescription()); + project.setVersion(newVersion); + project.setClassifier(source.getClassifier()); + project.setActive(source.isActive()); + project.setCpe(source.getCpe()); + project.setPurl(source.getPurl()); + project.setSwidTagId(source.getSwidTagId()); + if (includeComponents && includeServices) { + project.setDirectDependencies(source.getDirectDependencies()); + } + project.setParent(source.getParent()); + project = persist(project); + + if (source.getMetadata() != null) { + final var metadata = new ProjectMetadata(); + metadata.setProject(project); + metadata.setAuthors(source.getMetadata().getAuthors()); + metadata.setSupplier(source.getMetadata().getSupplier()); + persist(metadata); + } - if (includeTags) { - for (final Tag tag: source.getTags()) { - tag.getProjects().add(project); - persist(tag); + if (includeTags) { + for (final Tag tag : source.getTags()) { + tag.getProjects().add(project); + persist(tag); + } } - } - if (includeProperties && source.getProperties() != null) { - for (final ProjectProperty sourceProperty: source.getProperties()) { - final ProjectProperty property = new ProjectProperty(); - property.setProject(project); - property.setPropertyType(sourceProperty.getPropertyType()); - property.setGroupName(sourceProperty.getGroupName()); - property.setPropertyName(sourceProperty.getPropertyName()); - property.setPropertyValue(sourceProperty.getPropertyValue()); - property.setDescription(sourceProperty.getDescription()); - persist(property); + if (includeProperties && source.getProperties() != null) { + for (final ProjectProperty sourceProperty : source.getProperties()) { + final ProjectProperty property = new ProjectProperty(); + property.setProject(project); + property.setPropertyType(sourceProperty.getPropertyType()); + property.setGroupName(sourceProperty.getGroupName()); + property.setPropertyName(sourceProperty.getPropertyName()); + property.setPropertyValue(sourceProperty.getPropertyValue()); + property.setDescription(sourceProperty.getDescription()); + persist(property); + } } - } - final Map clonedComponents = new HashMap<>(); - if (includeComponents) { - final List sourceComponents = getAllComponents(source); - if (sourceComponents != null) { - for (final Component sourceComponent: sourceComponents) { - final Component clonedComponent = cloneComponent(sourceComponent, project, false); - // Add vulnerabilties and finding attribution from the source component to the cloned component - for (Vulnerability vuln: sourceComponent.getVulnerabilities()) { - final FindingAttribution sourceAttribution = this.getFindingAttribution(vuln, sourceComponent); - this.addVulnerability(vuln, clonedComponent, sourceAttribution.getAnalyzerIdentity(), sourceAttribution.getAlternateIdentifier(), sourceAttribution.getReferenceUrl(), sourceAttribution.getAttributedOn()); + final Map clonedComponents = new HashMap<>(); + if (includeComponents) { + final List sourceComponents = getAllComponents(source); + if (sourceComponents != null) { + for (final Component sourceComponent : sourceComponents) { + final Component clonedComponent = cloneComponent(sourceComponent, project, false); + // Add vulnerabilties and finding attribution from the source component to the cloned component + for (Vulnerability vuln : sourceComponent.getVulnerabilities()) { + final FindingAttribution sourceAttribution = this.getFindingAttribution(vuln, sourceComponent); + this.addVulnerability(vuln, clonedComponent, sourceAttribution.getAnalyzerIdentity(), sourceAttribution.getAlternateIdentifier(), sourceAttribution.getReferenceUrl(), sourceAttribution.getAttributedOn()); + } + clonedComponents.put(sourceComponent.getId(), clonedComponent); } - clonedComponents.put(sourceComponent.getId(), clonedComponent); } } - } - if (includeServices) { - final List sourceServices = getAllServiceComponents(source); - if (sourceServices != null) { - for (final ServiceComponent sourceService : sourceServices) { - cloneServiceComponent(sourceService, project, false); + if (includeServices) { + final List sourceServices = getAllServiceComponents(source); + if (sourceServices != null) { + for (final ServiceComponent sourceService : sourceServices) { + cloneServiceComponent(sourceService, project, false); + } } } - } - if (includeAuditHistory && includeComponents) { - final List analyses = super.getAnalyses(source); - if (analyses != null) { - for (final Analysis sourceAnalysis: analyses) { - Analysis analysis = new Analysis(); - analysis.setAnalysisState(sourceAnalysis.getAnalysisState()); - final Component clonedComponent = clonedComponents.get(sourceAnalysis.getComponent().getId()); - if (clonedComponent == null) { - break; - } - analysis.setComponent(clonedComponent); - analysis.setVulnerability(sourceAnalysis.getVulnerability()); - analysis.setSuppressed(sourceAnalysis.isSuppressed()); - analysis.setAnalysisResponse(sourceAnalysis.getAnalysisResponse()); - analysis.setAnalysisJustification(sourceAnalysis.getAnalysisJustification()); - analysis.setAnalysisState(sourceAnalysis.getAnalysisState()); - analysis.setAnalysisDetails(sourceAnalysis.getAnalysisDetails()); - analysis = persist(analysis); - if (sourceAnalysis.getAnalysisComments() != null) { - for (final AnalysisComment sourceComment: sourceAnalysis.getAnalysisComments()) { - final AnalysisComment analysisComment = new AnalysisComment(); - analysisComment.setAnalysis(analysis); - analysisComment.setTimestamp(sourceComment.getTimestamp()); - analysisComment.setComment(sourceComment.getComment()); - analysisComment.setCommenter(sourceComment.getCommenter()); - persist(analysisComment); + if (includeAuditHistory && includeComponents) { + final List analyses = super.getAnalyses(source); + if (analyses != null) { + for (final Analysis sourceAnalysis : analyses) { + Analysis analysis = new Analysis(); + analysis.setAnalysisState(sourceAnalysis.getAnalysisState()); + final Component clonedComponent = clonedComponents.get(sourceAnalysis.getComponent().getId()); + if (clonedComponent == null) { + break; + } + analysis.setComponent(clonedComponent); + analysis.setVulnerability(sourceAnalysis.getVulnerability()); + analysis.setSuppressed(sourceAnalysis.isSuppressed()); + analysis.setAnalysisResponse(sourceAnalysis.getAnalysisResponse()); + analysis.setAnalysisJustification(sourceAnalysis.getAnalysisJustification()); + analysis.setAnalysisState(sourceAnalysis.getAnalysisState()); + analysis.setAnalysisDetails(sourceAnalysis.getAnalysisDetails()); + analysis = persist(analysis); + if (sourceAnalysis.getAnalysisComments() != null) { + for (final AnalysisComment sourceComment : sourceAnalysis.getAnalysisComments()) { + final AnalysisComment analysisComment = new AnalysisComment(); + analysisComment.setAnalysis(analysis); + analysisComment.setTimestamp(sourceComment.getTimestamp()); + analysisComment.setComment(sourceComment.getComment()); + analysisComment.setCommenter(sourceComment.getCommenter()); + persist(analysisComment); + } } } } } - } - if (includeACL) { - List accessTeams = source.getAccessTeams(); - if (!CollectionUtils.isEmpty(accessTeams)) { - project.setAccessTeams(new ArrayList<>(accessTeams)); + if (includeACL) { + List accessTeams = source.getAccessTeams(); + if (!CollectionUtils.isEmpty(accessTeams)) { + project.setAccessTeams(new ArrayList<>(accessTeams)); + } } - } - - if(includeComponents && includePolicyViolations){ - final List sourcePolicyViolations = getAllPolicyViolations(source); - if(sourcePolicyViolations != null){ - for(final PolicyViolation policyViolation: sourcePolicyViolations){ - final Component destinationComponent = clonedComponents.get(policyViolation.getComponent().getId()); - final PolicyViolation clonedPolicyViolation = clonePolicyViolation(policyViolation, destinationComponent); - persist(clonedPolicyViolation); - } + + if (includeComponents && includePolicyViolations) { + final List sourcePolicyViolations = getAllPolicyViolations(source); + if (sourcePolicyViolations != null) { + for (final PolicyViolation policyViolation : sourcePolicyViolations) { + final Component destinationComponent = clonedComponents.get(policyViolation.getComponent().getId()); + final PolicyViolation clonedPolicyViolation = clonePolicyViolation(policyViolation, destinationComponent); + persist(clonedPolicyViolation); + } + } } - } - - project = getObjectById(Project.class, project.getId()); - Event.dispatch(new IndexEvent(IndexEvent.Action.CREATE, project)); + return project; + }); + + Event.dispatch(new IndexEvent(IndexEvent.Action.CREATE, clonedProject)); commitSearchIndex(true, Project.class); - return project; + return clonedProject; } /** diff --git a/src/main/java/org/dependencytrack/tasks/CloneProjectTask.java b/src/main/java/org/dependencytrack/tasks/CloneProjectTask.java index 37e2b0f46c..863c46ed58 100644 --- a/src/main/java/org/dependencytrack/tasks/CloneProjectTask.java +++ b/src/main/java/org/dependencytrack/tasks/CloneProjectTask.java @@ -25,9 +25,13 @@ import org.dependencytrack.model.Project; import org.dependencytrack.persistence.QueryManager; import org.dependencytrack.resources.v1.vo.CloneProjectRequest; +import org.slf4j.MDC; import java.util.UUID; +import static org.dependencytrack.common.MdcKeys.MDC_EVENT_TOKEN; +import static org.dependencytrack.common.MdcKeys.MDC_PROJECT_UUID; + public class CloneProjectTask implements Subscriber { private static final Logger LOGGER = Logger.getLogger(CloneProjectTask.class); @@ -36,15 +40,26 @@ public class CloneProjectTask implements Subscriber { * {@inheritDoc} */ public void inform(final Event e) { - if (e instanceof CloneProjectEvent) { - final CloneProjectEvent event = (CloneProjectEvent)e; + if (e instanceof final CloneProjectEvent event) { final CloneProjectRequest request = event.getRequest(); - LOGGER.info("Cloning project: " + request.getProject()); - try (QueryManager qm = new QueryManager()) { - final Project project = qm.clone(UUID.fromString(request.getProject()), - request.getVersion(), request.includeTags(), request.includeProperties(), - request.includeComponents(), request.includeServices(), request.includeAuditHistory(), request.includeACL(), request.includePolicyViolations()); - LOGGER.info("Cloned project: " + request.getProject() + " to " + project.getUuid()); + try (var ignoredMdcProjectUuid = MDC.putCloseable(MDC_PROJECT_UUID, request.getProject()); + var ignoredMdcEventToken = MDC.putCloseable(MDC_EVENT_TOKEN, event.getChainIdentifier().toString()); + final var qm = new QueryManager()) { + LOGGER.info("Cloning project for version %s".formatted(request.getVersion())); + final Project project = qm.clone( + UUID.fromString(request.getProject()), + request.getVersion(), + request.includeTags(), + request.includeProperties(), + request.includeComponents(), + request.includeServices(), + request.includeAuditHistory(), + request.includeACL(), + request.includePolicyViolations() + ); + LOGGER.info("Cloned project for version %s into project %s".formatted(project.getVersion(), project.getUuid())); + } catch (RuntimeException ex) { + LOGGER.error("Failed to clone project", ex); } } } diff --git a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java index 32628e3081..4703e41ae7 100644 --- a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java +++ b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java @@ -1216,6 +1216,83 @@ public void informIssue3957Test() { }); } + @Test + public void informIssue3981Test() { + final var project = new Project(); + project.setName("acme-license-app"); + project.setVersion("1.2.3"); + qm.persist(project); + + byte[] bomBytes = """ + { + "bomFormat": "CycloneDX", + "specVersion": "1.6", + "serialNumber": "urn:uuid:3e671687-395b-41f5-a30f-a58921a69b80", + "version": 1, + "metadata": { + "authors": [ + { + "name": "foo", + "email": "foo@example.com" + } + ] + }, + "components": [ + { + "type": "library", + "name": "acme-lib-x" + } + ] + } + """.getBytes(StandardCharsets.UTF_8); + + var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomBytes); + new BomUploadProcessingTask().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); + NOTIFICATIONS.clear(); + + final Project clonedProject = qm.clone(project.getUuid(), "3.2.1", true, true, true, true, true, true, true); + + bomBytes = """ + { + "bomFormat": "CycloneDX", + "specVersion": "1.6", + "serialNumber": "urn:uuid:3e671687-395b-41f5-a30f-a58921a69b80", + "version": 1, + "metadata": { + "authors": [ + { + "name": "bar", + "email": "bar@example.com" + } + ] + }, + "components": [ + { + "type": "library", + "name": "acme-lib-x" + } + ] + } + """.getBytes(StandardCharsets.UTF_8); + + bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, clonedProject.getId()), bomBytes); + new BomUploadProcessingTask().inform(bomUploadEvent); + awaitBomProcessedNotification(bomUploadEvent); + + qm.getPersistenceManager().evictAll(); + + assertThat(project.getMetadata().getAuthors()).satisfiesExactly(author -> { + assertThat(author.getName()).isEqualTo("foo"); + assertThat(author.getEmail()).isEqualTo("foo@example.com"); + }); + + assertThat(clonedProject.getMetadata().getAuthors()).satisfiesExactly(author -> { + assertThat(author.getName()).isEqualTo("bar"); + assertThat(author.getEmail()).isEqualTo("bar@example.com"); + }); + } + private void awaitBomProcessedNotification(final BomUploadEvent bomUploadEvent) { try { await("BOM Processed Notification")