From 68b6c456e8c134c4038806e74de3c27b05f4e09c Mon Sep 17 00:00:00 2001 From: Sahiba Mittal Date: Tue, 28 Jan 2025 11:51:01 +0000 Subject: [PATCH 1/2] Fix REST endpoints for adding tags Co-Authored-By: Niklas --- .../persistence/NotificationQueryManager.java | 24 +++++--- .../persistence/PolicyQueryManager.java | 24 +++++--- .../persistence/ProjectQueryManager.java | 55 +++++++++++++------ .../persistence/QueryManager.java | 12 ++++ .../persistence/TagQueryManager.java | 13 +---- .../resources/v1/TagResourceTest.java | 54 +++++++++++++++--- 6 files changed, 133 insertions(+), 49 deletions(-) diff --git a/src/main/java/org/dependencytrack/persistence/NotificationQueryManager.java b/src/main/java/org/dependencytrack/persistence/NotificationQueryManager.java index e1a53bfd8..92b21d082 100644 --- a/src/main/java/org/dependencytrack/persistence/NotificationQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/NotificationQueryManager.java @@ -257,21 +257,23 @@ public void deleteNotificationPublisher(final NotificationPublisher notification } /** - * @since 4.12.0 + * @since 4.12.3 */ @Override - public boolean bind(final NotificationRule notificationRule, final Collection tags) { + public boolean bind(final NotificationRule notificationRule, final Collection tags, final boolean keepExisting) { assertPersistent(notificationRule, "notificationRule must be persistent"); assertPersistentAll(tags, "tags must be persistent"); return callInTransaction(() -> { boolean modified = false; - for (final Tag existingTag : notificationRule.getTags()) { - if (!tags.contains(existingTag)) { - notificationRule.getTags().remove(existingTag); - existingTag.getNotificationRules().remove(notificationRule); - modified = true; + if (!keepExisting) { + for (final Tag existingTag : notificationRule.getTags()) { + if (!tags.contains(existingTag)) { + notificationRule.getTags().remove(existingTag); + existingTag.getNotificationRules().remove(notificationRule); + modified = true; + } } } for (final Tag tag : tags) { @@ -290,4 +292,12 @@ public boolean bind(final NotificationRule notificationRule, final Collection tags) { + return bind(notificationRule, tags, /* keepExisting */ false); + } } diff --git a/src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java b/src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java index 9e16665c6..55b806d35 100644 --- a/src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java @@ -693,20 +693,22 @@ public long getAuditedCount(final Component component, final PolicyViolation.Typ } /** - * @since 4.12.0 + * @since 4.12.3 */ @Override - public boolean bind(final Policy policy, final Collection tags) { + public boolean bind(final Policy policy, final Collection tags, final boolean keepExisting) { assertPersistent(policy, "policy must be persistent"); assertPersistentAll(tags, "tags must be persistent"); return callInTransaction(() -> { boolean modified = false; - for (final Tag existingTag : policy.getTags()) { - if (!tags.contains(existingTag)) { - policy.getTags().remove(existingTag); - existingTag.getPolicies().remove(policy); - modified = true; + if (!keepExisting) { + for (final Tag existingTag : policy.getTags()) { + if (!tags.contains(existingTag)) { + policy.getTags().remove(existingTag); + existingTag.getPolicies().remove(policy); + modified = true; + } } } @@ -725,6 +727,14 @@ public boolean bind(final Policy policy, final Collection tags) { }); } + /** + * @since 4.12.0 + */ + @Override + public boolean bind(final Policy policy, final Collection tags) { + return bind(policy, tags, /* keepExisting */ false); + } + private void processViolationsFilters(Map filters, Map params, List filterCriteria) { for (Map.Entry filter : filters.entrySet()) { switch (filter.getKey()) { diff --git a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java index 6bd764c67..40582f798 100644 --- a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java @@ -61,6 +61,7 @@ import java.io.IOException; import java.security.Principal; import java.util.ArrayList; +import java.util.Collection; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -70,6 +71,9 @@ import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; +import static org.dependencytrack.util.PersistenceUtil.assertPersistent; +import static org.dependencytrack.util.PersistenceUtil.assertPersistentAll; + final class ProjectQueryManager extends QueryManager implements IQueryManager { private static final Logger LOGGER = Logger.getLogger(ProjectQueryManager.class); @@ -921,33 +925,52 @@ public List getProjectProperties(final Project project) { } /** - * Binds the two objects together in a corresponding join table. - * - * @param project a Project object - * @param tags a List of Tag objects + * @since 4.12.3 */ @Override - public void bind(Project project, List tags) { - runInTransaction(() -> { - final Query query = pm.newQuery(Tag.class, "projects.contains(:project)"); - query.setParameters(project); - final List currentProjectTags = executeAndCloseList(query); + public boolean bind(final Project project, final Collection tags, final boolean keepExisting) { + assertPersistent(project, "project must be persistent"); + assertPersistentAll(tags, "tags must be persistent"); - for (final Tag tag : currentProjectTags) { - if (!tags.contains(tag)) { - tag.getProjects().remove(project); + return callInTransaction(() -> { + boolean modified = false; + + if (!keepExisting) { + for (final Tag existingTag : project.getTags()) { + if (!tags.contains(existingTag)) { + project.getTags().remove(existingTag); + existingTag.getProjects().remove(project); + modified = true; + } } } - project.setTags(tags); for (final Tag tag : tags) { - final List projects = tag.getProjects(); - if (!projects.contains(project)) { - projects.add(project); + if (!project.getTags().contains(tag)) { + project.getTags().add(tag); + + if (tag.getProjects() == null) { + tag.setProjects(new ArrayList<>(List.of(project))); + } else if (!tag.getProjects().contains(project)) { + tag.getProjects().add(project); + } + + modified = true; } } + return modified; }); } + /** + * Binds the two objects together in a corresponding join table. + * @param project a Project object + * @param tags a List of Tag objects + */ + @Override + public void bind(final Project project, final List tags) { + bind(project, tags, /* keepExisting */ false); + } + /** * Updates the last time a bom was imported. * diff --git a/src/main/java/org/dependencytrack/persistence/QueryManager.java b/src/main/java/org/dependencytrack/persistence/QueryManager.java index ed07d19d4..f68d3f062 100644 --- a/src/main/java/org/dependencytrack/persistence/QueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/QueryManager.java @@ -1411,14 +1411,26 @@ public boolean isEnabled(final ConfigPropertyConstants configPropertyConstants) return false; } + public boolean bind(final Project project, final Collection tags, final boolean keepExisting) { + return getProjectQueryManager().bind(project, tags, keepExisting); + } + public void bind(Project project, List tags) { getProjectQueryManager().bind(project, tags); } + public boolean bind(final Policy policy, final Collection tags, final boolean keepExisting) { + return getPolicyQueryManager().bind(policy, tags, keepExisting); + } + public boolean bind(final Policy policy, final Collection tags) { return getPolicyQueryManager().bind(policy, tags); } + public boolean bind(final NotificationRule notificationRule, final Collection tags, final boolean keepExisting) { + return getNotificationQueryManager().bind(notificationRule, tags, keepExisting); + } + public boolean bind(final NotificationRule notificationRule, final Collection tags) { return getNotificationQueryManager().bind(notificationRule, tags); } diff --git a/src/main/java/org/dependencytrack/persistence/TagQueryManager.java b/src/main/java/org/dependencytrack/persistence/TagQueryManager.java index ffc489b57..812dfd382 100644 --- a/src/main/java/org/dependencytrack/persistence/TagQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/TagQueryManager.java @@ -383,14 +383,7 @@ public void tagProjects(final String tagName, final Collection projectUu final List projects = executeAndCloseList(projectsQuery); for (final Project project : projects) { - if (project.getTags() == null || project.getTags().isEmpty()) { - project.setTags(List.of(tag)); - continue; - } - - if (!project.getTags().contains(tag)) { - project.getTags().add(tag); - } + bind(project, List.of(tag), /* keepExisting */ true); } }); } @@ -487,7 +480,7 @@ public void tagPolicies(final String tagName, final Collection policyUui final List policies = executeAndCloseList(policiesQuery); for (final Policy policy : policies) { - bind(policy, List.of(tag)); + bind(policy, List.of(tag), /* keepExisting */ true); } }); } @@ -695,7 +688,7 @@ public void tagNotificationRules(final String tagName, final Collection final List notificationRules = executeAndCloseList(notificationRulesQuery); for (final NotificationRule notificationRule : notificationRules) { - bind(notificationRule, List.of(tag)); + bind(notificationRule, List.of(tag), /* keepExisting */ true); } }); } diff --git a/src/test/java/org/dependencytrack/resources/v1/TagResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/TagResourceTest.java index c70cb4897..6624eb06c 100644 --- a/src/test/java/org/dependencytrack/resources/v1/TagResourceTest.java +++ b/src/test/java/org/dependencytrack/resources/v1/TagResourceTest.java @@ -661,15 +661,26 @@ public void tagProjectsTest() { qm.createTag("foo"); + final var projectC = new Project(); + projectC.setName("acme-app-c"); + qm.persist(projectC); + + qm.bind(projectC, List.of(qm.createTag("bar"))); + final Response response = jersey.target(V1_TAG + "/foo/project") .request() .header(X_API_KEY, apiKey) - .post(Entity.json(List.of(projectA.getUuid(), projectB.getUuid()))); + .post(Entity.json(List.of(projectA.getUuid(), projectB.getUuid(), projectC.getUuid()))); assertThat(response.getStatus()).isEqualTo(204); qm.getPersistenceManager().evictAll(); - assertThat(projectA.getTags()).satisfiesExactly(projectTag -> assertThat(projectTag.getName()).isEqualTo("foo")); - assertThat(projectB.getTags()).satisfiesExactly(projectTag -> assertThat(projectTag.getName()).isEqualTo("foo")); + assertThat(projectA.getTags()).satisfiesExactly( + projectTag -> assertThat(projectTag.getName()).isEqualTo("foo")); + assertThat(projectB.getTags()).satisfiesExactly( + projectTag -> assertThat(projectTag.getName()).isEqualTo("foo")); + assertThat(projectC.getTags()).satisfiesExactlyInAnyOrder( + projectTag -> assertThat(projectTag.getName()).isEqualTo("foo"), + projectTag -> assertThat(projectTag.getName()).isEqualTo("bar")); } @Test @@ -1071,15 +1082,28 @@ public void tagPoliciesTest() { qm.createTag("foo"); + final var policyC = new Policy(); + policyC.setName("policy-c"); + policyC.setOperator(Policy.Operator.ALL); + policyC.setViolationState(Policy.ViolationState.INFO); + qm.persist(policyC); + + qm.bind(policyC, List.of(qm.createTag("bar"))); + final Response response = jersey.target(V1_TAG + "/foo/policy") .request() .header(X_API_KEY, apiKey) - .post(Entity.json(List.of(policyA.getUuid(), policyB.getUuid()))); + .post(Entity.json(List.of(policyA.getUuid(), policyB.getUuid(), policyC.getUuid()))); assertThat(response.getStatus()).isEqualTo(204); qm.getPersistenceManager().evictAll(); - assertThat(policyA.getTags()).satisfiesExactly(policyTag -> assertThat(policyTag.getName()).isEqualTo("foo")); - assertThat(policyB.getTags()).satisfiesExactly(policyTag -> assertThat(policyTag.getName()).isEqualTo("foo")); + assertThat(policyA.getTags()).satisfiesExactly( + policyTag -> assertThat(policyTag.getName()).isEqualTo("foo")); + assertThat(policyB.getTags()).satisfiesExactly( + policyTag -> assertThat(policyTag.getName()).isEqualTo("foo")); + assertThat(policyC.getTags()).satisfiesExactlyInAnyOrder( + policyTag -> assertThat(policyTag.getName()).isEqualTo("foo"), + policyTag -> assertThat(policyTag.getName()).isEqualTo("bar")); } @Test @@ -1536,15 +1560,27 @@ public void tagNotificationRulesTest() { qm.createTag("foo"); + final var notificationRuleC = new NotificationRule(); + notificationRuleC.setName("rule-c"); + notificationRuleC.setScope(NotificationScope.PORTFOLIO); + qm.persist(notificationRuleC); + + qm.bind(notificationRuleC, List.of(qm.createTag("bar"))); + final Response response = jersey.target(V1_TAG + "/foo/notificationRule") .request() .header(X_API_KEY, apiKey) - .post(Entity.json(List.of(notificationRuleA.getUuid(), notificationRuleB.getUuid()))); + .post(Entity.json(List.of(notificationRuleA.getUuid(), notificationRuleB.getUuid(), notificationRuleC.getUuid()))); assertThat(response.getStatus()).isEqualTo(204); qm.getPersistenceManager().evictAll(); - assertThat(notificationRuleA.getTags()).satisfiesExactly(ruleTag -> assertThat(ruleTag.getName()).isEqualTo("foo")); - assertThat(notificationRuleB.getTags()).satisfiesExactly(ruleTag -> assertThat(ruleTag.getName()).isEqualTo("foo")); + assertThat(notificationRuleA.getTags()).satisfiesExactly( + ruleTag -> assertThat(ruleTag.getName()).isEqualTo("foo")); + assertThat(notificationRuleB.getTags()).satisfiesExactly( + ruleTag -> assertThat(ruleTag.getName()).isEqualTo("foo")); + assertThat(notificationRuleC.getTags()).satisfiesExactlyInAnyOrder( + ruleTag -> assertThat(ruleTag.getName()).isEqualTo("foo"), + ruleTag -> assertThat(ruleTag.getName()).isEqualTo("bar")); } @Test From e95dc317c56d5b5f76b7737f4267bf70ae1bafba Mon Sep 17 00:00:00 2001 From: Sahiba Mittal Date: Fri, 7 Feb 2025 16:06:49 +0000 Subject: [PATCH 2/2] fix test --- .../resources/v1/ProjectResource.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/dependencytrack/resources/v1/ProjectResource.java b/src/main/java/org/dependencytrack/resources/v1/ProjectResource.java index 46170fbe9..07ddbfd59 100644 --- a/src/main/java/org/dependencytrack/resources/v1/ProjectResource.java +++ b/src/main/java/org/dependencytrack/resources/v1/ProjectResource.java @@ -644,13 +644,6 @@ public Response updateProject(Project jsonProject) { .entity(e.getMessage()) .build()); } catch (RuntimeException e) { - if (isUniqueConstraintViolation(e)) { - throw new ClientErrorException(Response - .status(Response.Status.CONFLICT) - .entity("A project with the specified name and version already exists.") - .build()); - } - LOGGER.error("Failed to update project %s".formatted(jsonProject.getUuid()), e); throw new ServerErrorException(Response.Status.INTERNAL_SERVER_ERROR); } @@ -659,6 +652,15 @@ public Response updateProject(Project jsonProject) { LOGGER.info("Project " + updatedProject + " updated by " + super.getPrincipal().getName()); return Response.ok(updatedProject).build(); } + catch (RuntimeException e) { + if (isUniqueConstraintViolation(e)) { + throw new ClientErrorException(Response + .status(Response.Status.CONFLICT) + .entity("A project with the specified name and version already exists.") + .build()); + } + throw e; + } } @PATCH