From 8a6797e3c96b005e2fd795a733f3332a814a0e58 Mon Sep 17 00:00:00 2001 From: Jens Pelzetter Date: Sat, 22 Jan 2022 17:55:33 +0100 Subject: [PATCH] Several bugfixes for workflow handling (for Content Items) --- .../contentsection/ContentItemRepository.java | 28 +++++- .../librecms/contentsection/ContentType.java | 2 +- .../contentsection/ContentTypeManager.java | 95 +++++++++++++++---- .../contentsection/FolderManager.java | 4 +- .../ConfigurationWorkflowController.java | 16 ++++ .../org/libreccm/workflow/TaskManager.java | 89 ++++++++++------- .../libreccm/workflow/WorkflowManager.java | 13 +-- 7 files changed, 185 insertions(+), 62 deletions(-) diff --git a/ccm-cms/src/main/java/org/librecms/contentsection/ContentItemRepository.java b/ccm-cms/src/main/java/org/librecms/contentsection/ContentItemRepository.java index df48552f7..d0ed104a1 100644 --- a/ccm-cms/src/main/java/org/librecms/contentsection/ContentItemRepository.java +++ b/ccm-cms/src/main/java/org/librecms/contentsection/ContentItemRepository.java @@ -33,6 +33,7 @@ import org.libreccm.configuration.ConfigurationManager; import org.libreccm.core.CcmObject; import org.libreccm.core.CcmObjectRepository; import org.libreccm.core.UnexpectedErrorException; +import org.libreccm.security.Permission; import org.libreccm.security.PermissionChecker; import org.libreccm.security.Role; import org.libreccm.security.RoleManager; @@ -49,12 +50,17 @@ import javax.persistence.TypedQuery; import org.libreccm.security.Shiro; import org.libreccm.security.User; import org.libreccm.security.UserRepository; +import org.libreccm.workflow.AssignableTask; +import org.libreccm.workflow.AssignableTaskManager; import org.libreccm.workflow.Task; +import org.libreccm.workflow.TaskAssignment; import org.libreccm.workflow.TaskManager; import org.libreccm.workflow.Workflow; import org.libreccm.workflow.WorkflowRepository; +import java.util.ArrayList; import java.util.Collections; +import java.util.stream.Collectors; import javax.transaction.Transactional; @@ -69,6 +75,9 @@ public class ContentItemRepository private static final long serialVersionUID = -145167586339461600L; + @Inject + private AssignableTaskManager assignableTaskManager; + @Inject private CategoryManager categoryManager; @@ -691,12 +700,29 @@ public class ContentItemRepository if (draft.getWorkflow() != null) { final Workflow workflow = draft.getWorkflow(); - for (final Task task : workflow.getTasks()) { + final List tasks = new ArrayList<>(workflow.getTasks()); + for (final Task task : tasks) { + if (task instanceof AssignableTask) { + final AssignableTask assignable = (AssignableTask) task; + final List assignedRoles = assignable + .getAssignments() + .stream() + .map(TaskAssignment::getRole) + .collect(Collectors.toList()); + for (final Role role : assignedRoles) { + assignableTaskManager.retractTask(assignable, role); + } + } taskManager.removeTask(workflow, task); } workflowRepo.delete(workflow); } + final List permissions = draft.getPermissions(); + for (final Permission permission : permissions) { + getEntityManager().remove(permission); + } + super.delete(draft); } diff --git a/ccm-cms/src/main/java/org/librecms/contentsection/ContentType.java b/ccm-cms/src/main/java/org/librecms/contentsection/ContentType.java index cc9b511e4..dbeb62345 100644 --- a/ccm-cms/src/main/java/org/librecms/contentsection/ContentType.java +++ b/ccm-cms/src/main/java/org/librecms/contentsection/ContentType.java @@ -197,7 +197,7 @@ public class ContentType extends CcmObject implements Serializable { } protected void setDefaultWorkflow(final Workflow defaultWorkflow) { - if (!defaultWorkflow.isAbstractWorkflow()) { + if (defaultWorkflow != null && !defaultWorkflow.isAbstractWorkflow()) { throw new IllegalArgumentException( "The provided workflow is not an abstract workflow."); } diff --git a/ccm-cms/src/main/java/org/librecms/contentsection/ContentTypeManager.java b/ccm-cms/src/main/java/org/librecms/contentsection/ContentTypeManager.java index d81c36328..2fb9d8071 100644 --- a/ccm-cms/src/main/java/org/librecms/contentsection/ContentTypeManager.java +++ b/ccm-cms/src/main/java/org/librecms/contentsection/ContentTypeManager.java @@ -59,21 +59,31 @@ public class ContentTypeManager { * of {@link ContentItem}. */ @SuppressWarnings("unchecked") - public Class classNameToClass(final String className) { + public Class classNameToClass( + final String className + ) { final Class clazz; try { clazz = Class.forName(className); } catch (ClassNotFoundException ex) { - throw new IllegalArgumentException(String.format( - "No class with the name \"%s\" exists.", className), - ex); + throw new IllegalArgumentException( + String.format( + "No class with the name \"%s\" exists.", + className + ), + ex + ); } if (ContentItem.class.isAssignableFrom(clazz)) { return (Class) clazz; } else { - throw new IllegalArgumentException(String.format( - "Class \"%s\" is not a content type.", className)); + throw new IllegalArgumentException( + String.format( + "Class \"%s\" is not a content type.", + className + ) + ); } } @@ -89,13 +99,39 @@ public class ContentTypeManager { public void setDefaultLifecycle( @RequiresPrivilege(AdminPrivileges.ADMINISTER_CONTENT_TYPES) final ContentType type, - final LifecycleDefinition definition) { - + final LifecycleDefinition definition + ) { + Objects.requireNonNull( + type, + "Can't set default lifecycle for ContentType null." + ); + Objects.requireNonNull( + definition, + "Can't use LifecycleDefinition null as default lifecycle." + ); type.setDefaultLifecycle(definition); typeRepo.save(type); } + /** + * Removes the default lifecycle from a content type. This does not delete + * the lifecycle definition. + * + * @param type The type from which the default lifecycle is removed. + */ + @Transactional(Transactional.TxType.REQUIRED) + @AuthorizationRequired + public void removeDefaultLifecycle(final ContentType type) { + Objects + .requireNonNull( + type, + "Can't remove default lifecycle from ContentType null." + ) + .setDefaultLifecycle(null); + typeRepo.save(type); + } + /** * Sets the default workflow to use for new items of a content type. * @@ -108,14 +144,22 @@ public class ContentTypeManager { public void setDefaultWorkflow( @RequiresPrivilege(AdminPrivileges.ADMINISTER_CONTENT_TYPES) final ContentType type, - final Workflow template) { - - Objects.requireNonNull(type); - Objects.requireNonNull(template); + final Workflow template + ) { + Objects.requireNonNull( + type, + "Can't set default workflow for ContentType null." + ); + Objects.requireNonNull( + template, + "Can't use Workflow template null as default workflow." + ); if (!template.isAbstractWorkflow()) { throw new IllegalArgumentException( - "The provided workflow is not an abstract workflow."); + "The provided workflow is not a workflow template " + + "(abstract workflow)." + ); } type.setDefaultWorkflow(template); @@ -123,6 +167,23 @@ public class ContentTypeManager { typeRepo.save(type); } + /** + * Removes the default workflow from a {@link ContentType}. This does not + * delete the workflow template. + * + * @param type The type from which the default workflow is removed. + */ + public void removeDefaultWorkflow(final ContentType type) { + Objects + .requireNonNull( + type, + "Can't remove default workflow from ContentType null." + ) + .setDefaultWorkflow(null); + + typeRepo.save(type); + } + /** * Creates a permission granting the {@link TypePrivileges#USE_TYPE} * privilege to a role. @@ -135,8 +196,8 @@ public class ContentTypeManager { public void grantUsageOfType( @RequiresPrivilege(AdminPrivileges.ADMINISTER_CONTENT_TYPES) final ContentType type, - final Role role) { - + final Role role + ) { permissionManager.grantPrivilege(TypePrivileges.USE_TYPE, role, type); } @@ -153,8 +214,8 @@ public class ContentTypeManager { public void denyUsageOnType( @RequiresPrivilege(AdminPrivileges.ADMINISTER_CONTENT_TYPES) final ContentType type, - final Role role) { - + final Role role + ) { permissionManager.revokePrivilege(TypePrivileges.USE_TYPE, role, type); } diff --git a/ccm-cms/src/main/java/org/librecms/contentsection/FolderManager.java b/ccm-cms/src/main/java/org/librecms/contentsection/FolderManager.java index 147f8fbaf..e6ad58d21 100644 --- a/ccm-cms/src/main/java/org/librecms/contentsection/FolderManager.java +++ b/ccm-cms/src/main/java/org/librecms/contentsection/FolderManager.java @@ -497,7 +497,9 @@ public class FolderManager { final List tokens = new ArrayList<>(); - tokens.add(folder.getName()); + if (folder.getParentFolder() != null) { + tokens.add(folder.getName()); + } Folder current = folder; while (getParentFolder(current).isPresent()) { current = getParentFolder(current).get(); diff --git a/ccm-cms/src/main/java/org/librecms/ui/contentsections/ConfigurationWorkflowController.java b/ccm-cms/src/main/java/org/librecms/ui/contentsections/ConfigurationWorkflowController.java index 3f7375f9a..05078592e 100644 --- a/ccm-cms/src/main/java/org/librecms/ui/contentsections/ConfigurationWorkflowController.java +++ b/ccm-cms/src/main/java/org/librecms/ui/contentsections/ConfigurationWorkflowController.java @@ -36,6 +36,8 @@ import org.libreccm.workflow.WorkflowManager; import org.libreccm.workflow.WorkflowRepository; import org.librecms.contentsection.ContentSection; import org.librecms.contentsection.ContentSectionManager; +import org.librecms.contentsection.ContentType; +import org.librecms.contentsection.ContentTypeManager; import java.util.Collections; import java.util.List; @@ -95,6 +97,12 @@ public class ConfigurationWorkflowController { */ @Inject private ContentSectionsUi sectionsUi; + + /** + * Manager for content types. + */ + @Inject + private ContentTypeManager typeManager; /** * Used for globaliazation stuff. @@ -381,6 +389,14 @@ public class ConfigurationWorkflowController { sectionManager.removeWorkflowTemplateFromContentSection( workflow, section ); + final List typesUsingWorkflow = section + .getContentTypes() + .stream() + .filter(type -> workflow.equals(type.getDefaultWorkflow())) + .collect(Collectors.toList()); + for(final ContentType typeUsingWorkflow : typesUsingWorkflow) { + typeManager.removeDefaultWorkflow(typeUsingWorkflow); + } workflowRepo.delete(workflow); return String.format( diff --git a/ccm-core/src/main/java/org/libreccm/workflow/TaskManager.java b/ccm-core/src/main/java/org/libreccm/workflow/TaskManager.java index 35f745724..096e07ab5 100644 --- a/ccm-core/src/main/java/org/libreccm/workflow/TaskManager.java +++ b/ccm-core/src/main/java/org/libreccm/workflow/TaskManager.java @@ -37,6 +37,7 @@ import javax.transaction.Transactional; import java.util.Objects; import java.util.Optional; import java.util.UUID; +import java.util.stream.Collectors; import javax.persistence.TypedQuery; @@ -48,6 +49,7 @@ import javax.persistence.TypedQuery; */ @RequestScoped public class TaskManager implements Serializable { + private static final long serialVersionUID = -5605541655413527137L; private static final Logger LOGGER = LogManager.getLogger(TaskManager.class); @@ -91,6 +93,25 @@ public class TaskManager implements Serializable { @RequiresPrivilege(CoreConstants.PRIVILEGE_ADMIN) @Transactional(Transactional.TxType.REQUIRED) public void removeTask(final Workflow workflow, final Task task) { + final List blockedTasks = task + .getBlockedTasks() + .stream() + .map(TaskDependency::getBlockedTask) + .collect(Collectors.toList()); + + for (final Task blockedTask : blockedTasks) { + removeDependentTask(task, blockedTask); + } + + final List blockingTasks = task + .getBlockingTasks() + .stream() + .map(TaskDependency::getBlockingTask) + .collect(Collectors.toList()); + for (final Task blockingTask : blockingTasks) { + removeDependentTask(blockingTask, task); + } + workflow.removeTask(task); task.setWorkflow(null); @@ -110,17 +131,19 @@ public class TaskManager implements Serializable { @AuthorizationRequired @RequiresPrivilege(CoreConstants.PRIVILEGE_ADMIN) @Transactional(Transactional.TxType.REQUIRED) - public void addDependentTask(final Task blockingTask, - final Task blockedTask) - throws CircularTaskDependencyException { - + public void addDependentTask( + final Task blockingTask, + final Task blockedTask + ) throws CircularTaskDependencyException { Objects.requireNonNull(blockedTask); Objects.requireNonNull(blockingTask); - LOGGER.debug("Adding a dependency between task {} (blocking task) " - + "and task {} (blocked task)...", - Objects.toString(blockingTask), - Objects.toString(blockedTask)); + LOGGER.debug( + "Adding a dependency between task {} (blocking task) " + + "and task {} (blocked task)...", + Objects.toString(blockingTask), + Objects.toString(blockedTask) + ); LOGGER.debug("Checking for circular dependencies..."); checkForCircularDependencies(blockedTask, blockingTask); @@ -133,10 +156,12 @@ public class TaskManager implements Serializable { final Boolean dependencyExists = query.getSingleResult(); if (dependencyExists) { - LOGGER.info("Dependency between task {} (blocking task) " - + "and task {} (blocked task) already exists.", - Objects.toString(blockingTask), - Objects.toString(blockedTask)); + LOGGER.info( + "Dependency between task {} (blocking task) " + + "and task {} (blocked task) already exists.", + Objects.toString(blockingTask), + Objects.toString(blockedTask) + ); return; } @@ -144,13 +169,12 @@ public class TaskManager implements Serializable { dependency.setUuid(UUID.randomUUID().toString()); dependency.setBlockedTask(blockedTask); dependency.setBlockingTask(blockingTask); - + blockingTask.addBlockedTask(dependency); blockedTask.addBlockingTask(dependency); - + // blockingTask.addDependentTask(blockedTask); // blockedTask.addDependsOn(blockingTask); - entityManager.persist(dependency); taskRepo.save(blockedTask); taskRepo.save(blockingTask); @@ -160,30 +184,29 @@ public class TaskManager implements Serializable { * Removes a dependent task. * * @param blockingTask The task which blocks the other task. - * @param blockedTask The task which is blocked by the other task. + * @param blockedTask The task which is blocked by the other task. */ @AuthorizationRequired @RequiresPrivilege(CoreConstants.PRIVILEGE_ADMIN) @Transactional(Transactional.TxType.REQUIRED) - public void removeDependentTask(final Task blockingTask, + public void removeDependentTask(final Task blockingTask, final Task blockedTask) { final TypedQuery query = entityManager - .createNamedQuery("Task.findDependency", TaskDependency.class); + .createNamedQuery("Task.findDependency", TaskDependency.class); query.setParameter("blockedTask", blockedTask); query.setParameter("blockingTask", blockingTask); final List dependencies = query.getResultList(); - - for(final TaskDependency dependency : dependencies) { - + + for (final TaskDependency dependency : dependencies) { + entityManager.remove(dependency); blockingTask.removeBlockedTask(dependency); blockedTask.removeBlockingTask(dependency); } - + // blockingTask.removeDependentTask(blockedTask); // blockedTask.removeDependsOn(blockingTask); - taskRepo.save(blockedTask); taskRepo.save(blockingTask); } @@ -206,14 +229,14 @@ public class TaskManager implements Serializable { } private boolean dependsOn(final Task blockingTask, final Task blockedTask) { - + final TypedQuery query = entityManager - .createNamedQuery("Task.existsDependency", Boolean.class); + .createNamedQuery("Task.existsDependency", Boolean.class); query.setParameter("blockingTask", blockingTask); query.setParameter("blockedTask", blockedTask); - + return query.getSingleResult(); - + // for (final Task current : blockingTask.getDependsOn()) { // if (current.equals(blockedTask)) { // return true; @@ -316,9 +339,9 @@ public class TaskManager implements Serializable { * @param task The task to finish. */ public void finish(final Task task) { - + Objects.requireNonNull(task, "Can't finished null..."); - + if (task.getTaskState() != TaskState.ENABLED) { throw new IllegalArgumentException(String.format( "Task %s is not enabled.", @@ -343,9 +366,8 @@ public class TaskManager implements Serializable { * @param task */ protected void updateState(final Task task) { - Objects.requireNonNull(task); - + LOGGER.debug("Updating state for task {}...", Objects.toString(task)); @@ -355,8 +377,9 @@ public class TaskManager implements Serializable { return; } - for (final TaskDependency blockingTaskDependency : task.getBlockingTasks()) { - + for (final TaskDependency blockingTaskDependency : task + .getBlockingTasks()) { + final Task blockingTask = blockingTaskDependency.getBlockingTask(); LOGGER.debug("Checking dependency {}...", Objects.toString(blockingTask)); diff --git a/ccm-core/src/main/java/org/libreccm/workflow/WorkflowManager.java b/ccm-core/src/main/java/org/libreccm/workflow/WorkflowManager.java index 3c43d040d..ab9cb37ed 100644 --- a/ccm-core/src/main/java/org/libreccm/workflow/WorkflowManager.java +++ b/ccm-core/src/main/java/org/libreccm/workflow/WorkflowManager.java @@ -291,6 +291,9 @@ public class WorkflowManager implements Serializable { ) ); } + + taskRepo.save(task); + workflowRepo.save(workflow); } /** @@ -434,21 +437,14 @@ public class WorkflowManager implements Serializable { @RequiresPrivilege(CoreConstants.PRIVILEGE_ADMIN) @Transactional(Transactional.TxType.REQUIRED) public void start(final Workflow workflow) { - final WorkflowState oldState = workflow.getState(); - - workflow.setState(WorkflowState.STARTED); - if (oldState == WorkflowState.INIT) { workflow.setActive(true); updateState(workflow); -// for (final Task current : workflow.getTasks()) { -// current.setActive(true); -// taskManager.updateState(current); -// } final List tasks = workflow.getTasks(); if (!tasks.isEmpty()) { final Task firstTask = tasks.get(0); firstTask.setActive(true); + firstTask.setTaskState(TaskState.ENABLED); taskManager.updateState(firstTask); if (firstTask instanceof AssignableTask) { @@ -465,7 +461,6 @@ public class WorkflowManager implements Serializable { } } } - } workflowRepo.save(workflow); }