box
feat: `POC` of using `OkHttp` in Java SDK
This is a POC of replacing current Java 8 HTTP client with pure OkHttp client. I mean pure since I'm not dealing now with retries - so they are handled the same way.
In this POC I'm not adding any async code to Java SDK at this will be bery complicated. We would need to deal with the fact that when we want to get authorization token it will be a future that at some point in time will be resolved. The OkHttp client supports asynchronous HTTP operations and it is easy to return CompletableFuture from request.
Challenges
SslSocketFactory
In the old code we were setting SslSocketFactory on every request. Now this must be done on BoxAPIConnection. This is a breaking change since we know users are using reflection to override this setting.
Mutable BoxAPIConnection
OhHttp client is immutable and current BoxAPIConnection is not. We can deal with this with replacing OkHttp client each time some method that changes its behaviour is called - like adding a proxy. We can do this like so:
// copy existing client into the builder
OkHttp.Builder builder = new OkHttpClient.Builder(this.httpClient);
// now we can modify client behaviour
Or we can do a breaking change and make our BoxAPIConnection immutable as well.
RequestInterceptor
Request interceptor allows users to modify BoxAPIRequest request. Since we are building OkHttp Request after applying interceptor we should be fine as all changes are reflected on the final request.
AbstractBoxMultipartRequest and retrying
~~In our SDK we are using InputStream to send files to box. This might be a cause of errors when retrying uploads. We are reading the stream containing the file content and converting it to OkHttp RequestBody. However once the stream is read and we would like to try again the input stream with file is exhausted. We will have to reset its state somehow to allow for retries.~~
Postpone - this works the same way as it was before.
BoxFileUploadSession
The BoxFileUploadSession basically sends multiple multiform requests that are splitting file to parts. This should be doable. Same as with other uploads there is as outstanding issue with exhausted InputStream.
Redirects
~~When uploading a file Box returns new URL where file must be uploaded. The BoxAPIConnection was handling that but the OkHttp client can do that as well. We have to figure out which way to go.
With current solution there are warnings reported by OkHttp:
A connection to https://dl.boxcloud.com/ was leaked. which suggest we have to solve redirect problems.~~
Solved - OK Http handles redirects. Another client that is not following redirects was added and used when we do not want to follow them but extract data from response.
Proxy
OkHttp supports way more proxies than we do. We have to prepare new abstraction for setting proxy.
Closing connection
BoxAPIResponse holds response from OK HTTP as it contains stream of data. When this data is consumed by reading body or writing it to destination (binary data) we are closing the stream and it should unlock HTTP connection. However what if user would not read response body and ignore it. Then warnings from OK HTTP will appear about leaked connections. Maybe we should add this info to documentation. Or maybe BoxAPIResponse should implement Closable interface and this might help users to close stream automatically.
Things to remove
-
BatchAPIRequestthis deprecated class will not be converted
Pull Request Test Coverage Report for Build #2731
- 1182 of 1512 (78.17%) changed or added relevant lines in 58 files are covered.
- 104 unchanged lines in 18 files lost coverage.
- Overall coverage increased (+0.8%) to 71.133%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| src/main/java/com/box/sdk/BoxFileVersion.java | 3 | 4 | 75.0% |
| src/main/java/com/box/sdk/BoxJSONRequest.java | 6 | 7 | 85.71% |
| src/main/java/com/box/sdk/BoxLogger.java | 0 | 2 | 0.0% |
| src/main/java/com/box/sdk/BoxWebHook.java | 12 | 14 | 85.71% |
| src/main/java/com/box/sdk/EventStream.java | 19 | 21 | 90.48% |
| src/main/java/com/box/sdk/BinaryBodyUtils.java | 14 | 17 | 82.35% |
| src/main/java/com/box/sdk/BoxCollection.java | 9 | 12 | 75.0% |
| src/main/java/com/box/sdk/BoxFileRequest.java | 13 | 16 | 81.25% |
| src/main/java/com/box/sdk/BoxJSONResponse.java | 1 | 4 | 25.0% |
| src/main/java/com/box/sdk/BoxComment.java | 8 | 12 | 66.67% |
| <!-- | Total: | 1182 | 1512 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| src/main/java/com/box/sdk/BoxResourceIterable.java | 1 | 83.33% |
| src/main/java/com/box/sdk/BoxSignRequest.java | 1 | 76.74% |
| src/main/java/com/box/sdk/BoxTermsOfService.java | 1 | 60.87% |
| src/main/java/com/box/sdk/BoxTermsOfServiceUserStatus.java | 1 | 54.84% |
| src/main/java/com/box/sdk/MetadataQuery.java | 1 | 92.59% |
| src/main/java/com/box/sdk/BoxFileUploadSession.java | 2 | 64.67% |
| src/main/java/com/box/sdk/MetadataTemplate.java | 2 | 66.94% |
| src/main/java/com/box/sdk/BoxLogger.java | 3 | 29.03% |
| src/main/java/com/box/sdk/EventStream.java | 3 | 86.46% |
| src/main/java/com/box/sdk/RealtimeServerConnection.java | 3 | 80.56% |
| <!-- | Total: | 104 |
| Totals | |
|---|---|
| Change from base Build #2716: | 0.8% |
| Covered Lines: | 7092 |
| Relevant Lines: | 9970 |
