feat(gax): add ApiCallContext and request-level settings overloads to ResumableUploadCallable - #14251
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces new overloads for futureCall and resumeCall in ResumableUploadCallable to support passing an ApiCallContext alongside ResumableUploadCallSettings. It also updates ResumableUploadCallableImpl to handle these parameters and adds corresponding unit tests. The review feedback highlights that removing the original 3-parameter overloads breaks backward compatibility for existing callers and subclasses, and suggests retaining them as concrete methods that delegate to the new 4-parameter implementations.
| public abstract ResumableUploadFuture<ResponseT> futureCall( | ||
| RequestT request, InputStream payload, @Nullable ResumableUploadCallSettings settings); | ||
| RequestT request, | ||
| InputStream payload, | ||
| @Nullable ApiCallContext context, | ||
| @Nullable ResumableUploadCallSettings settings); |
There was a problem hiding this comment.
Removing the 3-parameter futureCall overload that accepts ResumableUploadCallSettings breaks backward compatibility for existing callers and subclasses. To maintain source and binary compatibility, we should keep the 3-parameter overload as a concrete method that delegates to the new 4-parameter method with a null context.
/**
* Performs a new resumable upload asynchronously with default call context and settings overrides.
*
* @param request the request message
* @param payload the data payload input stream to upload and close
* @param settings request-level call settings overrides; may be {@code null}
* @return future for tracking and controlling the upload
*/
public ResumableUploadFuture<ResponseT> futureCall(
RequestT request, InputStream payload, @Nullable ResumableUploadCallSettings settings) {
return futureCall(request, payload, null, settings);
}
/**
* Performs a new resumable upload asynchronously with call context and settings overrides.
*
* @param request the request message
* @param payload the data payload input stream to upload and close
* @param context call context overrides; may be {@code null}
* @param settings request-level call settings overrides; may be {@code null}
* @return future for tracking and controlling the upload
*/
public abstract ResumableUploadFuture<ResponseT> futureCall(
RequestT request,
InputStream payload,
@Nullable ApiCallContext context,
@Nullable ResumableUploadCallSettings settings);| public abstract ResumableUploadFuture<ResponseT> resumeCall( | ||
| String sessionUrl, InputStream payload, @Nullable ResumableUploadCallSettings settings); | ||
| String sessionUrl, | ||
| InputStream payload, | ||
| @Nullable ApiCallContext context, | ||
| @Nullable ResumableUploadCallSettings settings); |
There was a problem hiding this comment.
Removing the 3-parameter resumeCall overload that accepts ResumableUploadCallSettings breaks backward compatibility for existing callers and subclasses. To maintain source and binary compatibility, we should keep the 3-parameter overload as a concrete method that delegates to the new 4-parameter method with a null context.
/**
* Resumes an existing resumable upload session asynchronously with default call context and settings overrides.
*
* @param sessionUrl the upload session URL
* @param payload the data payload input stream to upload and close
* @param settings request-level call settings overrides; may be {@code null}
* @return future for tracking and controlling the upload
*/
public ResumableUploadFuture<ResponseT> resumeCall(
String sessionUrl, InputStream payload, @Nullable ResumableUploadCallSettings settings) {
return resumeCall(sessionUrl, payload, null, settings);
}
/**
* Resumes an existing resumable upload session asynchronously with call context and settings
* overrides.
*
* @param sessionUrl the upload session URL
* @param payload the data payload input stream to upload and close
* @param context call context overrides; may be {@code null}
* @param settings request-level call settings overrides; may be {@code null}
* @return future for tracking and controlling the upload
*/
public abstract ResumableUploadFuture<ResponseT> resumeCall(
String sessionUrl,
InputStream payload,
@Nullable ApiCallContext context,
@Nullable ResumableUploadCallSettings settings);9b9d299 to
fc4a138
Compare
… ResumableUploadCallable Add 2-, 3-, and 4-argument overloads to ResumableUploadCallable for futureCall and resumeCall, supporting: 1. Default context and settings (used by generated ServiceClient convenience methods). 2. Per-request ApiCallContext overrides for transport metadata (extra headers, credentials). 3. Per-request ResumableUploadCallSettings overrides for state-machine knobs per go/sdk:java-scotty-design. 4. Full method accepting both call context and settings overrides. ResumableUploadCallableImpl performs a 3-tier precedence merge on settings and merges ApiCallContext.
fc4a138 to
3a29329
Compare
|
❌ The last analysis has failed. |
|


Work in progress, not ready for review
This is needed to meet the resumable upload requirement to allow custom headers and other settings to be applied per-call