diff --git a/dspace-api/src/main/java/org/dspace/discovery/IndexEventConsumer.java b/dspace-api/src/main/java/org/dspace/discovery/IndexEventConsumer.java index 58ab7d2c8a..620d29c45d 100644 --- a/dspace-api/src/main/java/org/dspace/discovery/IndexEventConsumer.java +++ b/dspace-api/src/main/java/org/dspace/discovery/IndexEventConsumer.java @@ -14,7 +14,6 @@ import java.util.Set; import org.apache.logging.log4j.Logger; import org.dspace.content.Bundle; import org.dspace.content.DSpaceObject; -import org.dspace.content.Item; import org.dspace.core.Constants; import org.dspace.core.Context; import org.dspace.discovery.indexobject.factory.IndexFactory; @@ -22,9 +21,6 @@ import org.dspace.discovery.indexobject.factory.IndexObjectFactoryFactory; import org.dspace.event.Consumer; import org.dspace.event.Event; import org.dspace.services.factory.DSpaceServicesFactory; -import org.dspace.workflow.WorkflowItem; -import org.dspace.workflow.WorkflowItemService; -import org.dspace.workflow.factory.WorkflowServiceFactory; /** * Class for updating search indices in discovery from content events. @@ -51,9 +47,6 @@ public class IndexEventConsumer implements Consumer { IndexObjectFactoryFactory indexObjectServiceFactory = IndexObjectFactoryFactory.getInstance(); - private WorkflowItemService workflowItemService = WorkflowServiceFactory.getInstance().getWorkflowItemService(); - - @Override public void initialize() throws Exception { @@ -138,12 +131,16 @@ public class IndexEventConsumer implements Consumer { } else { log.debug("consume() adding event to update queue: " + event.toString()); if (event.getSubjectType() == Constants.ITEM) { - WorkflowItem workflowItem = workflowItemService.findByItem(ctx, (Item) subject); - if (workflowItem != null) { - String detail = - Constants.typeText[event.getSubjectType()] + "-" + event.getSubjectID().toString(); - uniqueIdsToDelete.add(detail); - } + // if it is an item we cannot know about its previous state, so it could be a + // workspaceitem that has been deposited right now or an approved/reject + // workflowitem. + // As the workflow is not necessary enabled it can happen than a workspaceitem + // became directly an item without giving us the chance to retrieve a + // workflowitem... so we need to force the unindex of all the related data + // before to index it again to be sure to don't leave any zombie in solr + String detail = + Constants.typeText[event.getSubjectType()] + "-" + event.getSubjectID().toString(); + uniqueIdsToDelete.add(detail); } objectsToUpdate.addAll(indexObjectServiceFactory.getIndexableObjects(ctx, subject)); } diff --git a/dspace-api/src/test/java/org/dspace/builder/WorkspaceItemBuilder.java b/dspace-api/src/test/java/org/dspace/builder/WorkspaceItemBuilder.java index eaee97e0fa..612ad82faa 100644 --- a/dspace-api/src/test/java/org/dspace/builder/WorkspaceItemBuilder.java +++ b/dspace-api/src/test/java/org/dspace/builder/WorkspaceItemBuilder.java @@ -22,6 +22,7 @@ import org.dspace.content.WorkspaceItem; import org.dspace.content.service.WorkspaceItemService; import org.dspace.core.Context; import org.dspace.eperson.EPerson; +import org.dspace.xmlworkflow.storedcomponents.XmlWorkflowItem; /** * Builder to construct WorkspaceItem objects @@ -110,6 +111,13 @@ public class WorkspaceItemBuilder extends AbstractBuilder indexableObjects = discoverResult.getIndexableObjects(); - assertEquals(1, indexableObjects.size()); - assertEquals(1, discoverResult.getTotalSearchResults()); + // we start with 3 ws items + assertSearchQuery(IndexableWorkspaceItem.TYPE, 3); + // simulate the deposit + deposit(workspaceItem); + // now we should have 1 archived item and 2 ws items, no wf items or tasks + assertSearchQuery(IndexableWorkflowItem.TYPE, 0); + assertSearchQuery(IndexablePoolTask.TYPE, 0); + assertSearchQuery(IndexableClaimedTask.TYPE, 0); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 2); + assertSearchQuery(IndexableItem.TYPE, 1); - context.turnOffAuthorisationSystem(); - workspaceItemService.deleteAll(context, workspaceItem); - context.dispatchEvents(); - context.restoreAuthSystemState(); + // simulate the deposit of the ws item in the workflow collection + deposit(workspaceItemInWfCollection); + // now we should have 1 wf, 1 pool task, 1 ws item and 1 item + assertSearchQuery(IndexableWorkflowItem.TYPE, 1); + assertSearchQuery(IndexablePoolTask.TYPE, 1); + assertSearchQuery(IndexableClaimedTask.TYPE, 0); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 1); + assertSearchQuery(IndexableItem.TYPE, 1); - discoverResult = searchService.search(context, discoverQuery); - indexableObjects = discoverResult.getIndexableObjects(); - assertEquals(0, indexableObjects.size()); - assertEquals(0, discoverResult.getTotalSearchResults()); + // simulate the delete of last workspace item + deleteSubmission(anotherWorkspaceItem); + + assertSearchQuery(IndexableWorkflowItem.TYPE, 1); + assertSearchQuery(IndexablePoolTask.TYPE, 1); + assertSearchQuery(IndexableClaimedTask.TYPE, 0); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 0); + assertSearchQuery(IndexableItem.TYPE, 1); } - @Ignore @Test - public void deleteWorkspaceItemSolrRecordAfterDeletionFromDbTestn() throws Exception { + public void solrRecordsAfterDealingWithWorkflowTest() throws Exception { context.turnOffAuthorisationSystem(); Community community = CommunityBuilder.createCommunity(context) .withName("Parent Community") @@ -100,62 +138,104 @@ public class DiscoveryIT extends AbstractIntegrationTestWithDatabase { Collection collection = CollectionBuilder.createCollection(context, community) .withWorkflowGroup(1, admin) .build(); + Workflow workflow = XmlWorkflowServiceFactory.getInstance().getWorkflowFactory().getWorkflow(collection); - WorkspaceItem wsi = WorkspaceItemBuilder.createWorkspaceItem(context, collection) - .withTitle("Test item") + ClaimedTask taskToApprove = ClaimedTaskBuilder.createClaimedTask(context, collection, admin) + .withTitle("Test workflow item to approve") .withIssueDate("2019-03-06") .withSubject("ExtraEntry") .build(); - - ItemBuilder.createItem(context, collection).build(); - - - - Workflow workflow = XmlWorkflowServiceFactory.getInstance().getWorkflowFactory().getWorkflow(collection); - - ItemBuilder.createItem(context, collection).build(); + ClaimedTask taskToReject = ClaimedTaskBuilder.createClaimedTask(context, collection, admin) + .withTitle("Test workflow item to reject") + .withIssueDate("2019-03-06") + .withSubject("ExtraEntry") + .build(); + PoolTask taskToClaim = PoolTaskBuilder.createPoolTask(context, collection, admin) + .withTitle("Test pool task to claim") + .withIssueDate("2019-03-06") + .withSubject("ExtraEntry") + .build(); + ClaimedTask taskToUnclaim = ClaimedTaskBuilder.createClaimedTask(context, collection, admin) + .withTitle("Test claimed task to unclaim") + .withIssueDate("2019-03-06") + .withSubject("ExtraEntry") + .build(); + XmlWorkflowItem wfiToDelete = WorkflowItemBuilder.createWorkflowItem(context, collection) + .withTitle("Test workflow item to return") + .withIssueDate("2019-03-06") + .withSubject("ExtraEntry") + .build(); context.restoreAuthSystemState(); + // we start with 5 workflow items, 3 claimed tasks, 2 pool task + assertSearchQuery(IndexableWorkflowItem.TYPE, 5); + assertSearchQuery(IndexableClaimedTask.TYPE, 3); + assertSearchQuery(IndexablePoolTask.TYPE, 2); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 0); + assertSearchQuery(IndexableItem.TYPE, 0); - MockHttpServletRequest httpServletRequest = new MockHttpServletRequest(); - httpServletRequest.setParameter("submit_approve", "submit_approve"); - - XmlWorkflowItem workflowItem = workflowService.startWithoutNotify(context, wsi); - context.dispatchEvents(); - indexer.commit(); - + // claim + claim(workflow, taskToClaim, admin); + assertSearchQuery(IndexableWorkflowItem.TYPE, 5); + assertSearchQuery(IndexableClaimedTask.TYPE, 4); assertSearchQuery(IndexablePoolTask.TYPE, 1); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 0); + assertSearchQuery(IndexableItem.TYPE, 0); - context.turnOffAuthorisationSystem(); - ItemBuilder.createItem(context, collection).build(); - context.restoreAuthSystemState(); + // unclaim + returnClaimedTask(taskToUnclaim); + assertSearchQuery(IndexableWorkflowItem.TYPE, 5); + assertSearchQuery(IndexableClaimedTask.TYPE, 3); + assertSearchQuery(IndexablePoolTask.TYPE, 2); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 0); + assertSearchQuery(IndexableItem.TYPE, 0); - executeWorkflowAction(httpServletRequest, admin, workflow, workflowItem, - "reviewstep", "claimaction"); + // reject + MockHttpServletRequest httpRejectRequest = new MockHttpServletRequest(); + httpRejectRequest.setParameter("submit_reject", "submit_reject"); + httpRejectRequest.setParameter("reason", "test"); + executeWorkflowAction(httpRejectRequest, workflow, taskToReject); + assertSearchQuery(IndexableWorkflowItem.TYPE, 4); + assertSearchQuery(IndexableClaimedTask.TYPE, 2); + assertSearchQuery(IndexablePoolTask.TYPE, 2); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 1); + assertSearchQuery(IndexableItem.TYPE, 0); - - context.dispatchEvents(); - indexer.commit(); - - context.turnOffAuthorisationSystem(); - ItemBuilder.createItem(context, collection).build(); - context.restoreAuthSystemState(); - - assertSearchQuery(IndexablePoolTask.TYPE, 0); + // approve + MockHttpServletRequest httpApproveRequest = new MockHttpServletRequest(); + httpApproveRequest.setParameter("submit_approve", "submit_approve"); + executeWorkflowAction(httpApproveRequest, workflow, taskToApprove); + assertSearchQuery(IndexableWorkflowItem.TYPE, 3); assertSearchQuery(IndexableClaimedTask.TYPE, 1); + assertSearchQuery(IndexablePoolTask.TYPE, 2); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 1); + assertSearchQuery(IndexableItem.TYPE, 1); - returnToPool(admin, workflowItem); - context.dispatchEvents(); - indexer.commit(); - - context.turnOffAuthorisationSystem(); - ItemBuilder.createItem(context, collection).build(); - context.restoreAuthSystemState(); - + // abort pool task + // as we have already unclaimed this task it is a pool task now + abort(taskToUnclaim.getWorkflowItem()); + assertSearchQuery(IndexableWorkflowItem.TYPE, 2); + assertSearchQuery(IndexableClaimedTask.TYPE, 1); assertSearchQuery(IndexablePoolTask.TYPE, 1); - assertSearchQuery(IndexableClaimedTask.TYPE, 0); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 2); + assertSearchQuery(IndexableItem.TYPE, 1); - workflowService.deleteWorkflowByWorkflowItem(context, workflowItem, admin); + // abort claimed task + // as we have already claimed this task it is a claimed task now + abort(taskToClaim.getWorkflowItem()); + assertSearchQuery(IndexableWorkflowItem.TYPE, 1); + assertSearchQuery(IndexableClaimedTask.TYPE, 0); + assertSearchQuery(IndexablePoolTask.TYPE, 1); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 3); + assertSearchQuery(IndexableItem.TYPE, 1); + + // delete pool task / workflow item + deleteWorkflowItem(wfiToDelete); + assertSearchQuery(IndexableWorkflowItem.TYPE, 0); + assertSearchQuery(IndexableClaimedTask.TYPE, 0); + assertSearchQuery(IndexablePoolTask.TYPE, 0); + assertSearchQuery(IndexableWorkspaceItem.TYPE, 3); + assertSearchQuery(IndexableItem.TYPE, 1); } private void assertSearchQuery(String resourceType, int size) throws SearchServiceException { @@ -168,24 +248,84 @@ public class DiscoveryIT extends AbstractIntegrationTestWithDatabase { assertEquals(size, discoverResult.getTotalSearchResults()); } - private void executeWorkflowAction(HttpServletRequest httpServletRequest, EPerson user, - Workflow workflow, XmlWorkflowItem workflowItem, String stepId, String actionId) - throws Exception { + + private void deposit(WorkspaceItem workspaceItem) + throws SQLException, AuthorizeException, IOException, WorkflowException, SearchServiceException { + context.turnOffAuthorisationSystem(); + workspaceItem = context.reloadEntity(workspaceItem); + XmlWorkflowItem workflowItem = workflowService.startWithoutNotify(context, workspaceItem); + context.commit(); + indexer.commit(); + context.restoreAuthSystemState(); + } + + private void deleteSubmission(WorkspaceItem anotherWorkspaceItem) + throws SQLException, AuthorizeException, IOException, SearchServiceException { + context.turnOffAuthorisationSystem(); + anotherWorkspaceItem = context.reloadEntity(anotherWorkspaceItem); + workspaceItemService.deleteAll(context, anotherWorkspaceItem); + context.commit(); + indexer.commit(); + context.restoreAuthSystemState(); + } + + private void deleteWorkflowItem(XmlWorkflowItem workflowItem) + throws SQLException, AuthorizeException, IOException, SearchServiceException { + context.turnOffAuthorisationSystem(); + workflowItem = context.reloadEntity(workflowItem); + workflowService.deleteWorkflowByWorkflowItem(context, workflowItem, admin); + context.commit(); + indexer.commit(); + context.restoreAuthSystemState(); + } + + private void returnClaimedTask(ClaimedTask taskToUnclaim) throws SQLException, IOException, + WorkflowConfigurationException, AuthorizeException, SearchServiceException { final EPerson previousUser = context.getCurrentUser(); - context.setCurrentUser(user); - workflowService.doState(context, user, httpServletRequest, workflowItem.getID(), workflow, - workflow.getStep(stepId).getActionConfig(actionId)); + taskToUnclaim = context.reloadEntity(taskToUnclaim); + context.setCurrentUser(taskToUnclaim.getOwner()); + XmlWorkflowItem workflowItem = taskToUnclaim.getWorkflowItem(); + workflowService.deleteClaimedTask(context, workflowItem, taskToUnclaim); + workflowRequirementsService.removeClaimedUser(context, workflowItem, taskToUnclaim.getOwner(), + taskToUnclaim.getStepID()); + context.commit(); + indexer.commit(); context.setCurrentUser(previousUser); } - private void returnToPool(EPerson user, XmlWorkflowItem workflowItem) + private void claim(Workflow workflow, PoolTask task, EPerson user) throws Exception { final EPerson previousUser = context.getCurrentUser(); + task = context.reloadEntity(task); context.setCurrentUser(user); - ClaimedTask task = claimedTaskService - .findByWorkflowIdAndEPerson(context, workflowItem, context.getCurrentUser()); - workflowService.deleteClaimedTask(context, workflowItem, task); - workflowRequirementsService.removeClaimedUser(context, workflowItem, task.getOwner(), task.getStepID()); + Step step = workflow.getStep(task.getStepID()); + WorkflowActionConfig currentActionConfig = step.getActionConfig(task.getActionID()); + workflowService.doState(context, user, null, task.getWorkflowItem().getID(), workflow, currentActionConfig); + context.commit(); + indexer.commit(); + context.setCurrentUser(previousUser); + } + + private void executeWorkflowAction(HttpServletRequest httpServletRequest, Workflow workflow, ClaimedTask task) + throws Exception { + final EPerson previousUser = context.getCurrentUser(); + task = context.reloadEntity(task); + context.setCurrentUser(task.getOwner()); + workflowService.doState(context, task.getOwner(), httpServletRequest, task.getWorkflowItem().getID(), workflow, + workflow.getStep(task.getStepID()).getActionConfig(task.getActionID())); + context.commit(); + indexer.commit(); + context.setCurrentUser(previousUser); + } + + private void abort(XmlWorkflowItem workflowItem) + throws SQLException, AuthorizeException, IOException, SearchServiceException { + final EPerson previousUser = context.getCurrentUser(); + workflowItem = context.reloadEntity(workflowItem); + context.setCurrentUser(admin); + workflowService.abort(context, workflowItem, admin); + context.commit(); + indexer.commit(); context.setCurrentUser(previousUser); } }