gitgabrio commented on code in PR #6325:
URL:
https://github.com/apache/incubator-kie-drools/pull/6325#discussion_r2065680567
##########
kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/ast/DMNConditionalEvaluatorTest.java:
##########
@@ -168,13 +184,18 @@ void manageBooleanOrNullIfResultWithNull() {
assertThat(conditionalEvaluationEvent.getExecutedId()).isEqualTo(ELSE_ELEMENT_ID);
}
+ private static DMNConditionalEvaluator[] dmnConditionEvaluators() {
Review Comment:
THanks @yesamer
Could you pls move this method to the bottom of the class ? Thanks!
##########
kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/ast/DMNConditionalEvaluatorTest.java:
##########
@@ -168,13 +184,18 @@ void manageBooleanOrNullIfResultWithNull() {
assertThat(conditionalEvaluationEvent.getExecutedId()).isEqualTo(ELSE_ELEMENT_ID);
}
+ private static DMNConditionalEvaluator[] dmnConditionEvaluators() {
+ return new DMNConditionalEvaluator[]
+ { dmnConditionalEvaluatorDecisionNode,
dmnConditionalEvaluatorBkmNode };
+ }
+
@Test
void testMapEvaluatorIdentifiers() {
Map<DMNConditionalEvaluator.EvaluatorType,
DMNConditionalEvaluator.EvaluatorIdentifier> mapEvaluatorIdentifiers =
DMNConditionalEvaluator.mapEvaluatorIdentifiers(EVALUATOR_ID_MAP);
-
assertThat(mapEvaluatorIdentifiers.get(DMNConditionalEvaluator.EvaluatorType.IF)).isEqualTo(ifIdentifier);
-
assertThat(mapEvaluatorIdentifiers.get(DMNConditionalEvaluator.EvaluatorType.THEN)).isEqualTo(thenIdentifier);
-
assertThat(mapEvaluatorIdentifiers.get(DMNConditionalEvaluator.EvaluatorType.ELSE)).isEqualTo(elseIdentifier);
+
assertThat(mapEvaluatorIdentifiers).containsEntry(DMNConditionalEvaluator.EvaluatorType.IF,
ifIdentifier)
Review Comment:
👍
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/ast/DMNConditionalEvaluator.java:
##########
@@ -101,8 +103,25 @@ static EvaluatorIdentifier
getEvaluatorIdentifier(Map<EvaluatorType, EvaluatorId
.orElseThrow(() -> new RuntimeException("Missing " + type + "
evaluator in evaluatorIdMap"));
}
- static String getDecisionName(DMNModelInstrumentedBase dmnElement) {
- return dmnElement instanceof Decision decision ? decision.getName() :
getDecisionName(dmnElement.getParentDRDElement());
+ /**
+ * Given a DMNModelInstrumentedBase element, it looks in the DMN hierarchy
the element related Decision node name or
+ * BusinessKnowledgeModel node name
+ * @param dmnElement
+ * @return
+ */
+ static String getDecisionOrBkmName(DMNModelInstrumentedBase dmnElement) {
+ if (dmnElement instanceof Decision decision) {
+ return decision.getName();
+ }
+ if (dmnElement instanceof BusinessKnowledgeModel
businessKnowledgeModel) {
+ return businessKnowledgeModel.getName();
+ }
+ if (dmnElement.getParentDRDElement() == null || dmnElement ==
dmnElement.getParentDRDElement()) {
+ logger.error("Root element id: {} reached. Can't find the related
Decision or BKM node name", dmnElement.getIdentifierString());
+ throw new IllegalStateException("Root element id: " +
dmnElement.getIdentifierString() + " reached. Can't find the related Decision
or BKM node name.");
Review Comment:
Do you know where this method is invoked ? I've the impression it would be
better/safer to log a warn/error and return some default instead of throwing
exception... it could break something downstream, potentially
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
