Fix zombie workspaceitem in the solr index, improve ITs to cover all the scenarios

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 it is an item we cannot know about its previous state, so it could be a
-                        if (workflowItem != null) {
+                    // workspaceitem that has been deposited right now or an approved/reject
-                            String detail =
+                    // workflowitem.
-                                    Constants.typeText[event.getSubjectType()] + "-" + event.getSubjectID().toString();
+                    // As the workflow is not necessary enabled it can happen than a workspaceitem
-                            uniqueIdsToDelete.add(detail);
+                    // 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));
                 }

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<WorkspaceItem, Workspa
             workspaceItem = c.reloadEntity(workspaceItem);
             if (workspaceItem != null) {
                 delete(c, workspaceItem);
+            } else {
+                item = c.reloadEntity(item);
+                // check if the wsi has been pushed to the workflow
+                XmlWorkflowItem wfi = workflowItemService.findByItem(c, item);
+                if (wfi != null) {
+                    workflowItemService.delete(c, wfi);
+                }
             }
             item = c.reloadEntity(item);
             if (item != null) {

dspace-api/src/test/java/org/dspace/discovery/DiscoveryIT.java

@@ -9,31 +9,44 @@ package org.dspace.discovery;
 
 import static org.junit.Assert.assertEquals;
 
+import java.io.IOException;
+import java.sql.SQLException;
 import java.util.List;
 import javax.servlet.http.HttpServletRequest;
 
 import org.dspace.AbstractIntegrationTestWithDatabase;
+import org.dspace.authorize.AuthorizeException;
+import org.dspace.builder.ClaimedTaskBuilder;
 import org.dspace.builder.CollectionBuilder;
 import org.dspace.builder.CommunityBuilder;
-import org.dspace.builder.ItemBuilder;
+import org.dspace.builder.PoolTaskBuilder;
+import org.dspace.builder.WorkflowItemBuilder;
 import org.dspace.builder.WorkspaceItemBuilder;
 import org.dspace.content.Collection;
 import org.dspace.content.Community;
 import org.dspace.content.WorkspaceItem;
 import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.content.service.ItemService;
 import org.dspace.content.service.WorkspaceItemService;
 import org.dspace.discovery.indexobject.IndexableClaimedTask;
+import org.dspace.discovery.indexobject.IndexableItem;
 import org.dspace.discovery.indexobject.IndexablePoolTask;
+import org.dspace.discovery.indexobject.IndexableWorkflowItem;
+import org.dspace.discovery.indexobject.IndexableWorkspaceItem;
 import org.dspace.eperson.EPerson;
 import org.dspace.services.factory.DSpaceServicesFactory;
+import org.dspace.workflow.WorkflowException;
+import org.dspace.xmlworkflow.WorkflowConfigurationException;
 import org.dspace.xmlworkflow.factory.XmlWorkflowServiceFactory;
 import org.dspace.xmlworkflow.service.WorkflowRequirementsService;
 import org.dspace.xmlworkflow.service.XmlWorkflowService;
+import org.dspace.xmlworkflow.state.Step;
 import org.dspace.xmlworkflow.state.Workflow;
+import org.dspace.xmlworkflow.state.actions.WorkflowActionConfig;
 import org.dspace.xmlworkflow.storedcomponents.ClaimedTask;
+import org.dspace.xmlworkflow.storedcomponents.PoolTask;
 import org.dspace.xmlworkflow.storedcomponents.XmlWorkflowItem;
 import org.dspace.xmlworkflow.storedcomponents.service.ClaimedTaskService;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 
@@ -52,47 +65,72 @@ public class DiscoveryIT extends AbstractIntegrationTestWithDatabase {
 
     ClaimedTaskService claimedTaskService = XmlWorkflowServiceFactory.getInstance().getClaimedTaskService();
 
+    ItemService itemService = ContentServiceFactory.getInstance().getItemService();
+
     IndexingService indexer = DSpaceServicesFactory.getInstance().getServiceManager()
                                                    .getServiceByName(IndexingService.class.getName(),
                                                                      IndexingService.class);
 
 
-    @Ignore
     @Test
-    public void deleteWorkspaceItemSolrRecordAfterDeletionFromDbTest() throws Exception {
+    public void solrRecordsAfterDepositOrDeletionOfWorkspaceItemTest() throws Exception {
         context.turnOffAuthorisationSystem();
         Community community = CommunityBuilder.createCommunity(context)
                                               .withName("Parent Community")
                                               .build();
         Collection col = CollectionBuilder.createCollection(context, community)
+                                          .withName("Collection without workflow")
                                           .build();
+        Collection colWithWorkflow = CollectionBuilder.createCollection(context, community)
+                .withName("Collection WITH workflow")
+                .withWorkflowGroup(1, admin)
+                .build();
         WorkspaceItem workspaceItem = WorkspaceItemBuilder.createWorkspaceItem(context, col)
+                                                          .withTitle("No workflow")
                                                           .withAbstract("headache")
                                                           .build();
+        WorkspaceItem anotherWorkspaceItem = WorkspaceItemBuilder.createWorkspaceItem(context, col)
+                .withTitle("Another WS Item in No workflow collection")
+                .withAbstract("headache")
+                .build();
+        WorkspaceItem workspaceItemInWfCollection = WorkspaceItemBuilder.createWorkspaceItem(context, colWithWorkflow)
+                .withTitle("WS Item in workflow collection")
+                .withAbstract("headache")
+                .build();
         context.restoreAuthSystemState();
 
-        DiscoverQuery discoverQuery = new DiscoverQuery();
+        // we start with 3 ws items
-        discoverQuery.setQuery("*:*");
+        assertSearchQuery(IndexableWorkspaceItem.TYPE, 3);
-        discoverQuery.addFilterQueries("search.resourceid:" + workspaceItem.getID());
+        // simulate the deposit
-        DiscoverResult discoverResult = searchService.search(context, discoverQuery);
+        deposit(workspaceItem);
-        List<IndexableObject> indexableObjects = discoverResult.getIndexableObjects();
+        // now we should have 1 archived item and 2 ws items, no wf items or tasks
-        assertEquals(1, indexableObjects.size());
+        assertSearchQuery(IndexableWorkflowItem.TYPE, 0);
-        assertEquals(1, discoverResult.getTotalSearchResults());
+        assertSearchQuery(IndexablePoolTask.TYPE, 0);
+        assertSearchQuery(IndexableClaimedTask.TYPE, 0);
+        assertSearchQuery(IndexableWorkspaceItem.TYPE, 2);
+        assertSearchQuery(IndexableItem.TYPE, 1);
 
-        context.turnOffAuthorisationSystem();
+        // simulate the deposit of the ws item in the workflow collection
-        workspaceItemService.deleteAll(context, workspaceItem);
+        deposit(workspaceItemInWfCollection);
-        context.dispatchEvents();
+        // now we should have 1 wf, 1 pool task, 1 ws item and 1 item
-        context.restoreAuthSystemState();
+        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);
+        // simulate the delete of last workspace item
-        indexableObjects = discoverResult.getIndexableObjects();
+        deleteSubmission(anotherWorkspaceItem);
-        assertEquals(0, indexableObjects.size());
+
-        assertEquals(0, discoverResult.getTotalSearchResults());
+        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)
+        ClaimedTask taskToApprove = ClaimedTaskBuilder.createClaimedTask(context, collection, admin)
-                                                .withTitle("Test item")
+                                                .withTitle("Test workflow item to approve")
                                                 .withIssueDate("2019-03-06")
                                                 .withSubject("ExtraEntry")
                                                 .build();
-
+        ClaimedTask taskToReject = ClaimedTaskBuilder.createClaimedTask(context, collection, admin)
-        ItemBuilder.createItem(context, collection).build();
+                .withTitle("Test workflow item to reject")
-
+                .withIssueDate("2019-03-06")
-
+                .withSubject("ExtraEntry")
-
+                .build();
-        Workflow workflow = XmlWorkflowServiceFactory.getInstance().getWorkflowFactory().getWorkflow(collection);
+        PoolTask taskToClaim = PoolTaskBuilder.createPoolTask(context, collection, admin)
-
+                .withTitle("Test pool task to claim")
-        ItemBuilder.createItem(context, collection).build();
+                .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();
+        // claim
-        httpServletRequest.setParameter("submit_approve", "submit_approve");
+        claim(workflow, taskToClaim, admin);
-
+        assertSearchQuery(IndexableWorkflowItem.TYPE, 5);
-        XmlWorkflowItem workflowItem = workflowService.startWithoutNotify(context, wsi);
+        assertSearchQuery(IndexableClaimedTask.TYPE, 4);
-        context.dispatchEvents();
-        indexer.commit();
-
         assertSearchQuery(IndexablePoolTask.TYPE, 1);
+        assertSearchQuery(IndexableWorkspaceItem.TYPE, 0);
+        assertSearchQuery(IndexableItem.TYPE, 0);
 
-        context.turnOffAuthorisationSystem();
+        // unclaim
-        ItemBuilder.createItem(context, collection).build();
+        returnClaimedTask(taskToUnclaim);
-        context.restoreAuthSystemState();
+        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,
+        // reject
-                "reviewstep", "claimaction");
+        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);
 
