Skip to content

Commit

Permalink
Merge pull request #233 from DependencyTrack/issue-636
Browse files Browse the repository at this point in the history 
Improve project deletion performance and make it transactional
  • Loading branch information
@nscuro
nscuro authored Jul 19, 2023
2 parents 7105503 + 6ddfd08 commit 4b09783
Show file tree
Hide file tree
Showing 8 changed files with 444 additions and 65 deletions.
61 changes: 43 additions & 18 deletions src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@
import org.dependencytrack.model.RepositoryMetaComponent;
import org.dependencytrack.model.RepositoryType;

import javax.jdo.FetchPlan;
import javax.jdo.PersistenceManager;
import javax.jdo.Query;
import javax.jdo.Transaction;
import javax.json.Json;
import javax.json.JsonArray;
import javax.json.JsonValue;
Expand Down Expand Up @@ -380,7 +380,11 @@ public Component updateComponent(Component transientComponent, boolean commitInd
*/
protected void deleteComponents(Project project) {
final Query<Component> query = pm.newQuery(Component.class, "project == :project");
query.deletePersistentAll(project);
try {
query.deletePersistentAll(project);
} finally {
query.closeAll();
}
}

/**
Expand All @@ -389,26 +393,47 @@ protected void deleteComponents(Project project) {
* @param commitIndex specifies if the search index should be committed (an expensive operation)
*/
public void recursivelyDelete(Component component, boolean commitIndex) {
if (component.getChildren() != null) {
for (final Component child: component.getChildren()) {
final Transaction trx = pm.currentTransaction();
final boolean isJoiningExistingTrx = trx.isActive();
try {
if (!isJoiningExistingTrx) {
trx.begin();
}

for (final Component child : component.getChildren()) {
recursivelyDelete(child, false);
pm.flush();
}
}
pm.getFetchPlan().setDetachmentOptions(FetchPlan.DETACH_LOAD_FIELDS);
try {
// final Component result = pm.getObjectById(Component.class, component.getId());
// Event.dispatch(new IndexEvent(IndexEvent.Action.DELETE, pm.detachCopy(result)));
deleteAnalysisTrail(component);
deleteViolationAnalysisTrail(component);
deleteMetrics(component);
deleteFindingAttributions(component);
deletePolicyViolations(component);
delete(component);

// Use bulk DELETE queries to avoid having to fetch every single object from the database first.
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.AnalysisComment WHERE analysis.component == :component"), component);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.Analysis WHERE component == :component"), component);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.ViolationAnalysisComment WHERE violationAnalysis.component == :component"), component);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.ViolationAnalysis WHERE component == :component"), component);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.DependencyMetrics WHERE component == :component"), component);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.FindingAttribution WHERE component == :component"), component);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.PolicyViolation WHERE component == :component"), component);

// The component itself must be deleted via deletePersistentAll, otherwise relationships
// (e.g. with Vulnerability via COMPONENTS_VULNERABILITIES table) will not be cleaned up properly.
final Query<Component> componentQuery = pm.newQuery(Component.class, "this == :component");
try {
componentQuery.deletePersistentAll(component);
} finally {
componentQuery.closeAll();
}

if (!isJoiningExistingTrx) {
trx.commit();
}

// Event.dispatch(new IndexEvent(IndexEvent.Action.DELETE, detachedComponent));
// commitSearchIndex(commitIndex, Component.class);
} catch (javax.jdo.JDOObjectNotFoundException | org.datanucleus.exceptions.NucleusObjectNotFoundException e) {
LOGGER.warn("Deletion of component failed because it didn't exist anymore.");
} finally {
if (!isJoiningExistingTrx && trx.isActive()) {
trx.rollback();
}
}

}

