Replace manual transaction commits with callInTransaction

This is to properly support nested transactions as introduced in stevespringett/Alpine#552.

Signed-off-by: nscuro <[email protected]>

src/main/java/org/dependencytrack/persistence/MetricsQueryManager.java

@@ -18,21 +18,19 @@
  */
 package org.dependencytrack.persistence;
 
-import java.util.Date;
-import java.util.List;
-
-import javax.jdo.PersistenceManager;
-import javax.jdo.Query;
-
+import alpine.persistence.PaginatedResult;
+import alpine.resources.AlpineRequest;
 import org.dependencytrack.model.Component;
 import org.dependencytrack.model.DependencyMetrics;
 import org.dependencytrack.model.PortfolioMetrics;
 import org.dependencytrack.model.Project;
 import org.dependencytrack.model.ProjectMetrics;
 import org.dependencytrack.model.VulnerabilityMetrics;
 
-import alpine.persistence.PaginatedResult;
-import alpine.resources.AlpineRequest;
+import javax.jdo.PersistenceManager;
+import javax.jdo.Query;
+import java.util.Date;
+import java.util.List;
 
 public class MetricsQueryManager extends QueryManager implements IQueryManager {
 
@@ -164,17 +162,14 @@ public List<DependencyMetrics> getDependencyMetricsSince(Component component, Da
     }
 
     public void synchronizeVulnerabilityMetrics(List<VulnerabilityMetrics> metrics) {
-        pm.currentTransaction().begin();
-        // No need for complex updating, just replace the existing ~400 rows with new ones
-        // Unless we have a contract with clients that the ID of metric records cannot change?
-
-        final Query<VulnerabilityMetrics> delete = pm.newQuery("DELETE FROM org.dependencytrack.model.VulnerabilityMetrics");
-        delete.execute();
-
-        // This still does ~400 queries, probably because not all databases can do bulk insert with autogenerated PKs
-        // Or because Datanucleus is trying to be smart as it wants to cache all these instances
-        pm.makePersistentAll(metrics);
-        pm.currentTransaction().commit();
+        runInTransaction(() -> {
+            // No need for complex updating, just replace the existing ~400 rows with new ones.
+            pm.newQuery(Query.JDOQL, "DELETE FROM org.dependencytrack.model.VulnerabilityMetrics").execute();
+
+            // This still does ~400 queries, probably because not all databases can do bulk insert with autogenerated PKs
+            // Or because Datanucleus is trying to be smart as it wants to cache all these instances
+            pm.makePersistentAll(metrics);
+        });
     }
 
     /**

src/main/java/org/dependencytrack/persistence/NotificationQueryManager.java

@@ -159,18 +159,16 @@ private NotificationPublisher getDefaultNotificationPublisher(final String clazz
     public NotificationPublisher createNotificationPublisher(final String name, final String description,
                                                              final Class<? extends Publisher> publisherClass, final String templateContent,
                                                              final String templateMimeType, final boolean defaultPublisher) {
-        pm.currentTransaction().begin();
-        final NotificationPublisher publisher = new NotificationPublisher();
-        publisher.setName(name);
-        publisher.setDescription(description);
-        publisher.setPublisherClass(publisherClass.getName());
-        publisher.setTemplate(templateContent);
-        publisher.setTemplateMimeType(templateMimeType);
-        publisher.setDefaultPublisher(defaultPublisher);
-        pm.makePersistent(publisher);
-        pm.currentTransaction().commit();
-        pm.getFetchPlan().addGroup(NotificationPublisher.FetchGroup.ALL.name());
-        return getObjectById(NotificationPublisher.class, publisher.getId());
+        return callInTransaction(() -> {
+            final NotificationPublisher publisher = new NotificationPublisher();
+            publisher.setName(name);
+            publisher.setDescription(description);
+            publisher.setPublisherClass(publisherClass.getName());
+            publisher.setTemplate(templateContent);
+            publisher.setTemplateMimeType(templateMimeType);
+            publisher.setDefaultPublisher(defaultPublisher);
+            return pm.makePersistent(publisher);
+        });
     }
 
     /**

src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java

@@ -908,25 +908,26 @@ public List<ProjectProperty> getProjectProperties(final Project project) {
      * @param project a Project object
      * @param tags a List of Tag objects
      */
-    @SuppressWarnings("unchecked")
     @Override
     public void bind(Project project, List<Tag> tags) {
-        final Query<Tag> query = pm.newQuery(Tag.class, "projects.contains(:project)");
-        final List<Tag> currentProjectTags = (List<Tag>)query.execute(project);
-        pm.currentTransaction().begin();
-        for (final Tag tag: currentProjectTags) {
-            if (!tags.contains(tag)) {
-                tag.getProjects().remove(project);
+        runInTransaction(() -> {
+            final Query<Tag> query = pm.newQuery(Tag.class, "projects.contains(:project)");
+            query.setParameters(project);
+            final List<Tag> currentProjectTags = executeAndCloseList(query);
+
+            for (final Tag tag : currentProjectTags) {
+                if (!tags.contains(tag)) {
+                    tag.getProjects().remove(project);
+                }
             }
-        }
-        project.setTags(tags);
-        for (final Tag tag: tags) {
-            final List<Project> projects = tag.getProjects();
-            if (!projects.contains(project)) {
-                projects.add(project);
+            project.setTags(tags);
+            for (final Tag tag : tags) {
+                final List<Project> projects = tag.getProjects();
+                if (!projects.contains(project)) {
+                    projects.add(project);
+                }
             }
-        }
-        pm.currentTransaction().commit();
+        });
     }
 
     /**

src/main/java/org/dependencytrack/persistence/QueryManager.java

@@ -1439,17 +1439,16 @@ public void ensureNoActiveTransaction() {
     }
 
     public void recursivelyDeleteTeam(Team team) {
-        pm.setProperty("datanucleus.query.sql.allowAll", true);
-        final Transaction trx = pm.currentTransaction();
-        pm.currentTransaction().begin();
-        pm.deletePersistentAll(team.getApiKeys());
-        String aclDeleteQuery = """
-            DELETE FROM \"PROJECT_ACCESS_TEAMS\" WHERE \"PROJECT_ACCESS_TEAMS\".\"TEAM_ID\" = ?
-        """;
-        final Query query = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, aclDeleteQuery);
-        query.executeWithArray(team.getId());
-        pm.deletePersistent(team);
-        pm.currentTransaction().commit();
+        runInTransaction(() -> {
+            pm.deletePersistentAll(team.getApiKeys());
+
+            final Query<?> aclDeleteQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
+                    DELETE FROM "PROJECT_ACCESS_TEAMS" WHERE "PROJECT_ACCESS_TEAMS"."TEAM_ID" = ?""");
+            aclDeleteQuery.setParameters(team.getId());
+            executeAndClose(aclDeleteQuery);
+
+            pm.deletePersistent(team);
+        });
     }
 
     /**

src/main/java/org/dependencytrack/persistence/VulnerabilityQueryManager.java

@@ -42,9 +42,9 @@
 import java.util.Comparator;
 import java.util.Date;
 import java.util.HashMap;
-import java.util.UUID;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 import java.util.stream.Collectors;
 
 final class VulnerabilityQueryManager extends QueryManager implements IQueryManager {
@@ -243,15 +243,16 @@ public void addVulnerability(Vulnerability vulnerability, Component component, A
      * @param component the component unaffected by the vulnerabiity
      */
     public void removeVulnerability(Vulnerability vulnerability, Component component) {
-        if (contains(vulnerability, component)) {
-            pm.currentTransaction().begin();
-            component.removeVulnerability(vulnerability);
-            pm.currentTransaction().commit();
-        }
-        final FindingAttribution fa = getFindingAttribution(vulnerability, component);
-        if (fa != null) {
-            delete(fa);
-        }
+        runInTransaction(() -> {
+            if (contains(vulnerability, component)) {
+                component.removeVulnerability(vulnerability);
+            }
+
+            final FindingAttribution fa = getFindingAttribution(vulnerability, component);
+            if (fa != null) {
+                delete(fa);
+            }
+        });
     }
 
     /**

src/main/java/org/dependencytrack/tasks/InternalComponentIdentificationTask.java

@@ -31,7 +31,6 @@
 
 import javax.jdo.PersistenceManager;
 import javax.jdo.Query;
-import javax.jdo.Transaction;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.List;
@@ -95,20 +94,11 @@ private void analyze() throws Exception {
                     }
 
                     if (component.isInternal() != internal) {
-                        final Transaction trx = pm.currentTransaction();
-                        try {
-                            trx.begin();
-                            component.setInternal(internal);
-                            trx.commit();
-                        } finally {
-                            if (trx.isActive()) {
-                                trx.rollback();
-                            }
-                        }
+                        qm.runInTransaction(() -> component.setInternal(internal));
                     }
                 }
 
-                final long lastId = components.get(components.size() - 1).getId();
+                final long lastId = components.getLast().getId();
                 components = fetchNextComponentsPage(pm, lastId);
             }
         }