feat(gax): include upload-status header in resumable upload command response objects #14420
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ | |
| import com.google.api.core.ApiFuture; | ||
| import com.google.api.gax.resumable.QueryStatusRequest; | ||
| import com.google.api.gax.resumable.QueryStatusResponse; | ||
| import com.google.api.gax.resumable.ResumableUploadStatus; | ||
| import com.google.api.gax.rpc.ApiCallContext; | ||
| import com.google.api.gax.rpc.ApiExceptionFactory; | ||
| import com.google.api.gax.rpc.ClientContext; | ||
|
|
@@ -63,7 +64,6 @@ class ResumableUploadQueryStatusCallable<ResponseT> | |
| private static final String UPLOAD_COMMAND_HEADER = "X-Goog-Upload-Command"; | ||
| private static final String UPLOAD_STATUS_HEADER = "X-Goog-Upload-Status"; | ||
| private static final String UPLOAD_SIZE_RECEIVED_HEADER = "X-Goog-Upload-Size-Received"; | ||
| private static final String COMMAND_QUERY = "query"; | ||
|
|
||
| private static final Map<String, List<String>> QUERY_STATUS_HEADERS = | ||
|
|
@@ -182,7 +182,7 @@ private static class QueryStatusResponseListener<ResponseT> | |
|
|
||
| private final ResumableUploadHttpJsonFuture<QueryStatusResponse<ResponseT>> future; | ||
| private final HttpResponseParser<ResponseT> responseParser; | ||
| private ResumableUploadStatus uploadStatus = ResumableUploadStatus.UNKNOWN; | ||
| @Nullable private Long committedOffset = null; | ||
| @Nullable private Throwable headerParsingException; | ||
| private String responseBody = ""; | ||
|
|
@@ -197,7 +197,9 @@ private QueryStatusResponseListener( | |
| @Override | ||
| public void onHeaders(HttpJsonMetadata responseHeaders) { | ||
| Map<String, Object> headers = responseHeaders.getHeaders(); | ||
| this.uploadStatus = | ||
| ResumableUploadStatus.fromHeader( | ||
| HttpHeadersUtils.getSingleHeader(headers, UPLOAD_STATUS_HEADER)); | ||
| try { | ||
| this.committedOffset = parseSizeReceived(responseHeaders); | ||
| } catch (Throwable t) { | ||
|
|
@@ -215,24 +217,26 @@ public void onMessage(@Nullable String message) { | |
| @Override | ||
| public void onClose(int statusCode, HttpJsonMetadata trailers) { | ||
| try { | ||
| if (statusCode >= 200 | ||
| && statusCode < 300 | ||
| && uploadStatus != ResumableUploadStatus.CANCELLED) { | ||
| if (headerParsingException != null) { | ||
| future.setException(headerParsingException); | ||
| return; | ||
| } | ||
| if (uploadStatus == ResumableUploadStatus.FINAL) { | ||
| InputStream stream = | ||
| new ByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8)); | ||
| future.set( | ||
| QueryStatusResponse.<ResponseT>newBuilder() | ||
| .setUploadStatus(uploadStatus) | ||
| .setResponse(responseParser.parse(stream)) | ||
| .build()); | ||
| } else if (committedOffset != null) { | ||
| future.set( | ||
| QueryStatusResponse.<ResponseT>newBuilder() | ||
| .setCommittedOffset(committedOffset) | ||
| .setUploadStatus(uploadStatus) | ||
| .build()); | ||
| } else { | ||
| future.setException( | ||
|
|
@@ -244,6 +248,11 @@ public void onClose(int statusCode, HttpJsonMetadata trailers) { | |
| HttpJsonStatusCode.of(StatusCode.Code.INTERNAL), | ||
| /* retryable= */ false)); | ||
| } | ||
| } else if (uploadStatus == ResumableUploadStatus.CANCELLED | ||
| || uploadStatus == ResumableUploadStatus.FINAL) { | ||
| future.setException( | ||
| ResumableUploadChunkCallable.createServerRejectionException( | ||
| statusCode, uploadStatus, trailers)); | ||
| } else { | ||
| Throwable cause = trailers.getException(); | ||
| future.setException( | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,10 +45,11 @@ | |
| import com.google.api.gax.resumable.QueryStatusRequest; | ||
| import com.google.api.gax.resumable.QueryStatusResponse; | ||
| import com.google.api.gax.resumable.ResumableUploadSession; | ||
| import com.google.api.gax.resumable.ResumableUploadStatus; | ||
| import com.google.api.gax.rpc.AbortedException; | ||
| import com.google.api.gax.rpc.ApiCallContext; | ||
| import com.google.api.gax.rpc.ClientContext; | ||
| import com.google.api.gax.rpc.FailedPreconditionException; | ||
| import com.google.api.gax.rpc.InternalException; | ||
| import com.google.api.gax.rpc.NotFoundException; | ||
| import com.google.api.gax.rpc.StatusCode; | ||
|
|
@@ -260,8 +261,8 @@ void uploadChunk_intermediateChunk_sendsUploadCommandAndReturnsActiveStatus() { | |
|
|
||
| ChunkUploadResponse<String> response = client.uploadChunkCallable().call(request); | ||
|
|
||
| assertThat(response.getResponse()).isNull(); | ||
| assertThat(response.getUploadStatus()).isEqualTo(ResumableUploadStatus.ACTIVE); | ||
|
|
||
| assertThat(transport.capturedUrl).isEqualTo(TEST_UPLOAD_URL); | ||
| assertThat(transport.capturedHeaders.get("x-goog-upload-command")).containsExactly("upload"); | ||
|
|
@@ -289,9 +290,9 @@ void uploadChunk_finalChunk_sendsUploadFinalizeAndReturnsResponseBody() { | |
|
|
||
| ChunkUploadResponse<String> response = client.uploadChunkCallable().call(request); | ||
|
|
||
| assertThat(response.getResponse()) | ||
| .isEqualTo("{\"name\":\"uploaded-file.txt\",\"size\":524288}"); | ||
| assertThat(response.getUploadStatus()).isEqualTo(ResumableUploadStatus.FINAL); | ||
|
|
||
| assertThat(transport.capturedHeaders.get("x-goog-upload-command")) | ||
| .containsExactly("upload, finalize"); | ||
|
|
@@ -318,9 +319,9 @@ void uploadChunk_emptyPayloadFinal_sendsFinalizeCommandAndReturnsResponseBody() | |
|
|
||
| ChunkUploadResponse<String> response = client.uploadChunkCallable().call(request); | ||
|
|
||
| assertThat(response.getResponse()) | ||
| .isEqualTo("{\"name\":\"uploaded-file.txt\",\"size\":1048576}"); | ||
| assertThat(response.getUploadStatus()).isEqualTo(ResumableUploadStatus.FINAL); | ||
|
|
||
| assertThat(transport.capturedHeaders.get("x-goog-upload-command")).containsExactly("finalize"); | ||
| assertThat(transport.capturedHeaders).doesNotContainKey("x-goog-upload-offset"); | ||
|
|
@@ -379,7 +380,7 @@ void uploadChunk_serverReturnsConflictOrError_throwsException() { | |
| } | ||
|
|
||
| @Test | ||
| void uploadChunk_missingUploadStatusHeader_returnsUnknownUploadStatusOnHttp200() { | ||
| MockLowLevelHttpResponse httpResponse = new MockLowLevelHttpResponse(); | ||
| httpResponse.setStatusCode(200); | ||
|
|
||
|
|
@@ -391,18 +392,14 @@ void uploadChunk_missingUploadStatusHeader_throwsInternalException() { | |
| .setOffset(0L) | ||
| .build(); | ||
|
|
||
| ChunkUploadResponse<String> response = client.uploadChunkCallable().call(request); | ||
|
|
||
| assertThat(response.getResponse()).isNull(); | ||
| assertThat(response.getUploadStatus()).isEqualTo(ResumableUploadStatus.UNKNOWN); | ||
| } | ||
|
|
||
| @Test | ||
| void uploadChunk_serverReturnsFinalStatusOnNon200_throwsFailedPreconditionException() { | ||
| MockLowLevelHttpResponse httpResponse = new MockLowLevelHttpResponse(); | ||
| httpResponse.setStatusCode(503); | ||
| httpResponse.addHeader("X-Goog-Upload-Status", "final"); | ||
|
|
@@ -420,10 +417,36 @@ void uploadChunk_serverReturnsFinalStatusOnNon200_marksExceptionNonRetryable() { | |
| assertThrows( | ||
| ExecutionException.class, () -> client.uploadChunkCallable().futureCall(request).get()); | ||
|
|
||
| assertThat(exception.getCause()).isInstanceOf(FailedPreconditionException.class); | ||
| FailedPreconditionException failedPrecondition = | ||
| (FailedPreconditionException) exception.getCause(); | ||
| assertThat(failedPrecondition.getStatusCode().getCode()) | ||
| .isEqualTo(StatusCode.Code.FAILED_PRECONDITION); | ||
| } | ||
|
|
||
| @Test | ||
| void uploadChunk_serverReturnsCancelledStatus_throwsFailedPreconditionException() { | ||
| MockLowLevelHttpResponse httpResponse = new MockLowLevelHttpResponse(); | ||
| httpResponse.setStatusCode(200); | ||
| httpResponse.addHeader("X-Goog-Upload-Status", "cancelled"); | ||
|
|
||
| HttpJsonResumableUploadClient<TestRequest, String> client = createClient(httpResponse); | ||
| ChunkUploadRequest request = | ||
| ChunkUploadRequest.newBuilder() | ||
| .setUploadUrl(TEST_UPLOAD_URL) | ||
| .setPayload("data".getBytes(StandardCharsets.UTF_8)) | ||
| .setOffset(0L) | ||
| .build(); | ||
|
|
||
| ExecutionException exception = | ||
| assertThrows( | ||
| ExecutionException.class, () -> client.uploadChunkCallable().futureCall(request).get()); | ||
|
|
||
| assertThat(exception.getCause()).isInstanceOf(FailedPreconditionException.class); | ||
| FailedPreconditionException failedPrecondition = | ||
| (FailedPreconditionException) exception.getCause(); | ||
| assertThat(failedPrecondition.getStatusCode().getCode()) | ||
| .isEqualTo(StatusCode.Code.FAILED_PRECONDITION); | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -439,9 +462,9 @@ void queryStatus_activeUpload_returnsCommittedOffset() { | |
|
|
||
| QueryStatusResponse<String> response = client.queryStatusCallable().call(request); | ||
|
|
||
| assertThat(response.getCommittedOffset()).isEqualTo(524288L); | ||
| assertThat(response.getResponse()).isNull(); | ||
| assertThat(response.getUploadStatus()).isEqualTo(ResumableUploadStatus.ACTIVE); | ||
|
|
||
| assertThat(transport.capturedHeaders.get("x-goog-upload-command")).containsExactly("query"); | ||
| } | ||
|
|
@@ -458,10 +481,10 @@ void queryStatus_finalUpload_returnsCompleteAndResponseBody() { | |
|
|
||
| QueryStatusResponse<String> response = client.queryStatusCallable().call(request); | ||
|
|
||
| assertThat(response.getCommittedOffset()).isNull(); | ||
| assertThat(response.getResponse()) | ||
| .isEqualTo("{\"name\":\"uploaded-file.txt\",\"size\":1048576}"); | ||
| assertThat(response.getUploadStatus()).isEqualTo(ResumableUploadStatus.FINAL); | ||
| } | ||
|
|
||
| @Test | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,35 +46,36 @@ | |
| @AutoValue | ||
| public abstract class ChunkUploadResponse<ResponseT> { | ||
|
|
||
| /** | ||
| * The response object returned by the server upon final completion (e.g. metadata of the uploaded | ||
| * resource), or {@code null} if the upload is still in progress. | ||
| */ | ||
| public abstract @Nullable ResponseT getResponse(); | ||
|
|
||
| /** Returns the status of the upload session returned by the server. */ | ||
| public abstract ResumableUploadStatus getUploadStatus(); | ||
|
|
||
| public abstract Builder<ResponseT> toBuilder(); | ||
|
|
||
| public static <ResponseT> Builder<ResponseT> newBuilder() { | ||
| return new AutoValue_ChunkUploadResponse.Builder<ResponseT>() | ||
| .setUploadStatus(ResumableUploadStatus.ACTIVE); | ||
| } | ||
|
|
||
| public static <ResponseT> ChunkUploadResponse<ResponseT> create( | ||
| ResumableUploadStatus uploadStatus, @Nullable ResponseT response) { | ||
| return new AutoValue_ChunkUploadResponse.Builder<ResponseT>() | ||
| .setUploadStatus(uploadStatus) | ||
| .setResponse(response) | ||
| .build(); | ||
| } | ||
|
|
||
| @AutoValue.Builder | ||
| public abstract static class Builder<ResponseT> { | ||
| public abstract Builder<ResponseT> setResponse(@Nullable ResponseT response); | ||
|
|
||
| public abstract Builder<ResponseT> setUploadStatus(ResumableUploadStatus uploadStatus); | ||
|
|
||
| public abstract ChunkUploadResponse<ResponseT> build(); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of creating a verbose anonymous class implementing
StatusCode, you can use the existingHttpJsonStatusCodeclass which is already available in this package. This simplifies the code and improves maintainability.