Copilot commented on code in PR #6303:
URL:
https://github.com/apache/incubator-kie-drools/pull/6303#discussion_r2166750734
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/functions/DateAndTimeFunction.java:
##########
@@ -62,10 +62,64 @@ public class DateAndTimeFunction
.toFormatter();
}
+
private DateAndTimeFunction() {
super(FEELConversionFunctionNames.DATE_AND_TIME);
}
+ static Optional<TemporalAccessor> getValidDate(TemporalAccessor date) {
+ if (date == null) {
+ return Optional.empty();
+ }
+ if (!(date instanceof LocalDate)) {
+ // FEEL Spec Table 58 "date is a date or date time [...] creates a
date time from the given date (ignoring any time component)" [that means
ignoring any TZ from `date` parameter, too]
+ // I try to convert `date` to a LocalDate, if the query method
returns null would signify conversion is not possible.
+ date = date.query(TemporalQueries.localDate());
+ return Optional.ofNullable(date);
+ }
+ return Optional.of(date);
+ }
+
+ static Optional<TemporalAccessor> getValidTime(TemporalAccessor time) {
+ if (time == null || !(time instanceof LocalTime ||
(time.query(TemporalQueries.localTime()) != null &&
time.query(TemporalQueries.zone()) != null))) {
+ return Optional.empty();
+ }
+ return Optional.of(time);
+ }
+
+ static Optional<ZoneId> getValidTimeZone(String timeZone) {
+ if (timeZone == null || timeZone.isEmpty()) {
+ return Optional.empty();
+ }
+ try {
+ return Optional.of(ZoneId.of(timeZone));
+ } catch (DateTimeException ex) {
+ return Optional.empty();
+ }
+ }
+
+ static FEELFnResult<TemporalAccessor>
generateDateTimeAndTimezone(TemporalAccessor date, TemporalAccessor time,
Optional<ZoneId> zoneId) {
+ Optional<TemporalAccessor> dateValidationResult = getValidDate(date);
+ Optional<TemporalAccessor> timeValidationResult = getValidTime(time);
+
+ try {
+ TemporalAccessor validatedDate =
dateValidationResult.orElseThrow(() -> new NoSuchElementException("Parameter
'date' is missing or invalid."));
+ TemporalAccessor validatedTime =
timeValidationResult.orElseThrow(() -> new NoSuchElementException("Parameter
'time' is missing or invalid."));
+ if (date instanceof LocalDate && time instanceof LocalTime) {
Review Comment:
In `generateDateTimeAndTimezone`, you check `date instanceof LocalDate`
against the original `date` parameter rather than the `validatedDate`. If the
input date was a date-time that gets converted to a LocalDate, this branch will
be skipped. Consider using `validatedDate instanceof LocalDate` to cover
conversion cases.
```suggestion
if (validatedDate instanceof LocalDate && time instanceof
LocalTime) {
```
##########
kie-dmn/kie-dmn-feel/src/test/java/org/kie/dmn/feel/runtime/functions/DateAndTimeFunctionTest.java:
##########
@@ -45,4 +55,219 @@ void invokeFromString() {
assertThat(retrievedZonedDateTime.getSecond()).isZero();
assertThat(retrievedZonedDateTime.getZone()).isEqualTo(ZoneId.of("Europe/Paris"));
}
+
+ @Test
+ void invokeParamStringNull() {
+ assertResultError(dateTimeFunction.invoke(null),
InvalidParametersEvent.class);
+ }
+
+ @Test
+ void invokeParamStringNotDateOrTime() {
+ assertResultError(dateTimeFunction.invoke("test"),
InvalidParametersEvent.class);
+ assertResultError(dateTimeFunction.invoke("2017-09-test"),
InvalidParametersEvent.class);
+ assertResultError(dateTimeFunction.invoke("2017-09-T89"),
InvalidParametersEvent.class);
+ }
+
+ @Test
+ void invokeParamStringDateTime() {
+
FunctionTestUtil.assertResult(dateTimeFunction.invoke("2017-09-07T10:20:30"),
LocalDateTime.of(2017, 9, 7, 10
+ , 20, 30));
+
FunctionTestUtil.assertResult(dateTimeFunction.invoke("99999-12-31T11:22:33"),
LocalDateTime.of(99999, 12, 31
+ , 11, 22, 33));
+ }
+
+ @Test
+ void invokeParamStringDateTimeZoned() {
+
FunctionTestUtil.assertResult(dateTimeFunction.invoke("2011-12-31T10:15:30@Europe/Paris"),
+ ZonedDateTime.of(2011, 12, 31, 10, 15, 30, 0,
ZoneId.of("Europe/Paris")));
+
FunctionTestUtil.assertResult(dateTimeFunction.invoke("2011-12-31T10:15:30.987@Europe/Paris"),
+ ZonedDateTime.of(2011, 12, 31, 10, 15, 30, 987_000_000,
ZoneId.of("Europe/Paris"
+ )));
+
FunctionTestUtil.assertResult(dateTimeFunction.invoke("2011-12-31T10:15:30.123456789@Europe/Paris"),
+ ZonedDateTime.of(2011, 12, 31, 10, 15, 30, 123_456_789,
ZoneId.of("Europe/Paris"
+ )));
+
FunctionTestUtil.assertResult(dateTimeFunction.invoke("999999999-12-31T23:59:59.999999999@Europe/Paris"),
Review Comment:
[nitpick] This class mixes calls to `FunctionTestUtil.assertResult` with a
static import of `assertResultError`. For consistency and readability, consider
also statically importing `assertResult`.
```suggestion
assertResult(dateTimeFunction.invoke("2017-09-07T10:20:30"),
LocalDateTime.of(2017, 9, 7, 10
, 20, 30));
assertResult(dateTimeFunction.invoke("99999-12-31T11:22:33"),
LocalDateTime.of(99999, 12, 31
, 11, 22, 33));
}
@Test
void invokeParamStringDateTimeZoned() {
assertResult(dateTimeFunction.invoke("2011-12-31T10:15:30@Europe/Paris"),
ZonedDateTime.of(2011, 12, 31, 10, 15, 30, 0,
ZoneId.of("Europe/Paris")));
assertResult(dateTimeFunction.invoke("2011-12-31T10:15:30.987@Europe/Paris"),
ZonedDateTime.of(2011, 12, 31, 10, 15, 30, 987_000_000,
ZoneId.of("Europe/Paris"
)));
assertResult(dateTimeFunction.invoke("2011-12-31T10:15:30.123456789@Europe/Paris"),
ZonedDateTime.of(2011, 12, 31, 10, 15, 30, 123_456_789,
ZoneId.of("Europe/Paris"
)));
assertResult(dateTimeFunction.invoke("999999999-12-31T23:59:59.999999999@Europe/Paris"),
```
--
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]
