[incubator-kie-issues#506] DMN - Add support for iteration of date ranges #5626

gitgabrio · 2023-12-15T08:08:27Z

This PR

implement iteration over local data
refactor ForIterationNode to allow different kind of iteratods
remove duplicated code

Thank you for submitting this pull request

NOTE!: Double check the target branch for this PR.
The default is main so it will target Drools 8 / Kogito.
If this PR is not strictly related to drools and kogito project in drools.git, it should probably target 7.xas a branch

Ports If a forward-port or a backport is needed, paste the forward port PR here

link

JIRA: (please edit the JIRA link if it exists)

link

referenced Pull Requests: (please edit the URLs of referenced pullrequests if they exist)

paste the link(s) from GitHub here
link 2
link 3 etc.

How to replicate CI configuration locally?

Build Chain tool does "simple" maven build(s), the builds are just Maven commands, but because the repositories relates and depends on each other and any change in API or class method could affect several of those repositories there is a need to use build-chain tool to handle cross repository builds and be sure that we always use latest version of the code for each repository.

build-chain tool is a build tool which can be used on command line locally or in Github Actions workflow(s), in case you need to change multiple repositories and send multiple dependent pull requests related with a change you can easily reproduce the same build by executing it on Github hosted environment or locally in your development environment. See local execution details to get more information about it.

How to retest this PR or trigger a specific build:

for pull request checks
Please add comment: Jenkins retest this
for a specific pull request check
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] tests
for a full downstream build
- for jenkins job: please add comment: Jenkins run fdb
- for github actions job: add the label run_fdb
a compile downstream build please add comment: Jenkins run cdb
an upstream build please add comment: Jenkins run upstream
for quarkus branch checks
Run checks against Quarkus current used branch
Please add comment: Jenkins run quarkus-branch
for a quarkus branch specific check
Run checks against Quarkus current used branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] quarkus-branch
for quarkus main checks
Run checks against Quarkus main branch
Please add comment: Jenkins run quarkus-main
for a specific quarkus main check
Run checks against Quarkus main branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] quarkus-main
for quarkus lts checks
Run checks against Quarkus lts branch
Please add comment: Jenkins run quarkus-lts
for a specific quarkus lts check
Run checks against Quarkus lts branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] quarkus-lts
for native checks
Run native checks
Please add comment: Jenkins run native
for a specific native check
Run native checks
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] native
for native lts checks
Run native checks against quarkus lts branch
Please add comment: Jenkins run native-lts
for a specific native lts check
Run native checks against quarkus lts branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] native-lts

How to backport a pull request to a different branch?

In order to automatically create a backporting pull request please add one or more labels having the following format backport-<branch-name>, where <branch-name> is the name of the branch where the pull request must be backported to (e.g., backport-7.67.x to backport the original PR to the 7.67.x branch).

NOTE: backporting is an action aiming to move a change (usually a commit) from a branch (usually the main one) to another one, which is generally referring to a still maintained release branch. Keeping it simple: it is about to move a specific change or a set of them from one branch to another.

Once the original pull request is successfully merged, the automated action will create one backporting pull request per each label (with the previous format) that has been added.

If something goes wrong, the author will be notified and at this point a manual backporting is needed.

NOTE: this automated backporting is triggered whenever a pull request on main branch is labeled or closed, but both conditions must be satisfied to get the new PR created.

…remove duplicated code and allow easy extension of use cases

# Conflicts: # kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/infixexecutors/AddExecutor.java

gitgabrio · 2023-12-20T13:16:35Z

@mariofusco @baldimir
Benchamarks
before PR
results_main.csv
after PR
results_506.csv

Keep in mind that now the code has to do something more then before

mariofusco · 2024-01-08T11:35:16Z

kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/codegen/feel11/CompiledFEELSupport.java

@@ -64,8 +56,14 @@
 import org.kie.dmn.feel.util.Msg;
 import org.kie.dmn.feel.util.MsgUtil;

+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.util.*;


Nitpick: please let's avoid wildcard imports.

mariofusco · 2024-01-08T11:36:51Z

...-dmn-feel/src/main/java/org/kie/dmn/feel/exceptions/EndpointOfRangeNotOfNumberException.java

