TSK-987: Fix multithread problem identified by spotbugs

This commit is contained in:
Benjamin Eckstein 2019-12-11 14:50:26 +01:00 committed by Mustapha Zorgati
parent 7ba8def59a
commit d41407a459
2 changed files with 44 additions and 40 deletions

View File

@ -11,7 +11,7 @@ import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.stream.LongStream; import java.util.stream.LongStream;
import java.util.stream.Stream; import java.util.stream.Stream;
@ -33,13 +33,12 @@ import pro.taskana.impl.util.LoggerUtils;
public final class DaysToWorkingDaysConverter { public final class DaysToWorkingDaysConverter {
private static final Logger LOGGER = LoggerFactory.getLogger(TaskMonitorServiceImpl.class); private static final Logger LOGGER = LoggerFactory.getLogger(TaskMonitorServiceImpl.class);
private static DaysToWorkingDaysConverter instance; private static boolean germanHolidaysEnabled;
private static Set<LocalDate> customHolidays = new HashSet<>();
private List<Integer> positiveDaysToWorkingDays; private List<Integer> positiveDaysToWorkingDays;
private List<Integer> negativeDaysToWorkingDays; private List<Integer> negativeDaysToWorkingDays;
private Instant dateCreated; private Instant dateCreated;
private LocalDate easterSunday; private LocalDate easterSunday;
private static boolean germanHolidaysEnabled;
private static Set<LocalDate> customHolidays = new HashSet<>();
private DaysToWorkingDaysConverter(List<? extends TimeIntervalColumnHeader> columnHeaders, private DaysToWorkingDaysConverter(List<? extends TimeIntervalColumnHeader> columnHeaders,
Instant referenceDate) { Instant referenceDate) {
@ -49,6 +48,11 @@ public final class DaysToWorkingDaysConverter {
negativeDaysToWorkingDays = generateNegativeDaysToWorkingDays(columnHeaders, referenceDate); negativeDaysToWorkingDays = generateNegativeDaysToWorkingDays(columnHeaders, referenceDate);
} }
public static DaysToWorkingDaysConverter initialize()
throws InvalidArgumentException {
return initialize(Collections.singletonList(new TimeIntervalColumnHeader(0)), Instant.now());
}
/** /**
* Initializes the DaysToWorkingDaysConverter for a list of {@link TimeIntervalColumnHeader}s and the current day. A * Initializes the DaysToWorkingDaysConverter for a list of {@link TimeIntervalColumnHeader}s and the current day. A
* new table is only created if there are bigger limits or the date has changed. * new table is only created if there are bigger limits or the date has changed.
@ -83,22 +87,8 @@ public final class DaysToWorkingDaysConverter {
if (referenceDate == null) { if (referenceDate == null) {
throw new InvalidArgumentException("ReferenceDate can´t be used as NULL-Parameter"); throw new InvalidArgumentException("ReferenceDate can´t be used as NULL-Parameter");
} }
int largesLowerLimit = TimeIntervalColumnHeader.getLargestLowerLimit(columnHeaders);
int smallestUpperLimit = TimeIntervalColumnHeader.getSmallestUpperLimit(columnHeaders);
if (instance == null
|| !instance.positiveDaysToWorkingDays.contains(largesLowerLimit)
|| !instance.negativeDaysToWorkingDays.contains(smallestUpperLimit)
|| !instance.dateCreated.truncatedTo(DAYS).equals(referenceDate.truncatedTo(DAYS))) {
instance = new DaysToWorkingDaysConverter(columnHeaders, referenceDate); return new DaysToWorkingDaysConverter(columnHeaders, referenceDate);
LOGGER.debug("Create new converter for the values from {} until {} for the date: {}.", largesLowerLimit,
smallestUpperLimit, instance.dateCreated);
}
return instance;
}
public static Optional<DaysToWorkingDaysConverter> getLastCreatedInstance() {
return Optional.ofNullable(instance);
} }
public static void setGermanPublicHolidaysEnabled(boolean germanPublicHolidaysEnabled) { public static void setGermanPublicHolidaysEnabled(boolean germanPublicHolidaysEnabled) {
@ -323,4 +313,23 @@ public final class DaysToWorkingDaysConverter {
return date.getDayOfMonth() == day && date.getMonthValue() == month; return date.getDayOfMonth() == day && date.getMonthValue() == month;
} }
} }
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
DaysToWorkingDaysConverter that = (DaysToWorkingDaysConverter) o;
return positiveDaysToWorkingDays.equals(that.positiveDaysToWorkingDays)
&& negativeDaysToWorkingDays.equals(that.negativeDaysToWorkingDays)
&& dateCreated.equals(that.dateCreated);
}
@Override
public int hashCode() {
return Objects.hash(positiveDaysToWorkingDays, negativeDaysToWorkingDays, dateCreated);
}
} }

View File

@ -14,7 +14,6 @@ import java.time.temporal.ChronoUnit;
import java.util.Collections; import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.Optional;
import java.util.function.Consumer; import java.util.function.Consumer;
import org.apache.ibatis.session.Configuration; import org.apache.ibatis.session.Configuration;
@ -151,7 +150,7 @@ class CreateTaskAccTest extends AbstractAccTest {
userName = "user_1_1", userName = "user_1_1",
groupNames = {"group_1"}) groupNames = {"group_1"})
@Test @Test
void testCreateTaskWithValidPlannedAndDue() throws ClassificationNotFoundException { void testCreateTaskWithValidPlannedAndDue() throws ClassificationNotFoundException, InvalidArgumentException {
Classification classification = classificationService.getClassification("T2100", "DOMAIN_A"); Classification classification = classificationService.getClassification("T2100", "DOMAIN_A");
long serviceLevelDays = Duration.parse(classification.getServiceLevel()).toDays(); long serviceLevelDays = Duration.parse(classification.getServiceLevel()).toDays();
@ -162,16 +161,14 @@ class CreateTaskAccTest extends AbstractAccTest {
newTask.setPrimaryObjRef(createObjectReference("COMPANY_A", "SYSTEM_A", "INSTANCE_A", "VNR", "1234567")); newTask.setPrimaryObjRef(createObjectReference("COMPANY_A", "SYSTEM_A", "INSTANCE_A", "VNR", "1234567"));
newTask.setOwner("user_1_1"); newTask.setOwner("user_1_1");
Optional<DaysToWorkingDaysConverter> converter = DaysToWorkingDaysConverter.getLastCreatedInstance(); DaysToWorkingDaysConverter converter = DaysToWorkingDaysConverter.initialize();
//TODO: this is a temporal bug fix because we did not define what happens when a task is planned on the weekend. //TODO: this is a temporal bug fix because we did not define what happens when a task is planned on the weekend.
long i = converter.get().convertWorkingDaysToDays(instantPlanned, 0); long i = converter.convertWorkingDaysToDays(instantPlanned, 0);
newTask.setPlanned(instantPlanned.plus(Duration.ofDays(i))); newTask.setPlanned(instantPlanned.plus(Duration.ofDays(i)));
//due date according to service level //due date according to service level
Instant shouldBeDueDate = converter long calendarDays = converter.convertWorkingDaysToDays(newTask.getPlanned(), serviceLevelDays);
.map(c -> c.convertWorkingDaysToDays(newTask.getPlanned(), serviceLevelDays)) Instant shouldBeDueDate = newTask.getPlanned().plus(Duration.ofDays(calendarDays));
.map(
calendarDays -> newTask.getPlanned().plus(Duration.ofDays(calendarDays))).orElseThrow(
RuntimeException::new);
newTask.setDue(shouldBeDueDate); newTask.setDue(shouldBeDueDate);
Assertions.assertDoesNotThrow(() -> taskService.createTask(newTask)); Assertions.assertDoesNotThrow(() -> taskService.createTask(newTask));
} }
@ -411,12 +408,11 @@ class CreateTaskAccTest extends AbstractAccTest {
assertNotNull(readTask); assertNotNull(readTask);
assertEquals(planned, readTask.getPlanned()); assertEquals(planned, readTask.getPlanned());
Optional<Instant> shouldBeDueDate = DaysToWorkingDaysConverter.getLastCreatedInstance() long calendarDays = DaysToWorkingDaysConverter.initialize()
.map(converter -> converter.convertWorkingDaysToDays(readTask.getPlanned(), serviceLevelDays)) .convertWorkingDaysToDays(readTask.getPlanned(), serviceLevelDays);
.map(
calendarDays -> readTask.getPlanned().plus(Duration.ofDays(calendarDays))); Instant shouldBeDueDate = readTask.getPlanned().plus(Duration.ofDays(calendarDays));
assertTrue(shouldBeDueDate.isPresent()); assertEquals(readTask.getDue(), shouldBeDueDate);
assertEquals(readTask.getDue(), shouldBeDueDate.get());
} }
@WithAccessId( @WithAccessId(
@ -445,14 +441,13 @@ class CreateTaskAccTest extends AbstractAccTest {
assertNotNull(readTask); assertNotNull(readTask);
assertEquals(due, readTask.getDue()); assertEquals(due, readTask.getDue());
Optional<Long> calendarDaysToSubstract = DaysToWorkingDaysConverter.getLastCreatedInstance() long calendarDaysToSubstract = DaysToWorkingDaysConverter.initialize()
.map(converter -> converter.convertWorkingDaysToDays(due, -serviceLevelDays)); .convertWorkingDaysToDays(due, -serviceLevelDays);
assertTrue(calendarDaysToSubstract.isPresent()); assertTrue(calendarDaysToSubstract < 0);
assertTrue(calendarDaysToSubstract.get() < 0); assertTrue(calendarDaysToSubstract <= -serviceLevelDays);
assertTrue(calendarDaysToSubstract.get() <= -serviceLevelDays);
Instant shouldBePlannedDate = due.plus(Duration.ofDays(calendarDaysToSubstract.get())); Instant shouldBePlannedDate = due.plus(Duration.ofDays(calendarDaysToSubstract));
assertEquals(readTask.getPlanned(), shouldBePlannedDate); assertEquals(readTask.getPlanned(), shouldBePlannedDate);
} }