Skip to content

Commit

Permalink
Merge pull request #462 from alxdarksage/errors-for-activity-apis
Browse files Browse the repository at this point in the history 
Errors for activity apis
  • Loading branch information
@alxdarksage
alxdarksage authored Nov 3, 2021
2 parents cfeeda2 + 7201826 commit 2b70abb
Show file tree
Hide file tree
Showing 11 changed files with 466 additions and 64 deletions.
4 changes: 2 additions & 2 deletions src/main/java/org/sagebionetworks/bridge/services/AccountService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public void createAccount(App app, Account account) {
.withObjectType(ENROLLMENT)
.withTimestamp(account.getCreatedOn());
for (Enrollment en : account.getEnrollments()) {
studyActivityEventService.publishEvent(builder.withStudyId(en.getStudyId()).build());
studyActivityEventService.publishEvent(builder.withStudyId(en.getStudyId()).build(), false);
}
}

Expand Down Expand Up @@ -315,7 +315,7 @@ public void updateAccount(Account account) {
.withTimestamp(account.getModifiedOn());

for (String studyId : newStudies) {
studyActivityEventService.publishEvent(builder.withStudyId(studyId).build());
studyActivityEventService.publishEvent(builder.withStudyId(studyId).build(), false);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/sagebionetworks/bridge/services/AdherenceService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ protected void publishEvent(String appId, TimelineMetadata meta, AdherenceRecord
builder.withObjectType(ASSESSMENT);
builder.withObjectId(meta.getAssessmentId());
}
studyActivityEventService.publishEvent(builder.build());
studyActivityEventService.publishEvent(builder.build(), false);
}
}

