TSK-1244: optimized due/planned logic for SLA P0D

This commit is contained in:
Mustapha Zorgati 2020-05-22 02:36:01 +02:00
parent c6755b35b7
commit ab6e5ecc33
4 changed files with 92 additions and 60 deletions

View File

@ -26,10 +26,10 @@ import pro.taskana.common.api.exceptions.SystemException;
public final class WorkingDaysToDaysConverter {
// offset in days from easter sunday
private static final int OFFSET_GOOD_FRIDAY = -2; // Karfreitag
private static final int OFFSET_EASTER_MONDAY = 1; // Ostermontag
private static final int OFFSET_ASCENSION_DAY = 39; // Himmelfahrt
private static final int OFFSET_WHIT_MONDAY = 50; // Pfingstmontag
private static final long OFFSET_GOOD_FRIDAY = -2; // Karfreitag
private static final long OFFSET_EASTER_MONDAY = 1; // Ostermontag
private static final long OFFSET_ASCENSION_DAY = 39; // Himmelfahrt
private static final long OFFSET_WHIT_MONDAY = 50; // Pfingstmontag
private static boolean germanHolidaysEnabled;
private static Set<CustomHoliday> customHolidays = new HashSet<>();
@ -120,21 +120,10 @@ public final class WorkingDaysToDaysConverter {
}
/** counts working days between two dates, inclusive for both margins. */
public boolean hasWorkingDaysInBetween(Instant leftInstant, Instant rightInstant) {
Instant left = leftInstant;
Instant right = rightInstant;
// make sure dates are ordered
if (leftInstant.isAfter(rightInstant)) {
left = rightInstant;
right = leftInstant;
}
long days = Duration.between(left, right).toDays();
for (long day = 0; day <= days; day++) {
if (isWorkingDay(day, left)) {
return true;
}
}
return false;
public boolean hasWorkingDaysInBetween(Instant left, Instant right) {
long days = Duration.between(left, right).abs().toDays();
Instant firstInstant = left.isBefore(right) ? left : right;
return LongStream.range(1, days).anyMatch(day -> isWorkingDay(day, firstInstant));
}
public boolean isWorkingDay(long day, Instant referenceDate) {
@ -235,7 +224,7 @@ public final class WorkingDaysToDaysConverter {
SUB_DAYS(-1),
ADD_DAYS(1);
private int direction;
private final int direction;
ZeroDirection(int direction) {
this.direction = direction;
@ -254,8 +243,8 @@ public final class WorkingDaysToDaysConverter {
CHRISTMAS1(12, 25),
CHRISTMAS2(12, 26);
private int month;
private int day;
private final int month;
private final int day;
GermanFixHolidays(int month, int day) {
this.month = month;

View File

@ -257,7 +257,7 @@ class ServiceLevelHandler {
newTaskImpl.setDue(
getFollowingWorkingDays(newTaskImpl.getPlanned(), durationPrioHolder.getDuration()));
} else if (dueIsUnchangedOrNull(newTaskImpl, oldTaskImpl) && newTaskImpl.getPlanned() != null) {
// case 2: due is null or only planned was changed -> recalculate due
// case 2: due is null or only planned was changed -> normalize planned & recalculate due
newTaskImpl.setPlanned(getFollowingWorkingDays(newTaskImpl.getPlanned(), Duration.ofDays(0)));
newTaskImpl.setDue(
getFollowingWorkingDays(newTaskImpl.getPlanned(), durationPrioHolder.getDuration()));
@ -308,13 +308,15 @@ class ServiceLevelHandler {
*
* @param task the task for the difference between planned and due must be duration
* @param duration the serviceLevel for the task
* @param planned the planned timestamp that was calculated based on due and duration
* @param calcPlanned the planned timestamp that was calculated based on due and duration
* @throws InvalidArgumentException if service level is violated.
*/
private void ensureServiceLevelIsNotViolated(TaskImpl task, Duration duration, Instant planned)
throws InvalidArgumentException {
if (task.getPlanned() != null && !task.getPlanned().equals(planned)) {
if (hasWorkingDaysInBetween(task.getPlanned(), planned)) {
private void ensureServiceLevelIsNotViolated(
TaskImpl task, Duration duration, Instant calcPlanned) throws InvalidArgumentException {
if (task.getPlanned() != null && !task.getPlanned().equals(calcPlanned)) {
// manual entered planned date is a different working day than computed value
if (converter.isWorkingDay(0, task.getPlanned())
|| converter.hasWorkingDaysInBetween(task.getPlanned(), calcPlanned)) {
throw new InvalidArgumentException(
String.format(
"Cannot update a task with given planned %s "
@ -324,41 +326,13 @@ class ServiceLevelHandler {
}
}
/**
* Check if there are working days between the dates. The method respects if calc > planned or
* vise versa and excludes the calculated planned date from the check since it is always a working
* day!
*
* @param planned the changed planned date of a task
* @param calculated the calculated planned date of a task
* @return true if there are working days between the two dates
*/
private boolean hasWorkingDaysInBetween(Instant planned, Instant calculated) {
Instant withoutCalculated;
// exclude calculated from check since it is always a working day
if (planned.isAfter(calculated)) {
withoutCalculated = calculated.plus(Duration.ofDays(1)); // start 1 day later
} else if (planned.isBefore(calculated)) {
withoutCalculated = calculated.minus(Duration.ofDays(1)); // stop 1 day earlier
} else {
return false; // no violation if they are equal
}
return converter.hasWorkingDaysInBetween(planned, withoutCalculated);
}
private TaskImpl updatePlannedDueOnCreationOfNewTask(
TaskImpl newTask, DurationPrioHolder durationPrioHolder) throws InvalidArgumentException {
if (newTask.getDue() != null) {
// due is specified: calculate back and check correctness
Instant calcDue = getPrecedingWorkingDays(newTask.getDue(), Duration.ofDays(0));
Instant calcPlanned = getPrecedingWorkingDays(calcDue, durationPrioHolder.getDuration());
if (newTask.getPlanned() != null && !calcPlanned.equals(newTask.getPlanned())) {
throw new InvalidArgumentException(
String.format(
"Cannot update a task with given planned %s "
+ "and due date %s not matching the service level %s.",
newTask.getPlanned(), newTask.getDue(), durationPrioHolder.getDuration()));
}
ensureServiceLevelIsNotViolated(newTask, durationPrioHolder.getDuration(), calcPlanned);
newTask.setDue(calcDue);
newTask.setPlanned(calcPlanned);
} else {

View File

@ -103,9 +103,11 @@ public abstract class AbstractAccTest {
protected TimeInterval toDaysInterval() {
Instant begin =
ZonedDateTime.of(LocalDate.now(ZoneId.of("UTC")), LocalTime.MIN, ZoneId.of("UTC")).toInstant();
ZonedDateTime.of(LocalDate.now(ZoneId.of("UTC")), LocalTime.MIN, ZoneId.of("UTC"))
.toInstant();
Instant end =
ZonedDateTime.of(LocalDate.now(ZoneId.of("UTC")), LocalTime.MAX, ZoneId.of("UTC")).toInstant();
ZonedDateTime.of(LocalDate.now(ZoneId.of("UTC")), LocalTime.MAX, ZoneId.of("UTC"))
.toInstant();
return new TimeInterval(begin, end);
}

View File

@ -6,8 +6,13 @@ import static pro.taskana.common.internal.util.WorkingDaysToDaysConverter.getEas
import java.time.Instant;
import java.time.LocalDate;
import java.util.Arrays;
import java.util.stream.Stream;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.DynamicContainer;
import org.junit.jupiter.api.DynamicNode;
import org.junit.jupiter.api.DynamicTest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestFactory;
import pro.taskana.common.api.CustomHoliday;
import pro.taskana.common.api.exceptions.InvalidArgumentException;
@ -23,6 +28,69 @@ class WorkingDaysToDaysConverterTest {
WorkingDaysToDaysConverter.setCustomHolidays(Arrays.asList(dayOfReformation, allSaintsDays));
}
@TestFactory
Stream<DynamicNode> testHasWorkingInBetween() throws InvalidArgumentException {
Instant referenceDay = Instant.parse("2020-02-01T07:00:00.000Z");
WorkingDaysToDaysConverter converter = WorkingDaysToDaysConverter.initialize(referenceDay);
Instant thursday = Instant.parse("2020-04-30T07:12:00.000Z");
Instant friday = Instant.parse("2020-05-01T07:12:00.000Z"); // german holiday
Instant saturday = Instant.parse("2020-05-02T07:12:00.000Z");
Instant sunday = Instant.parse("2020-05-03T07:12:00.000Z");
Instant monday = Instant.parse("2020-05-04T07:12:00.000Z");
Instant tuesday = Instant.parse("2020-05-05T07:12:00.000Z");
DynamicContainer noWorkingDaysInBetween =
DynamicContainer.dynamicContainer(
"no working days in between",
Stream.of(
DynamicTest.dynamicTest(
"tuesday <-> tuesday",
() ->
assertThat(converter.hasWorkingDaysInBetween(tuesday, tuesday)).isFalse()),
DynamicTest.dynamicTest(
"thursday <-> saturday (friday is holiday)",
() ->
assertThat(converter.hasWorkingDaysInBetween(thursday, saturday))
.isFalse()),
DynamicTest.dynamicTest(
"friday <-> friday",
() -> assertThat(converter.hasWorkingDaysInBetween(friday, friday)).isFalse()),
DynamicTest.dynamicTest(
"friday <-> monday",
() -> assertThat(converter.hasWorkingDaysInBetween(friday, monday)).isFalse()),
DynamicTest.dynamicTest(
"saturday <-> monday",
() ->
assertThat(converter.hasWorkingDaysInBetween(saturday, monday)).isFalse()),
DynamicTest.dynamicTest(
"sunday <-> monday",
() -> assertThat(converter.hasWorkingDaysInBetween(sunday, monday)).isFalse()),
DynamicTest.dynamicTest(
"monday <-> monday",
() -> assertThat(converter.hasWorkingDaysInBetween(sunday, monday)).isFalse()),
DynamicTest.dynamicTest(
"monday <-> sunday",
() -> assertThat(converter.hasWorkingDaysInBetween(monday, sunday)).isFalse()),
DynamicTest.dynamicTest(
"monday <-> friday",
() ->
assertThat(converter.hasWorkingDaysInBetween(monday, friday)).isFalse())));
DynamicContainer hasWorkingDaysInBetween =
DynamicContainer.dynamicContainer(
"has working days in between",
Stream.of(
DynamicTest.dynamicTest(
"friday <-> tuesday",
() -> assertThat(converter.hasWorkingDaysInBetween(friday, tuesday)).isTrue()),
DynamicTest.dynamicTest(
"sunday <-> tuesday",
() ->
assertThat(converter.hasWorkingDaysInBetween(sunday, tuesday)).isTrue())));
return Stream.of(noWorkingDaysInBetween, hasWorkingDaysInBetween);
}
@Test
void testConvertWorkingDaysToDaysForTasks() throws InvalidArgumentException {
Instant thursday0201 = Instant.parse("2018-02-01T07:00:00.000Z");
@ -97,7 +165,6 @@ class WorkingDaysToDaysConverterTest {
@Test
void testGetEasterSunday() {
assertThat(getEasterSunday(2018)).isEqualTo(LocalDate.of(2018, 4, 1));
assertThat(getEasterSunday(2019)).isEqualTo(LocalDate.of(2019, 4, 21));
assertThat(getEasterSunday(2020)).isEqualTo(LocalDate.of(2020, 4, 12));