Copilot commented on code in PR #6432:
URL:
https://github.com/apache/incubator-kie-drools/pull/6432#discussion_r2291069506
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/parser/feel11/ParserHelper.java:
##########
@@ -172,6 +172,7 @@ public void recoverScope( String name ) {
this.currentScope.define(new VariableSymbol( "year",
BuiltInType.NUMBER ));
this.currentScope.define(new VariableSymbol( "month",
BuiltInType.NUMBER ));
this.currentScope.define(new VariableSymbol( "day",
BuiltInType.NUMBER ));
+ this.currentScope.define(new VariableSymbol( "value",
BuiltInType.NUMBER ));
Review Comment:
The 'value' property is defined as NUMBER type for all temporal types, but
this may not be accurate for all cases. Consider if the return type should vary
based on the temporal type or if it should be a more generic type like ANY.
```suggestion
this.currentScope.define(new VariableSymbol(
"value", BuiltInType.ANY ));
```
##########
kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNRuntimeTest.java:
##########
@@ -3681,4 +3682,28 @@ void kieIssue270(boolean useExecModelCompiler) {
assertThat(dmnResult.getMessages()).extracting(DMNMessage::getText).contains("DMN:
Required dependency 'temperature' not found on node 'habitability' (DMN id:
_0699341C-A1BE-4B6D-B8D5-3972D67FCA45, The referenced node was not found) ",
"DMN: Required dependency 'oxygene' not found on node 'habitability' (DMN id:
_0699341C-A1BE-4B6D-B8D5-3972D67FCA45, The referenced node was not found) ");
}
+ @ParameterizedTest
+ @MethodSource("params")
+ void valuePropertyTest(boolean useExecModelCompiler) {
+ init(useExecModelCompiler);
+ final DMNRuntime runtime =
DMNRuntimeUtil.createRuntime("valid_models/DMNv1_6/TestValueProperty.dmn",
this.getClass());
+ final DMNModel dmnModel =
runtime.getModel("https://kie.org/dmn/_71C6EBC8-58FD-4917-A00D-3CFF5DF1C0D9";,
"DMN_8092A68F-7F00-44BA-8B59-9831ECE4EB8D");
+ assertThat(dmnModel).isNotNull();
+
assertThat(dmnModel.hasErrors()).as(DMNRuntimeUtil.formatMessages(dmnModel.getMessages())).isFalse();
+
+ final DMNContext dmnContext = DMNFactory.newContext();
+ dmnContext.set("InputA", LocalDate.of(2025,7,3));
+ dmnContext.set("InputB", ZonedDateTime.of(2025, 7, 8, 10, 0, 0, 0,
ZoneId.of("Z").normalized()));
+ dmnContext.set("InputC", Duration.of(1, ChronoUnit.DAYS).plusHours(1));
+ dmnContext.set("InputD", LocalTime.of(13, 20, 0));
+ dmnContext.set("InputE", Period.parse("P2Y1M"));
+
+ final DMNResult dmnResult = runtime.evaluateAll(dmnModel, dmnContext);
+
assertThat(dmnResult.getDecisionResultByName("TestDate").getResult()).isEqualTo(BigDecimal.valueOf(1751500800));
+
assertThat(dmnResult.getDecisionResultByName("TestDateAndTime").getResult()).isEqualTo(BigDecimal.valueOf(1751968800));
+
assertThat(dmnResult.getDecisionResultByName("TestDaysAndTime").getResult()).isEqualTo(BigDecimal.valueOf(90000));
+
assertThat(dmnResult.getDecisionResultByName("TestTime").getResult()).isEqualTo(BigDecimal.valueOf(48000));
+
assertThat(dmnResult.getDecisionResultByName("TestYearsAndMonths").getResult()).isEqualTo(BigDecimal.valueOf(25));
Review Comment:
The hardcoded expected values (1751500800, etc.) are magic numbers that lack
explanation. Consider adding comments to clarify what these values represent or
extract them to well-named constants.
```suggestion
assertThat(dmnResult.getDecisionResultByName("TestDate").getResult()).isEqualTo(EXPECTED_TEST_DATE);
assertThat(dmnResult.getDecisionResultByName("TestDateAndTime").getResult()).isEqualTo(EXPECTED_TEST_DATE_AND_TIME);
assertThat(dmnResult.getDecisionResultByName("TestDaysAndTime").getResult()).isEqualTo(EXPECTED_TEST_DAYS_AND_TIME);
assertThat(dmnResult.getDecisionResultByName("TestTime").getResult()).isEqualTo(EXPECTED_TEST_TIME);
assertThat(dmnResult.getDecisionResultByName("TestYearsAndMonths").getResult()).isEqualTo(EXPECTED_TEST_YEARS_AND_MONTHS);
```
##########
kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNRuntimeTest.java:
##########
@@ -3681,4 +3682,28 @@ void kieIssue270(boolean useExecModelCompiler) {
assertThat(dmnResult.getMessages()).extracting(DMNMessage::getText).contains("DMN:
Required dependency 'temperature' not found on node 'habitability' (DMN id:
_0699341C-A1BE-4B6D-B8D5-3972D67FCA45, The referenced node was not found) ",
"DMN: Required dependency 'oxygene' not found on node 'habitability' (DMN id:
_0699341C-A1BE-4B6D-B8D5-3972D67FCA45, The referenced node was not found) ");
}
+ @ParameterizedTest
+ @MethodSource("params")
+ void valuePropertyTest(boolean useExecModelCompiler) {
+ init(useExecModelCompiler);
+ final DMNRuntime runtime =
DMNRuntimeUtil.createRuntime("valid_models/DMNv1_6/TestValueProperty.dmn",
this.getClass());
+ final DMNModel dmnModel =
runtime.getModel("https://kie.org/dmn/_71C6EBC8-58FD-4917-A00D-3CFF5DF1C0D9";,
"DMN_8092A68F-7F00-44BA-8B59-9831ECE4EB8D");
+ assertThat(dmnModel).isNotNull();
+
assertThat(dmnModel.hasErrors()).as(DMNRuntimeUtil.formatMessages(dmnModel.getMessages())).isFalse();
+
+ final DMNContext dmnContext = DMNFactory.newContext();
+ dmnContext.set("InputA", LocalDate.of(2025,7,3));
+ dmnContext.set("InputB", ZonedDateTime.of(2025, 7, 8, 10, 0, 0, 0,
ZoneId.of("Z").normalized()));
Review Comment:
Using ZoneId.of("Z").normalized() is unnecessarily complex. Consider using
ZoneOffset.UTC directly for better readability and performance.
```suggestion
dmnContext.set("InputB", ZonedDateTime.of(2025, 7, 8, 10, 0, 0, 0,
ZoneOffset.UTC));
```
--
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]
