Database transactions should be rolled back when committing fails

[nscuro]

Current Behavior

The persist methods provided by AbstractAlpineQueryManager and inherited by DT's QueryManager do not rollback the active transaction when committing of the same fails. For example persist(T):

/**
 * Persists the specified PersistenceCapable object.
 * @param object a PersistenceCapable object
 * @param <T> the type to return
 * @return the persisted object
 */
@SuppressWarnings("unchecked")
public <T> T persist(T object) {
    pm.currentTransaction().begin();
    pm.makePersistent(object);
    pm.currentTransaction().commit();
    pm.getFetchPlan().setDetachmentOptions(FetchPlan.DETACH_LOAD_FIELDS);
    pm.refresh(object);
    return object;
}

Because none of those methods check whether a transaction is already active, and always start a new one instead (via PersistenceManager#currentTransaction()#begin), all operations following the initial failure with also fail with:

NucleusTransactionException: Invalid state. Transaction has already started

This can happen in situations similar to this one, where a single QueryManager instance is shared across multiple operations, and a single operation failing does not abort the overall execution:

try (final QueryManager qm = new QueryManager()) {
    final List<Project> projects = qm.getAllProjects(true);
    for (final Project project: projects) {
        final List<Component> components = qm.getAllComponents(project);
        for (final Component component: components) {
            try {
                doStuff(qm, component);
            } catch (Exception e) {
                LOGGER.warn("Doing stuff with component %s failed".formatted(component.getUuid()));
            }
        }
    }
}

If the first invocation of doStuff fails on commit, all other invocations will fail as well.

Proposed Behavior

Database transactions should be rolled back when committing fails.

Checklist

• [X] I have read and understand the contributing guidelines
• [X] I have checked the existing issues for whether this enhancement was already requested

[nscuro]

We already have QueryManager#runInTransaction that performs proper rollbacks, but majority of the code base uses QueryManager#persist.

https://github.com/DependencyTrack/dependency-track/blob/93321f8a82f2eee4c2d42bb6ffc664ee54a82b4d/src/main/java/org/dependencytrack/persistence/QueryManager.java#L1256-L1267

[nscuro]

Resolved in Alpine 2.2.6 via https://github.com/stevespringett/Alpine/pull/552.

[github-actions[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.