Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incubator-kie-issues-3871] Repeating timer gets executed immediately #3872

Merged
merged 3 commits into from
Apr 7, 2025
Merged

Incubator-kie-issues-3871] Repeating timer gets executed immediately #3872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f9ca3e
Incubator-kie-issues-3871] Repeating timer gets executed immediately
Mar 27, 2025
bd8942e
Merge branch 'apache:main' into kie-3871
abhijithumbe Apr 2, 2025
5b54430
[Incubator-kie-issues-3871] Repeating timer gets executed immediately.
Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions jbpm/jbpm-flow/src/main/java/org/jbpm/process/core/timer/DateTimeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.time.Duration;
import java.time.OffsetDateTime;
import java.time.Period;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoUnit;
import java.util.Objects;
Expand Down Expand Up @@ -77,13 +78,30 @@ public static String[] parseISORepeatable(String isoString) {
result[2] = elements[2];
} else {
result[0] = elements[0].substring(1);
result[1] = OffsetDateTime.now().format(DateTimeFormatter.ISO_DATE_TIME);
result[1] = OffsetDateTime.now().plus(Duration.of(getMillis(elements[1]), ChronoUnit.MILLIS)).format(DateTimeFormatter.ISO_DATE_TIME);
result[2] = elements[1];
}

return result;
}

public static long getMillis(String durationStr) {
if (durationStr.startsWith("PT")) { // ISO-8601 PTnHnMn.nS
return Duration.parse(durationStr).toMillis();
} else if (!durationStr.contains("T")) { // ISO-8601 PnYnMnWnD
Period period = Period.parse(durationStr);
OffsetDateTime now = OffsetDateTime.now();
return Duration.between(now, now.plus(period)).toMillis();
} else { // ISO-8601 PnYnMnWnDTnHnMn.nS
String[] elements = durationStr.split("T");
Period period = Period.parse(elements[0]);
Duration duration = Duration.parse("PT" + elements[1]);
OffsetDateTime now = OffsetDateTime.now();

return Duration.between(now, now.plus(period).plus(duration)).toMillis();
}
}

