Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import com.google.api.core.InternalApi;
import com.google.api.gax.resumable.ChunkUploadRequest;
import com.google.api.gax.resumable.ChunkUploadResponse;
import com.google.api.gax.resumable.QueryStatusRequest;
import com.google.api.gax.resumable.QueryStatusResponse;
import com.google.api.gax.resumable.ResumableUploadClient;
import com.google.api.gax.resumable.ResumableUploadSession;
import com.google.api.gax.retrying.ExponentialRetryAlgorithm;
Expand Down Expand Up @@ -78,6 +80,9 @@ public class ResumableUploadCallableImpl<RequestT, ResponseT>
private final ClientContext clientContext;
private final UnaryCallable<ChunkUploadRequest, ChunkUploadResponse<ResponseT>>
retryingUploadChunkCallable;
private final UnaryCallable<QueryStatusRequest, QueryStatusResponse<ResponseT>>
retryingQueryCallable;
private final ExponentialRetryAlgorithm recoveryAlgorithm;

public ResumableUploadCallableImpl(
ResumableUploadClient<RequestT, ResponseT> client,
Expand All @@ -90,6 +95,11 @@ public ResumableUploadCallableImpl(
this.retryingUploadChunkCallable =
createRetryingCallable(
client.uploadChunkCallable(), ResumableUploadCommand.UPLOAD, clientContext);
this.retryingQueryCallable =
createRetryingCallable(
client.queryStatusCallable(), ResumableUploadCommand.QUERY, clientContext);
this.recoveryAlgorithm =
new ExponentialRetryAlgorithm(RETRY_SETTINGS, clientContext.getClock());
}

@Override
Expand All @@ -113,9 +123,11 @@ public ResumableUploadFuture<ResponseT> futureCall(
return ResumableUploadFutureImpl.create(
startFuture,
retryingUploadChunkCallable,
retryingQueryCallable,
payload,
effectiveSettings,
clientContext.getDefaultCallContext());
clientContext,
recoveryAlgorithm);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,19 @@
import com.google.api.core.SettableApiFuture;
import com.google.api.gax.resumable.ChunkUploadRequest;
import com.google.api.gax.resumable.ChunkUploadResponse;
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.retrying.ExponentialRetryAlgorithm;
import com.google.api.gax.retrying.TimedAttemptSettings;
import com.google.api.gax.rpc.ResumableUploadErrorClassifier.Category;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The @Nullable annotation is used in this file (on line 269) but org.jspecify.annotations.Nullable is not imported. This will cause a compilation error. Please add the import.

Suggested change
import com.google.api.gax.rpc.ResumableUploadErrorClassifier.Category;
import com.google.api.gax.rpc.ResumableUploadErrorClassifier.Category;
import org.jspecify.annotations.Nullable;
References
  1. When adopting nullness annotations (such as JSpecify), prioritize adding @Nullable annotations to document nullability, even if doing so temporarily triggers static analysis warnings.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

False positive; this import is present in the file as the final import.

import com.google.common.util.concurrent.MoreExecutors;
import java.io.IOException;
import java.io.InputStream;
import java.util.concurrent.CancellationException;
import java.util.concurrent.Future;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

Expand All @@ -56,23 +65,37 @@

private final UnaryCallable<ChunkUploadRequest, ChunkUploadResponse<ResponseT>>
uploadChunkCallable;
private final UnaryCallable<QueryStatusRequest, QueryStatusResponse<ResponseT>>
queryStatusCallable;
private final String uploadUrl;
private final RewindableStreamBuffer buffer;
private final ApiCallContext callContext;
private final ExponentialRetryAlgorithm recoveryAlgorithm;
private final ScheduledExecutorService executor;
private final SettableApiFuture<ResponseT> uploadResultFuture = SettableApiFuture.create();
private volatile @Nullable ApiFuture<?> inFlightFuture;
private volatile @Nullable Future<?> inFlightFuture;

Check warning on line 76 in sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ResumableUploadChunkCoordinator.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Use a thread-safe type; adding "volatile" is not enough to make this field thread-safe.

See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_showcase&issues=AaDWe41WFzxs7Ja-ZOY3&open=AaDWe41WFzxs7Ja-ZOY3&pullRequest=14424
private TimedAttemptSettings recoverySettings;
private boolean madeProgressSinceRecovery = true;

ResumableUploadChunkCoordinator(
UnaryCallable<ChunkUploadRequest, ChunkUploadResponse<ResponseT>> uploadChunkCallable,
UnaryCallable<QueryStatusRequest, QueryStatusResponse<ResponseT>> queryStatusCallable,
String uploadUrl,
InputStream payload,
int chunkSize,
ApiCallContext callContext) {
ApiCallContext callContext,
ExponentialRetryAlgorithm recoveryAlgorithm,
ScheduledExecutorService executor) {
this.uploadChunkCallable =
checkNotNull(uploadChunkCallable, "uploadChunkCallable must not be null");
this.queryStatusCallable =
checkNotNull(queryStatusCallable, "queryStatusCallable must not be null");
this.uploadUrl = checkNotNull(uploadUrl, "uploadUrl must not be null");
checkNotNull(payload, "payload must not be null");
this.callContext = checkNotNull(callContext, "callContext must not be null");
this.recoveryAlgorithm = checkNotNull(recoveryAlgorithm, "recoveryAlgorithm must not be null");
this.executor = checkNotNull(executor, "executor must not be null");
this.recoverySettings = recoveryAlgorithm.createFirstAttempt();
this.buffer = new RewindableStreamBuffer(payload, chunkSize, uploadUrl);
}

Expand All @@ -83,42 +106,35 @@
void start() {
uploadResultFuture.addListener(
() -> {
ApiFuture<?> inFlight = inFlightFuture;
Future<?> inFlight = inFlightFuture;
if (uploadResultFuture.isCancelled() && inFlight != null) {
inFlight.cancel(true);
}
},
MoreExecutors.directExecutor());
transmitChunk();
try {
buffer.fill();
transmitChunk();
} catch (Throwable t) {

Check warning on line 118 in sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ResumableUploadChunkCoordinator.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Catch Exception instead of Throwable.

See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_showcase&issues=AaDJ_WhX4UBITZRG0xId&open=AaDJ_WhX4UBITZRG0xId&pullRequest=14424
uploadResultFuture.setException(t);
}
}

private void transmitChunk() {

Check failure on line 123 in sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ResumableUploadChunkCoordinator.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_showcase&issues=AaDWe41WFzxs7Ja-ZOY4&open=AaDWe41WFzxs7Ja-ZOY4&pullRequest=14424
try {
// Abort if the session was already completed or canceled.
if (uploadResultFuture.isDone()) {
return;
}

// Read the next chunk slice from the payload stream.
buffer.fill();

// Determine if this is the final chunk and build the chunk request.
ChunkUploadRequest chunkRequest =
ChunkUploadRequest.newBuilder()
.setUploadUrl(uploadUrl)
.setPayload(buffer.getPayload())
.setOffset(buffer.getBufferBaseOffset())
.setFinal(buffer.isFinal())
.build();
ChunkUploadRequest chunkRequest = buildCurrentChunkRequest();

// Dispatch the chunk upload call and register the in-flight future for cancellation.
boolean isFinal = chunkRequest.isFinal();
ApiFuture<ChunkUploadResponse<ResponseT>> chunkFuture =
uploadChunkCallable.futureCall(chunkRequest, callContext);
if (!tryRegisterInFlightFuture(chunkFuture)) {
return;
}

ApiFutures.addCallback(
chunkFuture,
new ApiFutureCallback<ChunkUploadResponse<ResponseT>>() {
Expand All @@ -127,15 +143,88 @@
if (uploadResultFuture.isDone()) {
return;
}
if (response.getUploadStatus() == ResumableUploadStatus.FINAL) {
uploadResultFuture.set(response.getResponse());
} else if (isFinal) {
uploadResultFuture.setException(
if (response.getUploadStatus() == ResumableUploadStatus.UNKNOWN) {
recover(
new IllegalStateException(
"Upload stream ended and final chunk was transmitted, but server returned"
+ " incomplete status"));
"Chunk upload response missing X-Goog-Upload-Status header for upload URL: "
+ uploadUrl));
} else {
transmitChunk();
try {
handleChunkResponse(response);
} catch (Throwable t) {

Check warning on line 154 in sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ResumableUploadChunkCoordinator.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Catch Exception instead of Throwable.

See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_showcase&issues=AaDWe41WFzxs7Ja-ZOY5&open=AaDWe41WFzxs7Ja-ZOY5&pullRequest=14424
uploadResultFuture.setException(t);
}
}
}

@Override
public void onFailure(Throwable t) {
if (t instanceof CancellationException || uploadResultFuture.isDone()) {
return;
}
Category category =
ResumableUploadErrorClassifier.classify(t, ResumableUploadCommand.UPLOAD);
if (category == Category.RECOVERABLE) {
recover(t);
} else {
// Category.TRANSIENT errors reaching here have already exhausted their retry budget
// in the underlying RetryingCallable and become fatal per protocol specification.
uploadResultFuture.setException(t);
}
}
},
MoreExecutors.directExecutor());
} catch (Throwable t) {
uploadResultFuture.setException(t);
}
}

