Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add /v1/project/batchDelete API method that deletes with SQL #4383

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add /v1/project/batchDelete API method that deletes with SQL #4383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2a0a7e5
Add /v1/project/batchDelete API method that deletes with SQL
mikael-carneholm-2-wcar Nov 14, 2024
7035a8c
Merge branch 'DependencyTrack:master' into master
mikael-carneholm-2-wcar Nov 20, 2024
58eff60
Merge branch 'DependencyTrack:master' into master
mikael-carneholm-2-wcar Nov 25, 2024
3450de7
Merge branch 'DependencyTrack:master' into master
mikael-carneholm-2-wcar Nov 29, 2024
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
262 changes: 262 additions & 0 deletions src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.dependencytrack.persistence;

import alpine.Config;
import alpine.common.logging.Logger;
import alpine.event.framework.Event;
import alpine.model.ApiKey;
Expand All @@ -27,6 +28,7 @@
import alpine.notification.Notification;
import alpine.notification.NotificationLevel;
import alpine.persistence.PaginatedResult;
import alpine.persistence.ScopedCustomization;
import alpine.resources.AlpineRequest;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
Expand Down Expand Up @@ -73,6 +75,8 @@
import java.util.UUID;
import java.util.concurrent.atomic.AtomicReference;

import static org.datanucleus.PropertyNames.PROPERTY_QUERY_SQL_ALLOWALL;

final class ProjectQueryManager extends QueryManager implements IQueryManager {

private static final Logger LOGGER = Logger.getLogger(ProjectQueryManager.class);
Expand Down Expand Up @@ -882,6 +886,264 @@ public void recursivelyDelete(final Project project, final boolean commitIndex)
delete(project);
}

/**
* Deletes a list of Projects (identified by their UUIDs) and all objects dependent on them.
* @param uuids the UUIDs of the Projects to delete
*
* NB: if ON DELETE rules had been set up, this would be as simple as "delete from project where ..."
*/
@Override
public void deleteProjectsByUUIDs(List<UUID> uuids) {
String[] uuidsArray = uuids.stream().map(UUID::toString).toArray(String[]::new);
runInTransaction(() -> {
try (var ignored = new ScopedCustomization(pm).withProperty(PROPERTY_QUERY_SQL_ALLOWALL, "true")) {
Query<?> sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "PROJECTMETRICS" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
)
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "DEPENDENCYMETRICS" WHERE "COMPONENT_ID" IN (
SELECT "ID" FROM "COMPONENT" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
)
) OR "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray, uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "FINDINGATTRIBUTION" WHERE "COMPONENT_ID" IN (
SELECT "ID" FROM "COMPONENT" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
)
) AND "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray, uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "COMPONENTS_VULNERABILITIES" WHERE "COMPONENT_ID" IN (
SELECT "ID" FROM "COMPONENT" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "ANALYSISCOMMENT" WHERE "ANALYSIS_ID" IN (
SELECT "ID" FROM "ANALYSIS" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "ANALYSIS" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "COMPONENT_PROPERTY" WHERE "COMPONENT_ID" IN (
SELECT "ID" FROM "COMPONENT" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

// Does not work with H2, but verified on Postgres
if (Config.getInstance().getProperty(Config.AlpineKey.DATABASE_DRIVER).equals("org.postgresql.Driver")) {
sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
WITH RECURSIVE c_family("ID", "PARENT_COMPONENT_ID") AS (
SELECT "COMPONENT"."ID", "PARENT_COMPONENT_ID"
FROM "COMPONENT" JOIN "PROJECT" ON "PROJECT_ID" = "PROJECT"."ID"
WHERE "PROJECT"."UUID" = ANY(?)
UNION ALL
SELECT "COMPONENT"."ID", "COMPONENT"."PARENT_COMPONENT_ID"
FROM c_family, "COMPONENT"
WHERE "COMPONENT"."ID" = c_family."PARENT_COMPONENT_ID"
)
DELETE FROM "COMPONENT"
WHERE "ID" = ANY(SELECT "ID" FROM c_family);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);
}

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "COMPONENT" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "BOM" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "PROJECT_METADATA" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "PROJECT_PROPERTY" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "PROJECTS_TAGS" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "TAG"
WHERE "ID" NOT IN (SELECT "TAG_ID" FROM "PROJECTS_TAGS");
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "NOTIFICATIONRULE_PROJECTS" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "NOTIFICATIONRULE"
WHERE "ID" NOT IN (SELECT "NOTIFICATIONRULE_ID" FROM "NOTIFICATIONRULE_PROJECTS");
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

// Does not work with H2, but verified on Postgres
if (Config.getInstance().getProperty(Config.AlpineKey.DATABASE_DRIVER).equals("org.postgresql.Driver")) {
sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
WITH v_ids AS (
SELECT "VIOLATIONANALYSIS"."ID"
FROM "VIOLATIONANALYSIS"
JOIN "PROJECT" ON "PROJECT_ID" = "PROJECT"."ID"
WHERE "PROJECT"."UUID" = ANY(?)
)
DELETE FROM "VIOLATIONANALYSISCOMMENT"
USING v_ids
WHERE "VIOLATIONANALYSIS_ID" = v_ids."ID";
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);
}

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "VIOLATIONANALYSIS" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "POLICYVIOLATION" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "POLICY_PROJECTS" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "POLICY"
WHERE "ID" NOT IN (SELECT "POLICY_ID" FROM "POLICY_PROJECTS");
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "PROJECT_ACCESS_TEAMS" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "VEX" WHERE "PROJECT_ID" IN (
SELECT "ID" FROM "PROJECT" WHERE "UUID" = ANY(?)
);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