/**
Expand Down
12 changes: 9 additions & 3 deletions src/main/java/org/dependencytrack/persistence/NotificationQueryManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,15 @@ public NotificationPublisher updateNotificationPublisher(NotificationPublisher t
@SuppressWarnings("unchecked")
public void removeProjectFromNotificationRules(final Project project) {
final Query<NotificationRule> query = pm.newQuery(NotificationRule.class, "projects.contains(:project)");
for (final NotificationRule rule: (List<NotificationRule>) query.execute(project)) {
rule.getProjects().remove(project);
persist(rule);
try {
for (final NotificationRule rule : (List<NotificationRule>) query.execute(project)) {
rule.getProjects().remove(project);
if (!pm.currentTransaction().isActive()) {
persist(rule);
}
}
} finally {
query.closeAll();
}
}

Expand Down
15 changes: 11 additions & 4 deletions src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -599,11 +599,18 @@ public void deletePolicyCondition(PolicyCondition policyCondition) {
*/
public void removeProjectFromPolicies(final Project project) {
final Query<Policy> query = pm.newQuery(Policy.class, "projects.contains(:project)");
query.setParameters(project);
try {
query.setParameters(project);

for (final Policy policy : query.executeList()) {
policy.getProjects().remove(project);

for (final Policy policy: query.executeList()) {
policy.getProjects().remove(project);
persist(policy);
if (!pm.currentTransaction().isActive()) {
persist(policy);
}
}
} finally {
query.closeAll();
}
}

Expand Down
72 changes: 48 additions & 24 deletions src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,16 @@
import org.dependencytrack.model.FindingAttribution;
import org.dependencytrack.model.Project;
import org.dependencytrack.model.ProjectProperty;
import org.dependencytrack.model.ServiceComponent;
import org.dependencytrack.model.Tag;
import org.dependencytrack.model.Vulnerability;
import org.dependencytrack.notification.NotificationConstants;
import org.dependencytrack.notification.NotificationGroup;
import org.dependencytrack.notification.NotificationScope;
import org.dependencytrack.util.NotificationUtil;

import javax.jdo.FetchPlan;
import javax.jdo.PersistenceManager;
import javax.jdo.Query;
import javax.jdo.Transaction;
import java.security.Principal;
import java.util.ArrayList;
import java.util.Date;
Expand Down Expand Up @@ -681,34 +680,59 @@ public Project clone(UUID from, String newVersion, boolean includeTags, boolean
* @param commitIndex specifies if the search index should be committed (an expensive operation)
*/
public void recursivelyDelete(final Project project, final boolean commitIndex) {
if (project.getChildren() != null) {
final Transaction trx = pm.currentTransaction();
final boolean isJoiningExistingTrx = trx.isActive();
try {
if (!isJoiningExistingTrx) {
trx.begin();
}

for (final Project child : project.getChildren()) {
// Note: This could be refactored such that each project is deleted
// in its own transaction. That would break semantics when it comes
// to joining an existing transaction though, so needs a bit more thought.
recursivelyDelete(child, false);
pm.flush();
}
}
pm.getFetchPlan().setDetachmentOptions(FetchPlan.DETACH_LOAD_FIELDS);
// final Project result = pm.getObjectById(Project.class, project.getId());
// Event.dispatch(new IndexEvent(IndexEvent.Action.DELETE, pm.detachCopy(result)));
// commitSearchIndex(commitIndex, Project.class);

deleteAnalysisTrail(project);
deleteViolationAnalysisTrail(project);
deleteMetrics(project);
deleteFindingAttributions(project);
deletePolicyViolations(project);
deleteComponents(project);
// Use bulk DELETE queries to avoid having to fetch every single object from the database first.
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.AnalysisComment WHERE analysis.project == :project"), project);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.Analysis WHERE project == :project"), project);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.ViolationAnalysisComment WHERE violationAnalysis.project == :project"), project);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.ViolationAnalysis WHERE project == :project"), project);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.DependencyMetrics WHERE project == :project"), project);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.ProjectMetrics WHERE project == :project"), project);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.FindingAttribution WHERE project == :project"), project);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.PolicyViolation WHERE project == :project"), project);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.Bom WHERE project == :project"), project);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.Vex WHERE project == :project"), project);
executeAndClose(pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.ProjectProperty WHERE project == :project"), project);

// Projects, Components, and ServiceComponents must be deleted via deletePersistentAll, otherwise relationships
// (e.g. with Vulnerability via COMPONENTS_VULNERABILITIES table) will not be cleaned up properly.
deleteComponents(project);
deleteServiceComponents(project);
removeProjectFromNotificationRules(project);
removeProjectFromPolicies(project);

final Query<Project> projectQuery = pm.newQuery(Project.class, "this == :project");
try {
projectQuery.deletePersistentAll(project);
} finally {
projectQuery.closeAll();
}

if (!isJoiningExistingTrx) {
trx.commit();
}

for (final ServiceComponent s : getAllServiceComponents(project)) {
recursivelyDelete(s, false);
// Event.dispatch(new IndexEvent(IndexEvent.Action.DELETE, detachedProject));
// commitSearchIndex(commitIndex, Project.class);
} finally {
if (!isJoiningExistingTrx && trx.isActive()) {
trx.rollback();
}
}
deleteBoms(project);
deleteVexs(project);
removeProjectFromNotificationRules(project);
removeProjectFromPolicies(project);
delete(project.getProperties());
delete(getAllBoms(project));
delete(project.getChildren());
delete(project);
}

