[incubator-kie-issues#854] DMN - Add implicit type conversion of date to date and time when necessary

[gitgabrio]

Add an implicit date to date and time conversion in DMN FEEL evaluation, based on this rule:
"When the type of the expression is date and the target type is date and time, the expression is converted to a date time value in which the time of day is UTC midnight (00:00:00)."

Acceptance criteria:

Implicit conversion is implemented in FEEL expressions, where date is used as the input type of the expression and the result needs to be date and time.
New code is 100% covered with tests.

Notes:

1. it is not possible to modify the FEELFunction itself because it does not receive the required type; so, conversion is executed inside DMNContextEvaluator
2. currently only LocalDate are transformed, and generated object is a ZonedDateTime
3. small refactoring of DMNContextEvaluator has been done to improve test coverage

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.

[kie-ci3]

PR job #1 was: UNSTABLE
Possible explanation: This should be test failures

Reproducer

build-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-drools/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-drools -u #5657 --skipParallelCheckout

NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution

Please look here: https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-5657/1/display/redirect

Test results:

• PASSED: 21874
• FAILED: 2

Those are the test failures:

org.kie.dmn.core.DMNStronglyTypedSupportTest.testDateAndTime[KIE_API_TYPECHECK]

expected:<...me.LocalDate), "from[]YearMonthDay"=1999-1...> but was:<...me.LocalDate), "from[StringToDateTime"=2015-12-24T00:00Z (java.time.ZonedDateTime), "from]YearMonthDay"=1999-1...>

org.kie.dmn.core.DMNStronglyTypedSupportTest.testDateAndTime[BUILDER_DEFAULT_NOCL_TYPECHECK]

expected:<...me.LocalDate), "from[]YearMonthDay"=1999-1...> but was:<...me.LocalDate), "from[StringToDateTime"=2015-12-24T00:00Z (java.time.ZonedDateTime), "from]YearMonthDay"=1999-1...>

[tarilabs]

The linked issue is private, so without context is difficult for me to provide a review.

I'm not sure either why the modification seems to be affecting only boxed context at the DMN level? What is strange to me is that this modification (if required) was not done at the FEEL level, only wiring where necessary at DMN level; based on the title alone I would have expected a modification at the FEEL level, with type check wirings at DMN level.

You may want to discuss this first with @baldimir to fully check the semantic aspects and implication with the DMN spec.

[gitgabrio]

@tarilabs
I add the ticket content in the description of this PR.

based on the title alone I would have expected a modification at the FEEL level,

yes, but I failed to find how to do that, and that's why I would ask you suggestion again 😄
Tibor currently is not available, but we have to keep going on in the meantime.
Debugging the code I found the FEELFunctions for date, but those does not receive other parameters to allow a different object return; and, IINW, the function to call is choosed automatically based on the expression itself, so maybe I'm missing some critical point

[tarilabs]

I really wish I could dedicate unlimited time, but I'm currently on sick leave.
The things I noticed, are:

A.
This seems a behavioural change introduced with DMNv1.5, I'd suggest first to bump the DMN engine to this level of spec conformance by parsing the metamodel, or (much less ideal) guard it under feature flag. That would also save you troubles of "old behaviour" version can be fix-pointed to the previous Drools releases. Myself and @baldimir discussed what bumping to DMNv1.5 entails during hand-over.

B.
Provided my understanding this is for DMNv1.5 is confirmed, this coercion seems to me applicable in 2 places:

• function invocation,
• and type-conformance in the DMN level.

if that is indeed the semantic discussed,

B.1.
For function invocation, you could consider to do similarly to:

incubator-kie-drools/kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/functions/BaseFEELFunction.java

