From 0a51cc0266658184be518587a62e55250f5e60f1 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 10 May 2019 01:42:02 -0400 Subject: [PATCH 1/9] add file.isPublic() function --- src/file.ts | 65 +++++++++++++++++++++++++++++++++++++++++- system-test/storage.ts | 34 ++++------------------ test/file.ts | 34 ++++++++++++++++++++++ 3 files changed, 104 insertions(+), 29 deletions(-) diff --git a/src/file.ts b/src/file.ts index 46a8fa1f0..bd8115822 100644 --- a/src/file.ts +++ b/src/file.ts @@ -57,7 +57,7 @@ import { } from '@google-cloud/common/build/src/util'; const duplexify: DuplexifyConstructor = require('duplexify'); import {normalize, objectEntries} from './util'; -import {Headers} from 'gaxios'; +import {GaxiosError, Headers, request as gaxiosRequest} from 'gaxios'; export type GetExpirationDateResponse = [Date]; export interface GetExpirationDateCallback { @@ -211,6 +211,12 @@ export type MakeFilePrivateResponse = [Metadata]; export interface MakeFilePrivateCallback extends SetFileMetadataCallback {} +export interface IsPublicCallback { + (err: Error | null, resp?: boolean): void; +} + +export type IsPublicResponse = [boolean]; + export type MakeFilePublicResponse = [Metadata]; export interface MakeFilePublicCallback { @@ -2595,6 +2601,63 @@ class File extends ServiceObject { }); } + isPublic(): Promise; + isPublic(callback: IsPublicCallback): void; + /** + * @callback IsPublicCallback + * @param {?Error} err Request error, if any. + * @param {boolean} resp Whether file is public or not. + */ + /** + * @typedef {array} IsPublicResponse + * @property {boolean} 0 Whether file is public or not. + */ + /** + * Check whether this file is public or not. + * @param {IsPublicCallback} [callback] Callback function. + * @returns {Promise} + * + * @example + * const {Storage} = require('@google-cloud/storage'); + * const storage = new Storage(); + * const myBucket = storage.bucket('my-bucket'); + * + * const file = myBucket.file('my-file'); + * + * //- + * // Check whether the file is publicly accessible. + * //- + * file.isPublic(function(err, resp) { + * if (err) { + * console.error(err); + * return; + * } + * console.log(`the file ${file.id} is public: ${resp}`) ; + * }) + * //- + * // If the callback is omitted, we'll return a Promise. + * //- + * file.isPublic().then(function(data) { + * const resp = data[0]; + * }); + */ + + isPublic(callback?: IsPublicCallback): Promise | void { + gaxiosRequest({ + method: 'HEAD', + url: `http://${this.bucket.name}.storage.googleapis.com/${this.id}`, + }).then( + () => callback!(null, true), + (err: GaxiosError) => { + if (err.code === '403') { + callback!(null, false); + } else { + callback!(err); + } + } + ); + } + makePrivate( options?: MakeFilePrivateOptions ): Promise; diff --git a/system-test/storage.ts b/system-test/storage.ts index 4f062ac0c..13504c594 100644 --- a/system-test/storage.ts +++ b/system-test/storage.ts @@ -387,10 +387,8 @@ describe('storage', () => { await bucket.makePublic({includeFiles: true}); const [files] = await bucket.getFiles(); - const resps = await Promise.all( - files.map(file => isFilePublicAsync(file)) - ); - resps.forEach(resp => assert.ok(resp)); + const [resps] = await Promise.all(files.map(file => file.isPublic())); + resps.forEach(resp => assert.strictEqual(resp, true)); await Promise.all([ bucket.acl.default.delete({entity: 'allUsers'}), bucket.deleteFiles(), @@ -419,10 +417,10 @@ describe('storage', () => { await bucket.makePrivate({includeFiles: true}); const [files] = await bucket.getFiles(); - const resps = await Promise.all( - files.map(file => isFilePublicAsync(file)) - ); - resps.forEach(resp => assert.ok(!resp)); + const [resps] = await Promise.all(files.map(file => file.isPublic())); + resps.forEach(resp => { + assert.strictEqual(resp, false); + }); await bucket.deleteFiles(); }); }); @@ -3140,26 +3138,6 @@ describe('storage', () => { ); } - async function isFilePublicAsync(file: File) { - try { - const [aclObject] = await file.acl.get({entity: 'allUsers'}); - if ( - (aclObject as AccessControlObject).entity === 'allUsers' && - (aclObject as AccessControlObject).role === 'READER' - ) { - return true; - } else { - return false; - } - } catch (error) { - if (error.code === 404) { - return false; - } else { - throw error; - } - } - } - // tslint:disable-next-line no-any function createFileAsync(fileObject: any) { return fileObject.file.save(fileObject.contents); diff --git a/test/file.ts b/test/file.ts index ed505a5e3..b90b01c1c 100644 --- a/test/file.ts +++ b/test/file.ts @@ -36,6 +36,7 @@ import * as through from 'through2'; import * as tmp from 'tmp'; import * as url from 'url'; import * as zlib from 'zlib'; +import * as gaxios from 'gaxios'; import { Bucket, @@ -3205,6 +3206,39 @@ describe('File', () => { }); }); + describe('isPublic', () => { + const sandbox = sinon.createSandbox(); + + afterEach(() => sandbox.restore()); + + it('should execute callback with `true` in response', done => { + sandbox.stub(gaxios, 'request').resolves(); + file.isPublic((err: gaxios.GaxiosError, resp: boolean) => { + assert.ifError(err); + assert.strictEqual(resp, true); + done(); + }); + }); + + it('should execute callback with `false` in response', done => { + sandbox.stub(gaxios, 'request').rejects({code: '403'}); + file.isPublic((err: gaxios.GaxiosError, resp: boolean) => { + assert.ifError(err); + assert.strictEqual(resp, false); + done(); + }); + }); + + it('should propagate different error to user', done => { + const error = {code: '400'}; + sandbox.stub(gaxios, 'request').rejects(error as gaxios.GaxiosError); + file.isPublic((err: gaxios.GaxiosError) => { + assert.strictEqual(err, error); + done(); + }); + }); + }); + describe('move', () => { describe('copy to destination', () => { function assertCopyFile( From 49949d00e53d24ed93847ae68daa07c48cf0c69c Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 May 2019 18:29:14 -0400 Subject: [PATCH 2/9] add gaxios dependency --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 52996b395..3b6a42712 100644 --- a/package.json +++ b/package.json @@ -59,6 +59,7 @@ "date-and-time": "^0.6.3", "duplexify": "^3.5.0", "extend": "^3.0.0", + "gaxios": "^2.0.1", "gcs-resumable-upload": "^2.0.0", "hash-stream-validation": "^0.2.1", "mime": "^2.2.0", From 769cc2a7fd2160d6176edc6ce8f307433c375cce Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 May 2019 22:19:55 -0400 Subject: [PATCH 3/9] test file.isPublic() via object without credentials --- system-test/storage.ts | 60 ++++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/system-test/storage.ts b/system-test/storage.ts index 13504c594..7b5043b0b 100644 --- a/system-test/storage.ts +++ b/system-test/storage.ts @@ -209,17 +209,12 @@ describe('storage', () => { bucket = storageWithoutAuth.bucket('gcp-public-data-landsat'); }); - it('should list and download a file', done => { - bucket.getFiles( - { - autoPaginate: false, - }, - (err, files) => { - assert.ifError(err); - const file = files![0]; - file.download(done); - } - ); + it('should list and download a file', async () => { + const [files] = await bucket.getFiles({autoPaginate: false}); + const file = files[0]; + const [isPblc] = await file.isPublic(); + assert.strictEqual(isPblc, true); + assert.doesNotReject(file.download()); }); }); @@ -232,13 +227,14 @@ describe('storage', () => { file = bucket.file(privateFile.id!); }); - it('should not download a file', done => { - file.download(err => { - assert( - err!.message.indexOf('does not have storage.objects.get') > -1 - ); - done(); - }); + it('should not download a file', async () => { + const [isPblc] = await file.isPublic(); + assert.strictEqual(isPblc, false); + assert.rejects( + file.download(), + (err: Error) => + err.message.indexOf('does not have storage.objects.get') > -1 + ); }); it('should not upload a file', done => { @@ -387,7 +383,9 @@ describe('storage', () => { await bucket.makePublic({includeFiles: true}); const [files] = await bucket.getFiles(); - const [resps] = await Promise.all(files.map(file => file.isPublic())); + const resps = await Promise.all( + files.map(file => isFilePublicAsync(file)) + ); resps.forEach(resp => assert.strictEqual(resp, true)); await Promise.all([ bucket.acl.default.delete({entity: 'allUsers'}), @@ -417,7 +415,9 @@ describe('storage', () => { await bucket.makePrivate({includeFiles: true}); const [files] = await bucket.getFiles(); - const [resps] = await Promise.all(files.map(file => file.isPublic())); + const resps = await Promise.all( + files.map(file => isFilePublicAsync(file)) + ); resps.forEach(resp => { assert.strictEqual(resp, false); }); @@ -3138,6 +3138,26 @@ describe('storage', () => { ); } + async function isFilePublicAsync(file: File) { + try { + const [aclObject] = await file.acl.get({entity: 'allUsers'}); + if ( + (aclObject as AccessControlObject).entity === 'allUsers' && + (aclObject as AccessControlObject).role === 'READER' + ) { + return true; + } else { + return false; + } + } catch (error) { + if (error.code === 404) { + return false; + } else { + throw error; + } + } + } + // tslint:disable-next-line no-any function createFileAsync(fileObject: any) { return fileObject.file.save(fileObject.contents); From 73381ffadb496023f8f7a456e5257b536279a8d4 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 14 May 2019 14:09:23 -0400 Subject: [PATCH 4/9] fix spelling and add tests --- src/file.ts | 4 +++- system-test/storage.ts | 8 ++++---- test/file.ts | 23 ++++++++++++++++++++++- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/file.ts b/src/file.ts index bd8115822..25e451639 100644 --- a/src/file.ts +++ b/src/file.ts @@ -2645,7 +2645,9 @@ class File extends ServiceObject { isPublic(callback?: IsPublicCallback): Promise | void { gaxiosRequest({ method: 'HEAD', - url: `http://${this.bucket.name}.storage.googleapis.com/${this.id}`, + url: `http://${ + this.bucket.name + }.storage.googleapis.com/${encodeURIComponent(this.name)}`, }).then( () => callback!(null, true), (err: GaxiosError) => { diff --git a/system-test/storage.ts b/system-test/storage.ts index 7b5043b0b..7e0ac9df4 100644 --- a/system-test/storage.ts +++ b/system-test/storage.ts @@ -212,8 +212,8 @@ describe('storage', () => { it('should list and download a file', async () => { const [files] = await bucket.getFiles({autoPaginate: false}); const file = files[0]; - const [isPblc] = await file.isPublic(); - assert.strictEqual(isPblc, true); + const [isPublic] = await file.isPublic(); + assert.strictEqual(isPublic, true); assert.doesNotReject(file.download()); }); }); @@ -228,8 +228,8 @@ describe('storage', () => { }); it('should not download a file', async () => { - const [isPblc] = await file.isPublic(); - assert.strictEqual(isPblc, false); + const [isPublic] = await file.isPublic(); + assert.strictEqual(isPublic, false); assert.rejects( file.download(), (err: Error) => diff --git a/test/file.ts b/test/file.ts index b90b01c1c..cb3de42ca 100644 --- a/test/file.ts +++ b/test/file.ts @@ -3229,7 +3229,7 @@ describe('File', () => { }); }); - it('should propagate different error to user', done => { + it('should propagate non-403 errors to user', done => { const error = {code: '400'}; sandbox.stub(gaxios, 'request').rejects(error as gaxios.GaxiosError); file.isPublic((err: gaxios.GaxiosError) => { @@ -3237,6 +3237,27 @@ describe('File', () => { done(); }); }); + + it('should correctly send a HEAD request', done => { + const spy = sandbox.spy(gaxios, 'request'); + file.isPublic((err: gaxios.GaxiosError) => { + assert.ifError(err); + assert.strictEqual(spy.calledWithMatch({method: 'HEAD'}), true); + done(); + }); + }); + + it('should correctly format URL in the request', done => { + const expecterURL = `http://${ + BUCKET.name + }.storage.googleapis.com/${encodeURIComponent(file.name)}`; + const spy = sandbox.spy(gaxios, 'request'); + file.isPublic((err: gaxios.GaxiosError) => { + assert.ifError(err); + assert.strictEqual(spy.calledWithMatch({url: expecterURL}), true); + done(); + }); + }); }); describe('move', () => { From 819bbc726fdc19ced74c6d59c66ed8f22a48f3e6 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 14 May 2019 14:18:53 -0400 Subject: [PATCH 5/9] lint --- test/file.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/file.ts b/test/file.ts index cb3de42ca..2259d9c77 100644 --- a/test/file.ts +++ b/test/file.ts @@ -3246,7 +3246,7 @@ describe('File', () => { done(); }); }); - + it('should correctly format URL in the request', done => { const expecterURL = `http://${ BUCKET.name From 2c6bcfb118e3b98438df52b3983c0dea65366d14 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 15 May 2019 10:24:49 -0400 Subject: [PATCH 6/9] use special character in unit test --- test/file.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/file.ts b/test/file.ts index 2259d9c77..6ee1480ec 100644 --- a/test/file.ts +++ b/test/file.ts @@ -3248,6 +3248,7 @@ describe('File', () => { }); it('should correctly format URL in the request', done => { + file = new File(BUCKET, 'my#file$.png'); const expecterURL = `http://${ BUCKET.name }.storage.googleapis.com/${encodeURIComponent(file.name)}`; From 2b2b0002a8cd3ac2db65a5ff72877ff7f8db147c Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 29 May 2019 01:38:28 -0400 Subject: [PATCH 7/9] update documentation to explain the function approach --- src/file.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/file.ts b/src/file.ts index 25e451639..1708d551f 100644 --- a/src/file.ts +++ b/src/file.ts @@ -2613,7 +2613,14 @@ class File extends ServiceObject { * @property {boolean} 0 Whether file is public or not. */ /** - * Check whether this file is public or not. + * Check whether this file is public or not by sending + * a HEAD request without credentials. + * No errors form the server indicates that the current + * file is public. + * A 403-Forbidden error {@link https://cloud.google.com/storage/docs/json_api/v1/status-codes#403_Forbidden} + * indicates that file is private. + * Any other non 403 error is propagated to user. + * * @param {IsPublicCallback} [callback] Callback function. * @returns {Promise} * From df42ac9272d9b3e608032f4aaa9530289b9c2372 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 29 May 2019 02:56:12 -0400 Subject: [PATCH 8/9] lint --- src/file.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/file.ts b/src/file.ts index 1708d551f..501547062 100644 --- a/src/file.ts +++ b/src/file.ts @@ -2614,13 +2614,13 @@ class File extends ServiceObject { */ /** * Check whether this file is public or not by sending - * a HEAD request without credentials. + * a HEAD request without credentials. * No errors form the server indicates that the current * file is public. * A 403-Forbidden error {@link https://cloud.google.com/storage/docs/json_api/v1/status-codes#403_Forbidden} * indicates that file is private. * Any other non 403 error is propagated to user. - * + * * @param {IsPublicCallback} [callback] Callback function. * @returns {Promise} * From 06474dc109e60258d45c51374fc0fa9964d51be9 Mon Sep 17 00:00:00 2001 From: Alex <7764119+AVaksman@users.noreply.github.com> Date: Wed, 29 May 2019 21:14:19 -0400 Subject: [PATCH 9/9] typo --- src/file.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/file.ts b/src/file.ts index 501547062..4e3f2dbac 100644 --- a/src/file.ts +++ b/src/file.ts @@ -2615,7 +2615,7 @@ class File extends ServiceObject { /** * Check whether this file is public or not by sending * a HEAD request without credentials. - * No errors form the server indicates that the current + * No errors from the server indicates that the current * file is public. * A 403-Forbidden error {@link https://cloud.google.com/storage/docs/json_api/v1/status-codes#403_Forbidden} * indicates that file is private.