TSK-1824: throw Exception in REST if Attachment has taskId

lib/taskana-core/src/test/java/acceptance/task/CreateTaskWithAttachmentAccTest.java → lib/taskana-core-test/src/test/java/acceptance/task/create/CreateTaskWithAttachmentAccTest.java

@@ -1,42 +1,42 @@
-package acceptance.task;
+package acceptance.task.create;
 
-import static acceptance.DefaultTestEntities.defaultTestClassification;
-import static acceptance.DefaultTestEntities.defaultTestObjectReference;
-import static acceptance.DefaultTestEntities.defaultTestWorkbasket;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static pro.taskana.testapi.DefaultTestEntities.defaultTestClassification;
+import static pro.taskana.testapi.DefaultTestEntities.defaultTestObjectReference;
+import static pro.taskana.testapi.DefaultTestEntities.defaultTestWorkbasket;
 
 import java.util.List;
 import java.util.stream.Stream;
+import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.DynamicTest;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestFactory;
 import org.junit.jupiter.api.function.ThrowingConsumer;
-import testapi.TaskanaInject;
-import testapi.TaskanaIntegrationTest;
 
 import pro.taskana.classification.api.ClassificationService;
 import pro.taskana.classification.api.exceptions.ClassificationNotFoundException;
-import pro.taskana.classification.api.models.Classification;
 import pro.taskana.classification.api.models.ClassificationSummary;
 import pro.taskana.common.api.exceptions.InvalidArgumentException;
-import pro.taskana.common.internal.util.Pair;
-import pro.taskana.common.internal.util.Quadruple;
-import pro.taskana.common.test.security.WithAccessId;
+import pro.taskana.common.internal.util.Triplet;
 import pro.taskana.task.api.TaskService;
 import pro.taskana.task.api.models.Attachment;
 import pro.taskana.task.api.models.ObjectReference;
 import pro.taskana.task.api.models.Task;
-import pro.taskana.task.internal.builder.TaskAttachmentBuilder;
-import pro.taskana.task.internal.builder.TaskBuilder;
+import pro.taskana.testapi.TaskanaInject;
+import pro.taskana.testapi.TaskanaIntegrationTest;
+import pro.taskana.testapi.builder.TaskAttachmentBuilder;
+import pro.taskana.testapi.builder.TaskBuilder;
+import pro.taskana.testapi.builder.WorkbasketAccessItemBuilder;
+import pro.taskana.testapi.security.WithAccessId;
 import pro.taskana.workbasket.api.WorkbasketPermission;
 import pro.taskana.workbasket.api.WorkbasketService;
 import pro.taskana.workbasket.api.models.WorkbasketSummary;
