Skip to content

Commit

Permalink
merge: #8919
Browse files Browse the repository at this point in the history 
8919: Support decision table inputs and outputs without names/labels r=korthout a=korthout

## Description

<!-- Please explain the changes you made here. -->
A decision tables' inputs and outputs must have an id and can have a name. The name (or label) is not mandatory.

Even though the internal decision engine was already able to deal with this, the workflow engine wasn't. It would try to write the EvaluationEvent record with an EvaluatedInputRecord (or EvaluatedOutputRecord) and then fail writing it because of a NullPointerException.

This makes sure these records can be written when the name of the input or output is undefined.

## Related issues

<!-- Which issues are closed by this PR or are related -->

closes #8909



Co-authored-by: Nico Korthout <nico.korthout@camunda.com>
  • Loading branch information
@korthout
zeebe-bors-cloud[bot] and korthout committed Mar 21, 2022
2 parents b18da9b + fdc57c1 commit 5bf7322
Show file tree
Hide file tree
Showing 11 changed files with 449 additions and 36 deletions.
2 changes: 2 additions & 0 deletions dmn/src/main/java/io/camunda/zeebe/dmn/EvaluatedInput.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ public interface EvaluatedInput {
* Returns the name of the evaluated input. Note that it uses the label of the input in absence of
* the name. The label is usually the one that is displayed in the decision table.
*
* <p>If a label is defined, it is favored as the output name. Otherwise, the expression is used.
*
* @return the name of the evaluated input
*/
String inputName();
Expand Down
2 changes: 2 additions & 0 deletions dmn/src/main/java/io/camunda/zeebe/dmn/EvaluatedOutput.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public interface EvaluatedOutput {
* from the label. The label is usually the one that is displayed in the decision table. But the
* name is used in the decision output if the decision table has more than one output.
*
* <p>If a label is defined, it is favored as the output name. Otherwise, the name is used.
*
* @return the name of the evaluated output
*/
String outputName();
Expand Down
28 changes: 27 additions & 1 deletion dmn/src/main/java/io/camunda/zeebe/dmn/impl/EvaluatedDmnScalaInput.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,41 @@
import java.util.function.Function;
import org.agrona.DirectBuffer;
import org.camunda.dmn.Audit;
import org.camunda.dmn.parser.FeelExpression;
import org.camunda.dmn.parser.ParsedInput;
import org.camunda.feel.syntaxtree.Val;

public record EvaluatedDmnScalaInput(String inputId, String inputName, DirectBuffer inputValue)
implements EvaluatedInput {

private static final String MAX_EXPRESSION_LENGTH = "30";
private static final String TRUNCATE_EXPRESSION_TEMPLATE = "%." + MAX_EXPRESSION_LENGTH + "s";

public static EvaluatedDmnScalaInput of(
final Audit.EvaluatedInput evaluatedInput, final Function<Val, DirectBuffer> converter) {
final var input = evaluatedInput.input();
final var inputValue = evaluatedInput.value();
return new EvaluatedDmnScalaInput(input.id(), input.name(), converter.apply(inputValue));
final var inputName = determineInputName(input);
return new EvaluatedDmnScalaInput(input.id(), inputName, converter.apply(inputValue));
}

private static String determineInputName(final ParsedInput input) {
final String inputName;
if (input.name() != null) {
inputName = input.name();
} else if (input.expression() instanceof FeelExpression feelExpression) {
inputName = truncateExpression(feelExpression.expression().text());
} else {
inputName = null;
}

return inputName;
}

private static String truncateExpression(final String text) {
if (text == null) {
return null;
}
return TRUNCATE_EXPRESSION_TEMPLATE.formatted(text);
}
}
4 changes: 3 additions & 1 deletion dmn/src/main/java/io/camunda/zeebe/dmn/impl/EvaluatedDmnScalaOutput.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ public static EvaluatedDmnScalaOutput of(
final Audit.EvaluatedOutput evaluatedOutput, final Function<Val, DirectBuffer> converter) {
final var output = evaluatedOutput.output();
final var outputValue = evaluatedOutput.value();
return new EvaluatedDmnScalaOutput(output.id(), output.name(), converter.apply(outputValue));
// Just like the Modeler, we favor the label over the name
final var outputName = output.label() != null ? output.label() : output.name();
return new EvaluatedDmnScalaOutput(output.id(), outputName, converter.apply(outputValue));
}
}
101 changes: 83 additions & 18 deletions dmn/src/test/java/io/camunda/zeebe/dmn/DmnEvaluatedDecisionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,23 @@ void shouldReturnPartialOutputIfEvaluationFailsOnRequiredDecision() {
assertThat(evaluatedDecision.matchedRules()).hasSize(0);
}

private DecisionEvaluationResult evaluateDecision(
final String resource, final String decisionId) {
final var inputStream = getClass().getResourceAsStream(resource);
final var parsedDrg = decisionEngine.parse(inputStream);

// when
final var result = decisionEngine.evaluateDecisionById(parsedDrg, decisionId, null);

// then
assertThat(result.isFailure())
.describedAs(
"Expect that the decision is evaluated successfully: %s", result.getFailureMessage())
.isFalse();

return result;
}

@Nested
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@DisplayName("If successfully evaluated, the decision")
Expand All @@ -204,28 +221,12 @@ Stream<DecisionTest> decisions() {
new DecisionTest("relation", DecisionType.RELATION, "[{'is':'okay'}]"));
}

