Skip to content

Commit ded6891

Browse files
HarishMalavadeHarishMalavade
authored
Fix restricted flag drop on task completion (#221)
2 parents db5d7b6 + 827cca4 commit ded6891

22 files changed

Lines changed: 215 additions & 74 deletions

File tree

‎engine/src/main/java/org/finos/fluxnova/bpm/engine/impl/bpmn/parser/BpmnParse.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,8 @@ public class BpmnParse extends Parse {
263263
public static final String PROPERTYNAME_IS_MULTI_INSTANCE = "isMultiInstance";
264264

265265
public static final Namespace CAMUNDA_BPMN_EXTENSIONS_NS = new Namespace(BpmnParser.CAMUNDA_BPMN_EXTENSIONS_NS, BpmnParser.ACTIVITI_BPMN_EXTENSIONS_NS);
266-
public static final Namespace FLUXNOVA_BPMN_EXTENSIONS_NS = new Namespace(BpmnParser.FLUXNOVA_BPMN_EXTENSIONS_NS);
266+
// Fluxnova extensions namespace, falling back to the legacy Camunda namespace for backwards compatibility
267+
public static final Namespace FLUXNOVA_BPMN_EXTENSIONS_NS = new Namespace(BpmnParser.FLUXNOVA_BPMN_EXTENSIONS_NS, BpmnParser.CAMUNDA_BPMN_EXTENSIONS_NS);
267268
public static final Namespace XSI_NS = new Namespace(BpmnParser.XSI_NS);
268269
public static final Namespace BPMN_DI_NS = new Namespace(BpmnParser.BPMN_DI_NS);
269270
public static final Namespace OMG_DI_NS = new Namespace(BpmnParser.OMG_DI_NS);
@@ -4250,8 +4251,7 @@ else if (strictValidation && target != null && target.isEmpty()) {
42504251
}
42514252
parameter.setTarget(target);
42524253

4253-
String restricted = parameterElement.attribute("restricted");
4254-
if (restricted != null && Boolean.parseBoolean(restricted.trim())) {
4254+
if (BpmnParseUtil.isRestricted(parameterElement)) {
42554255
parameter.setRestricted(true);
42564256
}
42574257
}

‎engine/src/main/java/org/finos/fluxnova/bpm/engine/impl/bpmn/parser/BpmnParseUtil.java‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,15 @@ public static void parseFluxnovaOutputParameters(Element inputOutputElement, IoM
113113
}
114114

115115
/**
116-
* Extracts the restricted attribute from a BPMN element.
116+
* Extracts the restricted attribute from a BPMN element. The attribute is read from the Fluxnova
117+
* extensions namespace first, falling back to the (legacy) Camunda namespace and finally to a
118+
* non-namespaced attribute, for backwards compatibility with existing models.
117119
*/
118120
public static boolean isRestricted(Element element) {
119-
String restricted = element.attribute("restricted");
121+
String restricted = element.attributeNS(BpmnParse.FLUXNOVA_BPMN_EXTENSIONS_NS, "restricted");
122+
if (restricted == null) {
123+
restricted = element.attribute("restricted");
124+
}
120125
return restricted != null && Boolean.parseBoolean(restricted.trim());
121126
}
122127

@@ -135,14 +140,10 @@ public static void parseInputParameterElement(Element inputParameterElement, IoM
135140
}
136141

137142
ParameterValueProvider valueProvider = parseNestedParamValueProvider(inputParameterElement);
138-
// add parameter
139-
InputParameter inputParameter = new InputParameter(nameAttribute, valueProvider, isTransient);
140143

141-
if (isRestricted(inputParameterElement)) {
142-
inputParameter.setRestricted(true);
143-
}
144144
// add parameter
145-
ioMapping.addInputParameter(inputParameter);
145+
ioMapping.addInputParameter(
146+
new InputParameter(nameAttribute, valueProvider, isTransient, isRestricted(inputParameterElement)));
146147
}
147148

148149
/**
@@ -160,14 +161,10 @@ public static void parseOutputParameterElement(Element outputParameterElement, I
160161
}
161162

162163
ParameterValueProvider valueProvider = parseNestedParamValueProvider(outputParameterElement);
163-
OutputParameter outputParameter = new OutputParameter(nameAttribute, valueProvider, isTransient);
164-
165-
if (isRestricted(outputParameterElement)) {
166-
outputParameter.setRestricted(true);
167-
}
168164

169165
// add parameter
170-
ioMapping.addOutputParameter(outputParameter);
166+
ioMapping.addOutputParameter(
167+
new OutputParameter(nameAttribute, valueProvider, isTransient, isRestricted(outputParameterElement)));
171168
}
172169

173170
/**

‎engine/src/main/java/org/finos/fluxnova/bpm/engine/impl/bpmn/parser/BpmnParser.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,6 @@ public class BpmnParser extends Parser {
4747
public static final String BPMN_20_SCHEMA_LOCATION = "org/finos/fluxnova/bpm/engine/impl/bpmn/parser/BPMN20.xsd";
4848

4949
/**
50-
* The namespace of the Fluxnova custom BPMN extensions.
51-
*/
52-
public static final String FLUXNOVA_BPMN_EXTENSIONS_NS = "http://fluxnova.finos.org/schema/1.0/bpmn";
53-
54-
/**
5550
* The namespace of the camunda custom BPMN extensions.
5651
*/
5752
public static final String CAMUNDA_BPMN_EXTENSIONS_NS = "http://camunda.org/schema/1.0/bpmn";
@@ -63,6 +58,11 @@ public class BpmnParser extends Parser {
6358
@Deprecated
6459
public static final String ACTIVITI_BPMN_EXTENSIONS_NS = "http://activiti.org/bpmn";
6560

61+
/**
62+
* The namespace of the Fluxnova custom BPMN extensions.
63+
*/
64+
public static final String FLUXNOVA_BPMN_EXTENSIONS_NS = "http://fluxnova.finos.org/schema/1.0/bpmn";
65+
6666
/**
6767
* The namepace of the BPMN 2.0 diagram interchange elements.
6868
*/

‎engine/src/main/java/org/finos/fluxnova/bpm/engine/impl/core/variable/mapping/InputParameter.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.finos.fluxnova.bpm.engine.impl.core.variable.scope.AbstractVariableScope;
2222
import org.finos.fluxnova.bpm.engine.impl.variable.InternalVariableContext;
2323
import org.finos.fluxnova.bpm.engine.variable.VariableOptions;
24-
import org.finos.fluxnova.bpm.engine.variable.Variables;
2524

2625
/**
2726
*
@@ -47,6 +46,10 @@ public InputParameter(String name, ParameterValueProvider valueProvider, boolean
4746
super(name, valueProvider, isTransient);
4847
}
4948

49+
public InputParameter(String name, ParameterValueProvider valueProvider, boolean isTransient, boolean restricted) {
50+
super(name, valueProvider, isTransient, restricted);
51+
}
52+
5053
protected void execute(AbstractVariableScope innerScope, AbstractVariableScope outerScope) {
5154

5255
// get value from outer scope
@@ -57,11 +60,7 @@ protected void execute(AbstractVariableScope innerScope, AbstractVariableScope o
5760
LOG.debugMappingValueFromOuterScopeToInnerScope(value,outerScope, name, innerScope);
5861

5962
// set variable in inner scope
60-
if(getIsTransient()) {
61-
innerScope.setVariableLocal(name, Variables.untypedValue(value, true), VariableOptions.options(false, restricted));
62-
} else {
63-
innerScope.setVariableLocal(name, value,VariableOptions.options(false, restricted));
64-
}
63+
innerScope.setVariableLocal(name, value, VariableOptions.options(getIsTransient(), restricted));
6564
}
6665

6766
}

‎engine/src/main/java/org/finos/fluxnova/bpm/engine/impl/core/variable/mapping/IoParameter.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,14 @@ public IoParameter(String name, ParameterValueProvider valueProvider) {
5050
}
5151

5252
public IoParameter(String name, ParameterValueProvider valueProvider, boolean isTransient) {
53+
this(name, valueProvider, isTransient, false);
54+
}
55+
56+
public IoParameter(String name, ParameterValueProvider valueProvider, boolean isTransient, boolean restricted) {
5357
this.name = name;
5458
this.valueProvider = valueProvider;
5559
this.isTransient = isTransient;
60+
this.restricted = restricted;
5661
}
5762

5863
/**

‎engine/src/main/java/org/finos/fluxnova/bpm/engine/impl/core/variable/mapping/OutputParameter.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import org.finos.fluxnova.bpm.engine.impl.core.CoreLogger;
2020
import org.finos.fluxnova.bpm.engine.impl.core.variable.mapping.value.ParameterValueProvider;
2121
import org.finos.fluxnova.bpm.engine.impl.core.variable.scope.AbstractVariableScope;
22-
import org.finos.fluxnova.bpm.engine.variable.Variables;
2322
import org.finos.fluxnova.bpm.engine.impl.variable.InternalVariableContext;
2423
import org.finos.fluxnova.bpm.engine.variable.VariableOptions;
2524

@@ -48,6 +47,10 @@ public OutputParameter(String name, ParameterValueProvider valueProvider, boolea
4847
super(name, valueProvider, isTransient);
4948
}
5049

50+
public OutputParameter(String name, ParameterValueProvider valueProvider, boolean isTransient, boolean restricted) {
51+
super(name, valueProvider, isTransient, restricted);
52+
}
53+
5154
protected void execute(AbstractVariableScope innerScope, AbstractVariableScope outerScope) {
5255

5356
// get value from inner scope
@@ -58,11 +61,7 @@ protected void execute(AbstractVariableScope innerScope, AbstractVariableScope o
5861
LOG.debugMappingValuefromInnerScopeToOuterScope(value, innerScope, name, outerScope);
5962

6063
// set variable in outer scope
61-
if(getIsTransient()) {
62-
outerScope.setVariable(name, Variables.untypedValue(value, true), VariableOptions.options(false, restricted));
63-
} else {
64-
outerScope.setVariable(name, value, VariableOptions.options(false, restricted));
65-
}
64+
outerScope.setVariable(name, value, VariableOptions.options(getIsTransient(), restricted));
6665
}
6766

6867
}

‎engine/src/main/java/org/finos/fluxnova/bpm/engine/impl/core/variable/scope/AbstractVariableScope.java‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -330,13 +330,21 @@ public void removeVariablesLocal(Collection<String> variableNames) {
330330
}
331331

332332
private TypedValue toTypedValue(Object value, boolean restricted) {
333-
boolean isTransient = value instanceof TypedValue && ((TypedValue) value).isTransient();
333+
return toTypedValue(value, false, restricted);
334+
}
335+
336+
private TypedValue toTypedValue(Object value, boolean isTransient, boolean restricted) {
337+
boolean finalTransient = isTransient || (value instanceof TypedValue && ((TypedValue) value).isTransient());
334338
boolean finalRestricted = restricted || (value instanceof TypedValue && ((TypedValue) value).isRestricted());
335-
return Variables.untypedValue(value, isTransient, finalRestricted);
339+
return Variables.untypedValue(value, finalTransient, finalRestricted);
336340
}
337341

338342
public void setVariableInternal(String variableName, Object value, boolean skipJavaSerializationFormatCheck, boolean restricted) {
339-
TypedValue typedValue = toTypedValue(value, restricted);
343+
setVariableInternal(variableName, value, false, skipJavaSerializationFormatCheck, restricted);
344+
}
345+
346+
public void setVariableInternal(String variableName, Object value, boolean isTransient, boolean skipJavaSerializationFormatCheck, boolean restricted) {
347+
TypedValue typedValue = toTypedValue(value, isTransient, restricted);
340348
setVariable(variableName, typedValue, getSourceActivityVariableScope(), skipJavaSerializationFormatCheck);
341349
}
342350

@@ -347,7 +355,7 @@ public void setVariable(String variableName, Object value, boolean skipJavaSeria
347355

348356
@Override
349357
public void setVariable(String variableName, Object value, VariableOptions options) {
350-
setVariableInternal(variableName, value, options.shouldSkipJavaSerializationFormatCheck(), options.isRestricted());
358+
setVariableInternal(variableName, value, options.isTransient(), options.shouldSkipJavaSerializationFormatCheck(), options.isRestricted());
351359
}
352360

353361
@Override
@@ -471,7 +479,11 @@ protected void invokeVariableLifecycleListenersUpdate(CoreVariableInstance varia
471479
}
472480

473481
public void setVariableLocalInternal(String variableName, Object value, boolean skipJavaSerializationFormatCheck, boolean restricted) {
474-
TypedValue typedValue = toTypedValue(value, restricted);
482+
setVariableLocalInternal(variableName, value, false, skipJavaSerializationFormatCheck, restricted);
483+
}
484+
485+
public void setVariableLocalInternal(String variableName, Object value, boolean isTransient, boolean skipJavaSerializationFormatCheck, boolean restricted) {
486+
TypedValue typedValue = toTypedValue(value, isTransient, restricted);
475487
setVariableLocal(variableName, typedValue, getSourceActivityVariableScope(), skipJavaSerializationFormatCheck);
476488
}
477489

@@ -482,7 +494,7 @@ public void setVariableLocal(String variableName, Object value, boolean skipJava
482494

483495
@Override
484496
public void setVariableLocal(String variableName, Object value, VariableOptions options) {
485-
setVariableLocalInternal(variableName, value, options.shouldSkipJavaSerializationFormatCheck(), options.isRestricted());
497+
setVariableLocalInternal(variableName, value, options.isTransient(), options.shouldSkipJavaSerializationFormatCheck(), options.isRestricted());
486498
}
487499

488500
@Override

‎engine/src/main/java/org/finos/fluxnova/bpm/engine/impl/variable/DefaultRestrictedVariableInterceptor.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.finos.fluxnova.bpm.engine.impl.interceptor.CommandContext;
1313
import org.finos.fluxnova.bpm.engine.impl.persistence.entity.AuthorizationManager;
1414
import org.finos.fluxnova.bpm.engine.runtime.VariableInstance;
15+
import org.finos.fluxnova.bpm.engine.variable.Variables;
1516
import org.finos.fluxnova.bpm.engine.variable.value.TypedValue;
1617

1718
/**
@@ -57,7 +58,9 @@ public TypedValue interceptUpdateVariable(CoreVariableInstance variableInstance,
5758
if (restricted
5859
&& hasNoEffectiveChange(variableInstance, newValue)
5960
&& isAuthorized(VariablePermissions.READ_RESTRICTED)) {
60-
return newValue;
61+
// Preserve the restricted flag: a resubmit that lost the metadata (e.g. a task form echoing
62+
// the value back as a plain value) must not silently un-restrict the variable.
63+
return Variables.untypedValue(newValue, newValue.isTransient(), true);
6164
}
6265

6366
checkAuthorization(VariablePermissions.UPDATE_RESTRICTED, restricted);

‎engine/src/test/java/org/finos/fluxnova/bpm/engine/test/api/authorization/task/RestrictedVariableTaskCompletionTest.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import org.finos.fluxnova.bpm.engine.AuthorizationException;
1616
import org.finos.fluxnova.bpm.engine.ProcessEngineConfiguration;
17+
import org.finos.fluxnova.bpm.engine.history.HistoricVariableInstance;
1718
import org.finos.fluxnova.bpm.engine.authorization.VariablePermissions;
1819
import org.finos.fluxnova.bpm.engine.runtime.ProcessInstance;
1920
import org.finos.fluxnova.bpm.engine.task.Task;
@@ -220,11 +221,13 @@ public void testCompleteAllowedOnResubmittingUnchangedRestrictedVariableWithRead
220221
// then
221222
assertNull(runtimeService.createProcessInstanceQuery().processInstanceId(processInstance.getId()).singleResult());
222223

223-
Object historicValue = runWithoutAuthorization(() -> historyService.createHistoricVariableInstanceQuery()
224-
.variableName(RESTRICTED_VARIABLE_NAME)
225-
.singleResult()
226-
.getValue());
227-
assertEquals(RESTRICTED_VARIABLE_VALUE, historicValue);
224+
HistoricVariableInstance historicVariable = runWithoutAuthorization(() ->
225+
historyService.createHistoricVariableInstanceQuery()
226+
.variableName(RESTRICTED_VARIABLE_NAME)
227+
.singleResult());
228+
assertEquals(RESTRICTED_VARIABLE_VALUE, historicVariable.getValue());
229+
// a resubmit that dropped the restriction tag must not un-restrict the variable
230+
assertTrue(historicVariable.isRestricted());
228231
}
229232

230233
@Test

‎engine/src/test/java/org/finos/fluxnova/bpm/engine/test/api/variables/RestrictedVariableTest.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,5 +173,22 @@ public void testSetRestrictedVariableUsingSerializedObjectBuilder() {
173173
assertTrue(variableInstance.isRestricted());
174174
}
175175

176+
@Test
177+
@Deployment(resources = {"org/finos/fluxnova/bpm/engine/test/api/oneTaskProcess.bpmn20.xml"})
178+
public void testUpdateRestrictedWithSameValuePreservesRestriction() {
179+
String processInstanceId = runtimeService.startProcessInstanceByKey("oneTaskProcess").getId();
180+
181+
runtimeService.setVariable(processInstanceId, "var", Variables.stringValue("v1", VariableOptions.options(false, true)));
182+
assertTrue(runtimeService.createVariableInstanceQuery().variableName("var").singleResult().isRestricted());
183+
184+
// Re-submit the unchanged value as a non-restricted typed value, as a task form / REST submit would
185+
// (same value and type, restricted flag dropped). The restriction must be preserved, not lost.
186+
runtimeService.setVariable(processInstanceId, "var", Variables.stringValue("v1"));
187+
VariableInstance variableInstance = runtimeService.createVariableInstanceQuery().variableName("var").singleResult();
188+
189+
assertTrue(variableInstance.isRestricted());
190+
assertEquals("v1", variableInstance.getValue());
191+
}
192+
176193
}
177194

0 commit comments

Comments
 (0)