From d8f8df7dadd9e4c17ba2e7ec5a32d241b9acbc9a Mon Sep 17 00:00:00 2001 From: nscuro Date: Fri, 7 Mar 2025 14:37:57 +0100 Subject: [PATCH] Materialize project hierarchies in separate table Signed-off-by: nscuro --- .../persistence/PolicyQueryManager.java | 65 ---------- .../persistence/ProjectQueryManager.java | 84 +++++------- .../persistence/QueryManager.java | 8 -- .../resources/migration/changelog-v5.6.0.xml | 121 ++++++++++++++++++ .../function_has-project-access.sql | 19 +-- .../persistence/PolicyQueryManagerTest.java | 60 +-------- .../resources/v1/ProjectResourceTest.java | 2 - 7 files changed, 159 insertions(+), 200 deletions(-) diff --git a/src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java b/src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java index 919465850..51acc1a27 100644 --- a/src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java @@ -95,71 +95,6 @@ public List getAllPolicies() { return query.executeList(); } - /** - * Fetch all {@link Policy}s that are applicable to a given {@link Project}. - * - * @param project The {@link Project} to fetch {@link Policy}s for - * @return A {@link List} of {@link Policy}s. - * @since 5.0.0 - */ - public List getApplicablePolicies(final Project project) { - var filter = """ - (this.projects.isEmpty() && this.tags.isEmpty()) - || (this.projects.contains(:project) - """; - var params = new HashMap(); - params.put("project", project); - - // To compensate for missing support for recursion of Common Table Expressions (CTEs) - // in JDO, we have to fetch the UUIDs of all parent projects upfront. Otherwise, we'll - // not be able to evaluate whether the policy is inherited from parent projects. - var variables = ""; - final List parentUuids = getParents(project); - if (!parentUuids.isEmpty()) { - filter += """ - || (this.includeChildren - && this.projects.contains(parentVar) - && :parentUuids.contains(parentVar.uuid)) - """; - variables += "org.dependencytrack.model.Project parentVar"; - params.put("parentUuids", parentUuids); - } - filter += ")"; - - // DataNucleus generates an invalid SQL query when using the idiomatic solution. - // The following works, but it's ugly and likely doesn't perform well if the project - // has many tags. Worth trying the idiomatic way again once DN has been updated to > 6.0.4. - // - // filter += "m|| (this.tags.contains(commonTag) && :project.tags.contains(commonTag))"; - // variables += "org.dependencytrack.model.Tag commonTag"; - if (project.getTags() != null && !project.getTags().isEmpty()) { - filter += " || ("; - for (int i = 0; i < project.getTags().size(); i++) { - filter += "this.tags.contains(:tag" + i + ")"; - params.put("tag" + i, project.getTags().get(i)); - if (i < (project.getTags().size() - 1)) { - filter += " || "; - } - } - filter += ")"; - } - - final List policies; - final Query query = pm.newQuery(Policy.class); - try { - query.setFilter(filter); - query.setNamedParameters(params); - if (!variables.isEmpty()) { - query.declareVariables(variables); - } - policies = List.copyOf(query.executeList()); - } finally { - query.closeAll(); - } - - return policies; - } - /** * Returns a policy by it's name. * @param name the name of the policy (required) diff --git a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java index 56be45698..21697b15e 100644 --- a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java @@ -1242,37 +1242,6 @@ public PaginatedResult getProjectsWithoutDescendantsOf(final String name, final return result; } - /** - * Fetch the {@link UUID}s of all parents of a given {@link Project}. - * - * @param project The {@link Project} to fetch the parent {@link UUID}s for - * @return A {@link List} of {@link UUID}s - */ - @Override - public List getParents(final Project project) { - return getParents(project.getUuid(), new ArrayList<>()); - } - - private List getParents(final UUID uuid, final List parents) { - final UUID parentUuid; - final Query query = pm.newQuery(Project.class); - try { - query.setFilter("uuid == :uuid && parent != null"); - query.setParameters(uuid); - query.setResult("parent.uuid"); - parentUuid = query.executeResultUnique(UUID.class); - } finally { - query.closeAll(); - } - - if (parentUuid == null) { - return parents; - } - - parents.add(parentUuid); - return getParents(parentUuid, parents); - } - /** * Check whether a {@link Project} with a given {@code name} and {@code version} exists. * @@ -1307,30 +1276,43 @@ public boolean doesProjectExist(final String name, final String version) { } } - private static boolean isChildOf(Project project, UUID uuid) { - boolean isChild = false; - if (project.getParent() != null) { - if (project.getParent().getUuid().equals(uuid)) { - return true; - } else { - isChild = isChildOf(project.getParent(), uuid); - } + private boolean isChildOf(Project project, UUID uuid) { + final Query query = pm.newQuery(Query.SQL, /* language=SQL */ """ + SELECT EXISTS( + SELECT 1 + FROM "PROJECT_HIERARCHY" + WHERE "PARENT_PROJECT_ID" = (SELECT "ID" FROM "PROJECT" WHERE "UUID" = ?) + AND "CHILD_PROJECT_ID" = ? + ) + """); + query.setParameters(uuid, project.getId()); + try { + return (boolean) query.executeUnique(); + } finally { + query.closeAll(); } - return isChild; } - private static boolean hasActiveChild(Project project) { - boolean hasActiveChild = false; - if (project.getChildren() != null) { - for (Project child : project.getChildren()) { - if (child.isActive() || hasActiveChild) { - return true; - } else { - hasActiveChild = hasActiveChild(child); - } - } + private boolean hasActiveChild(Project project) { + final Query query = pm.newQuery(Query.SQL, /* language=SQL */ """ + SELECT EXISTS( + SELECT 1 + FROM "PROJECT" AS "PARENT_PROJECT" + INNER JOIN "PROJECT_HIERARCHY" + ON "PROJECT_HIERARCHY"."PARENT_PROJECT_ID" = "PARENT_PROJECT"."ID" + INNER JOIN "PROJECT" AS "CHILD_PROJECT" + ON "CHILD_PROJECT"."ID" = "PROJECT_HIERARCHY"."CHILD_PROJECT_ID" + WHERE "PARENT_PROJECT"."ID" = ? + AND "CHILD_PROJECT"."INACTIVE_SINCE" IS NULL + AND "PROJECT_HIERARCHY"."DEPTH" > 0 + ) + """); + query.setParameters(project.getId()); + try { + return (boolean) query.executeUnique(); + } finally { + query.closeAll(); } - return hasActiveChild; } private List getProjectVersions(Project project) { diff --git a/src/main/java/org/dependencytrack/persistence/QueryManager.java b/src/main/java/org/dependencytrack/persistence/QueryManager.java index a02729aa7..6b3daeb7f 100644 --- a/src/main/java/org/dependencytrack/persistence/QueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/QueryManager.java @@ -566,10 +566,6 @@ public PaginatedResult getProjectsWithoutDescendantsOf(final String name, final return getProjectQueryManager().getProjectsWithoutDescendantsOf(name, excludeInactive, project); } - public List getParents(final Project project) { - return getProjectQueryManager().getParents(project); - } - public boolean hasAccess(final Principal principal, final Project project) { return getProjectQueryManager().hasAccess(principal, project); } @@ -781,10 +777,6 @@ public List getAllPolicies() { return getPolicyQueryManager().getAllPolicies(); } - public List getApplicablePolicies(final Project project) { - return getPolicyQueryManager().getApplicablePolicies(project); - } - public Policy getPolicy(final String name) { return getPolicyQueryManager().getPolicy(name); } diff --git a/src/main/resources/migration/changelog-v5.6.0.xml b/src/main/resources/migration/changelog-v5.6.0.xml index ef25e3fc6..b6782dde6 100644 --- a/src/main/resources/migration/changelog-v5.6.0.xml +++ b/src/main/resources/migration/changelog-v5.6.0.xml @@ -636,4 +636,125 @@ onDelete="CASCADE" onUpdate="NO ACTION" referencedColumnNames="ID" referencedTableName="WORKFLOW_STATE" validate="true"/> + + + + + + + + + + + + + + + + create function project_hierarchy_maintenance_on_project_insert() + returns trigger as $$ + begin + insert into "PROJECT_HIERARCHY"("PARENT_PROJECT_ID", "CHILD_PROJECT_ID", "DEPTH") + values(new."ID", new."ID", 0); + + insert into "PROJECT_HIERARCHY"("PARENT_PROJECT_ID", "CHILD_PROJECT_ID", "DEPTH") + select "PARENT_PROJECT_ID", new."ID", "DEPTH" + 1 + from "PROJECT_HIERARCHY" + where "CHILD_PROJECT_ID" = new."PARENT_PROJECT_ID"; + + return new; + end; + $$ language plpgsql; + + + + create function project_hierarchy_maintenance_on_project_update() + returns trigger as $$ + begin + delete from "PROJECT_HIERARCHY" where "CHILD_PROJECT_ID" = old."ID"; + + insert into "PROJECT_HIERARCHY"("PARENT_PROJECT_ID", "CHILD_PROJECT_ID", "DEPTH") + values(new."ID", new."ID", 0); + + insert into "PROJECT_HIERARCHY"("PARENT_PROJECT_ID", "CHILD_PROJECT_ID", "DEPTH") + select "PARENT_PROJECT_ID", new."ID", "DEPTH" + 1 + from "PROJECT_HIERARCHY" + where "CHILD_PROJECT_ID" = new."PARENT_PROJECT_ID"; + + return new; + end; + $$ language plpgsql; + + + + create function project_hierarchy_maintenance_on_project_delete() + returns trigger as $$ + begin + delete from "PROJECT_HIERARCHY" + where "PARENT_PROJECT_ID" in (select "ID" from old_table) + or "CHILD_PROJECT_ID" in (select "ID" from old_table); + + return null; + end; + $$ language plpgsql; + + + + create trigger trigger_project_hierarchy_maintenance_on_project_insert + after insert on "PROJECT" + for each row + execute function project_hierarchy_maintenance_on_project_insert(); + + create trigger trigger_project_hierarchy_maintenance_on_project_update + after update of "PARENT_PROJECT_ID" on "PROJECT" + for each row + execute function project_hierarchy_maintenance_on_project_update(); + + create trigger trigger_project_hierarchy_maintenance_on_project_delete + after delete on "PROJECT" + referencing old table as old_table + for each statement + execute function project_hierarchy_maintenance_on_project_delete(); + + + + + insert into "PROJECT_HIERARCHY"("PARENT_PROJECT_ID", "CHILD_PROJECT_ID", "DEPTH") + select "ID", "ID", 0 from "PROJECT"; + + with recursive cte_project_hierarchy as( + select "ID" as child_id + , "PARENT_PROJECT_ID" as parent_id + , 1 as depth + from "PROJECT" + union all + select child_id + , "PARENT_PROJECT_ID" as parent_id + , depth + 1 as depth + from cte_project_hierarchy + inner join "PROJECT" + on "PROJECT"."ID" = cte_project_hierarchy.parent_id + where "PROJECT"."PARENT_PROJECT_ID" is not null + ) + insert into "PROJECT_HIERARCHY"("PARENT_PROJECT_ID", "CHILD_PROJECT_ID", "DEPTH") + select parent_id, child_id, depth from cte_project_hierarchy; + + \ No newline at end of file diff --git a/src/main/resources/migration/procedures/function_has-project-access.sql b/src/main/resources/migration/procedures/function_has-project-access.sql index 25dd6b486..d425605e5 100644 --- a/src/main/resources/migration/procedures/function_has-project-access.sql +++ b/src/main/resources/migration/procedures/function_has-project-access.sql @@ -7,23 +7,12 @@ create or replace function has_project_access( stable as $$ -with recursive project_hierarchy(id, parent_id) as( - select "ID" as id - , "PARENT_PROJECT_ID" as parent_id - from "PROJECT" - where "ID" = project_id - union all - select "PROJECT"."ID" as id - , "PROJECT"."PARENT_PROJECT_ID" as parent_id - from "PROJECT" - inner join project_hierarchy - on project_hierarchy.parent_id = "PROJECT"."ID" -) select exists( select 1 - from project_hierarchy - inner join "PROJECT_ACCESS_TEAMS" - on "PROJECT_ACCESS_TEAMS"."PROJECT_ID" = project_hierarchy.id + from "PROJECT_ACCESS_TEAMS" + inner join "PROJECT_HIERARCHY" + on "PROJECT_HIERARCHY"."PARENT_PROJECT_ID" = "PROJECT_ACCESS_TEAMS"."PROJECT_ID" where "PROJECT_ACCESS_TEAMS"."TEAM_ID" = any(team_ids) + and "PROJECT_HIERARCHY"."CHILD_PROJECT_ID" = project_id ) $$; \ No newline at end of file diff --git a/src/test/java/org/dependencytrack/persistence/PolicyQueryManagerTest.java b/src/test/java/org/dependencytrack/persistence/PolicyQueryManagerTest.java index b37c1dabf..36c8a1429 100644 --- a/src/test/java/org/dependencytrack/persistence/PolicyQueryManagerTest.java +++ b/src/test/java/org/dependencytrack/persistence/PolicyQueryManagerTest.java @@ -23,12 +23,11 @@ import org.dependencytrack.model.Policy; import org.dependencytrack.model.PolicyViolation; import org.dependencytrack.model.Project; -import org.dependencytrack.model.Tag; import org.dependencytrack.model.ViolationAnalysis; import org.dependencytrack.model.ViolationAnalysisComment; import org.dependencytrack.model.ViolationAnalysisState; -import org.junit.Test; import org.junit.Assert; +import org.junit.Test; import java.util.ArrayList; import java.util.Date; @@ -38,63 +37,6 @@ public class PolicyQueryManagerTest extends PersistenceCapableTest { - @Test - public void testGetApplicablePolicies() { - final var grandParentProject = new Project(); - grandParentProject.setName("grandParent"); - qm.persist(grandParentProject); - - final var parentProject = new Project(); - parentProject.setParent(grandParentProject); - parentProject.setName("parent"); - qm.persist(parentProject); - - final var childProject = new Project(); - childProject.setParent(parentProject); - childProject.setName("child"); - qm.persist(childProject); - - final Tag policyTag = qm.createTag("foo"); - qm.bind(parentProject, List.of(policyTag)); - - final Tag nonPolicyTag = qm.createTag("bar"); - qm.bind(childProject, List.of(nonPolicyTag)); - - final var globalPolicy = new Policy(); - globalPolicy.setName("globalPolicy"); - globalPolicy.setOperator(Policy.Operator.ANY); - globalPolicy.setViolationState(Policy.ViolationState.FAIL); - qm.persist(globalPolicy); - - final var tagPolicy = new Policy(); - tagPolicy.setName("tagPolicy"); - tagPolicy.setOperator(Policy.Operator.ANY); - tagPolicy.setViolationState(Policy.ViolationState.FAIL); - qm.persist(tagPolicy); - qm.bind(tagPolicy, List.of(policyTag)); - - final var inheritedPolicy = new Policy(); - inheritedPolicy.setName("inheritedPolicy"); - inheritedPolicy.setOperator(Policy.Operator.ANY); - inheritedPolicy.setViolationState(Policy.ViolationState.FAIL); - inheritedPolicy.setProjects(List.of(parentProject)); - inheritedPolicy.setIncludeChildren(true); - qm.persist(inheritedPolicy); - - assertThat(qm.getApplicablePolicies(grandParentProject)).satisfiesExactly( - policy -> assertThat(policy.getName()).isEqualTo("globalPolicy") - ); - assertThat(qm.getApplicablePolicies(parentProject)).satisfiesExactly( - policy -> assertThat(policy.getName()).isEqualTo("globalPolicy"), - policy -> assertThat(policy.getName()).isEqualTo("tagPolicy"), - policy -> assertThat(policy.getName()).isEqualTo("inheritedPolicy") - ); - assertThat(qm.getApplicablePolicies(childProject)).satisfiesExactly( - policy -> assertThat(policy.getName()).isEqualTo("globalPolicy"), - policy -> assertThat(policy.getName()).isEqualTo("inheritedPolicy") - ); - } - @Test public void testRemoveProjectFromPolicies() { final Project project = qm.createProject("ACME Example", null, "1.0", null, null, null, null, false); diff --git a/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java index 1db7f7c7e..44433a8f7 100644 --- a/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java +++ b/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java @@ -3629,7 +3629,6 @@ public void patchActiveProjectToInactiveTest() { { "uuid": "${json-unit.any-string}", "name": "ABC-Updated", - "children": [], "properties": [], "tags": [], "inactiveSince": "${json-unit.any-number}", @@ -3694,7 +3693,6 @@ public void updateActiveProjectToInactiveTest() { "uuid": "${json-unit.any-string}", "name": "ABC-Updated", "classifier":"APPLICATION", - "children": [], "properties": [], "tags": [], "inactiveSince": "${json-unit.any-number}",