private DecisionEvaluationResult evaluateDecision(final String decisionId) {
final var inputStream = getClass().getResourceAsStream(DECISION_TYPES_DRG);
final var parsedDrg = decisionEngine.parse(inputStream);

// when
final var result = decisionEngine.evaluateDecisionById(parsedDrg, decisionId, null);

// then
assertThat(result.isFailure())
.describedAs(
"Expect that the decision is evaluated successfully", result.getFailureMessage())
.isFalse();

return result;
}

@ParameterizedTest
@MethodSource("decisions")
@DisplayName("Should return the type of the decision")
void shouldReturnTheDecisionType(final DecisionTest test) {
// when
final var result = evaluateDecision(test.decisionId);
final var result = evaluateDecision(DECISION_TYPES_DRG, test.decisionId);

// then
assertThat(result.getEvaluatedDecisions())
Expand All @@ -238,7 +239,7 @@ void shouldReturnTheDecisionType(final DecisionTest test) {
@DisplayName("Should return the output of the decision")
void shouldReturnDecisionResult(final DecisionTest test) {
// when
final var result = evaluateDecision(test.decisionId);
final var result = evaluateDecision(DECISION_TYPES_DRG, test.decisionId);

// then
assertThat(result.getEvaluatedDecisions())
Expand All @@ -248,4 +249,68 @@ void shouldReturnDecisionResult(final DecisionTest test) {

record DecisionTest(String decisionId, DecisionType expectedType, String expectedResult) {}
}

@Nested
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@DisplayName("If successfully evaluated, the decision table")
class DecisionTableTest {

// This drg contains different decision tables, each with a special case
private static final String DRG_DECISION_TABLE = "/drg-decision-table-io-names.dmn";

@Test
@DisplayName("Should use input label as input name, if label defined")
void shouldUseInputLabelAsInputName() {
// when
final var result = evaluateDecision(DRG_DECISION_TABLE, "labeled_input");

// then
assertThat(result.getEvaluatedDecisions())
.flatMap(EvaluatedDecision::evaluatedInputs)
.extracting(EvaluatedInput::inputName)
.containsExactly("input_label_is_used_as_input_name");
}

@Test
@DisplayName("Should use input expression as input name, if no label defined")
void shouldUseInputExpressionAsInputName() {
// when
final var result = evaluateDecision(DRG_DECISION_TABLE, "unlabeled_input");

// then
assertThat(result.getEvaluatedDecisions())
.flatMap(EvaluatedDecision::evaluatedInputs)
.extracting(EvaluatedInput::inputName)
// the expression is truncated at 30 chars when used as name
.containsExactly("\"expression is used as input n");
}

@Test
@DisplayName("Should use output label as output name, if label defined")
void shouldUseOutputLabelAsOutputName() {
// when
final var result = evaluateDecision(DRG_DECISION_TABLE, "labeled_output");

// then
assertThat(result.getEvaluatedDecisions())
.flatMap(EvaluatedDecision::matchedRules)
.flatMap(MatchedRule::evaluatedOutputs)
.extracting(EvaluatedOutput::outputName)
.containsExactly("output_label_is_used_as_output_name");
}

@Test
@DisplayName("Should use output name as output name, if no label defined")
void shouldUseOutputNameAsOutputName() {
// when
final var result = evaluateDecision(DRG_DECISION_TABLE, "unlabeled_output");

// then
assertThat(result.getEvaluatedDecisions())
.flatMap(EvaluatedDecision::matchedRules)
.flatMap(MatchedRule::evaluatedOutputs)
.extracting(EvaluatedOutput::outputName)
.containsExactly("output_name_is_used_as_output_name");
}
}
}
111 changes: 111 additions & 0 deletions dmn/src/test/resources/drg-decision-table-io-names.dmn
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
<?xml version="1.0" encoding="UTF-8"?>
<definitions xmlns="https://www.omg.org/spec/DMN/20191111/MODEL/" xmlns:dmndi="https://www.omg.org/spec/DMN/20191111/DMNDI/" xmlns:dc="http://www.omg.org/spec/DMN/20180521/DC/" xmlns:biodi="http://bpmn.io/schema/dmn/biodi/2.0" id="force_users" name="Force Users" namespace="http://camunda.org/schema/1.0/dmn" exporter="Camunda Modeler" exporterVersion="4.12.0">
<decision id="labeled_input" name="Decision Table">
<decisionTable id="DecisionTable_14n3bx1">
<input id="Input_1" label="input_label_is_used_as_input_name" biodi:width="192">
<inputExpression id="InputExpression_1" typeRef="boolean">
<text>true</text>
</inputExpression>
</input>
<output id="Output_1" label="Output" typeRef="string" biodi:width="192">
<outputValues id="UnaryTests_0hj3461">
<text></text>
</outputValues>
</output>
<rule id="DecisionRule_0zumzn1">
<inputEntry id="UnaryTests_0leuxq1">
<text>-</text>
</inputEntry>
<outputEntry id="LiteralExpression_0c9vpz1">
<text>"okay"</text>
</outputEntry>
</rule>
</decisionTable>
</decision>

<decision id="unlabeled_input" name="Decision Table">
<decisionTable id="DecisionTable_14n3bx2">
<input id="Input_2" biodi:width="192">
<inputExpression id="InputExpression_2" typeRef="boolean">
<text>"expression is used as input name" = "expression is used as input name"</text>
</inputExpression>
</input>
<output id="Output_2" label="Output" typeRef="string" biodi:width="192">
<outputValues id="UnaryTests_0hj3462">
<text></text>
</outputValues>
</output>
<rule id="DecisionRule_0zumzn2">
<inputEntry id="UnaryTests_0leuxq2">
<text>-</text>
</inputEntry>
<outputEntry id="LiteralExpression_0c9vpz2">
<text>"okay"</text>
</outputEntry>
</rule>
</decisionTable>
</decision>

<decision id="labeled_output" name="Decision Table">
<decisionTable id="DecisionTable_14n3bx3">
<input id="Input_3" biodi:width="192">
<inputExpression id="InputExpression_3" typeRef="boolean">
<text>true</text>
</inputExpression>
</input>
<output id="Output_3" name="Output" label="output_label_is_used_as_output_name" typeRef="string" biodi:width="192">
<outputValues id="UnaryTests_0hj3463">
<text></text>
</outputValues>
</output>
<rule id="DecisionRule_0zumzn3">
<inputEntry id="UnaryTests_0leuxq3">
<text>-</text>
</inputEntry>
<outputEntry id="LiteralExpression_0c9vpz3">
<text>"okay"</text>
</outputEntry>
</rule>
</decisionTable>
</decision>

<decision id="unlabeled_output" name="Decision Table">
<decisionTable id="DecisionTable_14n3bx4">
<input id="Input_4" biodi:width="192">
<inputExpression id="InputExpression_4" typeRef="boolean">
<text>true</text>
</inputExpression>
</input>
<output id="Output_4" name="output_name_is_used_as_output_name" typeRef="string" biodi:width="192">
<outputValues id="UnaryTests_0hj3464">
<text></text>
</outputValues>
</output>
<rule id="DecisionRule_0zumzn4">
<inputEntry id="UnaryTests_0leuxq4">
<text>-</text>
</inputEntry>
<outputEntry id="LiteralExpression_0c9vpz4">
<text>"okay"</text>
</outputEntry>
</rule>
</decisionTable>
</decision>

<dmndi:DMNDI>
<dmndi:DMNDiagram>
<dmndi:DMNShape dmnElementRef="labeled_input">
<dc:Bounds height="80" width="180" x="160" y="100" />
</dmndi:DMNShape>
<dmndi:DMNShape id="DMNShape_0oy0qd3" dmnElementRef="unlabeled_input">
<dc:Bounds height="80" width="180" x="380" y="100" />
</dmndi:DMNShape>
<dmndi:DMNShape id="DMNShape_0oy0qd4" dmnElementRef="labeled_output">
<dc:Bounds height="80" width="180" x="600" y="100" />
</dmndi:DMNShape>
<dmndi:DMNShape id="DMNShape_0oy0qd5" dmnElementRef="unlabeled_output">
<dc:Bounds height="80" width="180" x="820" y="100" />
</dmndi:DMNShape>
</dmndi:DMNDiagram>
</dmndi:DMNDI>
</definitions>
32 changes: 20 additions & 12 deletions .../src/main/java/io/camunda/zeebe/engine/processing/bpmn/behavior/BpmnDecisionBehavior.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,16 @@ private void addDecisionToEvaluationEvent(

private void addInputToEvaluationEvent(
final EvaluatedInput evaluatedInput, final EvaluatedDecisionRecord evaluatedDecisionRecord) {
evaluatedDecisionRecord
.evaluatedInputs()
.add()
.setInputId(evaluatedInput.inputId())
.setInputName(evaluatedInput.inputName())
.setInputValue(evaluatedInput.inputValue());
final var inputRecord =
evaluatedDecisionRecord
.evaluatedInputs()
.add()
.setInputId(evaluatedInput.inputId())
.setInputValue(evaluatedInput.inputValue());

if (evaluatedInput.inputName() != null) {
inputRecord.setInputName(evaluatedInput.inputName());
}
}

private void addMatchedRuleToEvaluationEvent(
Expand All @@ -311,12 +315,16 @@ private void addMatchedRuleToEvaluationEvent(

private void addOutputToEvaluationEvent(
final EvaluatedOutput evaluatedOutput, final MatchedRuleRecord matchedRuleRecord) {
matchedRuleRecord
.evaluatedOutputs()
.add()
.setOutputId(evaluatedOutput.outputId())
.setOutputName(evaluatedOutput.outputName())
.setOutputValue(evaluatedOutput.outputValue());
final var outputRecord =
matchedRuleRecord
.evaluatedOutputs()
.add()
.setOutputId(evaluatedOutput.outputId())
.setOutputValue(evaluatedOutput.outputValue());

if (evaluatedOutput.outputName() != null) {
outputRecord.setOutputName(evaluatedOutput.outputName());
}
}

private record DecisionInfo(long key, int version) {
Expand Down
Loading

0 comments on commit 5bf7322

Please sign in to comment.