Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gensupport): allow initial request for resumable uploads to retry w/ non-nil getBody #1690

Merged
merged 7 commits into from Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 20 additions & 9 deletions internal/gensupport/media.go
Expand Up @@ -289,13 +289,12 @@ func (mi *MediaInfo) UploadRequest(reqHeaders http.Header, body io.Reader) (newB
// be retried because the data is stored in the MediaBuffer.
media, _, _, _ = mi.buffer.Chunk()
}
toCleanup := []io.Closer{}
if media != nil {
fb := readerFunc(body)
fm := readerFunc(media)
combined, ctype := CombineBodyMedia(body, "application/json", media, mi.mType)
toCleanup := []io.Closer{
combined,
}
toCleanup = append(toCleanup, combined)
if fb != nil && fm != nil {
getBody = func() (io.ReadCloser, error) {
rb := ioutil.NopCloser(fb())
Expand All @@ -309,18 +308,30 @@ func (mi *MediaInfo) UploadRequest(reqHeaders http.Header, body io.Reader) (newB
return r, nil
}
}
cleanup = func() {
for _, closer := range toCleanup {
_ = closer.Close()
}

}
reqHeaders.Set("Content-Type", ctype)
body = combined
}
if mi.buffer != nil && mi.mType != "" && !mi.singleChunk {
// This happens when initiating a resumable upload session.
// The initial request includes a body but no media. It can be
cojenco marked this conversation as resolved.
Show resolved Hide resolved
// retried with a getBody function that re-creates the request body.
fb := readerFunc(body)
if fb != nil {
getBody = func() (io.ReadCloser, error) {
rb := ioutil.NopCloser(fb())
toCleanup = append(toCleanup, rb)
return rb, nil
}
}
reqHeaders.Set("X-Upload-Content-Type", mi.mType)
}
// Ensure that any bodies created in getBody are cleaned up.
cleanup = func() {
for _, closer := range toCleanup {
_ = closer.Close()
}

}
return body, getBody, cleanup
}

Expand Down
4 changes: 2 additions & 2 deletions internal/gensupport/media_test.go
Expand Up @@ -310,11 +310,11 @@ func TestUploadRequestGetBody(t *testing.T) {
},
{
desc: "chunk size < data size: MediaBuffer, >1 chunk, no getBody",
cojenco marked this conversation as resolved.
Show resolved Hide resolved
// No getBody here, because the initial request contains no media data
// Test that the initial request results in a getBody function that is non-nil.
// Note that ChunkSize = 1 is rounded up to googleapi.MinUploadChunkSize.
r: &nullReader{2 * googleapi.MinUploadChunkSize},
chunkSize: 1,
wantGetBody: false,
wantGetBody: true,
},
} {
cryptorand.Reader = mathrand.New(mathrand.NewSource(int64(i)))
Expand Down