feat(gax): add ResumableUploadClient and types for startUpload() - #14127
feat(gax): add ResumableUploadClient and types for startUpload()#14127whowes wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces low-level resumable upload client interfaces and request/session models, including ResumableUploadClient, ResumableUploadSession, and StartUploadRequest, along with their corresponding unit tests. The review feedback suggests improving the path normalization in StartUploadRequest to strip multiple leading slashes rather than just a single one, and adding corresponding test cases to verify this behavior.
| if (getPath() != null && getPath().startsWith("/")) { | ||
| setPath(getPath().substring(1)); | ||
| } |
There was a problem hiding this comment.
The current implementation only removes a single leading slash. If the input path contains multiple leading slashes (e.g., //v1/upload), the resulting path will still start with a slash (e.g., /v1/upload), which can lead to malformed URLs or signature mismatches when appended to a base URL.
Consider removing all leading slashes using a regular expression replacement.
| if (getPath() != null && getPath().startsWith("/")) { | |
| setPath(getPath().substring(1)); | |
| } | |
| String path = getPath(); | |
| if (path != null) { | |
| setPath(path.replaceFirst("^/+", "")); | |
| } |
| @CsvSource({ | ||
| "/v1/upload, v1/upload", | ||
| "v1/upload, v1/upload", | ||
| "/, ''" | ||
| }) |
There was a problem hiding this comment.
Before adding new test cases for input validation, verify the existing test suite to ensure these scenarios are not already covered. If they are not, add test cases to verify that multiple leading slashes are also correctly normalized and stripped from the path.
@CsvSource({
"/v1/upload, v1/upload",
"//v1/upload, v1/upload",
"///v1/upload, v1/upload",
"v1/upload, v1/upload",
"/, ''"
})References
- Before adding new test cases for input validation (such as non-numeric values or unrecognized properties), verify the existing test suite to ensure these scenarios are not already covered.
whowes
changed the title
2c264da to
c0e396a
Compare
|
|
1 participant
HTTP/JSON client implementation for startUpload is in #14091.