TSK-1524: Set the default service level to "P0D" (#1496)

* TSK-1524: Set the default service level to "P0D"

* TSK-1524: Improvements after review
This commit is contained in:
tge20 2021-03-01 10:03:04 +01:00 committed by GitHub
parent 0b55208aca
commit d57aeb3219
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 104 additions and 72 deletions

View File

@ -127,7 +127,7 @@ public class JaasExtension implements InvocationInterceptor, TestTemplateInvocat
// Currently a DynamicContainer has children from this type: Stream<DynamicNode>
// Because of this the children can only be extracted once (Streams can only be operated
// once). This is obviously not ok since we want to execute each node X times. So we have to
// manually persist all children recursively to extract them X times...
// manually insert all children recursively to extract them X times...
Map<String, List<DynamicNode>> childrenMap = new HashMap<>();
persistDynamicContainerChildren(newChildrenForDynamicContainer, childrenMap);

View File

@ -66,18 +66,29 @@ public interface ClassificationService {
throws ClassificationInUseException, ClassificationNotFoundException, NotAuthorizedException;
/**
* Persists a new classification after adding default values. <br>
* The classification will be added to master-domain, too - if not already existing.
* Inserts a new Classification after adding default values. <br>
* The Classification will be added to master-domain, too - if not already existing. <br>
* The default values are:
*
* @param classification the classification to insert
* @return classification which is persisted with unique ID.
* @throws ClassificationAlreadyExistException if the classification does already exists at the
* <ul>
* <li><b>id</b> - generated by {@linkplain pro.taskana.common.internal.util.IdGenerator
* IdGenerator}
* <li><b>parentId</b> - ""
* <li><b>parentKey</b> - ""
* <li><b>serviceLevel</b> - "P0D"
* <li><b>isValidInDomain</b> - {@code true} <br>
* if {@code domain} an empty string than {@code false}
* </ul>
*
* @param classification the Classification to insert
* @return Classification which is equipped with unique ID.
* @throws ClassificationAlreadyExistException if the Classification does already exists at the
* given domain.
* @throws NotAuthorizedException if the current user is not member of role BUSINESS_ADMIN or
* ADMIN
* @throws DomainNotFoundException if the domain does not exist in the configuration
* @throws InvalidArgumentException if the ServiceLevel property does not comply with the ISO 8601
* specification
* @throws DomainNotFoundException if the {@code domain} does not exist in the configuration
* @throws InvalidArgumentException if the {@code serviceLevel} property does not comply with the
* ISO 8601 specification
*/
Classification createClassification(Classification classification)
throws ClassificationAlreadyExistException, NotAuthorizedException, DomainNotFoundException,
@ -100,7 +111,7 @@ public interface ClassificationService {
InvalidArgumentException;
/**
* This method provides a query builder for quering the database.
* This method provides a query builder for querying the database.
*
* @return a {@link ClassificationQuery}
*/
@ -108,7 +119,7 @@ public interface ClassificationService {
/**
* Creating a new {@link Classification} with unchangeable default values. It will be only
* generated and is not persisted until CREATE-call.
* generated and is not inserted until CREATE-call.
*
* @param key the key of the classification
* @param domain the domain of the new classification

View File

@ -336,7 +336,6 @@ public class ClassificationServiceImpl implements ClassificationService {
"Invalid service level %s. Taskana does not support a negative service level.",
serviceLevel));
}
// check that the duration is based on format PnD, i.e. it must start with a P, end with a D
if (!serviceLevel.toLowerCase().startsWith("p") || !serviceLevel.toLowerCase().endsWith("d")) {
throw new InvalidArgumentException(
@ -450,9 +449,9 @@ public class ClassificationServiceImpl implements ClassificationService {
classification.setIsValidInDomain(true);
}
if (classification.getServiceLevel() == null) {
throw new InvalidArgumentException("Classification Service Level must not be null!");
} else if (!classification.getServiceLevel().isEmpty()) {
if (classification.getServiceLevel() == null || classification.getServiceLevel().isEmpty()) {
classification.setServiceLevel("P0D");
} else {
validateServiceLevel(classification.getServiceLevel());
}

View File

@ -5,7 +5,6 @@ import java.util.List;
import java.util.Map;
import pro.taskana.classification.api.exceptions.ClassificationNotFoundException;
import pro.taskana.classification.api.models.Classification;
import pro.taskana.common.api.BulkOperationResults;
import pro.taskana.common.api.exceptions.ConcurrencyException;
import pro.taskana.common.api.exceptions.InvalidArgumentException;
@ -103,7 +102,7 @@ public interface TaskService {
NotAuthorizedException;
/**
* Complete a Task and update State and Timestamps in every case if the Task exists. If task is
* Completes a Task and updates State and Timestamps in every case if the Task exists. If task is
* already completed, the task is returned as itself.
*
* @param taskId - Id of the Task which should be completed.
@ -119,16 +118,30 @@ public interface TaskService {
NotAuthorizedException;
/**
* Persists a not persisted Task which does not exist already.
* Inserts a not existing Task. <br>
* The default values of the created Task are:
*
* @param taskToCreate the transient task object to be persisted
* @return the created and persisted task
* <ul>
* <li><b>id</b> - generated by {@linkplain pro.taskana.common.internal.util.IdGenerator
* IdGenerator}
* <li><b>externalId</b> - generated by IdGenerator
* <li><b>businessProcessId</b> - generated by IdGenerator
* <li><b>name</b> - name of its Classification
* <li><b>description</b> - description of its Classification
* <li><b>creator</b> - id of current user
* <li><b>state</b> - 'READY'
* <li><b>isRead</b> - {@code false}
* <li><b>isTransferred</b> - {@code false}
* </ul>
*
* @param taskToCreate the transient task object to be inserted
* @return the created and inserted task
* @throws TaskAlreadyExistException if the Task does already exist.
* @throws NotAuthorizedException thrown if the current user is not authorized to create that task
* @throws WorkbasketNotFoundException thrown if the workbasket referenced by the task is not
* found
* @throws ClassificationNotFoundException thrown if the {@link Classification} referenced by the
* task is not found
* @throws ClassificationNotFoundException thrown if the Classification referenced by the task is
* not found
* @throws InvalidArgumentException thrown if the primary ObjectReference is invalid
*/
Task createTask(Task taskToCreate)
@ -204,18 +217,18 @@ public interface TaskService {
TaskQuery createTaskQuery();
/**
* Returns a not persisted instance of {@link Task}. The returned task has no workbasket Id set.
* Returns a not inserted instance of {@link Task}. The returned task has no workbasket Id set.
* When createTask() is invoked for this task, TaskService will call the TaskRouting SPI to
* determine a workbasket for the task. If the TaskRouting API is not active, e.g. because no
* TaskRouter is registered, or the TaskRouter(s) don't find a workbasket, the task will not be
* persisted.
* inserted.
*
* @return an empty new Task
*/
Task newTask();
/**
* Returns a not persisted instance of {@link Task}.
* Returns a not inserted instance of {@link Task}.
*
* @param workbasketId the id of the workbasket to which the task belongs
* @return an empty new Task
@ -223,7 +236,7 @@ public interface TaskService {
Task newTask(String workbasketId);
/**
* Returns a not persisted instance of {@link Task}.
* Returns a not inserted instance of {@link Task}.
*
* @param workbasketKey the key of the workbasket to which the task belongs
* @param domain the domain of the workbasket to which the task belongs
@ -232,7 +245,7 @@ public interface TaskService {
Task newTask(String workbasketKey, String domain);
/**
* Returns a not persisted instance of {@link TaskComment}.
* Returns a not inserted instance of {@link TaskComment}.
*
* @param taskId The id of the task to which the task comment belongs
* @return an empty new TaskComment
@ -240,7 +253,7 @@ public interface TaskService {
TaskComment newTaskComment(String taskId);
/**
* Returns a not persisted instance of {@link Attachment}.
* Returns a not inserted instance of {@link Attachment}.
*
* @return an empty new Attachment
*/

View File

@ -4,7 +4,7 @@ import pro.taskana.common.api.exceptions.TaskanaException;
/**
* Thrown, when an attachment should be inserted to the DB, but it does already exist.<br>
* This may happen when a not persisted attachment with ID will be added twice on a task. This can´t
* This may happen when a not inserted attachment with ID will be added twice on a task. This can´t
* be happen it the correct Task-Methods will be used instead the List ones.
*/
public class AttachmentPersistenceException extends TaskanaException {

View File

@ -22,7 +22,7 @@ public interface Task extends TaskSummary {
* Sets the external Id. It can be used to correlate the task to a task in an external system. The
* external Id is enforced to be unique. An attempt to create a task with an existing external Id
* will be rejected. So, this Id can be used to enforce idempotency of task creation. The
* externalId can only be set before the task is persisted. Taskana rejects attempts to modify
* externalId can only be set before the task is inserted. Taskana rejects attempts to modify
* externalId.
*
* @param externalId the external Id

View File

@ -46,14 +46,20 @@ public interface WorkbasketService {
throws WorkbasketNotFoundException, NotAuthorizedException;
/**
* Create a new Workbasket.
* Creates a new Workbasket. <br>
* The default values are:
*
* <ul>
* <li><b>id</b> - generated by {@linkplain pro.taskana.common.internal.util.IdGenerator
* IdGenerator}
* </ul>
*
* @param workbasket The Workbasket to create
* @return the created and persisted Workbasket
* @return the created and inserted Workbasket
* @throws InvalidWorkbasketException If a required property of the Workbasket is not set.
* @throws NotAuthorizedException if the current user is not member of role BUSINESS_ADMIN or
* ADMIN
* @throws WorkbasketAlreadyExistException if the workbasket exists already
* @throws WorkbasketAlreadyExistException if the Workbasket exists already
* @throws DomainNotFoundException if the domain does not exist in the configuration.
*/
Workbasket createWorkbasket(Workbasket workbasket)
@ -76,7 +82,7 @@ public interface WorkbasketService {
ConcurrencyException;
/**
* Returns a new WorkbasketAccessItem which is not persisted.
* Returns a new WorkbasketAccessItem which is not inserted.
*
* @param workbasketId the workbasket id used to identify the referenced workbasket
* @param accessId the group id or user id for which access is controlled
@ -85,7 +91,7 @@ public interface WorkbasketService {
WorkbasketAccessItem newWorkbasketAccessItem(String workbasketId, String accessId);
/**
* Create and persist a new {@link WorkbasketAccessItem} with a WorkbasketId, an accessId and
* Create and insert a new {@link WorkbasketAccessItem} with a WorkbasketId, an accessId and
* permissions.
*
* @param workbasketAccessItem the new workbasketAccessItem
@ -205,7 +211,7 @@ public interface WorkbasketService {
WorkbasketAccessItemQuery createWorkbasketAccessItemQuery() throws NotAuthorizedException;
/**
* Returns a new workbasket which is not persisted.
* Returns a new workbasket which is not inserted.
*
* @param key the workbasket key used to identify the workbasket
* @param domain the domain of the new workbasket

View File

@ -138,33 +138,6 @@ class CreateClassificationAccTest extends AbstractAccTest {
assertThat(actualChild).isNotNull();
}
@WithAccessId(user = "businessadmin")
@Test
void should_ThrowException_When_TryingToCreateClassificationWithMissingServiceLevel() {
Classification classification =
CLASSIFICATION_SERVICE.newClassification("someKey", "DOMAIN_A", "TASK");
assertThatThrownBy(() -> CLASSIFICATION_SERVICE.createClassification(classification))
.isInstanceOf(InvalidArgumentException.class);
}
@WithAccessId(user = "businessadmin")
@Test
void should_ThrowException_When_TryingToUpdateClassificationWithMissingServiceLevel()
throws Exception {
Classification classification =
CLASSIFICATION_SERVICE.newClassification("someKey2", "DOMAIN_A", "TASK");
classification.setServiceLevel("P1D");
Classification createdClassification =
CLASSIFICATION_SERVICE.createClassification(classification);
createdClassification.setServiceLevel(null);
assertThatThrownBy(() -> CLASSIFICATION_SERVICE.updateClassification(createdClassification))
.isInstanceOf(InvalidArgumentException.class);
}
@WithAccessId(user = "businessadmin")
@Test
void should_ThrowException_When_TryingToCreateClassificationWithNegativeServiceLevel() {
@ -290,4 +263,34 @@ class CreateClassificationAccTest extends AbstractAccTest {
assertThatThrownBy(createClassificationCall).isInstanceOf(NotAuthorizedException.class);
}
@WithAccessId(user = "admin")
@Test
void should_SetDefaultServiceLevel_When_TryingToCreateClassificationWithMissingServiceLevel()
throws Exception {
Classification classification =
CLASSIFICATION_SERVICE.newClassification("newKey718", "", "TASK");
classification = CLASSIFICATION_SERVICE.createClassification(classification);
assertThat(classification.getServiceLevel()).isEqualTo("P0D");
classification = CLASSIFICATION_SERVICE.newClassification("newKey71", "", "TASK");
classification.setServiceLevel("");
classification = CLASSIFICATION_SERVICE.createClassification(classification);
assertThat(classification.getServiceLevel()).isEqualTo("P0D");
}
@WithAccessId(user = "admin")
@Test
void should_SetDefaultServiceLevel_When_TryingToUpdateClassificationWithMissingServiceLevel()
throws Exception {
Classification classification =
CLASSIFICATION_SERVICE.getClassification("CLI:000000000000000000000000000000000001");
classification.setServiceLevel(null);
classification = CLASSIFICATION_SERVICE.updateClassification(classification);
assertThat(classification.getServiceLevel()).isEqualTo("P0D");
classification.setServiceLevel("");
classification = CLASSIFICATION_SERVICE.updateClassification(classification);
assertThat(classification.getServiceLevel()).isEqualTo("P0D");
}
}

View File

@ -183,9 +183,9 @@ class UpdateTaskAttachmentsAccTest extends AbstractAccTest {
// Try to set the Attachments to NULL and update it
((TaskImpl) task).setAttachments(null);
task = taskService.updateTask(task);
assertThat(task.getAttachments()).hasSize(attachmentCount); // locally, not persisted
assertThat(task.getAttachments()).hasSize(attachmentCount); // locally, not inserted
task = taskService.getTask(task.getId());
assertThat(task.getAttachments()).hasSize(attachmentCount); // persisted values not changed
assertThat(task.getAttachments()).hasSize(attachmentCount); // inserted values not changed
// Test no NullPointer on NULL-Value and removing it on current data.
// New loading can do this, but returned value should got this "function", too.
@ -194,9 +194,9 @@ class UpdateTaskAttachmentsAccTest extends AbstractAccTest {
task.getAttachments().add(null);
task.getAttachments().add(null);
task = taskService.updateTask(task);
assertThat(task.getAttachments()).hasSize(attachmentCount2); // locally, not persisted
assertThat(task.getAttachments()).hasSize(attachmentCount2); // locally, not inserted
task = taskService.getTask(task.getId());
assertThat(task.getAttachments()).hasSize(attachmentCount2); // persisted values not changed
assertThat(task.getAttachments()).hasSize(attachmentCount2); // inserted values not changed
assertThat(task.getPriority()).isEqualTo(1);
assertThat(task.getPlanned().plus(Duration.ofDays(1))).isEqualTo(task.getDue());
}
@ -213,9 +213,9 @@ class UpdateTaskAttachmentsAccTest extends AbstractAccTest {
task.removeAttachment(attachmentToRemove.getId());
task = taskService.updateTask(task);
assertThat(task.getAttachments())
.hasSize(attachmentCount - 1); // locally, removed and not persisted
.hasSize(attachmentCount - 1); // locally, removed and not inserted
task = taskService.getTask(task.getId());
assertThat(task.getAttachments()).hasSize(attachmentCount - 1); // persisted, values removed
assertThat(task.getAttachments()).hasSize(attachmentCount - 1); // inserted, values removed
assertThat(task.getPriority()).isEqualTo(1);
assertThat(task.getPlanned().plus(Duration.ofDays(1))).isEqualTo(task.getDue());
}
@ -231,13 +231,13 @@ class UpdateTaskAttachmentsAccTest extends AbstractAccTest {
task = taskService.updateTask(task);
assertThat(task.getAttachments()).hasSize(attachmentCount); // locally, nothing changed
task = taskService.getTask(task.getId());
assertThat(task.getAttachments()).hasSize(attachmentCount); // persisted, still same
assertThat(task.getAttachments()).hasSize(attachmentCount); // inserted, still same
task.removeAttachment("INVALID ID HERE");
task = taskService.updateTask(task);
assertThat(task.getAttachments()).hasSize(attachmentCount); // locally, nothing changed
task = taskService.getTask(task.getId());
assertThat(task.getAttachments()).hasSize(attachmentCount); // persisted, still same
assertThat(task.getAttachments()).hasSize(attachmentCount); // inserted, still same
}
@WithAccessId(user = "user-1-1")

View File

@ -135,7 +135,7 @@ public class ClassificationController {
*
* @title Create a new Classification
* @param repModel the Classification which should be created.
* @return The persisted Classification
* @return The inserted Classification
* @throws NotAuthorizedException if the current user is not allowed to create a Classification.
* @throws ClassificationAlreadyExistException if the new Classification already exists. This
* means that a Classification with the requested key and domain already exist.