Skip to content

Commit

Permalink
Follow Node's callback-last convention for client calls
Browse files Browse the repository at this point in the history
  • Loading branch information
@murgatroid99
murgatroid99 committed Mar 21, 2016
1 parent b08ee7a commit 9a0c741
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 79 deletions.
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
},
"bundledDependencies": ["node-pre-gyp"],
"dependencies": {
"arguejs": "^0.2.3",
"lodash": "^3.9.3",
"nan": "^2.0.0",
"protobufjs": "^4.0.0"
Expand Down
10 changes: 5 additions & 5 deletions src/node/interop/interop_client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ function cancelAfterFirstResponse(client, done) {
function timeoutOnSleepingServer(client, done) {
var deadline = new Date();
deadline.setMilliseconds(deadline.getMilliseconds() + 1);
var call = client.fullDuplexCall(null, {deadline: deadline});
var call = client.fullDuplexCall({deadline: deadline});
call.write({
payload: {body: zeroBuffer(27182)}
});
Expand Down Expand Up @@ -316,10 +316,10 @@ function customMetadata(client, done) {
body: zeroBuffer(271828)
}
};
var unary = client.unaryCall(arg, function(err, resp) {
var unary = client.unaryCall(arg, metadata, function(err, resp) {
assert.ifError(err);
done();
}, metadata);
});
unary.on('metadata', function(metadata) {
assert.deepEqual(metadata.get(ECHO_INITIAL_KEY),
['test_initial_metadata_value']);
Expand Down Expand Up @@ -455,14 +455,14 @@ function perRpcAuthTest(client, done, extra) {
credential = credential.createScoped(scope);
}
var creds = grpc.credentials.createFromGoogleCredential(credential);
client.unaryCall(arg, function(err, resp) {
client.unaryCall(arg, {credentials: creds}, function(err, resp) {
assert.ifError(err);
assert.strictEqual(resp.username, SERVICE_ACCOUNT_EMAIL);
assert(extra.oauth_scope.indexOf(resp.oauth_scope) > -1);
if (done) {
done();
}
}, null, {credentials: creds});
});
});
}

Expand Down
91 changes: 49 additions & 42 deletions src/node/src/client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
'use strict';

var _ = require('lodash');
var arguejs = require('arguejs');

var grpc = require('./grpc_extension');

Expand Down Expand Up @@ -353,31 +354,33 @@ function makeUnaryRequestFunction(method, serialize, deserialize) {
* @this {Client} Client object. Must have a channel member.
* @param {*} argument The argument to the call. Should be serializable with
* serialize
* @param {function(?Error, value=)} callback The callback to for when the
* response is received
* @param {Metadata=} metadata Metadata to add to the call
* @param {Object=} options Options map
* @param {function(?Error, value=)} callback The callback to for when the
* response is received
* @return {EventEmitter} An event emitter for stream related events
*/
function makeUnaryRequest(argument, callback, metadata, options) {
function makeUnaryRequest(argument, metadata, options, callback) {
/* jshint validthis: true */
/* While the arguments are listed in the function signature, those variables
* are not used directly. Instead, ArgueJS processes the arguments
* object. This allows for simple handling of optional arguments in the
* middle of the argument list, and also provides type checking. */
var args = arguejs({argument: null, metadata: [Metadata, new Metadata()],
options: [Object], callback: Function}, arguments);
var emitter = new EventEmitter();
var call = getCall(this.$channel, method, options);
if (metadata === null || metadata === undefined) {
metadata = new Metadata();
} else {
metadata = metadata.clone();
}
var call = getCall(this.$channel, method, args.options);
metadata = args.metadata.clone();
emitter.cancel = function cancel() {
call.cancel();
};
emitter.getPeer = function getPeer() {
return call.getPeer();
};
var client_batch = {};
var message = serialize(argument);
if (options) {
message.grpcWriteFlags = options.flags;
var message = serialize(args.argument);
if (args.options) {
message.grpcWriteFlags = args.options.flags;
}
client_batch[grpc.opType.SEND_INITIAL_METADATA] =
metadata._getCoreRepresentation();
Expand All @@ -395,7 +398,7 @@ function makeUnaryRequestFunction(method, serialize, deserialize) {
if (status.code === grpc.status.OK) {
if (err) {
// Got a batch error, but OK status. Something went wrong
callback(err);
args.callback(err);
return;
} else {
try {
Expand All @@ -414,9 +417,9 @@ function makeUnaryRequestFunction(method, serialize, deserialize) {
error = new Error(status.details);
error.code = status.code;
error.metadata = status.metadata;
callback(error);
args.callback(error);
} else {
callback(null, deserialized);
args.callback(null, deserialized);
}
emitter.emit('status', status);
emitter.emit('metadata', Metadata._fromCoreRepresentation(
Expand All @@ -440,21 +443,23 @@ function makeClientStreamRequestFunction(method, serialize, deserialize) {
* Make a client stream request with this method on the given channel with the
* given callback, etc.
* @this {Client} Client object. Must have a channel member.
* @param {function(?Error, value=)} callback The callback to for when the
* response is received
* @param {Metadata=} metadata Array of metadata key/value pairs to add to the
* call
* @param {Object=} options Options map
* @param {function(?Error, value=)} callback The callback to for when the
* response is received
* @return {EventEmitter} An event emitter for stream related events
*/
function makeClientStreamRequest(callback, metadata, options) {
function makeClientStreamRequest(metadata, options, callback) {
/* jshint validthis: true */
var call = getCall(this.$channel, method, options);
if (metadata === null || metadata === undefined) {
metadata = new Metadata();
} else {
metadata = metadata.clone();
}
/* While the arguments are listed in the function signature, those variables
* are not used directly. Instead, ArgueJS processes the arguments
* object. This allows for simple handling of optional arguments in the
* middle of the argument list, and also provides type checking. */
var args = arguejs({metadata: [Metadata, new Metadata()],
options: [Object], callback: Function}, arguments);
var call = getCall(this.$channel, method, args.options);
metadata = args.metadata.clone();
var stream = new ClientWritableStream(call, serialize);
var metadata_batch = {};
metadata_batch[grpc.opType.SEND_INITIAL_METADATA] =
Expand All @@ -481,7 +486,7 @@ function makeClientStreamRequestFunction(method, serialize, deserialize) {
if (status.code === grpc.status.OK) {
if (err) {
// Got a batch error, but OK status. Something went wrong
callback(err);
args.callback(err);
return;
} else {
try {
Expand All @@ -500,9 +505,9 @@ function makeClientStreamRequestFunction(method, serialize, deserialize) {
error = new Error(response.status.details);
error.code = status.code;
error.metadata = status.metadata;
callback(error);
args.callback(error);
} else {
callback(null, deserialized);
args.callback(null, deserialized);
}
stream.emit('status', status);
});
Expand Down Expand Up @@ -533,17 +538,18 @@ function makeServerStreamRequestFunction(method, serialize, deserialize) {
*/
function makeServerStreamRequest(argument, metadata, options) {
/* jshint validthis: true */
var call = getCall(this.$channel, method, options);
if (metadata === null || metadata === undefined) {
metadata = new Metadata();
} else {
metadata = metadata.clone();
}
/* While the arguments are listed in the function signature, those variables
* are not used directly. Instead, ArgueJS processes the arguments
* object. */
var args = arguejs({argument: null, metadata: [Metadata, new Metadata()],
options: [Object]}, arguments);
var call = getCall(this.$channel, method, args.options);
metadata = args.metadata.clone();
var stream = new ClientReadableStream(call, deserialize);
var start_batch = {};
var message = serialize(argument);
if (options) {
message.grpcWriteFlags = options.flags;
var message = serialize(args.argument);
if (args.options) {
message.grpcWriteFlags = args.options.flags;
}
start_batch[grpc.opType.SEND_INITIAL_METADATA] =
metadata._getCoreRepresentation();
Expand Down Expand Up @@ -595,12 +601,13 @@ function makeBidiStreamRequestFunction(method, serialize, deserialize) {
*/
function makeBidiStreamRequest(metadata, options) {
/* jshint validthis: true */
var call = getCall(this.$channel, method, options);
if (metadata === null || metadata === undefined) {
metadata = new Metadata();
} else {
metadata = metadata.clone();
}
/* While the arguments are listed in the function signature, those variables
* are not used directly. Instead, ArgueJS processes the arguments
* object. */
var args = arguejs({metadata: [Metadata, new Metadata()],
options: [Object]}, arguments);
var call = getCall(this.$channel, method, args.options);
metadata = args.metadata.clone();
var stream = new ClientDuplexStream(call, serialize, deserialize);
var start_batch = {};
start_batch[grpc.opType.SEND_INITIAL_METADATA] =
Expand Down
25 changes: 14 additions & 11 deletions src/node/test/credentials_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,34 +398,36 @@ describe('client credentials', function() {
metadataUpdater);
});
it('Should update metadata on a unary call', function(done) {
var call = client.unary({}, function(err, data) {
assert.ifError(err);
}, null, {credentials: updater_creds});
var call = client.unary({}, {credentials: updater_creds},
function(err, data) {
assert.ifError(err);
});
call.on('metadata', function(metadata) {
assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']);
done();
});
});
it('should update metadata on a client streaming call', function(done) {
var call = client.clientStream(function(err, data) {
assert.ifError(err);
}, null, {credentials: updater_creds});
var call = client.clientStream({credentials: updater_creds},
function(err, data) {
assert.ifError(err);
});
call.on('metadata', function(metadata) {
assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']);
done();
});
call.end();
});
it('should update metadata on a server streaming call', function(done) {
var call = client.serverStream({}, null, {credentials: updater_creds});
var call = client.serverStream({}, {credentials: updater_creds});
call.on('data', function() {});
call.on('metadata', function(metadata) {
assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']);
done();
});
});
it('should update metadata on a bidi streaming call', function(done) {
var call = client.bidiStream(null, {credentials: updater_creds});
var call = client.bidiStream({credentials: updater_creds});
call.on('data', function() {});
call.on('metadata', function(metadata) {
assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']);
Expand All @@ -443,9 +445,10 @@ describe('client credentials', function() {
altMetadataUpdater);
var combined_updater = grpc.credentials.combineCallCredentials(
updater_creds, alt_updater_creds);
var call = client.unary({}, function(err, data) {
assert.ifError(err);
}, null, {credentials: combined_updater});
var call = client.unary({}, {credentials: combined_updater},
function(err, data) {
assert.ifError(err);
});
call.on('metadata', function(metadata) {
assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']);
assert.deepEqual(metadata.get('other_plugin_key'),
Expand Down
Loading

0 comments on commit 9a0c741

Please sign in to comment.