Skip to content

Commit 62148fe

Browse files
author
Karl Rieb
committed
Add support for streaming uploads in OkHttpRequestor/OkHttp3Requestor.
Previously all streaming uploads would get buffered entirely in memory before being sent to the server. Fixes T105779
1 parent a2e8129 commit 62148fe

16 files changed

+703
-116
lines changed

‎ChangeLog.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
2.0.8
22
---------------------------------------------
3-
- Fix OkHttpRequestor and OkHttp3Requestor to support interceptors that consume request bodies, like Stetho.
3+
- Fix OkHttpRequestor/OkHttp3Requestor to support interceptors that consume request bodies, like Stetho.
4+
- Fix does not apply to streaming uploads.
5+
- Fix OkHttpRequestor/OkHttp3Requestor to properly handle streaming uploads.
6+
- The requestors no longer buffer entire request body in memory for streams.
47

58
2.0.7 (2016-06-20)
69
---------------------------------------------

‎build.gradle

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ dependencies {
9090
testCompile 'com.google.appengine:appengine-testing:1.9.38'
9191
testCompile 'com.squareup.okhttp:okhttp:2.7.5'
9292
testCompile 'com.squareup.okhttp3:okhttp:3.3.1'
93+
testCompile 'com.google.guava:guava:19.0'
9394
}
9495

