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

• I have read and understand the contributing guidelines
• 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.

dependency-track/src/main/java/org/dependencytrack/persistence/QueryManager.java

Lines 1256 to 1267 in 93321f8

	public void runInTransaction(final Runnable runnable) {
	final Transaction trx = pm.currentTransaction();
	try {
	trx.begin();
	runnable.run();
	trx.commit();
	} finally {
	if (trx.isActive()) {
	trx.rollback();
	}
	}
	}

[nscuro]

Resolved in Alpine 2.2.6 via stevespringett/Alpine#552.

[github-actions]

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.