Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implementation of a method subprocesses #2260

Merged
merged 11 commits into from Dec 21, 2018
Merged

Implementation of a method subprocesses #2260

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7837aee
feat: implementation of subprocesses method
CTI777 Dec 12, 2018
06effa7
simple test added
CTI777 Dec 12, 2018
5d04a69
refactoring made for changes in activiti-api
CTI777 Dec 12, 2018
60cee40
a simple test added
CTI777 Dec 12, 2018
871d787
Not needed test removed
CTI777 Dec 12, 2018
f80bf8a
feat: processDefinitionVersion, parentId supported
CTI777 Dec 12, 2018
1de2a77
refactoring made, test updated
CTI777 Dec 13, 2018
f354348
fix: add Mybatis superExecution parent entity mapping
igdianov Dec 18, 2018
c1e0fe0
Merge branch 'develop' into CTI777-2113-Create-new-method-to-expose-s…
CTI777 Dec 20, 2018
e3b67c9
fix: not needed superexecution removed
CTI777 Dec 20, 2018
6245bca
Trigger CI build
igdianov Dec 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 4 additions & 25 deletions ...e-impl/src/main/java/org/activiti/runtime/api/model/impl/APIProcessInstanceConverter.java
View file
Open in desktop
Expand Up @@ -20,11 +20,6 @@

import org.activiti.api.process.model.ProcessInstance;
import org.activiti.api.runtime.model.impl.ProcessInstanceImpl;
import org.activiti.engine.ProcessEngines;
import org.activiti.engine.impl.cfg.ProcessEngineConfigurationImpl;
import org.activiti.engine.impl.context.Context;
import org.activiti.engine.impl.interceptor.Command;
import org.activiti.engine.impl.interceptor.CommandContext;
import org.activiti.engine.impl.persistence.entity.ExecutionEntity;

public class APIProcessInstanceConverter extends ListConverter<org.activiti.engine.runtime.ProcessInstance, ProcessInstance>
Expand All @@ -47,27 +42,11 @@ public ProcessInstance from(org.activiti.engine.runtime.ProcessInstance internal
//To do: it is not the best way to search parentProcessId by this method, it will require extra queries!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be a general problem for lazy loaded associations when using converters outside of Activiti engine transaction. This will also create N+1 additional queries to database if we need to resolve child entities for each record. We should be able to solve this by implementing or using a query to join associated entities in one query from the database.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another question: do we really need the parentProcessInstanceId or rootProcessInstanceId and callerId(not sure how to name this one) is enough? If we really need it, maybe we should consider to add an extra property to the internal process instance and store it in the database: similar to what is done for rootProcessInstanceId. @igdianov what do you think about that? @salaboy you may be interested in this conversation as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rootProcessInstanceId - is present but not clear if it is supported (filled) at all.
parentId (parentProcessInstanceId) we will need for sure to get list if subprocesses...
What is callerId and do we need it is a question...

Copy link
Contributor

@igdianov igdianov Dec 13, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erdemedeiros It makes sense to expose and provide parentProcessInstanceId attribute for query and audit services. This use case and provided hack exposes several problems. Problem 1 is that internal entity model converters need to run inside Activiti transaction in order to be able to resolve lazily associated superExecution entity, to get its parent process instance id. Problem 2 is that resolving lazy associations this way for each execution record will create additional queries to the engine database from many places when resolving execution context for aggregated events and api queries that use internal converters because of Problem 1.

The parent process instance seems to be a common property shared and used by model entities and event classes, so this particular use case could be solved by making changes to the execution entity manager query in Activiti engine to always eagerly load super execution entity with join query. This should have better performance overall especially for queries via Rest api.

//Set parent ProcessInstance Id
if(internalProcessInstance.getSuperExecutionId()!=null && ExecutionEntity.class.isInstance(internalProcessInstance)) {
if(internalProcessInstance.getSuperExecutionId()!=null) {
ExecutionEntity executionEntity = ExecutionEntity.class.cast(internalProcessInstance);

if (Context.getCommandContext()==null) {
ProcessEngineConfigurationImpl impl = (ProcessEngineConfigurationImpl)
ProcessEngines.getDefaultProcessEngine().getProcessEngineConfiguration();

impl.getCommandExecutor().execute(new Command<Void> () {

@Override
public Void execute(CommandContext commandContext) {
Optional.ofNullable(executionEntity.getSuperExecution())
.ifPresent(superExecution -> processInstance.setParentId(superExecution.getProcessInstanceId()));

return null;
}});

} else {
Optional.ofNullable(executionEntity.getSuperExecution())
.ifPresent(superExecution -> processInstance.setParentId(superExecution.getProcessInstanceId()));
}
Copy link
Contributor

@igdianov igdianov Dec 18, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erdemedeiros @CTI777 I have removed the hack in APIProcessInstanceConverter.java with commit f354348 in favor of using Mybatis mapping to eagerly fetch superExecution entity via association property. It seems like we always need to resolve parent process instance attributes in event converters and queries, so I think it makes sense to always fetch it.

See Mybatis changes below: f354348#diff-b9f50b6ee46df36fb61073916d78de78 for details.


Optional.ofNullable(executionEntity.getSuperExecution())
.ifPresent(superExecution -> processInstance.setParentId(superExecution.getProcessInstanceId()));
}
return processInstance;
}
Expand Down
6 changes: 6 additions & 0 deletions activiti-engine/src/main/resources/org/activiti/db/mapping/entity/Execution.xml
View file
Open in desktop
Expand Up @@ -240,6 +240,7 @@
<result property="deadLetterJobCount" column="DEADLETTER_JOB_COUNT_" jdbcType="INTEGER" />
<result property="variableCount" column="VAR_COUNT_" jdbcType="INTEGER" />
<result property="identityLinkCount" column="ID_LINK_COUNT_" jdbcType="INTEGER" />
<association property="superExecution" column="SUPER_EXEC_" select="selectSuperExecution"/>
</resultMap>

<resultMap id="processInstanceResultMap" type="org.activiti.engine.impl.persistence.entity.ExecutionEntityImpl">
Expand Down Expand Up @@ -279,6 +280,7 @@
<result property="deadLetterJobCount" column="DEADLETTER_JOB_COUNT_" jdbcType="INTEGER" />
<result property="variableCount" column="VAR_COUNT_" jdbcType="INTEGER" />
<result property="identityLinkCount" column="ID_LINK_COUNT_" jdbcType="INTEGER" />
<association property="superExecution" column="SUPER_EXEC_" select="selectSuperExecution"/>
</resultMap>

<resultMap id="executionAndVariablesResultMap" type="org.activiti.engine.impl.persistence.entity.ExecutionEntityImpl">
Expand Down Expand Up @@ -386,6 +388,10 @@
<select id="selectExecution" parameterType="string" resultMap="executionResultMap">
select * from ${prefix}ACT_RU_EXECUTION where ID_ = #{id, jdbcType=VARCHAR}
</select>

<select id="selectSuperExecution" parameterType="string" resultMap="executionResultMap">
select * from ${prefix}ACT_RU_EXECUTION where ID_ = #{id, jdbcType=VARCHAR}
</select>

<select id="selectExecutionsByParentExecutionId" parameterType="org.activiti.engine.impl.db.ListQueryParameterObject" resultMap="executionResultMap">
select * from ${prefix}ACT_RU_EXECUTION
Expand Down