Fix case comment event emitting (#1058)

jbpm-case-mgmt/jbpm-case-mgmt-impl/src/main/java/org/jbpm/casemgmt/impl/command/CaseCommentCommand.java

@@ -113,7 +113,7 @@ public Void execute(Context context) {
             if (restrictedTo != null) {
                 ((CommentInstanceImpl)toUpdate).setRestrictedTo(restrictedTo);
             }
-            caseEventSupport.fireBeforeCaseCommentUpdated(caseFile.getCaseId(), caseFile, toUpdate);
+            caseEventSupport.fireAfterCaseCommentUpdated(caseFile.getCaseId(), caseFile, toUpdate);
         } else if (remove) {
             CommentInstance toRemove = ((CaseFileInstanceImpl)caseFile).getComments().stream()
                     .filter(c -> c.getId().equals(commentId))
@@ -125,7 +125,7 @@ public Void execute(Context context) {
 
             caseEventSupport.fireBeforeCaseCommentRemoved(caseFile.getCaseId(), caseFile, toRemove);
             ((CaseFileInstanceImpl)caseFile).removeComment(toRemove);
-            caseEventSupport.fireBeforeCaseCommentRemoved(caseFile.getCaseId(), caseFile, toRemove);
+            caseEventSupport.fireAfterCaseCommentRemoved(caseFile.getCaseId(), caseFile, toRemove);
         }
         ksession.update(factHandle, caseFile);
         triggerRules(ksession);

jbpm-case-mgmt/jbpm-case-mgmt-impl/src/main/java/org/jbpm/casemgmt/impl/event/CaseEventSupport.java

@@ -211,7 +211,7 @@ public void fireAfterCaseCommentAdded(String caseId, CaseFileInstance caseFile,
             final CaseCommentEvent event = new CaseCommentEvent(identityProvider.getName(), caseId, caseFile, commentInstance);
 
             do {
-                iter.next().beforeCaseCommentAdded(event);
+                iter.next().afterCaseCommentAdded(event);
             } while (iter.hasNext());
         }        
     }
@@ -238,7 +238,7 @@ public void fireAfterCaseCommentUpdated(String caseId, CaseFileInstance caseFile
             final CaseCommentEvent event = new CaseCommentEvent(identityProvider.getName(), caseId, caseFile, commentInstance);
 
             do {
-                iter.next().beforeCaseCommentUpdated(event);
+                iter.next().afterCaseCommentUpdated(event);
             } while (iter.hasNext());
         }        
     }
@@ -265,7 +265,7 @@ public void fireAfterCaseCommentRemoved(String caseId, CaseFileInstance caseFile
             final CaseCommentEvent event = new CaseCommentEvent(identityProvider.getName(), caseId, caseFile, commentInstance);
 
             do {
-                iter.next().beforeCaseCommentRemoved(event);
+                iter.next().afterCaseCommentRemoved(event);
             } while (iter.hasNext());
         }        
     }

jbpm-case-mgmt/jbpm-case-mgmt-impl/src/test/java/org/jbpm/casemgmt/impl/CaseEventEmittingTest.java

