feat(gax): implement uploadChunk in HttpJsonResumableUploadClient

[whowes]

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[gemini-code-assist]

Code Review

This pull request introduces support for chunked resumable uploads in the HTTP/JSON transport layer of gax-java. It adds the ChunkUploadRequest and ChunkUploadResponse value objects, updates the ResumableUploadClient interface and its implementation HttpJsonResumableUploadClient to support transmitting individual chunks, and renames StartUploadRequest.builder() to newBuilder() for consistency. Comprehensive unit tests have also been added. The review feedback suggests several improvements: explicitly failing the upload if a 308 Resume Incomplete status is returned without a valid committed offset header to prevent data corruption, removing redundant null checks on non-nullable payload fields, and cleaning up an unnecessary type cast to HttpJsonCallContext.

[gemini-code-assist]

If the server returns 308 Resume Incomplete but the X-Goog-Upload-Size-Received header is missing or malformed (causing committedOffset to remain -1), falling back to assuming the entire chunk was committed (request.getOffset() + request.getPayload().size()) can lead to silent data corruption or upload gaps. We should explicitly fail the upload if statusCode == 308 and committedOffset < 0.

      if ((statusCode >= 200 && statusCode < 300) || statusCode == 308) {
        if (statusCode == 308 && committedOffset < 0) {
          future.setException(
              ApiExceptionFactory.createException(
                  "Server returned 308 Resume Incomplete but the "
                      + UPLOAD_SIZE_RECEIVED_HEADER
                      + " header was missing or invalid.",
                  /* cause= */ null,
                  HttpJsonStatusCode.of(statusCode),
                  /* retryable= */ false));
          return;
        }
        long confirmedOffset =
            committedOffset >= 0
                ? committedOffset
                : request.getOffset() + request.getPayload().size();

[gemini-code-assist]

The null check request.getPayload() != null is redundant because ChunkUploadRequest is annotated with @NullMarked and getPayload() returns a non-nullable ByteString. We can safely simplify this to just check if the payload is empty.

                  if (!request.getPayload().isEmpty()) {

References

1. Avoid adding defensive null checks for values that are guaranteed to be non-null by design, as this can hide invariant breaks.

[gemini-code-assist]

The null check request.getPayload() != null is redundant because ChunkUploadRequest is annotated with @NullMarked and getPayload() returns a non-nullable ByteString. We can safely simplify this to just check if the payload is empty.

Suggested change

	(request.getPayload() != null && !request.getPayload().isEmpty())
	(!request.getPayload().isEmpty())

References

1. Avoid adding defensive null checks for values that are guaranteed to be non-null by design, as this can hide invariant breaks.

[gemini-code-assist]

The cast to (HttpJsonCallContext) is redundant here. Since HttpJsonCallContext.createDefault().nullToSelf(...).merge(...) already returns HttpJsonCallContext (via covariant return types), and withExtraHeaders is called on it, the resulting expression is already of type HttpJsonCallContext.

        HttpJsonCallContext context =
            HttpJsonCallContext.createDefault()
                .nullToSelf(clientContext.getDefaultCallContext())
                .merge(inputContext)
                .withExtraHeaders(chunkHeaders);

[sonarqubecloud]

Quality Gate passed for 'gapic-generator-java-root'

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
86.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[sonarqubecloud]

Quality Gate failed for 'gapic-generator-java-root'

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud