Skip to content

Commit

Permalink
Address PR comments & fix ConcurrentModificationException
Browse files Browse the repository at this point in the history
  • Loading branch information
Lingfei Zhang committed Aug 2, 2023
1 parent d61cad2 commit d33b1c3
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 32 deletions.
27 changes: 8 additions & 19 deletions src/main/java/org/sagebionetworks/bridge/services/Exporter3Service.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,6 @@ public Exporter3Configuration initExporter3ForStudy(String appId, String studyId
sendNotification(appId, studyId, "create study", createStudyNotificationTopicArn, notification);
}
}

return ex3Config;
}

Expand Down Expand Up @@ -971,8 +970,6 @@ private void exportUpload(String appId, String recordId) {
// so they need access to the Timeline information.).
public Exporter3Configuration exportTimelineForStudy(String appId, String studyId) throws BridgeSynapseException,
SynapseException, IOException{
boolean isConfigModified = false;

// Get Timeline to export for the study.
Study study = studyService.getStudy(appId, studyId, true);
if (study.getScheduleGuid() == null) {
Expand All @@ -985,23 +982,20 @@ public Exporter3Configuration exportTimelineForStudy(String appId, String studyI
synapseHelper.checkSynapseWritableOrThrow();

// If Exporter3 is not enabled for study, initiate Exporter3 for Study;
// If enabled, get exporter3Config for study.
Exporter3Configuration exporter3Config;
if (!study.isExporter3Enabled()) {
exporter3Config = initExporter3ForStudy(appId, studyId);
} else {
exporter3Config = study.getExporter3Configuration();
initExporter3ForStudy(appId, studyId);
study = studyService.getStudy(appId, studyId, true);
}
Exporter3Configuration exporter3Config = study.getExporter3Configuration();

// Export the study's Timeline to Synapse as a wiki page in JSON format.
// If we have an IOException, the wiki page will almost certainly fail to upload. Instead of catching the exception,
// we just let the exception get thrown.
JsonNode node = BridgeObjectMapper.get().valueToTree(timeline);
File outputFile = File.createTempFile("timelineFor" + studyId, ".txt");
CharSink charSink = Files.asCharSink(outputFile, Charsets.UTF_8, FileWriteMode.APPEND);
try {
charSink.write(node.toString());
} catch (IOException e) {
e.printStackTrace();
}
charSink.write(node.toString());

CloudProviderFileHandleInterface markdown = synapseClient.multipartUpload(outputFile,
exporter3Config.getStorageLocationId(), false, false);

Expand All @@ -1013,18 +1007,13 @@ public Exporter3Configuration exportTimelineForStudy(String appId, String studyI
wiki.setMarkdownFileHandleId(markdown.getId());
wiki = synapseClient.createV2WikiPage(exporter3Config.getProjectId(), ObjectType.ENTITY, wiki);
exporter3Config.setWikiPageId(wiki.getId());
isConfigModified = true;
studyService.updateStudy(appId, study);
} else {
WikiPageKey key = WikiPageKeyHelper.createWikiPageKey(exporter3Config.getProjectId(), ObjectType.ENTITY, exporter3Config.getWikiPageId());
V2WikiPage getWiki = synapseClient.getV2WikiPage(key);
getWiki.setMarkdownFileHandleId(markdown.getId());
synapseClient.updateV2WikiPage(exporter3Config.getProjectId(), ObjectType.ENTITY, getWiki);
}

// If exporter3 config is modified, update study.
if (isConfigModified) {
studyService.updateStudy(appId, study);
}
return exporter3Config;
}
}
22 changes: 9 additions & 13 deletions src/test/java/org/sagebionetworks/bridge/services/Exporter3ServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1612,35 +1612,35 @@ public void exportTimelineForStudy_1stTime() throws Exception {
// Execute and verify output.
Exporter3Configuration ex3Config = exporter3Service.exportTimelineForStudy(TEST_APP_ID,
TestConstants.TEST_STUDY_ID);

assertEquals(ex3Config, study.getExporter3Configuration());
assertEquals(ex3Config.getWikiPageId(), wiki.getId());
verifyEx3ConfigAndSynapse(ex3Config, TestConstants.TEST_APP_ID + '/' +
TestConstants.TEST_STUDY_ID);

// Verify call to studyService three times: one in initExporter3ForStudy & two in exportTimelineForStudy
// (Need to get study again after call initExporter3ForStudy).
verify(mockStudyService, times(3)).getStudy(TEST_APP_ID, TEST_STUDY_ID, true);

// Verify call to studyService twice: one in initExporter3ForStudy & one in exportTimelineForStudy.
verify(mockStudyService, times(2)).getStudy(TEST_APP_ID, TEST_STUDY_ID, true);
verify(mockStudyService, times(2)).updateStudy(TEST_APP_ID, study);

// Verify call to schedule2Service.
verify(mockSchedule2Service).getTimelineForSchedule(TEST_APP_ID, TestConstants.SCHEDULE_GUID);

// Verify call to SynapseHelper.
verify(mockSynapseHelper).checkSynapseWritableOrThrow();
// Verify call to SynapseClient.

// Verify call to SynapseClient.
verify(mockSynapseClient).createV2WikiPage(eq(study.getExporter3Configuration().getProjectId()), eq(ObjectType.ENTITY), wikiToCreateCaptor.capture());
verify(mockSynapseClient).multipartUpload(fileToUploadCaptor.capture(),eq(study.getExporter3Configuration().getStorageLocationId()), eq(false), eq(false));

// Verify file content
JsonNode node = BridgeObjectMapper.get().valueToTree(timeline);
String expectedFileContent = node.toString();
File capturedFile = fileToUploadCaptor.getValue();
CharSource charSource = Files.asCharSource(capturedFile, Charsets.UTF_8);
try {
String content = charSource.read();
assertEquals(content, expectedFileContent);
} catch (IOException e) {
System.out.println("An error occurred while reading the file: " + e.getMessage());
}
String content = charSource.read();
assertEquals(content, expectedFileContent);

// Verify WikiPage
V2WikiPage capturedWikiPage = wikiToCreateCaptor.getValue();
Expand All @@ -1657,10 +1657,6 @@ public void exportTimelineForStudy_2ndTime() throws Exception {
// Setup Study.
study.setScheduleGuid(TestConstants.SCHEDULE_GUID);

// Study has no exporter3config.
study.setExporter3Configuration(null);
study.setExporter3Enabled(false);

// Setup existing S3FileHandle
CloudProviderFileHandleInterface existingMarkdown = new S3FileHandle();
existingMarkdown.setStorageLocationId(STORAGE_LOCATION_ID);
Expand Down

0 comments on commit d33b1c3

Please sign in to comment.