Lines 239 to 261 in 9a3128e

	for ( int i = 0; i < parameterTypes.length; i++ ) {
	Class<?> currentIdxActualParameterType = cm.getActualClasses()[i];
	if ( currentIdxActualParameterType != null && !parameterTypes[i].isAssignableFrom( currentIdxActualParameterType ) ) {
	// singleton list spec defines that "a=[a]", i.e., singleton collections should be treated as the single element
	// and vice-versa
	if ( Collection.class.isAssignableFrom( currentIdxActualParameterType ) ) {
	Collection<?> valueCollection = (Collection<?>) actualParams[i];
	if ( valueCollection.size() == 1 ) {
	Object singletonValue = valueCollection.iterator().next();
	// re-perform the assignable-from check, this time using the element itself the singleton value from the original parameter list
	if ( singletonValue != null && parameterTypes[i].isAssignableFrom( singletonValue.getClass() ) ) {
	Object[] newParams = new Object[cm.getActualParams().length];
	System.arraycopy( cm.getActualParams(), 0, newParams, 0, cm.getActualParams().length ); // can't rely on adjustForVariableParameters() have actually copied
	newParams[i] = singletonValue;
	cm.setActualParams(newParams);
	continue;
	}
	}
	}
	found = false;
	break;
	}
	}

this coerces the actual parameter in the case of singleton lists.
You could consider similar, analogous strategy for the date->date and time coercion.
You will need to implement the coercion in the actual parameter being set in the CandidateMethod (eg actual parameters value as is, except for the coerced date -> date and time)

B.2.
For type-conformance in the DMN level, you could consider to do similarly to:

incubator-kie-drools/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNRuntimeImpl.java

Lines 675 to 687 in 9a3128e

	if (typeCheck && !d.getResultType().isAssignableValue(value)) {
	DMNMessage message = MsgUtil.reportMessage( logger,
	DMNMessage.Severity.ERROR,
	decision.getSource(),
	result,
	null,
	null,
	Msg.ERROR_EVAL_NODE_RESULT_WRONG_TYPE,
	getIdentifier( decision ),
	decision.getResultType(),
	value);
	reportFailure( dr, message, DMNDecisionResult.DecisionEvaluationStatus.FAILED );
	return false;

and

incubator-kie-drools/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNRuntimeImpl.java

Line 442 in 9a3128e

public static Object coerceUsingType(Object value, DMNType type, boolean typeCheck, BiConsumer<Object, DMNType> nullCallback) {

by introducing the additional coercion rule during type-conformance.

Hope this explain better why a modification in the DMN boxed Context does not seem the appropriate solution, but that's just me.
This is not a small task, so I would also suggest splitting it into subtasks.

I really hope this will help you.

[gitgabrio]

@tarilabs

[...]
I really hope this will help you.

Gosh, it really help a lot.
Sorry, I did not know you also were in sick leave, thanks again for all that help.
I'll start from what you wrote and see where I'll get to.
Thanks again and take care!

[kie-ci3]

PR job #2 was: UNSTABLE
Possible explanation: This should be test failures

Reproducer

build-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-drools/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-drools -u #5657 --skipParallelCheckout

NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution

Please look here: https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-5657/2/display/redirect

Test results:

• PASSED: 21874
• FAILED: 3

Those are the test failures:

org.kie.dmn.core.DMNRuntimeTest.testDateAndTime[false]

expected:<...me"=2015-12-24T00:00[Z (java.time.Zoned]DateTime), "fromYear...> but was:<...me"=2015-12-24T00:00[ (java.time.Local]DateTime), "fromYear...>

org.kie.dmn.core.DMNStronglyTypedSupportTest.testDateAndTime[KIE_API_TYPECHECK]

expected:<...me.LocalDate), "from[]YearMonthDay"=1999-1...> but was:<...me.LocalDate), "from[StringToDateTime"=2015-12-24T00:00 (java.time.LocalDateTime), "from]YearMonthDay"=1999-1...>

org.kie.dmn.core.DMNStronglyTypedSupportTest.testDateAndTime[BUILDER_DEFAULT_NOCL_TYPECHECK]

expected:<...me.LocalDate), "from[]YearMonthDay"=1999-1...> but was:<...me.LocalDate), "from[StringToDateTime"=2015-12-24T00:00 (java.time.LocalDateTime), "from]YearMonthDay"=1999-1...>