Skip to content

Commit 893dba2

Browse files
author
Kannan Goundan
committed
OkHttpRequestor: Clean up default settings helpers.
Deprecate the INSTANCE field. It depends on SSLConfig, which has an expensive initializer. Some clients don't use SSLConfig, so don't make them pay the initialization costs. Make defaultOkHttpClient() public as a replacement.
1 parent f4d35da commit 893dba2

File tree

3 files changed

+75
-64
lines changed

3 files changed

+75
-64
lines changed

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

+42-29
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,9 @@
77
import okhttp3.OkHttpClient;
88
import okhttp3.Request;
99
import okhttp3.RequestBody;
10-
import okhttp3.internal.Util;
1110

1211
import java.io.Closeable;
1312
import java.io.File;
14-
import java.io.InputStream;
1513
import java.io.InterruptedIOException;
1614
import java.io.IOException;
1715
import java.io.OutputStream;
@@ -20,60 +18,72 @@
2018
import java.util.Map;
2119
import java.util.concurrent.TimeUnit;
2220

23-
import okio.Buffer;
2421
import okio.BufferedSink;
2522

2623
/*>>> import checkers.nullness.quals.Nullable; */
2724

2825
/**
2926
* {@link HttpRequestor} implementation that uses <a href="http://square.github.io/okhttp/">OkHttp
3027
* v3</a>. You can only use this if your project includes the OkHttp v3 library.
31-
*
32-
* <p>
33-
* To use this, pass {@link #INSTANCE} to the {@link com.dropbox.core.DbxRequestConfig} constructor.
34-
* </p>
3528
*/
3629
public class OkHttp3Requestor extends HttpRequestor {
3730
/**
38-
* A thread-safe instance of {@code OkHttp3Requestor} that connects directly
39-
* (as opposed to using a proxy).
31+
* @deprecated This field will be removed. Instead, do:
32+
* {@code new OkHttp3Requestor(OkHttp3Requestor.defaultOkHttpClient())}
4033
*/
34+
@Deprecated
4135
public static final OkHttp3Requestor INSTANCE = new OkHttp3Requestor(defaultOkHttpClient());
4236

43-
private final OkHttpClient client;
37+
/**
38+
* Returns an {@code OkHttpClient} instance with the default settings for this SDK.
39+
*/
40+
public static OkHttpClient defaultOkHttpClient() {
41+
return defaultOkHttpClientBuilder().build();
42+
}
4443

45-
private static OkHttpClient defaultOkHttpClient() {
44+
/**
45+
* Returns an {@code OkHttpClient.Builder} instance with the default settings for this SDK.
46+
*/
47+
public static OkHttpClient.Builder defaultOkHttpClientBuilder() {
4648
return new OkHttpClient.Builder()
4749
.connectTimeout(DEFAULT_CONNECT_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS)
4850
.readTimeout(DEFAULT_READ_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS)
4951
.writeTimeout(DEFAULT_READ_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS)
5052
// enables certificate pinning
51-
.sslSocketFactory(SSLConfig.getSSLSocketFactory(), SSLConfig.getTrustManager())
52-
.build();
53+
.sslSocketFactory(SSLConfig.getSSLSocketFactory(), SSLConfig.getTrustManager());
5354
}
5455

56+
private final OkHttpClient client;
57+
5558
/**
5659
* Creates a new instance of this requestor that uses {@code client} for its requests.
5760
*
58-
* <p> NOTE: This constructor will not enable certificate pinning on the client. If you want
59-
* certificate pinning, use the default instance, {@link #INSTANCE}, or clone the default client
60-
* and modify it accordingly:
61+
* <pre>
62+
* OkHttpClient client = OkHttp3Requestor.defaultOkHttpClient();
63+
* HttpRequestor requestor = new OkHttp3Requestor(client);
64+
* </pre>
6165
*
62-
* <p> NOTE: This SDK requires that OkHttp clients do not use same-thread executors for issuing
63-
* calls. The SDK relies on the assumption that all asynchronous calls will actually be executed
64-
* asynchronously. Using a same-thread executor for your OkHttp client may result in dead-locks.
66+
* <p>
67+
* If you need to make modifications to the {@link OkHttpClient} settings:
68+
* </p>
6569
*
6670
* <pre>
67-
* OkHttpClient client = OkHttpRequestor.INSTANCE.getClient()
68-
* .readTimeout(2, TimeUnit.MINUTES)
69-
* // ... other modifications
70-
* .build();
71-
* HttpRequestor requestor = new OkHttpRequestor(client);
71+
* OkHttpClient client = OkHttp3Requestor.defaultOkHttpClientBuilder()
72+
* .readTimeout(2, TimeUnit.MINUTES)
73+
* ...
74+
* .build();
7275
* </pre>
7376
*
74-
* @param client {@code OkHttpClient} to use for requests, never {@code null}
77+
* If you don't use {@link #defaultOkHttpClient()} or {@link #defaultOkHttpClientBuilder()},
78+
* make sure to use Dropbox's hardened SSL settings from {@link SSLConfig}:
79+
* </p>
7580
*
76-
* @throws IllegalArgumentException if client uses a same-thread executor for its dispatcher
81+
* <pre>
82+
* OkHttpClient client = OkHttpClient.Builder()
83+
* ...
84+
* .sslSocketFactory(SSLConfig.getSSLSocketFactory(), SSLConfig.getTrustManager())
85+
* .build();
86+
* </pre>
7787
*/
7888
public OkHttp3Requestor(OkHttpClient client) {
7989
if (client == null) throw new NullPointerException("client");
@@ -82,10 +92,13 @@ public OkHttp3Requestor(OkHttpClient client) {
8292
}
8393

8494
/**
85-
* @deprecated If you need access to the {@link OkHttpClient} instance you passed
86-
* into the constructor, keep track of it yourself.
95+
* Returns the underlying {@code OkHttpClient} used to make requests.
96+
*
97+
* If you want to modify the client for a particular request, create a new instance of this
98+
* requestor with the modified client.
99+
*
100+
* @return underlying {@code OkHttpClient} used by this requestor.
87101
*/
88-
@Deprecated
89102
public OkHttpClient getClient() {
90103
return client;
91104
}

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

+31-31
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,9 @@
77
import com.squareup.okhttp.OkHttpClient;
88
import com.squareup.okhttp.Request;
99
import com.squareup.okhttp.RequestBody;
10-
import com.squareup.okhttp.internal.Util;
1110

1211
import java.io.Closeable;
1312
import java.io.File;
14-
import java.io.InputStream;
1513
import java.io.InterruptedIOException;
1614
import java.io.IOException;
1715
import java.io.OutputStream;
@@ -21,30 +19,29 @@
2119
import java.util.concurrent.TimeUnit;
2220

2321
import okio.BufferedSink;
24-
import okio.Okio;
2522

2623
/*>>> import checkers.nullness.quals.Nullable; */
2724

2825
/**
2926
* {@link HttpRequestor} implementation that uses <a href="http://square.github.io/okhttp/">OkHttp
3027
* v2</a>. You can only use this if your project includes the OkHttp v2 library.
3128
*
32-
* <p> To use OkHttp v3, see {@link OkHttp3Requestor}.
33-
*
34-
* <p> To use this, pass {@link #INSTANCE} to the {@link com.dropbox.core.DbxRequestConfig} constructor.
35-
*
36-
* @see OkHttp3Requestor
29+
* <p>
30+
* To use OkHttp v3, see {@link OkHttp3Requestor}.
31+
* </p>
3732
*/
3833
public class OkHttpRequestor extends HttpRequestor {
3934
/**
40-
* A thread-safe instance of {@code OkHttpRequestor} that connects directly
41-
* (as opposed to using a proxy).
35+
* @deprecated This field will be removed. Instead, do:
36+
* {@code new OkHttpRequestor(OkHttpRequestor.defaultOkHttpClient())}
4237
*/
38+
@Deprecated
4339
public static final OkHttpRequestor INSTANCE = new OkHttpRequestor(defaultOkHttpClient());
4440

45-
private final OkHttpClient client;
46-
47-
private static OkHttpClient defaultOkHttpClient() {
41+
/**
42+
* Returns an {@code OkHttpClient} instance with the default settings for this SDK.
43+
*/
44+
public static OkHttpClient defaultOkHttpClient() {
4845
OkHttpClient client = new OkHttpClient();
4946
client.setConnectTimeout(DEFAULT_CONNECT_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS);
5047
client.setReadTimeout(DEFAULT_READ_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS);
@@ -54,29 +51,29 @@ private static OkHttpClient defaultOkHttpClient() {
5451
return client;
5552
}
5653

54+
private final OkHttpClient client;
55+
5756
/**
5857
* Creates a new instance of this requestor that uses {@code client} for its requests.
58+
* The client will be cloned to prevent further modification.
5959
*
60-
* <p> The {@code OkHttpClient} will be cloned to prevent further modification.
61-
*
62-
* <p> NOTE: This constructor will not enable certificate pinning on the client. If you want
63-
* certificate pinning, use the default instance, {@link #INSTANCE}, or clone the default client
64-
* and modify it accordingly:
60+
* <pre>
61+
* OkHttpClient client = OkHttpRequestor.defaultOkHttpClient();
6562
*
66-
* <p> NOTE: This SDK requires that OkHttp clients do not use same-thread executors for issuing
67-
* calls. The SDK relies on the assumption that all asynchronous calls will actually be executed
68-
* asynchronously. Using a same-thread executor for your OkHttp client may result in dead-locks.
63+
* // Make modifications, if necessary
64+
* client.setReadTimeout(2, TimeUnit.MINUTES);
65+
* ...
6966
*
70-
* <pre>
71-
* OkHttpClient client = OkHttpRequestor.INSTANCE.getClient(); // returns a clone
72-
* client.setReadTimeout(2, TimeUnit.MINUTES);
73-
* // ... other modifications
74-
* HttpRequestor requestor = new OkHttpRequestor(client);
67+
* HttpRequestor requestor = new OkHttpRequestor(client);
7568
* </pre>
7669
*
77-
* @param client {@code OkHttpClient} to use for requests (will be cloned), never {@code null}
70+
* If you don't use {@link #defaultOkHttpClient()}, make sure to use Dropbox's
71+
* hardened SSL settings from {@link SSLConfig}:
72+
* </p>
7873
*
79-
* @throws IllegalArgumentException if client uses a same-thread executor for its dispatcher
74+
* <pre>
75+
* client.setSslSocketFactory(SSLConfig.getSSLSocketFactory())
76+
* </pre>
8077
*/
8178
public OkHttpRequestor(OkHttpClient client) {
8279
if (client == null) throw new NullPointerException("client");
@@ -85,10 +82,13 @@ public OkHttpRequestor(OkHttpClient client) {
8582
}
8683

8784
/**
88-
* @deprecated If you need access to the {@link OkHttpClient} instance you passed
89-
* into the constructor, keep track of it yourself.
85+
* Returns a clone of the underlying {@code OkHttpClient} used to make requests.
86+
*
87+
* If you want to modify the client for a particular request, create a new instance of this
88+
* requestor with the modified client.
89+
*
90+
* @return clone of the underlying {@code OkHttpClient} used by this requestor.
9091
*/
91-
@Deprecated
9292
public OkHttpClient getClient() {
9393
return client;
9494
}

‎src/test/java/com/dropbox/core/ITUtil.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
import org.testng.annotations.AfterSuite;
2222
import org.testng.annotations.BeforeSuite;
2323

24-
import com.dropbox.core.DbxAuthInfo;
25-
import com.dropbox.core.DbxRequestConfig;
2624
import com.dropbox.core.http.GoogleAppEngineRequestor;
2725
import com.dropbox.core.http.HttpRequestor;
2826
import com.dropbox.core.http.OkHttpRequestor;
@@ -98,13 +96,13 @@ public static HttpRequestor newHttpRequestor() {
9896
}
9997

10098
public static OkHttpRequestor newOkHttpRequestor() {
101-
OkHttpClient httpClient = OkHttpRequestor.INSTANCE.getClient().clone();
99+
OkHttpClient httpClient = OkHttpRequestor.defaultOkHttpClient().clone();
102100
httpClient.setReadTimeout(READ_TIMEOUT, TimeUnit.MILLISECONDS);
103101
return new OkHttpRequestor(httpClient);
104102
}
105103

106104
public static OkHttp3Requestor newOkHttp3Requestor() {
107-
okhttp3.OkHttpClient httpClient = OkHttp3Requestor.INSTANCE.getClient().newBuilder()
105+
okhttp3.OkHttpClient httpClient = OkHttp3Requestor.defaultOkHttpClient().newBuilder()
108106
.readTimeout(READ_TIMEOUT, TimeUnit.MILLISECONDS)
109107
.build();
110108
return new OkHttp3Requestor(httpClient);

0 commit comments

Comments
 (0)