Expand Down
46 changes: 38 additions & 8 deletions src/main/java/org/sagebionetworks/bridge/services/StudyActivityEventService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static org.sagebionetworks.bridge.BridgeConstants.API_MINIMUM_PAGE_SIZE;
import static org.sagebionetworks.bridge.BridgeConstants.NEGATIVE_OFFSET_ERROR;
import static org.sagebionetworks.bridge.BridgeConstants.PAGE_SIZE_ERROR;
import static org.sagebionetworks.bridge.BridgeUtils.COMMA_SPACE_JOINER;
import static org.sagebionetworks.bridge.BridgeUtils.formatActivityEventId;
import static org.sagebionetworks.bridge.BridgeUtils.getElement;
import static org.sagebionetworks.bridge.models.activities.ActivityEventObjectType.CREATED_ON;
Expand Down Expand Up @@ -38,6 +39,8 @@
import org.sagebionetworks.bridge.models.studies.Enrollment;
import org.sagebionetworks.bridge.models.studies.Study;
import org.sagebionetworks.bridge.validators.Validate;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Activity events that are scoped to a person participating in a specific study.
Expand All @@ -55,6 +58,7 @@
*/
@Component
public class StudyActivityEventService {
private static Logger LOG = LoggerFactory.getLogger(StudyActivityEventService.class);

static final String CREATED_ON_FIELD = CREATED_ON.name().toLowerCase();
static final String ENROLLMENT_FIELD = ENROLLMENT.name().toLowerCase();
Expand Down Expand Up @@ -97,7 +101,7 @@ DateTime getCreatedOn() {
* Only custom events can be deleted (if they are mutable). Other requests
* are silently ignored.
*/
public void deleteEvent(StudyActivityEvent event) {
public void deleteEvent(StudyActivityEvent event, boolean showError) {
checkNotNull(event);

Validate.entityThrowingException(DELETE_INSTANCE, event);
Expand All @@ -107,10 +111,17 @@ public void deleteEvent(StudyActivityEvent event) {

if (event.getUpdateType().canDelete(mostRecent, event)) {
dao.deleteCustomEvent(event);
} else {
if (LOG.isDebugEnabled()) {
LOG.debug("User " + event.getUserId() + " failed to delete study event: " + event.getEventId());
}
if (showError) {
throw new BadRequestException("Study event failed to delete: " + event.getEventId() + ".");
}
}
}

public void publishEvent(StudyActivityEvent event) {
public void publishEvent(StudyActivityEvent event, boolean showError) {
checkNotNull(event);

event.setCreatedOn(getCreatedOn());
Expand All @@ -120,13 +131,28 @@ public void publishEvent(StudyActivityEvent event) {
StudyActivityEvent mostRecent = dao.getRecentStudyActivityEvent(
event.getUserId(), event.getStudyId(), event.getEventId());

// Throwing exceptions will prevent study burst updates from happening if
// an error occurs in earlier order...so we collect errors and only show
// them at the end if we want to throw an exception.
List<String> failedEventIds = new ArrayList<>();
if (event.getUpdateType().canUpdate(mostRecent, event)) {
dao.publishEvent(event);
} else {
failedEventIds.add(event.getEventId());
}
Study study = studyService.getStudy(event.getAppId(), event.getStudyId(), true);
Schedule2 schedule = scheduleService.getScheduleForStudy(study.getAppId(), study).orElse(null);
if (schedule != null) {
createStudyBurstEvents(schedule, event);
createStudyBurstEvents(schedule, event, failedEventIds);
}
if (!failedEventIds.isEmpty()) {
String eventNames = COMMA_SPACE_JOINER.join(failedEventIds);
if (LOG.isDebugEnabled()) {
LOG.debug("User " + event.getUserId() + " failed to publish study event(s): " + eventNames);
}
if (showError) {
throw new BadRequestException("Study event(s) failed to publish: " + eventNames + ".");
}
}
}

Expand Down Expand Up @@ -212,11 +238,12 @@ private void addIfPresent(List<StudyActivityEvent> events, Map<String, DateTime>
events.add(event);
}
}

/**
* If the triggering event is mutable, these events can be created as well.
* If the triggering event is mutable, study burst events can be created as well. Any errors
* that occur are collected in the list of failedEventIds.
*/
private void createStudyBurstEvents(Schedule2 schedule, StudyActivityEvent event) {
private void createStudyBurstEvents(Schedule2 schedule, StudyActivityEvent event, List<String> failedEventIds) {
String eventId = event.getEventId();

StudyActivityEvent.Builder builder = new StudyActivityEvent.Builder()
Expand All @@ -229,14 +256,15 @@ private void createStudyBurstEvents(Schedule2 schedule, StudyActivityEvent event

for(StudyBurst burst : schedule.getStudyBursts()) {
if (burst.getOriginEventId().equals(eventId)) {
builder.withUpdateType(burst.getUpdateType());

DateTime eventTime = new DateTime(event.getTimestamp());
int len = burst.getOccurrences().intValue();

for (int i=0; i < len; i++) {
String iteration = Strings.padStart(Integer.toString(i+1), 2, '0');
eventTime = new DateTime(eventTime).plus(burst.getInterval());

StudyActivityEvent burstEvent = builder
.withObjectId(burst.getIdentifier())
.withAnswerValue(iteration)
Expand All @@ -249,7 +277,9 @@ private void createStudyBurstEvents(Schedule2 schedule, StudyActivityEvent event
// Study bursts also have an update type that must be respected.
if (burst.getUpdateType().canUpdate(mostRecent, burstEvent)) {
dao.publishEvent(burstEvent);
}
} else {
failedEventIds.add(burstEvent.getEventId());
}
}
}
}
Expand Down
17 changes: 10 additions & 7 deletions src/main/java/org/sagebionetworks/bridge/spring/controllers/ActivityEventController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ public ResourceList<ActivityEvent> getSelfActivityEvents() throws Exception {
/* v2 study-scoped APIs for participants */

@GetMapping("/v5/studies/{studyId}/participants/self/activityevents")
public ResourceList<StudyActivityEvent> getRecentActivityEventsForSelf(@PathVariable String studyId)
throws JsonProcessingException {
public ResourceList<StudyActivityEvent> getRecentActivityEventsForSelf(@PathVariable String studyId) throws JsonProcessingException {
UserSession session = getAuthenticatedAndConsentedSession();

if (!session.getParticipant().getStudyIds().contains(studyId)) {
Expand All @@ -123,7 +122,7 @@ public ResourceList<StudyActivityEvent> getRecentActivityEventsForSelf(@PathVari
.withStudyId(studyId)
.withUserId(session.getId())
.withObjectType(TIMELINE_RETRIEVED)
.withTimestamp(timelineRequestedOn).build());
.withTimestamp(timelineRequestedOn).build(), false);

return studyActivityEventService.getRecentStudyActivityEvents(session.getAppId(), session.getId(), studyId);
}
Expand All @@ -150,7 +149,8 @@ public ResourceList<StudyActivityEvent> getActivityEventHistoryForSelf(@PathVari

@PostMapping("/v5/studies/{studyId}/participants/self/activityevents")
@ResponseStatus(HttpStatus.CREATED)
public StatusMessage publishActivityEventForSelf(@PathVariable String studyId) {
public StatusMessage publishActivityEventForSelf(@PathVariable String studyId,
@RequestParam(required = false) String showError) {
UserSession session = getAuthenticatedAndConsentedSession();

if (!session.getParticipant().getStudyIds().contains(studyId)) {
Expand All @@ -159,18 +159,20 @@ public StatusMessage publishActivityEventForSelf(@PathVariable String studyId) {

StudyActivityEventRequest request = parseJson(StudyActivityEventRequest.class);
StudyActivityEventIdsMap eventMap = studyService.getStudyActivityEventIdsMap(session.getAppId(), studyId);
boolean showErrorBool = "true".equals(showError);

studyActivityEventService.publishEvent(request.parse(eventMap)
.withAppId(session.getAppId())
.withStudyId(studyId)
.withUserId(session.getId())
.build());
.build(), showErrorBool);

return EVENT_RECORDED_MSG;
}

@DeleteMapping("/v5/studies/{studyId}/participants/self/activityevents/{eventId}")
public StatusMessage deleteActivityEventForSelf(@PathVariable String studyId, @PathVariable String eventId) {
public StatusMessage deleteActivityEventForSelf(@PathVariable String studyId, @PathVariable String eventId,
@RequestParam(required = false) String showError) {
UserSession session = getAuthenticatedAndConsentedSession();

if (!session.getParticipant().getStudyIds().contains(studyId)) {
Expand All @@ -179,11 +181,12 @@ public StatusMessage deleteActivityEventForSelf(@PathVariable String studyId, @P

StudyActivityEventRequest request = new StudyActivityEventRequest(eventId, null, null, null);
StudyActivityEventIdsMap eventMap = studyService.getStudyActivityEventIdsMap(session.getAppId(), studyId);
boolean showErrorBool = "true".equals(showError);

studyActivityEventService.deleteEvent(request.parse(eventMap)
.withAppId(session.getAppId())
.withStudyId(studyId)
.withUserId(session.getId()).build());
.withUserId(session.getId()).build(), showErrorBool);

return EVENT_DELETED_MSG;
}
Expand Down
16 changes: 10 additions & 6 deletions src/main/java/org/sagebionetworks/bridge/spring/controllers/StudyParticipantController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public ResponseEntity<Timeline> getTimelineForSelf(@PathVariable String studyId)
.withStudyId(studyId)
.withUserId(session.getId())
.withObjectType(TIMELINE_RETRIEVED)
.withTimestamp(timelineRequestedOn).build());
.withTimestamp(timelineRequestedOn).build(), false);

return new ResponseEntity<>(INSTANCE.calculateTimeline(schedule), OK);
}
Expand Down Expand Up @@ -508,36 +508,40 @@ public ResourceList<StudyActivityEvent> getActivityEventHistory(@PathVariable St

@PostMapping("/v5/studies/{studyId}/participants/{userId}/activityevents")
@ResponseStatus(HttpStatus.CREATED)
public StatusMessage publishActivityEvent(@PathVariable String studyId, @PathVariable String userId) {
public StatusMessage publishActivityEvent(@PathVariable String studyId, @PathVariable String userId,
@RequestParam(required = false) String showError) {
UserSession session = getAdministrativeSession();

Account account = getValidAccountInStudy(session.getAppId(), studyId, userId);

StudyActivityEventRequest request = parseJson(StudyActivityEventRequest.class);
StudyActivityEventIdsMap eventMap = studyService.getStudyActivityEventIdsMap(session.getAppId(), studyId);
boolean showErrorBool = "true".equals(showError);

studyActivityEventService.publishEvent(request.parse(eventMap)
.withAppId(session.getAppId())
.withStudyId(studyId)
.withUserId(account.getId()).build());
.withUserId(account.getId()).build(), showErrorBool);

return EVENT_RECORDED_MSG;
}

@DeleteMapping("/v5/studies/{studyId}/participants/{userId}/activityevents/{eventId}")
public StatusMessage deleteActivityEvent(@PathVariable String studyId,
@PathVariable String userId,
@PathVariable String eventId) {
@PathVariable String eventId,
@RequestParam(required = false) String showError) {
UserSession session = getAdministrativeSession();
Account account = getValidAccountInStudy(session.getAppId(), studyId, userId);

StudyActivityEventRequest request = new StudyActivityEventRequest(eventId, null, null, null);
StudyActivityEventIdsMap eventMap = studyService.getStudyActivityEventIdsMap(session.getAppId(), studyId);

boolean showErrorBool = "true".equals(showError);

studyActivityEventService.deleteEvent(request.parse(eventMap)
.withAppId(session.getAppId())
.withStudyId(studyId)
.withUserId(account.getId()).build());
.withUserId(account.getId()).build(), showErrorBool);

return EVENT_DELETED_MSG;
}
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/org/sagebionetworks/bridge/dynamodb/DynamoHealthDataDocumentationDaoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public void createOrUpdateDocumentation_UpdateDoc() {
verify(mockMapper).save(doc);
}

@SuppressWarnings("unchecked")
@Test
public void deleteDocumentationForParentId() {
// mock dependencies
Expand Down Expand Up @@ -153,6 +154,7 @@ public void getDocumentationForIdentifier_NoDoc() {
assertNull(returned);
}

@SuppressWarnings("unchecked")
@Test
public void getDocumentationForParentId() {
// mock dependencies
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/org/sagebionetworks/bridge/services/AccountServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,7 @@ public void createAccountSuccess() throws Exception {
assertEquals(createdAccount.getMigrationVersion(), MIGRATION_VERSION);

verify(activityEventService, never()).publishEnrollmentEvent(any(), any(), any());
verify(studyActivityEventService, never()).publishEvent(any());
verify(studyActivityEventService, never()).publishEvent(any(), anyBoolean());
}

@Test
Expand All @@ -1054,7 +1054,7 @@ public void createAccountPublishesEnrollmentEvent() throws Exception {

verify(mockAccountDao).createAccount(app, account);
verify(activityEventService).publishEnrollmentEvent(any(), any(), any());
verify(studyActivityEventService, times(2)).publishEvent(eventCaptor.capture());
verify(studyActivityEventService, times(2)).publishEvent(eventCaptor.capture(), eq(false));

StudyActivityEvent event1 = getElement(
eventCaptor.getAllValues(), StudyActivityEvent::getStudyId, STUDY_A).orElse(null);
Expand Down Expand Up @@ -1134,7 +1134,7 @@ public void updateSuccess() throws Exception {
assertEquals(updatedAccount.getClientTimeZone(), OTHER_CLIENT_TIME_ZONE);

verify(activityEventService, never()).publishEnrollmentEvent(any(), any(), any());
verify(studyActivityEventService, never()).publishEvent(any());
verify(studyActivityEventService, never()).publishEvent(any(), anyBoolean());
}

@Test
Expand Down Expand Up @@ -1169,7 +1169,7 @@ public void updateAccountPublishesEnrollmentEvent() throws Exception {

verify(activityEventService).publishEnrollmentEvent(
eq(app), eq(HEALTH_CODE), any(DateTime.class));
verify(studyActivityEventService).publishEvent(eventCaptor.capture());
verify(studyActivityEventService).publishEvent(eventCaptor.capture(), eq(false));
StudyActivityEvent event = eventCaptor.getValue();
assertEquals(event.getAppId(), TEST_APP_ID);
assertEquals(event.getStudyId(), STUDY_B);
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/org/sagebionetworks/bridge/services/AdherenceServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void updateAdherenceRecords() {
assertEquals(recordCaptor.getAllValues().get(2).getInstanceGuid(), "sessionInstanceGuid");

// Nothing is finished, nothing is published.
verify(mockStudyActivityEventService, never()).publishEvent(any());
verify(mockStudyActivityEventService, never()).publishEvent(any(), eq(false));
}

@Test(expectedExceptions = BadRequestException.class)
Expand Down Expand Up @@ -172,7 +172,7 @@ public void updateAdherenceRecords_recordSessionFinished() {

verify(mockDao).updateAdherenceRecord(list.getRecords().get(0));
verify(mockDao).updateAdherenceRecord(list.getRecords().get(1));
verify(mockStudyActivityEventService, times(3)).publishEvent(eventCaptor.capture());
verify(mockStudyActivityEventService, times(3)).publishEvent(eventCaptor.capture(), eq(false));

StudyActivityEvent event = eventCaptor.getAllValues().get(2);
assertEquals(event.getAppId(), TEST_APP_ID);
Expand All @@ -197,7 +197,7 @@ public void updateAdherenceRecords_recordAssessmentFinished() throws Exception {

verify(mockDao).updateAdherenceRecord(list.getRecords().get(0));
verify(mockDao).updateAdherenceRecord(list.getRecords().get(1));
verify(mockStudyActivityEventService, times(1)).publishEvent(eventCaptor.capture());
verify(mockStudyActivityEventService, times(1)).publishEvent(eventCaptor.capture(), eq(false));

StudyActivityEvent event = eventCaptor.getValue();
assertEquals(event.getAppId(), TEST_APP_ID);
Expand All @@ -220,7 +220,7 @@ public void updateAdherenceRecords_eventWithoutMetadata() {

service.updateAdherenceRecords(TEST_APP_ID, list);

verify(mockStudyActivityEventService, never()).publishEvent(any());
verify(mockStudyActivityEventService, never()).publishEvent(any(), eq(false));
}

private AdherenceRecord mockAssessmentRecord(String id) {
Expand Down Expand Up @@ -266,7 +266,7 @@ public void updateAdherenceRecords_doNothingWithFinishedSession() {
service.updateSessionState(TEST_APP_ID, container, list.getRecords().get(0));

verify(mockDao, never()).updateAdherenceRecord(any());
verify(mockStudyActivityEventService, never()).publishEvent(any());
verify(mockStudyActivityEventService, never()).publishEvent(any(), eq(false));
}

@Test
Expand Down Expand Up @@ -433,7 +433,7 @@ public void updateAdherenceRecords_unfinishExistingSessionRecord() {
assertNull(captured.getFinishedOn());
assertFalse(captured.isDeclined());

verify(mockStudyActivityEventService, never()).publishEvent(any());
verify(mockStudyActivityEventService, never()).publishEvent(any(), eq(false));
}

@Test
Expand Down
Loading

0 comments on commit 2b70abb

Please sign in to comment.