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.

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#1612] Fix BusinessCalendar behavior with inconsistent (full/partial) set of properties on calendar.properties #3788

Merged
merged 47 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
f47333c
incubator-kie-issues-1612
Abhitocode Nov 20, 2024
580313a
incubator-kie-issues-1612
Abhitocode Nov 20, 2024
2af0ea4
incubator-kie-issues-1612
Abhitocode Nov 21, 2024
bd4a633
Merge remote-tracking branch 'Abhitocode/incubator-kie-issues#1612' i…
Abhitocode Nov 21, 2024
8f9f231
Merge remote-tracking branch 'origin/main' into incubator-kie-issues#…
Nov 21, 2024
d3e3e1a
[incubator-kie-issues#1612] TODO setup
Nov 21, 2024
bd71ddc
Merge remote-tracking branch 'origin/main' into incubator-kie-issues#…
Nov 22, 2024
bb9f787
[incubator-kie-issues#1612] Separating different concerns in differen…
Nov 22, 2024
5b8fa04
incubator-kie-issues-1612
Abhitocode Nov 25, 2024
eeb406e
[incubator-kie-issues#1612] Example test
Nov 25, 2024
35bf3bb
incubator-kie-issues-1612
Abhitocode Nov 26, 2024
683290b
incubator-kie-issues-1612
Abhitocode Nov 27, 2024
f04993f
[incubator-kie-issues#1612] Extend test coverage. Minor refactoring r…
Nov 27, 2024
d6ac71e
incubator-kie-issues-1612
Abhitocode Nov 28, 2024
6f4a647
[incubator-kie-issues#1612] Comment on tests
Nov 28, 2024
352542d
[incubator-kie-issues#1612] Minor fixes
Nov 28, 2024
5735268
[incubator-kie-issues#1612] Extend test coverage. Minor refactoring r…
Nov 28, 2024
b8ef261
[incubator-kie-issues#1612] Minor refactoring
Nov 28, 2024
977b5cf
incubator-kie-issues-1612
Abhitocode Nov 29, 2024
6af2d9e
Merge remote-tracking branch 'origin/main' into incubator-kie-issues#…
Nov 29, 2024
7598827
[incubator-kie-issues#1612] Fixing tests. Including incubator-kie-iss…
Nov 29, 2024
cb32a67
[incubator-kie-issues#1612] WIP - implementing nightly hour
Nov 29, 2024
898f327
[incubator-kie-issues#1612] WIP - Simplify test - moving tested metho…
Nov 29, 2024
b01e8de
[incubator-kie-issues#1612] WIP - Cleanup
Nov 29, 2024
dfc594c
[incubator-kie-issues#1612] Working tests.
Nov 29, 2024
0f5c0b6
incubator-kie-issues-1612
Abhitocode Dec 2, 2024
73579b1
incubator-kie-issues-1612
Abhitocode Dec 2, 2024
7d8d1c9
incubator-kie-issues-1612
Abhitocode Dec 2, 2024
0ef21b2
Merge remote-tracking branch 'Abhitocode/incubator-kie-issues#1612' i…
Abhitocode Dec 2, 2024
dfbee16
[incubator-kie-issues#1612] Fixed logging
Dec 2, 2024
8e9a93b
[incubator-kie-issues#1612] Fixed minute/second reset on calendarRolling
Dec 2, 2024
ae9e082
[incubator-kie-issues#1612] Fixed assertiont JUnit5 -> assertj
Dec 2, 2024
a664e19
[incubator-kie-issues#1612] Fixed test
Dec 2, 2024
6136ad2
Merge remote-tracking branch 'origin/main' into Abhitocode_incubator-…
Dec 2, 2024
7ef7f11
incubator-kie-issues-1612
Abhitocode Dec 2, 2024
73269e8
Merge remote-tracking branch 'origin/main' into incubator-kie-issues#…
Dec 3, 2024
2850074
Merge remote-tracking branch 'origin/main' into incubator-kie-issues#…
Abhitocode Dec 3, 2024
b4f5138
Merge branch 'Abhitocode_incubator-kie-issues#1612' into incubator-ki…
Dec 3, 2024
815e450
incubator-kie-issues-1612
Abhitocode Dec 4, 2024
887f7f9
Merge remote-tracking branch 'Abhitocode/incubator-kie-issues#1612' i…
Dec 5, 2024
e8f52b3
[incubator-kie-issues#1612] Avoid minute/second reset on rolling hour…
Dec 5, 2024
670e4f8
[incubator-kie-issues#1612] Fix naming
Dec 5, 2024
dc0702b
[incubator-kie-issues#1612] Add minute / second test/fix
Dec 5, 2024
a861c0b
[incubator-kie-issues#1612] Extend test coverage
Dec 5, 2024
59fb324
updated logging
Abhitocode Dec 6, 2024
790b177
logger update
Abhitocode Dec 9, 2024
c0df20d
logger update
Abhitocode Dec 9, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,14 @@ public class BusinessCalendarImpl implements BusinessCalendar {
private List<TimePeriod> holidays;
private List<Integer> weekendDays = new ArrayList<>();
private SessionClock clock;
private final StringBuilder errorMessage = new StringBuilder();

private static final int SIM_WEEK = 3;
private static final int SIM_DAY = 5;
private static final int SIM_HOU = 7;
private static final int SIM_MIN = 9;
private static final int SIM_SEC = 11;

public static final String DAYS_PER_WEEK = "business.days.per.week";
public static final String HOURS_PER_DAY = "business.hours.per.day";
Abhitocode marked this conversation as resolved.
Show resolved Hide resolved
public static final String START_HOUR = "business.start.hour";
public static final String END_HOUR = "business.end.hour";
// holidays are given as date range and can have more than one value separated with comma
Expand Down Expand Up @@ -137,13 +136,12 @@ public BusinessCalendarImpl(Properties configuration, SessionClock clock) {
}

protected void init() {
daysPerWeek = getPropertyAsInt(DAYS_PER_WEEK, "5");
hoursInDay = getPropertyAsInt(HOURS_PER_DAY, "8");
startHour = getPropertyAsInt(START_HOUR, "9");
endHour = getPropertyAsInt(END_HOUR, "17");
holidays = parseHolidays();
parseWeekendDays();
daysPerWeek = 7 - weekendDays.size();
this.timezone = businessCalendarConfiguration.getProperty(TIMEZONE);

validateProperties();
}

protected String adoptISOFormat(String timeExpression) {
Expand Down Expand Up @@ -338,6 +336,12 @@ protected int getPropertyAsInt(String propertyName, String defaultValue) {
return Integer.parseInt(value);
}

protected int getPropertyAsInt(String propertyName) {
String value = businessCalendarConfiguration.getProperty(propertyName);

return Integer.parseInt(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to return a nicer error message if this fails:

Caused by: java.lang.NumberFormatException: For input string: ""
	at java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:67)
	at java.base/java.lang.Integer.parseInt(Integer.java:678)
	at java.base/java.lang.Integer.parseInt(Integer.java:786)
	at org.jbpm.process.core.timer.BusinessCalendarImpl.getPropertyAsInt(BusinessCalendarImpl.java:342)

}

protected List<TimePeriod> parseHolidays() {
String holidaysString = businessCalendarConfiguration.getProperty(HOLIDAYS);
List<TimePeriod> holidays = new ArrayList<>();
Expand Down Expand Up @@ -427,6 +431,7 @@ protected List<TimePeriod> parseHolidays() {
}
} catch (Exception e) {
logger.error("Error while parsing holiday in business calendar", e);
errorMessage.append("Invalid holidays: Error while parsing holiday in business calendar(").append(e.getMessage()).append(")\n");
}
}
}
Expand All @@ -442,9 +447,10 @@ protected void parseWeekendDays() {
} else {
String[] days = weekendDays.split(",");
for (String day : days) {
this.weekendDays.add(Integer.parseInt(day));
this.weekendDays.add(Integer.parseInt(day.trim()));
}
}
this.weekendDays.removeIf(weekend -> weekend == 0);
}

private class TimePeriod {
Expand Down Expand Up @@ -494,4 +500,66 @@ protected void handleWeekend(Calendar c, boolean resetTime) {
dayOfTheWeek = c.get(Calendar.DAY_OF_WEEK);
}
}

private void validateProperties() {

boolean startHourProvided = validateRequiredProperty(START_HOUR);
boolean endHourProvided = validateRequiredProperty(END_HOUR);
if (startHourProvided) {
startHour = getPropertyAsInt(START_HOUR);
validateRangeForProperty(startHour, START_HOUR, 0, 23, null, null);
}
if (endHourProvided) {
endHour = getPropertyAsInt(END_HOUR);
validateRangeForProperty(endHour, END_HOUR, 0, 23, null, null);
}
if (startHourProvided && endHourProvided) {
hoursInDay = startHour < endHour ? endHour - startHour : (24 - startHour) + endHour;
}

for (int weekendDay : weekendDays) {
validateRangeForProperty(weekendDay, WEEKEND_DAYS, 0, 7, "No weekend day", "Saturday");
}
if (timezone != null && !isValidTimeZone(timezone)) {
errorMessage.append("Invalid timezone: ").append(timezone).append(". Refer to valid timezones: https://docs.oracle.com/javase/7/docs/api/java/util/TimeZone.html\n");
}

if (!errorMessage.isEmpty()) {
throw new IllegalArgumentException(errorMessage.toString());
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to emit a log message here that summarizes the settings, including the configured ones.


private boolean validateRequiredProperty(String property) {
String value = businessCalendarConfiguration.getProperty(property);
if (Objects.isNull(value)) {
errorMessage.append("Property required: ").append(property).append(" is not provided.\n");
return false;
}
return true;
}

private boolean isValidTimeZone(String timeZone) {
String[] validIDs = TimeZone.getAvailableIDs();
for (String id : validIDs) {
if (id.equals(timeZone)) {
return true;
}
}
return false;
}

private void validateRangeForProperty(int value, String propertyName, int lowerBound, int upperBound, String lowerBoundDescription, String upperBoundDescription) {
if ((value < lowerBound || value > upperBound)) {
errorMessage.append("Invalid property: ")
.append(propertyName)
.append(" must be between ")
.append(lowerBound)
.append(Objects.nonNull(lowerBoundDescription) ? "(" + lowerBoundDescription + ")" : "")
.append(" and ")
.append(upperBound)
.append(Objects.nonNull(upperBoundDescription) ? "(" + upperBoundDescription + ")" : "")
.append(".\n");
}
}

}
Loading
Loading