9596
processResources {
@@ -109,7 +110,7 @@ test {
109110
useTestNG()
110111

111112
// TestNG specific options
112-
options.parallel 'classesAndMethods'
113+
options.parallel 'methods'
113114
options.threadCount 4
114115

115116
// exclude integration tests

‎release-check

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
#!/bin/bash
2+
3+
DIR=`dirname "${BASE_SOURCE[0]}"`
4+
5+
die() {
6+
echo >&2 "error: release checks failed"
7+
exit 1
8+
}
9+
10+
run() {
11+
echo "================================================================================"
12+
echo " CMD: $@"
13+
echo " OUTPUT:"
14+
$@ || die
15+
echo "================================================================================"
16+
}
17+
18+
gradle() {
19+
run "${DIR}/gradlew" \
20+
-b "${DIR}/build.gradle" \
21+
-c "${DIR}/standalone-settings.gradle" \
22+
"$@"
23+
}
24+
25+
examples_gradle() {
26+
run "${DIR}/examples/gradlew" \
27+
-b "${DIR}/examples/build.gradle" \
28+
"$@"
29+
}
30+
31+
android_gradle() {
32+
run "${DIR}/examples/android/gradlew" \
33+
-b "${DIR}/examples/android/build.gradle" \
34+
"$@"
35+
}
36+
37+
run_example() {
38+
run "${DIR}/examples/run" "$@"
39+
}
40+
41+
if [ $# -ne 1 ] ; then
42+
script=`basename "${0}"`
43+
echo >&2 "usage: $script production-user-auth.json"
44+
exit 1
45+
fi
46+
47+
AUTH_FILE="${1}"
48+
49+
if [ ! -f "${AUTH_FILE}" ] ; then
50+
echo >&2 "error: no such file: ${AUTH_FILE}"
51+
exit 1
52+
fi
53+
54+
trap 'die' ERR
55+
trap 'die' SIGTERM
56+
57+
echo "Running release checks..."
58+
echo ""
59+
echo "A series of tests will be performed to verify package is ready for release."
60+
echo "These tests may take a while to complete."
61+
echo ""
62+
63+
gradle clean
64+
gradle check
65+
66+
# test major HTTP requestor implementations
67+
for requestor in "StandardHttpRequestor" "OkHttpRequestor" "OkHttp3Requestor" ; do
68+
gradle -Pcom.dropbox.test.httpRequestor="${requestor}" integrationTest proguardTest
69+
done
70+
71+
# prepare examples to be run
72+
gradle install
73+
examples_gradle clean
74+
examples_gradle classes
75+
76+
# run examples
77+
run_example account-info "${AUTH_FILE}"
78+
run_example upload-file "${AUTH_FILE}" <(cat /dev/urandom | head -c 1024) /test/dropbox-sdk-java/examples/upload-file/small-1KiB.dat
79+
run_example upload-file "${AUTH_FILE}" <(cat /dev/urandom | head -c $(( 32 << 20 ))) /test/dropbox-sdk-java/examples/upload-file/large-32MiB.dat
80+
81+
# build android
82+
android_gradle clean
83+
android_gradle assemble

‎src/main/java/com/dropbox/core/DbxRequestUtil.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ public static HttpRequestor.Response startPostRaw(DbxRequestConfig requestConfig
231231
try {
232232
HttpRequestor.Uploader uploader = requestConfig.getHttpRequestor().startPost(uri, headers);
233233
try {
234-
uploader.getBody().write(body);
234+
uploader.upload(body);
235235
return uploader.finish();
236236
} finally {
237237
uploader.close();

‎src/main/java/com/dropbox/core/DbxUploader.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,7 @@ protected DbxUploader(HttpRequestor.Uploader httpUploader, StoneSerializer<R> re
9292
public R uploadAndFinish(InputStream in) throws X, DbxException, IOException {
9393
try {
9494
try {
95-
IOUtil.copyStreamToStream(in, getOutputStream());
96-
} catch (IOUtil.ReadException ex) {
97-
// read exceptions should be IOException
98-
throw ex.getCause();
95+
httpUploader.upload(in);
9996
} catch (IOException ex) {
10097
// write exceptions and everything else is a Network I/O problem
10198
throw new NetworkIOException(ex);

‎src/main/java/com/dropbox/core/http/GoogleAppEngineRequestor.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.io.ByteArrayInputStream;
44
import java.io.ByteArrayOutputStream;
55
import java.io.IOException;
6+
import java.io.OutputStream;
67
import java.net.URL;
78
import java.util.ArrayList;
89
import java.util.HashMap;
@@ -75,13 +76,13 @@ public Response doGet(String url, Iterable<Header> headers) throws IOException {
7576
@Override
7677
public Uploader startPost(String url, Iterable<Header> headers) throws IOException {
7778
HTTPRequest request = newRequest(url, HTTPMethod.POST, headers);
78-
return new Uploader(service, request, new ByteArrayOutputStream());
79+
return new FetchServiceUploader(service, request, new ByteArrayOutputStream());
7980
}
8081

8182
@Override
8283
public Uploader startPut(String url, Iterable<Header> headers) throws IOException {
8384
HTTPRequest request = newRequest(url, HTTPMethod.POST, headers);
84-
return new Uploader(service, request, new ByteArrayOutputStream());
85+
return new FetchServiceUploader(service, request, new ByteArrayOutputStream());
8586
}
8687

8788
private HTTPRequest newRequest(String url, HTTPMethod method, Iterable<Header> headers) throws IOException {
@@ -123,19 +124,23 @@ private static Response toRequestorResponse(HTTPResponse response) {
123124
headers);
124125
}
125126

126-
private static class Uploader extends HttpRequestor.Uploader {
127+
private static class FetchServiceUploader extends Uploader {
127128
private final URLFetchService service;
128129
private final ByteArrayOutputStream body;
129130

130131
private HTTPRequest request;
131132

132-
public Uploader(URLFetchService service, HTTPRequest request, ByteArrayOutputStream body) {
133-
super(body);
133+
public FetchServiceUploader(URLFetchService service, HTTPRequest request, ByteArrayOutputStream body) {
134134
this.service = service;
135135
this.request = request;
136136
this.body = body;
137137
}
138138

139+
@Override
140+
public OutputStream getBody() {
141+
return body;
142+
}
143+
139144
@Override
140145
public void close() {
141146
if (request != null) {

‎src/main/java/com/dropbox/core/http/HttpRequestor.java

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.dropbox.core.http;
22

3+
import java.io.File;
4+
import java.io.FileInputStream;
35
import java.io.IOException;
46
import java.io.InputStream;
57
import java.io.OutputStream;
@@ -10,6 +12,8 @@
1012

1113
import java.util.concurrent.TimeUnit;
1214

15+
import com.dropbox.core.util.IOUtil;
16+
1317
/**
1418
* An interface that the Dropbox client library uses to make HTTP requests.
1519
* If you're fine with the standard Java {@link java.net.HttpURLConnection}
@@ -69,14 +73,38 @@ public String getValue() {
6973
}
7074

7175
public static abstract class Uploader {
72-
private final OutputStream body;
73-
74-
protected Uploader(OutputStream body) { this.body = body; }
75-
76-
public OutputStream getBody() { return body; }
76+
public abstract OutputStream getBody();
7777
public abstract void close();
7878
public abstract void abort();
7979
public abstract Response finish() throws IOException;
80+
81+
public void upload(File file) throws IOException {
82+
upload(new FileInputStream(file));
83+
}
84+
85+
public void upload(InputStream in, long limit) throws IOException {
86+
upload(IOUtil.limit(in, limit));
87+
}
88+
89+
public void upload(InputStream in) throws IOException {
90+
OutputStream out = getBody();
91+
try {
92+
IOUtil.copyStreamToStream(in, out);
93+
} catch (IOUtil.ReadException ex) {
94+
throw ex.getCause();
95+
} finally {
96+
out.close();
97+
}
98+
}
99+
100+
public void upload(byte [] body) throws IOException {
101+
OutputStream out = getBody();
102+
try {
103+
out.write(body);
104+
} finally {
105+
out.close();
106+
}
107+
}
80108
}
81109

82110
public static final class Response {

0 commit comments

Comments
 (0)