-import pro.taskana.workbasket.internal.builder.WorkbasketAccessItemBuilder;
 
 /**
- * Acceptance test for all "create task" scenarios that involve {@linkplain Attachment Attachments}.
+ * Acceptance test for all "create task" scenarios that involve {@linkplain
+ * pro.taskana.task.api.models.Attachment Attachments}.
  */
 @TaskanaIntegrationTest
 class CreateTaskWithAttachmentAccTest {
@@ -83,7 +83,7 @@ void setup() throws Exception {
 
   @WithAccessId(user = "user-1-1")
   @Test
-  void should_SetTaskIdOfAttachmentCorrectly_WhenCopyingAttachment() throws Exception {
+  void should_SetTaskIdOfAttachmentCorrectly_When_CopyingAttachment() throws Exception {
     Attachment copiedAttachment =
         taskService.getTask(defaultTaskWithAttachment.getId()).getAttachments().get(0).copy();
     Task taskToCreate = taskService.newTask(defaultWorkbasketSummary.getId());
@@ -122,79 +122,81 @@ void should_FetchAttachmentClassification_When_CreatingTaskWithAttachments() thr
   @TestFactory
   Stream<DynamicTest>
       should_ThrowException_When_CreatingTaskWithInvalidObjectReferenceOfAttachment() {
-    ObjectReference objRefTypeNull =
-        taskService.newObjectReference("Company", "System", "Instance", null, "Value");
-    ObjectReference objRefValueNull =
-        taskService.newObjectReference("Company", "System", "Instance", "Type", null);
-    ObjectReference objRefCompanyNull =
-        taskService.newObjectReference(null, "System", "Instance", "Type", "Value");
-    List<Pair<String, ObjectReference>> valuesForTests =
+    List<Triplet<String, ObjectReference, String>> valuesForTests =
         List.of(
-            Pair.of("ObjRef is null", null),
-            Pair.of("Type of objRef is null", objRefTypeNull),
-            Pair.of("Value of objRef is null", objRefValueNull),
-            Pair.of("Company of objRef is null", objRefCompanyNull));
-
-    ThrowingConsumer<Pair<String, ObjectReference>> test =
-        p -> {
+            Triplet.of("ObjRef is null", null, "ObjectReference of Attachment must not be null."),
+            Triplet.of(
+                "Type of objRef is null",
+                defaultTestObjectReference().type(null).build(),
+                "Type of ObjectReference of Attachment must not be empty"),
+            Triplet.of(
+                "Value of objRef is null",
+                defaultTestObjectReference().value(null).build(),
+                "Value of ObjectReference of Attachment must not be empty"),
+            Triplet.of(
+                "Company of objRef is null",
+                defaultTestObjectReference().company(null).build(),
+                "Company of ObjectReference of Attachment must not be empty"));
+
+    ThrowingConsumer<Triplet<String, ObjectReference, String>> test =
+        t -> {
+          ObjectReference objectReference = t.getMiddle();
+          Attachment attachment = taskService.newAttachment();
+          attachment.setClassificationSummary(defaultClassificationSummary);
+          attachment.setObjectReference(objectReference);
           Task task = taskService.newTask(defaultWorkbasketSummary.getId());
           task.setClassificationKey(defaultClassificationSummary.getKey());
           task.setPrimaryObjRef(defaultObjectReference);
-          Attachment attachment = taskService.newAttachment();
-          attachment.setClassificationSummary(defaultClassificationSummary);
-          attachment.setObjectReference(p.getRight());
           task.addAttachment(attachment);
-          assertThatThrownBy(() -> taskService.createTask(task))
-              .isInstanceOf(InvalidArgumentException.class);
+          ThrowingCallable call = () -> taskService.createTask(task);
+          String errorMessage = t.getRight();
+          assertThatThrownBy(call)
+              .isInstanceOf(InvalidArgumentException.class)
+              .hasMessageContaining(errorMessage);
         };
-
-    return DynamicTest.stream(valuesForTests.iterator(), Pair::getLeft, test);
+    return DynamicTest.stream(valuesForTests.iterator(), Triplet::getLeft, test);
   }
 
   @WithAccessId(user = "user-1-1")
   @TestFactory
   Stream<DynamicTest>
       should_ThrowException_When_CreatingTaskWithInvalidClassificationOfAttachment() {
-    Classification nonExistingClassification =
-        classificationService.newClassification(
-            "123key345", defaultWorkbasketSummary.getDomain(), "non existing type");
-    Classification classificationWithoutKey =
-        classificationService.newClassification(
-            null, defaultClassificationSummary.getDomain(), defaultClassificationSummary.getType());
-    List<Quadruple<String, Classification, String, Class<?>>> valuesForTests =
+    ClassificationSummary nonExistingClassification =
+        classificationService.newClassification("non existing", "DOMAIN_A", "Valid type");
+    ClassificationSummary classificationWithoutKey =
+        classificationService.newClassification(null, "DOMAIN_A", "Valid type");
+    List<Triplet<String, ClassificationSummary, Exception>> valuesForTests =
         List.of(
-            Quadruple.of(
-                "Classification is null",
-                null,
-                "Classification of Attachment must not be null",
-                InvalidArgumentException.class),
-            Quadruple.of(
+            Triplet.of(
                 "Classification doesn't exist",
                 nonExistingClassification,
-                "Classification with key '123key345' and domain '"
-                    + defaultWorkbasketSummary.getDomain()
-                    + "' could not be found",
-                ClassificationNotFoundException.class),
-            Quadruple.of(
+                new ClassificationNotFoundException("non existing", "DOMAIN_A")),
+            Triplet.of(
+                "Classification is null",
+                null,
+                new InvalidArgumentException("Classification of Attachment must not be null")),
+            Triplet.of(
                 "Classification has no key",
                 classificationWithoutKey,
-                "ClassificationKey of Attachment must not be empty.",
-                InvalidArgumentException.class));
+                new InvalidArgumentException(
+                    "ClassificationKey of Attachment must not be empty.")));
 
-    ThrowingConsumer<Quadruple<String, Classification, String, Class<?>>> test =
+    ThrowingConsumer<Triplet<String, ClassificationSummary, Exception>> test =
         q -> {
           Task task = taskService.newTask(defaultWorkbasketSummary.getId());
           task.setClassificationKey(defaultClassificationSummary.getKey());
           task.setPrimaryObjRef(defaultObjectReference);
           Attachment attachment = taskService.newAttachment();
-          attachment.setClassificationSummary(q.getSecond());
+          ClassificationSummary classificationSummary = q.getMiddle();
+          attachment.setClassificationSummary(classificationSummary);
           attachment.setObjectReference(defaultObjectReference);
           task.addAttachment(attachment);
+          Exception exception = q.getRight();
           assertThatThrownBy(() -> taskService.createTask(task))
-              .isInstanceOf(q.getFourth())
-              .hasMessageContaining(q.getThird());
+              .usingRecursiveComparison()
+              .isEqualTo(exception);
         };
 
-    return DynamicTest.stream(valuesForTests.iterator(), Quadruple::getFirst, test);
+    return DynamicTest.stream(valuesForTests.iterator(), Triplet::getLeft, test);
   }
 }

lib/taskana-core/src/test/java/acceptance/task/create/CreateTaskAccTest.java

@@ -29,7 +29,6 @@
 import pro.taskana.task.api.TaskService;
 import pro.taskana.task.api.TaskState;
 import pro.taskana.task.api.exceptions.TaskAlreadyExistException;
-import pro.taskana.task.api.models.Attachment;
 import pro.taskana.task.api.models.AttachmentSummary;
 import pro.taskana.task.api.models.ObjectReference;
 import pro.taskana.task.api.models.Task;
@@ -762,21 +761,6 @@ void testCreateTaskWithWorkbasketMarkedForDeletion() throws Exception {
         .isInstanceOf(WorkbasketNotFoundException.class);
   }
 
-  // Rewritten
-  @WithAccessId(user = "user-1-1")
-  @Test
-  void should_ThrowException_When_CreatingTaskWithAttachmentClassificationNull() {
-    TaskImpl task = (TaskImpl) makeNewTask(taskService);
-    Attachment attachment = taskService.newAttachment();
-    attachment.setObjectReference(
-        createObjectReference("COMPANY_A", "SYSTEM_A", "INSTANCE_A", "VNR", "1234567"));
-    task.addAttachment(attachment);
-
-    assertThatThrownBy(() -> taskService.createTask(task))
-        .isInstanceOf(InvalidArgumentException.class)
-        .hasMessage("Classification of Attachment must not be null.");
-  }
-
   private Task setTaskProperties(Task task) {
     task.setClassificationKey("L12010");
     task.setPrimaryObjRef(

rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskController.java

@@ -48,6 +48,7 @@
 import pro.taskana.task.api.models.TaskSummary;
 import pro.taskana.task.rest.assembler.TaskRepresentationModelAssembler;
 import pro.taskana.task.rest.assembler.TaskSummaryRepresentationModelAssembler;
+import pro.taskana.task.rest.models.AttachmentRepresentationModel;
 import pro.taskana.task.rest.models.TaskRepresentationModel;
 import pro.taskana.task.rest.models.TaskSummaryCollectionRepresentationModel;
 import pro.taskana.task.rest.models.TaskSummaryPagedRepresentationModel;
@@ -347,6 +348,7 @@ public ResponseEntity<TaskRepresentationModel> createTask(
       throws WorkbasketNotFoundException, ClassificationNotFoundException, NotAuthorizedException,
           TaskAlreadyExistException, InvalidArgumentException, AttachmentPersistenceException,
           ObjectReferencePersistenceException {
+    verifyTaskIdOfAttachments(taskRepresentationModel.getAttachments(), null);
     Task fromResource = taskRepresentationModelAssembler.toEntityModel(taskRepresentationModel);
     Task createdTask = taskService.createTask(fromResource);
 
@@ -416,11 +418,23 @@ public ResponseEntity<TaskRepresentationModel> updateTask(
                   + "object in the payload which should be updated. ID=('%s')",
               taskId, taskRepresentationModel.getTaskId()));
     }
+    verifyTaskIdOfAttachments(
+        taskRepresentationModel.getAttachments(), taskRepresentationModel.getTaskId());
     Task task = taskRepresentationModelAssembler.toEntityModel(taskRepresentationModel);
     task = taskService.updateTask(task);
     return ResponseEntity.ok(taskRepresentationModelAssembler.toModel(task));
   }
 
+  private void verifyTaskIdOfAttachments(
+      List<AttachmentRepresentationModel> attachments, String taskId)
+      throws InvalidArgumentException {
+    for (AttachmentRepresentationModel attachment : attachments) {
+      if (attachment.getTaskId() != null && !attachment.getTaskId().equals(taskId)) {
+        throw new InvalidArgumentException("TaskId of Attachment should be empty.");
+      }
+    }
+  }
+
   public enum TaskQuerySortBy implements QuerySortBy<TaskQuery> {
     CLASSIFICATION_KEY(TaskQuery::orderByClassificationKey),
     CLASSIFICATION_NAME(TaskQuery::orderByClassificationName),

rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskControllerIntTest.java

@@ -38,6 +38,7 @@
 import pro.taskana.common.test.rest.TaskanaSpringBootTest;
 import pro.taskana.sampledata.SampleDataGenerator;
 import pro.taskana.task.api.TaskState;
+import pro.taskana.task.rest.models.AttachmentRepresentationModel;
 import pro.taskana.task.rest.models.ObjectReferenceRepresentationModel;
 import pro.taskana.task.rest.models.TaskRepresentationModel;
 import pro.taskana.task.rest.models.TaskRepresentationModel.CustomAttribute;
@@ -677,6 +678,57 @@ void should_NotGetEmptyObjectReferencesList_When_GettingTaskWithObjectReferences
     assertThat(repModel.getSecondaryObjectReferences()).isNotEmpty();
   }
 
+  @Test
+  void should_ThrowException_When_CreatingTaskAndAttachmentAlreadyHasTaskId() {
+    TaskRepresentationModel taskRepresentationModel = getTaskResourceSample();
+    AttachmentRepresentationModel attachment = new AttachmentRepresentationModel();
+    attachment.setTaskId("TKI:1");
+    attachment.setClassificationSummary(taskRepresentationModel.getClassificationSummary());
+    attachment.setObjectReference(taskRepresentationModel.getPrimaryObjRef());
+    taskRepresentationModel.setAttachments(List.of(attachment));
+
+    HttpEntity<Object> auth =
+        new HttpEntity<>(taskRepresentationModel, RestHelper.generateHeadersForUser("admin"));
+    String url = restHelper.toUrl(RestEndpoints.URL_TASKS);
+
+    ThrowingCallable httpCall =
+        () -> TEMPLATE.exchange(url, HttpMethod.POST, auth, TASK_MODEL_TYPE);
+    assertThatThrownBy(httpCall)
+        .isInstanceOf(HttpStatusCodeException.class)
+        .hasMessageContaining("TaskId of Attachment should be empty")
+        .extracting(HttpStatusCodeException.class::cast)
+        .extracting(HttpStatusCodeException::getStatusCode)
+        .isEqualTo(HttpStatus.BAD_REQUEST);
+  }
+
+  @Test
+  void should_ThrowException_When_UpdatingTaskAndAttachmentAlreadyHasTaskId() {
+    String url =
+        restHelper.toUrl(RestEndpoints.URL_TASKS_ID, "TKI:100000000000000000000000000000000000");
+    HttpEntity<Object> httpEntityWithoutBody =
+        new HttpEntity<>(RestHelper.generateHeadersForUser("teamlead-1"));
+    ResponseEntity<TaskRepresentationModel> responseGet =
+        TEMPLATE.exchange(url, HttpMethod.GET, httpEntityWithoutBody, TASK_MODEL_TYPE);
+    TaskRepresentationModel originalTask = responseGet.getBody();
+    AttachmentRepresentationModel attachment = new AttachmentRepresentationModel();
+    attachment.setTaskId("TKI:1");
+    attachment.setClassificationSummary(originalTask.getClassificationSummary());
+    attachment.setObjectReference(originalTask.getPrimaryObjRef());
+    originalTask.setAttachments(List.of(attachment));
+    HttpEntity<TaskRepresentationModel> httpEntity =
+        new HttpEntity<>(originalTask, RestHelper.generateHeadersForUser("teamlead-1"));
+
+    ThrowingCallable httpCall =
+        () -> TEMPLATE.exchange(url, HttpMethod.PUT, httpEntity, TASK_MODEL_TYPE);
+
+    assertThatThrownBy(httpCall)
+        .isInstanceOf(HttpStatusCodeException.class)
+        .hasMessageContaining("TaskId of Attachment should be empty")
+        .extracting(HttpStatusCodeException.class::cast)
+        .extracting(HttpStatusCodeException::getStatusCode)
+        .isEqualTo(HttpStatus.BAD_REQUEST);
+  }
+
   @Test
   void should_ReturnFilteredTasks_When_GettingTasksBySecondaryObjectReferenceValue() {
     String url = restHelper.toUrl(RestEndpoints.URL_TASKS) + "?sor-value=Value2";