+package org.kie.dmn.feel.exceptions;
+
+public class EndpointOfRangeNotOfNumberException extends RuntimeException {
+    private static final long serialVersionUID = 1L;


Is this serialVersionUID necessary? Why?

@mariofusco tbh this class is a refactoring - it was in the original one, anyway I'll remove...

mariofusco · 2024-01-08T11:41:13Z

...l/src/main/java/org/kie/dmn/feel/lang/ast/forexpressioniterators/LocalDateRangeIterator.java

+
+public class LocalDateRangeIterator implements Iterator<LocalDate> {
+
+    private enum Direction {


Does it make sense to have the same enum repeated for all Iterator implementations? Wouldn't it be better to convert it into a top level class and reuse it everywhere?

mariofusco · 2024-01-08T11:41:54Z

.../test/java/org/kie/dmn/feel/lang/ast/forexpressioniterators/BigDecimalRangeIteratorTest.java

+import java.util.List;
+import java.util.stream.IntStream;
+
+import static org.junit.Assert.*;


Wildcard import.

mariofusco · 2024-01-08T11:43:42Z

kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/Msg.java

@@ -38,7 +38,7 @@ public final class Msg {
    public static final Message2 INDEX_OUT_OF_BOUND = new Message2("Index out of bound: list of %s elements, index %s; will evaluate as FEEL null");
    public static final Message2 X_TYPE_INCOMPATIBLE_WITH_Y_TYPE = new Message2("%s type incompatible with %s type");
    public static final Message1 INCOMPATIBLE_TYPE_FOR_RANGE = new Message1("Type %s can not be used in a range unary test");
-    public static final Message1 VALUE_X_NOT_A_VALID_ENDPOINT_FOR_RANGE_BECAUSE_NOT_A_NUMBER = new Message1("Value %s is not a valid endpoint for range, because not a feel:number");
+    public static final Message1 VALUE_X_NOT_A_VALID_ENDPOINT_FOR_RANGE_BECAUSE_NOT_A_NUMBER_NOT_A_DATE = new Message1("Value %s is not a valid endpoint for range, because not a feel:number nor a feel:date");


Reword error message with: "because neither a feel:number nor a feel:date"

mariofusco · 2024-01-08T11:44:22Z

...mn-feel/src/test/java/org/kie/dmn/feel/lang/ast/forexpressioniterators/ForIterationTest.java

+import java.util.List;
+import java.util.stream.IntStream;
+
+import static org.junit.Assert.*;


Wildcard import.

mariofusco · 2024-01-08T11:44:44Z

...el/src/test/java/org/kie/dmn/feel/lang/ast/forexpressioniterators/ForIterationUtilsTest.java

+import static org.kie.dmn.feel.codegen.feel11.CodegenTestUtil.newEmptyEvaluationContext;
+import static org.kie.dmn.feel.lang.ast.forexpressioniterators.ForIterationUtils.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.*;


Wildcard import.

mariofusco · 2024-01-08T11:45:02Z

...c/test/java/org/kie/dmn/feel/lang/ast/forexpressioniterators/LocalDateRangeIteratorTest.java

+import java.util.List;
+import java.util.stream.IntStream;
+
+import static org.junit.Assert.*;


Wildcard import.

mariofusco · 2024-01-08T11:45:22Z

...st/java/org/kie/dmn/feel/lang/ast/forexpressioniterators/ZonedDateTimeRangeIteratorTest.java

+import java.util.List;
+import java.util.stream.IntStream;
+
+import static org.junit.Assert.*;


Wildcard import.

gitgabrio · 2024-01-08T12:09:28Z

@mariofusco I think I fix all your suggestion.
Here's updated benchmarks:
main branch
this PR

…nges (apache#5626) * [incubator-kie-issues#506] Implemented Date iteration. Refactored to remove duplicated code and allow easy extension of use cases * [incubator-kie-issues#506] WIP * [incubator-kie-issues#506] Update with main. Add unit tests * [incubator-kie-issues#506] WIP * [incubator-kie-issues#506] Restored code-level 11 syntax * [incubator-kie-issues#506] Merged with main * [incubator-kie-issues#506] Fix as per PR suggestion --------- Co-authored-by: BAMOE CI <[email protected]>

[incubator-kie-issues#506] Implemented Date iteration. Refactored to …

9c66210

…remove duplicated code and allow easy extension of use cases

gitgabrio requested review from baldimir and mariofusco December 15, 2023 08:08

[incubator-kie-issues#506] WIP

9cc63d8

mariofusco approved these changes Dec 20, 2023

View reviewed changes

BAMOE CI added 5 commits December 20, 2023 10:32

Merge remote-tracking branch 'origin/main' into incubator-kie-issues#506

8660d41

# Conflicts: # kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/infixexecutors/AddExecutor.java

[incubator-kie-issues#506] Update with main. Add unit tests

33fac3d

Merge remote-tracking branch 'origin/main' into incubator-kie-issues#506

f4bb649

[incubator-kie-issues#506] WIP

c9f7ffd

[incubator-kie-issues#506] Restored code-level 11 syntax

3f61b9a

BAMOE CI added 3 commits January 8, 2024 11:05

Merge remote-tracking branch 'origin/main' into incubator-kie-issues#506

dd1daf9

[incubator-kie-issues#506] Merged with main

aaeaf52

Merge remote-tracking branch 'origin/main' into incubator-kie-issues#506

b4f7eb2

mariofusco reviewed Jan 8, 2024

View reviewed changes

[incubator-kie-issues#506] Fix as per PR suggestion

450b848

mariofusco approved these changes Jan 8, 2024

View reviewed changes

gitgabrio requested a review from yesamer January 8, 2024 13:18

gitgabrio merged commit e1fc1b4 into apache:main Jan 9, 2024
9 checks passed

gitgabrio deleted the incubator-kie-issues#506 branch January 9, 2024 08:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[incubator-kie-issues#506] DMN - Add support for iteration of date ranges #5626

[incubator-kie-issues#506] DMN - Add support for iteration of date ranges #5626

gitgabrio commented Dec 15, 2023

gitgabrio commented Dec 20, 2023 •

edited

Loading

mariofusco Jan 8, 2024

gitgabrio Jan 8, 2024

mariofusco Jan 8, 2024

gitgabrio Jan 8, 2024

mariofusco Jan 8, 2024

gitgabrio Jan 8, 2024

mariofusco Jan 8, 2024

gitgabrio Jan 8, 2024

mariofusco Jan 8, 2024

gitgabrio Jan 8, 2024

mariofusco Jan 8, 2024

mariofusco Jan 8, 2024

gitgabrio Jan 8, 2024

mariofusco Jan 8, 2024

gitgabrio Jan 8, 2024

mariofusco Jan 8, 2024

gitgabrio Jan 8, 2024

gitgabrio commented Jan 8, 2024


		public class LocalDateRangeIterator implements Iterator<LocalDate> {

		private enum Direction {

[incubator-kie-issues#506] DMN - Add support for iteration of date ranges #5626

[incubator-kie-issues#506] DMN - Add support for iteration of date ranges #5626

Conversation

gitgabrio commented Dec 15, 2023

gitgabrio commented Dec 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitgabrio commented Jan 8, 2024

gitgabrio commented Dec 20, 2023 •

edited

Loading