From b137a2899452e7f847c11eff4a28620a2f4f0003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 27 Oct 2020 14:12:21 -0700 Subject: [PATCH 1/5] Download submission or attachment files if they don't exist Previously, it was impossible to get a complete dataset after a cancellation or 'success with errors' that resulted in some subset of submission files being saved. --- .../pull/central/PullFromCentral.java | 49 ++++++++++--------- .../pull/central/PullFromCentralTracker.java | 20 ++++---- .../pull/central/PullFromCentralTest.java | 2 +- 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java b/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java index 2ec1db262..4ac4afbe1 100644 --- a/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java +++ b/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java @@ -40,7 +40,7 @@ import org.opendatakit.briefcase.model.form.FormKey; import org.opendatakit.briefcase.model.form.FormMetadataPort; import org.opendatakit.briefcase.pull.PullEvent; -import org.opendatakit.briefcase.reused.Triple; +import org.opendatakit.briefcase.reused.Pair; import org.opendatakit.briefcase.reused.http.Http; import org.opendatakit.briefcase.reused.http.response.Response; import org.opendatakit.briefcase.reused.job.Job; @@ -98,28 +98,31 @@ public Job pull(FormStatus form) { tracker.trackNoSubmissions(); submissions.stream() - .map(instanceId -> Triple.of(submissionNumber.getAndIncrement(), instanceId, db.hasRecordedInstance(instanceId) == null)) - .peek(triple -> { - if (!triple.get3()) - tracker.trackSubmissionAlreadyDownloaded(triple.get1(), totalSubmissions); - }) - .filter(Triple::get3) - .forEach(triple -> { - int currentSubmissionNumber = triple.get1(); - String instanceId = triple.get2(); - downloadSubmission(form, instanceId, token, runnerStatus, tracker, currentSubmissionNumber, totalSubmissions); + .map(instanceId -> Pair.of(submissionNumber.getAndIncrement(), instanceId)) + .forEach(submissionNumberId -> { + int currentSubmissionNumber = submissionNumberId.getLeft(); + String instanceId = submissionNumberId.getRight(); + + boolean inDb = db.hasRecordedInstance(instanceId) != null; Path downloadedSubmissionPath = form.getSubmissionFile(briefcaseDir, instanceId); - if (downloadedSubmissionPath.toFile().exists()) { - XmlElement root = XmlElement.from(new String(readAllBytes(downloadedSubmissionPath))); - SubmissionMetaData metaData = new SubmissionMetaData(root); - metaData.getVersion().ifPresent(submissionVersions::add); + if (!inDb || !downloadedSubmissionPath.toFile().exists()) { + downloadSubmission(form, instanceId, token, runnerStatus, tracker, currentSubmissionNumber, totalSubmissions); + if (downloadedSubmissionPath.toFile().exists()) { + XmlElement root = XmlElement.from(new String(readAllBytes(downloadedSubmissionPath))); + SubmissionMetaData metaData = new SubmissionMetaData(root); + metaData.getVersion().ifPresent(submissionVersions::add); + } + } else { + tracker.trackSubmissionAlreadyDownloaded(currentSubmissionNumber, totalSubmissions); } - List attachments = getSubmissionAttachments(form, instanceId, token, runnerStatus, tracker, currentSubmissionNumber, totalSubmissions); + List attachments = getSubmissionAttachmentList(form, instanceId, token, runnerStatus, tracker, currentSubmissionNumber, totalSubmissions); int totalAttachments = attachments.size(); AtomicInteger attachmentNumber = new AtomicInteger(1); - attachments.forEach(attachment -> - downloadSubmissionAttachment(form, instanceId, attachment, token, runnerStatus, tracker, currentSubmissionNumber, totalSubmissions, attachmentNumber.getAndIncrement(), totalAttachments) + attachments.stream() + .filter(attachment -> !inDb || !form.getSubmissionMediaFile(briefcaseDir, instanceId, attachment.getName()).toFile().exists()) + .forEach(attachment -> + downloadSubmissionAttachment(form, instanceId, attachment, token, runnerStatus, tracker, currentSubmissionNumber, totalSubmissions, attachmentNumber.getAndIncrement(), totalAttachments) ); db.putRecordedInstanceDirectory(instanceId, form.getSubmissionDir(briefcaseDir, instanceId).toFile()); }); @@ -218,21 +221,21 @@ void downloadSubmission(FormStatus form, String instanceId, String token, Runner tracker.trackErrorDownloadingSubmission(submissionNumber, totalSubmissions, response); } - List getSubmissionAttachments(FormStatus form, String instanceId, String token, RunnerStatus runnerStatus, PullFromCentralTracker tracker, int submissionNumber, int totalSubmissions) { + List getSubmissionAttachmentList(FormStatus form, String instanceId, String token, RunnerStatus runnerStatus, PullFromCentralTracker tracker, int submissionNumber, int totalSubmissions) { if (runnerStatus.isCancelled()) { tracker.trackCancellation("Get submission attachments of " + instanceId); return emptyList(); } - tracker.trackStartGettingSubmissionAttachments(submissionNumber, totalSubmissions); + tracker.trackStartGettingSubmissionAttachmentList(submissionNumber, totalSubmissions); Response> response = http.execute(server.getSubmissionAttachmentListRequest(form.getFormId(), instanceId, token)); if (!response.isSuccess()) { - tracker.trackErrorGettingSubmissionAttachments(submissionNumber, totalSubmissions, response); + tracker.trackErrorGettingSubmissionAttachmentList(instanceId, response); return emptyList(); } List attachments = response.get(); - tracker.trackEndGettingSubmissionAttachments(submissionNumber, totalSubmissions); + tracker.trackEndGettingSubmissionAttachmentList(submissionNumber, totalSubmissions); return attachments; } @@ -250,6 +253,6 @@ void downloadSubmissionAttachment(FormStatus form, String instanceId, CentralAtt if (response.isSuccess()) tracker.trackEndDownloadingSubmissionAttachment(submissionNumber, totalSubmissions, attachmentNumber, totalAttachments); else - tracker.trackErrorDownloadingSubmissionAttachment(submissionNumber, totalSubmissions, attachmentNumber, totalAttachments, response); + tracker.trackErrorDownloadingSubmissionAttachment(instanceId, attachment.getName(), response); } } diff --git a/src/org/opendatakit/briefcase/pull/central/PullFromCentralTracker.java b/src/org/opendatakit/briefcase/pull/central/PullFromCentralTracker.java index 5c2be6dfd..7169c14d4 100644 --- a/src/org/opendatakit/briefcase/pull/central/PullFromCentralTracker.java +++ b/src/org/opendatakit/briefcase/pull/central/PullFromCentralTracker.java @@ -186,46 +186,46 @@ void trackEndDownloadingSubmission(int submissionNumber, int totalSubmissions) { notifyTrackingEvent(); } - void trackStartGettingSubmissionAttachments(int submissionNumber, int totalSubmissions) { - String message = "Start getting attachments of submission " + submissionNumber + " of " + totalSubmissions; + void trackStartGettingSubmissionAttachmentList(int submissionNumber, int totalSubmissions) { + String message = " Start getting attachment list of submission " + submissionNumber + " of " + totalSubmissions; form.setStatusString(message); log.info("Pull {} - {}", form.getFormName(), message); notifyTrackingEvent(); } - void trackErrorGettingSubmissionAttachments(int submissionNumber, int totalSubmissions, Response response) { + void trackErrorGettingSubmissionAttachmentList(String instanceId, Response response) { errored = true; - String message = "Error getting attachments of submission " + submissionNumber + " of " + totalSubmissions; + String message = " Error getting attachment list of submission " + instanceId; form.setStatusString(message + ": " + response.getStatusPhrase()); log.error("Pull {} - {}: HTTP {} {}", form.getFormName(), message, response.getStatusCode(), response.getStatusPhrase()); notifyTrackingEvent(); } - void trackEndGettingSubmissionAttachments(int submissionNumber, int totalSubmissions) { - String message = "Got all the attachments of submission " + submissionNumber + " of " + totalSubmissions; + void trackEndGettingSubmissionAttachmentList(int submissionNumber, int totalSubmissions) { + String message = " Got attachment list of submission " + submissionNumber + " of " + totalSubmissions; form.setStatusString(message); log.info("Pull {} - {}", form.getFormName(), message); notifyTrackingEvent(); } void trackStartDownloadingSubmissionAttachment(int submissionNumber, int totalSubmissions, int attachmentNumber, int totalAttachments) { - String message = "Start downloading attachment " + attachmentNumber + " of " + totalAttachments + " of submission " + submissionNumber + " of " + totalSubmissions; + String message = " Start downloading attachment " + attachmentNumber + " of " + totalAttachments + " of submission " + submissionNumber + " of " + totalSubmissions; form.setStatusString(message); log.info("Pull {} - {}", form.getFormName(), message); notifyTrackingEvent(); } void trackEndDownloadingSubmissionAttachment(int submissionNumber, int totalSubmissions, int attachmentNumber, int totalAttachments) { - String message = "Attachment " + attachmentNumber + " of " + totalAttachments + " of submission " + submissionNumber + " of " + totalSubmissions + " downloaded"; + String message = " Attachment " + attachmentNumber + " of " + totalAttachments + " of submission " + submissionNumber + " of " + totalSubmissions + " downloaded"; form.setStatusString(message); log.info("Pull {} - {}", form.getFormName(), message); notifyTrackingEvent(); } - void trackErrorDownloadingSubmissionAttachment(int submissionNumber, int totalSubmissions, int attachmentNumber, int totalAttachments, Response response) { + void trackErrorDownloadingSubmissionAttachment(String instanceId, String attachmentName, Response response) { errored = true; - String message = "Error downloading attachment " + attachmentNumber + " of " + totalAttachments + " of submission " + submissionNumber + " of " + totalSubmissions; + String message = " Error downloading attachment " + attachmentName + " of submission " + instanceId; form.setStatusString(message + ": " + response.getStatusPhrase()); log.error("Pull {} - {}: HTTP {} {}", form.getFormName(), message, response.getStatusCode(), response.getStatusPhrase()); notifyTrackingEvent(); diff --git a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java index 8018b9349..9c3701e72 100644 --- a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java +++ b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java @@ -218,7 +218,7 @@ public void knows_how_to_get_submission_attachments() { ok(jsonOfAttachments(expectedAttachments)) ); - List actualAttachments = pullOp.getSubmissionAttachments(form, instanceId, token, runnerStatus, tracker, 1, 1); + List actualAttachments = pullOp.getSubmissionAttachmentList(form, instanceId, token, runnerStatus, tracker, 1, 1); assertThat(actualAttachments, hasSize(expectedAttachments.size())); for (CentralAttachment attachment : actualAttachments) assertThat(expectedAttachments, hasItem(attachment)); From 03077b0cde99fa928dd5201955068c388e21e813 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 27 Oct 2020 14:13:40 -0700 Subject: [PATCH 2/5] If pull is cancelled, don't save instance in db --- .../briefcase/pull/aggregate/PullFromAggregate.java | 4 +++- .../opendatakit/briefcase/pull/central/PullFromCentral.java | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/org/opendatakit/briefcase/pull/aggregate/PullFromAggregate.java b/src/org/opendatakit/briefcase/pull/aggregate/PullFromAggregate.java index d875b303f..355bd8732 100644 --- a/src/org/opendatakit/briefcase/pull/aggregate/PullFromAggregate.java +++ b/src/org/opendatakit/briefcase/pull/aggregate/PullFromAggregate.java @@ -149,7 +149,9 @@ public Job pull(FormStatus form, Optional lastCursor) { submissionAttachments.parallelStream().forEach(attachment -> downloadSubmissionAttachment(form, submission, attachment, rs, tracker, currentSubmissionNumber, totalSubmissions, submissionAttachmentNumber.getAndIncrement(), totalSubmissionAttachments) ); - db.putRecordedInstanceDirectory(submission.getInstanceId(), form.getSubmissionDir(briefcaseDir, submission.getInstanceId()).toFile()); + if (!rs.isCancelled()) { + db.putRecordedInstanceDirectory(submission.getInstanceId(), form.getSubmissionDir(briefcaseDir, submission.getInstanceId()).toFile()); + } }); }); diff --git a/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java b/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java index 4ac4afbe1..6fc2e2e6c 100644 --- a/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java +++ b/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java @@ -123,8 +123,10 @@ public Job pull(FormStatus form) { .filter(attachment -> !inDb || !form.getSubmissionMediaFile(briefcaseDir, instanceId, attachment.getName()).toFile().exists()) .forEach(attachment -> downloadSubmissionAttachment(form, instanceId, attachment, token, runnerStatus, tracker, currentSubmissionNumber, totalSubmissions, attachmentNumber.getAndIncrement(), totalAttachments) - ); - db.putRecordedInstanceDirectory(instanceId, form.getSubmissionDir(briefcaseDir, instanceId).toFile()); + ); + if (!runnerStatus.isCancelled()) { + db.putRecordedInstanceDirectory(instanceId, form.getSubmissionDir(briefcaseDir, instanceId).toFile()); + } }); tracker.trackEnd(); From 0e542e620976ad2727405e80dc0eaa1c1bc78d44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 27 Oct 2020 16:08:39 -0700 Subject: [PATCH 3/5] Update expected messages in tests --- .../central/PullFromCentralIntegrationTest.java | 8 ++++---- .../pull/central/PullFromCentralTest.java | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java index 0e26ec9c8..a11e197dc 100644 --- a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java +++ b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java @@ -161,12 +161,12 @@ public void knows_how_to_pull_a_form() throws Exception { assertThat(form.getStatusHistory(), containsString("Form attachment 2 of 2 downloaded")); assertThat(form.getStatusHistory(), containsString("Start downloading submission 1 of 250")); assertThat(form.getStatusHistory(), containsString("Submission 1 of 250 downloaded")); - assertThat(form.getStatusHistory(), containsString("Start getting attachments of submission 1 of 250")); - assertThat(form.getStatusHistory(), containsString("Got all the attachments of submission 1 of 250")); + assertThat(form.getStatusHistory(), containsString(" Start getting attachment list of submission 1 of 250")); + assertThat(form.getStatusHistory(), containsString(" Got attachment list of submission 1 of 250")); assertThat(form.getStatusHistory(), containsString("Start downloading submission 250 of 250")); assertThat(form.getStatusHistory(), containsString("Submission 250 of 250 downloaded")); - assertThat(form.getStatusHistory(), containsString("Start getting attachments of submission 250 of 250")); - assertThat(form.getStatusHistory(), containsString("Got all the attachments of submission 250 of 250")); + assertThat(form.getStatusHistory(), containsString(" Start getting attachment list of submission 250 of 250")); + assertThat(form.getStatusHistory(), containsString(" Got attachment list of submission 250 of 250")); assertThat(form.getStatusHistory(), containsString("Start downloading attachment 1 of 2 of submission 250 of 250")); assertThat(form.getStatusHistory(), containsString("Attachment 1 of 2 of submission 250 of 250 downloaded")); assertThat(form.getStatusHistory(), containsString("Start downloading attachment 2 of 2 of submission 250 of 250")); diff --git a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java index 9c3701e72..32c2e5bd9 100644 --- a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java +++ b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java @@ -224,8 +224,8 @@ public void knows_how_to_get_submission_attachments() { assertThat(expectedAttachments, hasItem(attachment)); assertThat(events, contains( - "Start getting attachments of submission 1 of 1", - "Got all the attachments of submission 1 of 1" + " Start getting attachment list of submission 1 of 1", + " Got attachment list of submission 1 of 1" )); } @@ -242,12 +242,12 @@ public void knows_how_to_download_a_submission_attachment() { expectedAttachments.forEach(attachment -> assertThat(form.getSubmissionMediaFile(briefcaseDir, instanceId, attachment.getName()), exists())); assertThat(events, contains( - "Start downloading attachment 1 of 3 of submission 1 of 1", - "Attachment 1 of 3 of submission 1 of 1 downloaded", - "Start downloading attachment 2 of 3 of submission 1 of 1", - "Attachment 2 of 3 of submission 1 of 1 downloaded", - "Start downloading attachment 3 of 3 of submission 1 of 1", - "Attachment 3 of 3 of submission 1 of 1 downloaded" + " Start downloading attachment 1 of 3 of submission 1 of 1", + " Attachment 1 of 3 of submission 1 of 1 downloaded", + " Start downloading attachment 2 of 3 of submission 1 of 1", + " Attachment 2 of 3 of submission 1 of 1 downloaded", + " Start downloading attachment 3 of 3 of submission 1 of 1", + " Attachment 3 of 3 of submission 1 of 1 downloaded" )); } } From 2271261886d65bfed71783543f78c05896fdd606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Wed, 28 Oct 2020 13:51:21 -0700 Subject: [PATCH 4/5] Add test for conditions under which submission.xml is downloaded --- .../pull/central/PullFromCentral.java | 2 +- .../PullFromCentralIntegrationTest.java | 83 +++++++++++++++++++ .../pull/central/PullFromCentralTest.java | 42 +++++----- .../reused/transfer/TransferTestHelpers.java | 2 +- 4 files changed, 106 insertions(+), 23 deletions(-) diff --git a/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java b/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java index 6fc2e2e6c..6f42c6e5c 100644 --- a/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java +++ b/src/org/opendatakit/briefcase/pull/central/PullFromCentral.java @@ -124,7 +124,7 @@ public Job pull(FormStatus form) { .forEach(attachment -> downloadSubmissionAttachment(form, instanceId, attachment, token, runnerStatus, tracker, currentSubmissionNumber, totalSubmissions, attachmentNumber.getAndIncrement(), totalAttachments) ); - if (!runnerStatus.isCancelled()) { + if (!runnerStatus.isCancelled() && !inDb) { db.putRecordedInstanceDirectory(instanceId, form.getSubmissionDir(briefcaseDir, instanceId).toFile()); } }); diff --git a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java index a11e197dc..daec39a35 100644 --- a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java +++ b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java @@ -23,7 +23,9 @@ import static com.github.npathai.hamcrestopt.OptionalMatchers.isPresent; import static java.util.stream.Collectors.joining; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; +import static org.opendatakit.briefcase.matchers.PathMatchers.exists; import static org.opendatakit.briefcase.pull.central.PullFromCentralTest.buildAttachments; import static org.opendatakit.briefcase.pull.central.PullFromCentralTest.jsonOfAttachments; import static org.opendatakit.briefcase.pull.central.PullFromCentralTest.jsonOfSubmissions; @@ -35,13 +37,16 @@ import static org.opendatakit.briefcase.reused.job.JobsRunner.launchSync; import static org.opendatakit.briefcase.reused.transfer.TransferTestHelpers.buildFormStatus; import static org.opendatakit.briefcase.reused.transfer.TransferTestHelpers.buildMediaFileXml; +import static org.opendatakit.briefcase.reused.transfer.TransferTestHelpers.buildSubmissionXml; import com.github.dreamhead.moco.HttpServer; import java.net.URL; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.time.OffsetDateTime; import java.time.format.DateTimeFormatter; +import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -175,4 +180,82 @@ public void knows_how_to_pull_a_form() throws Exception { // Assert that saves form metadata assertThat(formMetadataPort.fetch(FormKey.from(form)), isPresent()); } + + @Test + public void downloads_submission_xml_if_not_on_disk() throws Exception { + String instanceId = "some instance id"; + String expectedSubmissionXml = buildSubmissionXml(instanceId); + + stubServerForSingleSubmission(instanceId, expectedSubmissionXml); + + // Stub a submission without attachments + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + ".xml"))) + .response(expectedSubmissionXml); + + // Confirm the submission was downloaded + Path submissionFile = form.getSubmissionFile(briefcaseDir, instanceId); + running(server, () -> launchSync(pullOp.pull(form))); + assertThat(submissionFile, exists()); + + Files.delete(submissionFile); + + // Confirm the submission was re-downloaded + running(server, () -> launchSync(pullOp.pull(form))); + assertThat(submissionFile, exists()); + String actualSubmissionXml = new String(readAllBytes(submissionFile)); + assertThat(actualSubmissionXml, is(expectedSubmissionXml)); + } + + @Test + public void skips_submission_file_if_already_on_disk() throws Exception { + String instanceId = "some instance id"; + String expectedSubmissionXml = buildSubmissionXml(instanceId); + + stubServerForSingleSubmission(instanceId, expectedSubmissionXml); + + // Stub a submission without attachments + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + ".xml"))) + .response(expectedSubmissionXml); + + // Confirm the submission was downloaded + Path submissionFile = form.getSubmissionFile(briefcaseDir, instanceId); + running(server, () -> launchSync(pullOp.pull(form))); + assertThat(submissionFile, exists()); + + // Stub a bad submission to make sure it doesn't get downloaded + server = httpServer(serverPort); + stubServerForSingleSubmission(instanceId, expectedSubmissionXml); + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + ".xml"))) + .response("other"); + + // Confirm the submission was not re-downloaded + running(server, () -> launchSync(pullOp.pull(form))); + String actualSubmissionXml = new String(readAllBytes(submissionFile)); + assertThat(actualSubmissionXml, is(expectedSubmissionXml)); + } + + public void stubServerForSingleSubmission(String instanceId, String expectedSubmissionXml) { + // Stub the token request + server + .request(by(uri("/v1/sessions"))) + .response("{\n" + + " \"createdAt\": \"2018-04-18T03:04:51.695Z\",\n" + + " \"expiresAt\": \"2018-04-19T03:04:51.695Z\",\n" + + " \"token\": \"" + token + "\"\n" + + "}"); + + // Stub the form XML request + server + .request(by(uri("/v1/projects/1/forms/some-form.xml"))) + .response(new String(readAllBytes(getPath("simple-form.xml")))); + + // Stub the submissions request + List expectedInstanceIds = Collections.singletonList(instanceId); + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions"))) + .response(jsonOfSubmissions(expectedInstanceIds)); + } } diff --git a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java index 32c2e5bd9..d54170838 100644 --- a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java +++ b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralTest.java @@ -126,26 +126,6 @@ public void knows_how_to_download_a_form() throws IOException { )); } - @Test - public void knows_how_to_download_a_submission() { - String instanceId = "uuid:515a13cf-d7a5-4606-a18f-84940b0944b2"; - String expectedSubmissionXml = buildSubmissionXml(instanceId); - Path submissionFile = form.getSubmissionFile(briefcaseDir, instanceId); - http.stub(server.getDownloadSubmissionRequest(form.getFormId(), instanceId, submissionFile, token), ok(expectedSubmissionXml)); - - pullOp.downloadSubmission(form, instanceId, token, runnerStatus, tracker, 1, 1); - - String actualSubmissionXml = new String(readAllBytes(submissionFile)); - assertThat(submissionFile, exists()); - assertThat(actualSubmissionXml, is(expectedSubmissionXml)); - // There's actually another event from the call to tracker.trackTotalSubmissions(1) in this test. - // Since we don't really care about it, we use Matchers.hasItem() instead of Matchers.contains() - assertThat(events, contains( - "Start downloading submission 1 of 1", - "Submission 1 of 1 downloaded" - )); - } - @Test public void knows_how_to_get_form_attachments() { List expectedAttachments = buildAttachments(3); @@ -190,7 +170,7 @@ public void knows_how_to_download_a_form_attachment() { } @Test - public void knows_how_to_get_submission() { + public void knows_how_to_get_submission_list() { List expectedInstanceIds = IntStream.range(0, 250) .mapToObj(i -> "submission instanceID " + i) .collect(Collectors.toList()); @@ -209,6 +189,26 @@ public void knows_how_to_get_submission() { )); } + @Test + public void knows_how_to_download_a_submission() { + String instanceId = "uuid:515a13cf-d7a5-4606-a18f-84940b0944b2"; + String expectedSubmissionXml = buildSubmissionXml(instanceId); + Path submissionFile = form.getSubmissionFile(briefcaseDir, instanceId); + http.stub(server.getDownloadSubmissionRequest(form.getFormId(), instanceId, submissionFile, token), ok(expectedSubmissionXml)); + + pullOp.downloadSubmission(form, instanceId, token, runnerStatus, tracker, 1, 1); + + String actualSubmissionXml = new String(readAllBytes(submissionFile)); + assertThat(submissionFile, exists()); + assertThat(actualSubmissionXml, is(expectedSubmissionXml)); + // There's actually another event from the call to tracker.trackTotalSubmissions(1) in this test. + // Since we don't really care about it, we use Matchers.hasItem() instead of Matchers.contains() + assertThat(events, contains( + "Start downloading submission 1 of 1", + "Submission 1 of 1 downloaded" + )); + } + @Test public void knows_how_to_get_submission_attachments() { String instanceId = "uuid:515a13cf-d7a5-4606-a18f-84940b0944b2"; diff --git a/test/java/org/opendatakit/briefcase/reused/transfer/TransferTestHelpers.java b/test/java/org/opendatakit/briefcase/reused/transfer/TransferTestHelpers.java index 8799d91d3..e05f6d8da 100644 --- a/test/java/org/opendatakit/briefcase/reused/transfer/TransferTestHelpers.java +++ b/test/java/org/opendatakit/briefcase/reused/transfer/TransferTestHelpers.java @@ -48,7 +48,7 @@ public class TransferTestHelpers { public static String buildSubmissionXml(String instanceId) { return "" + - "\n" + + "\n" + " \n" + " " + instanceId + "\n" + " \n" + From 8a010b1c03b6e9d4ad164f20a1309b4710c89c14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Wed, 28 Oct 2020 14:24:10 -0700 Subject: [PATCH 5/5] Add tests for submission attachment download conditions --- .../PullFromCentralIntegrationTest.java | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java index daec39a35..3ca84b644 100644 --- a/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java +++ b/test/java/org/opendatakit/briefcase/pull/central/PullFromCentralIntegrationTest.java @@ -207,6 +207,40 @@ public void downloads_submission_xml_if_not_on_disk() throws Exception { assertThat(actualSubmissionXml, is(expectedSubmissionXml)); } + @Test + public void downloads_submission_attachment_if_not_on_disk() throws Exception { + String instanceId = "some instance id"; + String expectedSubmissionXml = buildSubmissionXml(instanceId); + + stubServerForSingleSubmission(instanceId, expectedSubmissionXml); + + // Stub a submission with one attachment + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + ".xml"))) + .response(expectedSubmissionXml); + + List attachments = buildAttachments(1); + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + "/attachments"))) + .response(jsonOfAttachments(attachments)); + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + "/attachments/" + attachments.get(0).getName()))) + .response("some attachment content"); + + // Confirm the attachment was downloaded + Path attachmentFile = form.getSubmissionMediaFile(briefcaseDir, instanceId, attachments.get(0).getName()); + running(server, () -> launchSync(pullOp.pull(form))); + assertThat(attachmentFile, exists()); + + Files.delete(attachmentFile); + + // Confirm the attachment was re-downloaded + running(server, () -> launchSync(pullOp.pull(form))); + assertThat(attachmentFile, exists()); + String actualSubmissionXml = new String(readAllBytes(attachmentFile)); + assertThat(actualSubmissionXml, is("some attachment content")); + } + @Test public void skips_submission_file_if_already_on_disk() throws Exception { String instanceId = "some instance id"; @@ -237,6 +271,53 @@ public void skips_submission_file_if_already_on_disk() throws Exception { assertThat(actualSubmissionXml, is(expectedSubmissionXml)); } + @Test + public void skips_submission_attachment_if_already_on_disk() throws Exception { + String instanceId = "some instance id"; + String expectedSubmissionXml = buildSubmissionXml(instanceId); + + stubServerForSingleSubmission(instanceId, expectedSubmissionXml); + + // Stub a submission with one attachment + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + ".xml"))) + .response(expectedSubmissionXml); + + List attachments = buildAttachments(1); + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + "/attachments"))) + .response(jsonOfAttachments(attachments)); + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + "/attachments/" + attachments.get(0).getName()))) + .response("some attachment content"); + + // Confirm the attachment was downloaded + Path attachmentFile = form.getSubmissionMediaFile(briefcaseDir, instanceId, attachments.get(0).getName()); + running(server, () -> launchSync(pullOp.pull(form))); + assertThat(attachmentFile, exists()); + + // Stub a bad attachment to make sure it doesn't get downloaded + server = httpServer(serverPort); + stubServerForSingleSubmission(instanceId, expectedSubmissionXml); + // Stub a submission with one attachment + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + ".xml"))) + .response(expectedSubmissionXml); + + attachments = buildAttachments(1); + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + "/attachments"))) + .response(jsonOfAttachments(attachments)); + server + .request(by(uri("/v1/projects/1/forms/some-form/submissions/" + instanceId + "/attachments/" + attachments.get(0).getName()))) + .response("some bad attachment content"); + + // Confirm the attachment was not re-downloaded + running(server, () -> launchSync(pullOp.pull(form))); + String actualSubmissionXml = new String(readAllBytes(attachmentFile)); + assertThat(actualSubmissionXml, is("some attachment content")); + } + public void stubServerForSingleSubmission(String instanceId, String expectedSubmissionXml) { // Stub the token request server