From aef5e8d44c767518737ffd709743dbfaf0304ecc Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 19 May 2020 09:32:05 -0700 Subject: [PATCH] storagetransfer: update to use latest auth.getClient (#1812) Fixes: #1747 --- storage-transfer/system-test/transfer.test.js | 1 + storage-transfer/test/transfer.test.js | 175 +++++------------- storage-transfer/transfer.js | 74 ++------ 3 files changed, 57 insertions(+), 193 deletions(-) diff --git a/storage-transfer/system-test/transfer.test.js b/storage-transfer/system-test/transfer.test.js index daa798c43e..d25461ba1f 100644 --- a/storage-transfer/system-test/transfer.test.js +++ b/storage-transfer/system-test/transfer.test.js @@ -100,6 +100,7 @@ it('should create a storage transfer job', (done) => { program.createTransferJob(options, (err, transferJob) => { assert.ifError(err); + // jobName is used by other tests below. jobName = transferJob.name; assert.strictEqual(transferJob.name.indexOf('transferJobs/'), 0); assert.strictEqual(transferJob.description, description); diff --git a/storage-transfer/test/transfer.test.js b/storage-transfer/test/transfer.test.js index 41ba07879e..965adfdb89 100644 --- a/storage-transfer/test/transfer.test.js +++ b/storage-transfer/test/transfer.test.js @@ -53,7 +53,7 @@ const getSample = () => { const googleMock = { storagetransfer: sinon.stub().returns(storagetransferMock), auth: { - getApplicationDefault: sinon.stub().yields(null, {}), + getClient: sinon.stub().returns({}), }, }; @@ -87,7 +87,7 @@ const restoreConsole = function () { beforeEach(stubConsole); afterEach(restoreConsole); -it('should create a transfer job', () => { +it('should create a transfer job', async () => { const description = 'description'; const sample = getSample(); const callback = sinon.stub(); @@ -100,7 +100,7 @@ it('should create a transfer job', () => { time: time, }; - sample.program.createTransferJob(options, callback); + await sample.program.createTransferJob(options, callback); assert.strictEqual( sample.mocks.storagetransfer.transferJobs.create.calledOnce, @@ -123,7 +123,7 @@ it('should create a transfer job', () => { ]); options.description = description; - sample.program.createTransferJob(options, callback); + await sample.program.createTransferJob(options, callback); assert.strictEqual( sample.mocks.storagetransfer.transferJobs.create.calledTwice, @@ -146,35 +146,23 @@ it('should create a transfer job', () => { ]); }); -it('should handle auth error', () => { - const error = new Error('error'); - const sample = getSample(); - const callback = sinon.stub(); - sample.mocks.googleapis.google.auth.getApplicationDefault.yields(error); - - sample.program.createTransferJob({}, callback); - - assert.strictEqual(callback.calledOnce, true); - assert.deepStrictEqual(callback.firstCall.args, [error]); -}); - -it('should handle create error', () => { +it('should handle create error', async () => { const error = new Error('error'); const sample = getSample(); const callback = sinon.stub(); sample.mocks.storagetransfer.transferJobs.create.yields(error); - sample.program.createTransferJob({}, callback); + await sample.program.createTransferJob({}, callback); assert.strictEqual(callback.calledOnce, true); assert.deepStrictEqual(callback.firstCall.args, [error]); }); -it('should get a transfer job', () => { +it('should get a transfer job', async () => { const sample = getSample(); const callback = sinon.stub(); - sample.program.getTransferJob(jobName, callback); + await sample.program.getTransferJob(jobName, callback); assert.strictEqual( sample.mocks.storagetransfer.transferJobs.get.calledOnce, @@ -202,31 +190,19 @@ it('should get a transfer job', () => { ]); }); -it('should handle auth error', () => { - const error = new Error('error'); - const sample = getSample(); - const callback = sinon.stub(); - sample.mocks.googleapis.google.auth.getApplicationDefault.yields(error); - - sample.program.getTransferJob(jobName, callback); - - assert.strictEqual(callback.calledOnce, true); - assert.deepStrictEqual(callback.firstCall.args, [error]); -}); - -it('should handle get error', () => { +it('should handle get error', async () => { const error = new Error('error'); const sample = getSample(); const callback = sinon.stub(); sample.mocks.storagetransfer.transferJobs.get.yields(error); - sample.program.getTransferJob(jobName, callback); + await sample.program.getTransferJob(jobName, callback); assert.strictEqual(callback.calledOnce, true); assert.deepStrictEqual(callback.firstCall.args, [error]); }); -it('should update a transfer job', () => { +it('should update a transfer job', async () => { const sample = getSample(); const callback = sinon.stub(); const options = { @@ -235,7 +211,7 @@ it('should update a transfer job', () => { value: 'DISABLED', }; - sample.program.updateTransferJob(options, callback); + await sample.program.updateTransferJob(options, callback); assert.strictEqual( sample.mocks.storagetransfer.transferJobs.patch.calledOnce, @@ -272,7 +248,7 @@ it('should update a transfer job', () => { options.field = 'description'; options.value = 'description'; - sample.program.updateTransferJob(options, callback); + await sample.program.updateTransferJob(options, callback); assert.strictEqual( sample.mocks.storagetransfer.transferJobs.patch.calledTwice, @@ -312,7 +288,7 @@ it('should update a transfer job', () => { options.field = 'transferSpec'; options.value = '{"foo":"bar"}'; - sample.program.updateTransferJob(options, callback); + await sample.program.updateTransferJob(options, callback); assert.strictEqual( sample.mocks.storagetransfer.transferJobs.patch.calledThrice, @@ -347,7 +323,7 @@ it('should update a transfer job', () => { ]); }); -it('should handle auth error', () => { +it('should handle patch error', async () => { const error = new Error('error'); const sample = getSample(); const callback = sinon.stub(); @@ -356,36 +332,31 @@ it('should handle auth error', () => { field: 'status', value: 'DISABLED', }; - sample.mocks.googleapis.google.auth.getApplicationDefault.yields(error); + sample.mocks.storagetransfer.transferJobs.patch.yields(error); - sample.program.updateTransferJob(options, callback); + await sample.program.updateTransferJob(options, callback); assert.strictEqual(callback.calledOnce, true); assert.deepStrictEqual(callback.firstCall.args, [error]); }); -it('should handle patch error', () => { +it('should handle list error', async () => { const error = new Error('error'); const sample = getSample(); const callback = sinon.stub(); - const options = { - job: jobName, - field: 'status', - value: 'DISABLED', - }; - sample.mocks.storagetransfer.transferJobs.patch.yields(error); + sample.mocks.storagetransfer.transferJobs.list.yields(error); - sample.program.updateTransferJob(options, callback); + await sample.program.listTransferJobs(callback); assert.strictEqual(callback.calledOnce, true); assert.deepStrictEqual(callback.firstCall.args, [error]); }); -it('should list transfer jobs', () => { +it('should list transfer jobs', async () => { const sample = getSample(); const callback = sinon.stub(); - sample.program.listTransferJobs(callback); + await sample.program.listTransferJobs(callback); assert.strictEqual( sample.mocks.storagetransfer.transferJobs.list.calledOnce, @@ -409,7 +380,7 @@ it('should list transfer jobs', () => { assert.deepStrictEqual(console.log.firstCall.args, ['Found %d jobs!', 1]); sample.mocks.storagetransfer.transferJobs.list.yields(null, {}); - sample.program.listTransferJobs(callback); + await sample.program.listTransferJobs(callback); assert.strictEqual( sample.mocks.storagetransfer.transferJobs.list.calledTwice, @@ -429,36 +400,24 @@ it('should list transfer jobs', () => { assert.strictEqual(console.log.calledOnce, true); }); -it('should handle auth error', () => { - const error = new Error('error'); - const sample = getSample(); - const callback = sinon.stub(); - sample.mocks.googleapis.google.auth.getApplicationDefault.yields(error); - - sample.program.listTransferJobs(callback); - - assert.strictEqual(callback.calledOnce, true); - assert.deepStrictEqual(callback.firstCall.args, [error]); -}); - -it('should handle list error', () => { +it('should handle list error', async () => { const error = new Error('error'); const sample = getSample(); const callback = sinon.stub(); sample.mocks.storagetransfer.transferJobs.list.yields(error); - sample.program.listTransferJobs(callback); + await sample.program.listTransferJobs(callback); assert.strictEqual(callback.calledOnce, true); assert.deepStrictEqual(callback.firstCall.args, [error]); }); -it('should list transfer operations', () => { +it('should list transfer operations', async () => { const sample = getSample(); const callback = sinon.stub(); // Test that all operations get listed - sample.program.listTransferOperations(undefined, callback); + await sample.program.listTransferOperations(undefined, callback); assert.strictEqual( sample.mocks.storagetransfer.transferOperations.list.calledOnce, @@ -489,7 +448,7 @@ it('should list transfer operations', () => { ]); // Test that operations for a specific job get listed - sample.program.listTransferOperations(jobName, callback); + await sample.program.listTransferOperations(jobName, callback); assert.strictEqual( sample.mocks.storagetransfer.transferOperations.list.calledTwice, @@ -524,7 +483,7 @@ it('should list transfer operations', () => { // Test that operations for a specific job get listed when the API response with just an object sample.mocks.storagetransfer.transferOperations.list.yields(null, {}); - sample.program.listTransferOperations(jobName, callback); + await sample.program.listTransferOperations(jobName, callback); assert.strictEqual( sample.mocks.storagetransfer.transferOperations.list.calledThrice, @@ -551,35 +510,23 @@ it('should list transfer operations', () => { assert.strictEqual(console.log.calledTwice, true); }); -it('should handle auth error', () => { - const error = new Error('error'); - const sample = getSample(); - const callback = sinon.stub(); - sample.mocks.googleapis.google.auth.getApplicationDefault.yields(error); - - sample.program.listTransferOperations(undefined, callback); - - assert.strictEqual(callback.calledOnce, true); - assert.deepStrictEqual(callback.firstCall.args, [error]); -}); - -it('should handle list error', () => { +it('should handle list error', async () => { const error = new Error('error'); const sample = getSample(); const callback = sinon.stub(); sample.mocks.storagetransfer.transferOperations.list.yields(error); - sample.program.listTransferOperations(undefined, callback); + await sample.program.listTransferOperations(undefined, callback); assert.strictEqual(callback.calledOnce, true); assert.deepStrictEqual(callback.firstCall.args, [error]); }); -it('should get a transfer operation', () => { +it('should get a transfer operation', async () => { const sample = getSample(); const callback = sinon.stub(); - sample.program.getTransferOperation(transferOperationName, callback); + await sample.program.getTransferOperation(transferOperationName, callback); assert.strictEqual(callback.calledOnce, true); assert.strictEqual( @@ -621,35 +568,23 @@ it('should get a transfer operation', () => { ]); }); -it('should handle auth error', () => { - const error = new Error('error'); - const sample = getSample(); - const callback = sinon.stub(); - sample.mocks.googleapis.google.auth.getApplicationDefault.yields(error); - - sample.program.getTransferOperation(jobName, callback); - - assert.strictEqual(callback.calledOnce, true); - assert.deepStrictEqual(callback.firstCall.args, [error]); -}); - -it('should handle get error', () => { +it('should handle get error', async () => { const error = new Error('error'); const sample = getSample(); const callback = sinon.stub(); sample.mocks.storagetransfer.transferOperations.get.yields(error); - sample.program.getTransferOperation(jobName, callback); + await sample.program.getTransferOperation(jobName, callback); assert.strictEqual(callback.calledOnce, true); assert.deepStrictEqual(callback.firstCall.args, [error]); }); -it('should pause a transfer operation', () => { +it('should pause a transfer operation', async () => { const sample = getSample(); const callback = sinon.stub(); - sample.program.pauseTransferOperation(transferOperationName, callback); + await sample.program.pauseTransferOperation(transferOperationName, callback); assert.strictEqual(callback.calledOnce, true); assert.strictEqual( @@ -684,35 +619,23 @@ it('should pause a transfer operation', () => { ]); }); -it('should handle auth error', () => { - const error = new Error('error'); - const sample = getSample(); - const callback = sinon.stub(); - sample.mocks.googleapis.google.auth.getApplicationDefault.yields(error); - - sample.program.pauseTransferOperation(jobName, callback); - - assert.strictEqual(callback.calledOnce, true); - assert.deepStrictEqual(callback.firstCall.args, [error]); -}); - -it('should handle pause error', () => { +it('should handle pause error', async () => { const error = new Error('error'); const sample = getSample(); const callback = sinon.stub(); sample.mocks.storagetransfer.transferOperations.pause.yields(error); - sample.program.pauseTransferOperation(jobName, callback); + await sample.program.pauseTransferOperation(jobName, callback); assert.strictEqual(callback.calledOnce, true); assert.deepStrictEqual(callback.firstCall.args, [error]); }); -it('should resume a transfer operation', () => { +it('should resume a transfer operation', async () => { const sample = getSample(); const callback = sinon.stub(); - sample.program.resumeTransferOperation(transferOperationName, callback); + await sample.program.resumeTransferOperation(transferOperationName, callback); assert.strictEqual(callback.calledOnce, true); assert.strictEqual( @@ -747,25 +670,13 @@ it('should resume a transfer operation', () => { ]); }); -it('should handle auth error', () => { - const error = new Error('error'); - const sample = getSample(); - const callback = sinon.stub(); - sample.mocks.googleapis.google.auth.getApplicationDefault.yields(error); - - sample.program.resumeTransferOperation(jobName, callback); - - assert.strictEqual(callback.calledOnce, true); - assert.deepStrictEqual(callback.firstCall.args, [error]); -}); - -it('should handle resume error', () => { +it('should handle resume error', async () => { const error = new Error('error'); const sample = getSample(); const callback = sinon.stub(); sample.mocks.storagetransfer.transferOperations.resume.yields(error); - sample.program.resumeTransferOperation(jobName, callback); + await sample.program.resumeTransferOperation(jobName, callback); assert.strictEqual(callback.calledOnce, true); assert.deepStrictEqual(callback.firstCall.args, [error]); diff --git a/storage-transfer/transfer.js b/storage-transfer/transfer.js index af8e13e88a..a4fdf6641b 100644 --- a/storage-transfer/transfer.js +++ b/storage-transfer/transfer.js @@ -24,27 +24,12 @@ const storagetransfer = google.storagetransfer('v1'); // [END setup] // [START auth] -const auth = (callback) => { - google.auth.getApplicationDefault((err, authClient) => { - if (err) { - return callback(err); - } - - // The createScopedRequired method returns true when running on GAE or a - // local developer machine. In that case, the desired scopes must be passed - // in manually. When the code is running in GCE or GAE Flexible, the scopes - // are pulled from the GCE metadata server. - // See https://cloud.google.com/compute/docs/authentication for more - // information. - if (authClient.createScopedRequired && authClient.createScopedRequired()) { - // Scopes can be specified either as an array or as a single, - // space-delimited string. - authClient = authClient.createScoped([ - 'https://www.googleapis.com/auth/cloud-platform', - ]); - } - callback(null, authClient); +const auth = async (callback) => { + const authClient = await google.auth.getClient({ + scopes: ['https://www.googleapis.com/auth/cloud-platform'] }); + + callback(authClient); }; // [END auth] @@ -64,11 +49,7 @@ const createTransferJob = (options, callback) => { const startDate = moment(options.date, 'YYYY/MM/DD'); const transferTime = moment(options.time, 'HH:mm'); - auth((err, authClient) => { - if (err) { - return callback(err); - } - + auth((authClient) => { const transferJob = { projectId: process.env.GCLOUD_PROJECT, status: 'ENABLED', @@ -109,7 +90,6 @@ const createTransferJob = (options, callback) => { if (err) { return callback(err); } - const transferJob = response.data; console.log('Created transfer job: %s', transferJob.name); return callback(null, transferJob); @@ -127,11 +107,7 @@ const createTransferJob = (options, callback) => { * @param {function} callback The callback function. */ const getTransferJob = (jobName, callback) => { - auth((err, authClient) => { - if (err) { - return callback(err); - } - + auth((authClient) => { storagetransfer.transferJobs.get( { auth: authClient, @@ -163,11 +139,7 @@ const getTransferJob = (jobName, callback) => { * @param {function} callback The callback function. */ const updateTransferJob = (options, callback) => { - auth((err, authClient) => { - if (err) { - return callback(err); - } - + auth((authClient) => { const patchRequest = { projectId: process.env.GCLOUD_PROJECT, transferJob: { @@ -211,11 +183,7 @@ const updateTransferJob = (options, callback) => { * @param {function} callback The callback function. */ const listTransferJobs = (callback) => { - auth((err, authClient) => { - if (err) { - return callback(err); - } - + auth((authClient) => { storagetransfer.transferJobs.list( { auth: authClient, @@ -244,11 +212,7 @@ const listTransferJobs = (callback) => { * @param {function} callback The callback function. */ const listTransferOperations = (jobName, callback) => { - auth((err, authClient) => { - if (err) { - return callback(err); - } - + auth((authClient) => { const filter = { project_id: process.env.GCLOUD_PROJECT, }; @@ -286,11 +250,7 @@ const listTransferOperations = (jobName, callback) => { * @param {function} callback The callback function. */ const getTransferOperation = (transferOperationName, callback) => { - auth((err, authClient) => { - if (err) { - return callback(err); - } - + auth((authClient) => { storagetransfer.transferOperations.get( { name: transferOperationName, @@ -318,11 +278,7 @@ const getTransferOperation = (transferOperationName, callback) => { * @param {function} callback The callback function. */ const pauseTransferOperation = (transferOperationName, callback) => { - auth((err, authClient) => { - if (err) { - return callback(err); - } - + auth((authClient) => { storagetransfer.transferOperations.pause( { name: transferOperationName, @@ -349,11 +305,7 @@ const pauseTransferOperation = (transferOperationName, callback) => { * @param {function} callback The callback function. */ const resumeTransferOperation = (transferOperationName, callback) => { - auth((err, authClient) => { - if (err) { - return callback(err); - } - + auth((authClient) => { storagetransfer.transferOperations.resume( { name: transferOperationName,