docs(samples): Samples for v4 Signed URL #654
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
googlebot
added
the
cla: yes
JustinBeckwith
approved these changes
Apr 2, 2019
|
That's a fine lookin API! |
frankyn
requested changes
Apr 2, 2019
frankyn
requested changes
Apr 3, 2019
samples/files.js
Outdated
| const options = { | ||
| version: 'v4', | ||
| action: 'read', | ||
| expires: Date.now() + 1000 * 60 * 60, // one hour |
There was a problem hiding this comment.
Missed this in the last review, please change expires time to 15 minutes to be consistent with other language samples. For Read and Upload samples.
bcoe
approved these changes
Apr 3, 2019
| .getSignedUrl(options); | ||
|
|
||
| console.log('Generated GET signed URL:'); | ||
| console.log(url); |
There was a problem hiding this comment.
this is a little wordy, I might be tempted to consolidate in a single template string comment:
console.info(`generated the signed URL ${url} which is valid for one hour`)
There was a problem hiding this comment.
Good call out, but as @frankyn mentioned, we have standardized the outputs of these samples across languages so we'd just have to follow the canonical :)
samples/system-test/files.test.js
Outdated
| `${cmd} generate-v4-upload-signed-url ${bucketName} ${signedFileName}` | ||
| ); | ||
| const regExp = new RegExp( | ||
| 'Generated PUT signed URL:\n' + |
There was a problem hiding this comment.
Again I'd probably try to reduce the size of this regex, perhaps just:
output.match(/URL: ([^ ]+)/)
frankyn
approved these changes
Apr 3, 2019
jkwlui
added a commit
that referenced
this pull request
Apr 4, 2019
* feat: v4 signed url WIP * make version an optinoal * lint * add required host header * fix canonical query params joining * add test for defaulting to v2 * fix v4 signing with test * fix tests * refactor post-signing url construction v2/v4 * add v4 tests: response-content-type and generate parameters * convert signature to HEX * add conformance test * npm run fix * remove reduntant toLowerCase * do not promisify getDate() * add system-test for v4 signing * run conformance test along with system test * fix tests * add content-md5 and content-type to signed headers if given * Object.entries in not ES5 * npm run fix * split out conformance test * test: invalid version param given * mock out Date when testing instead of override Date * use Array.map to build canonical query * refactor: use dateformat library instead of writing a function * refactor: remove unused * remove reference to deleted methods * refactor: use string interpolation and dot accessor * refactor: use method chaining * add a comment about singled valued header * refactor signedHeaders * Number(x) * refactor: use query-string module as url.parse is deprecated * npm run fix * use query-string to build canonicalQueryParams * refactor: verb => method * refactor: use sinon.FakeTimers instead of timekeeper * npm run fix * use async/await for new system-test * use async/await for new test (2) * convert getSignedUrl tests to async/await * refactor: correctly filter out undefined header values * npm run fix * make v4 multi-valued header delimiter consistent with v2, (single `,`, no space) * copy stack trace * fix: replace ALL sequential spaces * OutgoingHttpHeaders use generic type .map; rename flattenObject => objectEntries * refactor: use build-in querystring * break up v2/v4 signed url query interface * add comments about the construction of canonical headers * npm run fix * git merge failed to put code in right place * test: assert SigningError thrown * test(v4): resumable upload * prefer string.includes over indexOf * npm run fix * docs: add a note on POST and X-Goog-Resumable * SEVEN_DAYS = 604800 * emit warning when using default version * emitWarning if using default version for signing * restore stub * add new line * test: v4 signed URL: conformance test (#638) * Revert "split out conformance test" This reverts commit ce00255. * sync conformance-test cases; parse cases without modification * this.skip * refactor: lift interface * remove default v2 warning * docs(samples): Samples for v4 Signed URL (#654) * remove timeout on conformance-test * npm run fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.