From 609c4f8092c390e30ac32ef5929c09b9ba69181d Mon Sep 17 00:00:00 2001 From: nscuro Date: Mon, 27 Nov 2023 20:07:51 +0100 Subject: [PATCH] Ensure `project.supplier` can be `PATCH`ed Signed-off-by: nscuro --- .../model/OrganizationalContact.java | 15 +++++ .../model/OrganizationalEntity.java | 18 ++++++ .../persistence/ProjectQueryManager.java | 1 + .../resources/v1/ProjectResource.java | 26 +-------- .../resources/v1/ProjectResourceTest.java | 56 +++++++++++++++---- 5 files changed, 81 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/dependencytrack/model/OrganizationalContact.java b/src/main/java/org/dependencytrack/model/OrganizationalContact.java index f03f09f01e..42e7ce3696 100644 --- a/src/main/java/org/dependencytrack/model/OrganizationalContact.java +++ b/src/main/java/org/dependencytrack/model/OrganizationalContact.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import java.io.Serializable; +import java.util.Objects; /** * Model class for tracking organizational contacts. @@ -67,4 +68,18 @@ public String getPhone() { public void setPhone(String phone) { this.phone = phone; } + + @Override + public boolean equals(final Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final OrganizationalContact that = (OrganizationalContact) o; + return Objects.equals(name, that.name) && Objects.equals(email, that.email) && Objects.equals(phone, that.phone); + } + + @Override + public int hashCode() { + return Objects.hash(name, email, phone); + } + } diff --git a/src/main/java/org/dependencytrack/model/OrganizationalEntity.java b/src/main/java/org/dependencytrack/model/OrganizationalEntity.java index 134fc7cbfb..d47cd80777 100644 --- a/src/main/java/org/dependencytrack/model/OrganizationalEntity.java +++ b/src/main/java/org/dependencytrack/model/OrganizationalEntity.java @@ -25,7 +25,9 @@ import java.io.Serializable; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.Objects; /** * Model class for tracking organizational entities (provider, supplier, manufacturer, etc). @@ -76,4 +78,20 @@ public void addContact(OrganizationalContact contact) { public void setContacts(List contacts) { this.contacts = contacts; } + + @Override + public boolean equals(final Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final OrganizationalEntity that = (OrganizationalEntity) o; + return Objects.equals(name, that.name) && Arrays.equals(urls, that.urls) && Objects.equals(contacts, that.contacts); + } + + @Override + public int hashCode() { + int result = Objects.hash(name, contacts); + result = 31 * result + Arrays.hashCode(urls); + return result; + } + } diff --git a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java index 03918793b1..9aa6aad106 100644 --- a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java @@ -553,6 +553,7 @@ public Project updateProject(Project transientProject, boolean commitIndex) { final Project project = getObjectByUuid(Project.class, transientProject.getUuid()); project.setAuthor(transientProject.getAuthor()); project.setPublisher(transientProject.getPublisher()); + project.setSupplier(transientProject.getSupplier()); project.setGroup(transientProject.getGroup()); project.setName(transientProject.getName()); project.setDescription(transientProject.getDescription()); diff --git a/src/main/java/org/dependencytrack/resources/v1/ProjectResource.java b/src/main/java/org/dependencytrack/resources/v1/ProjectResource.java index 49f245b32c..6a30590dd3 100644 --- a/src/main/java/org/dependencytrack/resources/v1/ProjectResource.java +++ b/src/main/java/org/dependencytrack/resources/v1/ProjectResource.java @@ -35,7 +35,6 @@ import org.dependencytrack.auth.Permissions; import org.dependencytrack.event.CloneProjectEvent; import org.dependencytrack.model.Classifier; -import org.dependencytrack.model.OrganizationalEntity; import org.dependencytrack.model.Project; import org.dependencytrack.model.Tag; import org.dependencytrack.persistence.QueryManager; @@ -56,10 +55,8 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import java.security.Principal; -import java.util.Arrays; import java.util.Collection; import java.util.List; -import java.util.Objects; import java.util.Set; import java.util.function.BiConsumer; import java.util.function.Function; @@ -385,6 +382,7 @@ public Response patchProject( modified |= setIfDifferent(jsonProject, project, Project::getPurl, Project::setPurl); modified |= setIfDifferent(jsonProject, project, Project::getSwidTagId, Project::setSwidTagId); modified |= setIfDifferent(jsonProject, project, Project::isActive, Project::setActive); + modified |= setIfDifferent(jsonProject, project, Project::getSupplier, Project::setSupplier); if (jsonProject.getParent() != null && jsonProject.getParent().getUuid() != null) { final Project parent = qm.getObjectByUuid(Project.class, jsonProject.getParent().getUuid()); if (parent == null) { @@ -404,10 +402,6 @@ public Response patchProject( modified = true; project.setExternalReferences(jsonProject.getExternalReferences()); } - if (isOrganizationalEntityModified(jsonProject.getSupplier(), project.getSupplier())) { - modified = true; - project.setSupplier(jsonProject.getSupplier()); - } if (modified) { try { project = qm.updateProject(project, true); @@ -426,24 +420,6 @@ public Response patchProject( } } - private static boolean isOrganizationalEntityModified(final OrganizationalEntity updated, final OrganizationalEntity original) { - if (updated == null) { - return false; - } - if (original == null) { - return true; - } - - if (!Objects.equals(updated.getName(), original.getName())) { - return true; - } - if (!Arrays.equals(updated.getUrls(), original.getUrls())) { - return true; - } - - return !Collections.isEmpty(updated.getContacts()) || !Collections.isEmpty(original.getContacts()); - } - /** * returns `true` if the given [updated] collection should be considered an update of the [original] collection. */ diff --git a/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java index a05d6eed4c..bada619280 100644 --- a/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java +++ b/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java @@ -66,8 +66,10 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; +import static org.hamcrest.Matchers.equalTo; public class ProjectResourceTest extends ResourceTest { @@ -585,6 +587,14 @@ public void patchProjectNotFoundTest() { public void patchProjectSuccessfullyPatchedTest() { final var tags = Stream.of("tag1", "tag2").map(qm::createTag).collect(Collectors.toUnmodifiableList()); final var p1 = qm.createProject("ABC", "Test project", "1.0", tags, null, null, true, false); + final var projectSupplierContact = new OrganizationalContact(); + projectSupplierContact.setName("supplierContactName"); + final var projectSupplier = new OrganizationalEntity(); + projectSupplier.setName("supplierName"); + projectSupplier.setUrls(new String[]{"https://supplier.example.com"}); + projectSupplier.setContacts(List.of(projectSupplierContact)); + p1.setSupplier(projectSupplier); + qm.persist(p1); final var jsonProject = new Project(); jsonProject.setActive(false); jsonProject.setName("new name"); @@ -594,22 +604,48 @@ public void patchProjectSuccessfullyPatchedTest() { t.setName(name); return t; }).collect(Collectors.toUnmodifiableList())); + final var jsonProjectSupplierContact = new OrganizationalContact(); + jsonProjectSupplierContact.setName("newSupplierContactName"); + final var jsonProjectSupplier = new OrganizationalEntity(); + jsonProjectSupplier.setName("supplierName"); + jsonProjectSupplier.setUrls(new String[]{"https://supplier.example.com"}); + jsonProjectSupplier.setContacts(List.of(jsonProjectSupplierContact)); + jsonProject.setSupplier(jsonProjectSupplier); final var response = target(V1_PROJECT + "/" + p1.getUuid()) .request() .header(X_API_KEY, apiKey) .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true) .method("PATCH", Entity.json(jsonProject)); Assert.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); - final var json = parseJsonObject(response); - Assert.assertEquals(p1.getUuid().toString(), json.getString("uuid")); - Assert.assertEquals(p1.getDescription(), json.getString("description")); - Assert.assertEquals(p1.getVersion(), json.getString("version")); - Assert.assertEquals(jsonProject.getName(), json.getString("name")); - Assert.assertEquals(jsonProject.getPublisher(), json.getString("publisher")); - Assert.assertEquals(false, json.getBoolean("active")); - final var jsonTags = json.getJsonArray("tags"); - Assert.assertEquals(1, jsonTags.size()); - Assert.assertEquals("tag4", jsonTags.get(0).asJsonObject().getString("name")); + assertThatJson(getPlainTextBody(response)) + .withMatcher("projectUuid", equalTo(p1.getUuid().toString())) + .isEqualTo(""" + { + "publisher": "new publisher", + "supplier": { + "name": "supplierName", + "urls": [ + "https://supplier.example.com" + ], + "contacts": [ + { + "name": "newSupplierContactName" + } + ] + }, + "name": "new name", + "description": "Test project", + "version": "1.0", + "uuid": "${json-unit.matches:projectUuid}", + "properties": [], + "tags": [ + { + "name": "tag4" + } + ], + "active": false + } + """); } @Test