From 2b3812c2af928d3649167a84eb4076343b571f51 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 30 Jan 2023 19:59:45 +0100 Subject: [PATCH 01/34] Initial implementation of step state processor --- .../data/database/persistence/CommentDAO.java | 7 + .../kitodo/config/enums/ParameterCore.java | 2 + .../production/forms/CurrentTaskForm.java | 10 ++ .../interfaces/activemq/ActiveMQDirector.java | 2 +- .../interfaces/activemq/StepState.java | 9 + .../activemq/StepStateProcessor.java | 154 ++++++++++++++++++ .../services/data/CommentService.java | 5 + .../workflow/WorkflowControllerService.java | 12 +- .../main/resources/kitodo_config.properties | 2 + 9 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepState.java create mode 100644 Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepStateProcessor.java diff --git a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java index 9eb6eadf74f..d4157865b93 100644 --- a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java +++ b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java @@ -16,6 +16,7 @@ import org.kitodo.data.database.beans.Comment; import org.kitodo.data.database.beans.Process; +import org.kitodo.data.database.beans.Task; import org.kitodo.data.database.exceptions.DAOException; public class CommentDAO extends BaseDAO { @@ -54,6 +55,12 @@ public List getAllByProcess(Process process) { Collections.singletonMap("processId", process.getId())); } + public List getAllByCurrentTask(Task task) { + return getByQuery("FROM Comment WHERE currentTask_id = :taskId ORDER BY id ASC", + Collections.singletonMap("taskId", task.getId())); + } + + /** * Save list of comments. * diff --git a/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java b/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java index 1233d20ebac..572b7eab210 100644 --- a/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java +++ b/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java @@ -605,6 +605,8 @@ public enum ParameterCore implements ParameterInterface { ACTIVE_MQ_FINALIZE_STEP_QUEUE(new Parameter("activeMQ.finalizeStep.queue")), + ACTIVE_MQ_STEP_STATE_QUEUE(new Parameter("activeMQ.stepState.queue")), + ACTIVE_MQ_USER(new Parameter("activeMQ.user")), ACTIVE_MQ_RESULTS_TOPIC(new Parameter("activeMQ.results.topic")), diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/CurrentTaskForm.java b/Kitodo/src/main/java/org/kitodo/production/forms/CurrentTaskForm.java index 8546558c61e..1a3d948af8d 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/CurrentTaskForm.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/CurrentTaskForm.java @@ -29,6 +29,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.kitodo.data.database.beans.Batch; +import org.kitodo.data.database.beans.Comment; import org.kitodo.data.database.beans.Folder; import org.kitodo.data.database.beans.Process; import org.kitodo.data.database.beans.Property; @@ -295,6 +296,15 @@ public String closeTaskByUser() { return tasksPage; } + + public void reportProblem(Comment comment) { + try { + workflowControllerService.reportProblem(comment); + } catch (DataException e) { + Helper.setErrorMessage(ERROR_SAVING, new Object[] {ObjectType.TASK.getTranslationSingular() }, logger, e); + } + } + /** * Unlock the current task's process. * diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java index 5f88d41aec6..22ab7c01e6f 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java @@ -57,7 +57,7 @@ public class ActiveMQDirector implements Runnable, ServletContextListener { private static Collection services; static { - services = Arrays.asList(new FinalizeStepProcessor()); + services = Arrays.asList(new FinalizeStepProcessor(), new StepStateProcessor()); } private static Connection connection = null; diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepState.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepState.java new file mode 100644 index 00000000000..d805126b9d5 --- /dev/null +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepState.java @@ -0,0 +1,9 @@ +package org.kitodo.production.interfaces.activemq; + +public enum StepState { + INFO, + ERROR_OPEN, + ERROR_CLOSE, + PROCESS, + CLOSE +} diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepStateProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepStateProcessor.java new file mode 100644 index 00000000000..8f1e988c0a3 --- /dev/null +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepStateProcessor.java @@ -0,0 +1,154 @@ +/* + * (c) Kitodo. Key to digital objects e. V. + * + * This file is part of the Kitodo project. + * + * It is licensed under GNU General Public License version 3 or later. + * + * For the full copyright and license information, please read the + * GPL3-License.txt file that was distributed with this source code. + */ + +package org.kitodo.production.interfaces.activemq; + +import java.io.IOException; +import java.util.Date; +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +import javax.jms.JMSException; + +import org.kitodo.config.ConfigCore; +import org.kitodo.config.enums.ParameterCore; +import org.kitodo.data.database.beans.Comment; +import org.kitodo.data.database.beans.Task; +import org.kitodo.data.database.beans.User; +import org.kitodo.data.database.enums.CommentType; +import org.kitodo.data.database.enums.TaskEditType; +import org.kitodo.data.database.enums.TaskStatus; +import org.kitodo.data.database.exceptions.DAOException; +import org.kitodo.data.elasticsearch.exceptions.CustomResponseException; +import org.kitodo.data.exceptions.DataException; +import org.kitodo.production.forms.CommentForm; +import org.kitodo.production.forms.CurrentTaskForm; +import org.kitodo.production.services.ServiceManager; + +/** + * This is a web service interface to close steps. You have to provide the step id as “id”; you can add a field + * “message” which will be added to the wiki field. + */ +public class StepStateProcessor extends ActiveMQProcessor { + + /** + * The default constructor looks up the queue name to use in kitodo_config.properties. If that is not configured and + * “null” is passed to the super constructor, this will prevent ActiveMQDirector.registerListeners() from starting + * this service. + */ + public StepStateProcessor() { + super(ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_STEP_STATE_QUEUE).orElse(null)); + } + + /** + * This is the main routine processing incoming tickets. It gets an CurrentTaskForm object, sets it to the + * appropriate step which is retrieved from the database, appends the message − if any − to the wiki field, and + * executes the form’s the step close function. + * + * @param ticket + * the incoming message + */ + @Override + protected void process(MapMessageObjectReader ticket) throws DAOException, JMSException { + CurrentTaskForm currentTaskForm = new CurrentTaskForm(); + Integer taskId = ticket.getMandatoryInteger("id"); + String state = ticket.getMandatoryString("state"); + String message = ticket.getMandatoryString("message"); + Task currentTask = ServiceManager.getTaskService().getById(taskId); + currentTaskForm.setCurrentTask(currentTask); + User currentUser = ServiceManager.getUserService().getCurrentUser(); + + Comment comment = new Comment(); + comment.setProcess(currentTaskForm.getCurrentTask().getProcess()); + comment.setAuthor(ServiceManager.getUserService().getCurrentUser()); + comment.setMessage(message); + comment.setCreationDate(new Date()); + comment.setType(CommentType.INFO); + comment.setCurrentTask(currentTask); + + if (StepState.PROCESS.name().equals(state)) { + if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus())) { + return; + } + currentTask.setProcessingStatus(TaskStatus.INWORK); + currentTask.setEditType(TaskEditType.AUTOMATIC); + currentTask.setProcessingTime(new Date()); + ServiceManager.getTaskService().replaceProcessingUser(currentTask, currentUser); + if (Objects.isNull(currentTask.getProcessingBegin())) { + currentTask.setProcessingBegin(new Date()); + try { + ServiceManager.getTaskService().save(currentTask); + } catch (DataException e) { + throw new RuntimeException(e); + } + } + } else if (StepState.ERROR_OPEN.name().equals(state)) { + if (ticket.hasField("correctionTaskId")) { + Integer correctionTaskId = Integer.parseInt(ticket.getString("correctionTaskId")); + Task correctionTask = ServiceManager.getTaskService().getById(correctionTaskId); + comment.setCorrectionTask(correctionTask); + } + comment.setType(CommentType.ERROR); + } else if (StepState.ERROR_CLOSE.name().equals(state)) { + List comments = ServiceManager.getCommentService().getAllCommentsByCurrentTask(currentTask); + Optional optionalComment; + if (ticket.hasField("correctionTaskId")) { + Integer correctionTaskId = Integer.parseInt(ticket.getString("correctionTaskId")); + optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( + currentTaskComment.getType()) && !currentTaskComment.isCorrected() && correctionTaskId.equals( + currentTaskComment.getCorrectionTask().getId())).findFirst(); + } else { + optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( + currentTaskComment.getType()) && !currentTaskComment.isCorrected() && Objects.isNull( + currentTaskComment.getCorrectionTask())).findFirst(); + } + if (optionalComment.isPresent()) { + CommentForm commentForm = new CommentForm(); + commentForm.solveProblem(optionalComment.get()); + + currentTask.setProcessingStatus(TaskStatus.OPEN); + currentTask.setEditType(TaskEditType.AUTOMATIC); + currentTask.setProcessingBegin(null); + ServiceManager.getTaskService().replaceProcessingUser(currentTask, currentUser); + try { + ServiceManager.getTaskService().save(currentTask); + } catch (DataException e) { + throw new RuntimeException(e); + } + } + } else if (StepState.CLOSE.name().equals(state)) { + currentTaskForm.closeTaskByUser(); + } + + ServiceManager.getCommentService().saveToDatabase(comment); + try { + ServiceManager.getProcessService().saveToIndex(currentTask.getProcess(), true); + + for (Task task : currentTask.getProcess().getTasks()) { + // update tasks in elastic search index, which includes correction comment status + ServiceManager.getTaskService().saveToIndex(task, true); + } + } catch (CustomResponseException e) { + throw new RuntimeException(e); + } catch (DataException e) { + throw new RuntimeException(e); + } catch (IOException e) { + throw new RuntimeException(e); + } + + if (CommentType.ERROR.equals(comment.getType())) { + currentTaskForm.reportProblem(comment); + } + + } + +} diff --git a/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java b/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java index 182bf566b09..46277bb63c8 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java @@ -17,6 +17,7 @@ import org.kitodo.data.database.beans.Comment; import org.kitodo.data.database.beans.Process; +import org.kitodo.data.database.beans.Task; import org.kitodo.data.database.exceptions.DAOException; import org.kitodo.data.database.persistence.CommentDAO; import org.kitodo.production.services.data.base.SearchDatabaseService; @@ -71,6 +72,10 @@ public List getAllCommentsByProcess(Process process) { return dao.getAllByProcess(process); } + public List getAllCommentsByCurrentTask(Task task) { + return dao.getAllByCurrentTask(task); + } + /** * Save list of comments to database. * diff --git a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java index deb3ad5888d..434f222e554 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java @@ -195,6 +195,10 @@ private boolean validateMetadata(Task task) throws IOException, DAOException { * object */ public void closeTaskByUser(Task task) throws DataException, IOException, DAOException { + if( Objects.isNull(task) ) { + return; + } + // if the result of the task is to be verified first, then if necessary, // cancel the completion if (task.isTypeCloseVerify()) { @@ -373,9 +377,13 @@ public void reportProblem(Comment comment) throws DataException { * as Comment object */ public void solveProblem(Comment comment) throws DataException, DAOException, IOException { - closeTaskByUser(comment.getCorrectionTask()); comment.setCurrentTask(ServiceManager.getTaskService().getById(comment.getCurrentTask().getId())); - comment.setCorrectionTask(ServiceManager.getTaskService().getById(comment.getCorrectionTask().getId())); + if( Objects.isNull(comment.getCorrectionTask()) ) { + MetadataLock.setFree(comment.getCurrentTask().getProcess().getId()); + } else { + closeTaskByUser(comment.getCorrectionTask()); + comment.setCorrectionTask(ServiceManager.getTaskService().getById(comment.getCorrectionTask().getId())); + } comment.setCorrected(Boolean.TRUE); comment.setCorrectionDate(new Date()); try { diff --git a/Kitodo/src/main/resources/kitodo_config.properties b/Kitodo/src/main/resources/kitodo_config.properties index ddfe7fab35e..846b1ccb048 100644 --- a/Kitodo/src/main/resources/kitodo_config.properties +++ b/Kitodo/src/main/resources/kitodo_config.properties @@ -623,6 +623,8 @@ activeMQ.user=testAdmin # You can provide a queue from which messages are read to finalize steps #activeMQ.finalizeStep.queue=KitodoProduction.FinalizeStep.Queue +# You can provide a queue from which messages are read to change steps states +activeMQ.stepState.queue=KitodoProduction.StepState.Queue # ----------------------------------- # Elasticsearch properties From 234bdc48e2cc3a841834acc777eda1932464a2f7 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 3 May 2023 12:22:27 +0200 Subject: [PATCH 02/34] Refactoring, improve documentation and checkstyle --- .../kitodo/exceptions/ProcessorException.java | 30 ++++ .../production/forms/CurrentTaskForm.java | 10 -- .../interfaces/activemq/ActiveMQDirector.java | 2 +- .../activemq/ActiveMQProcessor.java | 3 +- .../activemq/FinalizeStepProcessor.java | 45 ++--- .../interfaces/activemq/StepState.java | 9 - .../activemq/StepStateProcessor.java | 154 ---------------- .../interfaces/activemq/TaskState.java | 20 +++ .../activemq/TaskStateProcessor.java | 164 ++++++++++++++++++ .../workflow/WorkflowControllerService.java | 4 +- .../main/resources/kitodo_config.properties | 2 +- 11 files changed, 245 insertions(+), 198 deletions(-) create mode 100644 Kitodo/src/main/java/org/kitodo/exceptions/ProcessorException.java delete mode 100644 Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepState.java delete mode 100644 Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepStateProcessor.java create mode 100644 Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskState.java create mode 100644 Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java diff --git a/Kitodo/src/main/java/org/kitodo/exceptions/ProcessorException.java b/Kitodo/src/main/java/org/kitodo/exceptions/ProcessorException.java new file mode 100644 index 00000000000..2e777fa8160 --- /dev/null +++ b/Kitodo/src/main/java/org/kitodo/exceptions/ProcessorException.java @@ -0,0 +1,30 @@ +/* + * (c) Kitodo. Key to digital objects e. V. + * + * This file is part of the Kitodo project. + * + * It is licensed under GNU General Public License version 3 or later. + * + * For the full copyright and license information, please read the + * GPL3-License.txt file that was distributed with this source code. + */ + +package org.kitodo.exceptions; + +public class ProcessorException extends Exception { + + /** + * Constructor with given exception message. + * + * @param exception + * as Exception + */ + public ProcessorException(Exception exception) { + super(exception); + } + + public ProcessorException(String message) { + super(message); + } + +} diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/CurrentTaskForm.java b/Kitodo/src/main/java/org/kitodo/production/forms/CurrentTaskForm.java index 25c2999aacf..ac72fe4625f 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/CurrentTaskForm.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/CurrentTaskForm.java @@ -29,7 +29,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.kitodo.data.database.beans.Batch; -import org.kitodo.data.database.beans.Comment; import org.kitodo.data.database.beans.Folder; import org.kitodo.data.database.beans.Process; import org.kitodo.data.database.beans.Property; @@ -298,15 +297,6 @@ public String closeTaskByUser() { return tasksPage; } - - public void reportProblem(Comment comment) { - try { - workflowControllerService.reportProblem(comment); - } catch (DataException e) { - Helper.setErrorMessage(ERROR_SAVING, new Object[] {ObjectType.TASK.getTranslationSingular() }, logger, e); - } - } - /** * Unlock the current task's process. * diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java index 22ab7c01e6f..f3e8fd7b2f5 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java @@ -57,7 +57,7 @@ public class ActiveMQDirector implements Runnable, ServletContextListener { private static Collection services; static { - services = Arrays.asList(new FinalizeStepProcessor(), new StepStateProcessor()); + services = Arrays.asList(new FinalizeStepProcessor(), new TaskStateProcessor()); } private static Connection connection = null; diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQProcessor.java index 44f9879699e..9a62c509ac3 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQProcessor.java @@ -27,6 +27,7 @@ import org.kitodo.data.database.beans.Client; import org.kitodo.data.database.beans.User; import org.kitodo.data.database.exceptions.DAOException; +import org.kitodo.exceptions.ProcessorException; import org.kitodo.production.enums.ReportLevel; import org.kitodo.production.helper.Helper; import org.kitodo.production.security.SecurityUserDetails; @@ -63,7 +64,7 @@ public abstract class ActiveMQProcessor implements MessageListener { * an object providing access to the fields of the received map * message */ - protected abstract void process(MapMessageObjectReader ticket) throws DAOException, JMSException; + protected abstract void process(MapMessageObjectReader ticket) throws ProcessorException, JMSException; /** * Instantiating the class ActiveMQProcessor always requires to pass the diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/FinalizeStepProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/FinalizeStepProcessor.java index e70bdf68f38..1331075781b 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/FinalizeStepProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/FinalizeStepProcessor.java @@ -23,6 +23,7 @@ import org.kitodo.data.database.beans.Property; import org.kitodo.data.database.enums.CommentType; import org.kitodo.data.database.exceptions.DAOException; +import org.kitodo.exceptions.ProcessorException; import org.kitodo.production.forms.CurrentTaskForm; import org.kitodo.production.services.ServiceManager; @@ -44,32 +45,36 @@ public FinalizeStepProcessor() { } /** - * This is the main routine processing incoming tickets. It gets an - * CurrentTaskForm object, sets it to the appropriate step which is - * retrieved from the database, appends the message − if any − to the wiki - * field, and executes the form’s the step close function. + * This is the main routine processing incoming tickets. It gets an CurrentTaskForm object, sets it to the + * appropriate step which is retrieved from the database, appends the message − if any − to the wiki field, and + * executes the form’s the step close function. * * @param ticket - * the incoming message + * the incoming message */ @Override - protected void process(MapMessageObjectReader ticket) throws DAOException, JMSException { + protected void process(MapMessageObjectReader ticket) throws ProcessorException { CurrentTaskForm dialog = new CurrentTaskForm(); - Integer stepID = ticket.getMandatoryInteger("id"); - dialog.setCurrentTask(ServiceManager.getTaskService().getById(stepID)); - if (ticket.hasField("properties")) { - updateProperties(dialog, ticket.getMapOfStringToString("properties")); - } - if (ticket.hasField("message")) { - Comment comment = new Comment(); - comment.setProcess(dialog.getCurrentTask().getProcess()); - comment.setAuthor(ServiceManager.getUserService().getCurrentUser()); - comment.setMessage(ticket.getString("message")); - comment.setType(CommentType.INFO); - comment.setCreationDate(new Date()); - ServiceManager.getCommentService().saveToDatabase(comment); + try { + Integer stepID = ticket.getMandatoryInteger("id"); + dialog.setCurrentTask(ServiceManager.getTaskService().getById(stepID)); + + if (ticket.hasField("properties")) { + updateProperties(dialog, ticket.getMapOfStringToString("properties")); + } + if (ticket.hasField("message")) { + Comment comment = new Comment(); + comment.setProcess(dialog.getCurrentTask().getProcess()); + comment.setAuthor(ServiceManager.getUserService().getCurrentUser()); + comment.setMessage(ticket.getString("message")); + comment.setType(CommentType.INFO); + comment.setCreationDate(new Date()); + ServiceManager.getCommentService().saveToDatabase(comment); + } + dialog.closeTaskByUser(); + } catch (DAOException | JMSException e) { + throw new ProcessorException(e); } - dialog.closeTaskByUser(); } /** diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepState.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepState.java deleted file mode 100644 index d805126b9d5..00000000000 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepState.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.kitodo.production.interfaces.activemq; - -public enum StepState { - INFO, - ERROR_OPEN, - ERROR_CLOSE, - PROCESS, - CLOSE -} diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepStateProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepStateProcessor.java deleted file mode 100644 index 8f1e988c0a3..00000000000 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/StepStateProcessor.java +++ /dev/null @@ -1,154 +0,0 @@ -/* - * (c) Kitodo. Key to digital objects e. V. - * - * This file is part of the Kitodo project. - * - * It is licensed under GNU General Public License version 3 or later. - * - * For the full copyright and license information, please read the - * GPL3-License.txt file that was distributed with this source code. - */ - -package org.kitodo.production.interfaces.activemq; - -import java.io.IOException; -import java.util.Date; -import java.util.List; -import java.util.Objects; -import java.util.Optional; - -import javax.jms.JMSException; - -import org.kitodo.config.ConfigCore; -import org.kitodo.config.enums.ParameterCore; -import org.kitodo.data.database.beans.Comment; -import org.kitodo.data.database.beans.Task; -import org.kitodo.data.database.beans.User; -import org.kitodo.data.database.enums.CommentType; -import org.kitodo.data.database.enums.TaskEditType; -import org.kitodo.data.database.enums.TaskStatus; -import org.kitodo.data.database.exceptions.DAOException; -import org.kitodo.data.elasticsearch.exceptions.CustomResponseException; -import org.kitodo.data.exceptions.DataException; -import org.kitodo.production.forms.CommentForm; -import org.kitodo.production.forms.CurrentTaskForm; -import org.kitodo.production.services.ServiceManager; - -/** - * This is a web service interface to close steps. You have to provide the step id as “id”; you can add a field - * “message” which will be added to the wiki field. - */ -public class StepStateProcessor extends ActiveMQProcessor { - - /** - * The default constructor looks up the queue name to use in kitodo_config.properties. If that is not configured and - * “null” is passed to the super constructor, this will prevent ActiveMQDirector.registerListeners() from starting - * this service. - */ - public StepStateProcessor() { - super(ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_STEP_STATE_QUEUE).orElse(null)); - } - - /** - * This is the main routine processing incoming tickets. It gets an CurrentTaskForm object, sets it to the - * appropriate step which is retrieved from the database, appends the message − if any − to the wiki field, and - * executes the form’s the step close function. - * - * @param ticket - * the incoming message - */ - @Override - protected void process(MapMessageObjectReader ticket) throws DAOException, JMSException { - CurrentTaskForm currentTaskForm = new CurrentTaskForm(); - Integer taskId = ticket.getMandatoryInteger("id"); - String state = ticket.getMandatoryString("state"); - String message = ticket.getMandatoryString("message"); - Task currentTask = ServiceManager.getTaskService().getById(taskId); - currentTaskForm.setCurrentTask(currentTask); - User currentUser = ServiceManager.getUserService().getCurrentUser(); - - Comment comment = new Comment(); - comment.setProcess(currentTaskForm.getCurrentTask().getProcess()); - comment.setAuthor(ServiceManager.getUserService().getCurrentUser()); - comment.setMessage(message); - comment.setCreationDate(new Date()); - comment.setType(CommentType.INFO); - comment.setCurrentTask(currentTask); - - if (StepState.PROCESS.name().equals(state)) { - if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus())) { - return; - } - currentTask.setProcessingStatus(TaskStatus.INWORK); - currentTask.setEditType(TaskEditType.AUTOMATIC); - currentTask.setProcessingTime(new Date()); - ServiceManager.getTaskService().replaceProcessingUser(currentTask, currentUser); - if (Objects.isNull(currentTask.getProcessingBegin())) { - currentTask.setProcessingBegin(new Date()); - try { - ServiceManager.getTaskService().save(currentTask); - } catch (DataException e) { - throw new RuntimeException(e); - } - } - } else if (StepState.ERROR_OPEN.name().equals(state)) { - if (ticket.hasField("correctionTaskId")) { - Integer correctionTaskId = Integer.parseInt(ticket.getString("correctionTaskId")); - Task correctionTask = ServiceManager.getTaskService().getById(correctionTaskId); - comment.setCorrectionTask(correctionTask); - } - comment.setType(CommentType.ERROR); - } else if (StepState.ERROR_CLOSE.name().equals(state)) { - List comments = ServiceManager.getCommentService().getAllCommentsByCurrentTask(currentTask); - Optional optionalComment; - if (ticket.hasField("correctionTaskId")) { - Integer correctionTaskId = Integer.parseInt(ticket.getString("correctionTaskId")); - optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( - currentTaskComment.getType()) && !currentTaskComment.isCorrected() && correctionTaskId.equals( - currentTaskComment.getCorrectionTask().getId())).findFirst(); - } else { - optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( - currentTaskComment.getType()) && !currentTaskComment.isCorrected() && Objects.isNull( - currentTaskComment.getCorrectionTask())).findFirst(); - } - if (optionalComment.isPresent()) { - CommentForm commentForm = new CommentForm(); - commentForm.solveProblem(optionalComment.get()); - - currentTask.setProcessingStatus(TaskStatus.OPEN); - currentTask.setEditType(TaskEditType.AUTOMATIC); - currentTask.setProcessingBegin(null); - ServiceManager.getTaskService().replaceProcessingUser(currentTask, currentUser); - try { - ServiceManager.getTaskService().save(currentTask); - } catch (DataException e) { - throw new RuntimeException(e); - } - } - } else if (StepState.CLOSE.name().equals(state)) { - currentTaskForm.closeTaskByUser(); - } - - ServiceManager.getCommentService().saveToDatabase(comment); - try { - ServiceManager.getProcessService().saveToIndex(currentTask.getProcess(), true); - - for (Task task : currentTask.getProcess().getTasks()) { - // update tasks in elastic search index, which includes correction comment status - ServiceManager.getTaskService().saveToIndex(task, true); - } - } catch (CustomResponseException e) { - throw new RuntimeException(e); - } catch (DataException e) { - throw new RuntimeException(e); - } catch (IOException e) { - throw new RuntimeException(e); - } - - if (CommentType.ERROR.equals(comment.getType())) { - currentTaskForm.reportProblem(comment); - } - - } - -} diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskState.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskState.java new file mode 100644 index 00000000000..bbaee71deb9 --- /dev/null +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskState.java @@ -0,0 +1,20 @@ +/* + * (c) Kitodo. Key to digital objects e. V. + * + * This file is part of the Kitodo project. + * + * It is licensed under GNU General Public License version 3 or later. + * + * For the full copyright and license information, please read the + * GPL3-License.txt file that was distributed with this source code. + */ + +package org.kitodo.production.interfaces.activemq; + +public enum TaskState { + INFO, + ERROR_OPEN, + ERROR_CLOSE, + PROCESS, + CLOSE +} diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java new file mode 100644 index 00000000000..00f9ebeace9 --- /dev/null +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java @@ -0,0 +1,164 @@ +/* + * (c) Kitodo. Key to digital objects e. V. + * + * This file is part of the Kitodo project. + * + * It is licensed under GNU General Public License version 3 or later. + * + * For the full copyright and license information, please read the + * GPL3-License.txt file that was distributed with this source code. + */ + +package org.kitodo.production.interfaces.activemq; + +import java.io.IOException; +import java.util.Date; +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +import javax.jms.JMSException; + +import org.kitodo.config.ConfigCore; +import org.kitodo.config.enums.ParameterCore; +import org.kitodo.data.database.beans.Comment; +import org.kitodo.data.database.beans.Task; +import org.kitodo.data.database.beans.User; +import org.kitodo.data.database.enums.CommentType; +import org.kitodo.data.database.enums.TaskEditType; +import org.kitodo.data.database.enums.TaskStatus; +import org.kitodo.data.database.exceptions.DAOException; +import org.kitodo.data.elasticsearch.exceptions.CustomResponseException; +import org.kitodo.data.exceptions.DataException; +import org.kitodo.exceptions.ProcessorException; +import org.kitodo.production.forms.CommentForm; +import org.kitodo.production.forms.CurrentTaskForm; +import org.kitodo.production.services.ServiceManager; +import org.kitodo.production.services.workflow.WorkflowControllerService; + +/** + * This is a web service interface to modify task states. + */ +public class TaskStateProcessor extends ActiveMQProcessor { + + /** + * The default constructor looks up the queue name to use in kitodo_config.properties. If that is not configured and + * “null” is passed to the super constructor, this will prevent ActiveMQDirector.registerListeners() from starting + * this service. + */ + public TaskStateProcessor() { + super(ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_STEP_STATE_QUEUE).orElse(null)); + } + + /** + * This is the main routine processing incoming tickets. It gets an CurrentTaskForm object, sets it to the + * appropriate step which is retrieved from the database, appends the message − if any − to the wiki field, and + * executes the form’s the step close function. + * + * @param mapMessageObjectReader + * the incoming message + */ + @Override + protected void process(MapMessageObjectReader mapMessageObjectReader) throws ProcessorException, JMSException { + CurrentTaskForm currentTaskForm = new CurrentTaskForm(); + Integer taskId = mapMessageObjectReader.getMandatoryInteger("id"); + String state = mapMessageObjectReader.getMandatoryString("state"); + TaskState taskState; + try { + taskState = TaskState.valueOf(state); + } catch (IllegalArgumentException e) { + throw new ProcessorException("Unknown task state: " + state); + } + + try { + Task currentTask = ServiceManager.getTaskService().getById(taskId); + currentTaskForm.setCurrentTask(currentTask); + Comment comment = new Comment(); + comment.setProcess(currentTaskForm.getCurrentTask().getProcess()); + comment.setAuthor(ServiceManager.getUserService().getCurrentUser()); + + String message = mapMessageObjectReader.getMandatoryString("message"); + comment.setMessage(message); + comment.setCreationDate(new Date()); + comment.setType(CommentType.INFO); + comment.setCurrentTask(currentTask); + + User currentUser = ServiceManager.getUserService().getCurrentUser(); + if (TaskState.PROCESS.equals(taskState) && processTaskStateProcess(currentTask, currentUser)) { + return; + } else if (TaskState.ERROR_OPEN.equals(taskState)) { + processTaskStateErrorOpen(mapMessageObjectReader, comment); + } else if (TaskState.ERROR_CLOSE.equals(taskState)) { + processTaskStateErrorClose(mapMessageObjectReader, currentTask, currentUser); + } else if (TaskState.CLOSE.equals(taskState)) { + currentTaskForm.closeTaskByUser(); + } + + ServiceManager.getCommentService().saveToDatabase(comment); + ServiceManager.getProcessService().saveToIndex(currentTask.getProcess(), true); + + for (Task task : currentTask.getProcess().getTasks()) { + // update tasks in elastic search index, which includes correction comment status + ServiceManager.getTaskService().saveToIndex(task, true); + } + + } catch (DataException | DAOException | CustomResponseException | IOException e) { + throw new ProcessorException(e); + } + } + + private static void processTaskStateErrorOpen(MapMessageObjectReader mapMessageObjectReader, Comment comment) + throws JMSException, DataException, DAOException { + if (mapMessageObjectReader.hasField("correctionTaskId")) { + Integer correctionTaskId = Integer.parseInt(mapMessageObjectReader.getString("correctionTaskId")); + Task correctionTask = ServiceManager.getTaskService().getById(correctionTaskId); + comment.setCorrectionTask(correctionTask); + } + comment.setType(CommentType.ERROR); + new WorkflowControllerService().reportProblem(comment); + } + + private static boolean processTaskStateProcess(Task currentTask, User currentUser) throws DataException { + if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus())) { + return true; + } + currentTask.setProcessingStatus(TaskStatus.INWORK); + currentTask.setEditType(TaskEditType.AUTOMATIC); + currentTask.setProcessingTime(new Date()); + ServiceManager.getTaskService().replaceProcessingUser(currentTask, currentUser); + if (Objects.isNull(currentTask.getProcessingBegin())) { + currentTask.setProcessingBegin(new Date()); + ServiceManager.getTaskService().save(currentTask); + } + return false; + } + + private static void processTaskStateErrorClose(MapMessageObjectReader mapMessageObjectReader, Task currentTask, + User currentUser) throws JMSException, DataException { + List comments = ServiceManager.getCommentService().getAllCommentsByCurrentTask(currentTask); + Optional optionalComment; + if (mapMessageObjectReader.hasField("correctionTaskId")) { + Integer correctionTaskId = Integer.parseInt(mapMessageObjectReader.getString("correctionTaskId")); + optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( + currentTaskComment.getType()) && !currentTaskComment.isCorrected() && correctionTaskId.equals( + currentTaskComment.getCorrectionTask().getId())).findFirst(); + } else { + optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( + currentTaskComment.getType()) && !currentTaskComment.isCorrected() && Objects.isNull( + currentTaskComment.getCorrectionTask())).findFirst(); + } + if (optionalComment.isPresent()) { + CommentForm commentForm = new CommentForm(); + commentForm.solveProblem(optionalComment.get()); + + currentTask.setProcessingStatus(TaskStatus.OPEN); + currentTask.setEditType(TaskEditType.AUTOMATIC); + currentTask.setProcessingBegin(null); + currentTask.setProcessingTime(null); + + ServiceManager.getTaskService().replaceProcessingUser(currentTask, currentUser); + ServiceManager.getTaskService().save(currentTask); + } + } + +} diff --git a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java index 42faf43912a..281cd769efc 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java @@ -201,7 +201,7 @@ private boolean validateMetadata(Task task) throws IOException, DAOException { * object */ public void closeTaskByUser(Task task) throws DataException, IOException, DAOException { - if( Objects.isNull(task) ) { + if ( Objects.isNull(task) ) { return; } @@ -384,7 +384,7 @@ public void reportProblem(Comment comment) throws DataException { */ public void solveProblem(Comment comment) throws DataException, DAOException, IOException { comment.setCurrentTask(ServiceManager.getTaskService().getById(comment.getCurrentTask().getId())); - if( Objects.isNull(comment.getCorrectionTask()) ) { + if ( Objects.isNull(comment.getCorrectionTask()) ) { MetadataLock.setFree(comment.getCurrentTask().getProcess().getId()); } else { closeTaskByUser(comment.getCorrectionTask()); diff --git a/Kitodo/src/main/resources/kitodo_config.properties b/Kitodo/src/main/resources/kitodo_config.properties index 64dad7ffeaf..1f902815522 100644 --- a/Kitodo/src/main/resources/kitodo_config.properties +++ b/Kitodo/src/main/resources/kitodo_config.properties @@ -624,7 +624,7 @@ activeMQ.user=testAdmin #activeMQ.finalizeStep.queue=KitodoProduction.FinalizeStep.Queue # You can provide a queue from which messages are read to change steps states -activeMQ.stepState.queue=KitodoProduction.StepState.Queue +activeMQ.taskState.queue=KitodoProduction.StepState.Queue # ----------------------------------- # Elasticsearch properties From a294a17659489d54caf59f77e474ae82f51c87f7 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 3 May 2023 12:28:35 +0200 Subject: [PATCH 03/34] Rename step to task --- Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java | 2 +- .../production/interfaces/activemq/TaskStateProcessor.java | 2 +- Kitodo/src/main/resources/kitodo_config.properties | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java b/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java index 3998720779f..4283af11cfe 100644 --- a/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java +++ b/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java @@ -610,7 +610,7 @@ public enum ParameterCore implements ParameterInterface { ACTIVE_MQ_FINALIZE_STEP_QUEUE(new Parameter("activeMQ.finalizeStep.queue")), - ACTIVE_MQ_STEP_STATE_QUEUE(new Parameter("activeMQ.stepState.queue")), + ACTIVE_MQ_TASK_STATE_QUEUE(new Parameter("activeMQ.taskState.queue")), ACTIVE_MQ_USER(new Parameter("activeMQ.user")), diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java index 00f9ebeace9..a8f4dc89f6d 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java @@ -47,7 +47,7 @@ public class TaskStateProcessor extends ActiveMQProcessor { * this service. */ public TaskStateProcessor() { - super(ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_STEP_STATE_QUEUE).orElse(null)); + super(ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_TASK_STATE_QUEUE).orElse(null)); } /** diff --git a/Kitodo/src/main/resources/kitodo_config.properties b/Kitodo/src/main/resources/kitodo_config.properties index 1f902815522..350e6f3431b 100644 --- a/Kitodo/src/main/resources/kitodo_config.properties +++ b/Kitodo/src/main/resources/kitodo_config.properties @@ -624,7 +624,7 @@ activeMQ.user=testAdmin #activeMQ.finalizeStep.queue=KitodoProduction.FinalizeStep.Queue # You can provide a queue from which messages are read to change steps states -activeMQ.taskState.queue=KitodoProduction.StepState.Queue +activeMQ.taskState.queue=KitodoProduction.TaskState.Queue # ----------------------------------- # Elasticsearch properties From dfbec970a0631e1d43e7e92b9a1ff14e09317c8f Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 3 May 2023 14:21:20 +0200 Subject: [PATCH 04/34] Minor improvements --- .../java/org/kitodo/exceptions/ProcessorException.java | 10 ++++++++-- .../interfaces/activemq/FinalizeStepProcessor.java | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/exceptions/ProcessorException.java b/Kitodo/src/main/java/org/kitodo/exceptions/ProcessorException.java index 2e777fa8160..3e5c464e00f 100644 --- a/Kitodo/src/main/java/org/kitodo/exceptions/ProcessorException.java +++ b/Kitodo/src/main/java/org/kitodo/exceptions/ProcessorException.java @@ -14,15 +14,21 @@ public class ProcessorException extends Exception { /** - * Constructor with given exception message. + * Constructor with given exception. * * @param exception - * as Exception + * as Exception */ public ProcessorException(Exception exception) { super(exception); } + /** + * Constructor with given message. + * + * @param message + * the exception message + */ public ProcessorException(String message) { super(message); } diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/FinalizeStepProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/FinalizeStepProcessor.java index 1331075781b..eba784164cf 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/FinalizeStepProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/FinalizeStepProcessor.java @@ -53,7 +53,7 @@ public FinalizeStepProcessor() { * the incoming message */ @Override - protected void process(MapMessageObjectReader ticket) throws ProcessorException { + protected void process(MapMessageObjectReader ticket) throws ProcessorException, JMSException { CurrentTaskForm dialog = new CurrentTaskForm(); try { Integer stepID = ticket.getMandatoryInteger("id"); @@ -65,14 +65,14 @@ protected void process(MapMessageObjectReader ticket) throws ProcessorException if (ticket.hasField("message")) { Comment comment = new Comment(); comment.setProcess(dialog.getCurrentTask().getProcess()); - comment.setAuthor(ServiceManager.getUserService().getCurrentUser()); comment.setMessage(ticket.getString("message")); + comment.setAuthor(ServiceManager.getUserService().getCurrentUser()); comment.setType(CommentType.INFO); comment.setCreationDate(new Date()); ServiceManager.getCommentService().saveToDatabase(comment); } dialog.closeTaskByUser(); - } catch (DAOException | JMSException e) { + } catch (DAOException e) { throw new ProcessorException(e); } } From 623ce86d6a6f926167b46404d8dad929f5996e8e Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 3 May 2023 14:33:38 +0200 Subject: [PATCH 05/34] Update Kitodo/src/main/resources/kitodo_config.properties --- Kitodo/src/main/resources/kitodo_config.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Kitodo/src/main/resources/kitodo_config.properties b/Kitodo/src/main/resources/kitodo_config.properties index 350e6f3431b..ee46c54a77a 100644 --- a/Kitodo/src/main/resources/kitodo_config.properties +++ b/Kitodo/src/main/resources/kitodo_config.properties @@ -624,7 +624,7 @@ activeMQ.user=testAdmin #activeMQ.finalizeStep.queue=KitodoProduction.FinalizeStep.Queue # You can provide a queue from which messages are read to change steps states -activeMQ.taskState.queue=KitodoProduction.TaskState.Queue +#activeMQ.taskState.queue=KitodoProduction.TaskState.Queue # ----------------------------------- # Elasticsearch properties From 464b229a5e48be30b8685c5f5bac95b7800d9f69 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 3 May 2023 15:22:36 +0200 Subject: [PATCH 06/34] Revert WorkflowControllerService changes --- .../services/workflow/WorkflowControllerService.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java index 281cd769efc..0ee053fc5fb 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java @@ -201,10 +201,6 @@ private boolean validateMetadata(Task task) throws IOException, DAOException { * object */ public void closeTaskByUser(Task task) throws DataException, IOException, DAOException { - if ( Objects.isNull(task) ) { - return; - } - // if the result of the task is to be verified first, then if necessary, // cancel the completion if (task.isTypeCloseVerify()) { @@ -383,13 +379,9 @@ public void reportProblem(Comment comment) throws DataException { * as Comment object */ public void solveProblem(Comment comment) throws DataException, DAOException, IOException { + closeTaskByUser(comment.getCorrectionTask()); comment.setCurrentTask(ServiceManager.getTaskService().getById(comment.getCurrentTask().getId())); - if ( Objects.isNull(comment.getCorrectionTask()) ) { - MetadataLock.setFree(comment.getCurrentTask().getProcess().getId()); - } else { - closeTaskByUser(comment.getCorrectionTask()); - comment.setCorrectionTask(ServiceManager.getTaskService().getById(comment.getCorrectionTask().getId())); - } + comment.setCorrectionTask(ServiceManager.getTaskService().getById(comment.getCorrectionTask().getId())); comment.setCorrected(Boolean.TRUE); comment.setCorrectionDate(new Date()); try { From 2c22629ce52ab4931da7aa2ca893b68e71e2e455 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 3 May 2023 16:15:34 +0200 Subject: [PATCH 07/34] Revert changes and add null check again --- .../services/workflow/WorkflowControllerService.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java index 0ee053fc5fb..9b96615faa0 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java @@ -201,6 +201,10 @@ private boolean validateMetadata(Task task) throws IOException, DAOException { * object */ public void closeTaskByUser(Task task) throws DataException, IOException, DAOException { + if ( Objects.isNull(task) ) { + return; + } + // if the result of the task is to be verified first, then if necessary, // cancel the completion if (task.isTypeCloseVerify()) { @@ -379,9 +383,13 @@ public void reportProblem(Comment comment) throws DataException { * as Comment object */ public void solveProblem(Comment comment) throws DataException, DAOException, IOException { - closeTaskByUser(comment.getCorrectionTask()); comment.setCurrentTask(ServiceManager.getTaskService().getById(comment.getCurrentTask().getId())); - comment.setCorrectionTask(ServiceManager.getTaskService().getById(comment.getCorrectionTask().getId())); + if ( Objects.nonNull(comment.getCorrectionTask()) ) { + closeTaskByUser(comment.getCorrectionTask()); + comment.setCorrectionTask(ServiceManager.getTaskService().getById(comment.getCorrectionTask().getId())); + } else { + MetadataLock.setFree(comment.getCurrentTask().getProcess().getId()); + } comment.setCorrected(Boolean.TRUE); comment.setCorrectionDate(new Date()); try { From 55c1b80919f7ff0fa2104b84e299eb5bafcc5f92 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 3 May 2023 16:49:41 +0200 Subject: [PATCH 08/34] Improve order of null check --- .../services/workflow/WorkflowControllerService.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java index 9b96615faa0..9ad29a3325c 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java @@ -201,10 +201,6 @@ private boolean validateMetadata(Task task) throws IOException, DAOException { * object */ public void closeTaskByUser(Task task) throws DataException, IOException, DAOException { - if ( Objects.isNull(task) ) { - return; - } - // if the result of the task is to be verified first, then if necessary, // cancel the completion if (task.isTypeCloseVerify()) { @@ -383,13 +379,11 @@ public void reportProblem(Comment comment) throws DataException { * as Comment object */ public void solveProblem(Comment comment) throws DataException, DAOException, IOException { - comment.setCurrentTask(ServiceManager.getTaskService().getById(comment.getCurrentTask().getId())); - if ( Objects.nonNull(comment.getCorrectionTask()) ) { + if (Objects.nonNull(comment.getCorrectionTask())) { closeTaskByUser(comment.getCorrectionTask()); comment.setCorrectionTask(ServiceManager.getTaskService().getById(comment.getCorrectionTask().getId())); - } else { - MetadataLock.setFree(comment.getCurrentTask().getProcess().getId()); } + comment.setCurrentTask(ServiceManager.getTaskService().getById(comment.getCurrentTask().getId())); comment.setCorrected(Boolean.TRUE); comment.setCorrectionDate(new Date()); try { From 0a6d5d66eafddfd06fcf0074477d67895144f4c4 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Thu, 4 May 2023 17:13:08 +0200 Subject: [PATCH 09/34] Rename processor and enium to more unambiguous name, inital adding test --- .../interfaces/activemq/ActiveMQDirector.java | 2 +- ...or.java => TaskStatusChangeProcessor.java} | 50 +++--- ...skState.java => TaskStatusChangeType.java} | 2 +- .../workflow/WorkflowControllerService.java | 10 +- .../activemq/TaskStatusChangeProcessorIT.java | 145 ++++++++++++++++++ 5 files changed, 179 insertions(+), 30 deletions(-) rename Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/{TaskStateProcessor.java => TaskStatusChangeProcessor.java} (78%) rename Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/{TaskState.java => TaskStatusChangeType.java} (92%) create mode 100644 Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessorIT.java diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java index f3e8fd7b2f5..0ff353f58e1 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java @@ -57,7 +57,7 @@ public class ActiveMQDirector implements Runnable, ServletContextListener { private static Collection services; static { - services = Arrays.asList(new FinalizeStepProcessor(), new TaskStateProcessor()); + services = Arrays.asList(new FinalizeStepProcessor(), new TaskStatusChangeProcessor()); } private static Connection connection = null; diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessor.java similarity index 78% rename from Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java rename to Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessor.java index a8f4dc89f6d..da089824c8b 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStateProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessor.java @@ -34,19 +34,22 @@ import org.kitodo.production.forms.CommentForm; import org.kitodo.production.forms.CurrentTaskForm; import org.kitodo.production.services.ServiceManager; +import org.kitodo.production.services.data.TaskService; import org.kitodo.production.services.workflow.WorkflowControllerService; /** * This is a web service interface to modify task states. */ -public class TaskStateProcessor extends ActiveMQProcessor { +public class TaskStatusChangeProcessor extends ActiveMQProcessor { + + private final TaskService taskService = ServiceManager.getTaskService(); /** * The default constructor looks up the queue name to use in kitodo_config.properties. If that is not configured and * “null” is passed to the super constructor, this will prevent ActiveMQDirector.registerListeners() from starting * this service. */ - public TaskStateProcessor() { + public TaskStatusChangeProcessor() { super(ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_TASK_STATE_QUEUE).orElse(null)); } @@ -62,16 +65,16 @@ public TaskStateProcessor() { protected void process(MapMessageObjectReader mapMessageObjectReader) throws ProcessorException, JMSException { CurrentTaskForm currentTaskForm = new CurrentTaskForm(); Integer taskId = mapMessageObjectReader.getMandatoryInteger("id"); - String state = mapMessageObjectReader.getMandatoryString("state"); - TaskState taskState; + String state = mapMessageObjectReader.getMandatoryString("type"); + TaskStatusChangeType taskStatusChangeType; try { - taskState = TaskState.valueOf(state); + taskStatusChangeType = TaskStatusChangeType.valueOf(state); } catch (IllegalArgumentException e) { throw new ProcessorException("Unknown task state: " + state); } try { - Task currentTask = ServiceManager.getTaskService().getById(taskId); + Task currentTask = taskService.getById(taskId); currentTaskForm.setCurrentTask(currentTask); Comment comment = new Comment(); comment.setProcess(currentTaskForm.getCurrentTask().getProcess()); @@ -84,13 +87,16 @@ protected void process(MapMessageObjectReader mapMessageObjectReader) throws Pro comment.setCurrentTask(currentTask); User currentUser = ServiceManager.getUserService().getCurrentUser(); - if (TaskState.PROCESS.equals(taskState) && processTaskStateProcess(currentTask, currentUser)) { - return; - } else if (TaskState.ERROR_OPEN.equals(taskState)) { + if (TaskStatusChangeType.PROCESS.equals(taskStatusChangeType)) { + if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus())) { + throw new ProcessorException("Status of task is not OPEN."); + } + processTaskStateProcess(currentTask, currentUser); + } else if (TaskStatusChangeType.ERROR_OPEN.equals(taskStatusChangeType)) { processTaskStateErrorOpen(mapMessageObjectReader, comment); - } else if (TaskState.ERROR_CLOSE.equals(taskState)) { + } else if (TaskStatusChangeType.ERROR_CLOSE.equals(taskStatusChangeType)) { processTaskStateErrorClose(mapMessageObjectReader, currentTask, currentUser); - } else if (TaskState.CLOSE.equals(taskState)) { + } else if (TaskStatusChangeType.CLOSE.equals(taskStatusChangeType)) { currentTaskForm.closeTaskByUser(); } @@ -99,7 +105,7 @@ protected void process(MapMessageObjectReader mapMessageObjectReader) throws Pro for (Task task : currentTask.getProcess().getTasks()) { // update tasks in elastic search index, which includes correction comment status - ServiceManager.getTaskService().saveToIndex(task, true); + taskService.saveToIndex(task, true); } } catch (DataException | DAOException | CustomResponseException | IOException e) { @@ -107,33 +113,29 @@ protected void process(MapMessageObjectReader mapMessageObjectReader) throws Pro } } - private static void processTaskStateErrorOpen(MapMessageObjectReader mapMessageObjectReader, Comment comment) + private void processTaskStateErrorOpen(MapMessageObjectReader mapMessageObjectReader, Comment comment) throws JMSException, DataException, DAOException { if (mapMessageObjectReader.hasField("correctionTaskId")) { Integer correctionTaskId = Integer.parseInt(mapMessageObjectReader.getString("correctionTaskId")); - Task correctionTask = ServiceManager.getTaskService().getById(correctionTaskId); + Task correctionTask = taskService.getById(correctionTaskId); comment.setCorrectionTask(correctionTask); } comment.setType(CommentType.ERROR); new WorkflowControllerService().reportProblem(comment); } - private static boolean processTaskStateProcess(Task currentTask, User currentUser) throws DataException { - if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus())) { - return true; - } + private void processTaskStateProcess(Task currentTask, User currentUser) throws DataException { currentTask.setProcessingStatus(TaskStatus.INWORK); currentTask.setEditType(TaskEditType.AUTOMATIC); currentTask.setProcessingTime(new Date()); - ServiceManager.getTaskService().replaceProcessingUser(currentTask, currentUser); + taskService.replaceProcessingUser(currentTask, currentUser); if (Objects.isNull(currentTask.getProcessingBegin())) { currentTask.setProcessingBegin(new Date()); - ServiceManager.getTaskService().save(currentTask); + taskService.save(currentTask); } - return false; } - private static void processTaskStateErrorClose(MapMessageObjectReader mapMessageObjectReader, Task currentTask, + private void processTaskStateErrorClose(MapMessageObjectReader mapMessageObjectReader, Task currentTask, User currentUser) throws JMSException, DataException { List comments = ServiceManager.getCommentService().getAllCommentsByCurrentTask(currentTask); Optional optionalComment; @@ -156,8 +158,8 @@ private static void processTaskStateErrorClose(MapMessageObjectReader mapMessage currentTask.setProcessingBegin(null); currentTask.setProcessingTime(null); - ServiceManager.getTaskService().replaceProcessingUser(currentTask, currentUser); - ServiceManager.getTaskService().save(currentTask); + taskService.replaceProcessingUser(currentTask, currentUser); + taskService.save(currentTask); } } diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskState.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeType.java similarity index 92% rename from Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskState.java rename to Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeType.java index bbaee71deb9..5d52f76bc2d 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskState.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeType.java @@ -11,7 +11,7 @@ package org.kitodo.production.interfaces.activemq; -public enum TaskState { +public enum TaskStatusChangeType { INFO, ERROR_OPEN, ERROR_CLOSE, diff --git a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java index 9ad29a3325c..fc3eae07b4d 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java @@ -363,10 +363,12 @@ public void reportProblem(Comment comment) throws DataException { taskService.save(currentTask); Task correctionTask = comment.getCorrectionTask(); - correctionTask.setProcessingStatus(TaskStatus.OPEN); - correctionTask.setProcessingEnd(null); - correctionTask.setCorrection(true); - taskService.save(correctionTask); + if ( Objects.isNull(correctionTask) ) { + correctionTask.setProcessingStatus(TaskStatus.OPEN); + correctionTask.setProcessingEnd(null); + correctionTask.setCorrection(true); + taskService.save(correctionTask); + } lockTasksBetweenCurrentAndCorrectionTask(currentTask, correctionTask); updateProcessSortHelperStatus(currentTask.getProcess()); diff --git a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessorIT.java b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessorIT.java new file mode 100644 index 00000000000..899fb85e3d8 --- /dev/null +++ b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessorIT.java @@ -0,0 +1,145 @@ +/* + * (c) Kitodo. Key to digital objects e. V. + * + * This file is part of the Kitodo project. + * + * It is licensed under GNU General Public License version 3 or later. + * + * For the full copyright and license information, please read the + * GPL3-License.txt file that was distributed with this source code. + */ + +package org.kitodo.production.interfaces.activemq; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +import java.io.File; + +import org.apache.commons.lang3.SystemUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.TestMethodOrder; +import org.kitodo.ExecutionPermission; +import org.kitodo.MockDatabase; +import org.kitodo.SecurityTestUtils; +import org.kitodo.config.ConfigCore; +import org.kitodo.config.enums.ParameterCore; +import org.kitodo.data.database.beans.Task; +import org.kitodo.data.database.enums.TaskStatus; +import org.kitodo.exceptions.ProcessorException; +import org.kitodo.production.services.ServiceManager; +import org.kitodo.production.services.data.TaskService; +import org.kitodo.production.services.file.FileService; +import org.kitodo.production.services.workflow.WorkflowControllerService; + +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +public class TaskStatusChangeProcessorIT { + + private static final File scriptCreateDirUserHome = new File( + ConfigCore.getParameter(ParameterCore.SCRIPT_CREATE_DIR_USER_HOME)); + private static final File scriptCreateSymLink = new File( + ConfigCore.getParameter(ParameterCore.SCRIPT_CREATE_SYMLINK)); + private static final File scriptNotWorking = new File("src/test/resources/scripts/not_working_script.sh"); + private static final File scriptWorking = new File("src/test/resources/scripts/working_script.sh"); + private static final File usersDirectory = new File("src/test/resources/users"); + private static final FileService fileService = ServiceManager.getFileService(); + private static final TaskService taskService = ServiceManager.getTaskService(); + private static final WorkflowControllerService workflowService = new WorkflowControllerService(); + + @Before + public void prepareDatabase() throws Exception { + MockDatabase.startNode(); + MockDatabase.insertProcessesForWorkflowFull(); + SecurityTestUtils.addUserDataToSecurityContext(ServiceManager.getUserService().getById(1), 1); + + usersDirectory.mkdir(); + + if (!SystemUtils.IS_OS_WINDOWS) { + ExecutionPermission.setExecutePermission(scriptCreateDirUserHome); + ExecutionPermission.setExecutePermission(scriptCreateSymLink); + ExecutionPermission.setExecutePermission(scriptNotWorking); + ExecutionPermission.setExecutePermission(scriptWorking); + } + } + + @After + public void cleanDatabase() throws Exception { + MockDatabase.stopNode(); + MockDatabase.cleanDatabase(); + SecurityTestUtils.cleanSecurityContext(); + + if (!SystemUtils.IS_OS_WINDOWS) { + ExecutionPermission.setNoExecutePermission(scriptCreateDirUserHome); + ExecutionPermission.setNoExecutePermission(scriptCreateSymLink); + ExecutionPermission.setNoExecutePermission(scriptNotWorking); + ExecutionPermission.setNoExecutePermission(scriptWorking); + } + + usersDirectory.delete(); + } + + @Test + public void useTaskStatusChangeTypeProcess() throws Exception { + Task task = taskService.getById(9); + assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + task.getProcessingStatus()); + + TaskStatusChangeProcessor taskStatusChangeProcessor = spy(TaskStatusChangeProcessor.class); + + MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); + when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(task.getId()); + when(mapMessageObjectReader.getMandatoryString("type")).thenReturn(TaskStatusChangeType.PROCESS.name()); + taskStatusChangeProcessor.process(mapMessageObjectReader); + assertEquals("Task '" + task.getTitle() + "' status should be INWORK!", TaskStatus.INWORK, + taskService.getById(task.getId()).getProcessingStatus()); + } + + @Test(expected = ProcessorException.class) + public void useTaskStatusChangeTypeProcessForTaskWithoutOpenStatus() throws Exception { + Task task = taskService.getById(8); + assertEquals("Task '" + task.getTitle() + "' status should be INWORK!", TaskStatus.INWORK, + task.getProcessingStatus()); + + TaskStatusChangeProcessor taskStatusChangeProcessor = spy(TaskStatusChangeProcessor.class); + MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); + when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(task.getId()); + when(mapMessageObjectReader.getMandatoryString("type")).thenReturn(TaskStatusChangeType.PROCESS.name()); + taskStatusChangeProcessor.process(mapMessageObjectReader); + } + + + @Test + public void useTaskStatusChangeTypeClose() throws Exception { + Task task = taskService.getById(9); + assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + task.getProcessingStatus()); + TaskStatusChangeProcessor taskStatusChangeProcessor = spy(TaskStatusChangeProcessor.class); + MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); + when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(task.getId()); + when(mapMessageObjectReader.getMandatoryString("type")).thenReturn(TaskStatusChangeType.CLOSE.name()); + taskStatusChangeProcessor.process(mapMessageObjectReader); + assertEquals("Task '" + task.getTitle() + "' status should be DONE!", TaskStatus.DONE, + taskService.getById(task.getId()).getProcessingStatus()); + } + + @Test + public void useTaskStatusChangeTypeErrorOpen() throws Exception { + Task task = taskService.getById(9); + assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + task.getProcessingStatus()); + TaskStatusChangeProcessor taskStatusChangeProcessor = spy(TaskStatusChangeProcessor.class); + MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); + when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(task.getId()); + when(mapMessageObjectReader.getMandatoryString("type")).thenReturn(TaskStatusChangeType.ERROR_OPEN.name()); + taskStatusChangeProcessor.process(mapMessageObjectReader); + assertEquals("Task '" + task.getTitle() + "' status should be DONE!", TaskStatus.DONE, + taskService.getById(task.getId()).getProcessingStatus()); + } + + +} From 5a89fed77cabf7a1f4059184e0a325939a3a6fdb Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Fri, 5 May 2023 17:27:25 +0200 Subject: [PATCH 10/34] Renaming processor and enum, improve tests and processor --- .../data/database/enums/TaskEditType.java | 7 +- .../kitodo/config/enums/ParameterCore.java | 2 +- .../kitodo/production/forms/CommentForm.java | 3 +- .../interfaces/activemq/ActiveMQDirector.java | 2 +- ...kStatusChangeType.java => TaskAction.java} | 23 +++- ...rocessor.java => TaskActionProcessor.java} | 75 ++++++----- .../workflow/WorkflowControllerService.java | 9 +- .../main/resources/kitodo_config.properties | 4 +- ...ssorIT.java => TaskActionProcessorIT.java} | 119 ++++++++++++------ 9 files changed, 165 insertions(+), 79 deletions(-) rename Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/{TaskStatusChangeType.java => TaskAction.java} (55%) rename Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/{TaskStatusChangeProcessor.java => TaskActionProcessor.java} (70%) rename Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/{TaskStatusChangeProcessorIT.java => TaskActionProcessorIT.java} (52%) diff --git a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/enums/TaskEditType.java b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/enums/TaskEditType.java index 867a2d8be32..0451d30e40d 100644 --- a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/enums/TaskEditType.java +++ b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/enums/TaskEditType.java @@ -43,7 +43,12 @@ public enum TaskEditType { /** * Automatic = all kinds of automatic steps. */ - AUTOMATIC(4, "automatic"); + AUTOMATIC(4, "automatic"), + + /** + * Queue = all kinds of changes by ActiveMQ. + */ + QUEUE(5, "queue"); private int value; private String title; diff --git a/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java b/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java index 4283af11cfe..755ac4b7bf7 100644 --- a/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java +++ b/Kitodo/src/main/java/org/kitodo/config/enums/ParameterCore.java @@ -610,7 +610,7 @@ public enum ParameterCore implements ParameterInterface { ACTIVE_MQ_FINALIZE_STEP_QUEUE(new Parameter("activeMQ.finalizeStep.queue")), - ACTIVE_MQ_TASK_STATE_QUEUE(new Parameter("activeMQ.taskState.queue")), + ACTIVE_MQ_TASK_ACTION_QUEUE(new Parameter("activeMQ.taskAction.queue")), ACTIVE_MQ_USER(new Parameter("activeMQ.user")), diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java b/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java index 6d92306dbab..9931c747414 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java @@ -27,6 +27,7 @@ import org.kitodo.data.database.beans.Process; import org.kitodo.data.database.beans.Task; import org.kitodo.data.database.enums.CommentType; +import org.kitodo.data.database.enums.TaskEditType; import org.kitodo.data.database.exceptions.DAOException; import org.kitodo.data.elasticsearch.exceptions.CustomResponseException; import org.kitodo.data.exceptions.DataException; @@ -159,7 +160,7 @@ public String addCommentToAllBatchProcesses() { */ private void reportProblem(Comment comment) { try { - this.workflowControllerService.reportProblem(comment); + this.workflowControllerService.reportProblem(comment, TaskEditType.MANUAL_SINGLE); } catch (DataException e) { Helper.setErrorMessage("reportingProblem", logger, e); } diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java index 0ff353f58e1..73a2a316b61 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/ActiveMQDirector.java @@ -57,7 +57,7 @@ public class ActiveMQDirector implements Runnable, ServletContextListener { private static Collection services; static { - services = Arrays.asList(new FinalizeStepProcessor(), new TaskStatusChangeProcessor()); + services = Arrays.asList(new FinalizeStepProcessor(), new TaskActionProcessor()); } private static Connection connection = null; diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeType.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java similarity index 55% rename from Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeType.java rename to Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java index 5d52f76bc2d..dfade5a408c 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeType.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java @@ -11,10 +11,29 @@ package org.kitodo.production.interfaces.activemq; -public enum TaskStatusChangeType { - INFO, +public enum TaskAction { + /** + * Adds a comment to the task. + */ + COMMENT, + + /** + * Lock a task and add an error comment when task status is OPEN or INWORK. + */ ERROR_OPEN, + + /** + * Set task status of logged task to OPEN. + */ ERROR_CLOSE, + + /** + * Set task status of open task to INWORK. + */ PROCESS, + + /** + * Close a task + */ CLOSE } diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java similarity index 70% rename from Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessor.java rename to Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index da089824c8b..3a09c2e89dd 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -40,7 +40,7 @@ /** * This is a web service interface to modify task states. */ -public class TaskStatusChangeProcessor extends ActiveMQProcessor { +public class TaskActionProcessor extends ActiveMQProcessor { private final TaskService taskService = ServiceManager.getTaskService(); @@ -49,8 +49,8 @@ public class TaskStatusChangeProcessor extends ActiveMQProcessor { * “null” is passed to the super constructor, this will prevent ActiveMQDirector.registerListeners() from starting * this service. */ - public TaskStatusChangeProcessor() { - super(ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_TASK_STATE_QUEUE).orElse(null)); + public TaskActionProcessor() { + super(ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_TASK_ACTION_QUEUE).orElse(null)); } /** @@ -65,21 +65,24 @@ public TaskStatusChangeProcessor() { protected void process(MapMessageObjectReader mapMessageObjectReader) throws ProcessorException, JMSException { CurrentTaskForm currentTaskForm = new CurrentTaskForm(); Integer taskId = mapMessageObjectReader.getMandatoryInteger("id"); - String state = mapMessageObjectReader.getMandatoryString("type"); - TaskStatusChangeType taskStatusChangeType; + String state = mapMessageObjectReader.getMandatoryString("action"); + TaskAction taskAction; try { - taskStatusChangeType = TaskStatusChangeType.valueOf(state); + taskAction = TaskAction.valueOf(state); } catch (IllegalArgumentException e) { - throw new ProcessorException("Unknown task state: " + state); + throw new ProcessorException("Unknown task state " + state); } try { Task currentTask = taskService.getById(taskId); + if (Objects.isNull(currentTask)) { + throw new ProcessorException("Task with id " + taskId + "not found."); + } + currentTaskForm.setCurrentTask(currentTask); Comment comment = new Comment(); comment.setProcess(currentTaskForm.getCurrentTask().getProcess()); comment.setAuthor(ServiceManager.getUserService().getCurrentUser()); - String message = mapMessageObjectReader.getMandatoryString("message"); comment.setMessage(message); comment.setCreationDate(new Date()); @@ -87,16 +90,23 @@ protected void process(MapMessageObjectReader mapMessageObjectReader) throws Pro comment.setCurrentTask(currentTask); User currentUser = ServiceManager.getUserService().getCurrentUser(); - if (TaskStatusChangeType.PROCESS.equals(taskStatusChangeType)) { + if (TaskAction.PROCESS.equals(taskAction)) { if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus())) { throw new ProcessorException("Status of task is not OPEN."); } - processTaskStateProcess(currentTask, currentUser); - } else if (TaskStatusChangeType.ERROR_OPEN.equals(taskStatusChangeType)) { - processTaskStateErrorOpen(mapMessageObjectReader, comment); - } else if (TaskStatusChangeType.ERROR_CLOSE.equals(taskStatusChangeType)) { - processTaskStateErrorClose(mapMessageObjectReader, currentTask, currentUser); - } else if (TaskStatusChangeType.CLOSE.equals(taskStatusChangeType)) { + actionProcess(currentTask, currentUser); + } else if (TaskAction.ERROR_OPEN.equals(taskAction)) { + if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus()) && !TaskStatus.INWORK.equals( + currentTask.getProcessingStatus())) { + throw new ProcessorException("Status of task is not OPEN or INWORK."); + } + actionErrorOpen(mapMessageObjectReader, comment); + } else if (TaskAction.ERROR_CLOSE.equals(taskAction)) { + if (!TaskStatus.LOCKED.equals(currentTask.getProcessingStatus())) { + throw new ProcessorException("Status of task is not LOCKED."); + } + actionErrorClose(mapMessageObjectReader, currentTask, currentUser); + } else if (TaskAction.CLOSE.equals(taskAction)) { currentTaskForm.closeTaskByUser(); } @@ -107,26 +117,28 @@ protected void process(MapMessageObjectReader mapMessageObjectReader) throws Pro // update tasks in elastic search index, which includes correction comment status taskService.saveToIndex(task, true); } - } catch (DataException | DAOException | CustomResponseException | IOException e) { throw new ProcessorException(e); } } - private void processTaskStateErrorOpen(MapMessageObjectReader mapMessageObjectReader, Comment comment) - throws JMSException, DataException, DAOException { + private void actionErrorOpen(MapMessageObjectReader mapMessageObjectReader, Comment comment) + throws ProcessorException, JMSException, DAOException, DataException { if (mapMessageObjectReader.hasField("correctionTaskId")) { - Integer correctionTaskId = Integer.parseInt(mapMessageObjectReader.getString("correctionTaskId")); + Integer correctionTaskId = mapMessageObjectReader.getMandatoryInteger("correctionTaskId"); Task correctionTask = taskService.getById(correctionTaskId); + if (Objects.isNull(correctionTask)) { + throw new ProcessorException("Correction task with id " + correctionTaskId + " not found."); + } comment.setCorrectionTask(correctionTask); } comment.setType(CommentType.ERROR); - new WorkflowControllerService().reportProblem(comment); + new WorkflowControllerService().reportProblem(comment, TaskEditType.QUEUE); } - private void processTaskStateProcess(Task currentTask, User currentUser) throws DataException { + private void actionProcess(Task currentTask, User currentUser) throws DataException { currentTask.setProcessingStatus(TaskStatus.INWORK); - currentTask.setEditType(TaskEditType.AUTOMATIC); + currentTask.setEditType(TaskEditType.QUEUE); currentTask.setProcessingTime(new Date()); taskService.replaceProcessingUser(currentTask, currentUser); if (Objects.isNull(currentTask.getProcessingBegin())) { @@ -135,7 +147,7 @@ private void processTaskStateProcess(Task currentTask, User currentUser) throws } } - private void processTaskStateErrorClose(MapMessageObjectReader mapMessageObjectReader, Task currentTask, + private void actionErrorClose(MapMessageObjectReader mapMessageObjectReader, Task currentTask, User currentUser) throws JMSException, DataException { List comments = ServiceManager.getCommentService().getAllCommentsByCurrentTask(currentTask); Optional optionalComment; @@ -149,18 +161,19 @@ private void processTaskStateErrorClose(MapMessageObjectReader mapMessageObjectR currentTaskComment.getType()) && !currentTaskComment.isCorrected() && Objects.isNull( currentTaskComment.getCorrectionTask())).findFirst(); } - if (optionalComment.isPresent()) { + + if (!optionalComment.isEmpty()) { CommentForm commentForm = new CommentForm(); commentForm.solveProblem(optionalComment.get()); + } - currentTask.setProcessingStatus(TaskStatus.OPEN); - currentTask.setEditType(TaskEditType.AUTOMATIC); - currentTask.setProcessingBegin(null); - currentTask.setProcessingTime(null); + currentTask.setProcessingStatus(TaskStatus.OPEN); + currentTask.setEditType(TaskEditType.QUEUE); + currentTask.setProcessingBegin(null); + currentTask.setProcessingTime(null); - taskService.replaceProcessingUser(currentTask, currentUser); - taskService.save(currentTask); - } + taskService.replaceProcessingUser(currentTask, currentUser); + taskService.save(currentTask); } } diff --git a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java index fc3eae07b4d..4b0de954248 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java @@ -349,28 +349,27 @@ public void unassignTaskFromUser(Task task) throws DataException { * * @param comment as Comment object */ - public void reportProblem(Comment comment) throws DataException { + public void reportProblem(Comment comment, TaskEditType taskEditType) throws DataException { Task currentTask = comment.getCurrentTask(); if (currentTask.isTypeImagesRead() || currentTask.isTypeImagesWrite()) { this.webDav.uploadFromHome(getCurrentUser(), comment.getProcess()); } Date date = new Date(); currentTask.setProcessingStatus(TaskStatus.LOCKED); - currentTask.setEditType(TaskEditType.MANUAL_SINGLE); + currentTask.setEditType(taskEditType); currentTask.setProcessingTime(date); taskService.replaceProcessingUser(currentTask, getCurrentUser()); currentTask.setProcessingBegin(null); taskService.save(currentTask); Task correctionTask = comment.getCorrectionTask(); - if ( Objects.isNull(correctionTask) ) { + if ( Objects.nonNull(correctionTask) ) { correctionTask.setProcessingStatus(TaskStatus.OPEN); correctionTask.setProcessingEnd(null); correctionTask.setCorrection(true); taskService.save(correctionTask); + lockTasksBetweenCurrentAndCorrectionTask(currentTask, correctionTask); } - - lockTasksBetweenCurrentAndCorrectionTask(currentTask, correctionTask); updateProcessSortHelperStatus(currentTask.getProcess()); } diff --git a/Kitodo/src/main/resources/kitodo_config.properties b/Kitodo/src/main/resources/kitodo_config.properties index ee46c54a77a..454e407f192 100644 --- a/Kitodo/src/main/resources/kitodo_config.properties +++ b/Kitodo/src/main/resources/kitodo_config.properties @@ -521,6 +521,8 @@ useLocalDirectory=true ldap_useTLS=false +# You can provide a queue from which messages are read to change steps states +#activeMQ.taskAction.queue=KitodoProduction.TaskState.Queue # ----------------------------------- # Authority control configuration @@ -624,7 +626,7 @@ activeMQ.user=testAdmin #activeMQ.finalizeStep.queue=KitodoProduction.FinalizeStep.Queue # You can provide a queue from which messages are read to change steps states -#activeMQ.taskState.queue=KitodoProduction.TaskState.Queue +#activeMQ.taskAction.queue=KitodoProduction.TaskAction.Queue # ----------------------------------- # Elasticsearch properties diff --git a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessorIT.java b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java similarity index 52% rename from Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessorIT.java rename to Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java index 899fb85e3d8..e7d3f69b925 100644 --- a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskStatusChangeProcessorIT.java +++ b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java @@ -17,28 +17,27 @@ import static org.mockito.Mockito.when; import java.io.File; +import java.util.List; + +import javax.jms.JMSException; import org.apache.commons.lang3.SystemUtils; import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.junit.jupiter.api.MethodOrderer; -import org.junit.jupiter.api.TestMethodOrder; import org.kitodo.ExecutionPermission; import org.kitodo.MockDatabase; import org.kitodo.SecurityTestUtils; import org.kitodo.config.ConfigCore; import org.kitodo.config.enums.ParameterCore; +import org.kitodo.data.database.beans.Comment; import org.kitodo.data.database.beans.Task; import org.kitodo.data.database.enums.TaskStatus; import org.kitodo.exceptions.ProcessorException; import org.kitodo.production.services.ServiceManager; import org.kitodo.production.services.data.TaskService; -import org.kitodo.production.services.file.FileService; -import org.kitodo.production.services.workflow.WorkflowControllerService; -@TestMethodOrder(MethodOrderer.OrderAnnotation.class) -public class TaskStatusChangeProcessorIT { +public class TaskActionProcessorIT { private static final File scriptCreateDirUserHome = new File( ConfigCore.getParameter(ParameterCore.SCRIPT_CREATE_DIR_USER_HOME)); @@ -47,9 +46,7 @@ public class TaskStatusChangeProcessorIT { private static final File scriptNotWorking = new File("src/test/resources/scripts/not_working_script.sh"); private static final File scriptWorking = new File("src/test/resources/scripts/working_script.sh"); private static final File usersDirectory = new File("src/test/resources/users"); - private static final FileService fileService = ServiceManager.getFileService(); private static final TaskService taskService = ServiceManager.getTaskService(); - private static final WorkflowControllerService workflowService = new WorkflowControllerService(); @Before public void prepareDatabase() throws Exception { @@ -83,63 +80,113 @@ public void cleanDatabase() throws Exception { usersDirectory.delete(); } + @Test(expected = ProcessorException.class) + public void testTaskNotFound() throws Exception { + processAction(Integer.MIN_VALUE, TaskAction.COMMENT.name(), ""); + } + + @Test(expected = ProcessorException.class) + public void testUnsupportedAction() throws Exception { + processAction(9, "UNSUPPORTED", ""); + } + @Test - public void useTaskStatusChangeTypeProcess() throws Exception { + public void testActionProcess() throws Exception { Task task = taskService.getById(9); assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, task.getProcessingStatus()); - - TaskStatusChangeProcessor taskStatusChangeProcessor = spy(TaskStatusChangeProcessor.class); - - MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); - when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(task.getId()); - when(mapMessageObjectReader.getMandatoryString("type")).thenReturn(TaskStatusChangeType.PROCESS.name()); - taskStatusChangeProcessor.process(mapMessageObjectReader); + processAction(task, TaskAction.PROCESS); assertEquals("Task '" + task.getTitle() + "' status should be INWORK!", TaskStatus.INWORK, taskService.getById(task.getId()).getProcessingStatus()); } @Test(expected = ProcessorException.class) - public void useTaskStatusChangeTypeProcessForTaskWithoutOpenStatus() throws Exception { + public void testActionProcessWithoutTaskStatusOpen() throws Exception { Task task = taskService.getById(8); assertEquals("Task '" + task.getTitle() + "' status should be INWORK!", TaskStatus.INWORK, task.getProcessingStatus()); - - TaskStatusChangeProcessor taskStatusChangeProcessor = spy(TaskStatusChangeProcessor.class); - MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); - when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(task.getId()); - when(mapMessageObjectReader.getMandatoryString("type")).thenReturn(TaskStatusChangeType.PROCESS.name()); - taskStatusChangeProcessor.process(mapMessageObjectReader); + processAction(task, TaskAction.PROCESS); } @Test - public void useTaskStatusChangeTypeClose() throws Exception { + public void testActionClose() throws Exception { Task task = taskService.getById(9); assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, task.getProcessingStatus()); - TaskStatusChangeProcessor taskStatusChangeProcessor = spy(TaskStatusChangeProcessor.class); - MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); - when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(task.getId()); - when(mapMessageObjectReader.getMandatoryString("type")).thenReturn(TaskStatusChangeType.CLOSE.name()); - taskStatusChangeProcessor.process(mapMessageObjectReader); + processAction(task, TaskAction.CLOSE); assertEquals("Task '" + task.getTitle() + "' status should be DONE!", TaskStatus.DONE, taskService.getById(task.getId()).getProcessingStatus()); } @Test - public void useTaskStatusChangeTypeErrorOpen() throws Exception { + public void testActionErrorOpen() throws Exception { + Task openTask = taskService.getById(9); + assertEquals("Task '" + openTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + openTask.getProcessingStatus()); + processAction(openTask, TaskAction.ERROR_OPEN); + assertEquals("Task '" + openTask.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + taskService.getById(openTask.getId()).getProcessingStatus()); + + Task inWorkTask = taskService.getById(8); + assertEquals("Task '" + inWorkTask.getTitle() + "' status should be INWORK!", TaskStatus.INWORK, + inWorkTask.getProcessingStatus()); + processAction(inWorkTask, TaskAction.ERROR_OPEN); + assertEquals("Task '" + inWorkTask.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + taskService.getById(inWorkTask.getId()).getProcessingStatus()); + } + + @Test(expected = ProcessorException.class) + public void testActionErrorOpenWithoutTaskStatusOpenOrInWork() throws Exception { + Task task = taskService.getById(10); + assertEquals("Task '" + task.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + task.getProcessingStatus()); + processAction(task, TaskAction.ERROR_OPEN); + } + + @Test + public void testActionErrorClose() throws Exception { + Task task = taskService.getById(10); + assertEquals("Task '" + task.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + task.getProcessingStatus()); + processAction(task, TaskAction.ERROR_CLOSE); + assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + taskService.getById(task.getId()).getProcessingStatus()); + } + + @Test(expected = ProcessorException.class) + public void testActionErrorCloseWithoutTaskStatusLocked() throws Exception { Task task = taskService.getById(9); assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, task.getProcessingStatus()); - TaskStatusChangeProcessor taskStatusChangeProcessor = spy(TaskStatusChangeProcessor.class); - MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); - when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(task.getId()); - when(mapMessageObjectReader.getMandatoryString("type")).thenReturn(TaskStatusChangeType.ERROR_OPEN.name()); - taskStatusChangeProcessor.process(mapMessageObjectReader); - assertEquals("Task '" + task.getTitle() + "' status should be DONE!", TaskStatus.DONE, + processAction(task, TaskAction.ERROR_CLOSE); + } + + @Test + public void testActionComment() throws Exception { + Task task = taskService.getById(10); + assertEquals("Task '" + task.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + task.getProcessingStatus()); + processAction(task, TaskAction.ERROR_CLOSE); + assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, taskService.getById(task.getId()).getProcessingStatus()); } + private static void processAction(Task task, TaskAction taskAction) throws JMSException, ProcessorException { + String message = "Process action " + taskAction.name(); + processAction(task.getId(), taskAction.name(), message); + List comments = ServiceManager.getCommentService().getAllCommentsByCurrentTask(task); + assertEquals("Comment should be created!", 1, comments.size()); + assertEquals("Comment message should be '" + message + "'!", message, comments.get(0).getMessage()); + } + + private static void processAction(Integer taskId, String action, String message) throws JMSException, ProcessorException { + TaskActionProcessor taskActionProcessor = spy(TaskActionProcessor.class); + MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); + when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(taskId); + when(mapMessageObjectReader.getMandatoryString("action")).thenReturn(action); + when(mapMessageObjectReader.getMandatoryString("message")).thenReturn(message); + taskActionProcessor.process(mapMessageObjectReader); + } } From b566b5f582a69b0132c98e48de59c0125b566c65 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 8 May 2023 16:48:53 +0200 Subject: [PATCH 11/34] Improvements comments and add futher tests --- .../interfaces/activemq/TaskAction.java | 2 +- .../activemq/TaskActionProcessor.java | 134 ++++++++++-------- .../production/services/data/TaskService.java | 1 - .../activemq/TaskActionProcessorIT.java | 12 +- .../workflow/WorkflowControllerServiceIT.java | 5 +- 5 files changed, 90 insertions(+), 64 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java index dfade5a408c..1fcf2e55d54 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java @@ -33,7 +33,7 @@ public enum TaskAction { PROCESS, /** - * Close a task + * Close a task. */ CLOSE } diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index 3a09c2e89dd..eae15815dca 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -31,8 +31,6 @@ import org.kitodo.data.elasticsearch.exceptions.CustomResponseException; import org.kitodo.data.exceptions.DataException; import org.kitodo.exceptions.ProcessorException; -import org.kitodo.production.forms.CommentForm; -import org.kitodo.production.forms.CurrentTaskForm; import org.kitodo.production.services.ServiceManager; import org.kitodo.production.services.data.TaskService; import org.kitodo.production.services.workflow.WorkflowControllerService; @@ -42,7 +40,12 @@ */ public class TaskActionProcessor extends ActiveMQProcessor { + public static final String KEY_CORRECTION_TASK_ID = "correctionTaskId"; + public static final String KEY_MESSAGE = "message"; + public static final String KEY_TASK_ACTION = "action"; + public static final String KEY_TASK_ID = "id"; private final TaskService taskService = ServiceManager.getTaskService(); + private WorkflowControllerService workflowControllerService; /** * The default constructor looks up the queue name to use in kitodo_config.properties. If that is not configured and @@ -51,6 +54,7 @@ public class TaskActionProcessor extends ActiveMQProcessor { */ public TaskActionProcessor() { super(ConfigCore.getOptionalString(ParameterCore.ACTIVE_MQ_TASK_ACTION_QUEUE).orElse(null)); + workflowControllerService = new WorkflowControllerService(); } /** @@ -63,9 +67,8 @@ public TaskActionProcessor() { */ @Override protected void process(MapMessageObjectReader mapMessageObjectReader) throws ProcessorException, JMSException { - CurrentTaskForm currentTaskForm = new CurrentTaskForm(); - Integer taskId = mapMessageObjectReader.getMandatoryInteger("id"); - String state = mapMessageObjectReader.getMandatoryString("action"); + Integer taskId = mapMessageObjectReader.getMandatoryInteger(KEY_TASK_ID); + String state = mapMessageObjectReader.getMandatoryString(KEY_TASK_ACTION); TaskAction taskAction; try { taskAction = TaskAction.valueOf(state); @@ -79,38 +82,7 @@ protected void process(MapMessageObjectReader mapMessageObjectReader) throws Pro throw new ProcessorException("Task with id " + taskId + "not found."); } - currentTaskForm.setCurrentTask(currentTask); - Comment comment = new Comment(); - comment.setProcess(currentTaskForm.getCurrentTask().getProcess()); - comment.setAuthor(ServiceManager.getUserService().getCurrentUser()); - String message = mapMessageObjectReader.getMandatoryString("message"); - comment.setMessage(message); - comment.setCreationDate(new Date()); - comment.setType(CommentType.INFO); - comment.setCurrentTask(currentTask); - - User currentUser = ServiceManager.getUserService().getCurrentUser(); - if (TaskAction.PROCESS.equals(taskAction)) { - if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus())) { - throw new ProcessorException("Status of task is not OPEN."); - } - actionProcess(currentTask, currentUser); - } else if (TaskAction.ERROR_OPEN.equals(taskAction)) { - if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus()) && !TaskStatus.INWORK.equals( - currentTask.getProcessingStatus())) { - throw new ProcessorException("Status of task is not OPEN or INWORK."); - } - actionErrorOpen(mapMessageObjectReader, comment); - } else if (TaskAction.ERROR_CLOSE.equals(taskAction)) { - if (!TaskStatus.LOCKED.equals(currentTask.getProcessingStatus())) { - throw new ProcessorException("Status of task is not LOCKED."); - } - actionErrorClose(mapMessageObjectReader, currentTask, currentUser); - } else if (TaskAction.CLOSE.equals(taskAction)) { - currentTaskForm.closeTaskByUser(); - } - - ServiceManager.getCommentService().saveToDatabase(comment); + processAction(mapMessageObjectReader, taskAction, currentTask); ServiceManager.getProcessService().saveToIndex(currentTask.getProcess(), true); for (Task task : currentTask.getProcess().getTasks()) { @@ -122,10 +94,57 @@ protected void process(MapMessageObjectReader mapMessageObjectReader) throws Pro } } + private void processAction(MapMessageObjectReader mapMessageObjectReader, TaskAction taskAction, Task currentTask) + throws JMSException, ProcessorException, DataException, DAOException, IOException { + Comment comment = null; + if (mapMessageObjectReader.hasField(KEY_MESSAGE)) { + comment = buildComment(currentTask, mapMessageObjectReader.getMandatoryString(KEY_MESSAGE)); + } + + User currentUser = ServiceManager.getUserService().getCurrentUser(); + if (TaskAction.PROCESS.equals(taskAction)) { + if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus())) { + throw new ProcessorException("Status of task is not OPEN."); + } + actionProcess(currentTask, currentUser); + } else if (TaskAction.ERROR_OPEN.equals(taskAction)) { + if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus()) && !TaskStatus.INWORK.equals( + currentTask.getProcessingStatus())) { + throw new ProcessorException("Status of task is not OPEN or INWORK."); + } + if (!mapMessageObjectReader.hasField(KEY_MESSAGE)) { + throw new ProcessorException("Message field of task action ERROR_OPEN is required."); + } + actionErrorOpen(mapMessageObjectReader, comment); + } else if (TaskAction.ERROR_CLOSE.equals(taskAction)) { + if (!TaskStatus.LOCKED.equals(currentTask.getProcessingStatus())) { + throw new ProcessorException("Status of task is not LOCKED."); + } + actionErrorClose(mapMessageObjectReader, currentTask, currentUser); + } else if (TaskAction.CLOSE.equals(taskAction)) { + workflowControllerService.closeTaskByUser(currentTask); + } + + if (Objects.nonNull(comment)) { + ServiceManager.getCommentService().saveToDatabase(comment); + } + } + + private static Comment buildComment(Task currentTask, String message) { + Comment comment = new Comment(); + comment.setProcess(currentTask.getProcess()); + comment.setAuthor(ServiceManager.getUserService().getCurrentUser()); + comment.setMessage(message); + comment.setCreationDate(new Date()); + comment.setType(CommentType.INFO); + comment.setCurrentTask(currentTask); + return comment; + } + private void actionErrorOpen(MapMessageObjectReader mapMessageObjectReader, Comment comment) throws ProcessorException, JMSException, DAOException, DataException { - if (mapMessageObjectReader.hasField("correctionTaskId")) { - Integer correctionTaskId = mapMessageObjectReader.getMandatoryInteger("correctionTaskId"); + if (mapMessageObjectReader.hasField(KEY_CORRECTION_TASK_ID)) { + Integer correctionTaskId = mapMessageObjectReader.getMandatoryInteger(KEY_CORRECTION_TASK_ID); Task correctionTask = taskService.getById(correctionTaskId); if (Objects.isNull(correctionTask)) { throw new ProcessorException("Correction task with id " + correctionTaskId + " not found."); @@ -133,7 +152,7 @@ private void actionErrorOpen(MapMessageObjectReader mapMessageObjectReader, Comm comment.setCorrectionTask(correctionTask); } comment.setType(CommentType.ERROR); - new WorkflowControllerService().reportProblem(comment, TaskEditType.QUEUE); + workflowControllerService.reportProblem(comment, TaskEditType.QUEUE); } private void actionProcess(Task currentTask, User currentUser) throws DataException { @@ -147,26 +166,11 @@ private void actionProcess(Task currentTask, User currentUser) throws DataExcept } } - private void actionErrorClose(MapMessageObjectReader mapMessageObjectReader, Task currentTask, - User currentUser) throws JMSException, DataException { - List comments = ServiceManager.getCommentService().getAllCommentsByCurrentTask(currentTask); - Optional optionalComment; - if (mapMessageObjectReader.hasField("correctionTaskId")) { - Integer correctionTaskId = Integer.parseInt(mapMessageObjectReader.getString("correctionTaskId")); - optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( - currentTaskComment.getType()) && !currentTaskComment.isCorrected() && correctionTaskId.equals( - currentTaskComment.getCorrectionTask().getId())).findFirst(); - } else { - optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( - currentTaskComment.getType()) && !currentTaskComment.isCorrected() && Objects.isNull( - currentTaskComment.getCorrectionTask())).findFirst(); - } - - if (!optionalComment.isEmpty()) { - CommentForm commentForm = new CommentForm(); - commentForm.solveProblem(optionalComment.get()); + private void actionErrorClose(MapMessageObjectReader mapMessageObjectReader, Task currentTask, User currentUser) + throws JMSException, DataException, DAOException, IOException { + if (mapMessageObjectReader.hasField(KEY_CORRECTION_TASK_ID)) { + closeCorrectionTask(currentTask, mapMessageObjectReader.getMandatoryInteger(KEY_CORRECTION_TASK_ID)); } - currentTask.setProcessingStatus(TaskStatus.OPEN); currentTask.setEditType(TaskEditType.QUEUE); currentTask.setProcessingBegin(null); @@ -176,4 +180,16 @@ private void actionErrorClose(MapMessageObjectReader mapMessageObjectReader, Tas taskService.save(currentTask); } + private void closeCorrectionTask(Task currentTask, Integer correctionTaskId) + throws DAOException, DataException, IOException { + List comments = ServiceManager.getCommentService().getAllCommentsByCurrentTask(currentTask); + Optional optionalComment; + optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( + currentTaskComment.getType()) && !currentTaskComment.isCorrected() && correctionTaskId.equals( + currentTaskComment.getCorrectionTask().getId())).findFirst(); + if (!optionalComment.isEmpty()) { + workflowControllerService.solveProblem(optionalComment.get()); + } + } + } diff --git a/Kitodo/src/main/java/org/kitodo/production/services/data/TaskService.java b/Kitodo/src/main/java/org/kitodo/production/services/data/TaskService.java index 6daec1aa77e..a600459fce6 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/data/TaskService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/data/TaskService.java @@ -572,7 +572,6 @@ private void finishOrReturnAutomaticTask(Task task, boolean automatic, boolean s new WorkflowControllerService().close(task); } else { task.setProcessingStatus(TaskStatus.OPEN); - save(task); } } } diff --git a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java index e7d3f69b925..7ea4a570abc 100644 --- a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java +++ b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java @@ -21,6 +21,7 @@ import javax.jms.JMSException; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.SystemUtils; import org.junit.After; import org.junit.Before; @@ -144,6 +145,12 @@ public void testActionErrorOpenWithoutTaskStatusOpenOrInWork() throws Exception processAction(task, TaskAction.ERROR_OPEN); } + @Test(expected = ProcessorException.class) + public void testActionErrorOpenWithoutMessage() throws Exception { + Task task = taskService.getById(10); + processAction(task.getId(), TaskAction.ERROR_OPEN.name(), StringUtils.EMPTY); + } + @Test public void testActionErrorClose() throws Exception { Task task = taskService.getById(10); @@ -186,7 +193,10 @@ private static void processAction(Integer taskId, String action, String message) MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(taskId); when(mapMessageObjectReader.getMandatoryString("action")).thenReturn(action); - when(mapMessageObjectReader.getMandatoryString("message")).thenReturn(message); + if(StringUtils.isNotEmpty(message)) { + when(mapMessageObjectReader.hasField("message")).thenReturn(Boolean.TRUE); + when(mapMessageObjectReader.getMandatoryString("message")).thenReturn(message); + } taskActionProcessor.process(mapMessageObjectReader); } } diff --git a/Kitodo/src/test/java/org/kitodo/production/services/workflow/WorkflowControllerServiceIT.java b/Kitodo/src/test/java/org/kitodo/production/services/workflow/WorkflowControllerServiceIT.java index e1c21fe0f38..87fe5a17b61 100644 --- a/Kitodo/src/test/java/org/kitodo/production/services/workflow/WorkflowControllerServiceIT.java +++ b/Kitodo/src/test/java/org/kitodo/production/services/workflow/WorkflowControllerServiceIT.java @@ -38,6 +38,7 @@ import org.kitodo.data.database.beans.Task; import org.kitodo.data.database.beans.WorkflowCondition; import org.kitodo.data.database.enums.CommentType; +import org.kitodo.data.database.enums.TaskEditType; import org.kitodo.data.database.enums.TaskStatus; import org.kitodo.data.database.exceptions.DAOException; import org.kitodo.data.exceptions.DataException; @@ -423,7 +424,7 @@ public void shouldReportProblem() throws Exception { ServiceManager.getCommentService().saveToDatabase(problem); - workflowService.reportProblem(problem); + workflowService.reportProblem(problem, TaskEditType.MANUAL_SINGLE); assertEquals( "Report of problem was incorrect - task '" + correctionTask.getTitle() + "' is not set up to open!", @@ -459,7 +460,7 @@ public void shouldSolveProblem() throws Exception { ServiceManager.getCommentService().saveToDatabase(correctionComment); - workflowService.reportProblem(correctionComment); + workflowService.reportProblem(correctionComment, TaskEditType.MANUAL_SINGLE); workflowService.solveProblem(correctionComment); Process process = ServiceManager.getProcessService().getById(currentTask.getProcess().getId()); From 5fc60bcf59f1803e2df6e8bf994addafb0726a7a Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 8 May 2023 16:56:51 +0200 Subject: [PATCH 12/34] Rename comment dao function --- .../kitodo/data/database/persistence/CommentDAO.java | 10 ++++++++-- .../interfaces/activemq/TaskActionProcessor.java | 2 +- .../production/services/data/CommentService.java | 4 ++-- .../interfaces/activemq/TaskActionProcessorIT.java | 2 +- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java index d4157865b93..3e8647eeb0d 100644 --- a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java +++ b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java @@ -55,12 +55,18 @@ public List getAllByProcess(Process process) { Collections.singletonMap("processId", process.getId())); } - public List getAllByCurrentTask(Task task) { + /** + * Get all comments by task ordered by id ascending. + * + * @param task + * The current task to get the comments for + * @return List of comments + */ + public List getAllByTask(Task task) { return getByQuery("FROM Comment WHERE currentTask_id = :taskId ORDER BY id ASC", Collections.singletonMap("taskId", task.getId())); } - /** * Save list of comments. * diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index eae15815dca..229481aa022 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -182,7 +182,7 @@ private void actionErrorClose(MapMessageObjectReader mapMessageObjectReader, Tas private void closeCorrectionTask(Task currentTask, Integer correctionTaskId) throws DAOException, DataException, IOException { - List comments = ServiceManager.getCommentService().getAllCommentsByCurrentTask(currentTask); + List comments = ServiceManager.getCommentService().getAllCommentsByTask(currentTask); Optional optionalComment; optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( currentTaskComment.getType()) && !currentTaskComment.isCorrected() && correctionTaskId.equals( diff --git a/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java b/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java index 46277bb63c8..2c70c637120 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java @@ -72,8 +72,8 @@ public List getAllCommentsByProcess(Process process) { return dao.getAllByProcess(process); } - public List getAllCommentsByCurrentTask(Task task) { - return dao.getAllByCurrentTask(task); + public List getAllCommentsByTask(Task task) { + return dao.getAllByTask(task); } /** diff --git a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java index 7ea4a570abc..eea7e575231 100644 --- a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java +++ b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java @@ -183,7 +183,7 @@ private static void processAction(Task task, TaskAction taskAction) throws JMSEx String message = "Process action " + taskAction.name(); processAction(task.getId(), taskAction.name(), message); - List comments = ServiceManager.getCommentService().getAllCommentsByCurrentTask(task); + List comments = ServiceManager.getCommentService().getAllCommentsByTask(task); assertEquals("Comment should be created!", 1, comments.size()); assertEquals("Comment message should be '" + message + "'!", message, comments.get(0).getMessage()); } From f85e32cb1eeb1de015f87c60f5b4856487a11a5e Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 8 May 2023 17:11:12 +0200 Subject: [PATCH 13/34] Improve comment and fix bug --- .../kitodo/production/services/data/CommentService.java | 7 +++++++ .../org/kitodo/production/services/data/TaskService.java | 1 + 2 files changed, 8 insertions(+) diff --git a/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java b/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java index 2c70c637120..63b1470591b 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/data/CommentService.java @@ -72,6 +72,13 @@ public List getAllCommentsByProcess(Process process) { return dao.getAllByProcess(process); } + /** + * Get all comments by task ordered by id ascending. + * + * @param task + * The current task to get the comments for + * @return List of comments + */ public List getAllCommentsByTask(Task task) { return dao.getAllByTask(task); } diff --git a/Kitodo/src/main/java/org/kitodo/production/services/data/TaskService.java b/Kitodo/src/main/java/org/kitodo/production/services/data/TaskService.java index a600459fce6..6daec1aa77e 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/data/TaskService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/data/TaskService.java @@ -572,6 +572,7 @@ private void finishOrReturnAutomaticTask(Task task, boolean automatic, boolean s new WorkflowControllerService().close(task); } else { task.setProcessingStatus(TaskStatus.OPEN); + save(task); } } } From ae85dfb207182d05d8c93f898308ebcd72af090f Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 8 May 2023 17:15:37 +0200 Subject: [PATCH 14/34] Remove unnecessary comment from kitodo_config.properties --- Kitodo/src/main/resources/kitodo_config.properties | 2 -- 1 file changed, 2 deletions(-) diff --git a/Kitodo/src/main/resources/kitodo_config.properties b/Kitodo/src/main/resources/kitodo_config.properties index 454e407f192..f3e8303147b 100644 --- a/Kitodo/src/main/resources/kitodo_config.properties +++ b/Kitodo/src/main/resources/kitodo_config.properties @@ -601,8 +601,6 @@ taskProcessPropertyColumns= # Active MQ services need an existing user as whom to act in the system activeMQ.user=testAdmin -# You can provide a topic that Kitodo reports results and status messages to -#activeMQ.results.topic=KitodoProduction.ResultMessages.Topic # By default, Kitodo instructs the server to keep status messages for a # equivalent of 7 days. You can change this value (in milliseconds) to meet From 69d4f1aa5238c1d87483c61c1d5da3d329960859 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 8 May 2023 17:50:12 +0200 Subject: [PATCH 15/34] Improve comment --- .../production/interfaces/activemq/TaskActionProcessor.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index 229481aa022..e4a26610541 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -58,9 +58,8 @@ public TaskActionProcessor() { } /** - * This is the main routine processing incoming tickets. It gets an CurrentTaskForm object, sets it to the - * appropriate step which is retrieved from the database, appends the message − if any − to the wiki field, and - * executes the form’s the step close function. + * This is the main routine processing incoming tickets. It gets the task id and the task action for processing. + * Every action has its own behavior so please read the comment on the action for more information. * * @param mapMessageObjectReader * the incoming message From b288bbe19c7d0347424e0fd38ee0e3f0356d695d Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 10 May 2023 11:07:36 +0200 Subject: [PATCH 16/34] Use switch case, rename comment --- .../interfaces/activemq/TaskAction.java | 2 +- .../activemq/TaskActionProcessor.java | 46 ++++++++++--------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java index 1fcf2e55d54..54583e28e78 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java @@ -23,7 +23,7 @@ public enum TaskAction { ERROR_OPEN, /** - * Set task status of logged task to OPEN. + * Set task status of locked task to OPEN. */ ERROR_CLOSE, diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index e4a26610541..d4b210ef3d9 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -101,27 +101,31 @@ private void processAction(MapMessageObjectReader mapMessageObjectReader, TaskAc } User currentUser = ServiceManager.getUserService().getCurrentUser(); - if (TaskAction.PROCESS.equals(taskAction)) { - if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus())) { - throw new ProcessorException("Status of task is not OPEN."); - } - actionProcess(currentTask, currentUser); - } else if (TaskAction.ERROR_OPEN.equals(taskAction)) { - if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus()) && !TaskStatus.INWORK.equals( - currentTask.getProcessingStatus())) { - throw new ProcessorException("Status of task is not OPEN or INWORK."); - } - if (!mapMessageObjectReader.hasField(KEY_MESSAGE)) { - throw new ProcessorException("Message field of task action ERROR_OPEN is required."); - } - actionErrorOpen(mapMessageObjectReader, comment); - } else if (TaskAction.ERROR_CLOSE.equals(taskAction)) { - if (!TaskStatus.LOCKED.equals(currentTask.getProcessingStatus())) { - throw new ProcessorException("Status of task is not LOCKED."); - } - actionErrorClose(mapMessageObjectReader, currentTask, currentUser); - } else if (TaskAction.CLOSE.equals(taskAction)) { - workflowControllerService.closeTaskByUser(currentTask); + switch (taskAction) { + case PROCESS: + if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus())) { + throw new ProcessorException("Status of task is not OPEN."); + } + actionProcess(currentTask, currentUser); + break; + case ERROR_OPEN: + if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus()) && !TaskStatus.INWORK.equals( + currentTask.getProcessingStatus())) { + throw new ProcessorException("Status of task is not OPEN or INWORK."); + } + if (!mapMessageObjectReader.hasField(KEY_MESSAGE)) { + throw new ProcessorException("Message field of task action ERROR_OPEN is required."); + } + actionErrorOpen(mapMessageObjectReader, comment); + break; + case ERROR_CLOSE: + if (!TaskStatus.LOCKED.equals(currentTask.getProcessingStatus())) { + throw new ProcessorException("Status of task is not LOCKED."); + } + actionErrorClose(mapMessageObjectReader, currentTask, currentUser); + break; + case CLOSE: + workflowControllerService.closeTaskByUser(currentTask); } if (Objects.nonNull(comment)) { From b81be7896b070cbc8cb1e8cd75679a0fee83c78b Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 10 May 2023 11:21:28 +0200 Subject: [PATCH 17/34] Add default to switch case --- .../production/interfaces/activemq/TaskActionProcessor.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index d4b210ef3d9..7768ca73614 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -126,6 +126,11 @@ private void processAction(MapMessageObjectReader mapMessageObjectReader, TaskAc break; case CLOSE: workflowControllerService.closeTaskByUser(currentTask); + break; + default: + if (!mapMessageObjectReader.hasField(KEY_MESSAGE)) { + throw new ProcessorException("Message field of task action COMMENT is required."); + } } if (Objects.nonNull(comment)) { From 44ec676ee0a43cf5cb6bc9fcef5243674053ea05 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 10 May 2023 16:59:51 +0200 Subject: [PATCH 18/34] Improvments of error open and close behaviour --- .../kitodo/data/database/beans/Comment.java | 13 +++- .../kitodo/production/forms/CommentForm.java | 4 +- .../activemq/TaskActionProcessor.java | 31 ++++++--- .../workflow/WorkflowControllerService.java | 19 ++++-- .../main/resources/kitodo_config.properties | 3 - .../activemq/TaskActionProcessorIT.java | 67 ++++++++----------- .../workflow/WorkflowControllerServiceIT.java | 2 +- 7 files changed, 78 insertions(+), 61 deletions(-) diff --git a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java index 6c55dbb93f1..067c0e32b2c 100644 --- a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java +++ b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java @@ -12,6 +12,7 @@ package org.kitodo.data.database.beans; import java.util.Date; +import java.util.Objects; import javax.persistence.Column; import javax.persistence.Entity; @@ -234,12 +235,22 @@ public Task getCorrectionTask() { /** * Set correctionTask. * - * @param correctionTask as org.kitodo.data.database.beans.Task + * @param correctionTask + * as org.kitodo.data.database.beans.Task */ public void setCorrectionTask(Task correctionTask) { this.correctionTask = correctionTask; } + /** + * Check if the comment has a correction task which differs from the current task. + * + * @return true if the comment has a separate correction task + */ + public boolean hasSeparateCorrectionTask() { + return Objects.nonNull(correctionTask) && !correctionTask.equals(currentTask); + } + /** * Get process. * diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java b/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java index 9931c747414..b6c6ec754fa 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java @@ -226,11 +226,11 @@ public int getSizeOfPreviousStepsForProblemReporting() { */ public String solveProblem(Comment comment) { try { - this.workflowControllerService.solveProblem(comment); + this.workflowControllerService.solveProblem(comment, TaskEditType.MANUAL_SINGLE); } catch (DataException | DAOException | IOException e) { Helper.setErrorMessage("SolveProblem", logger, e); } - refreshProcess(this.currentTask.getProcess()); + refreshProcess(comment.getCurrentTask().getProcess()); return MessageFormat.format(REDIRECT_PATH, "tasks"); } diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index 7768ca73614..e0a67932891 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -67,12 +67,12 @@ public TaskActionProcessor() { @Override protected void process(MapMessageObjectReader mapMessageObjectReader) throws ProcessorException, JMSException { Integer taskId = mapMessageObjectReader.getMandatoryInteger(KEY_TASK_ID); - String state = mapMessageObjectReader.getMandatoryString(KEY_TASK_ACTION); + String taskActionField = mapMessageObjectReader.getMandatoryString(KEY_TASK_ACTION); TaskAction taskAction; try { - taskAction = TaskAction.valueOf(state); + taskAction = TaskAction.valueOf(mapMessageObjectReader.getMandatoryString(KEY_TASK_ACTION)); } catch (IllegalArgumentException e) { - throw new ProcessorException("Unknown task state " + state); + throw new ProcessorException("Unknown task action " + taskActionField); } try { @@ -116,7 +116,7 @@ private void processAction(MapMessageObjectReader mapMessageObjectReader, TaskAc if (!mapMessageObjectReader.hasField(KEY_MESSAGE)) { throw new ProcessorException("Message field of task action ERROR_OPEN is required."); } - actionErrorOpen(mapMessageObjectReader, comment); + actionErrorOpen(mapMessageObjectReader, currentTask, comment); break; case ERROR_CLOSE: if (!TaskStatus.LOCKED.equals(currentTask.getProcessingStatus())) { @@ -149,7 +149,7 @@ private static Comment buildComment(Task currentTask, String message) { return comment; } - private void actionErrorOpen(MapMessageObjectReader mapMessageObjectReader, Comment comment) + private void actionErrorOpen(MapMessageObjectReader mapMessageObjectReader, Task currentTask, Comment comment) throws ProcessorException, JMSException, DAOException, DataException { if (mapMessageObjectReader.hasField(KEY_CORRECTION_TASK_ID)) { Integer correctionTaskId = mapMessageObjectReader.getMandatoryInteger(KEY_CORRECTION_TASK_ID); @@ -158,6 +158,8 @@ private void actionErrorOpen(MapMessageObjectReader mapMessageObjectReader, Comm throw new ProcessorException("Correction task with id " + correctionTaskId + " not found."); } comment.setCorrectionTask(correctionTask); + } else { + comment.setCorrectionTask(currentTask); } comment.setType(CommentType.ERROR); workflowControllerService.reportProblem(comment, TaskEditType.QUEUE); @@ -176,19 +178,26 @@ private void actionProcess(Task currentTask, User currentUser) throws DataExcept private void actionErrorClose(MapMessageObjectReader mapMessageObjectReader, Task currentTask, User currentUser) throws JMSException, DataException, DAOException, IOException { - if (mapMessageObjectReader.hasField(KEY_CORRECTION_TASK_ID)) { - closeCorrectionTask(currentTask, mapMessageObjectReader.getMandatoryInteger(KEY_CORRECTION_TASK_ID)); - } currentTask.setProcessingStatus(TaskStatus.OPEN); currentTask.setEditType(TaskEditType.QUEUE); currentTask.setProcessingBegin(null); currentTask.setProcessingTime(null); - taskService.replaceProcessingUser(currentTask, currentUser); taskService.save(currentTask); + + if (mapMessageObjectReader.hasField(KEY_CORRECTION_TASK_ID)) { + markErrorCommentAsCorrected(currentTask, + mapMessageObjectReader.getMandatoryInteger(KEY_CORRECTION_TASK_ID)); + } else { + markErrorCommentAsCorrected(currentTask); + } + } + + private void markErrorCommentAsCorrected(Task currentTask) throws DAOException, DataException, IOException { + markErrorCommentAsCorrected(currentTask, currentTask.getId()); } - private void closeCorrectionTask(Task currentTask, Integer correctionTaskId) + private void markErrorCommentAsCorrected(Task currentTask, Integer correctionTaskId) throws DAOException, DataException, IOException { List comments = ServiceManager.getCommentService().getAllCommentsByTask(currentTask); Optional optionalComment; @@ -196,7 +205,7 @@ private void closeCorrectionTask(Task currentTask, Integer correctionTaskId) currentTaskComment.getType()) && !currentTaskComment.isCorrected() && correctionTaskId.equals( currentTaskComment.getCorrectionTask().getId())).findFirst(); if (!optionalComment.isEmpty()) { - workflowControllerService.solveProblem(optionalComment.get()); + workflowControllerService.solveProblem(optionalComment.get(), TaskEditType.QUEUE); } } diff --git a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java index 4b0de954248..9b19127f41e 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java @@ -362,8 +362,8 @@ public void reportProblem(Comment comment, TaskEditType taskEditType) throws Dat currentTask.setProcessingBegin(null); taskService.save(currentTask); - Task correctionTask = comment.getCorrectionTask(); - if ( Objects.nonNull(correctionTask) ) { + if (comment.hasSeparateCorrectionTask()) { + Task correctionTask = comment.getCorrectionTask(); correctionTask.setProcessingStatus(TaskStatus.OPEN); correctionTask.setProcessingEnd(null); correctionTask.setCorrection(true); @@ -377,12 +377,21 @@ public void reportProblem(Comment comment, TaskEditType taskEditType) throws Dat * Unified method for solve problem. * * @param comment - * as Comment object + * as Comment object */ - public void solveProblem(Comment comment) throws DataException, DAOException, IOException { - if (Objects.nonNull(comment.getCorrectionTask())) { + public void solveProblem(Comment comment, TaskEditType taskEditType) + throws DataException, DAOException, IOException { + if (comment.hasSeparateCorrectionTask()) { closeTaskByUser(comment.getCorrectionTask()); comment.setCorrectionTask(ServiceManager.getTaskService().getById(comment.getCorrectionTask().getId())); + } else { + Task currentTask = comment.getCurrentTask(); + currentTask.setProcessingStatus(TaskStatus.OPEN); + currentTask.setEditType(taskEditType); + currentTask.setProcessingTime(new Date()); + currentTask.setProcessingBegin(null); + taskService.replaceProcessingUser(currentTask, getCurrentUser()); + taskService.save(currentTask); } comment.setCurrentTask(ServiceManager.getTaskService().getById(comment.getCurrentTask().getId())); comment.setCorrected(Boolean.TRUE); diff --git a/Kitodo/src/main/resources/kitodo_config.properties b/Kitodo/src/main/resources/kitodo_config.properties index f3e8303147b..6f203486202 100644 --- a/Kitodo/src/main/resources/kitodo_config.properties +++ b/Kitodo/src/main/resources/kitodo_config.properties @@ -521,9 +521,6 @@ useLocalDirectory=true ldap_useTLS=false -# You can provide a queue from which messages are read to change steps states -#activeMQ.taskAction.queue=KitodoProduction.TaskState.Queue - # ----------------------------------- # Authority control configuration # ----------------------------------- diff --git a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java index eea7e575231..616658a74eb 100644 --- a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java +++ b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java @@ -16,21 +16,16 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; -import java.io.File; import java.util.List; import javax.jms.JMSException; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.SystemUtils; import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.kitodo.ExecutionPermission; import org.kitodo.MockDatabase; import org.kitodo.SecurityTestUtils; -import org.kitodo.config.ConfigCore; -import org.kitodo.config.enums.ParameterCore; import org.kitodo.data.database.beans.Comment; import org.kitodo.data.database.beans.Task; import org.kitodo.data.database.enums.TaskStatus; @@ -40,45 +35,32 @@ public class TaskActionProcessorIT { - private static final File scriptCreateDirUserHome = new File( - ConfigCore.getParameter(ParameterCore.SCRIPT_CREATE_DIR_USER_HOME)); - private static final File scriptCreateSymLink = new File( - ConfigCore.getParameter(ParameterCore.SCRIPT_CREATE_SYMLINK)); - private static final File scriptNotWorking = new File("src/test/resources/scripts/not_working_script.sh"); - private static final File scriptWorking = new File("src/test/resources/scripts/working_script.sh"); - private static final File usersDirectory = new File("src/test/resources/users"); private static final TaskService taskService = ServiceManager.getTaskService(); + /** + * Prepare the data for every test. + * + * @throws Exception + * if something goes wrong + */ @Before - public void prepareDatabase() throws Exception { + public void prepare() throws Exception { MockDatabase.startNode(); MockDatabase.insertProcessesForWorkflowFull(); SecurityTestUtils.addUserDataToSecurityContext(ServiceManager.getUserService().getById(1), 1); - - usersDirectory.mkdir(); - - if (!SystemUtils.IS_OS_WINDOWS) { - ExecutionPermission.setExecutePermission(scriptCreateDirUserHome); - ExecutionPermission.setExecutePermission(scriptCreateSymLink); - ExecutionPermission.setExecutePermission(scriptNotWorking); - ExecutionPermission.setExecutePermission(scriptWorking); - } } + /** + * Clean the data after every test. + * + * @throws Exception + * if something goes wrong + */ @After - public void cleanDatabase() throws Exception { + public void clean() throws Exception { MockDatabase.stopNode(); MockDatabase.cleanDatabase(); SecurityTestUtils.cleanSecurityContext(); - - if (!SystemUtils.IS_OS_WINDOWS) { - ExecutionPermission.setNoExecutePermission(scriptCreateDirUserHome); - ExecutionPermission.setNoExecutePermission(scriptCreateSymLink); - ExecutionPermission.setNoExecutePermission(scriptNotWorking); - ExecutionPermission.setNoExecutePermission(scriptWorking); - } - - usersDirectory.delete(); } @Test(expected = ProcessorException.class) @@ -123,8 +105,7 @@ public void testActionClose() throws Exception { @Test public void testActionErrorOpen() throws Exception { Task openTask = taskService.getById(9); - assertEquals("Task '" + openTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, - openTask.getProcessingStatus()); + assertEquals("Task '" + openTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, openTask.getProcessingStatus()); processAction(openTask, TaskAction.ERROR_OPEN); assertEquals("Task '" + openTask.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, taskService.getById(openTask.getId()).getProcessingStatus()); @@ -137,6 +118,16 @@ public void testActionErrorOpen() throws Exception { taskService.getById(inWorkTask.getId()).getProcessingStatus()); } + public void testActionErrorOpenWithCorrectionTask() throws Exception { + Task openTask = taskService.getById(9); + assertEquals("Task '" + openTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + openTask.getProcessingStatus()); + processAction(openTask, TaskAction.ERROR_OPEN); + assertEquals("Task '" + openTask.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + taskService.getById(openTask.getId()).getProcessingStatus()); + + } + @Test(expected = ProcessorException.class) public void testActionErrorOpenWithoutTaskStatusOpenOrInWork() throws Exception { Task task = taskService.getById(10); @@ -182,21 +173,21 @@ public void testActionComment() throws Exception { private static void processAction(Task task, TaskAction taskAction) throws JMSException, ProcessorException { String message = "Process action " + taskAction.name(); processAction(task.getId(), taskAction.name(), message); - List comments = ServiceManager.getCommentService().getAllCommentsByTask(task); assertEquals("Comment should be created!", 1, comments.size()); assertEquals("Comment message should be '" + message + "'!", message, comments.get(0).getMessage()); } - private static void processAction(Integer taskId, String action, String message) throws JMSException, ProcessorException { - TaskActionProcessor taskActionProcessor = spy(TaskActionProcessor.class); + private static void processAction(Integer taskId, String action, String message) throws JMSException, + ProcessorException { MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(taskId); when(mapMessageObjectReader.getMandatoryString("action")).thenReturn(action); - if(StringUtils.isNotEmpty(message)) { + if (StringUtils.isNotEmpty(message)) { when(mapMessageObjectReader.hasField("message")).thenReturn(Boolean.TRUE); when(mapMessageObjectReader.getMandatoryString("message")).thenReturn(message); } + TaskActionProcessor taskActionProcessor = spy(TaskActionProcessor.class); taskActionProcessor.process(mapMessageObjectReader); } } diff --git a/Kitodo/src/test/java/org/kitodo/production/services/workflow/WorkflowControllerServiceIT.java b/Kitodo/src/test/java/org/kitodo/production/services/workflow/WorkflowControllerServiceIT.java index 87fe5a17b61..18839d9a4d0 100644 --- a/Kitodo/src/test/java/org/kitodo/production/services/workflow/WorkflowControllerServiceIT.java +++ b/Kitodo/src/test/java/org/kitodo/production/services/workflow/WorkflowControllerServiceIT.java @@ -461,7 +461,7 @@ public void shouldSolveProblem() throws Exception { ServiceManager.getCommentService().saveToDatabase(correctionComment); workflowService.reportProblem(correctionComment, TaskEditType.MANUAL_SINGLE); - workflowService.solveProblem(correctionComment); + workflowService.solveProblem(correctionComment, TaskEditType.MANUAL_SINGLE); Process process = ServiceManager.getProcessService().getById(currentTask.getProcess().getId()); for (Task task : process.getTasks()) { From 07989da542cb1853409a112a527ecd110412ff31 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 10 May 2023 17:47:38 +0200 Subject: [PATCH 19/34] Add javadoc --- .../activemq/TaskActionProcessorIT.java | 64 ++++++++++++++++++- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java index 616658a74eb..d2ab6e918e9 100644 --- a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java +++ b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java @@ -73,6 +73,12 @@ public void testUnsupportedAction() throws Exception { processAction(9, "UNSUPPORTED", ""); } + /** + * Test the task action PROCESS. + * + * @throws Exception + * if something goes wrong + */ @Test public void testActionProcess() throws Exception { Task task = taskService.getById(9); @@ -83,6 +89,12 @@ public void testActionProcess() throws Exception { taskService.getById(task.getId()).getProcessingStatus()); } + /** + * Test the error case without task status OPEN for task action PROCESS. + * + * @throws Exception + * if something goes wrong + */ @Test(expected = ProcessorException.class) public void testActionProcessWithoutTaskStatusOpen() throws Exception { Task task = taskService.getById(8); @@ -91,7 +103,12 @@ public void testActionProcessWithoutTaskStatusOpen() throws Exception { processAction(task, TaskAction.PROCESS); } - + /** + * Test the task action CLOSE. + * + * @throws Exception + * if something goes wrong + */ @Test public void testActionClose() throws Exception { Task task = taskService.getById(9); @@ -102,10 +119,17 @@ public void testActionClose() throws Exception { taskService.getById(task.getId()).getProcessingStatus()); } + /** + * Test the task action ERROR_OPEN. + * + * @throws Exception + * if something goes wrong + */ @Test public void testActionErrorOpen() throws Exception { Task openTask = taskService.getById(9); - assertEquals("Task '" + openTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, openTask.getProcessingStatus()); + assertEquals("Task '" + openTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + openTask.getProcessingStatus()); processAction(openTask, TaskAction.ERROR_OPEN); assertEquals("Task '" + openTask.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, taskService.getById(openTask.getId()).getProcessingStatus()); @@ -118,6 +142,12 @@ public void testActionErrorOpen() throws Exception { taskService.getById(inWorkTask.getId()).getProcessingStatus()); } + /** + * Test the task action ERROR_OPEN. + * + * @throws Exception + * if something goes wrong + */ public void testActionErrorOpenWithCorrectionTask() throws Exception { Task openTask = taskService.getById(9); assertEquals("Task '" + openTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, @@ -128,6 +158,12 @@ public void testActionErrorOpenWithCorrectionTask() throws Exception { } + /** + * Test the error case with wrong task status for task action ERROR_OPEN. + * + * @throws Exception + * if something goes wrong + */ @Test(expected = ProcessorException.class) public void testActionErrorOpenWithoutTaskStatusOpenOrInWork() throws Exception { Task task = taskService.getById(10); @@ -136,12 +172,24 @@ public void testActionErrorOpenWithoutTaskStatusOpenOrInWork() throws Exception processAction(task, TaskAction.ERROR_OPEN); } + /** + * Test the error case without message for task action ERROR_OPEN. + * + * @throws Exception + * if something goes wrong + */ @Test(expected = ProcessorException.class) public void testActionErrorOpenWithoutMessage() throws Exception { Task task = taskService.getById(10); processAction(task.getId(), TaskAction.ERROR_OPEN.name(), StringUtils.EMPTY); } + /** + * Test the task action ERROR_CLOSE. + * + * @throws Exception + * if something goes wrong + */ @Test public void testActionErrorClose() throws Exception { Task task = taskService.getById(10); @@ -152,6 +200,12 @@ public void testActionErrorClose() throws Exception { taskService.getById(task.getId()).getProcessingStatus()); } + /** + * Test the error case without task status LOCKED for task action ERROR_CLOSE. + * + * @throws Exception + * if something goes wrong + */ @Test(expected = ProcessorException.class) public void testActionErrorCloseWithoutTaskStatusLocked() throws Exception { Task task = taskService.getById(9); @@ -160,6 +214,12 @@ public void testActionErrorCloseWithoutTaskStatusLocked() throws Exception { processAction(task, TaskAction.ERROR_CLOSE); } + /** + * Test the task action COMMENT. + * + * @throws Exception + * if something goes wrong + */ @Test public void testActionComment() throws Exception { Task task = taskService.getById(10); From bfb6ea1ef85dd821716b97373a30c5f77458703e Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 10 May 2023 19:22:34 +0200 Subject: [PATCH 20/34] Add test for correction task --- .../activemq/TaskActionProcessorIT.java | 83 ++++++++++++++----- 1 file changed, 62 insertions(+), 21 deletions(-) diff --git a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java index d2ab6e918e9..9c448b1f697 100644 --- a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java +++ b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java @@ -17,6 +17,7 @@ import static org.mockito.Mockito.when; import java.util.List; +import java.util.Objects; import javax.jms.JMSException; @@ -65,12 +66,12 @@ public void clean() throws Exception { @Test(expected = ProcessorException.class) public void testTaskNotFound() throws Exception { - processAction(Integer.MIN_VALUE, TaskAction.COMMENT.name(), ""); + processAction(Integer.MIN_VALUE, TaskAction.COMMENT.name(), StringUtils.EMPTY, null); } @Test(expected = ProcessorException.class) public void testUnsupportedAction() throws Exception { - processAction(9, "UNSUPPORTED", ""); + processAction(9, "UNSUPPORTED", StringUtils.EMPTY, null); } /** @@ -143,19 +144,22 @@ public void testActionErrorOpen() throws Exception { } /** - * Test the task action ERROR_OPEN. + * Test the task action ERROR_OPEN with a correction task. * * @throws Exception * if something goes wrong */ + @Test public void testActionErrorOpenWithCorrectionTask() throws Exception { - Task openTask = taskService.getById(9); - assertEquals("Task '" + openTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, - openTask.getProcessingStatus()); - processAction(openTask, TaskAction.ERROR_OPEN); - assertEquals("Task '" + openTask.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, - taskService.getById(openTask.getId()).getProcessingStatus()); - + Task task = taskService.getById(9); + Task correctionTask = taskService.getById(6); + assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + task.getProcessingStatus()); + processAction(task, TaskAction.ERROR_OPEN, correctionTask.getId(), 1); + assertEquals("Task '" + task.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + taskService.getById(task.getId()).getProcessingStatus()); + assertEquals("Correction task '" + correctionTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + taskService.getById(correctionTask.getId()).getProcessingStatus()); } /** @@ -181,7 +185,7 @@ public void testActionErrorOpenWithoutTaskStatusOpenOrInWork() throws Exception @Test(expected = ProcessorException.class) public void testActionErrorOpenWithoutMessage() throws Exception { Task task = taskService.getById(10); - processAction(task.getId(), TaskAction.ERROR_OPEN.name(), StringUtils.EMPTY); + processAction(task.getId(), TaskAction.ERROR_OPEN.name(), StringUtils.EMPTY, null); } /** @@ -200,6 +204,32 @@ public void testActionErrorClose() throws Exception { taskService.getById(task.getId()).getProcessingStatus()); } + /** + * Test the task action ERROR_CLOSE with a correction task. + * + * @throws Exception + * if something goes wrong + */ + @Test + public void testActionErrorCloseWithCorrectionTask() throws Exception { + Task task = taskService.getById(9); + Task correctionTask = taskService.getById(6); + assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + task.getProcessingStatus()); + + processAction(task, TaskAction.ERROR_OPEN, correctionTask.getId(), 1); + assertEquals("Task '" + task.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + taskService.getById(task.getId()).getProcessingStatus()); + assertEquals("Correction task '" + correctionTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + taskService.getById(correctionTask.getId()).getProcessingStatus()); + + processAction(task, TaskAction.ERROR_CLOSE, correctionTask.getId(), 2); + assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + taskService.getById(task.getId()).getProcessingStatus()); + assertEquals("Correction task '" + correctionTask.getTitle() + "' status should be DONE!", TaskStatus.DONE, + taskService.getById(correctionTask.getId()).getProcessingStatus()); + } + /** * Test the error case without task status LOCKED for task action ERROR_CLOSE. * @@ -231,21 +261,32 @@ public void testActionComment() throws Exception { } private static void processAction(Task task, TaskAction taskAction) throws JMSException, ProcessorException { - String message = "Process action " + taskAction.name(); - processAction(task.getId(), taskAction.name(), message); + processAction(task, taskAction, null, 1); + } + + private static void processAction(Task task, TaskAction taskAction, Integer correctionTaskId, int commentCount) + throws JMSException, ProcessorException { + String message = "Process action " + taskAction.name(); + processAction(task.getId(), taskAction.name(), message, correctionTaskId); List comments = ServiceManager.getCommentService().getAllCommentsByTask(task); - assertEquals("Comment should be created!", 1, comments.size()); - assertEquals("Comment message should be '" + message + "'!", message, comments.get(0).getMessage()); + assertEquals("Comment should be created!", commentCount, comments.size()); + assertEquals("Comment message should be '" + message + "'!", message, + comments.get(commentCount - 1).getMessage()); } - private static void processAction(Integer taskId, String action, String message) throws JMSException, - ProcessorException { + private static void processAction(Integer taskId, String action, String message, Integer correctionTaskId) + throws JMSException, ProcessorException { MapMessageObjectReader mapMessageObjectReader = mock(MapMessageObjectReader.class); - when(mapMessageObjectReader.getMandatoryInteger("id")).thenReturn(taskId); - when(mapMessageObjectReader.getMandatoryString("action")).thenReturn(action); + when(mapMessageObjectReader.getMandatoryInteger(TaskActionProcessor.KEY_TASK_ID)).thenReturn(taskId); + when(mapMessageObjectReader.getMandatoryString(TaskActionProcessor.KEY_TASK_ACTION)).thenReturn(action); if (StringUtils.isNotEmpty(message)) { - when(mapMessageObjectReader.hasField("message")).thenReturn(Boolean.TRUE); - when(mapMessageObjectReader.getMandatoryString("message")).thenReturn(message); + when(mapMessageObjectReader.hasField(TaskActionProcessor.KEY_MESSAGE)).thenReturn(Boolean.TRUE); + when(mapMessageObjectReader.getMandatoryString(TaskActionProcessor.KEY_MESSAGE)).thenReturn(message); + } + if (Objects.nonNull(correctionTaskId)) { + when(mapMessageObjectReader.hasField(TaskActionProcessor.KEY_CORRECTION_TASK_ID)).thenReturn(Boolean.TRUE); + when(mapMessageObjectReader.getMandatoryInteger(TaskActionProcessor.KEY_CORRECTION_TASK_ID)).thenReturn( + correctionTaskId); } TaskActionProcessor taskActionProcessor = spy(TaskActionProcessor.class); taskActionProcessor.process(mapMessageObjectReader); From f73738daa913ae656f015999c5a25f55140d499c Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Thu, 11 May 2023 14:44:20 +0200 Subject: [PATCH 21/34] Change javadoc --- .../production/interfaces/activemq/TaskActionProcessor.java | 4 ++-- Kitodo/src/main/resources/kitodo_config.properties | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index e0a67932891..f8e0668e7ae 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -36,7 +36,7 @@ import org.kitodo.production.services.workflow.WorkflowControllerService; /** - * This is a web service interface to modify task states. + * This is a web service interface to modify task status. */ public class TaskActionProcessor extends ActiveMQProcessor { @@ -58,7 +58,7 @@ public TaskActionProcessor() { } /** - * This is the main routine processing incoming tickets. It gets the task id and the task action for processing. + * This is the main routine processing incoming messages. It gets the task id and the task action for processing. * Every action has its own behavior so please read the comment on the action for more information. * * @param mapMessageObjectReader diff --git a/Kitodo/src/main/resources/kitodo_config.properties b/Kitodo/src/main/resources/kitodo_config.properties index 6f203486202..ec9ee4e2841 100644 --- a/Kitodo/src/main/resources/kitodo_config.properties +++ b/Kitodo/src/main/resources/kitodo_config.properties @@ -620,7 +620,7 @@ activeMQ.user=testAdmin # You can provide a queue from which messages are read to finalize steps #activeMQ.finalizeStep.queue=KitodoProduction.FinalizeStep.Queue -# You can provide a queue from which messages are read to change steps states +# You can provide a queue from which messages are read to process task actions #activeMQ.taskAction.queue=KitodoProduction.TaskAction.Queue # ----------------------------------- From 13bd1d3e1d50be3e717c983727aa2c9dcbf2bad1 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Tue, 30 May 2023 16:22:50 +0200 Subject: [PATCH 22/34] Update Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java Co-authored-by: Arved Solth --- .../production/interfaces/activemq/TaskActionProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index f8e0668e7ae..187813e5123 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -45,7 +45,7 @@ public class TaskActionProcessor extends ActiveMQProcessor { public static final String KEY_TASK_ACTION = "action"; public static final String KEY_TASK_ID = "id"; private final TaskService taskService = ServiceManager.getTaskService(); - private WorkflowControllerService workflowControllerService; + private final WorkflowControllerService workflowControllerService; /** * The default constructor looks up the queue name to use in kitodo_config.properties. If that is not configured and From df3472dde042074e56629425a2606403ce6ac6d1 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Tue, 30 May 2023 16:23:02 +0200 Subject: [PATCH 23/34] Update Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java Co-authored-by: Arved Solth --- .../production/interfaces/activemq/TaskActionProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index 187813e5123..e9924132583 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -78,7 +78,7 @@ protected void process(MapMessageObjectReader mapMessageObjectReader) throws Pro try { Task currentTask = taskService.getById(taskId); if (Objects.isNull(currentTask)) { - throw new ProcessorException("Task with id " + taskId + "not found."); + throw new ProcessorException("Task with id " + taskId + " not found."); } processAction(mapMessageObjectReader, taskAction, currentTask); From 2c2d391305133f7f2ae7363a9c38a500f63bd6d0 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Tue, 30 May 2023 16:23:57 +0200 Subject: [PATCH 24/34] Update Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java Co-authored-by: Arved Solth --- .../production/interfaces/activemq/TaskActionProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index e9924132583..ffb0afc999f 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -204,7 +204,7 @@ private void markErrorCommentAsCorrected(Task currentTask, Integer correctionTas optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( currentTaskComment.getType()) && !currentTaskComment.isCorrected() && correctionTaskId.equals( currentTaskComment.getCorrectionTask().getId())).findFirst(); - if (!optionalComment.isEmpty()) { + if (optionalComment.isPresent()) { workflowControllerService.solveProblem(optionalComment.get(), TaskEditType.QUEUE); } } From 48286d9aa36e65231a80e36a09c2e0dabec0e815 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 5 Jun 2023 17:01:49 +0200 Subject: [PATCH 25/34] Refactoring correction task --- .../interfaces/activemq/TaskActionProcessor.java | 10 +++++----- .../services/workflow/WorkflowControllerService.java | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index ffb0afc999f..9776340d658 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -158,8 +158,6 @@ private void actionErrorOpen(MapMessageObjectReader mapMessageObjectReader, Task throw new ProcessorException("Correction task with id " + correctionTaskId + " not found."); } comment.setCorrectionTask(correctionTask); - } else { - comment.setCorrectionTask(currentTask); } comment.setType(CommentType.ERROR); workflowControllerService.reportProblem(comment, TaskEditType.QUEUE); @@ -194,7 +192,7 @@ private void actionErrorClose(MapMessageObjectReader mapMessageObjectReader, Tas } private void markErrorCommentAsCorrected(Task currentTask) throws DAOException, DataException, IOException { - markErrorCommentAsCorrected(currentTask, currentTask.getId()); + markErrorCommentAsCorrected(currentTask, null); } private void markErrorCommentAsCorrected(Task currentTask, Integer correctionTaskId) @@ -202,8 +200,10 @@ private void markErrorCommentAsCorrected(Task currentTask, Integer correctionTas List comments = ServiceManager.getCommentService().getAllCommentsByTask(currentTask); Optional optionalComment; optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( - currentTaskComment.getType()) && !currentTaskComment.isCorrected() && correctionTaskId.equals( - currentTaskComment.getCorrectionTask().getId())).findFirst(); + currentTaskComment.getType()) && !currentTaskComment.isCorrected() && ((Objects.isNull( + correctionTaskId) && Objects.isNull(currentTaskComment.getCorrectionTask())) || (Objects.nonNull( + correctionTaskId) && correctionTaskId.equals(currentTaskComment.getCorrectionTask().getId())))) + .findFirst(); if (optionalComment.isPresent()) { workflowControllerService.solveProblem(optionalComment.get(), TaskEditType.QUEUE); } diff --git a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java index 454a7fe68cf..4f659c09108 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java @@ -363,7 +363,7 @@ public void reportProblem(Comment comment, TaskEditType taskEditType) throws Dat currentTask.setProcessingBegin(null); taskService.save(currentTask); - if (comment.hasSeparateCorrectionTask()) { + if (Objects.nonNull(comment.getCorrectionTask())) { Task correctionTask = comment.getCorrectionTask(); correctionTask.setProcessingStatus(TaskStatus.OPEN); correctionTask.setProcessingEnd(null); @@ -382,7 +382,7 @@ public void reportProblem(Comment comment, TaskEditType taskEditType) throws Dat */ public void solveProblem(Comment comment, TaskEditType taskEditType) throws DataException, DAOException, IOException { - if (comment.hasSeparateCorrectionTask()) { + if (Objects.nonNull(comment.getCorrectionTask())) { closeTaskByUser(comment.getCorrectionTask()); comment.setCorrectionTask(ServiceManager.getTaskService().getById(comment.getCorrectionTask().getId())); } else { From 92987da5339218a3f20bb95616bb9b8ef1b6c2b1 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 5 Jun 2023 17:03:39 +0200 Subject: [PATCH 26/34] Remove legacy check function --- .../java/org/kitodo/data/database/beans/Comment.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java index 067c0e32b2c..792b8339240 100644 --- a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java +++ b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java @@ -242,15 +242,6 @@ public void setCorrectionTask(Task correctionTask) { this.correctionTask = correctionTask; } - /** - * Check if the comment has a correction task which differs from the current task. - * - * @return true if the comment has a separate correction task - */ - public boolean hasSeparateCorrectionTask() { - return Objects.nonNull(correctionTask) && !correctionTask.equals(currentTask); - } - /** * Get process. * From a44f395244981cabcb30a33ad1ca3a2d41d9ff44 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 5 Jun 2023 17:09:30 +0200 Subject: [PATCH 27/34] Remove unnecessary formatting --- .../main/java/org/kitodo/data/database/beans/Comment.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java index 792b8339240..0631cccbe9e 100644 --- a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java +++ b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java @@ -12,7 +12,6 @@ package org.kitodo.data.database.beans; import java.util.Date; -import java.util.Objects; import javax.persistence.Column; import javax.persistence.Entity; @@ -25,6 +24,7 @@ import org.kitodo.data.database.enums.CommentType; + @Entity @Table(name = "comment") public class Comment extends BaseBean { @@ -235,8 +235,7 @@ public Task getCorrectionTask() { /** * Set correctionTask. * - * @param correctionTask - * as org.kitodo.data.database.beans.Task + * @param correctionTask as org.kitodo.data.database.beans.Task */ public void setCorrectionTask(Task correctionTask) { this.correctionTask = correctionTask; From 71360641667924deab3a18ac81c9a1a6e6b8ed09 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 5 Jun 2023 17:10:08 +0200 Subject: [PATCH 28/34] Remove unnecessary formatting --- .../src/main/java/org/kitodo/data/database/beans/Comment.java | 1 - 1 file changed, 1 deletion(-) diff --git a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java index 0631cccbe9e..6c55dbb93f1 100644 --- a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java +++ b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Comment.java @@ -24,7 +24,6 @@ import org.kitodo.data.database.enums.CommentType; - @Entity @Table(name = "comment") public class Comment extends BaseBean { From dc0395ed8ec94781d15099721417d2e390423fb3 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 7 Jun 2023 10:49:03 +0200 Subject: [PATCH 29/34] Review changes --- .../java/org/kitodo/data/database/persistence/CommentDAO.java | 2 +- Kitodo/src/main/resources/kitodo_config.properties | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java index 3e8647eeb0d..d9c12ed5376 100644 --- a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java +++ b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java @@ -63,7 +63,7 @@ public List getAllByProcess(Process process) { * @return List of comments */ public List getAllByTask(Task task) { - return getByQuery("FROM Comment WHERE currentTask_id = :taskId ORDER BY id ASC", + return getByQuery("FROM comment WHERE currentTask_id = :taskId ORDER BY id ASC", Collections.singletonMap("taskId", task.getId())); } diff --git a/Kitodo/src/main/resources/kitodo_config.properties b/Kitodo/src/main/resources/kitodo_config.properties index ec9ee4e2841..4f00df84098 100644 --- a/Kitodo/src/main/resources/kitodo_config.properties +++ b/Kitodo/src/main/resources/kitodo_config.properties @@ -598,6 +598,8 @@ taskProcessPropertyColumns= # Active MQ services need an existing user as whom to act in the system activeMQ.user=testAdmin +# You can provide a topic that Kitodo reports results and status messages to +#activeMQ.results.topic=KitodoProduction.ResultMessages.Topic # By default, Kitodo instructs the server to keep status messages for a # equivalent of 7 days. You can change this value (in milliseconds) to meet From 1d00f3f2c623bfc5ae65c288a6b961ec9a7692d1 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 7 Jun 2023 11:49:24 +0200 Subject: [PATCH 30/34] Revert change of table name --- .../java/org/kitodo/data/database/persistence/CommentDAO.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java index d9c12ed5376..3e8647eeb0d 100644 --- a/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java +++ b/Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/CommentDAO.java @@ -63,7 +63,7 @@ public List getAllByProcess(Process process) { * @return List of comments */ public List getAllByTask(Task task) { - return getByQuery("FROM comment WHERE currentTask_id = :taskId ORDER BY id ASC", + return getByQuery("FROM Comment WHERE currentTask_id = :taskId ORDER BY id ASC", Collections.singletonMap("taskId", task.getId())); } From 1660538fcf8d6985730f5c4d90cf74b9295afcf5 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Fri, 23 Jun 2023 16:52:55 +0200 Subject: [PATCH 31/34] Add code review improvements, change state to open errors and set task to state INWORK when correction id is not set --- .../activemq/TaskActionProcessor.java | 37 +++++++++++-------- .../workflow/WorkflowControllerService.java | 8 +++- .../activemq/TaskActionProcessorIT.java | 36 ++++++++---------- 3 files changed, 43 insertions(+), 38 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index 9776340d658..bf57005f869 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -82,11 +82,13 @@ protected void process(MapMessageObjectReader mapMessageObjectReader) throws Pro } processAction(mapMessageObjectReader, taskAction, currentTask); - ServiceManager.getProcessService().saveToIndex(currentTask.getProcess(), true); - for (Task task : currentTask.getProcess().getTasks()) { - // update tasks in elastic search index, which includes correction comment status - taskService.saveToIndex(task, true); + if (mapMessageObjectReader.hasField(KEY_CORRECTION_TASK_ID)) { + for (Task task : currentTask.getProcess().getTasks()) { + taskService.saveToIndex(task, true); + } + } else { + taskService.saveToIndex(currentTask, true); } } catch (DataException | DAOException | CustomResponseException | IOException e) { throw new ProcessorException(e); @@ -109,18 +111,20 @@ private void processAction(MapMessageObjectReader mapMessageObjectReader, TaskAc actionProcess(currentTask, currentUser); break; case ERROR_OPEN: - if (!TaskStatus.OPEN.equals(currentTask.getProcessingStatus()) && !TaskStatus.INWORK.equals( - currentTask.getProcessingStatus())) { - throw new ProcessorException("Status of task is not OPEN or INWORK."); + if (!TaskStatus.INWORK.equals(currentTask.getProcessingStatus())) { + throw new ProcessorException("Status of task is not INWORK."); } if (!mapMessageObjectReader.hasField(KEY_MESSAGE)) { throw new ProcessorException("Message field of task action ERROR_OPEN is required."); } - actionErrorOpen(mapMessageObjectReader, currentTask, comment); + actionErrorOpen(mapMessageObjectReader, comment); break; case ERROR_CLOSE: - if (!TaskStatus.LOCKED.equals(currentTask.getProcessingStatus())) { - throw new ProcessorException("Status of task is not LOCKED."); + if ((!mapMessageObjectReader.hasField(KEY_CORRECTION_TASK_ID) && !TaskStatus.INWORK.equals( + currentTask.getProcessingStatus())) || (mapMessageObjectReader.hasField( + KEY_CORRECTION_TASK_ID) && !TaskStatus.LOCKED.equals(currentTask.getProcessingStatus()))) { + throw new ProcessorException( + "Status of task is not INWORK if there is a no corrected task ID or LOCKED if there is a corrected task ID."); } actionErrorClose(mapMessageObjectReader, currentTask, currentUser); break; @@ -149,7 +153,7 @@ private static Comment buildComment(Task currentTask, String message) { return comment; } - private void actionErrorOpen(MapMessageObjectReader mapMessageObjectReader, Task currentTask, Comment comment) + private void actionErrorOpen(MapMessageObjectReader mapMessageObjectReader, Comment comment) throws ProcessorException, JMSException, DAOException, DataException { if (mapMessageObjectReader.hasField(KEY_CORRECTION_TASK_ID)) { Integer correctionTaskId = mapMessageObjectReader.getMandatoryInteger(KEY_CORRECTION_TASK_ID); @@ -200,13 +204,16 @@ private void markErrorCommentAsCorrected(Task currentTask, Integer correctionTas List comments = ServiceManager.getCommentService().getAllCommentsByTask(currentTask); Optional optionalComment; optionalComment = comments.stream().filter(currentTaskComment -> CommentType.ERROR.equals( - currentTaskComment.getType()) && !currentTaskComment.isCorrected() && ((Objects.isNull( - correctionTaskId) && Objects.isNull(currentTaskComment.getCorrectionTask())) || (Objects.nonNull( - correctionTaskId) && correctionTaskId.equals(currentTaskComment.getCorrectionTask().getId())))) - .findFirst(); + currentTaskComment.getType()) && !currentTaskComment.isCorrected() && isEqualCorrectionTask( + correctionTaskId, currentTaskComment.getCorrectionTask())).findFirst(); if (optionalComment.isPresent()) { workflowControllerService.solveProblem(optionalComment.get(), TaskEditType.QUEUE); } } + private static boolean isEqualCorrectionTask(Integer correctionTaskId, Task correctionTask) { + return (Objects.isNull(correctionTaskId) && Objects.isNull(correctionTask)) || (Objects.nonNull( + correctionTaskId) && correctionTaskId.equals(correctionTask.getId())); + } + } diff --git a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java index 4f659c09108..032a0611810 100644 --- a/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java +++ b/Kitodo/src/main/java/org/kitodo/production/services/workflow/WorkflowControllerService.java @@ -348,7 +348,10 @@ public void unassignTaskFromUser(Task task) throws DataException { /** * Unified method for report problem . * - * @param comment as Comment object + * @param comment + * as Comment object + * @param taskEditType + * of task change */ public void reportProblem(Comment comment, TaskEditType taskEditType) throws DataException { Task currentTask = comment.getCurrentTask(); @@ -356,7 +359,8 @@ public void reportProblem(Comment comment, TaskEditType taskEditType) throws Dat this.webDav.uploadFromHome(getCurrentUser(), comment.getProcess()); } Date date = new Date(); - currentTask.setProcessingStatus(TaskStatus.LOCKED); + currentTask.setProcessingStatus( + Objects.nonNull(comment.getCorrectionTask()) ? TaskStatus.LOCKED : TaskStatus.INWORK); currentTask.setEditType(taskEditType); currentTask.setProcessingTime(date); taskService.replaceProcessingUser(currentTask, getCurrentUser()); diff --git a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java index 9c448b1f697..c31762420bb 100644 --- a/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java +++ b/Kitodo/src/test/java/org/kitodo/production/interfaces/activemq/TaskActionProcessorIT.java @@ -128,18 +128,11 @@ public void testActionClose() throws Exception { */ @Test public void testActionErrorOpen() throws Exception { - Task openTask = taskService.getById(9); - assertEquals("Task '" + openTask.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, - openTask.getProcessingStatus()); - processAction(openTask, TaskAction.ERROR_OPEN); - assertEquals("Task '" + openTask.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, - taskService.getById(openTask.getId()).getProcessingStatus()); - Task inWorkTask = taskService.getById(8); assertEquals("Task '" + inWorkTask.getTitle() + "' status should be INWORK!", TaskStatus.INWORK, inWorkTask.getProcessingStatus()); processAction(inWorkTask, TaskAction.ERROR_OPEN); - assertEquals("Task '" + inWorkTask.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + assertEquals("Task '" + inWorkTask.getTitle() + "' status should be INWORK!", TaskStatus.INWORK, taskService.getById(inWorkTask.getId()).getProcessingStatus()); } @@ -151,9 +144,9 @@ public void testActionErrorOpen() throws Exception { */ @Test public void testActionErrorOpenWithCorrectionTask() throws Exception { - Task task = taskService.getById(9); + Task task = taskService.getById(8); Task correctionTask = taskService.getById(6); - assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + assertEquals("Task '" + task.getTitle() + "' status should be INWORK!", TaskStatus.INWORK, task.getProcessingStatus()); processAction(task, TaskAction.ERROR_OPEN, correctionTask.getId(), 1); assertEquals("Task '" + task.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, @@ -169,9 +162,9 @@ public void testActionErrorOpenWithCorrectionTask() throws Exception { * if something goes wrong */ @Test(expected = ProcessorException.class) - public void testActionErrorOpenWithoutTaskStatusOpenOrInWork() throws Exception { - Task task = taskService.getById(10); - assertEquals("Task '" + task.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + public void testActionErrorOpenWithoutTaskStatusInWork() throws Exception { + Task task = taskService.getById(9); + assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, task.getProcessingStatus()); processAction(task, TaskAction.ERROR_OPEN); } @@ -196,8 +189,8 @@ public void testActionErrorOpenWithoutMessage() throws Exception { */ @Test public void testActionErrorClose() throws Exception { - Task task = taskService.getById(10); - assertEquals("Task '" + task.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + Task task = taskService.getById(8); + assertEquals("Task '" + task.getTitle() + "' status should be INWORK!", TaskStatus.INWORK, task.getProcessingStatus()); processAction(task, TaskAction.ERROR_CLOSE); assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, @@ -212,9 +205,9 @@ public void testActionErrorClose() throws Exception { */ @Test public void testActionErrorCloseWithCorrectionTask() throws Exception { - Task task = taskService.getById(9); + Task task = taskService.getById(8); Task correctionTask = taskService.getById(6); - assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, + assertEquals("Task '" + task.getTitle() + "' status should be INWORK!", TaskStatus.INWORK, task.getProcessingStatus()); processAction(task, TaskAction.ERROR_OPEN, correctionTask.getId(), 1); @@ -237,7 +230,7 @@ public void testActionErrorCloseWithCorrectionTask() throws Exception { * if something goes wrong */ @Test(expected = ProcessorException.class) - public void testActionErrorCloseWithoutTaskStatusLocked() throws Exception { + public void testActionErrorCloseWithoutTaskStatusInWork() throws Exception { Task task = taskService.getById(9); assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, task.getProcessingStatus()); @@ -252,10 +245,11 @@ public void testActionErrorCloseWithoutTaskStatusLocked() throws Exception { */ @Test public void testActionComment() throws Exception { - Task task = taskService.getById(10); - assertEquals("Task '" + task.getTitle() + "' status should be LOCKED!", TaskStatus.LOCKED, + Task task = taskService.getById(9); + assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, task.getProcessingStatus()); - processAction(task, TaskAction.ERROR_CLOSE); + processAction(task, TaskAction.COMMENT, null, 1); + processAction(task, TaskAction.COMMENT, null, 2); assertEquals("Task '" + task.getTitle() + "' status should be OPEN!", TaskStatus.OPEN, taskService.getById(task.getId()).getProcessingStatus()); } From b3aa64e279829a86146b2043f954d482dac5a966 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Fri, 23 Jun 2023 17:06:27 +0200 Subject: [PATCH 32/34] Update task action comments --- .../org/kitodo/production/interfaces/activemq/TaskAction.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java index 54583e28e78..b58fd37df74 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskAction.java @@ -18,12 +18,12 @@ public enum TaskAction { COMMENT, /** - * Lock a task and add an error comment when task status is OPEN or INWORK. + * Add an error comment when task status is INWORK and set the task status to LOCKED if the correction id is set. */ ERROR_OPEN, /** - * Set task status of locked task to OPEN. + * Set task status of LOCKED (if correction id is set) or INWORK (if correction id is not set) task to OPEN. */ ERROR_CLOSE, From 12be95bbfad418bdea809da9cb7fb1a147666fb0 Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Wed, 5 Jul 2023 12:15:28 +0200 Subject: [PATCH 33/34] Remove seperate saveToIndex cause within save function the saveToIndex is already call, set processing date all the time --- .../activemq/TaskActionProcessor.java | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java index bf57005f869..4d5ea5d7537 100644 --- a/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java +++ b/Kitodo/src/main/java/org/kitodo/production/interfaces/activemq/TaskActionProcessor.java @@ -80,17 +80,8 @@ protected void process(MapMessageObjectReader mapMessageObjectReader) throws Pro if (Objects.isNull(currentTask)) { throw new ProcessorException("Task with id " + taskId + " not found."); } - processAction(mapMessageObjectReader, taskAction, currentTask); - - if (mapMessageObjectReader.hasField(KEY_CORRECTION_TASK_ID)) { - for (Task task : currentTask.getProcess().getTasks()) { - taskService.saveToIndex(task, true); - } - } else { - taskService.saveToIndex(currentTask, true); - } - } catch (DataException | DAOException | CustomResponseException | IOException e) { + } catch (DataException | DAOException | IOException e) { throw new ProcessorException(e); } } @@ -172,10 +163,8 @@ private void actionProcess(Task currentTask, User currentUser) throws DataExcept currentTask.setEditType(TaskEditType.QUEUE); currentTask.setProcessingTime(new Date()); taskService.replaceProcessingUser(currentTask, currentUser); - if (Objects.isNull(currentTask.getProcessingBegin())) { - currentTask.setProcessingBegin(new Date()); - taskService.save(currentTask); - } + currentTask.setProcessingBegin(new Date()); + taskService.save(currentTask); } private void actionErrorClose(MapMessageObjectReader mapMessageObjectReader, Task currentTask, User currentUser) From b7fa8357dc88f9f0f290a1250353dd6ad13ef23d Mon Sep 17 00:00:00 2001 From: Markus Weigelt Date: Mon, 2 Oct 2023 11:50:16 +0200 Subject: [PATCH 34/34] add verify method to compare correction tasks --- .../kitodo/production/forms/CommentForm.java | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java b/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java index b6c6ec754fa..05e8ebf9c20 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/CommentForm.java @@ -240,8 +240,8 @@ public String solveProblem(Comment comment) { public String solveProblemForAllBatchProcesses(Comment comment) { for (Task task : batchHelper.getSteps()) { for (Comment processComment : ServiceManager.getCommentService().getAllCommentsByProcess(task.getProcess())) { - if (!processComment.isCorrected() - && processComment.getCorrectionTask().getTitle().equals(comment.getCorrectionTask().getTitle())) { + if (!processComment.isCorrected() && verifyCorrectionTasks(comment.getCorrectionTask(), + processComment.getCorrectionTask())) { solveProblem(processComment); } } @@ -249,6 +249,26 @@ public String solveProblemForAllBatchProcesses(Comment comment) { return MessageFormat.format(REDIRECT_PATH, "tasks"); } + /** + * Verify whether both correction tasks are null or share identical titles. + * + * @param commentCorrectionTask + * The comment correction task + * @param processCommentCorrectionTask + * The process comment correction task + * @return True if they are null or have equal titles + */ + private static boolean verifyCorrectionTasks(Task commentCorrectionTask, Task processCommentCorrectionTask) { + if (Objects.isNull(commentCorrectionTask) && Objects.isNull(processCommentCorrectionTask)) { + return true; + } else if (Objects.isNull(commentCorrectionTask) && Objects.nonNull( + processCommentCorrectionTask) || Objects.nonNull(commentCorrectionTask) && Objects.isNull( + processCommentCorrectionTask)) { + return false; + } + return processCommentCorrectionTask.getTitle().equals(commentCorrectionTask.getTitle()); + } + /** * refresh the process in the session. *