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

MORE-562 schedule study timeframe alignment #129

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
@@ -0,0 +1,27 @@
package io.redlink.more.studymanager.configuration;

import io.redlink.more.studymanager.properties.TimezoneProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

import java.sql.Time;
import java.time.ZoneId;
import java.util.TimeZone;

@Configuration
@EnableConfigurationProperties({TimezoneProperties.class})
public class TimezoneConfiguration {
final TimezoneProperties properties;

public TimezoneConfiguration(TimezoneProperties properties) {
this.properties = properties;
}

@Bean
public ZoneId ZoneId() {
return TimeZone().toZoneId();
}

@Bean public TimeZone TimeZone() { return TimeZone.getTimeZone(properties.identifier()); }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package io.redlink.more.studymanager.model;

import java.time.LocalDate;

public record Timeframe (
LocalDate from,
LocalDate to
) {}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
/**
* Common basic transformers.
*/

public final class Transformers {

private static final ZoneId HOME = ZoneId.of("Europe/Vienna");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package io.redlink.more.studymanager.properties;

import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties(prefix = "more.timezone")
public record TimezoneProperties (
String identifier
) {}
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
package io.redlink.more.studymanager.repository;

import io.redlink.more.studymanager.model.Contact;
import io.redlink.more.studymanager.model.Study;
import io.redlink.more.studymanager.model.StudyRole;
import io.redlink.more.studymanager.model.User;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import io.redlink.more.studymanager.exception.BadRequestException;
import io.redlink.more.studymanager.model.*;
import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.stereotype.Component;

import java.util.List;
import java.util.Optional;
import java.util.Set;

@Component
public class StudyRepository {

Expand Down Expand Up @@ -46,6 +46,7 @@ public class StudyRepository {
private static final String SET_PAUSED_STATE_BY_ID = "UPDATE studies SET status = 'paused', modified = now() WHERE study_id = ?";
private static final String SET_CLOSED_STATE_BY_ID = "UPDATE studies SET status = 'closed', end_date = now(), modified = now() WHERE study_id = ?";
private static final String STUDY_HAS_STATE = "SELECT study_id FROM studies WHERE study_id = :study_id AND status::varchar IN (:study_status)";
private static final String GET_STUDY_TIMEFRAME = "SELECT planned_start_date, planned_end_date FROM studies WHERE study_id = ?";

private final JdbcTemplate template;
private final NamedParameterJdbcTemplate namedTemplate;
Expand Down Expand Up @@ -113,6 +114,17 @@ private String getStatusQuery(Study.Status status) {
};
}

public Timeframe getStudyTimeframe(Long studyId) {
try {
return template.queryForObject(
GET_STUDY_TIMEFRAME,
getStudyTimeframeRowMapper(),
studyId);
} catch (EmptyResultDataAccessException e) {
throw new BadRequestException("Study " + studyId + " does not exist");
}
}

private static MapSqlParameterSource studyToParams(Study study) {
return new MapSqlParameterSource()
.addValue("title", study.getTitle())
Expand Down Expand Up @@ -149,6 +161,12 @@ private static RowMapper<Study> getStudyRowMapper() {
.setPhoneNumber(rs.getString("contact_phone")));
}

private static RowMapper<Timeframe> getStudyTimeframeRowMapper() {
return (rs,rowNum) -> new Timeframe(
RepositoryUtils.readLocalDate(rs, "planned_start_date"),
RepositoryUtils.readLocalDate(rs, "planned_end_date"));
}

private static RowMapper<Study> getStudyRowMapperWithUserRoles() {
return ((rs, rowNum) -> {
var study = getStudyRowMapper().mapRow(rs, rowNum);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ public class SchedulingService {
public static final String TRIGGER = "trigger";
public static final String JOB = "job";
private final Scheduler scheduler;
private final TimeZone timeZone;

public SchedulingService(SchedulerFactoryBean factory) throws SchedulerException {
public SchedulingService(SchedulerFactoryBean factory, TimeZone timeZone) throws SchedulerException {
this.scheduler = factory.getScheduler();
this.scheduler.start();
this.timeZone = timeZone;
}

public <T extends Job> String scheduleJob(String issuer, Map<String,Object> data, Schedule schedule, Class<T> type) {
Expand Down Expand Up @@ -69,7 +71,7 @@ public void preDestroy() throws SchedulerException {
private ScheduleBuilder<? extends Trigger> getSchedulerBuilderFor(Schedule schedule) {
if(schedule instanceof CronSchedule) {
return CronScheduleBuilder.cronSchedule(((CronSchedule) schedule).getCronExpression())
.inTimeZone(TimeZone.getTimeZone("Europe/Vienna")); //TODO make configurable per study
.inTimeZone(timeZone); //TODO make configurable per study
} else {
throw new NotImplementedException("SchedulerType " + schedule.getClass().getSimpleName() + " not yet supportet");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,26 @@
import io.redlink.more.studymanager.core.component.Component;
import io.redlink.more.studymanager.core.exception.ConfigurationValidationException;
import io.redlink.more.studymanager.core.factory.ActionFactory;
import io.redlink.more.studymanager.core.factory.TriggerFactory;
import io.redlink.more.studymanager.core.validation.ConfigurationValidationReport;
import io.redlink.more.studymanager.exception.BadRequestException;
import io.redlink.more.studymanager.exception.NotFoundException;
import io.redlink.more.studymanager.model.Action;
import io.redlink.more.studymanager.core.factory.TriggerFactory;
import io.redlink.more.studymanager.model.Intervention;
import io.redlink.more.studymanager.model.Study;
import io.redlink.more.studymanager.model.Trigger;
import io.redlink.more.studymanager.repository.InterventionRepository;
import io.redlink.more.studymanager.repository.StudyRepository;
import io.redlink.more.studymanager.sdk.MoreSDK;
import io.redlink.more.studymanager.utils.LoggingUtils;

import java.text.ParseException;

import org.quartz.CronExpression;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.context.event.ContextRefreshedEvent;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Service;

import java.text.ParseException;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,19 @@
@Service
public class ObservationService {

private final ScheduleService scheduleService;
private final StudyStateService studyStateService;
private final ObservationRepository repository;

private final Map<String, ObservationFactory> observationFactories;
private final MoreSDK sdk;

public ObservationService(StudyStateService studyStateService,
public ObservationService(ScheduleService scheduleService,
StudyStateService studyStateService,
ObservationRepository repository,
Map<String, ObservationFactory> observationFactories,
MoreSDK sdk) {
this.scheduleService = scheduleService;
this.studyStateService = studyStateService;
this.repository = repository;
this.observationFactories = observationFactories;
Expand All @@ -36,6 +39,7 @@ public ObservationService(StudyStateService studyStateService,

public Observation addObservation(Observation observation) {
studyStateService.assertStudyNotInState(observation.getStudyId(), Study.Status.CLOSED);
scheduleService.assertScheduleWithinStudyTime(observation.getStudyId(), observation.getSchedule());
return repository.insert(validate(observation));
}

Expand All @@ -58,6 +62,7 @@ public List<Observation> listObservations(Long studyId) {

public Observation updateObservation(Observation observation) {
studyStateService.assertStudyNotInState(observation.getStudyId(), Study.Status.CLOSED);
scheduleService.assertScheduleWithinStudyTime(observation.getStudyId(), observation.getSchedule());
return repository.updateObservation(validate(observation));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.redlink.more.studymanager.service;

import io.redlink.more.studymanager.exception.BadRequestException;
import io.redlink.more.studymanager.model.Event;
import io.redlink.more.studymanager.model.Timeframe;
import io.redlink.more.studymanager.repository.StudyRepository;
import org.springframework.stereotype.Service;

import java.time.LocalDate;
import java.time.ZoneId;

@Service
public class ScheduleService {
private final StudyRepository studyRepository;
private final ZoneId zoneId;

public ScheduleService(StudyRepository studyRepository, ZoneId zoneId) {
this.studyRepository = studyRepository;
this.zoneId = zoneId;
}

public Event assertScheduleWithinStudyTime(Long studyId, Event schedule) {
Timeframe timeframe = studyRepository.getStudyTimeframe(studyId);
if(LocalDate.ofInstant(schedule.getDateStart(), zoneId).isBefore(timeframe.from())
|| LocalDate.ofInstant(schedule.getDateEnd(), zoneId).isAfter(timeframe.to())) {
throw new BadRequestException("Schedule should lie within study timeframe");
}
return schedule;
}
}
3 changes: 3 additions & 0 deletions studymanager/src/main/resources/application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ more:
more-admin:
- 'more-admin'

timezone:
identifier: "${MORE_TIMEZONE:Europe/Vienna}"

frontend:
title: "${MORE_FE_TITLE:MORE Studymanager}"
keycloak:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class InterventionServiceTest {
StudyPermissionService studyPermissionService;
@Mock
StudyStateService studyStateService;
@Mock
ScheduleService scheduleService;
@InjectMocks
InterventionService interventionService;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ class ObservationServiceTest {
@Mock
StudyStateService studyStateService;

@Mock
ScheduleService scheduleService;

@InjectMocks
ObservationService observationService;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package io.redlink.more.studymanager.service;

import io.redlink.more.studymanager.configuration.TimezoneConfiguration;
import io.redlink.more.studymanager.exception.BadRequestException;
import io.redlink.more.studymanager.model.Event;
import io.redlink.more.studymanager.model.Timeframe;
import io.redlink.more.studymanager.repository.StudyRepository;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.boot.test.util.TestPropertyValues;
import org.springframework.context.ApplicationContextInitializer;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.test.context.ContextConfiguration;

import java.time.Instant;
import java.time.LocalDate;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.when;

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE)
@ExtendWith(MockitoExtension.class)
@ContextConfiguration(initializers = ScheduleServiceTest.EnvInitializer.class,
classes = {
ScheduleService.class,
TimezoneConfiguration.class,
})
public class ScheduleServiceTest {

@MockBean
StudyRepository studyRepository;

@Autowired
@InjectMocks
ScheduleService scheduleService;


@Test
void testAssertPasses() {
when(studyRepository.getStudyTimeframe(anyLong())).thenReturn(
new Timeframe(
LocalDate.of(2023, 6, 2),
LocalDate.of(2023,6,3))
);
Event schedule = new Event()
.setDateStart(Instant.parse("2023-06-02T00:00:00.00Z"))
.setDateEnd(Instant.parse("2023-06-03T00:00:00.00Z"));
assertThat(scheduleService.assertScheduleWithinStudyTime(1L, schedule)).isEqualTo(schedule);
}

@Test
void testAssertFails() {
when(studyRepository.getStudyTimeframe(anyLong())).thenReturn(
new Timeframe(
LocalDate.of(2023, 6, 2),
LocalDate.of(2023,6,3))
);
Event scheduleBefore = new Event()
.setDateStart(Instant.parse("2023-06-01T00:00:00.00Z"))
.setDateEnd(Instant.parse("2023-06-02T00:00:00.00Z"));
Event scheduleAfter = new Event()
.setDateStart(Instant.parse("2023-06-02T00:00:00.00Z"))
.setDateEnd(Instant.parse("2023-06-04T00:00:00.00Z"));
assertThrows(BadRequestException.class, () -> scheduleService.assertScheduleWithinStudyTime(1L, scheduleBefore));
assertThrows(BadRequestException.class, () -> scheduleService.assertScheduleWithinStudyTime(1L, scheduleAfter));
}

static class EnvInitializer implements ApplicationContextInitializer<ConfigurableApplicationContext> {

@Override
public void initialize(ConfigurableApplicationContext applicationContext) {
TestPropertyValues.of(
"more.timeframe.identifier=Europe/Vienna"
).applyTo(applicationContext);
}
}
}
Loading