From 5971786f7797b13509cb9d4a92050acfb23d59f5 Mon Sep 17 00:00:00 2001 From: hmatt1 Date: Tue, 21 Jul 2020 23:40:45 -0500 Subject: [PATCH 1/7] ProcessInstanceInfo set start date when constructed In the `PrometheusProcessEventListener`, the field `startDate` from a `processInstance` is used to time the process instance duration metric. https://github.com/kiegroup/droolsjbpm-integration/blob/4fb9e4c239dfc21ea99ea5b701877cafeafcbe1d/kie-server-parent/kie-server-services/kie-server-services-prometheus/src/main/java/org/kie/server/services/prometheus/PrometheusProcessEventListener.java#L96 However, the start date was always null because it is being set when `getProcessInstance` is first called. Since `getProcessInstance` is not called before we trigger the event listener, the start date was never set. The `ProcessInstance` start date should be set when the `ProcessInstanceInfo` object is first created. --- .../processinstance/ProcessInstanceInfo.java | 11 +++++++++++ .../jbpm/persistence/map/impl/MapPersistenceTest.java | 7 +++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java index 976f31937c..b246dcadc2 100644 --- a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java +++ b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java @@ -55,6 +55,7 @@ import org.jbpm.marshalling.impl.ProtobufRuleFlowProcessInstanceMarshaller; import org.jbpm.persistence.api.PersistentProcessInstance; import org.jbpm.process.instance.impl.ProcessInstanceImpl; +import org.jbpm.workflow.instance.WorkflowProcessInstance; import org.jbpm.workflow.instance.impl.WorkflowProcessInstanceImpl; import org.kie.api.runtime.Environment; import org.kie.api.runtime.process.ProcessInstance; @@ -103,6 +104,16 @@ public ProcessInstanceInfo(ProcessInstance processInstance) { this.processInstance = processInstance; this.processId = processInstance.getProcessId(); startDate = new Date(); + + // If we are creating a second Process Instance Info for the same process instance, + // it should not generate a new start date + if (this.processInstance != null) { + if (((WorkflowProcessInstanceImpl) this.processInstance).getStartDate() == null) { + ((WorkflowProcessInstanceImpl) processInstance).internalSetStartDate(this.startDate); + } else { + startDate = ((WorkflowProcessInstanceImpl) this.processInstance).getStartDate(); + } + } } public ProcessInstanceInfo(ProcessInstance processInstance, diff --git a/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java b/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java index f7bb018433..1a83c3f68e 100644 --- a/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java +++ b/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java @@ -272,7 +272,11 @@ public void processWithNotNullStartDateTest() { KieBase kbase = createKieBase(process); StatefulKnowledgeSession crmPersistentSession = createSession(kbase); - RuleFlowProcessInstance processInstance = (RuleFlowProcessInstance) crmPersistentSession.startProcess( processId ); + RuleFlowProcessInstance processInstance = (RuleFlowProcessInstance) crmPersistentSession.startProcess( processId ); + + // Ensure the Process Instance Start Date is set immediately when the processInstanceInfo is created + Assert.assertNotNull(processInstance.getStartDate()); + InternalKnowledgeRuntime kruntime = processInstance.getKnowledgeRuntime(); Assert.assertEquals( ProcessInstance.STATE_ACTIVE, processInstance.getState() ); @@ -280,7 +284,6 @@ public void processWithNotNullStartDateTest() { ProcessInstanceInfo processInstanceInfo = new ProcessInstanceInfo(processInstance); processInstance = (RuleFlowProcessInstance) processInstanceInfo.getProcessInstance(kruntime, crmPersistentSession.getEnvironment()); - Assert.assertNotNull(processInstance.getStartDate()); Assert.assertEquals(processInstance.getStartDate(), processInstanceInfo.getStartDate()); } From 2d6d719fde19a5564100d72a948274785e34b152 Mon Sep 17 00:00:00 2001 From: hmatt1 Date: Mon, 27 Jul 2020 20:30:37 -0500 Subject: [PATCH 2/7] twoProcessInstanceInfoSameStartDateTest The twoProcessInstanceInfoSameStartDateTest was added along with other minor changes based on review comments --- .../processinstance/ProcessInstanceInfo.java | 16 +++++----------- .../map/impl/MapPersistenceTest.java | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java index b246dcadc2..c7d0e02a0b 100644 --- a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java +++ b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java @@ -48,14 +48,12 @@ import org.drools.core.marshalling.impl.PersisterHelper; import org.drools.core.marshalling.impl.ProcessMarshallerWriteContext; import org.drools.core.marshalling.impl.ProtobufMarshaller; -import org.drools.persistence.api.Transformable; import org.jbpm.marshalling.impl.JBPMMessages; import org.jbpm.marshalling.impl.ProcessInstanceMarshaller; import org.jbpm.marshalling.impl.ProcessMarshallerRegistry; import org.jbpm.marshalling.impl.ProtobufRuleFlowProcessInstanceMarshaller; import org.jbpm.persistence.api.PersistentProcessInstance; import org.jbpm.process.instance.impl.ProcessInstanceImpl; -import org.jbpm.workflow.instance.WorkflowProcessInstance; import org.jbpm.workflow.instance.impl.WorkflowProcessInstanceImpl; import org.kie.api.runtime.Environment; import org.kie.api.runtime.process.ProcessInstance; @@ -103,16 +101,12 @@ protected ProcessInstanceInfo() { public ProcessInstanceInfo(ProcessInstance processInstance) { this.processInstance = processInstance; this.processId = processInstance.getProcessId(); - startDate = new Date(); - // If we are creating a second Process Instance Info for the same process instance, - // it should not generate a new start date - if (this.processInstance != null) { - if (((WorkflowProcessInstanceImpl) this.processInstance).getStartDate() == null) { - ((WorkflowProcessInstanceImpl) processInstance).internalSetStartDate(this.startDate); - } else { - startDate = ((WorkflowProcessInstanceImpl) this.processInstance).getStartDate(); - } + if (((WorkflowProcessInstanceImpl) this.processInstance).getStartDate() == null) { + startDate = new Date(); + ((WorkflowProcessInstanceImpl) processInstance).internalSetStartDate(this.startDate); + } else { + startDate = ((WorkflowProcessInstanceImpl) this.processInstance).getStartDate(); } } diff --git a/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java b/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java index 1a83c3f68e..1c30228f5e 100644 --- a/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java +++ b/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java @@ -286,7 +286,24 @@ public void processWithNotNullStartDateTest() { Assert.assertEquals(processInstance.getStartDate(), processInstanceInfo.getStartDate()); } - + + @Test + public void twoProcessInstanceInfoSameStartDateTest() { + String processId = "twoProcessInstanceInfoSameStartDateTest"; + String eventType = "myEvent"; + RuleFlowProcess process = ProcessCreatorForHelp.newSimpleEventProcess( processId, + eventType ); + + KieBase kbase = createKieBase(process); + StatefulKnowledgeSession crmPersistentSession = createSession(kbase); + + RuleFlowProcessInstance processInstance = (RuleFlowProcessInstance) crmPersistentSession.startProcess( processId ); + ProcessInstanceInfo firstProcessInstanceInfo = new ProcessInstanceInfo(processInstance); + ProcessInstanceInfo secondProcessInstanceInfo = new ProcessInstanceInfo(processInstance); + + Assert.assertEquals(firstProcessInstanceInfo.getStartDate(), secondProcessInstanceInfo.getStartDate()); + } + protected abstract StatefulKnowledgeSession createSession(KieBase kbase); protected abstract StatefulKnowledgeSession disposeAndReloadSession(StatefulKnowledgeSession crmPersistentSession, From fcd66559913a4d133a900e8369d64e91d6f4e258 Mon Sep 17 00:00:00 2001 From: hmatt1 Date: Mon, 27 Jul 2020 20:36:40 -0500 Subject: [PATCH 3/7] Using this.startDate consistently --- .../jbpm/persistence/processinstance/ProcessInstanceInfo.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java index c7d0e02a0b..20503eef7c 100644 --- a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java +++ b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java @@ -103,10 +103,10 @@ public ProcessInstanceInfo(ProcessInstance processInstance) { this.processId = processInstance.getProcessId(); if (((WorkflowProcessInstanceImpl) this.processInstance).getStartDate() == null) { - startDate = new Date(); + this.startDate = new Date(); ((WorkflowProcessInstanceImpl) processInstance).internalSetStartDate(this.startDate); } else { - startDate = ((WorkflowProcessInstanceImpl) this.processInstance).getStartDate(); + this.startDate = ((WorkflowProcessInstanceImpl) this.processInstance).getStartDate(); } } From fdcc0d9d06341b98df7ab90a92dbb5f4df2526fa Mon Sep 17 00:00:00 2001 From: hmatt1 Date: Thu, 30 Jul 2020 23:50:03 -0500 Subject: [PATCH 4/7] Revert "Using this.startDate consistently" This reverts commit fcd66559913a4d133a900e8369d64e91d6f4e258. --- .../jbpm/persistence/processinstance/ProcessInstanceInfo.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java index 20503eef7c..c7d0e02a0b 100644 --- a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java +++ b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java @@ -103,10 +103,10 @@ public ProcessInstanceInfo(ProcessInstance processInstance) { this.processId = processInstance.getProcessId(); if (((WorkflowProcessInstanceImpl) this.processInstance).getStartDate() == null) { - this.startDate = new Date(); + startDate = new Date(); ((WorkflowProcessInstanceImpl) processInstance).internalSetStartDate(this.startDate); } else { - this.startDate = ((WorkflowProcessInstanceImpl) this.processInstance).getStartDate(); + startDate = ((WorkflowProcessInstanceImpl) this.processInstance).getStartDate(); } } From e100dbb9366a62da6e06f48900611e18bdddbe64 Mon Sep 17 00:00:00 2001 From: hmatt1 Date: Thu, 30 Jul 2020 23:50:10 -0500 Subject: [PATCH 5/7] Revert "twoProcessInstanceInfoSameStartDateTest" This reverts commit 2d6d719fde19a5564100d72a948274785e34b152. --- .../processinstance/ProcessInstanceInfo.java | 16 +++++++++++----- .../map/impl/MapPersistenceTest.java | 19 +------------------ 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java index c7d0e02a0b..b246dcadc2 100644 --- a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java +++ b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java @@ -48,12 +48,14 @@ import org.drools.core.marshalling.impl.PersisterHelper; import org.drools.core.marshalling.impl.ProcessMarshallerWriteContext; import org.drools.core.marshalling.impl.ProtobufMarshaller; +import org.drools.persistence.api.Transformable; import org.jbpm.marshalling.impl.JBPMMessages; import org.jbpm.marshalling.impl.ProcessInstanceMarshaller; import org.jbpm.marshalling.impl.ProcessMarshallerRegistry; import org.jbpm.marshalling.impl.ProtobufRuleFlowProcessInstanceMarshaller; import org.jbpm.persistence.api.PersistentProcessInstance; import org.jbpm.process.instance.impl.ProcessInstanceImpl; +import org.jbpm.workflow.instance.WorkflowProcessInstance; import org.jbpm.workflow.instance.impl.WorkflowProcessInstanceImpl; import org.kie.api.runtime.Environment; import org.kie.api.runtime.process.ProcessInstance; @@ -101,12 +103,16 @@ protected ProcessInstanceInfo() { public ProcessInstanceInfo(ProcessInstance processInstance) { this.processInstance = processInstance; this.processId = processInstance.getProcessId(); + startDate = new Date(); - if (((WorkflowProcessInstanceImpl) this.processInstance).getStartDate() == null) { - startDate = new Date(); - ((WorkflowProcessInstanceImpl) processInstance).internalSetStartDate(this.startDate); - } else { - startDate = ((WorkflowProcessInstanceImpl) this.processInstance).getStartDate(); + // If we are creating a second Process Instance Info for the same process instance, + // it should not generate a new start date + if (this.processInstance != null) { + if (((WorkflowProcessInstanceImpl) this.processInstance).getStartDate() == null) { + ((WorkflowProcessInstanceImpl) processInstance).internalSetStartDate(this.startDate); + } else { + startDate = ((WorkflowProcessInstanceImpl) this.processInstance).getStartDate(); + } } } diff --git a/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java b/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java index 1c30228f5e..1a83c3f68e 100644 --- a/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java +++ b/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java @@ -286,24 +286,7 @@ public void processWithNotNullStartDateTest() { Assert.assertEquals(processInstance.getStartDate(), processInstanceInfo.getStartDate()); } - - @Test - public void twoProcessInstanceInfoSameStartDateTest() { - String processId = "twoProcessInstanceInfoSameStartDateTest"; - String eventType = "myEvent"; - RuleFlowProcess process = ProcessCreatorForHelp.newSimpleEventProcess( processId, - eventType ); - - KieBase kbase = createKieBase(process); - StatefulKnowledgeSession crmPersistentSession = createSession(kbase); - - RuleFlowProcessInstance processInstance = (RuleFlowProcessInstance) crmPersistentSession.startProcess( processId ); - ProcessInstanceInfo firstProcessInstanceInfo = new ProcessInstanceInfo(processInstance); - ProcessInstanceInfo secondProcessInstanceInfo = new ProcessInstanceInfo(processInstance); - - Assert.assertEquals(firstProcessInstanceInfo.getStartDate(), secondProcessInstanceInfo.getStartDate()); - } - + protected abstract StatefulKnowledgeSession createSession(KieBase kbase); protected abstract StatefulKnowledgeSession disposeAndReloadSession(StatefulKnowledgeSession crmPersistentSession, From b12f8880d975bf366a0dd020c7eb36295a090572 Mon Sep 17 00:00:00 2001 From: hmatt1 Date: Thu, 30 Jul 2020 23:50:25 -0500 Subject: [PATCH 6/7] Revert "ProcessInstanceInfo set start date when constructed" This reverts commit 5971786f7797b13509cb9d4a92050acfb23d59f5. --- .../processinstance/ProcessInstanceInfo.java | 11 ----------- .../jbpm/persistence/map/impl/MapPersistenceTest.java | 7 ++----- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java index b246dcadc2..976f31937c 100644 --- a/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java +++ b/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java @@ -55,7 +55,6 @@ import org.jbpm.marshalling.impl.ProtobufRuleFlowProcessInstanceMarshaller; import org.jbpm.persistence.api.PersistentProcessInstance; import org.jbpm.process.instance.impl.ProcessInstanceImpl; -import org.jbpm.workflow.instance.WorkflowProcessInstance; import org.jbpm.workflow.instance.impl.WorkflowProcessInstanceImpl; import org.kie.api.runtime.Environment; import org.kie.api.runtime.process.ProcessInstance; @@ -104,16 +103,6 @@ public ProcessInstanceInfo(ProcessInstance processInstance) { this.processInstance = processInstance; this.processId = processInstance.getProcessId(); startDate = new Date(); - - // If we are creating a second Process Instance Info for the same process instance, - // it should not generate a new start date - if (this.processInstance != null) { - if (((WorkflowProcessInstanceImpl) this.processInstance).getStartDate() == null) { - ((WorkflowProcessInstanceImpl) processInstance).internalSetStartDate(this.startDate); - } else { - startDate = ((WorkflowProcessInstanceImpl) this.processInstance).getStartDate(); - } - } } public ProcessInstanceInfo(ProcessInstance processInstance, diff --git a/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java b/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java index 1a83c3f68e..f7bb018433 100644 --- a/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java +++ b/jbpm-persistence/jbpm-persistence-jpa/src/test/java/org/jbpm/persistence/map/impl/MapPersistenceTest.java @@ -272,11 +272,7 @@ public void processWithNotNullStartDateTest() { KieBase kbase = createKieBase(process); StatefulKnowledgeSession crmPersistentSession = createSession(kbase); - RuleFlowProcessInstance processInstance = (RuleFlowProcessInstance) crmPersistentSession.startProcess( processId ); - - // Ensure the Process Instance Start Date is set immediately when the processInstanceInfo is created - Assert.assertNotNull(processInstance.getStartDate()); - + RuleFlowProcessInstance processInstance = (RuleFlowProcessInstance) crmPersistentSession.startProcess( processId ); InternalKnowledgeRuntime kruntime = processInstance.getKnowledgeRuntime(); Assert.assertEquals( ProcessInstance.STATE_ACTIVE, processInstance.getState() ); @@ -284,6 +280,7 @@ public void processWithNotNullStartDateTest() { ProcessInstanceInfo processInstanceInfo = new ProcessInstanceInfo(processInstance); processInstance = (RuleFlowProcessInstance) processInstanceInfo.getProcessInstance(kruntime, crmPersistentSession.getEnvironment()); + Assert.assertNotNull(processInstance.getStartDate()); Assert.assertEquals(processInstance.getStartDate(), processInstanceInfo.getStartDate()); } From dccf21bb85757512972ff5ae637ac984a49e268f Mon Sep 17 00:00:00 2001 From: hmatt1 Date: Fri, 31 Jul 2020 00:04:52 -0500 Subject: [PATCH 7/7] Set startDate in WorkflowProcessInstanceImpl Set the start date when `start()` is called --- .../workflow/instance/impl/WorkflowProcessInstanceImpl.java | 1 + .../org/jbpm/workflow/instance/node/StartNodeInstanceTest.java | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/jbpm-flow/src/main/java/org/jbpm/workflow/instance/impl/WorkflowProcessInstanceImpl.java b/jbpm-flow/src/main/java/org/jbpm/workflow/instance/impl/WorkflowProcessInstanceImpl.java index 32fbfdb918..57612768ff 100644 --- a/jbpm-flow/src/main/java/org/jbpm/workflow/instance/impl/WorkflowProcessInstanceImpl.java +++ b/jbpm-flow/src/main/java/org/jbpm/workflow/instance/impl/WorkflowProcessInstanceImpl.java @@ -491,6 +491,7 @@ public void start() { @Override public void start(String trigger) { synchronized (this) { + this.startDate = new Date(); registerExternalEventNodeListeners(); // activate timer event sub processes Node[] nodes = getNodeContainer().getNodes(); diff --git a/jbpm-flow/src/test/java/org/jbpm/workflow/instance/node/StartNodeInstanceTest.java b/jbpm-flow/src/test/java/org/jbpm/workflow/instance/node/StartNodeInstanceTest.java index 0154784305..53abd32345 100644 --- a/jbpm-flow/src/test/java/org/jbpm/workflow/instance/node/StartNodeInstanceTest.java +++ b/jbpm-flow/src/test/java/org/jbpm/workflow/instance/node/StartNodeInstanceTest.java @@ -75,7 +75,8 @@ public void testStartNode() { assertEquals( ProcessInstance.STATE_PENDING, processInstance.getState() ); processInstance.start(); assertEquals( ProcessInstance.STATE_ACTIVE, processInstance.getState() ); - + assertNotNull(processInstance.getStartDate()); + MockNodeInstance mockNodeInstance = mockNodeFactory.getMockNodeInstance(); List triggeredBy = mockNodeInstance.getTriggers().get(org.jbpm.workflow.core.Node.CONNECTION_DEFAULT_TYPE);