/**
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/org/dependencytrack/persistence/QueryManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,10 @@ public ServiceComponent updateServiceComponent(ServiceComponent transientService
return getServiceComponentQueryManager().updateServiceComponent(transientServiceComponent, commitIndex);
}

public void deleteServiceComponents(final Project project) {
getServiceComponentQueryManager().deleteServiceComponents(project);
}

public void recursivelyDelete(ServiceComponent service, boolean commitIndex) {
getServiceComponentQueryManager().recursivelyDelete(service, commitIndex);
}
Expand Down Expand Up @@ -1517,4 +1521,20 @@ public VulnerabilityScan recordVulnerabilityScanResult(final String scanToken) {
public VulnerableSoftware getVulnerableSoftwareByPurlAndVersion(String purlType, String purlNamespace, String purlName, String version) {
return getVulnerableSoftwareQueryManager().getVulnerableSoftwareByPurlAndVersion(purlType, purlNamespace, purlName, version);
}

/**
* Execute a give {@link Query} and ensure that resources associated with it are released post execution.
*
* @param query The {@link Query} to execute
* @param parameters The parameters of the query
* @return The result of the query
*/
public Object executeAndClose(final Query<?> query, final Object... parameters) {
try {
return query.executeWithArray(parameters);
} finally {
query.closeAll();
}
}

}
58 changes: 42 additions & 16 deletions src/main/java/org/dependencytrack/persistence/ServiceComponentQueryManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
import org.dependencytrack.model.Project;
import org.dependencytrack.model.ServiceComponent;

import javax.jdo.FetchPlan;
import javax.jdo.PersistenceManager;
import javax.jdo.Query;
import javax.jdo.Transaction;
import java.util.ArrayList;
import java.util.List;

Expand Down Expand Up @@ -233,9 +233,13 @@ public ServiceComponent updateServiceComponent(ServiceComponent transientService
* Deletes all services for the specified Project.
* @param project the Project to delete services of
*/
private void deleteServiceComponents(Project project) {
public void deleteServiceComponents(Project project) {
final Query<ServiceComponent> query = pm.newQuery(ServiceComponent.class, "project == :project");
query.deletePersistentAll(project);
try {
query.deletePersistentAll(project);
} finally {
query.closeAll();
}
}

/**
Expand All @@ -244,21 +248,43 @@ private void deleteServiceComponents(Project project) {
* @param commitIndex specifies if the search index should be committed (an expensive operation)
*/
public void recursivelyDelete(ServiceComponent service, boolean commitIndex) {
if (service.getChildren() != null) {
for (final ServiceComponent child: service.getChildren()) {
final Transaction trx = pm.currentTransaction();
final boolean isJoiningExistingTrx = trx.isActive();
try {
if (!isJoiningExistingTrx) {
trx.begin();
}

for (final ServiceComponent child : service.getChildren()) {
recursivelyDelete(child, false);
pm.flush();
}

// TODO: Add these in when these features are supported by service components
//deleteAnalysisTrail(service);
//deleteViolationAnalysisTrail(service);
//deleteMetrics(service);
//deleteFindingAttributions(service);
//deletePolicyViolations(service);

final Query<ServiceComponent> query = pm.newQuery(ServiceComponent.class);
query.setFilter("this == :service");
try {
query.deletePersistentAll(service);
} finally {
query.closeAll();
}

if (!isJoiningExistingTrx) {
trx.commit();
}

// Event.dispatch(new IndexEvent(IndexEvent.Action.DELETE, detachedService));
// commitSearchIndex(commitIndex, ServiceComponent.class);
} finally {
if (!isJoiningExistingTrx && trx.isActive()) {
trx.rollback();
}
}
pm.getFetchPlan().setDetachmentOptions(FetchPlan.DETACH_LOAD_FIELDS);
// final ServiceComponent result = pm.getObjectById(ServiceComponent.class, service.getId());
// Event.dispatch(new IndexEvent(IndexEvent.Action.DELETE, pm.detachCopy(result)));
// TODO: Add these in when these features are supported by service components
//deleteAnalysisTrail(service);
//deleteViolationAnalysisTrail(service);
//deleteMetrics(service);
//deleteFindingAttributions(service);
//deletePolicyViolations(service);
delete(service);
// commitSearchIndex(commitIndex, ServiceComponent.class);
}
}
Loading

0 comments on commit 4b09783

Please sign in to comment.