-
+        // approve
-        context.dispatchEvents();
+        MockHttpServletRequest httpApproveRequest = new MockHttpServletRequest();
-        indexer.commit();
+        httpApproveRequest.setParameter("submit_approve", "submit_approve");
-
+        executeWorkflowAction(httpApproveRequest, workflow, taskToApprove);
-        context.turnOffAuthorisationSystem();
+        assertSearchQuery(IndexableWorkflowItem.TYPE, 3);
-        ItemBuilder.createItem(context, collection).build();
-        context.restoreAuthSystemState();
-
-        assertSearchQuery(IndexablePoolTask.TYPE, 0);
         assertSearchQuery(IndexableClaimedTask.TYPE, 1);
+        assertSearchQuery(IndexablePoolTask.TYPE, 2);
+        assertSearchQuery(IndexableWorkspaceItem.TYPE, 1);
+        assertSearchQuery(IndexableItem.TYPE, 1);
 
-        returnToPool(admin, workflowItem);
+        // abort pool task
-        context.dispatchEvents();
+        // as we have already unclaimed this task it is a pool task now
-        indexer.commit();
+        abort(taskToUnclaim.getWorkflowItem());
-
+        assertSearchQuery(IndexableWorkflowItem.TYPE, 2);
-        context.turnOffAuthorisationSystem();
+        assertSearchQuery(IndexableClaimedTask.TYPE, 1);
-        ItemBuilder.createItem(context, collection).build();
-        context.restoreAuthSystemState();
-
         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)
+    private void deposit(WorkspaceItem workspaceItem)
-            throws Exception {
+            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);
+        taskToUnclaim = context.reloadEntity(taskToUnclaim);
-        workflowService.doState(context, user, httpServletRequest, workflowItem.getID(), workflow,
+        context.setCurrentUser(taskToUnclaim.getOwner());
-                                workflow.getStep(stepId).getActionConfig(actionId));
+        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
+        Step step = workflow.getStep(task.getStepID());
-                .findByWorkflowIdAndEPerson(context, workflowItem, context.getCurrentUser());
+        WorkflowActionConfig currentActionConfig = step.getActionConfig(task.getActionID());
-        workflowService.deleteClaimedTask(context, workflowItem, task);
+        workflowService.doState(context, user, null, task.getWorkflowItem().getID(), workflow, currentActionConfig);
-        workflowRequirementsService.removeClaimedUser(context, workflowItem, task.getOwner(), task.getStepID());
+        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);
     }
 }