@@ -0,0 +1,93 @@
+/*
+ * Copyright 2017 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jbpm.casemgmt.impl;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.assertj.core.api.Assertions;
+import org.jbpm.casemgmt.api.event.CaseCommentEvent;
+import org.jbpm.casemgmt.api.event.CaseEvent;
+import org.jbpm.casemgmt.api.event.CaseStartEvent;
+import org.jbpm.casemgmt.api.model.instance.CommentInstance;
+import org.jbpm.casemgmt.impl.util.AbstractCaseServicesBaseTest;
+import org.jbpm.casemgmt.impl.util.RecordingCaseEventListener;
+import org.jbpm.casemgmt.impl.util.RecordingCaseEventListenerFactory;
+import org.jbpm.casemgmt.impl.util.RecordingCaseEventListener.CaseEventInfo;
+import org.jbpm.casemgmt.impl.util.RecordingCaseEventListener.CaseEventInfo.EventFired;
+import org.junit.Before;
+import org.junit.Test;
+import org.kie.api.runtime.query.QueryContext;
+
+public class CaseEventEmittingTest extends AbstractCaseServicesBaseTest {
+
+    private List<String> caseIds = new ArrayList<>();
+
+    private RecordingCaseEventListener caseEventListener =
+            RecordingCaseEventListenerFactory.get(getClass().getSimpleName());
+
+    @Override
+    protected List<String> getProcessDefinitionFiles() {
+        return Arrays.asList(
+            "cases/EmptyCase.bpmn2"
+        );
+    }
+
+    @Before
+    public void setUp() throws Exception {
+        registerListenerMvelDefinition("org.jbpm.casemgmt.impl.util.RecordingCaseEventListenerFactory.get(\""
+                + getClass().getSimpleName() + "\")");
+        super.setUp();
+    }
+
+    @Test
+    public void testCaseCommentEventsEmitting() {
+        String caseId = caseService.startCase(deploymentUnit.getIdentifier(), EMPTY_CASE_P_ID);
+        caseIds.add(caseId);
+
+        assertNextEvent(EventFired.BEFORE, CaseStartEvent.class);
+        assertNextEvent(EventFired.AFTER, CaseStartEvent.class);
+
+        caseService.addCaseComment(caseId, "no one", "My little comment");
+        String commentId = getFirstCommentId(caseId);
+        assertNextEvent(EventFired.BEFORE, CaseCommentEvent.class);
+        assertNextEvent(EventFired.AFTER, CaseCommentEvent.class);
+
+        caseService.updateCaseComment(caseId, commentId, "no one","My new comment text");
+        assertNextEvent(EventFired.BEFORE, CaseCommentEvent.class);
+        assertNextEvent(EventFired.AFTER, CaseCommentEvent.class);
+
+        caseService.removeCaseComment(caseId, commentId);
+        assertNextEvent(EventFired.BEFORE, CaseCommentEvent.class);
+        assertNextEvent(EventFired.AFTER, CaseCommentEvent.class);
+    }
+
+    private String getFirstCommentId(String caseId) {
+        return caseService.getCaseComments(caseId, new QueryContext())
+                .stream()
+                .findFirst()
+                .map(CommentInstance::getId)
+                .orElseThrow(() -> new IllegalArgumentException("Single comment expected to be retrieved"));
+    }
+
+    private void assertNextEvent(EventFired expectedToBeFired, Class<? extends CaseEvent> eventType) {
+        CaseEventInfo caseEventInfo = caseEventListener.getEvents().remove();
+
+        Assertions.assertThat(caseEventInfo.fired).isEqualTo(expectedToBeFired);
+        Assertions.assertThat(caseEventInfo.caseEvent).isInstanceOf(eventType);
+    }
+}

jbpm-case-mgmt/jbpm-case-mgmt-impl/src/test/java/org/jbpm/casemgmt/impl/util/AbstractCaseServicesBaseTest.java

@@ -134,6 +134,8 @@ public abstract class AbstractCaseServicesBaseTest {
 
     protected AuthorizationManager authorizationManager;
 
+    private List<String> listenerMvelDefinitions = new ArrayList<>();
+
     protected static final String EMPTY_CASE_P_ID = "EmptyCase";
     protected static final String USER_TASK_STAGE_CASE_P_ID = "UserTaskWithStageCase";
     protected static final String USER_TASK_CASE_P_ID = "UserTaskCase";
@@ -184,23 +186,6 @@ public void setUp() throws Exception {
         deploymentUnit = prepareDeploymentUnit();
     }
 
-    protected DeploymentUnit prepareDeploymentUnit() {
-        assertThat(deploymentService).isNotNull();
-        KModuleDeploymentUnit deploymentUnit = new KModuleDeploymentUnit(GROUP_ID, ARTIFACT_ID, VERSION);
-
-        final DeploymentDescriptor descriptor = new DeploymentDescriptorImpl();
-        descriptor.getBuilder().addEventListener(new NamedObjectModel(
-                "mvel",
-                "processIdentity",
-                "new org.jbpm.kie.services.impl.IdentityProviderAwareProcessListener(ksession)"
-        ));
-        deploymentUnit.setDeploymentDescriptor(descriptor);
-        deploymentUnit.setStrategy(RuntimeStrategy.PER_CASE);
-
-        deploymentService.deploy(deploymentUnit);
-        return deploymentUnit;
-    }
-
     @After
     public void tearDown() {
         clearDocumentStorageProperty();
@@ -222,6 +207,23 @@ public void tearDown() {
         ServiceRegistry.get().clear();
     }
 
+    protected DeploymentUnit prepareDeploymentUnit() {
+        assertThat(deploymentService).isNotNull();
+        KModuleDeploymentUnit deploymentUnit = new KModuleDeploymentUnit(GROUP_ID, ARTIFACT_ID, VERSION);
+
+        final DeploymentDescriptor descriptor = new DeploymentDescriptorImpl();
+        descriptor.getBuilder().addEventListener(new NamedObjectModel(
+                "mvel",
+                "processIdentity",
+                "new org.jbpm.kie.services.impl.IdentityProviderAwareProcessListener(ksession)"
+        ));
+        deploymentUnit.setDeploymentDescriptor(descriptor);
+        deploymentUnit.setStrategy(RuntimeStrategy.PER_CASE);
+
+        deploymentService.deploy(deploymentUnit);
+        return deploymentUnit;
+    }
+
     protected void close() {
         DataSetCore.set(null);
         if (emf != null) {
@@ -342,17 +344,8 @@ protected InternalKieModule createKieJar(KieServices ks, ReleaseId releaseId, Li
 
         KieFileSystem kfs = createKieFileSystemWithKProject(ks);
         kfs.writePomXML(getPom(releaseId));
-        // set the deployment descriptor so we use per case runtime strategy
-        DeploymentDescriptor customDescriptor = new DeploymentDescriptorImpl("org.jbpm.domain");
-        DeploymentDescriptorBuilder ddBuilder = customDescriptor.getBuilder()
-                .runtimeStrategy(RuntimeStrategy.PER_CASE)
-                .addMarshalingStrategy(new ObjectModel("mvel", CaseMarshallerFactory.builder().withDoc().toString()))
-                .addEventListener(new ObjectModel("mvel", "new org.jbpm.casemgmt.impl.util.TrackingCaseEventListener()"))
-                .addWorkItemHandler(new NamedObjectModel("mvel", "StartCaseInstance", "new org.jbpm.casemgmt.impl.wih.StartCaseWorkItemHandler(ksession)"));
 
-        for (ObjectModel listener : getProcessListeners()) {
-            ddBuilder.addEventListener(listener);
-        }
+        DeploymentDescriptor customDescriptor = createDeploymentDescriptor();
 
         if (extraResources == null) {
             extraResources = new HashMap<String, String>();
@@ -379,6 +372,27 @@ protected InternalKieModule createKieJar(KieServices ks, ReleaseId releaseId, Li
         return (InternalKieModule) kieBuilder.getKieModule();
     }
 
+    protected DeploymentDescriptor createDeploymentDescriptor() {
+        //add this listener by default
+        listenerMvelDefinitions.add("new org.jbpm.casemgmt.impl.util.TrackingCaseEventListener()");
+
+        DeploymentDescriptor customDescriptor = new DeploymentDescriptorImpl("org.jbpm.domain");
+        DeploymentDescriptorBuilder ddBuilder = customDescriptor.getBuilder()
+                .runtimeStrategy(RuntimeStrategy.PER_CASE)
+                .addMarshalingStrategy(new ObjectModel("mvel", CaseMarshallerFactory.builder().withDoc().toString()))
+                .addWorkItemHandler(new NamedObjectModel("mvel", "StartCaseInstance", "new org.jbpm.casemgmt.impl.wih.StartCaseWorkItemHandler(ksession)"));
+
+        listenerMvelDefinitions.forEach(
+                listenerDefinition -> ddBuilder.addEventListener(new ObjectModel("mvel", listenerDefinition))
+        );
+
+        getProcessListeners().forEach(
+                listener -> ddBuilder.addEventListener(listener)
+        );
+
+        return customDescriptor;
+    }
+
     protected KieFileSystem createKieFileSystemWithKProject(KieServices ks) {
         KieModuleModel kproj = ks.newKieModuleModel();
 
@@ -434,6 +448,10 @@ public boolean accept(File dir, String name) {
         }
     }
 
+    protected void registerListenerMvelDefinition(String listenerMvelDefinition) {
+        this.listenerMvelDefinitions.add(listenerMvelDefinition);
+    }
+
     public void setDeploymentService(DeploymentService deploymentService) {
         this.deploymentService = deploymentService;
     }