variable declaration and history support

[aguizar]

A previous discussion touched the subject of JBPM-2506. While the proposed code looks great I noticed a particularity I wanted to discuss. ExecutionService has gained the following methods:

/** creates or overwrites a variable value on the referenced execution and marks the variable to be stored in history*/
void setVariable(String executionId, String name, Object value, boolean historyEnabled);
  
/** creates or overwrites the variable values on the referenced execution and marks the variables to be stored in history*/
void setVariables(String executionId, Map<String, ?> variables, boolean historyEnabled);

These methods eventually reach a new overload for ScopeInstanceImpl.setVariable()

public void setVariable(String key, Object value, boolean historyEnabled) {
  // ...
  if (variable!=null) {
    log.debug("updating variable '"+key+"' in '"+this+"' to value '"+value+"'");
    variable.setValue(value, this);
  } else if (getParentVariableScope()==null) {
    createVariable(key, value, null, historyEnabled);
  } else {
    getParentVariableScope().setVariable(key,value, historyEnabled);
  }
}

Notice that, if the variable already exists, historyEnabled is ignored. On the other hand, ExecutionService does not expose the createVariable methods of ScopeInstanceImpl. Method createVariable is sometimes useful since setVariable always tries to create the variable in the parent scope.

My proposal is to make the following additions to ExecutionService instead. This way we expose useful new functionality and ensure that the historyEnabled flag is always observed.

/** creates a variable value on the referenced execution. optionally marks the variable to be stored in history*/
void createVariable(String executionId, String name, Object value, boolean historyEnabled);
  
/** creates variable values on the referenced execution. optionally marks the variables to be stored in history*/
void createVariables(String executionId, Map<String, ?> variables, boolean historyEnabled);

[swiderski.maciej]

Good to hear you found some time to look into it

So what you're saying is that we shall expose createVariable methods instead of setVariable?

Would that mean that once variable is created (without history being enabled) it won't be possible to alter it?

[aguizar]

Good to hear you found some time to look into it

Quite the opposite, thanks for the patience

Would that mean that once variable is created (without history being  enabled) it won't be possible to alter it?

Yes, it won't. However, what I wanted to point out with my extract from ScopeInstanceImpl.setVariable is that it ignores the historyEnabled parameter if the variable already exists. I do not know whether it makes sense to enable/disable history for an existing variable. In any case that would be a feature request that we can defer for the time being.

[swiderski.maciej]

Alejandro Guizar wrote:

Yes, it won't. However, what I wanted to point out with my extract from ScopeInstanceImpl.setVariable is that it ignores the historyEnabled parameter if the variable already exists. I do not know whether it makes sense to enable/disable history for an existing variable. In any case that would be a feature request that we can defer for the time being.

Fair enough, just wanted to make sure we are on the same page, so to say.

Agree that we could make further enhancement later on.

Will you make any other changes or shall I commit history stuff as well?

[aguizar]

Will you make any other changes or shall I commit history stuff as well?

Nope. Please go ahead.

[swiderski.maciej]

Code committed. I added a comment to jira but did not resolve the issue.