Skip to content

Test performance improvements #4901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .idea/runConfigurations/Tests.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,7 @@
<value>src/test/resources/logging.properties</value>
</property>
</systemProperties>
<reuseForks>true</reuseForks>
</configuration>
</plugin>
<plugin>
Expand Down
38 changes: 15 additions & 23 deletions src/main/java/org/dependencytrack/resources/v1/SearchResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,20 @@
import io.swagger.v3.oas.annotations.security.SecurityRequirement;
import io.swagger.v3.oas.annotations.security.SecurityRequirements;
import io.swagger.v3.oas.annotations.tags.Tag;
import org.apache.commons.lang3.StringUtils;
import org.dependencytrack.auth.Permissions;
import org.dependencytrack.resources.v1.vo.BomUploadResponse;
import org.dependencytrack.search.FuzzyVulnerableSoftwareSearchManager;
import org.dependencytrack.search.SearchManager;
import org.dependencytrack.search.SearchResult;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.QueryParam;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import org.apache.commons.lang3.StringUtils;
import org.dependencytrack.auth.Permissions;
import org.dependencytrack.resources.v1.vo.BomUploadResponse;
import org.dependencytrack.search.FuzzyVulnerableSoftwareSearchManager;
import org.dependencytrack.search.SearchManager;
import org.dependencytrack.search.SearchResult;

import java.util.Collections;
import java.util.Set;

Expand Down Expand Up @@ -75,8 +75,7 @@ public class SearchResource extends AlpineResource {
})
@PermissionRequired(Permissions.Constants.VIEW_PORTFOLIO)
public Response aggregateSearch(@QueryParam("query") String query) {
final SearchManager searchManager = new SearchManager();
final SearchResult searchResult = searchManager.searchIndices(query, 1000);
final SearchResult searchResult = SearchManager.searchIndices(query, 1000);
return Response.ok(searchResult).build();
}

Expand All @@ -97,8 +96,7 @@ public Response aggregateSearch(@QueryParam("query") String query) {
})
@PermissionRequired(Permissions.Constants.VIEW_PORTFOLIO)
public Response projectSearch(@QueryParam("query") String query) {
final SearchManager searchManager = new SearchManager();
final SearchResult searchResult = searchManager.searchProjectIndex(query, 1000);
final SearchResult searchResult = SearchManager.searchProjectIndex(query, 1000);
return Response.ok(searchResult).build();
}

Expand All @@ -119,8 +117,7 @@ public Response projectSearch(@QueryParam("query") String query) {
})
@PermissionRequired(Permissions.Constants.VIEW_PORTFOLIO)
public Response componentSearch(@QueryParam("query") String query) {
final SearchManager searchManager = new SearchManager();
final SearchResult searchResult = searchManager.searchComponentIndex(query, 1000);
final SearchResult searchResult = SearchManager.searchComponentIndex(query, 1000);
return Response.ok(searchResult).build();
}

Expand All @@ -141,8 +138,7 @@ public Response componentSearch(@QueryParam("query") String query) {
})
@PermissionRequired(Permissions.Constants.VIEW_PORTFOLIO)
public Response serviceSearch(@QueryParam("query") String query) {
final SearchManager searchManager = new SearchManager();
final SearchResult searchResult = searchManager.searchServiceComponentIndex(query, 1000);
final SearchResult searchResult = SearchManager.searchServiceComponentIndex(query, 1000);
return Response.ok(searchResult).build();
}

Expand All @@ -163,8 +159,7 @@ public Response serviceSearch(@QueryParam("query") String query) {
})
@PermissionRequired(Permissions.Constants.VIEW_PORTFOLIO)
public Response licenseSearch(@QueryParam("query") String query) {
final SearchManager searchManager = new SearchManager();
final SearchResult searchResult = searchManager.searchLicenseIndex(query, 1000);
final SearchResult searchResult = SearchManager.searchLicenseIndex(query, 1000);
return Response.ok(searchResult).build();
}

Expand All @@ -185,8 +180,7 @@ public Response licenseSearch(@QueryParam("query") String query) {
})
@PermissionRequired(Permissions.Constants.VIEW_PORTFOLIO)
public Response vulnerabilitySearch(@QueryParam("query") String query) {
final SearchManager searchManager = new SearchManager();
final SearchResult searchResult = searchManager.searchVulnerabilityIndex(query, 1000);
final SearchResult searchResult = SearchManager.searchVulnerabilityIndex(query, 1000);
return Response.ok(searchResult).build();
}

