Skip to content

feat(gax): add ResumableUploadCallable and ResumableUploadCallSettings - #14052

Open
blakeli0 wants to merge 1 commit into
googleapis:mainfrom
blakeli0:feat/gax-resumable-upload-public-api
Open

feat(gax): add ResumableUploadCallable and ResumableUploadCallSettings#14052
blakeli0 wants to merge 1 commit into
googleapis:mainfrom
blakeli0:feat/gax-resumable-upload-public-api

Conversation

@blakeli0

@blakeli0 blakeli0 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Description

This PR introduces the foundational GAX Public API surface for HTTP/JSON resumable uploads.

Changes

  1. ResumableUploadCallSettings: Non-generic @AutoValue configuration class with merge(other) method and chunkSize = 8MB default.
  2. ResumableUploadCallable: Abstract transport-agnostic base class with a single futureCall(request, payload, settings) method.
  3. ResumableUploadCallSettingsTest: JUnit 5 (Jupiter) unit tests verifying default chunk boundaries and merge(other) overrides.

@blakeli0
blakeli0 requested review from a team as code owners August 12, 2026 05:55

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces the ResumableUploadCallSettings and ResumableUploadCallable classes to support transport-independent resumable uploads, along with associated unit tests. The review feedback suggests adding input validation to ensure chunkSize is positive and totalBytes is non-negative, as well as removing several redundant null casts in the overloaded call and futureCall methods to clean up the code.

Comment on lines +113 to +116
public Builder<RequestT, ResponseT> setChunkSize(Integer chunkSize) {
this.chunkSize = chunkSize;
return this;
}

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.

medium

It is highly recommended to validate that chunkSize is positive when it is configured. This prevents invalid configurations (such as negative or zero chunk sizes) from being set and causing runtime failures during uploads.

    public Builder<RequestT, ResponseT> setChunkSize(Integer chunkSize) {
      if (chunkSize != null) {
        com.google.common.base.Preconditions.checkArgument(
            chunkSize > 0, "chunkSize must be positive: %s", chunkSize);
      }
      this.chunkSize = chunkSize;
      return this;
    }

Comment on lines +123 to +126
public Builder<RequestT, ResponseT> setTotalBytes(Long totalBytes) {
this.totalBytes = totalBytes;
return this;
}

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.

medium

It is highly recommended to validate that totalBytes is non-negative when configured. This prevents invalid negative values from being set.

    public Builder<RequestT, ResponseT> setTotalBytes(Long totalBytes) {
      if (totalBytes != null) {
        com.google.common.base.Preconditions.checkArgument(
            totalBytes >= 0, "totalBytes must be non-negative: %s", totalBytes);
      }
      this.totalBytes = totalBytes;
      return this;
    }

Comment on lines +66 to +71
public ApiFuture<ResponseT> futureCall(
RequestT request,
InputStream payload,
ResumableUploadCallSettings<RequestT, ResponseT> perRequestSettings) {
return futureCall(request, payload, perRequestSettings, (ApiCallContext) null);
}

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.

medium

The cast (ApiCallContext) null is redundant because there is only one 4-argument overload of futureCall, so the compiler can resolve the method call unambiguously. Removing the redundant cast improves code readability.

Suggested change
public ApiFuture<ResponseT> futureCall(
RequestT request,
InputStream payload,
ResumableUploadCallSettings<RequestT, ResponseT> perRequestSettings) {
return futureCall(request, payload, perRequestSettings, (ApiCallContext) null);
}
public ApiFuture<ResponseT> futureCall(
RequestT request,
InputStream payload,
ResumableUploadCallSettings<RequestT, ResponseT> perRequestSettings) {
return futureCall(request, payload, perRequestSettings, null);
}

Comment on lines +76 to +79
public ApiFuture<ResponseT> futureCall(RequestT request, InputStream payload) {
return futureCall(
request, payload, (ResumableUploadCallSettings<RequestT, ResponseT>) null, (ApiCallContext) null);
}

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.

medium

The casts (ResumableUploadCallSettings<RequestT, ResponseT>) null and (ApiCallContext) null are redundant because there is only one 4-argument overload of futureCall. Removing them simplifies the code and improves readability.

  public ApiFuture<ResponseT> futureCall(RequestT request, InputStream payload) {
    return futureCall(request, payload, null, null);
  }

Comment on lines +96 to +101
public ResponseT call(
RequestT request,
InputStream payload,
ResumableUploadCallSettings<RequestT, ResponseT> perRequestSettings) {
return call(request, payload, perRequestSettings, (ApiCallContext) null);
}

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.

medium

The cast (ApiCallContext) null is redundant because there is only one 4-argument overload of call. Removing it improves readability.

Suggested change
public ResponseT call(
RequestT request,
InputStream payload,
ResumableUploadCallSettings<RequestT, ResponseT> perRequestSettings) {
return call(request, payload, perRequestSettings, (ApiCallContext) null);
}
public ResponseT call(
RequestT request,
InputStream payload,
ResumableUploadCallSettings<RequestT, ResponseT> perRequestSettings) {
return call(request, payload, perRequestSettings, null);
}

Comment on lines +106 to +109
public ResponseT call(RequestT request, InputStream payload) {
return call(
request, payload, (ResumableUploadCallSettings<RequestT, ResponseT>) null, (ApiCallContext) null);
}

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.

medium

The casts (ResumableUploadCallSettings<RequestT, ResponseT>) null and (ApiCallContext) null are redundant because there is only one 4-argument overload of call. Removing them simplifies the code.

  public ResponseT call(RequestT request, InputStream payload) {
    return call(request, payload, null, null);
  }

@blakeli0
blakeli0 force-pushed the feat/gax-resumable-upload-public-api branch 8 times, most recently from 483ea79 to c52d225 Compare August 12, 2026 06:31
Clean up ResumableUploadCallSettingsTest JUnit 5 test suite by removing redundant equals/hashCode test for @autovalue. Add ResumableUploadCallable abstract base class.
@blakeli0
blakeli0 force-pushed the feat/gax-resumable-upload-public-api branch from c52d225 to 2cf5521 Compare August 12, 2026 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant