Skip to content

Commit bd8b5d5

Browse files
tijsrademakerstijsrademakers
committed
Fix validation context sharing
1 parent c92f1b7 commit bd8b5d5

File tree

4 files changed

+128
-8
lines changed

4 files changed

+128
-8
lines changed

‎modules/flowable-case-validation/src/main/java/org/flowable/cmmn/validation/CaseValidatorImpl.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,26 @@ public List<ValidationEntry> validate(CmmnModel model) {
3939
public List<ValidationEntry> validate(CmmnModel model, CaseValidationContext validationContext) {
4040
List<ValidationEntry> allEntries = new ArrayList<>();
4141

42-
CaseValidationContext currentValidationContext = validationContext;
4342
for (ValidatorSet validatorSet : validatorSets) {
44-
if (currentValidationContext == null) {
43+
CaseValidationContext currentValidationContext;
44+
if (validationContext == null) {
4545
currentValidationContext = new CaseValidationContextImpl(validatorSet);
46-
4746
} else {
47+
currentValidationContext = validationContext;
4848
currentValidationContext.setCurrentValidatorSet(validatorSet);
4949
}
5050

5151
for (Validator validator : validatorSet.getValidators()) {
5252
validator.validate(model, currentValidationContext);
5353
}
5454

55-
allEntries.addAll(currentValidationContext.getEntries());
55+
if (validationContext == null) {
56+
allEntries.addAll(currentValidationContext.getEntries());
57+
}
58+
}
59+
60+
if (validationContext != null) {
61+
allEntries.addAll(validationContext.getEntries());
5662
}
5763

5864
return allEntries;

‎modules/flowable-case-validation/src/test/java/org/flowable/cmmn/validation/CaseValidatorImplTest.java‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/* Licensed under the Apache License, Version 2.0 (the "License");
2+
* you may not use this file except in compliance with the License.
3+
* You may obtain a copy of the License at
4+
*
5+
* http://www.apache.org/licenses/LICENSE-2.0
6+
*
7+
* Unless required by applicable law or agreed to in writing, software
8+
* distributed under the License is distributed on an "AS IS" BASIS,
9+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
* See the License for the specific language governing permissions and
11+
* limitations under the License.
12+
*/
13+
package org.flowable.cmmn.validation;
14+
15+
import static org.assertj.core.api.Assertions.assertThat;
16+
import static org.assertj.core.api.Assertions.tuple;
17+
18+
import java.util.List;
19+
20+
import org.flowable.cmmn.model.CmmnModel;
21+
import org.flowable.cmmn.validation.validator.ValidationEntry;
22+
import org.flowable.cmmn.validation.validator.ValidatorSet;
23+
import org.junit.jupiter.api.Test;
24+
25+
/**
26+
* @author Tijs Rademakers
27+
*/
28+
class CaseValidatorImplTest {
29+
30+
@Test
31+
void testValidateWithSharedContextDoesNotDuplicateEntries() {
32+
CaseValidatorImpl validator = new CaseValidatorImpl();
33+
34+
ValidatorSet set1 = new ValidatorSet("set1");
35+
set1.addValidator((model, validationContext) -> validationContext.addError("PROBLEM_A", "Error from set 1"));
36+
37+
ValidatorSet set2 = new ValidatorSet("set2");
38+
set2.addValidator((model, validationContext) -> validationContext.addError("PROBLEM_B", "Error from set 2"));
39+
40+
validator.addValidatorSet(set1);
41+
validator.addValidatorSet(set2);
42+
43+
CmmnModel cmmnModel = new CmmnModel();
44+
45+
// Validate with a shared external context
46+
CaseValidationContextImpl sharedContext = new CaseValidationContextImpl(set1);
47+
List<ValidationEntry> entries = validator.validate(cmmnModel, sharedContext);
48+
49+
// Must be exactly 2 entries (1 per set), not 3 which would happen
50+
// if the cumulative getEntries() list was re-added each iteration
51+
assertThat(entries)
52+
.extracting(ValidationEntry::getProblem, ValidationEntry::getValidatorSetName)
53+
.containsExactly(
54+
tuple("PROBLEM_A", "set1"),
55+
tuple("PROBLEM_B", "set2")
56+
);
57+
58+
// Verify no-context path produces the same result count
59+
List<ValidationEntry> entriesWithoutContext = validator.validate(cmmnModel, null);
60+
assertThat(entriesWithoutContext)
61+
.extracting(ValidationEntry::getProblem, ValidationEntry::getValidatorSetName)
62+
.containsExactly(
63+
tuple("PROBLEM_A", "set1"),
64+
tuple("PROBLEM_B", "set2")
65+
);
66+
}
67+
}

‎modules/flowable-engine/src/test/java/org/flowable/standalone/validation/DefaultProcessValidatorTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,13 @@
3030
import org.flowable.bpmn.model.BpmnModel;
3131
import org.flowable.common.engine.api.io.InputStreamProvider;
3232
import org.flowable.engine.test.util.TestProcessUtil;
33+
import org.flowable.validation.ProcessValidationContextImpl;
3334
import org.flowable.validation.ProcessValidator;
3435
import org.flowable.validation.ProcessValidatorFactory;
36+
import org.flowable.validation.ProcessValidatorImpl;
3537
import org.flowable.validation.ValidationError;
3638
import org.flowable.validation.validator.Problems;
39+
import org.flowable.validation.validator.ValidatorSet;
3740
import org.flowable.validation.validator.ValidatorSetNames;
3841
import org.junit.jupiter.api.BeforeEach;
3942
import org.junit.jupiter.api.Test;
@@ -487,6 +490,44 @@ protected List<ValidationError> findErrors(List<ValidationError> errors, String
487490
return results;
488491
}
489492

493+
@Test
494+
public void testValidateWithSharedContextDoesNotDuplicateErrors() {
495+
ProcessValidatorImpl validator = new ProcessValidatorImpl();
496+
497+
ValidatorSet set1 = new ValidatorSet("set1");
498+
set1.addValidator((bpmnModel, validationContext) -> validationContext.addError("PROBLEM_A", "Error from set 1"));
499+
500+
ValidatorSet set2 = new ValidatorSet("set2");
501+
set2.addValidator((bpmnModel, validationContext) -> validationContext.addError("PROBLEM_B", "Error from set 2"));
502+
503+
validator.addValidatorSet(set1);
504+
validator.addValidatorSet(set2);
505+
506+
BpmnModel bpmnModel = new BpmnModel();
507+
508+
// Validate with a shared external context
509+
ProcessValidationContextImpl sharedContext = new ProcessValidationContextImpl(set1);
510+
List<ValidationError> errors = validator.validate(bpmnModel, sharedContext);
511+
512+
// Must be exactly 2 errors (1 per set), not 3 which would happen
513+
// if the cumulative getEntries() list was re-added each iteration
514+
assertThat(errors)
515+
.extracting(ValidationError::getProblem, ValidationError::getValidatorSetName)
516+
.containsExactly(
517+
tuple("PROBLEM_A", "set1"),
518+
tuple("PROBLEM_B", "set2")
519+
);
520+
521+
// Verify no-context path produces the same result count
522+
List<ValidationError> errorsWithoutContext = validator.validate(bpmnModel, null);
523+
assertThat(errorsWithoutContext)
524+
.extracting(ValidationError::getProblem, ValidationError::getValidatorSetName)
525+
.containsExactly(
526+
tuple("PROBLEM_A", "set1"),
527+
tuple("PROBLEM_B", "set2")
528+
);
529+
}
530+
490531
protected BpmnModel readBpmnModelFromXml(String resource) {
491532
InputStreamProvider xmlStream = () -> DefaultProcessValidatorTest.class.getClassLoader().getResourceAsStream(resource);
492533
return new BpmnXMLConverter().convertToBpmnModel(xmlStream, true, true);

‎modules/flowable-process-validation/src/main/java/org/flowable/validation/ProcessValidatorImpl.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,20 +35,26 @@ public List<ValidationError> validate(BpmnModel bpmnModel) {
3535
public List<ValidationError> validate(BpmnModel bpmnModel, ProcessValidationContext validationContext) {
3636
List<ValidationError> allErrors = new ArrayList<>();
3737

38-
ProcessValidationContext currentValidationContext = validationContext;;
3938
for (ValidatorSet validatorSet : validatorSets) {
40-
if (currentValidationContext == null) {
39+
ProcessValidationContext currentValidationContext;
40+
if (validationContext == null) {
4141
currentValidationContext = new ProcessValidationContextImpl(validatorSet);
42-
4342
} else {
43+
currentValidationContext = validationContext;
4444
currentValidationContext.setCurrentValidatorSet(validatorSet);
4545
}
4646

4747
for (Validator validator : validatorSet.getValidators()) {
4848
validator.validate(bpmnModel, currentValidationContext);
4949
}
5050

51-
allErrors.addAll(currentValidationContext.getEntries());
51+
if (validationContext == null) {
52+
allErrors.addAll(currentValidationContext.getEntries());
53+
}
54+
}
55+
56+
if (validationContext != null) {
57+
allErrors.addAll(validationContext.getEntries());
5258
}
5359

5460
return allErrors;

0 commit comments

Comments
 (0)