Expand All @@ -212,8 +206,7 @@ public Response vulnerableSoftwareSearch(@QueryParam("query") String query, @Que
final SearchResult searchResult = searchManager.searchIndex(FuzzyVulnerableSoftwareSearchManager.getLuceneCpeRegexp(cpe));
return Response.ok(searchResult).build();
} else {
final SearchManager searchManager = new SearchManager();
final SearchResult searchResult = searchManager.searchVulnerableSoftwareIndex(query, 1000);
final SearchResult searchResult = SearchManager.searchVulnerableSoftwareIndex(query, 1000);
return Response.ok(searchResult).build();
}
}
Expand All @@ -240,8 +233,7 @@ public Response reindex(@QueryParam("type") Set<String> type) {
return Response.status(Response.Status.BAD_REQUEST).entity("No valid index type was provided").build();
}
try {
final SearchManager searchManager = new SearchManager();
String chainIdentifier = searchManager.reindex(type);
String chainIdentifier = SearchManager.reindex(type);
return Response.ok(Collections.singletonMap("token", chainIdentifier)).build();
} catch (IllegalArgumentException exception) {
return Response.status(Response.Status.BAD_REQUEST).entity(exception.getMessage()).build();
Expand Down
20 changes: 11 additions & 9 deletions src/main/java/org/dependencytrack/search/SearchManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ public class SearchManager {

private static final Logger LOGGER = Logger.getLogger(SearchManager.class);

public SearchResult searchIndices(final String queryString, final int limit) {
private SearchManager() { }

public static SearchResult searchIndices(final String queryString, final int limit) {
final SearchResult searchResult = new SearchResult();
final IndexManager[] indexManagers = {
ProjectIndexer.getInstance(),
Expand All @@ -71,7 +73,7 @@ public SearchResult searchIndices(final String queryString, final int limit) {
return searchResult;
}

public SearchResult searchIndex(final IndexManager indexManager, final String queryString, final int limit) {
public static SearchResult searchIndex(final IndexManager indexManager, final String queryString, final int limit) {
final SearchResult searchResult = new SearchResult();
final List<Map<String, String>> resultSet = new ArrayList<>();
try {
Expand Down Expand Up @@ -139,31 +141,31 @@ public SearchResult searchIndex(final IndexManager indexManager, final String qu
return searchResult;
}

public SearchResult searchProjectIndex(final String queryString, final int limit) {
public static SearchResult searchProjectIndex(final String queryString, final int limit) {
return searchIndex(ProjectIndexer.getInstance(), queryString, limit);
}

public SearchResult searchComponentIndex(final String queryString, final int limit) {
public static SearchResult searchComponentIndex(final String queryString, final int limit) {
return searchIndex(ComponentIndexer.getInstance(), queryString, limit);
}

public SearchResult searchServiceComponentIndex(final String queryString, final int limit) {
public static SearchResult searchServiceComponentIndex(final String queryString, final int limit) {
return searchIndex(ServiceComponentIndexer.getInstance(), queryString, limit);
}

public SearchResult searchLicenseIndex(final String queryString, final int limit) {
public static SearchResult searchLicenseIndex(final String queryString, final int limit) {
return searchIndex(LicenseIndexer.getInstance(), queryString, limit);
}

public SearchResult searchVulnerabilityIndex(final String queryString, final int limit) {
public static SearchResult searchVulnerabilityIndex(final String queryString, final int limit) {
return searchIndex(VulnerabilityIndexer.getInstance(), queryString, limit);
}

public SearchResult searchVulnerableSoftwareIndex(final String queryString, final int limit) {
public static SearchResult searchVulnerableSoftwareIndex(final String queryString, final int limit) {
return searchIndex(VulnerableSoftwareIndexer.getInstance(), queryString, limit);
}

public String reindex(Set<String> type) {
public static String reindex(Set<String> type) {
List<IndexManager.IndexType> indexTypes = type.stream().flatMap(t -> IndexManager.IndexType.getIndexType(t).stream()).toList();
if(indexTypes.isEmpty()) {
throw new IllegalArgumentException("No valid index type was provided");
Expand Down
11 changes: 9 additions & 2 deletions src/test/java/org/dependencytrack/JerseyTestExtension.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
import org.junit.jupiter.api.extension.AfterAllCallback;
import org.junit.jupiter.api.extension.BeforeAllCallback;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junitpioneer.jupiter.DefaultLocale;

import java.util.Locale;
import java.util.function.Supplier;

/**
Expand All @@ -48,8 +50,14 @@ public JerseyTestExtension(final Supplier<ResourceConfig> resourceConfigSupplier

@Override
public void beforeAll(ExtensionContext context) throws Exception {
this.jerseyTest = new JerseyTest() {
final var testClass = context.getRequiredTestClass();
final var defaultLocale = testClass.getAnnotation(DefaultLocale.class);
if (defaultLocale != null) {
final var locale = Locale.forLanguageTag(defaultLocale.value());
Locale.setDefault(locale);
}

this.jerseyTest = new JerseyTest() {
@Override
protected TestContainerFactory getTestContainerFactory() throws TestContainerException {
return new DTGrizzlyWebTestContainerFactory();
Expand All @@ -68,7 +76,6 @@ protected DeploymentContext configureDeployment() {
return ServletDeploymentContext.forServlet(new ServletContainer(resourceConfigSupplier.get()
.packages("org.dependencytrack.resources.v1.exception"))).build();
}

};
jerseyTest.setUp();
}
Expand Down
43 changes: 37 additions & 6 deletions src/test/java/org/dependencytrack/PersistenceCapableTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,15 @@

import alpine.Config;
import alpine.server.persistence.PersistenceManagerFactory;
import org.datanucleus.PropertyNames;
import org.dependencytrack.persistence.QueryManager;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;

import javax.jdo.Query;
import java.time.Duration;

public abstract class PersistenceCapableTest {

protected QueryManager qm;
Expand All @@ -34,15 +38,15 @@ static void init() {
Config.enableUnitTests();

// ensure nothing is left open so the database is properly cleaned up between tests
System.setProperty(Config.AlpineKey.DATABASE_POOL_TX_MAX_LIFETIME.getPropertyName(), "0");
System.setProperty(Config.AlpineKey.DATABASE_POOL_NONTX_MAX_LIFETIME.getPropertyName(), "0");
System.setProperty(Config.AlpineKey.DATABASE_POOL_TX_IDLE_TIMEOUT.getPropertyName(), "0");
System.setProperty(Config.AlpineKey.DATABASE_POOL_NONTX_IDLE_TIMEOUT.getPropertyName(), "0");
System.setProperty(Config.AlpineKey.DATABASE_POOL_ENABLED.getPropertyName(), "false");
}

@BeforeEach
final void initQueryManager() {
final void initQueryManager() throws InterruptedException {
this.qm = new QueryManager();
for (int i = 0; i < 5 && qm.getPersistenceManager().isClosed(); ++i) {
Thread.sleep(Duration.ofSeconds(1));
}
}

@AfterEach
Expand All @@ -56,7 +60,34 @@ final void tearDownQueryManager() {
qm.getPersistenceManager().currentTransaction().rollback();
}

PersistenceManagerFactory.tearDown();
// Add a small delay to allow pending operations to complete
// FIXME This is a very dumb "solution" and probably only reduces the probability of any connection errors to
// occur. The underlying issue (it seems) is that some resource is not closed properly in time. The error
// is non-deterministic.
try {
Thread.sleep(Duration.ofMillis(100));
} catch (InterruptedException e) {
// Ignore
}

try {
// Make sure the in-memory H2 database is closed before the next test is run.
qm.getPersistenceManager().setProperty(PropertyNames.PROPERTY_QUERY_SQL_ALLOWALL, "true");
try(final var q = qm.getPersistenceManager().newQuery(Query.SQL, "SHUTDOWN IMMEDIATELY")) {
q.execute();
}
} catch (Exception e) {
// ignored, DB already closed.
}

qm.close();
qm = null;

try {
PersistenceManagerFactory.tearDown();
} catch (NullPointerException e) {
// ignored, may happen if there is no transaction left
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,9 @@ void notificationTest() {
// Evaluate policies and ensure that a notification has been sent.
final var policyEngine = new PolicyEngine();
assertThat(policyEngine.evaluate(List.of(component))).hasSize(1);
assertThat(NOTIFICATIONS).hasSize(2);
await("Notifications")
.atMost(Duration.ofSeconds(3))
.untilAsserted(() -> assertThat(NOTIFICATIONS).hasSize(2));

// Create an additional policy condition that matches on the exact version of the component,
// and re-evaluate policies. Ensure that only one notification per newly violated condition was sent.
Expand Down
Loading
Loading