Skip to content

Commit

Permalink
Adjust to transaction behavior change in Alpine
Browse files Browse the repository at this point in the history
The biggest impact is that some tests now require eviction of the L1 cache to see changes made in the functionality they're testing.

stevespringett/Alpine#552
Signed-off-by: nscuro <[email protected]>
  • Loading branch information
nscuro committed Apr 20, 2024
1 parent 8e44ee5 commit 47fd21f
Show file tree
Hide file tree
Showing 12 changed files with 18 additions and 70 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<parent>
<groupId>us.springett</groupId>
<artifactId>alpine-parent</artifactId>
<version>2.2.5</version>
<version>2.2.6-SNAPSHOT</version>
</parent>

<modelVersion>4.0.0</modelVersion>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class ProjectProperty implements IConfigProperty, Serializable {

@Persistent
@Column(name = "PROJECT_ID", allowsNull = "false")
@JsonIgnore
private Project project;

@Persistent
Expand Down
56 changes: 0 additions & 56 deletions src/main/java/org/dependencytrack/persistence/QueryManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,6 @@
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;

/**
* This QueryManager provides a concrete extension of {@link AlpineQueryManager} by
Expand Down Expand Up @@ -1380,59 +1377,6 @@ public <T> Query<T> getObjectsByUuidsQuery(final Class<T> clazz, final List<UUID
return query;
}

/**
* Convenience method to execute a given {@link Runnable} within the context of a {@link Transaction}.
* <p>
* Eventually, this may be moved to {@link alpine.persistence.AbstractAlpineQueryManager}.
*
* @param runnable The {@link Runnable} to execute
* @since 4.6.0
*/
public void runInTransaction(final Runnable runnable) {
runInTransaction((Function<Transaction, Void>) trx -> {
runnable.run();
return null;
});
}

public void runInTransaction(final Consumer<Transaction> consumer) {
runInTransaction((Function<Transaction, Void>) trx -> {
consumer.accept(trx);
return null;
});
}

/**
* Convenience method to execute a given {@link Supplier} within the context of a {@link Transaction}.
* <p>
* Eventually, this may be moved to {@link alpine.persistence.AbstractAlpineQueryManager}.
*
* @param supplier The {@link Supplier} to execute
* @since 4.9.0
*/
public <T> T runInTransaction(final Supplier<T> supplier) {
return runInTransaction((Function<Transaction, T>) trx -> supplier.get());
}

public <T> T runInTransaction(final Function<Transaction, T> function) {
final Transaction trx = pm.currentTransaction();
final boolean isJoiningExisting = trx.isActive();
try {
if (!isJoiningExisting) {
trx.begin();
}
final T result = function.apply(trx);
if (!isJoiningExisting) {
trx.commit();
}
return result;
} finally {
if (!isJoiningExisting && trx.isActive()) {
trx.rollback();
}
}
}

/**
* Convenience method to ensure that any active transaction is rolled back.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ public List<AffectedProject> getAffectedProjects(Vulnerability vulnerability) {
}

public synchronized VulnerabilityAlias synchronizeVulnerabilityAlias(final VulnerabilityAlias alias) {
return runInTransaction(() -> {
return callInTransaction(() -> {
// Query existing aliases that match AT LEAST ONE identifier of the given alias.
//
// For each data source, we want to know the existing aliases where the respective identifier either:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ public Response updateApiKeyComment(@PathParam("key") final String key,
try (final var qm = new QueryManager()) {
qm.getPersistenceManager().setProperty(PROPERTY_RETAIN_VALUES, "true");

return qm.runInTransaction(() -> {
return qm.callInTransaction(() -> {
final ApiKey apiKey = qm.getApiKey(key);
if (apiKey == null) {
return Response
Expand Down
10 changes: 4 additions & 6 deletions src/main/java/org/dependencytrack/tasks/NistApiMirrorTask.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import alpine.event.framework.LoggableUncaughtExceptionHandler;
import alpine.event.framework.Subscriber;
import alpine.model.ConfigProperty;
import alpine.persistence.Transaction;
import alpine.security.crypto.DataEncryption;
import io.github.jeremylong.openvulnerability.client.HttpAsyncClientSupplier;
import io.github.jeremylong.openvulnerability.client.nvd.CveItem;
Expand Down Expand Up @@ -232,9 +233,8 @@ public void inform(final Event e) {
}

private static Vulnerability synchronizeVulnerability(final QueryManager qm, final Vulnerability vuln) {
final Pair<Vulnerability, IndexEvent> vulnIndexEventPair = qm.runInTransaction(trx -> {
trx.setSerializeRead(true); // SELECT ... FOR UPDATE

final Transaction.Options trxOptions = Transaction.defaultOptions().withSerializeRead(true);
final Pair<Vulnerability, IndexEvent> vulnIndexEventPair = qm.callInTransaction(trxOptions, () -> {
Vulnerability persistentVuln = getVulnerabilityByCveId(qm, vuln.getVulnId());
if (persistentVuln == null) {
persistentVuln = qm.getPersistenceManager().makePersistent(vuln);
Expand Down Expand Up @@ -262,9 +262,7 @@ private static Vulnerability synchronizeVulnerability(final QueryManager qm, fin
}

private static void synchronizeVulnerableSoftware(final QueryManager qm, final Vulnerability persistentVuln, final List<VulnerableSoftware> vsList) {
qm.runInTransaction(tx -> {
tx.setSerializeRead(false);

qm.runInTransaction(() -> {
// Get all VulnerableSoftware records that are currently associated with the vulnerability.
// Note: For SOME ODD REASON, duplicate (as in, same database ID and all) VulnerableSoftware
// records are returned, when operating on data that was originally created by the feed-based
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/logback.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<configuration scan="true">

<statusListener class="ch.qos.logback.core.status.NopStatusListener" />
<appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
<file>${user.home}/.dependency-track/dependency-track.log</file>
<rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void testLoadDefaultLicensesUpdatesExistingLicenses() throws Exception {
method.setAccessible(true);
method.invoke(generator);

qm.getPersistenceManager().refresh(license);
qm.getPersistenceManager().evictAll();
assertThat(license.getLicenseId()).isEqualTo("LGPL-2.1+");
assertThat(license.getName()).isEqualTo("GNU Lesser General Public License v2.1 or later");
assertThat(license.getComment()).isNotEqualTo("comment");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ public void patchProjectParentTest() {
""");

// Ensure the parent was updated.
qm.getPersistenceManager().refresh(project);
qm.getPersistenceManager().evictAll();
assertThat(project.getParent()).isNotNull();
assertThat(project.getParent().getUuid()).isEqualTo(newParent.getUuid());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,9 @@ public void informWithExistingDuplicateComponentsTest() {
componentB.setVersion("2.0.0");
qm.persist(componentB);

final Component transientComponentA = qm.makeTransient(componentA);
final Component transientComponentB = qm.makeTransient(componentB);

final byte[] bomBytes = """
{
"bomFormat": "CycloneDX",
Expand Down Expand Up @@ -466,15 +469,15 @@ public void informWithExistingDuplicateComponentsTest() {
assertThat(indexEvent.getIndexableClass()).isEqualTo(Component.class);
assertThat(indexEvent.getAction()).isEqualTo(IndexEvent.Action.UPDATE);
final var searchDoc = (ComponentDocument) indexEvent.getDocument();
assertThat(searchDoc.uuid()).isEqualTo(componentA.getUuid());
assertThat(searchDoc.uuid()).isEqualTo(transientComponentA.getUuid());
},
event -> {
assertThat(event).isInstanceOf(IndexEvent.class);
final var indexEvent = (IndexEvent) event;
assertThat(indexEvent.getIndexableClass()).isEqualTo(Component.class);
assertThat(indexEvent.getAction()).isEqualTo(IndexEvent.Action.DELETE);
final var searchDoc = (ComponentDocument) indexEvent.getDocument();
assertThat(searchDoc.uuid()).isEqualTo(componentB.getUuid());
assertThat(searchDoc.uuid()).isEqualTo(transientComponentB.getUuid());
},
event -> {
assertThat(event).isInstanceOf(IndexEvent.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ public void testUpdateDatasourceVulnerableVersionRanges() {
final var task = new GitHubAdvisoryMirrorTask();
task.updateDatasource(List.of(ghAdvisory));

qm.getPersistenceManager().evictAll();
final Vulnerability vuln = qm.getVulnerabilityByVulnId(Source.GITHUB, "GHSA-57j2-w4cx-62h2");
assertThat(vuln).isNotNull();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ public void testUpdateDatasourceVulnerableVersionRanges() {
}
""")));

qm.getPersistenceManager().evictAll();
final Vulnerability vuln = qm.getVulnerabilityByVulnId(Vulnerability.Source.GITHUB, "GHSA-57j2-w4cx-62h2");
assertThat(vuln).isNotNull();

Expand Down

0 comments on commit 47fd21f

Please sign in to comment.