Skip to content

Commit

Permalink
MORE-562: check if observation/intervetion schedule lies within study… (
Browse files Browse the repository at this point in the history
#125)

* MORE-562: check if observation/intervetion schedule lies within study timeframe

* MORE-562: requested changes implemented

* MORE-562: fixed testing error

* MORE-562: impl timezone as bean in other places

* MORE-562: some requested changes
  • Loading branch information
drtyyj committed Jun 26, 2023
1 parent 81ae8cc commit e33078f
Show file tree
Hide file tree
Showing 13 changed files with 210 additions and 15 deletions.
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_end_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 @@ -6,11 +6,8 @@
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.model.*;
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;
Expand All @@ -25,6 +22,8 @@
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Service;

import java.time.LocalDate;
import java.time.ZoneId;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -33,6 +32,7 @@
@Service
public class InterventionService {

private final ScheduleService scheduleService;
private final StudyStateService studyStateService;
private final InterventionRepository repository;
private final StudyRepository studyRepository;
Expand All @@ -43,11 +43,13 @@ public class InterventionService {
private static final Logger LOGGER = LoggerFactory.getLogger(InterventionService.class);


public InterventionService(StudyStateService studyStateService,
public InterventionService(ScheduleService scheduleService,
StudyStateService studyStateService,
InterventionRepository repository, StudyRepository studyRepository,
MoreSDK sdk,
Map<String, TriggerFactory> triggerFactories,
Map<String, ActionFactory> actionFactories) {
this.scheduleService = scheduleService;
this.studyStateService = studyStateService;
this.repository = repository;
this.studyRepository = studyRepository;
Expand All @@ -58,6 +60,7 @@ public InterventionService(StudyStateService studyStateService,

public Intervention addIntervention(Intervention intervention) {
studyStateService.assertStudyNotInState(intervention.getStudyId(), Study.Status.CLOSED);
scheduleService.assertScheduleWithinStudyTime(intervention.getStudyId(), intervention.getSchedule());
return repository.insert(intervention);
}

Expand All @@ -76,6 +79,7 @@ public void deleteIntervention(Long studyId, Integer interventionId) {

public Intervention updateIntervention(Intervention intervention) {
studyStateService.assertStudyNotInState(intervention.getStudyId(), Study.Status.CLOSED);
scheduleService.assertScheduleWithinStudyTime(intervention.getStudyId(), intervention.getSchedule());
return repository.updateIntervention(intervention);
}

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);
}
}
}

0 comments on commit e33078f

Please sign in to comment.