// The below has only been tested with Postgres, but should work on any RDBMS supporting SQL:1999
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For completeness, how many RDBMSes have you tested with this? Having tested at least H2, MSSQL, and PostgreSQL would be good. We have a Docker Compose file for MSSQL here: https://github.com/DependencyTrack/dependency-track/blob/master/dev/docker-compose.mssql.yml

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've only tested it with H2 and PostgreSQL so far (H2 in unit tests, PostgreSQL in system tests) but I can try migrating my test data to MSSQL and test that as well. I'll get back to you with the results when done!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to do a full load test or anything. We just need assurance that the queries work as expected. Sadly the small differences in SQL syntax between the different RDBMSes turn out to be tripwires more often than not. :D

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, true, and true :) But I'll need some data in the schema to pass the condition that the UUID list is not empty, so I'll try to migrate the relatively small exported sample that I created with pg_sample.

if (Config.getInstance().getProperty(Config.AlpineKey.DATABASE_DRIVER).equals("org.postgresql.Driver")) {
sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
WITH sc_cte AS (
SELECT "SERVICECOMPONENT"."ID" FROM "SERVICECOMPONENT"
JOIN "PROJECT" ON "SERVICECOMPONENT"."PROJECT_ID" = "PROJECT"."ID"
WHERE "PROJECT"."UUID" = ANY(?)
)
DELETE FROM "SERVICECOMPONENTS_VULNERABILITIES"
USING sc_cte
WHERE "SERVICECOMPONENTS_VULNERABILITIES"."SERVICECOMPONENT_ID" = sc_cte."ID";
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
WITH RECURSIVE sc_family("ID", "PARENT_SERVICECOMPONENT_ID") AS (
SELECT "SERVICECOMPONENT"."ID", "PARENT_SERVICECOMPONENT_ID"
FROM "SERVICECOMPONENT"
JOIN "PROJECT" ON "PROJECT"."ID" = "SERVICECOMPONENT"."PROJECT_ID"
WHERE "PROJECT"."UUID" = ANY(?)
UNION ALL
SELECT "SERVICECOMPONENT"."ID", "SERVICECOMPONENT"."PARENT_SERVICECOMPONENT_ID"
FROM sc_family, "SERVICECOMPONENT"
WHERE "SERVICECOMPONENT"."ID" = sc_family."PARENT_SERVICECOMPONENT_ID"
)
DELETE FROM "SERVICECOMPONENT"
WHERE "ID" = ANY(SELECT "ID" FROM sc_family);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
WITH RECURSIVE p_family("ID", "PARENT_PROJECT_ID") AS (
SELECT "PROJECT"."ID", "PARENT_PROJECT_ID"
FROM "PROJECT"
WHERE "PROJECT"."UUID" = ANY(?)
UNION ALL
SELECT "PROJECT"."ID", "PROJECT"."PARENT_PROJECT_ID"
FROM p_family, "PROJECT"
WHERE "PROJECT"."ID" = p_family."PARENT_PROJECT_ID"
)
DELETE FROM "PROJECT"
WHERE "ID" = ANY(SELECT "ID" FROM p_family);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);
}

sqlQuery = pm.newQuery(JDOQuery.SQL_QUERY_LANGUAGE, """
DELETE FROM "PROJECT" WHERE "UUID" = ANY(?);
""");
executeAndCloseWithArray(sqlQuery, (Object) uuidsArray);
}
});
}


