From 0acebc3cbee3743a062a41b663b8e710437cefe3 Mon Sep 17 00:00:00 2001 From: Jonathan Lui Date: Tue, 6 Aug 2019 17:26:28 -0700 Subject: [PATCH 1/3] fix: optional projectId argument --- src/hmacKey.ts | 19 ++++++++++++++++-- src/storage.ts | 50 +++++++++++++++++++++++++++++++++++++--------- test/hmacKey.ts | 8 ++++++++ test/index.ts | 53 +++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 117 insertions(+), 13 deletions(-) diff --git a/src/hmacKey.ts b/src/hmacKey.ts index 530f9f3ad..45773a729 100644 --- a/src/hmacKey.ts +++ b/src/hmacKey.ts @@ -17,6 +17,10 @@ import {Metadata, ServiceObject} from '@google-cloud/common'; import {Storage} from './storage'; +export interface HmacKeyOptions { + projectId?: string; +} + export interface HmacKeyMetadata { accessId: string; etag?: string; @@ -58,6 +62,12 @@ export type HmacKeyMetadataResponse = [HmacKeyMetadata, Metadata]; export class HmacKey extends ServiceObject { metadata: HmacKeyMetadata | undefined; + /** + * @typedef {object} HmacKeyOptions + * @property {string} [projectId] The project ID of the project that owns + * the service account of the requested HMAC key. If not provided, + * the project ID used to instantiate the Storage client will be used. + */ /** * Constructs an HmacKey object. * @@ -67,12 +77,13 @@ export class HmacKey extends ServiceObject { * @param {Storage} storage The Storage instance this HMAC key is * attached to. * @param {string} accessId The unique accessId for this HMAC key. + * @param {HmacKeyOptions} options Constructor configurations. * @example * const {Storage} = require('@google-cloud/storage'); * const storage = new Storage(); * const hmacKey = storage.hmacKey('access-id'); */ - constructor(storage: Storage, accessId: string) { + constructor(storage: Storage, accessId: string, options?: HmacKeyOptions) { const methods = { /** * @typedef {object} DeleteHmacKeyOptions @@ -303,10 +314,14 @@ export class HmacKey extends ServiceObject { }, }; + const projectId = + (options && options.projectId) || + storage.projectId; + super({ parent: storage, - baseUrl: `/projects/${storage.projectId}/hmacKeys`, id: accessId, + baseUrl: `/projects/${projectId}/hmacKeys`, methods, }); } diff --git a/src/storage.ts b/src/storage.ts index e711b5570..0cc84753d 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -25,7 +25,7 @@ import {Bucket} from './bucket'; import {Channel} from './channel'; import {File} from './file'; import {normalize} from './util'; -import {HmacKey, HmacKeyMetadata} from './hmacKey'; +import {HmacKey, HmacKeyMetadata, HmacKeyOptions} from './hmacKey'; export interface GetServiceAccountOptions { userProject?: string; @@ -108,6 +108,7 @@ export interface HmacKeyResourceResponse { export type CreateHmacKeyResponse = [HmacKey, string, HmacKeyResourceResponse]; export interface CreateHmacKeyOptions { + projectId?: string; userProject?: string; } @@ -121,6 +122,7 @@ export interface CreateHmacKeyCallback { } export interface GetHmacKeysOptions { + projectId?: string; serviceAccountEmail?: string; showDeletedKeys?: boolean; autoPaginate?: boolean; @@ -190,6 +192,15 @@ export class Storage extends Service { */ static File: typeof File = File; + /** + * {@link HmacKey} class. + * + * @name Storage.HmacKey + * @see HmacKey + * @type {Constructor} + */ + static HmacKey: typeof HmacKey = HmacKey; + /** * Cloud Storage uses access control lists (ACLs) to manage object and * bucket access. ACLs are the mechanism you use to share objects with other @@ -637,6 +648,9 @@ export class Storage extends Service { ): void; /** * @typedef {object} CreateHmacKeyOptions + * @property {string} [projectId] The project ID of the project that owns + * the service account of the requested HMAC key. If not provided, + * the project ID used to instantiate the Storage client will be used. * @property {string} [userProject] This parameter is currently ignored. */ /** @@ -719,11 +733,13 @@ export class Storage extends Service { CreateHmacKeyCallback >(optionsOrCb, cb); const query = Object.assign({}, options, {serviceAccountEmail}); + const projectId = query.projectId || this.projectId; + delete query.projectId; this.request( { method: 'POST', - uri: `/projects/${this.projectId}/hmacKeys`, + uri: `/projects/${projectId}/hmacKeys`, qs: query, }, (err, resp: HmacKeyResourceResponse) => { @@ -732,7 +748,10 @@ export class Storage extends Service { return; } - const hmacKey = new HmacKey(this, resp.metadata.accessId); + const metadata = resp.metadata; + const hmacKey = this.hmacKey(metadata.accessId, { + projectId: metadata.projectId, + }); hmacKey.metadata = resp.metadata; callback!(null, hmacKey, resp.secret, resp); @@ -858,6 +877,9 @@ export class Storage extends Service { * Query object for listing HMAC keys. * * @typedef {object} GetHmacKeysOptions + * @property {string} [projectId] The project ID of the project that owns + * the service account of the requested HMAC key. If not provided, + * the project ID used to instantiate the Storage client will be used. * @property {string} [serviceAccountEmail] If present, only HMAC keys for the * given service account are returned. * @property {boolean} [showDeletedKeys=false] If true, include keys in the DELETE @@ -932,11 +954,14 @@ export class Storage extends Service { cb?: GetHmacKeysCallback ): Promise | void { const {options, callback} = normalize(optionsOrCb, cb); + const query = Object.assign({}, options); + const projectId = query.projectId || this.projectId; + delete query.projectId; this.request( { - uri: `/projects/${this.projectId}/hmacKeys`, - qs: options, + uri: `/projects/${projectId}/hmacKeys`, + qs: query, }, (err, resp) => { if (err) { @@ -944,8 +969,10 @@ export class Storage extends Service { return; } - const hmacKeys = arrify(resp.items).map((hmacKey: Metadata) => { - const hmacKeyInstance = this.hmacKey(hmacKey.accessId); + const hmacKeys = arrify(resp.items).map((hmacKey: HmacKeyMetadata) => { + const hmacKeyInstance = this.hmacKey(hmacKey.accessId, { + projectId: hmacKey.projectId, + }); hmacKeyInstance.metadata = hmacKey; return hmacKeyInstance; }); @@ -1058,7 +1085,12 @@ export class Storage extends Service { * Note: this does not fetch the HMAC key's metadata. Use HmacKey#get() to * retrieve and populate the metadata. * + * To get a reference to an HMAC key that's not created for a service + * account in the same project used to instantiate the Storage client, + * supply the project's ID as `projectId` in the `options` argument. + * * @param {string} accessId The HMAC key's access ID. + * @param {HmacKeyOptions} options HmacKey constructor owptions. * @returns {HmacKey} * @see HmacKey * @@ -1067,12 +1099,12 @@ export class Storage extends Service { * const storage = new Storage(); * const hmacKey = storage.hmacKey('ACCESS_ID'); */ - hmacKey(accessId: string) { + hmacKey(accessId: string, options?: HmacKeyOptions) { if (!accessId) { throw new Error('An access ID is needed to create an HmacKey object.'); } - return new HmacKey(this, accessId); + return new HmacKey(this, accessId, options); } } diff --git a/test/hmacKey.ts b/test/hmacKey.ts index a11daddf3..33c0bb909 100644 --- a/test/hmacKey.ts +++ b/test/hmacKey.ts @@ -55,6 +55,7 @@ describe('HmacKey', () => { STORAGE = { request: util.noop, + projectId: 'my-project', }; hmacKey = new HmacKey(STORAGE, ACCESS_ID); @@ -65,6 +66,7 @@ describe('HmacKey', () => { const ctorArg = serviceObjectSpy.firstCall.args[0]; assert(ctorArg.parent, STORAGE); assert(ctorArg.id, ACCESS_ID); + assert(ctorArg.baseUrl, '/projects/my-project/hmacKeys'); assert.deepStrictEqual(ctorArg.methods, { delete: true, get: true, @@ -76,5 +78,11 @@ describe('HmacKey', () => { }, }); }); + + it('should use projectId in options as baseUrl if given', () => { + hmacKey = new HmacKey(STORAGE, ACCESS_ID, {projectId: 'another-project'}); + const ctorArg = serviceObjectSpy.firstCall.args[0]; + assert(ctorArg.baseUrl, '/projects/another-project/hmacKeys'); + }); }); }); diff --git a/test/index.ts b/test/index.ts index abd4d6b12..31fd97b45 100644 --- a/test/index.ts +++ b/test/index.ts @@ -33,6 +33,9 @@ import sinon = require('sinon'); import {HmacKey} from '../src/hmacKey'; import {HmacKeyResourceResponse} from '../src/storage'; +const hmacKeyModule = require('../src/hmacKey'); +const FakeHmacKey = hmacKeyModule.HmacKey; + class FakeChannel { calledWith_: Array<{}>; constructor(...args: Array<{}>) { @@ -98,6 +101,7 @@ describe('Storage', () => { Service: FakeService, }, './channel.js': {Channel: FakeChannel}, + './hmacKey': hmacKeyModule, }).Storage; Bucket = Storage.Bucket; }); @@ -194,11 +198,30 @@ describe('Storage', () => { }); describe('hmacKey', () => { + let hmacKeyCtor: sinon.SinonSpy; + beforeEach(() => { + hmacKeyCtor = sinon.spy(hmacKeyModule, 'HmacKey'); + }); + + afterEach(() => { + hmacKeyCtor.restore(); + }); + it('should throw if accessId is not provided', () => { assert.throws(() => { storage.hmacKey(); }, /An access ID is needed to create an HmacKey object./); }); + + it('should pass options object to HmacKey constructor', () => { + const options = {myOpts: 'a'}; + storage.hmacKey('access-id', options); + assert.deepStrictEqual(hmacKeyCtor.getCall(0).args, [ + storage, + 'access-id', + options, + ]); + }); }); describe('createHmacKey', () => { @@ -222,6 +245,15 @@ describe('Storage', () => { some: 'value', }; + let hmacKeyCtor: sinon.SinonSpy; + beforeEach(() => { + hmacKeyCtor = sinon.spy(hmacKeyModule, 'HmacKey'); + }); + + afterEach(() => { + hmacKeyCtor.restore(); + }); + it('should make correct API request', done => { storage.request = ( reqOpts: DecorateRequestOptions, @@ -296,7 +328,11 @@ describe('Storage', () => { (err: Error, hmacKey: HmacKey, secret: string) => { assert.ifError(err); assert.strictEqual(secret, response.secret); - assert(hmacKey instanceof HmacKey); + assert.deepStrictEqual(hmacKeyCtor.getCall(0).args, [ + storage, + response.metadata.accessId, + {projectId: response.metadata.projectId}, + ]); assert.strictEqual(hmacKey.metadata, metadataResponse); done(); } @@ -683,6 +719,15 @@ describe('Storage', () => { }); }); + let hmacKeyCtor: sinon.SinonSpy; + beforeEach(() => { + hmacKeyCtor = sinon.spy(hmacKeyModule, 'HmacKey'); + }); + + afterEach(() => { + hmacKeyCtor.restore(); + }); + it('should get HmacKeys without a query', done => { storage.getHmacKeys(() => { const firstArg = storage.request.firstCall.args[0]; @@ -790,7 +835,11 @@ describe('Storage', () => { storage.getHmacKeys((err: Error, hmacKeys: HmacKey[]) => { assert.ifError(err); - assert(hmacKeys![0] instanceof HmacKey); + assert.deepStrictEqual(hmacKeyCtor.getCall(0).args, [ + storage, + metadataResponse.accessId, + {projectId: metadataResponse.projectId} + ]); assert.deepStrictEqual(hmacKeys[0].metadata, metadataResponse); done(); }); From 805ab1c517218674aa4e56d692043e4c38218452 Mon Sep 17 00:00:00 2001 From: Jonathan Lui Date: Tue, 6 Aug 2019 17:31:30 -0700 Subject: [PATCH 2/3] gts fix --- src/hmacKey.ts | 4 +--- test/index.ts | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/hmacKey.ts b/src/hmacKey.ts index 45773a729..e9d04168e 100644 --- a/src/hmacKey.ts +++ b/src/hmacKey.ts @@ -314,9 +314,7 @@ export class HmacKey extends ServiceObject { }, }; - const projectId = - (options && options.projectId) || - storage.projectId; + const projectId = (options && options.projectId) || storage.projectId; super({ parent: storage, diff --git a/test/index.ts b/test/index.ts index 31fd97b45..54a3f11f1 100644 --- a/test/index.ts +++ b/test/index.ts @@ -34,7 +34,6 @@ import {HmacKey} from '../src/hmacKey'; import {HmacKeyResourceResponse} from '../src/storage'; const hmacKeyModule = require('../src/hmacKey'); -const FakeHmacKey = hmacKeyModule.HmacKey; class FakeChannel { calledWith_: Array<{}>; @@ -838,7 +837,7 @@ describe('Storage', () => { assert.deepStrictEqual(hmacKeyCtor.getCall(0).args, [ storage, metadataResponse.accessId, - {projectId: metadataResponse.projectId} + {projectId: metadataResponse.projectId}, ]); assert.deepStrictEqual(hmacKeys[0].metadata, metadataResponse); done(); From b5bec65c9fd3dc7b9dee2293f0b4da9ac5d74af4 Mon Sep 17 00:00:00 2001 From: Jonathan Lui Date: Tue, 6 Aug 2019 17:32:42 -0700 Subject: [PATCH 3/3] better description --- test/hmacKey.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hmacKey.ts b/test/hmacKey.ts index 33c0bb909..b36424ea8 100644 --- a/test/hmacKey.ts +++ b/test/hmacKey.ts @@ -79,7 +79,7 @@ describe('HmacKey', () => { }); }); - it('should use projectId in options as baseUrl if given', () => { + it('should form baseUrl using options.projectId if given', () => { hmacKey = new HmacKey(STORAGE, ACCESS_ID, {projectId: 'another-project'}); const ctorArg = serviceObjectSpy.firstCall.args[0]; assert(ctorArg.baseUrl, '/projects/another-project/hmacKeys');