public static long[] parseRepeatableDateTime(String dateTimeStr) {
long[] result = new long[3];
if (isRepeatable(Objects.requireNonNull(dateTimeStr, "Date-time string cannot be a null value!"))) {
Expand All @@ -104,7 +122,7 @@ public static long[] parseRepeatableDateTime(String dateTimeStr) {
} else if (DateTimeUtils.isPeriod(periodIn)) {
// If period is specified as duration then delay variable carry start time information
OffsetDateTime startTime = OffsetDateTime.parse(delayIn, DateTimeFormatter.ISO_DATE_TIME);
period = Duration.parse(periodIn);
period = Duration.of(getMillis(periodIn), ChronoUnit.MILLIS);
startAtDelayDur = Duration.between(OffsetDateTime.now(), startTime);
} else {
// Both delay and period are specified as start and end times
Expand Down
70 changes: 68 additions & 2 deletions jbpm/jbpm-flow/src/test/java/org/jbpm/process/core/timer/DateTimeUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
*/
package org.jbpm.process.core.timer;

import java.time.Duration;
import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoUnit;

import org.jbpm.test.util.AbstractBaseTest;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -134,8 +136,72 @@ public void testParseRepeatablePeriodOnly() {
long[] parsedRepeatable = DateTimeUtils.parseRepeatableDateTime(isoString);

assertThat(parsedRepeatable[0]).isEqualTo(-1L);
// Default delay time is 1000ms
assertThat(parsedRepeatable[1]).isEqualTo(1000L);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we need to cover this targeted logic, we can add below junit


@Test
    public void testDefaultDelay() {
        String isoString = "R/PT0M";
        long[] parsedRepeatable = DateTimeUtils.parseRepeatableDateTime(isoString);
        // Default delay time is 1000ms
        assertThat(parsedRepeatable[1]).isEqualTo(1000L);
    }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have the same test here as it was used in v7:
https://github.com/kiegroup/jbpm/blob/main/jbpm-flow/src/test/java/org/jbpm/process/core/timer/DateTimeUtilsTest.java#L126-L136

In the new syntax, this would be:

        assertThat(parsedRepeatable[0]).isEqualTo(-1L);
        assertThat(parsedRepeatable[1] <= MINUTE_IN_MILLISECONDS).as("Parsed delay is bigger than " + MINUTE_IN_MILLISECONDS).isTrue();
        assertThat(parsedRepeatable[1] > FIFTY_NINE_SECONDS_IN_MILLISECONDS)
                .as("Parsed delay is too low! Expected value is between " + MINUTE_IN_MILLISECONDS + " and " + FIFTY_NINE_SECONDS_IN_MILLISECONDS + " but is " + parsedRepeatable[1]).isTrue();
        assertThat(parsedRepeatable[2]).as("Parsed period should be one minute in milliseconds but is " + parsedRepeatable[2]).isEqualTo(MINUTE_IN_MILLISECONDS);

Not directly linked to this change, but as you are on it, I think it might be a good idea to add the additional tests from v7 (starting in L138) to the codebase as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martinweiler Thank you for looking into this. I have included additional tests as you suggested.Please take a look.

assertThat(parsedRepeatable[1] <= MINUTE_IN_MILLISECONDS).as("Parsed delay is bigger than " + MINUTE_IN_MILLISECONDS).isTrue();
assertThat(parsedRepeatable[1] > FIFTY_NINE_SECONDS_IN_MILLISECONDS)
.as("Parsed delay is too low! Expected value is between " + MINUTE_IN_MILLISECONDS + " and " + FIFTY_NINE_SECONDS_IN_MILLISECONDS + " but is " + parsedRepeatable[1]).isTrue();
assertThat(parsedRepeatable[2]).as("Parsed period should be one minute in milliseconds but is " + parsedRepeatable[2]).isEqualTo(MINUTE_IN_MILLISECONDS);
}

@Test
public void testParseRepeatablePeriodPnYTnMOnly() {
OffsetDateTime now = OffsetDateTime.now();
long expectedMillis = Duration.between(now, now.plus(10, ChronoUnit.YEARS).plus(10, ChronoUnit.MINUTES)).toMillis();
String isoString = "R/P10YT10M";

long[] parsedRepeatable = DateTimeUtils.parseRepeatableDateTime(isoString);

assertThat(parsedRepeatable[0]).isEqualTo(-1L);
assertThat(parsedRepeatable[1] <= expectedMillis).as("Parsed delay is bigger than " + expectedMillis).isTrue();
assertThat(parsedRepeatable[1] > expectedMillis - 1000)
.as("Parsed delay is too low! Expected value is between " + expectedMillis + " and " + (expectedMillis - 1000) + " but is " + parsedRepeatable[1]).isTrue();
assertThat(parsedRepeatable[2]).as("Parsed period should be 10 years and 10 minutes in milliseconds but is " + parsedRepeatable[2]).isEqualTo(expectedMillis);
}

@Test
public void testParseRepeatablePeriodPTnHnMnOnly() {
OffsetDateTime now = OffsetDateTime.now();
long expectedMillis = Duration.between(now, now.plus(10, ChronoUnit.HOURS).plus(10, ChronoUnit.MINUTES).plus(10, ChronoUnit.SECONDS)).toMillis();
String isoString = "R/PT10H10M10S"; // ISO-8601 PTnHnMn.nS

long[] parsedRepeatable = DateTimeUtils.parseRepeatableDateTime(isoString);

assertThat(parsedRepeatable[0]).isEqualTo(-1L);
assertThat(parsedRepeatable[1] <= expectedMillis).as("Parsed delay is bigger than " + expectedMillis).isTrue();
assertThat(parsedRepeatable[1] > expectedMillis - 1000)
.as("Parsed delay is too low! Expected value is between " + expectedMillis + " and " + (expectedMillis - 1000) + " but is " + parsedRepeatable[1]).isTrue();
assertThat(parsedRepeatable[2]).as("Parsed period should be 10 hours, 10 minutes and 10 seconds in milliseconds but is " + parsedRepeatable[2]).isEqualTo(expectedMillis);
}

@Test
public void testParseRepeatablePeriodPnYnMnWnDOnly() {
OffsetDateTime now = OffsetDateTime.now();
long expectedMillis = Duration
.between(now, now.plus(10, ChronoUnit.YEARS).plus(10, ChronoUnit.MONTHS).plus(10, ChronoUnit.WEEKS).plus(10, ChronoUnit.DAYS)).toMillis();
String isoString = "R/P10Y10M10W10D"; // ISO-8601 PnYnMnWnD

long[] parsedRepeatable = DateTimeUtils.parseRepeatableDateTime(isoString);

assertThat(parsedRepeatable[0]).isEqualTo(-1L);
assertThat(parsedRepeatable[1] <= expectedMillis).as("Parsed delay is bigger than " + expectedMillis).isTrue();
assertThat(parsedRepeatable[1] > expectedMillis - 1000)
.as("Parsed delay is too low! Expected value is between " + expectedMillis + " and " + (expectedMillis - 1000) + " but is " + parsedRepeatable[1]).isTrue();
assertThat(parsedRepeatable[2]).as("Parsed period should be 10 years, 10 months, 10 weeks and 10 days in milliseconds but is " + parsedRepeatable[2]).isEqualTo(expectedMillis);
}

@Test
public void testParseRepeatablePeriodPnYnMnWnDTnHnMnOnly() {
OffsetDateTime now = OffsetDateTime.now();
long expectedMillis = Duration.between(now, now.plus(10, ChronoUnit.YEARS).plus(10, ChronoUnit.MONTHS).plus(10, ChronoUnit.WEEKS)
.plus(10, ChronoUnit.DAYS).plus(10, ChronoUnit.HOURS).plus(10, ChronoUnit.MINUTES).plus(10, ChronoUnit.SECONDS)).toMillis();
String isoString = "R/P10Y10M10W10DT10H10M10S"; // ISO-8601 PnYnMnWnDTnHnMn.nS

long[] parsedRepeatable = DateTimeUtils.parseRepeatableDateTime(isoString);

assertThat(parsedRepeatable[0]).isEqualTo(-1L);
assertThat(parsedRepeatable[1] <= expectedMillis).as("Parsed delay is bigger than " + expectedMillis).isTrue();
assertThat(parsedRepeatable[1] > expectedMillis - 1000)
.as("Parsed delay is too low! Expected value is between " + expectedMillis + " and " + (expectedMillis - 1000) + " but is " + parsedRepeatable[1]).isTrue();
assertThat(parsedRepeatable[2]).as("Parsed period should be 10 years, 10 months, 10 weeks, 10 days, 10 hours, 10 minutes and 10 seconds in milliseconds but is " + parsedRepeatable[2])
.isEqualTo(expectedMillis);
}
}