/**
* Creates a key/value pair (ProjectProperty) for the specified Project.
* @param project the Project to create the property for
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/dependencytrack/persistence/QueryManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,10 @@ public void recursivelyDelete(final Project project, final boolean commitIndex)
getProjectQueryManager().recursivelyDelete(project, commitIndex);
}

public void deleteProjectsByUUIDs(List<UUID> uuids) {
getProjectQueryManager().deleteProjectsByUUIDs(uuids);
}

public ProjectProperty createProjectProperty(final Project project, final String groupName, final String propertyName,
final String propertyValue, final ProjectProperty.PropertyType propertyType,
final String description) {
Expand Down
52 changes: 52 additions & 0 deletions src/main/java/org/dependencytrack/resources/v1/ProjectResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@
import jakarta.ws.rs.core.Response;
import javax.jdo.FetchGroup;
import java.security.Principal;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -804,6 +806,56 @@ public Response deleteProject(
}
}

@POST
@Path("/batchDelete")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
@Operation(
summary = "Deletes a list of projects specified by their UUIDs",
description = "<p>Requires permission <strong>PORTFOLIO_MANAGEMENT</strong></p>"
)
@ApiResponses(value = {
@ApiResponse(responseCode = "204", description = "Projects removed successfully"),
@ApiResponse(responseCode = "207", description = "Access is forbidden to the projects listed in the response"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sold on this status code. Some surface-level research leads me to believe using this outside of WebDAV is not intended: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/207

Wouldn't it be better to atomically fail the request if deletion of one or more projects failed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, the 207 indeed seems to be reserved for WebDAV. The reason for implementing it this way was based on the outcome of this discussion, where the suggestion by sebD was to return a 204 with a body containing the list of inaccessible projects. The 207 was my suggestion, and since I got a thumbs up I thought I would do it the same way in this PR. I see a value in letting the client know which projects that are inaccessible, so I'm leaning towards going with sebD's suggestion instead of just returning a 401. Would that be an acceptable solution?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think sebD's original comment was geared towards the situation where we proceed with deleting accessible projects when there have been inaccessible projects in the provided list. In that case you do need to communicate the partial success somehow.

My point is that no projects should be deleted at all, if at least one of them is inaccessible. You can then still respond with 401, and you could use ProblemDetails to communicate the inaccessible projects in a machine-readable way.

We do this for tag endpoints already:

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll look into using ProblemDetails since that will make it consistent with how failed tag deletion is handled. Thanks for the tip!

@ApiResponse(responseCode = "401", description = "Unauthorized")
})
@PermissionRequired(Permissions.Constants.PORTFOLIO_MANAGEMENT)
public Response deleteProjects(List<UUID> uuids) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using @Size(min = 1, max = ...) here. The limits should then also appear in the OpenAPI spec automatically.

I think there should be some upper limit to the number of UUIDs being submitted in one go. We can always extend it later if people need it, but we can't reduce it later if larger batches end up causing problems.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. I have tested it with up to 1000 UUIDs, so maybe that could be a reasonable max size to start with?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that sounds reasonable.

List<UUID> inaccessibleProjects = new ArrayList<>();
try (QueryManager qm = new QueryManager()) {
for (Iterator<UUID> it = uuids.iterator(); it.hasNext();) {
UUID uuid = it.next();
final Project project = qm.getObjectByUuid(Project.class, uuid, Project.FetchGroup.ALL.name());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for using FetchGroup.ALL here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, that doesn't seem to be needed since Project.accessTeams is part of the default fetch group. ProjectQueryManager.hasAccess only needs that property from the project, so I'll change to the getObjectByUuid(Class, UUID) signature instead.

if (project != null) {
if (!qm.hasAccess(super.getPrincipal(), project)) {
LOGGER.warn(super.getPrincipal().getName() + " lacks access to project " + project
+ ", delete request denied"
);
inaccessibleProjects.add(uuid);
it.remove();
}
} else {
LOGGER.warn("No project found by UUID " + uuid);
it.remove();
}
}

if (uuids.isEmpty()) {
return Response.status(Response.Status.FORBIDDEN)
.entity(inaccessibleProjects)
.build();
}

qm.deleteProjectsByUUIDs(uuids);

if (inaccessibleProjects.isEmpty()) {
return Response.status(Response.Status.NO_CONTENT).build();
} else {
return Response.status(207).entity(inaccessibleProjects).build();
}
}
}

@PUT
@Path("/clone")
@Consumes(MediaType.APPLICATION_JSON)
Expand Down
Loading
Loading