Skip to content

Commit c522d81

Browse files
stephenplusplusstephenplusplus
authored
core: only recurse relevant request objects (#2100)
1 parent aeed573 commit c522d81

7 files changed

Lines changed: 169 additions & 28 deletions

File tree

‎packages/common-grpc/src/service.js‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ GrpcService.prototype.request = function(protoOpts, reqOpts, callback) {
260260
}
261261

262262
try {
263-
reqOpts = util.decorateRequest(reqOpts, { projectId: this.projectId });
263+
reqOpts = this.decorateRequest_(reqOpts);
264264
} catch(e) {
265265
callback(e);
266266
return;
@@ -362,7 +362,7 @@ GrpcService.prototype.requestStream = function(protoOpts, reqOpts) {
362362
}
363363

364364
try {
365-
reqOpts = util.decorateRequest(reqOpts, { projectId: this.projectId });
365+
reqOpts = this.decorateRequest_(reqOpts);
366366
} catch(e) {
367367
setImmediate(function() {
368368
stream.destroy(e);
@@ -443,7 +443,7 @@ GrpcService.prototype.requestWritableStream = function(protoOpts, reqOpts) {
443443
}
444444

445445
try {
446-
reqOpts = util.decorateRequest(reqOpts, { projectId: this.projectId });
446+
reqOpts = this.decorateRequest_(reqOpts);
447447
} catch (e) {
448448
setImmediate(function() {
449449
stream.destroy(e);
@@ -687,6 +687,22 @@ GrpcService.structToObj_ = function(struct) {
687687
return convertedObject;
688688
};
689689

690+
/**
691+
* Assign a projectId if one is specified to all request options.
692+
*
693+
* @param {object} reqOpts - The request options.
694+
* @return {object} - The decorated request object.
695+
*/
696+
GrpcService.prototype.decorateRequest_ = function(reqOpts) {
697+
reqOpts = extend({}, reqOpts);
698+
699+
delete reqOpts.autoPaginate;
700+
delete reqOpts.autoPaginateVal;
701+
delete reqOpts.objectMode;
702+
703+
return util.replaceProjectIdToken(reqOpts, this.projectId);
704+
};
705+
690706
/**
691707
* To authorize requests through gRPC, we must get the raw google-auth-library
692708
* auth client object.
@@ -711,7 +727,7 @@ GrpcService.prototype.getGrpcCredentials_ = function(callback) {
711727
);
712728

713729
if (!self.projectId || self.projectId === '{{projectId}}') {
714-
self.projectId = authClient.projectId;
730+
self.projectId = self.authClient.projectId;
715731
}
716732

717733
callback(null, credentials);

‎packages/common-grpc/test/service.js‎

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ describe('GrpcService', function() {
515515

516516
describe('request', function() {
517517
var PROTO_OPTS = { service: 'service', method: 'method', timeout: 3000 };
518-
var REQ_OPTS = {};
518+
var REQ_OPTS = { reqOpts: true };
519519
var GRPC_CREDENTIALS = {};
520520

521521
function ProtoService() {}
@@ -747,9 +747,8 @@ describe('GrpcService', function() {
747747
it('should decorate the request', function(done) {
748748
var decoratedRequest = {};
749749

750-
fakeUtil.decorateRequest = function(reqOpts, config) {
751-
assert.strictEqual(reqOpts, REQ_OPTS);
752-
assert.deepEqual(config, { projectId: grpcService.projectId });
750+
grpcService.decorateRequest_ = function(reqOpts) {
751+
assert.deepEqual(reqOpts, REQ_OPTS);
753752
return decoratedRequest;
754753
};
755754

@@ -770,7 +769,7 @@ describe('GrpcService', function() {
770769
var error = new Error('Error.');
771770

772771
it('should return a thrown error to the callback', function(done) {
773-
fakeUtil.decorateRequest = function() {
772+
grpcService.decorateRequest_ = function() {
774773
throw error;
775774
};
776775

@@ -787,7 +786,7 @@ describe('GrpcService', function() {
787786
grpcService.getService_ = function() {
788787
return {
789788
method: function(reqOpts) {
790-
assert.strictEqual(reqOpts, REQ_OPTS);
789+
assert.deepEqual(reqOpts, REQ_OPTS);
791790
done();
792791
}
793792
};
@@ -1063,9 +1062,8 @@ describe('GrpcService', function() {
10631062
it('should decorate the request', function(done) {
10641063
var decoratedRequest = {};
10651064

1066-
fakeUtil.decorateRequest = function(reqOpts, config) {
1065+
grpcService.decorateRequest_ = function(reqOpts) {
10671066
assert.strictEqual(reqOpts, REQ_OPTS);
1068-
assert.deepEqual(config, { projectId: grpcService.projectId });
10691067
return decoratedRequest;
10701068
};
10711069

@@ -1084,7 +1082,7 @@ describe('GrpcService', function() {
10841082
it('should end stream with a thrown error', function(done) {
10851083
var error = new Error('Error.');
10861084

1087-
fakeUtil.decorateRequest = function() {
1085+
grpcService.decorateRequest_ = function() {
10881086
throw error;
10891087
};
10901088

@@ -1340,9 +1338,8 @@ describe('GrpcService', function() {
13401338
it('should decorate the request', function(done) {
13411339
var decoratedRequest = {};
13421340

1343-
fakeUtil.decorateRequest = function(reqOpts, config) {
1341+
grpcService.decorateRequest_ = function(reqOpts) {
13441342
assert.strictEqual(reqOpts, REQ_OPTS);
1345-
assert.deepEqual(config, { projectId: grpcService.projectId });
13461343
return decoratedRequest;
13471344
};
13481345

@@ -1360,7 +1357,7 @@ describe('GrpcService', function() {
13601357
var error = new Error('Error.');
13611358

13621359
it('should end stream with a thrown error', function(done) {
1363-
fakeUtil.decorateRequest = function() {
1360+
grpcService.decorateRequest_ = function() {
13641361
throw error;
13651362
};
13661363

@@ -1660,6 +1657,41 @@ describe('GrpcService', function() {
16601657
});
16611658
});
16621659

1660+
describe('decorateRequest_', function() {
1661+
it('should delete custom API values without modifying object', function() {
1662+
var reqOpts = {
1663+
autoPaginate: true,
1664+
autoPaginateVal: true,
1665+
objectMode: true
1666+
};
1667+
1668+
var originalReqOpts = extend({}, reqOpts);
1669+
1670+
assert.deepEqual(grpcService.decorateRequest_(reqOpts), {});
1671+
assert.deepEqual(reqOpts, originalReqOpts);
1672+
});
1673+
1674+
it('should execute and return replaceProjectIdToken', function() {
1675+
var reqOpts = {
1676+
a: 'b',
1677+
c: 'd'
1678+
};
1679+
1680+
var replacedReqOpts = {};
1681+
1682+
fakeUtil.replaceProjectIdToken = function(reqOpts_, projectId) {
1683+
assert.deepEqual(reqOpts_, reqOpts);
1684+
assert.strictEqual(projectId, grpcService.projectId);
1685+
return replacedReqOpts;
1686+
};
1687+
1688+
assert.strictEqual(
1689+
grpcService.decorateRequest_(reqOpts),
1690+
replacedReqOpts
1691+
);
1692+
});
1693+
});
1694+
16631695
describe('getGrpcCredentials_', function() {
16641696
it('should get credentials from the auth client', function(done) {
16651697
grpcService.authClient = {
@@ -1698,6 +1730,7 @@ describe('GrpcService', function() {
16981730
beforeEach(function() {
16991731
grpcService.authClient = {
17001732
getAuthClient: function(callback) {
1733+
grpcService.authClient = AUTH_CLIENT;
17011734
callback(null, AUTH_CLIENT);
17021735
}
17031736
};

‎packages/common/src/util.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,14 +487,18 @@ function decorateRequest(reqOpts, config) {
487487
if (is.object(reqOpts.qs)) {
488488
delete reqOpts.qs.autoPaginate;
489489
delete reqOpts.qs.autoPaginateVal;
490+
reqOpts.qs = util.replaceProjectIdToken(reqOpts.qs, config.projectId);
490491
}
491492

492493
if (is.object(reqOpts.json)) {
493494
delete reqOpts.json.autoPaginate;
494495
delete reqOpts.json.autoPaginateVal;
496+
reqOpts.json = util.replaceProjectIdToken(reqOpts.json, config.projectId);
495497
}
496498

497-
return util.replaceProjectIdToken(reqOpts, config.projectId);
499+
reqOpts.uri = util.replaceProjectIdToken(reqOpts.uri, config.projectId);
500+
501+
return reqOpts;
498502
}
499503

500504
util.decorateRequest = decorateRequest;

‎packages/common/test/util.js‎

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,22 +1276,66 @@ describe('common/util', function() {
12761276
assert.strictEqual(decoratedReqOpts.json.autoPaginateVal, undefined);
12771277
});
12781278

1279+
it('should replace project ID tokens for qs object', function() {
1280+
var config = {
1281+
projectId: 'project-id'
1282+
};
1283+
var reqOpts = {
1284+
uri: 'http://',
1285+
qs: {}
1286+
};
1287+
var decoratedQs = {};
1288+
1289+
utilOverrides.replaceProjectIdToken = function(qs, projectId) {
1290+
utilOverrides = {};
1291+
assert.strictEqual(qs, reqOpts.qs);
1292+
assert.strictEqual(projectId, config.projectId);
1293+
return decoratedQs;
1294+
};
1295+
1296+
var decoratedRequest = util.decorateRequest(reqOpts, config);
1297+
assert.strictEqual(decoratedRequest.qs, decoratedQs);
1298+
});
1299+
1300+
it('should replace project ID tokens for json object', function() {
1301+
var config = {
1302+
projectId: 'project-id'
1303+
};
1304+
var reqOpts = {
1305+
uri: 'http://',
1306+
json: {}
1307+
};
1308+
var decoratedJson = {};
1309+
1310+
utilOverrides.replaceProjectIdToken = function(json, projectId) {
1311+
utilOverrides = {};
1312+
assert.strictEqual(reqOpts.json, json);
1313+
assert.strictEqual(projectId, config.projectId);
1314+
return decoratedJson;
1315+
};
1316+
1317+
var decoratedRequest = util.decorateRequest(reqOpts, config);
1318+
assert.strictEqual(decoratedRequest.json, decoratedJson);
1319+
});
1320+
12791321
it('should decorate the request', function() {
12801322
var config = {
12811323
projectId: 'project-id'
12821324
};
1283-
var reqOpts = {};
1284-
var decoratedReqOpts = {};
1325+
var reqOpts = {
1326+
uri: 'http://'
1327+
};
1328+
var decoratedUri = 'http://decorated';
12851329

1286-
utilOverrides.replaceProjectIdToken = function(reqOpts_, projectId) {
1287-
assert.strictEqual(reqOpts_, reqOpts);
1330+
utilOverrides.replaceProjectIdToken = function(uri, projectId) {
1331+
assert.strictEqual(uri, reqOpts.uri);
12881332
assert.strictEqual(projectId, config.projectId);
1289-
return decoratedReqOpts;
1333+
return decoratedUri;
12901334
};
12911335

1292-
assert.strictEqual(
1336+
assert.deepEqual(
12931337
util.decorateRequest(reqOpts, config),
1294-
decoratedReqOpts
1338+
{ uri: decoratedUri }
12951339
);
12961340
});
12971341
});

‎packages/compute/package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@
6363
"concat-stream": "^1.5.0",
6464
"mocha": "^3.0.1",
6565
"propprop": "^0.3.0",
66-
"proxyquire": "^1.7.10"
66+
"proxyquire": "^1.7.10",
67+
"uuid": "^3.0.1"
6768
},
6869
"scripts": {
6970
"publish-module": "node ../../scripts/publish.js compute",

‎packages/compute/src/vm.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,11 @@ VM.prototype.detachDisk = function(disk, callback) {
342342
return;
343343
}
344344

345+
var diskName = common.util.replaceProjectIdToken(
346+
disk.formattedName,
347+
self.zone.compute.authClient.projectId
348+
);
349+
345350
var deviceName;
346351
var baseUrl = 'https://www.googleapis.com/compute/v1/';
347352
var disks = metadata.disks || [];
@@ -352,7 +357,7 @@ VM.prototype.detachDisk = function(disk, callback) {
352357
var attachedDisk = disks[i];
353358
var source = attachedDisk.source.replace(baseUrl, '');
354359

355-
if (source === disk.formattedName) {
360+
if (source === diskName) {
356361
deviceName = attachedDisk.deviceName;
357362
}
358363
}

‎packages/compute/test/vm.js‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ var proxyquire = require('proxyquire');
2323
var ServiceObject = require('@google-cloud/common').ServiceObject;
2424
var util = require('@google-cloud/common').util;
2525

26+
var utilCached = extend({}, util);
27+
2628
var promisified = false;
2729
var fakeUtil = extend({}, util, {
2830
promisifyAll: function(Class) {
@@ -46,7 +48,12 @@ describe('VM', function() {
4648
var Disk;
4749
var DISK;
4850

49-
var COMPUTE = { projectId: 'project-id' };
51+
var COMPUTE = {
52+
authClient: {
53+
projectId: 'project-id'
54+
},
55+
projectId: 'project-id'
56+
};
5057
var ZONE = {
5158
compute: COMPUTE,
5259
name: 'us-central1-a',
@@ -70,6 +77,7 @@ describe('VM', function() {
7077
});
7178

7279
beforeEach(function() {
80+
extend(fakeUtil, utilCached);
7381
vm = new VM(ZONE, VM_NAME);
7482
DISK = new Disk(ZONE, 'disk-name');
7583
});
@@ -229,7 +237,7 @@ describe('VM', function() {
229237
beforeEach(function() {
230238
DEVICE_NAME = DISK.formattedName;
231239

232-
METADATA = METADATA = {
240+
METADATA = {
233241
disks: [
234242
{
235243
source: DEVICE_NAME,
@@ -254,6 +262,36 @@ describe('VM', function() {
254262
});
255263
});
256264

265+
it('should replace projectId token in disk name', function(done) {
266+
var REPLACED_DEVICE_NAME = 'replaced-device-name';
267+
268+
fakeUtil.replaceProjectIdToken = function(value, projectId) {
269+
assert.strictEqual(value, DISK.formattedName);
270+
assert.strictEqual(projectId, COMPUTE.authClient.projectId);
271+
return REPLACED_DEVICE_NAME;
272+
};
273+
274+
vm.getMetadata = function(callback) {
275+
var metadata = {
276+
disks: [
277+
{
278+
source: REPLACED_DEVICE_NAME,
279+
deviceName: REPLACED_DEVICE_NAME
280+
}
281+
]
282+
};
283+
284+
callback(null, metadata, metadata);
285+
};
286+
287+
vm.request = function(reqOpts) {
288+
assert.strictEqual(reqOpts.qs.deviceName, REPLACED_DEVICE_NAME);
289+
done();
290+
};
291+
292+
vm.detachDisk(DISK, assert.ifError);
293+
});
294+
257295
it('should return an error if device name not found', function(done) {
258296
var metadata = {
259297
disks: [

0 commit comments

Comments
 (0)