diff --git a/packages/common-grpc/src/service.js b/packages/common-grpc/src/service.js index e83e7ffd44f..6d3a114fe7f 100644 --- a/packages/common-grpc/src/service.js +++ b/packages/common-grpc/src/service.js @@ -260,7 +260,7 @@ GrpcService.prototype.request = function(protoOpts, reqOpts, callback) { } try { - reqOpts = util.decorateRequest(reqOpts, { projectId: this.projectId }); + reqOpts = this.decorateRequest_(reqOpts); } catch(e) { callback(e); return; @@ -362,7 +362,7 @@ GrpcService.prototype.requestStream = function(protoOpts, reqOpts) { } try { - reqOpts = util.decorateRequest(reqOpts, { projectId: this.projectId }); + reqOpts = this.decorateRequest_(reqOpts); } catch(e) { setImmediate(function() { stream.destroy(e); @@ -443,7 +443,7 @@ GrpcService.prototype.requestWritableStream = function(protoOpts, reqOpts) { } try { - reqOpts = util.decorateRequest(reqOpts, { projectId: this.projectId }); + reqOpts = this.decorateRequest_(reqOpts); } catch (e) { setImmediate(function() { stream.destroy(e); @@ -687,6 +687,22 @@ GrpcService.structToObj_ = function(struct) { return convertedObject; }; +/** + * Assign a projectId if one is specified to all request options. + * + * @param {object} reqOpts - The request options. + * @return {object} - The decorated request object. + */ +GrpcService.prototype.decorateRequest_ = function(reqOpts) { + reqOpts = extend({}, reqOpts); + + delete reqOpts.autoPaginate; + delete reqOpts.autoPaginateVal; + delete reqOpts.objectMode; + + return util.replaceProjectIdToken(reqOpts, this.projectId); +}; + /** * To authorize requests through gRPC, we must get the raw google-auth-library * auth client object. @@ -711,7 +727,7 @@ GrpcService.prototype.getGrpcCredentials_ = function(callback) { ); if (!self.projectId || self.projectId === '{{projectId}}') { - self.projectId = authClient.projectId; + self.projectId = self.authClient.projectId; } callback(null, credentials); diff --git a/packages/common-grpc/test/service.js b/packages/common-grpc/test/service.js index a0163723bea..60f9f2690e0 100644 --- a/packages/common-grpc/test/service.js +++ b/packages/common-grpc/test/service.js @@ -515,7 +515,7 @@ describe('GrpcService', function() { describe('request', function() { var PROTO_OPTS = { service: 'service', method: 'method', timeout: 3000 }; - var REQ_OPTS = {}; + var REQ_OPTS = { reqOpts: true }; var GRPC_CREDENTIALS = {}; function ProtoService() {} @@ -747,9 +747,8 @@ describe('GrpcService', function() { it('should decorate the request', function(done) { var decoratedRequest = {}; - fakeUtil.decorateRequest = function(reqOpts, config) { - assert.strictEqual(reqOpts, REQ_OPTS); - assert.deepEqual(config, { projectId: grpcService.projectId }); + grpcService.decorateRequest_ = function(reqOpts) { + assert.deepEqual(reqOpts, REQ_OPTS); return decoratedRequest; }; @@ -770,7 +769,7 @@ describe('GrpcService', function() { var error = new Error('Error.'); it('should return a thrown error to the callback', function(done) { - fakeUtil.decorateRequest = function() { + grpcService.decorateRequest_ = function() { throw error; }; @@ -787,7 +786,7 @@ describe('GrpcService', function() { grpcService.getService_ = function() { return { method: function(reqOpts) { - assert.strictEqual(reqOpts, REQ_OPTS); + assert.deepEqual(reqOpts, REQ_OPTS); done(); } }; @@ -1063,9 +1062,8 @@ describe('GrpcService', function() { it('should decorate the request', function(done) { var decoratedRequest = {}; - fakeUtil.decorateRequest = function(reqOpts, config) { + grpcService.decorateRequest_ = function(reqOpts) { assert.strictEqual(reqOpts, REQ_OPTS); - assert.deepEqual(config, { projectId: grpcService.projectId }); return decoratedRequest; }; @@ -1084,7 +1082,7 @@ describe('GrpcService', function() { it('should end stream with a thrown error', function(done) { var error = new Error('Error.'); - fakeUtil.decorateRequest = function() { + grpcService.decorateRequest_ = function() { throw error; }; @@ -1340,9 +1338,8 @@ describe('GrpcService', function() { it('should decorate the request', function(done) { var decoratedRequest = {}; - fakeUtil.decorateRequest = function(reqOpts, config) { + grpcService.decorateRequest_ = function(reqOpts) { assert.strictEqual(reqOpts, REQ_OPTS); - assert.deepEqual(config, { projectId: grpcService.projectId }); return decoratedRequest; }; @@ -1360,7 +1357,7 @@ describe('GrpcService', function() { var error = new Error('Error.'); it('should end stream with a thrown error', function(done) { - fakeUtil.decorateRequest = function() { + grpcService.decorateRequest_ = function() { throw error; }; @@ -1660,6 +1657,41 @@ describe('GrpcService', function() { }); }); + describe('decorateRequest_', function() { + it('should delete custom API values without modifying object', function() { + var reqOpts = { + autoPaginate: true, + autoPaginateVal: true, + objectMode: true + }; + + var originalReqOpts = extend({}, reqOpts); + + assert.deepEqual(grpcService.decorateRequest_(reqOpts), {}); + assert.deepEqual(reqOpts, originalReqOpts); + }); + + it('should execute and return replaceProjectIdToken', function() { + var reqOpts = { + a: 'b', + c: 'd' + }; + + var replacedReqOpts = {}; + + fakeUtil.replaceProjectIdToken = function(reqOpts_, projectId) { + assert.deepEqual(reqOpts_, reqOpts); + assert.strictEqual(projectId, grpcService.projectId); + return replacedReqOpts; + }; + + assert.strictEqual( + grpcService.decorateRequest_(reqOpts), + replacedReqOpts + ); + }); + }); + describe('getGrpcCredentials_', function() { it('should get credentials from the auth client', function(done) { grpcService.authClient = { @@ -1698,6 +1730,7 @@ describe('GrpcService', function() { beforeEach(function() { grpcService.authClient = { getAuthClient: function(callback) { + grpcService.authClient = AUTH_CLIENT; callback(null, AUTH_CLIENT); } }; diff --git a/packages/common/src/util.js b/packages/common/src/util.js index 00f52b3a821..d4592cb0319 100644 --- a/packages/common/src/util.js +++ b/packages/common/src/util.js @@ -487,14 +487,18 @@ function decorateRequest(reqOpts, config) { if (is.object(reqOpts.qs)) { delete reqOpts.qs.autoPaginate; delete reqOpts.qs.autoPaginateVal; + reqOpts.qs = util.replaceProjectIdToken(reqOpts.qs, config.projectId); } if (is.object(reqOpts.json)) { delete reqOpts.json.autoPaginate; delete reqOpts.json.autoPaginateVal; + reqOpts.json = util.replaceProjectIdToken(reqOpts.json, config.projectId); } - return util.replaceProjectIdToken(reqOpts, config.projectId); + reqOpts.uri = util.replaceProjectIdToken(reqOpts.uri, config.projectId); + + return reqOpts; } util.decorateRequest = decorateRequest; diff --git a/packages/common/test/util.js b/packages/common/test/util.js index 92e11d6f67c..ad807ca2819 100644 --- a/packages/common/test/util.js +++ b/packages/common/test/util.js @@ -1276,22 +1276,66 @@ describe('common/util', function() { assert.strictEqual(decoratedReqOpts.json.autoPaginateVal, undefined); }); + it('should replace project ID tokens for qs object', function() { + var config = { + projectId: 'project-id' + }; + var reqOpts = { + uri: 'http://', + qs: {} + }; + var decoratedQs = {}; + + utilOverrides.replaceProjectIdToken = function(qs, projectId) { + utilOverrides = {}; + assert.strictEqual(qs, reqOpts.qs); + assert.strictEqual(projectId, config.projectId); + return decoratedQs; + }; + + var decoratedRequest = util.decorateRequest(reqOpts, config); + assert.strictEqual(decoratedRequest.qs, decoratedQs); + }); + + it('should replace project ID tokens for json object', function() { + var config = { + projectId: 'project-id' + }; + var reqOpts = { + uri: 'http://', + json: {} + }; + var decoratedJson = {}; + + utilOverrides.replaceProjectIdToken = function(json, projectId) { + utilOverrides = {}; + assert.strictEqual(reqOpts.json, json); + assert.strictEqual(projectId, config.projectId); + return decoratedJson; + }; + + var decoratedRequest = util.decorateRequest(reqOpts, config); + assert.strictEqual(decoratedRequest.json, decoratedJson); + }); + it('should decorate the request', function() { var config = { projectId: 'project-id' }; - var reqOpts = {}; - var decoratedReqOpts = {}; + var reqOpts = { + uri: 'http://' + }; + var decoratedUri = 'http://decorated'; - utilOverrides.replaceProjectIdToken = function(reqOpts_, projectId) { - assert.strictEqual(reqOpts_, reqOpts); + utilOverrides.replaceProjectIdToken = function(uri, projectId) { + assert.strictEqual(uri, reqOpts.uri); assert.strictEqual(projectId, config.projectId); - return decoratedReqOpts; + return decoratedUri; }; - assert.strictEqual( + assert.deepEqual( util.decorateRequest(reqOpts, config), - decoratedReqOpts + { uri: decoratedUri } ); }); }); diff --git a/packages/compute/package.json b/packages/compute/package.json index 7aea04822c7..5e66ab70bdb 100644 --- a/packages/compute/package.json +++ b/packages/compute/package.json @@ -63,7 +63,8 @@ "concat-stream": "^1.5.0", "mocha": "^3.0.1", "propprop": "^0.3.0", - "proxyquire": "^1.7.10" + "proxyquire": "^1.7.10", + "uuid": "^3.0.1" }, "scripts": { "publish-module": "node ../../scripts/publish.js compute", diff --git a/packages/compute/src/vm.js b/packages/compute/src/vm.js index 84b5a361f4d..4907fd4e912 100644 --- a/packages/compute/src/vm.js +++ b/packages/compute/src/vm.js @@ -342,6 +342,11 @@ VM.prototype.detachDisk = function(disk, callback) { return; } + var diskName = common.util.replaceProjectIdToken( + disk.formattedName, + self.zone.compute.authClient.projectId + ); + var deviceName; var baseUrl = 'https://www.googleapis.com/compute/v1/'; var disks = metadata.disks || []; @@ -352,7 +357,7 @@ VM.prototype.detachDisk = function(disk, callback) { var attachedDisk = disks[i]; var source = attachedDisk.source.replace(baseUrl, ''); - if (source === disk.formattedName) { + if (source === diskName) { deviceName = attachedDisk.deviceName; } } diff --git a/packages/compute/test/vm.js b/packages/compute/test/vm.js index 380381e01b1..99e06aa6b53 100644 --- a/packages/compute/test/vm.js +++ b/packages/compute/test/vm.js @@ -23,6 +23,8 @@ var proxyquire = require('proxyquire'); var ServiceObject = require('@google-cloud/common').ServiceObject; var util = require('@google-cloud/common').util; +var utilCached = extend({}, util); + var promisified = false; var fakeUtil = extend({}, util, { promisifyAll: function(Class) { @@ -46,7 +48,12 @@ describe('VM', function() { var Disk; var DISK; - var COMPUTE = { projectId: 'project-id' }; + var COMPUTE = { + authClient: { + projectId: 'project-id' + }, + projectId: 'project-id' + }; var ZONE = { compute: COMPUTE, name: 'us-central1-a', @@ -70,6 +77,7 @@ describe('VM', function() { }); beforeEach(function() { + extend(fakeUtil, utilCached); vm = new VM(ZONE, VM_NAME); DISK = new Disk(ZONE, 'disk-name'); }); @@ -229,7 +237,7 @@ describe('VM', function() { beforeEach(function() { DEVICE_NAME = DISK.formattedName; - METADATA = METADATA = { + METADATA = { disks: [ { source: DEVICE_NAME, @@ -254,6 +262,36 @@ describe('VM', function() { }); }); + it('should replace projectId token in disk name', function(done) { + var REPLACED_DEVICE_NAME = 'replaced-device-name'; + + fakeUtil.replaceProjectIdToken = function(value, projectId) { + assert.strictEqual(value, DISK.formattedName); + assert.strictEqual(projectId, COMPUTE.authClient.projectId); + return REPLACED_DEVICE_NAME; + }; + + vm.getMetadata = function(callback) { + var metadata = { + disks: [ + { + source: REPLACED_DEVICE_NAME, + deviceName: REPLACED_DEVICE_NAME + } + ] + }; + + callback(null, metadata, metadata); + }; + + vm.request = function(reqOpts) { + assert.strictEqual(reqOpts.qs.deviceName, REPLACED_DEVICE_NAME); + done(); + }; + + vm.detachDisk(DISK, assert.ifError); + }); + it('should return an error if device name not found', function(done) { var metadata = { disks: [