private void recover(Throwable cause) {
try {
if (madeProgressSinceRecovery) {
recoverySettings =
recoveryAlgorithm.createNextAttempt(recoveryAlgorithm.createFirstAttempt());
} else {
recoverySettings = recoveryAlgorithm.createNextAttempt(recoverySettings);
}
madeProgressSinceRecovery = false;
if (!recoveryAlgorithm.shouldRetry(recoverySettings)) {
uploadResultFuture.setException(cause);
return;
}
tryRegisterInFlightFuture(
executor.schedule(
this::queryStatus,
recoverySettings.getRandomizedRetryDelayDuration().toNanos(),
TimeUnit.NANOSECONDS));
} catch (Throwable t) {

Check warning on line 200 in sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ResumableUploadChunkCoordinator.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Catch Exception instead of Throwable.

See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_showcase&issues=AaDWe41WFzxs7Ja-ZOY6&open=AaDWe41WFzxs7Ja-ZOY6&pullRequest=14424
uploadResultFuture.setException(t);
}
}

private void queryStatus() {
try {
if (uploadResultFuture.isDone()) {
return;
}
// Dispatch the query status call and register the in-flight future for cancellation.
ApiFuture<QueryStatusResponse<ResponseT>> queryFuture =
queryStatusCallable.futureCall(QueryStatusRequest.create(uploadUrl), callContext);
if (!tryRegisterInFlightFuture(queryFuture)) {
return;
}
ApiFutures.addCallback(
queryFuture,
new ApiFutureCallback<QueryStatusResponse<ResponseT>>() {
@Override
public void onSuccess(QueryStatusResponse<ResponseT> queryResponse) {
if (uploadResultFuture.isDone()) {
return;
}
try {
handleQueryResponse(queryResponse);
} catch (Throwable t) {

Check warning on line 226 in sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ResumableUploadChunkCoordinator.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Catch Exception instead of Throwable.

See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_showcase&issues=AaDJ_WhX4UBITZRG0xIe&open=AaDJ_WhX4UBITZRG0xIe&pullRequest=14424
uploadResultFuture.setException(t);
}
}

Expand All @@ -157,12 +246,58 @@
* Registers the in-flight future for possible cancellation, returning false if the upload was
* already cancelled.
*/
private boolean tryRegisterInFlightFuture(ApiFuture<?> future) {
private boolean tryRegisterInFlightFuture(Future<?> future) {
this.inFlightFuture = future;
if (uploadResultFuture.isCancelled()) {
future.cancel(true);
return false;
}
return true;
}

private void handleQueryResponse(QueryStatusResponse<ResponseT> queryResponse)
throws IOException {
if (queryResponse.getUploadStatus() == ResumableUploadStatus.UNKNOWN) {
throw new IllegalStateException(
"Query status response missing X-Goog-Upload-Status header for upload URL: " + uploadUrl);
}
if (queryResponse.getUploadStatus() == ResumableUploadStatus.FINAL) {
uploadResultFuture.set(queryResponse.getResponse());
return;
}
Long committedOffset = queryResponse.getCommittedOffset();
if (committedOffset == null) {
throw new IllegalStateException(
"Incomplete query status response did not include a committed offset for upload URL: "
+ uploadUrl);
}
buffer.realignTo(committedOffset);
transmitChunk();
}

private void handleChunkResponse(ChunkUploadResponse<ResponseT> response) throws IOException {
if (response.getUploadStatus() == ResumableUploadStatus.FINAL) {
uploadResultFuture.set(response.getResponse());
} else if (buffer.isFinal()) {
uploadResultFuture.setException(
new IllegalStateException(
"Upload stream ended and final chunk was transmitted, but server returned"
+ " incomplete status for upload URL: "
+ uploadUrl));
} else {
madeProgressSinceRecovery = true;
buffer.fill();
transmitChunk();
}
}

private ChunkUploadRequest buildCurrentChunkRequest() {
// Determine if this is the final chunk and build the chunk request.
return ChunkUploadRequest.newBuilder()
.setUploadUrl(uploadUrl)
.setPayload(buffer.getPayload())
.setOffset(buffer.getBufferBaseOffset())
.setFinal(buffer.isFinal())
.build();
}
}
Loading
Loading