From e363fe5aa877610c6ff97735461d989e697524df Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 21 Jul 2015 14:27:56 -0700 Subject: [PATCH 01/41] Added user-agent setting code, and a test for it --- src/client.js | 6 +++++- test/surface_test.js | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/client.js b/src/client.js index b7bad949..06a0f363 100644 --- a/src/client.js +++ b/src/client.js @@ -47,6 +47,7 @@ var Readable = stream.Readable; var Writable = stream.Writable; var Duplex = stream.Duplex; var util = require('util'); +var version = require('../package.json').version; util.inherits(ClientWritableStream, Writable); @@ -517,7 +518,10 @@ function makeClientConstructor(methods, serviceName) { callback(null, metadata); }; } - + if (!options) { + options = {}; + } + options.GRPC_ARG_PRIMARY_USER_AGENT_STRING = 'grpc-node/' + version; this.server_address = address.replace(/\/$/, ''); this.channel = new grpc.Channel(address, options); this.auth_uri = this.server_address + '/' + serviceName; diff --git a/test/surface_test.js b/test/surface_test.js index 18178e49..3cb68f8c 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -258,6 +258,16 @@ describe('Echo metadata', function() { }); call.end(); }); + it('shows the correct user-agent string', function(done) { + var version = require('../package.json').version; + var call = client.unary({}, function(err, data) { + assert.ifError(err); + }, {key: ['value']}); + call.on('metadata', function(metadata) { + assert(_.startsWith(metadata['user-agent'], 'grpc-node/' + version)); + done(); + }); + }); }); describe('Other conditions', function() { var client; From 8f3e14210edaf52adc59afb37f5c77620d4c63c4 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 21 Jul 2015 18:11:44 -0700 Subject: [PATCH 02/41] C++ is also a language that can be insecure --- ext/channel.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/channel.cc b/ext/channel.cc index d37bf763..28fa2550 100644 --- a/ext/channel.cc +++ b/ext/channel.cc @@ -103,7 +103,7 @@ NAN_METHOD(Channel::New) { NanUtf8String *host = new NanUtf8String(args[0]); NanUtf8String *host_override = NULL; if (args[1]->IsUndefined()) { - wrapped_channel = grpc_channel_create(**host, NULL); + wrapped_channel = grpc_insecure_channel_create(**host, NULL); } else if (args[1]->IsObject()) { grpc_credentials *creds = NULL; Handle args_hash(args[1]->ToObject()->Clone()); @@ -148,7 +148,7 @@ NAN_METHOD(Channel::New) { } } if (creds == NULL) { - wrapped_channel = grpc_channel_create(**host, &channel_args); + wrapped_channel = grpc_insecure_channel_create(**host, &channel_args); } else { wrapped_channel = grpc_secure_channel_create(creds, **host, &channel_args); From c9a6dcc722586389f410411a6e691708925fb55a Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Tue, 21 Jul 2015 23:02:16 -0700 Subject: [PATCH 03/41] Adding option to force client auth on the server SSL creds. --- ext/server_credentials.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ext/server_credentials.cc b/ext/server_credentials.cc index d2b63cdc..709105c7 100644 --- a/ext/server_credentials.cc +++ b/ext/server_credentials.cc @@ -140,8 +140,10 @@ NAN_METHOD(ServerCredentials::CreateSsl) { return NanThrowTypeError("createSsl's third argument must be a Buffer"); } key_cert_pair.cert_chain = ::node::Buffer::Data(args[2]); + // TODO Add a force_client_auth parameter and pass it as the last parameter + // here. NanReturnValue(WrapStruct( - grpc_ssl_server_credentials_create(root_certs, &key_cert_pair, 1))); + grpc_ssl_server_credentials_create(root_certs, &key_cert_pair, 1, 0))); } NAN_METHOD(ServerCredentials::CreateFake) { From c2ead2aefc344a634487cf4f6f912d5b0ad20ec4 Mon Sep 17 00:00:00 2001 From: yang-g Date: Tue, 21 Jul 2015 23:54:36 -0700 Subject: [PATCH 04/41] move fake_transport_security_credentials to private API --- ext/credentials.cc | 7 ------- 1 file changed, 7 deletions(-) diff --git a/ext/credentials.cc b/ext/credentials.cc index 34872017..d6cff063 100644 --- a/ext/credentials.cc +++ b/ext/credentials.cc @@ -79,8 +79,6 @@ void Credentials::Init(Handle exports) { NanNew(CreateComposite)->GetFunction()); ctr->Set(NanNew("createGce"), NanNew(CreateGce)->GetFunction()); - ctr->Set(NanNew("createFake"), - NanNew(CreateFake)->GetFunction()); ctr->Set(NanNew("createIam"), NanNew(CreateIam)->GetFunction()); constructor = new NanCallback(ctr); @@ -180,11 +178,6 @@ NAN_METHOD(Credentials::CreateGce) { NanReturnValue(WrapStruct(grpc_compute_engine_credentials_create())); } -NAN_METHOD(Credentials::CreateFake) { - NanScope(); - NanReturnValue(WrapStruct(grpc_fake_transport_security_credentials_create())); -} - NAN_METHOD(Credentials::CreateIam) { NanScope(); if (!args[0]->IsString()) { From 169fa73f667d2450b250524e529c2bac277d0413 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 22 Jul 2015 09:58:38 -0700 Subject: [PATCH 05/41] Fixed setting user-agent string --- src/client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client.js b/src/client.js index 06a0f363..da6327b4 100644 --- a/src/client.js +++ b/src/client.js @@ -521,9 +521,9 @@ function makeClientConstructor(methods, serviceName) { if (!options) { options = {}; } - options.GRPC_ARG_PRIMARY_USER_AGENT_STRING = 'grpc-node/' + version; - this.server_address = address.replace(/\/$/, ''); + options['grpc.primary_user_agent'] = 'grpc-node/' + version; this.channel = new grpc.Channel(address, options); + this.server_address = address.replace(/\/$/, ''); this.auth_uri = this.server_address + '/' + serviceName; this.updateMetadata = updateMetadata; } From 00875bdf61963562938d67e4bf25fb7c468e84f0 Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 22 Jul 2015 10:33:18 -0700 Subject: [PATCH 06/41] Fix node test. Remove all the server fake credentials references --- ext/server_credentials.cc | 8 -------- ext/server_credentials.h | 1 - test/server_test.js | 10 ++++++++-- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/ext/server_credentials.cc b/ext/server_credentials.cc index d2b63cdc..66aaa330 100644 --- a/ext/server_credentials.cc +++ b/ext/server_credentials.cc @@ -73,8 +73,6 @@ void ServerCredentials::Init(Handle exports) { Handle ctr = tpl->GetFunction(); ctr->Set(NanNew("createSsl"), NanNew(CreateSsl)->GetFunction()); - ctr->Set(NanNew("createFake"), - NanNew(CreateFake)->GetFunction()); constructor = new NanCallback(ctr); exports->Set(NanNew("ServerCredentials"), ctr); } @@ -144,11 +142,5 @@ NAN_METHOD(ServerCredentials::CreateSsl) { grpc_ssl_server_credentials_create(root_certs, &key_cert_pair, 1))); } -NAN_METHOD(ServerCredentials::CreateFake) { - NanScope(); - NanReturnValue( - WrapStruct(grpc_fake_transport_security_server_credentials_create())); -} - } // namespace node } // namespace grpc diff --git a/ext/server_credentials.h b/ext/server_credentials.h index aaa7ef29..80747504 100644 --- a/ext/server_credentials.h +++ b/ext/server_credentials.h @@ -63,7 +63,6 @@ class ServerCredentials : public ::node::ObjectWrap { static NAN_METHOD(New); static NAN_METHOD(CreateSsl); - static NAN_METHOD(CreateFake); static NanCallback *constructor; // Used for typechecking instances of this javascript class static v8::Persistent fun_tpl; diff --git a/test/server_test.js b/test/server_test.js index 7cb34fa0..9c7bb465 100644 --- a/test/server_test.js +++ b/test/server_test.js @@ -34,6 +34,8 @@ 'use strict'; var assert = require('assert'); +var fs = require('fs'); +var path = require('path'); var grpc = require('bindings')('grpc.node'); describe('server', function() { @@ -67,9 +69,13 @@ describe('server', function() { before(function() { server = new grpc.Server(); }); - it('should bind to an unused port with fake credentials', function() { + it('should bind to an unused port with ssl credentials', function() { var port; - var creds = grpc.ServerCredentials.createFake(); + var key_path = path.join(__dirname, '../test/data/server1.key'); + var pem_path = path.join(__dirname, '../test/data/server1.pem'); + var key_data = fs.readFileSync(key_path); + var pem_data = fs.readFileSync(pem_path); + var creds = grpc.ServerCredentials.createSsl(null, key_data, pem_data); assert.doesNotThrow(function() { port = server.addSecureHttp2Port('0.0.0.0:0', creds); }); From 2c426652aa90c963fb0982f3226e5f97a77814e6 Mon Sep 17 00:00:00 2001 From: Jeff Peoples Date: Wed, 22 Jul 2015 16:40:52 -0700 Subject: [PATCH 07/41] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 78781dab..7d3d8c7f 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ An object with factory methods for creating credential objects for clients. ServerCredentials ``` -An object with factory methods fro creating credential objects for servers. +An object with factory methods for creating credential objects for servers. [homebrew]:http://brew.sh [linuxbrew]:https://github.com/Homebrew/linuxbrew#installation From 845bdd941e3c0b44d86b2549e189c3beb1377c4b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 23 Jul 2015 09:52:11 -0700 Subject: [PATCH 08/41] Make the server report monotonic times for deadlines For very high performance systems, we're going to want to be able to simply push the value reported from the server down onto clients. If we report realtime now, then all wrapped languages are going to assume it, meaning that such a change will be impossible later. --- ext/timeval.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/timeval.cc b/ext/timeval.cc index 60de4d81..bf68513c 100644 --- a/ext/timeval.cc +++ b/ext/timeval.cc @@ -52,6 +52,7 @@ gpr_timespec MillisecondsToTimespec(double millis) { } double TimespecToMilliseconds(gpr_timespec timespec) { + timespec = gpr_convert_clock_type(timespec, GPR_CLOCK_REALTIME); if (gpr_time_cmp(timespec, gpr_inf_future(GPR_CLOCK_REALTIME)) == 0) { return std::numeric_limits::infinity(); } else if (gpr_time_cmp(timespec, gpr_inf_past(GPR_CLOCK_REALTIME)) == 0) { From 7e26efc345d6084387cce412e2fc87efe3b57252 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 23 Jul 2015 10:40:19 -0700 Subject: [PATCH 09/41] Changed object keys to valid identifier names --- ext/call.cc | 8 +-- ext/server.cc | 2 +- src/server.js | 2 +- test/call_test.js | 4 +- test/end_to_end_test.js | 110 ++++++++++++++++++++-------------------- 5 files changed, 63 insertions(+), 63 deletions(-) diff --git a/ext/call.cc b/ext/call.cc index 15c9b2d9..9a018169 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -192,7 +192,7 @@ class SendMetadataOp : public Op { } protected: std::string GetTypeString() const { - return "send metadata"; + return "send_metadata"; } }; @@ -216,7 +216,7 @@ class SendMessageOp : public Op { } protected: std::string GetTypeString() const { - return "send message"; + return "send_message"; } }; @@ -232,7 +232,7 @@ class SendClientCloseOp : public Op { } protected: std::string GetTypeString() const { - return "client close"; + return "client_close"; } }; @@ -276,7 +276,7 @@ class SendServerStatusOp : public Op { } protected: std::string GetTypeString() const { - return "send status"; + return "send_status"; } }; diff --git a/ext/server.cc b/ext/server.cc index 34cde9ff..8554fce7 100644 --- a/ext/server.cc +++ b/ext/server.cc @@ -108,7 +108,7 @@ class NewCallOp : public Op { protected: std::string GetTypeString() const { - return "new call"; + return "new_call"; } }; diff --git a/src/server.js b/src/server.js index 0a3a0031..68647741 100644 --- a/src/server.js +++ b/src/server.js @@ -544,7 +544,7 @@ function Server(options) { if (err) { return; } - var details = event['new call']; + var details = event.new_call; var call = details.call; var method = details.method; var metadata = details.metadata; diff --git a/test/call_test.js b/test/call_test.js index 98158fff..ff41b26b 100644 --- a/test/call_test.js +++ b/test/call_test.js @@ -132,7 +132,7 @@ describe('call', function() { 'key2': ['value2']}; call.startBatch(batch, function(err, resp) { assert.ifError(err); - assert.deepEqual(resp, {'send metadata': true}); + assert.deepEqual(resp, {'send_metadata': true}); done(); }); }); @@ -147,7 +147,7 @@ describe('call', function() { }; call.startBatch(batch, function(err, resp) { assert.ifError(err); - assert.deepEqual(resp, {'send metadata': true}); + assert.deepEqual(resp, {'send_metadata': true}); done(); }); }); diff --git a/test/end_to_end_test.js b/test/end_to_end_test.js index 667852f3..5d3baf82 100644 --- a/test/end_to_end_test.js +++ b/test/end_to_end_test.js @@ -85,37 +85,37 @@ describe('end-to-end', function() { call.startBatch(client_batch, function(err, response) { assert.ifError(err); assert.deepEqual(response, { - 'send metadata': true, - 'client close': true, - 'metadata': {}, - 'status': { - 'code': grpc.status.OK, - 'details': status_text, - 'metadata': {} + send_metadata: true, + client_close: true, + metadata: {}, + status: { + code: grpc.status.OK, + details: status_text, + metadata: {} } }); done(); }); server.requestCall(function(err, call_details) { - var new_call = call_details['new call']; + var new_call = call_details.new_call; assert.notEqual(new_call, null); var server_call = new_call.call; assert.notEqual(server_call, null); var server_batch = {}; server_batch[grpc.opType.SEND_INITIAL_METADATA] = {}; server_batch[grpc.opType.SEND_STATUS_FROM_SERVER] = { - 'metadata': {}, - 'code': grpc.status.OK, - 'details': status_text + metadata: {}, + code: grpc.status.OK, + details: status_text }; server_batch[grpc.opType.RECV_CLOSE_ON_SERVER] = true; server_call.startBatch(server_batch, function(err, response) { assert.ifError(err); assert.deepEqual(response, { - 'send metadata': true, - 'send status': true, - 'cancelled': false + send_metadata: true, + send_status: true, + cancelled: false }); done(); }); @@ -131,7 +131,7 @@ describe('end-to-end', function() { Infinity); var client_batch = {}; client_batch[grpc.opType.SEND_INITIAL_METADATA] = { - 'client_key': ['client_value'] + client_key: ['client_value'] }; client_batch[grpc.opType.SEND_CLOSE_FROM_CLIENT] = true; client_batch[grpc.opType.RECV_INITIAL_METADATA] = true; @@ -139,18 +139,18 @@ describe('end-to-end', function() { call.startBatch(client_batch, function(err, response) { assert.ifError(err); assert.deepEqual(response,{ - 'send metadata': true, - 'client close': true, + send_metadata: true, + client_close: true, metadata: {server_key: ['server_value']}, - status: {'code': grpc.status.OK, - 'details': status_text, - 'metadata': {}} + status: {code: grpc.status.OK, + details: status_text, + metadata: {}} }); done(); }); server.requestCall(function(err, call_details) { - var new_call = call_details['new call']; + var new_call = call_details.new_call; assert.notEqual(new_call, null); assert.strictEqual(new_call.metadata.client_key[0], 'client_value'); @@ -158,20 +158,20 @@ describe('end-to-end', function() { assert.notEqual(server_call, null); var server_batch = {}; server_batch[grpc.opType.SEND_INITIAL_METADATA] = { - 'server_key': ['server_value'] + server_key: ['server_value'] }; server_batch[grpc.opType.SEND_STATUS_FROM_SERVER] = { - 'metadata': {}, - 'code': grpc.status.OK, - 'details': status_text + metadata: {}, + code: grpc.status.OK, + details: status_text }; server_batch[grpc.opType.RECV_CLOSE_ON_SERVER] = true; server_call.startBatch(server_batch, function(err, response) { assert.ifError(err); assert.deepEqual(response, { - 'send metadata': true, - 'send status': true, - 'cancelled': false + send_metadata: true, + send_status: true, + cancelled: false }); done(); }); @@ -196,19 +196,19 @@ describe('end-to-end', function() { client_batch[grpc.opType.RECV_STATUS_ON_CLIENT] = true; call.startBatch(client_batch, function(err, response) { assert.ifError(err); - assert(response['send metadata']); - assert(response['client close']); + assert(response.send_metadata); + assert(response.client_close); assert.deepEqual(response.metadata, {}); - assert(response['send message']); + assert(response.send_message); assert.strictEqual(response.read.toString(), reply_text); - assert.deepEqual(response.status, {'code': grpc.status.OK, - 'details': status_text, - 'metadata': {}}); + assert.deepEqual(response.status, {code: grpc.status.OK, + details: status_text, + metadata: {}}); done(); }); server.requestCall(function(err, call_details) { - var new_call = call_details['new call']; + var new_call = call_details.new_call; assert.notEqual(new_call, null); var server_call = new_call.call; assert.notEqual(server_call, null); @@ -217,18 +217,18 @@ describe('end-to-end', function() { server_batch[grpc.opType.RECV_MESSAGE] = true; server_call.startBatch(server_batch, function(err, response) { assert.ifError(err); - assert(response['send metadata']); + assert(response.send_metadata); assert.strictEqual(response.read.toString(), req_text); var response_batch = {}; response_batch[grpc.opType.SEND_MESSAGE] = new Buffer(reply_text); response_batch[grpc.opType.SEND_STATUS_FROM_SERVER] = { - 'metadata': {}, - 'code': grpc.status.OK, - 'details': status_text + metadata: {}, + code: grpc.status.OK, + details: status_text }; response_batch[grpc.opType.RECV_CLOSE_ON_SERVER] = true; server_call.startBatch(response_batch, function(err, response) { - assert(response['send status']); + assert(response.send_status); assert(!response.cancelled); done(); }); @@ -251,9 +251,9 @@ describe('end-to-end', function() { call.startBatch(client_batch, function(err, response) { assert.ifError(err); assert.deepEqual(response, { - 'send metadata': true, - 'send message': true, - 'metadata': {} + send_metadata: true, + send_message: true, + metadata: {} }); var req2_batch = {}; req2_batch[grpc.opType.SEND_MESSAGE] = new Buffer(requests[1]); @@ -262,12 +262,12 @@ describe('end-to-end', function() { call.startBatch(req2_batch, function(err, resp) { assert.ifError(err); assert.deepEqual(resp, { - 'send message': true, - 'client close': true, - 'status': { - 'code': grpc.status.OK, - 'details': status_text, - 'metadata': {} + send_message: true, + client_close: true, + status: { + code: grpc.status.OK, + details: status_text, + metadata: {} } }); done(); @@ -275,7 +275,7 @@ describe('end-to-end', function() { }); server.requestCall(function(err, call_details) { - var new_call = call_details['new call']; + var new_call = call_details.new_call; assert.notEqual(new_call, null); var server_call = new_call.call; assert.notEqual(server_call, null); @@ -284,7 +284,7 @@ describe('end-to-end', function() { server_batch[grpc.opType.RECV_MESSAGE] = true; server_call.startBatch(server_batch, function(err, response) { assert.ifError(err); - assert(response['send metadata']); + assert(response.send_metadata); assert.strictEqual(response.read.toString(), requests[0]); var snd_batch = {}; snd_batch[grpc.opType.RECV_MESSAGE] = true; @@ -294,13 +294,13 @@ describe('end-to-end', function() { var end_batch = {}; end_batch[grpc.opType.RECV_CLOSE_ON_SERVER] = true; end_batch[grpc.opType.SEND_STATUS_FROM_SERVER] = { - 'metadata': {}, - 'code': grpc.status.OK, - 'details': status_text + metadata: {}, + code: grpc.status.OK, + details: status_text }; server_call.startBatch(end_batch, function(err, response) { assert.ifError(err); - assert(response['send status']); + assert(response.send_status); assert(!response.cancelled); done(); }); From ddfc22f305b7e6c56428518009202e910a63d60e Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 23 Jul 2015 11:28:16 -0700 Subject: [PATCH 10/41] Merge github.com:grpc/grpc into sometimes-its-good-just-to-check-in-with-each-other --- test/surface_test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/surface_test.js b/test/surface_test.js index 3cb68f8c..1dbe4e28 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -260,9 +260,8 @@ describe('Echo metadata', function() { }); it('shows the correct user-agent string', function(done) { var version = require('../package.json').version; - var call = client.unary({}, function(err, data) { - assert.ifError(err); - }, {key: ['value']}); + var call = client.unary({}, function(err, data) { assert.ifError(err); }, + {key: ['value']}); call.on('metadata', function(metadata) { assert(_.startsWith(metadata['user-agent'], 'grpc-node/' + version)); done(); From 7693e6676bc2534c321e1a04b2a9329c7ee92da3 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 24 Jul 2015 10:43:27 -0700 Subject: [PATCH 11/41] Exposed channel target and call peer in Node wrapper --- ext/call.cc | 14 ++++++++++++++ ext/call.h | 1 + ext/channel.cc | 11 +++++++++++ ext/channel.h | 1 + src/client.js | 16 ++++++++++++++++ src/server.js | 16 ++++++++++++++++ test/call_test.js | 6 ++++++ test/channel_test.js | 6 ++++++ test/surface_test.js | 40 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 111 insertions(+) diff --git a/ext/call.cc b/ext/call.cc index 15c9b2d9..b647735e 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -453,6 +453,8 @@ void Call::Init(Handle exports) { NanNew(StartBatch)->GetFunction()); NanSetPrototypeTemplate(tpl, "cancel", NanNew(Cancel)->GetFunction()); + NanSetPrototypeTemplate(tpl, "getPeer", + NanNew(GetPeer)->GetFunction()); NanAssignPersistent(fun_tpl, tpl); Handle ctr = tpl->GetFunction(); ctr->Set(NanNew("WRITE_BUFFER_HINT"), @@ -608,5 +610,17 @@ NAN_METHOD(Call::Cancel) { NanReturnUndefined(); } +NAN_METHOD(Call::GetPeer) { + NanScope(); + if (!HasInstance(args.This())) { + return NanThrowTypeError("getPeer can only be called on Call objects"); + } + Call *call = ObjectWrap::Unwrap(args.This()); + char *peer = grpc_call_get_peer(call->wrapped_call); + Handle peer_value = NanNew(peer); + gpr_free(peer); + NanReturnValue(peer_value); +} + } // namespace node } // namespace grpc diff --git a/ext/call.h b/ext/call.h index 43142c70..6acda761 100644 --- a/ext/call.h +++ b/ext/call.h @@ -120,6 +120,7 @@ class Call : public ::node::ObjectWrap { static NAN_METHOD(New); static NAN_METHOD(StartBatch); static NAN_METHOD(Cancel); + static NAN_METHOD(GetPeer); static NanCallback *constructor; // Used for typechecking instances of this javascript class static v8::Persistent fun_tpl; diff --git a/ext/channel.cc b/ext/channel.cc index d37bf763..0b7333e4 100644 --- a/ext/channel.cc +++ b/ext/channel.cc @@ -76,6 +76,8 @@ void Channel::Init(Handle exports) { tpl->InstanceTemplate()->SetInternalFieldCount(1); NanSetPrototypeTemplate(tpl, "close", NanNew(Close)->GetFunction()); + NanSetPrototypeTemplate(tpl, "getTarget", + NanNew(GetTarget)->GetFunction()); NanAssignPersistent(fun_tpl, tpl); Handle ctr = tpl->GetFunction(); constructor = new NanCallback(ctr); @@ -185,5 +187,14 @@ NAN_METHOD(Channel::Close) { NanReturnUndefined(); } +NAN_METHOD(Channel::GetTarget) { + NanScope(); + if (!HasInstance(args.This())) { + return NanThrowTypeError("getTarget can only be called on Channel objects"); + } + Channel *channel = ObjectWrap::Unwrap(args.This()); + NanReturnValue(NanNew(grpc_channel_get_target(channel->wrapped_channel))); +} + } // namespace node } // namespace grpc diff --git a/ext/channel.h b/ext/channel.h index b3aa0f70..6725ebb0 100644 --- a/ext/channel.h +++ b/ext/channel.h @@ -66,6 +66,7 @@ class Channel : public ::node::ObjectWrap { static NAN_METHOD(New); static NAN_METHOD(Close); + static NAN_METHOD(GetTarget); static NanCallback *constructor; static v8::Persistent fun_tpl; diff --git a/src/client.js b/src/client.js index da6327b4..d89c656c 100644 --- a/src/client.js +++ b/src/client.js @@ -187,6 +187,19 @@ ClientReadableStream.prototype.cancel = cancel; ClientWritableStream.prototype.cancel = cancel; ClientDuplexStream.prototype.cancel = cancel; +/** + * Get the endpoint this call/stream is connected to. + * @return {string} The URI of the endpoint + */ +function getPeer() { + /* jshint validthis: true */ + return this.call.getPeer(); +} + +ClientReadableStream.prototype.getPeer = getPeer; +ClientWritableStream.prototype.getPeer = getPeer; +ClientDuplexStream.prototype.getPeer = getPeer; + /** * Get a function that can make unary requests to the specified method. * @param {string} method The name of the method to request @@ -223,6 +236,9 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { emitter.cancel = function cancel() { call.cancel(); }; + emitter.getPeer = function getPeer() { + return call.getPeer(); + }; this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); diff --git a/src/server.js b/src/server.js index 0a3a0031..cb86b95f 100644 --- a/src/server.js +++ b/src/server.js @@ -373,6 +373,19 @@ ServerDuplexStream.prototype._read = _read; ServerDuplexStream.prototype._write = _write; ServerDuplexStream.prototype.sendMetadata = sendMetadata; +/** + * Get the endpoint this call/stream is connected to. + * @return {string} The URI of the endpoint + */ +function getPeer() { + /* jshint validthis: true */ + return this.call.getPeer(); +} + +ServerReadableStream.prototype.getPeer = getPeer; +ServerWritableStream.prototype.getPeer = getPeer; +ServerDuplexStream.prototype.getPeer = getPeer; + /** * Fully handle a unary call * @param {grpc.Call} call The call to handle @@ -389,6 +402,9 @@ function handleUnary(call, handler, metadata) { call.startBatch(batch, function() {}); } }; + emitter.getPeer = function() { + return call.getPeer(); + }; emitter.on('error', function(error) { handleError(call, error); }); diff --git a/test/call_test.js b/test/call_test.js index 98158fff..0079144a 100644 --- a/test/call_test.js +++ b/test/call_test.js @@ -184,4 +184,10 @@ describe('call', function() { }); }); }); + describe('getPeer', function() { + it('should return a string', function() { + var call = new grpc.Call(channel, 'method', getDeadline(1)); + assert.strictEqual(typeof call.getPeer(), 'string'); + }); + }); }); diff --git a/test/channel_test.js b/test/channel_test.js index 33200c99..3e61d3bb 100644 --- a/test/channel_test.js +++ b/test/channel_test.js @@ -87,4 +87,10 @@ describe('channel', function() { }); }); }); + describe('getTarget', function() { + it('should return a string', function() { + var channel = new grpc.Channel('localhost', {}); + assert.strictEqual(typeof channel.getTarget(), 'string'); + }); + }); }); diff --git a/test/surface_test.js b/test/surface_test.js index 3cb68f8c..9005cbd5 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -344,6 +344,9 @@ describe('Other conditions', function() { after(function() { server.shutdown(); }); + it('channel.getTarget should be available', function() { + assert.strictEqual(typeof client.channel.getTarget(), 'string'); + }); describe('Server recieving bad input', function() { var misbehavingClient; var badArg = new Buffer([0xFF]); @@ -549,6 +552,43 @@ describe('Other conditions', function() { }); }); }); + describe('call.getPeer should return the peer', function() { + it('for a unary call', function(done) { + var call = client.unary({error: false}, function(err, data) { + assert.ifError(err); + done(); + }); + assert.strictEqual(typeof call.getPeer(), 'string'); + }); + it('for a client stream call', function(done) { + var call = client.clientStream(function(err, data) { + assert.ifError(err); + done(); + }); + assert.strictEqual(typeof call.getPeer(), 'string'); + call.write({error: false}); + call.end(); + }); + it('for a server stream call', function(done) { + var call = client.serverStream({error: false}); + assert.strictEqual(typeof call.getPeer(), 'string'); + call.on('data', function(){}); + call.on('status', function(status) { + assert.strictEqual(status.code, grpc.status.OK); + done(); + }); + }); + it('for a bidi stream call', function(done) { + var call = client.bidiStream(); + assert.strictEqual(typeof call.getPeer(), 'string'); + call.write({error: false}); + call.end(); + call.on('data', function(){}); + call.on('status', function(status) { + done(); + }); + }); + }); }); describe('Cancelling surface client', function() { var client; From 5be7958fcb52bba4b34fa01d99fb6a17d31bf92a Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 24 Jul 2015 14:23:44 -0700 Subject: [PATCH 12/41] Added documentation command and settings to Node package --- jsdoc_conf.json | 22 ++++++++++++++++++++++ package.json | 4 +++- 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 jsdoc_conf.json diff --git a/jsdoc_conf.json b/jsdoc_conf.json new file mode 100644 index 00000000..876a8e19 --- /dev/null +++ b/jsdoc_conf.json @@ -0,0 +1,22 @@ +{ + "tags": { + "allowUnknownTags": true + }, + "source": { + "include": [ "index.js", "src" ], + "includePattern": ".+\\.js(doc)?$", + "excludePattern": "(^|\\/|\\\\)_" + }, + "opts": { + "package": "package.json", + "readme": "README.md" + }, + "plugins": [], + "templates": { + "cleverLinks": false, + "monospaceLinks": false, + "default": { + "outputSourceFiles": true + } + } +} diff --git a/package.json b/package.json index 1caf1587..756d41b0 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,8 @@ }, "scripts": { "lint": "node ./node_modules/jshint/bin/jshint src test examples interop index.js", - "test": "node ./node_modules/mocha/bin/mocha && npm run-script lint" + "test": "node ./node_modules/mocha/bin/mocha && npm run-script lint", + "gen_docs": "./node_modules/.bin/jsdoc -c jsdoc_conf.json" }, "dependencies": { "bindings": "^1.2.0", @@ -32,6 +33,7 @@ "devDependencies": { "async": "^0.9.0", "google-auth-library": "^0.9.2", + "jsdoc": "^3.3.2", "jshint": "^2.5.0", "minimist": "^1.1.0", "mocha": "~1.21.0", From 8ea6be054acfb9def20bb3bfd7eff4d26dabf68a Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 27 Jul 2015 11:23:13 -0700 Subject: [PATCH 13/41] Rearranged some code for jsdoc, added some documentation --- index.js | 32 +++++++++++++------------------ src/client.js | 25 ++++++++++++++----------- src/common.js | 52 ++++++++++++++++++++------------------------------- src/server.js | 45 ++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 90 insertions(+), 64 deletions(-) diff --git a/index.js b/index.js index d81e7804..b26ab35f 100644 --- a/index.js +++ b/index.js @@ -48,7 +48,7 @@ var grpc = require('bindings')('grpc'); * @param {ProtoBuf.Reflect.Namespace} value The ProtoBuf object to load. * @return {Object} The resulting gRPC object */ -function loadObject(value) { +exports.loadObject = function loadObject(value) { var result = {}; if (value.className === 'Namespace') { _.each(value.children, function(child) { @@ -62,7 +62,9 @@ function loadObject(value) { } else { return value; } -} +}; + +var loadObject = exports.loadObject; /** * Load a gRPC object from a .proto file. @@ -71,7 +73,7 @@ function loadObject(value) { * 'json'. Defaults to 'proto' * @return {Object} The resulting gRPC object */ -function load(filename, format) { +exports.load = function load(filename, format) { if (!format) { format = 'proto'; } @@ -88,7 +90,7 @@ function load(filename, format) { } return loadObject(builder.ns); -} +}; /** * Get a function that a client can use to update metadata with authentication @@ -97,7 +99,7 @@ function load(filename, format) { * @param {Object} credential The credential object to use * @return {function(Object, callback)} Metadata updater function */ -function getGoogleAuthDelegate(credential) { +exports.getGoogleAuthDelegate = function getGoogleAuthDelegate(credential) { /** * Update a metadata object with authentication information. * @param {string} authURI The uri to authenticate to @@ -120,20 +122,10 @@ function getGoogleAuthDelegate(credential) { callback(null, metadata); }); }; -} +}; /** - * See docs for loadObject - */ -exports.loadObject = loadObject; - -/** - * See docs for load - */ -exports.load = load; - -/** - * See docs for Server + * @see module:src/server.Server */ exports.Server = server.Server; @@ -141,6 +133,7 @@ exports.Server = server.Server; * Status name to code number mapping */ exports.status = grpc.status; + /** * Call error name to code number mapping */ @@ -156,6 +149,7 @@ exports.Credentials = grpc.Credentials; */ exports.ServerCredentials = grpc.ServerCredentials; -exports.getGoogleAuthDelegate = getGoogleAuthDelegate; - +/** + * @see module:src/client.makeClientConstructor + */ exports.makeGenericClientConstructor = client.makeClientConstructor; diff --git a/src/client.js b/src/client.js index da6327b4..a7cfd0e6 100644 --- a/src/client.js +++ b/src/client.js @@ -31,6 +31,11 @@ * */ +/** + * Server module + * @module + */ + 'use strict'; var _ = require('lodash'); @@ -72,6 +77,7 @@ function ClientWritableStream(call, serialize) { /** * Attempt to write the given chunk. Calls the callback when done. This is an * implementation of a method needed for implementing stream.Writable. + * @access private * @param {Buffer} chunk The chunk to write * @param {string} encoding Ignored * @param {function(Error=)} callback Called when the write is complete @@ -110,6 +116,7 @@ function ClientReadableStream(call, deserialize) { /** * Read the next object from the stream. + * @access private * @param {*} size Ignored because we use objectMode=true */ function _read(size) { @@ -503,7 +510,7 @@ var requester_makers = { * @param {string} serviceName The name of the service * @return {function(string, Object)} New client constructor */ -function makeClientConstructor(methods, serviceName) { +exports.makeClientConstructor = function(methods, serviceName) { /** * Create a client with the given methods * @constructor @@ -552,7 +559,7 @@ function makeClientConstructor(methods, serviceName) { }); return Client; -} +}; /** * Creates a constructor for clients for the given service @@ -560,22 +567,18 @@ function makeClientConstructor(methods, serviceName) { * for * @return {function(string, Object)} New client constructor */ -function makeProtobufClientConstructor(service) { +exports.makeProtobufClientConstructor = function(service) { var method_attrs = common.getProtobufServiceAttrs(service, service.name); - var Client = makeClientConstructor(method_attrs); + var Client = exports.makeClientConstructor(method_attrs); Client.service = service; - return Client; -} - -exports.makeClientConstructor = makeClientConstructor; - -exports.makeProtobufClientConstructor = makeProtobufClientConstructor; +}; /** - * See docs for client.status + * Map of status code names to status codes */ exports.status = grpc.status; + /** * See docs for client.callError */ diff --git a/src/common.js b/src/common.js index feaa859a..5551ebee 100644 --- a/src/common.js +++ b/src/common.js @@ -31,6 +31,10 @@ * */ +/** + * @module + */ + 'use strict'; var _ = require('lodash'); @@ -40,7 +44,7 @@ var _ = require('lodash'); * @param {function()} cls The constructor of the message type to deserialize * @return {function(Buffer):cls} The deserialization function */ -function deserializeCls(cls) { +exports.deserializeCls = function deserializeCls(cls) { /** * Deserialize a buffer to a message object * @param {Buffer} arg_buf The buffer to deserialize @@ -51,14 +55,16 @@ function deserializeCls(cls) { // and longs as strings (second argument) return cls.decode(arg_buf).toRaw(false, true); }; -} +}; + +var deserializeCls = exports.deserializeCls; /** * Get a function that serializes objects to a buffer by protobuf class. * @param {function()} Cls The constructor of the message type to serialize * @return {function(Cls):Buffer} The serialization function */ -function serializeCls(Cls) { +exports.serializeCls = function serializeCls(Cls) { /** * Serialize an object to a Buffer * @param {Object} arg The object to serialize @@ -67,14 +73,16 @@ function serializeCls(Cls) { return function serialize(arg) { return new Buffer(new Cls(arg).encode().toBuffer()); }; -} +}; + +var serializeCls = exports.serializeCls; /** * Get the fully qualified (dotted) name of a ProtoBuf.Reflect value. * @param {ProtoBuf.Reflect.Namespace} value The value to get the name of * @return {string} The fully qualified name of the value */ -function fullyQualifiedName(value) { +exports.fullyQualifiedName = function fullyQualifiedName(value) { if (value === null || value === undefined) { return ''; } @@ -89,7 +97,9 @@ function fullyQualifiedName(value) { } } return name; -} +}; + +var fullyQualifiedName = exports.fullyQualifiedName; /** * Wrap a function to pass null-like values through without calling it. If no @@ -97,7 +107,7 @@ function fullyQualifiedName(value) { * @param {?function} func The function to wrap * @return {function} The wrapped function */ -function wrapIgnoreNull(func) { +exports.wrapIgnoreNull = function wrapIgnoreNull(func) { if (!func) { return _.identity; } @@ -107,14 +117,14 @@ function wrapIgnoreNull(func) { } return func(arg); }; -} +}; /** * Return a map from method names to method attributes for the service. * @param {ProtoBuf.Reflect.Service} service The service to get attributes for * @return {Object} The attributes map */ -function getProtobufServiceAttrs(service) { +exports.getProtobufServiceAttrs = function getProtobufServiceAttrs(service) { var prefix = '/' + fullyQualifiedName(service) + '/'; return _.object(_.map(service.children, function(method) { return [_.camelCase(method.name), { @@ -127,26 +137,4 @@ function getProtobufServiceAttrs(service) { responseDeserialize: deserializeCls(method.resolvedResponseType.build()) }]; })); -} - -/** - * See docs for deserializeCls - */ -exports.deserializeCls = deserializeCls; - -/** - * See docs for serializeCls - */ -exports.serializeCls = serializeCls; - -/** - * See docs for fullyQualifiedName - */ -exports.fullyQualifiedName = fullyQualifiedName; - -/** - * See docs for wrapIgnoreNull - */ -exports.wrapIgnoreNull = wrapIgnoreNull; - -exports.getProtobufServiceAttrs = getProtobufServiceAttrs; +}; diff --git a/src/server.js b/src/server.js index 0a3a0031..1c8a53e0 100644 --- a/src/server.js +++ b/src/server.js @@ -31,6 +31,11 @@ * */ +/** + * Server module + * @module + */ + 'use strict'; var _ = require('lodash'); @@ -50,6 +55,7 @@ var EventEmitter = require('events').EventEmitter; /** * Handle an error on a call by sending it as a status + * @access private * @param {grpc.Call} call The call to send the error on * @param {Object} error The error object */ @@ -82,6 +88,7 @@ function handleError(call, error) { /** * Wait for the client to close, then emit a cancelled event if the client * cancelled. + * @access private * @param {grpc.Call} call The call object to wait on * @param {EventEmitter} emitter The event emitter to emit the cancelled event * on @@ -102,6 +109,7 @@ function waitForCancel(call, emitter) { /** * Send a response to a unary or client streaming call. + * @access private * @param {grpc.Call} call The call to respond on * @param {*} value The value to respond with * @param {function(*):Buffer=} serialize Serialization function for the @@ -130,6 +138,7 @@ function sendUnaryResponse(call, value, serialize, metadata) { /** * Initialize a writable stream. This is used for both the writable and duplex * stream constructors. + * @access private * @param {Writable} stream The stream to set up * @param {function(*):Buffer=} Serialization function for responses */ @@ -203,6 +212,7 @@ function setUpWritable(stream, serialize) { /** * Initialize a readable stream. This is used for both the readable and duplex * stream constructors. + * @access private * @param {Readable} stream The stream to initialize * @param {function(Buffer):*=} deserialize Deserialization function for * incoming data. @@ -242,6 +252,7 @@ function ServerWritableStream(call, serialize) { /** * Start writing a chunk of data. This is an implementation of a method required * for implementing stream.Writable. + * @access private * @param {Buffer} chunk The chunk of data to write * @param {string} encoding Ignored * @param {function(Error=)} callback Callback to indicate that the write is @@ -266,6 +277,11 @@ function _write(chunk, encoding, callback) { ServerWritableStream.prototype._write = _write; +/** + * Send the initial metadata for a writable stream. + * @param {Object>} responseMetadata Metadata + * to send + */ function sendMetadata(responseMetadata) { /* jshint validthis: true */ if (!this.call.metadataSent) { @@ -281,6 +297,10 @@ function sendMetadata(responseMetadata) { } } +/** + * @inheritdoc + * @alias module:src/server~ServerWritableStream#sendMetadata + */ ServerWritableStream.prototype.sendMetadata = sendMetadata; util.inherits(ServerReadableStream, Readable); @@ -301,6 +321,7 @@ function ServerReadableStream(call, deserialize) { /** * Start reading from the gRPC data source. This is an implementation of a * method required for implementing stream.Readable + * @access private * @param {number} size Ignored */ function _read(size) { @@ -375,6 +396,7 @@ ServerDuplexStream.prototype.sendMetadata = sendMetadata; /** * Fully handle a unary call + * @access private * @param {grpc.Call} call The call to handle * @param {Object} handler Request handler object for the method that was called * @param {Object} metadata Metadata from the client @@ -426,6 +448,7 @@ function handleUnary(call, handler, metadata) { /** * Fully handle a server streaming call + * @access private * @param {grpc.Call} call The call to handle * @param {Object} handler Request handler object for the method that was called * @param {Object} metadata Metadata from the client @@ -454,6 +477,7 @@ function handleServerStreaming(call, handler, metadata) { /** * Fully handle a client streaming call + * @access private * @param {grpc.Call} call The call to handle * @param {Object} handler Request handler object for the method that was called * @param {Object} metadata Metadata from the client @@ -488,6 +512,7 @@ function handleClientStreaming(call, handler, metadata) { /** * Fully handle a bidirectional streaming call + * @access private * @param {grpc.Call} call The call to handle * @param {Object} handler Request handler object for the method that was called * @param {Object} metadata Metadata from the client @@ -571,7 +596,8 @@ function Server(options) { } server.requestCall(handleNewCall); }; - /** Shuts down the server. + /** + * Shuts down the server. */ this.shutdown = function() { server.shutdown(); @@ -605,6 +631,15 @@ Server.prototype.register = function(name, handler, serialize, deserialize, return true; }; +/** + * Add a service to the server, with a corresponding implementation. If you are + * generating this from a proto file, you should instead use + * addProtoService. + * @param {Object} service The service descriptor, as + * {@link module:src/common.getProtobufServiceAttrs} returns + * @param {Object} implementation Map of method names to + * method implementation for the provided service. + */ Server.prototype.addService = function(service, implementation) { if (this.started) { throw new Error('Can\'t add a service to a started server.'); @@ -642,6 +677,12 @@ Server.prototype.addService = function(service, implementation) { }); }; +/** + * Add a proto service to the server, with a corresponding implementation + * @param {Protobuf.Reflect.Service} service The proto service descriptor + * @param {Object} implementation Map of method names to + * method implementation for the provided service. + */ Server.prototype.addProtoService = function(service, implementation) { this.addService(common.getProtobufServiceAttrs(service), implementation); }; @@ -665,6 +706,6 @@ Server.prototype.bind = function(port, creds) { }; /** - * See documentation for Server + * @see module:src/server~Server */ exports.Server = Server; From 77d3141bf8785b626f359873e62108f91e2b2607 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 27 Jul 2015 14:16:44 -0700 Subject: [PATCH 14/41] Added explicit insecure credentials constructors --- ext/credentials.cc | 47 ++++++++++++++++++++++++++++++++++------------ ext/credentials.h | 1 + 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/ext/credentials.cc b/ext/credentials.cc index 34872017..23fe20ed 100644 --- a/ext/credentials.cc +++ b/ext/credentials.cc @@ -83,6 +83,8 @@ void Credentials::Init(Handle exports) { NanNew(CreateFake)->GetFunction()); ctr->Set(NanNew("createIam"), NanNew(CreateIam)->GetFunction()); + ctr->Set(NanNew("createInsecure"), + NanNew(CreateIam)->GetFunction()); constructor = new NanCallback(ctr); exports->Set(NanNew("Credentials"), ctr); } @@ -94,9 +96,6 @@ bool Credentials::HasInstance(Handle val) { Handle Credentials::WrapStruct(grpc_credentials *credentials) { NanEscapableScope(); - if (credentials == NULL) { - return NanEscapeScope(NanNull()); - } const int argc = 1; Handle argv[argc] = { NanNew(reinterpret_cast(credentials))}; @@ -130,7 +129,11 @@ NAN_METHOD(Credentials::New) { NAN_METHOD(Credentials::CreateDefault) { NanScope(); - NanReturnValue(WrapStruct(grpc_google_default_credentials_create())); + grpc_credentials *creds = grpc_google_default_credentials_create(); + if (creds == NULL) { + NanReturnNull(); + } + NanReturnValue(WrapStruct(creds)); } NAN_METHOD(Credentials::CreateSsl) { @@ -154,9 +157,12 @@ NAN_METHOD(Credentials::CreateSsl) { return NanThrowTypeError( "createSSl's third argument must be a Buffer if provided"); } - - NanReturnValue(WrapStruct(grpc_ssl_credentials_create( - root_certs, key_cert_pair.private_key == NULL ? NULL : &key_cert_pair))); + grpc_credentials *creds = grpc_ssl_credentials_create( + root_certs, key_cert_pair.private_key == NULL ? NULL : &key_cert_pair); + if (creds == NULL) { + NanReturnNull(); + } + NanReturnValue(WrapStruct(creds)); } NAN_METHOD(Credentials::CreateComposite) { @@ -171,13 +177,21 @@ NAN_METHOD(Credentials::CreateComposite) { } Credentials *creds1 = ObjectWrap::Unwrap(args[0]->ToObject()); Credentials *creds2 = ObjectWrap::Unwrap(args[1]->ToObject()); - NanReturnValue(WrapStruct(grpc_composite_credentials_create( - creds1->wrapped_credentials, creds2->wrapped_credentials))); + grpc_credentials *creds = grpc_composite_credentials_create( + creds1->wrapped_credentials, creds2->wrapped_credentials); + if (creds == NULL) { + NanReturnNull(); + } + NanReturnValue(WrapStruct(creds)); } NAN_METHOD(Credentials::CreateGce) { NanScope(); - NanReturnValue(WrapStruct(grpc_compute_engine_credentials_create())); + grpc_credentials *creds = grpc_compute_engine_credentials_create(); + if (creds == NULL) { + NanReturnNull(); + } + NanReturnValue(WrapStruct(creds)); } NAN_METHOD(Credentials::CreateFake) { @@ -195,8 +209,17 @@ NAN_METHOD(Credentials::CreateIam) { } NanUtf8String auth_token(args[0]); NanUtf8String auth_selector(args[1]); - NanReturnValue( - WrapStruct(grpc_iam_credentials_create(*auth_token, *auth_selector))); + grpc_credentisl *creds = grpc_iam_credentials_create(*auth_token, + *auth_selector); + if (creds == NULL) { + NanReturnNull(); + } + NanReturnValue(WrapStruct(creds)); +} + +NAN_METHOD(Credentials::CreateInsecure) { + NanScope(); + NanReturnValue(WrapStruct(NULL)); } } // namespace node diff --git a/ext/credentials.h b/ext/credentials.h index 794736fe..62957e61 100644 --- a/ext/credentials.h +++ b/ext/credentials.h @@ -68,6 +68,7 @@ class Credentials : public ::node::ObjectWrap { static NAN_METHOD(CreateGce); static NAN_METHOD(CreateFake); static NAN_METHOD(CreateIam); + static NAN_METHOD(CreateInsecure); static NanCallback *constructor; // Used for typechecking instances of this javascript class static v8::Persistent fun_tpl; From 954538063c547210cb87a2a28c26bac1de37e99b Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 27 Jul 2015 14:56:40 -0700 Subject: [PATCH 15/41] Made credentials an explicit required argument to channels --- examples/perf_test.js | 3 +- examples/qps_test.js | 3 +- examples/route_guide_client.js | 3 +- examples/stock_client.js | 3 +- ext/channel.cc | 52 ++++++++++++++++++---------------- ext/credentials.cc | 4 +-- interop/interop_client.js | 8 ++++-- src/client.js | 6 ++-- test/call_test.js | 6 ++-- test/channel_test.js | 30 +++++++++++++------- test/end_to_end_test.js | 4 ++- test/health_test.js | 3 +- test/math_client_test.js | 3 +- test/surface_test.js | 14 +++++---- 14 files changed, 85 insertions(+), 57 deletions(-) diff --git a/examples/perf_test.js b/examples/perf_test.js index da919ece..0f38725f 100644 --- a/examples/perf_test.js +++ b/examples/perf_test.js @@ -41,7 +41,8 @@ var interop_server = require('../interop/interop_server.js'); function runTest(iterations, callback) { var testServer = interop_server.getServer(0, false); testServer.server.listen(); - var client = new testProto.TestService('localhost:' + testServer.port); + var client = new testProto.TestService('localhost:' + testServer.port, + grpc.Credentials.createInsecure()); function runIterations(finish) { var start = process.hrtime(); diff --git a/examples/qps_test.js b/examples/qps_test.js index 00293b46..1ce4dbe0 100644 --- a/examples/qps_test.js +++ b/examples/qps_test.js @@ -61,7 +61,8 @@ var interop_server = require('../interop/interop_server.js'); function runTest(concurrent_calls, seconds, callback) { var testServer = interop_server.getServer(0, false); testServer.server.listen(); - var client = new testProto.TestService('localhost:' + testServer.port); + var client = new testProto.TestService('localhost:' + testServer.port, + grpc.Credentials.createInsecure()); var warmup_num = 100; diff --git a/examples/route_guide_client.js b/examples/route_guide_client.js index 8cd532fe..647f3ffa 100644 --- a/examples/route_guide_client.js +++ b/examples/route_guide_client.js @@ -40,7 +40,8 @@ var path = require('path'); var _ = require('lodash'); var grpc = require('..'); var examples = grpc.load(__dirname + '/route_guide.proto').examples; -var client = new examples.RouteGuide('localhost:50051'); +var client = new examples.RouteGuide('localhost:50051', + grpc.Credentials.createInsecure()); var COORD_FACTOR = 1e7; diff --git a/examples/stock_client.js b/examples/stock_client.js index b37e66df..ab9b050e 100644 --- a/examples/stock_client.js +++ b/examples/stock_client.js @@ -38,7 +38,8 @@ var examples = grpc.load(__dirname + '/stock.proto').examples; * This exports a client constructor for the Stock service. The usage looks like * * var StockClient = require('stock_client.js'); - * var stockClient = new StockClient(server_address); + * var stockClient = new StockClient(server_address, + * grpc.Credentials.createInsecure()); * stockClient.getLastTradePrice({symbol: 'GOOG'}, function(error, response) { * console.log(error || response); * }); diff --git a/ext/channel.cc b/ext/channel.cc index c43b55f1..d02ed956 100644 --- a/ext/channel.cc +++ b/ext/channel.cc @@ -98,31 +98,30 @@ NAN_METHOD(Channel::New) { if (args.IsConstructCall()) { if (!args[0]->IsString()) { - return NanThrowTypeError("Channel expects a string and an object"); + return NanThrowTypeError( + "Channel expects a string, a credential and an object"); } grpc_channel *wrapped_channel; // Owned by the Channel object NanUtf8String *host = new NanUtf8String(args[0]); NanUtf8String *host_override = NULL; - if (args[1]->IsUndefined()) { + grpc_credentials *creds; + if (!Credentials::HasInstance(args[1])) { + return NanThrowTypeError( + "Channel's second argument must be a credential"); + } + Credentials *creds_object = ObjectWrap::Unwrap( + args[1]->ToObject()); + creds = creds_object->GetWrappedCredentials(); + grpc_channel_args *channel_args_ptr; + if (args[2]->IsUndefined()) { + channel_args_ptr = NULL; wrapped_channel = grpc_insecure_channel_create(**host, NULL); - } else if (args[1]->IsObject()) { - grpc_credentials *creds = NULL; - Handle args_hash(args[1]->ToObject()->Clone()); + } else if (args[2]->IsObject()) { + Handle args_hash(args[2]->ToObject()->Clone()); if (args_hash->HasOwnProperty(NanNew(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG))) { host_override = new NanUtf8String(args_hash->Get(NanNew(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG))); } - if (args_hash->HasOwnProperty(NanNew("credentials"))) { - Handle creds_value = args_hash->Get(NanNew("credentials")); - if (!Credentials::HasInstance(creds_value)) { - return NanThrowTypeError( - "credentials arg must be a Credentials object"); - } - Credentials *creds_object = - ObjectWrap::Unwrap(creds_value->ToObject()); - creds = creds_object->GetWrappedCredentials(); - args_hash->Delete(NanNew("credentials")); - } Handle keys(args_hash->GetOwnPropertyNames()); grpc_channel_args channel_args; channel_args.num_args = keys->Length(); @@ -149,16 +148,19 @@ NAN_METHOD(Channel::New) { return NanThrowTypeError("Arg values must be strings"); } } - if (creds == NULL) { - wrapped_channel = grpc_insecure_channel_create(**host, &channel_args); - } else { - wrapped_channel = - grpc_secure_channel_create(creds, **host, &channel_args); - } - free(channel_args.args); + channel_args_ptr = &channel_args; } else { return NanThrowTypeError("Channel expects a string and an object"); } + if (creds == NULL) { + wrapped_channel = grpc_insecure_channel_create(**host, channel_args_ptr); + } else { + wrapped_channel = + grpc_secure_channel_create(creds, **host, channel_args_ptr); + } + if (channel_args_ptr != NULL) { + free(channel_args_ptr->args); + } Channel *channel; if (host_override == NULL) { channel = new Channel(wrapped_channel, host); @@ -168,8 +170,8 @@ NAN_METHOD(Channel::New) { channel->Wrap(args.This()); NanReturnValue(args.This()); } else { - const int argc = 2; - Local argv[argc] = {args[0], args[1]}; + const int argc = 3; + Local argv[argc] = {args[0], args[1], args[2]}; NanReturnValue(constructor->GetFunction()->NewInstance(argc, argv)); } } diff --git a/ext/credentials.cc b/ext/credentials.cc index a695e365..21d61f1a 100644 --- a/ext/credentials.cc +++ b/ext/credentials.cc @@ -82,7 +82,7 @@ void Credentials::Init(Handle exports) { ctr->Set(NanNew("createIam"), NanNew(CreateIam)->GetFunction()); ctr->Set(NanNew("createInsecure"), - NanNew(CreateIam)->GetFunction()); + NanNew(CreateInsecure)->GetFunction()); constructor = new NanCallback(ctr); exports->Set(NanNew("Credentials"), ctr); } @@ -202,7 +202,7 @@ NAN_METHOD(Credentials::CreateIam) { } NanUtf8String auth_token(args[0]); NanUtf8String auth_selector(args[1]); - grpc_credentisl *creds = grpc_iam_credentials_create(*auth_token, + grpc_credentials *creds = grpc_iam_credentials_create(*auth_token, *auth_selector); if (creds == NULL) { NanReturnNull(); diff --git a/interop/interop_client.js b/interop/interop_client.js index e810e68e..236b3661 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -397,6 +397,7 @@ var test_cases = { function runTest(address, host_override, test_case, tls, test_ca, done) { // TODO(mlumish): enable TLS functionality var options = {}; + var creds; if (tls) { var ca_path; if (test_ca) { @@ -405,13 +406,14 @@ function runTest(address, host_override, test_case, tls, test_ca, done) { ca_path = process.env.SSL_CERT_FILE; } var ca_data = fs.readFileSync(ca_path); - var creds = grpc.Credentials.createSsl(ca_data); - options.credentials = creds; + creds = grpc.Credentials.createSsl(ca_data); if (host_override) { options['grpc.ssl_target_name_override'] = host_override; } + } else { + creds = grpc.Credentials.createInsecure(); } - var client = new testProto.TestService(address, options); + var client = new testProto.TestService(address, creds, options); test_cases[test_case](client, done); } diff --git a/src/client.js b/src/client.js index d89c656c..50338a6e 100644 --- a/src/client.js +++ b/src/client.js @@ -524,11 +524,13 @@ function makeClientConstructor(methods, serviceName) { * Create a client with the given methods * @constructor * @param {string} address The address of the server to connect to + * @param {grpc.Credentials} credentials Credentials to use to connect + * to the server * @param {Object} options Options to pass to the underlying channel * @param {function(string, Object, function)=} updateMetadata function to * update the metadata for each request */ - function Client(address, options, updateMetadata) { + function Client(address, credentials, options, updateMetadata) { if (!updateMetadata) { updateMetadata = function(uri, metadata, callback) { callback(null, metadata); @@ -538,7 +540,7 @@ function makeClientConstructor(methods, serviceName) { options = {}; } options['grpc.primary_user_agent'] = 'grpc-node/' + version; - this.channel = new grpc.Channel(address, options); + this.channel = new grpc.Channel(address, credentials, options); this.server_address = address.replace(/\/$/, ''); this.auth_uri = this.server_address + '/' + serviceName; this.updateMetadata = updateMetadata; diff --git a/test/call_test.js b/test/call_test.js index 942c31ac..c5edab8b 100644 --- a/test/call_test.js +++ b/test/call_test.js @@ -48,6 +48,8 @@ function getDeadline(timeout_secs) { return deadline; } +var insecureCreds = grpc.Credentials.createInsecure(); + describe('call', function() { var channel; var server; @@ -55,7 +57,7 @@ describe('call', function() { server = new grpc.Server(); var port = server.addHttp2Port('localhost:0'); server.start(); - channel = new grpc.Channel('localhost:' + port); + channel = new grpc.Channel('localhost:' + port, insecureCreds); }); after(function() { server.shutdown(); @@ -82,7 +84,7 @@ describe('call', function() { }); }); it('should fail with a closed channel', function() { - var local_channel = new grpc.Channel('hostname'); + var local_channel = new grpc.Channel('hostname', insecureCreds); local_channel.close(); assert.throws(function() { new grpc.Call(channel, 'method'); diff --git a/test/channel_test.js b/test/channel_test.js index 3e61d3bb..3cb43334 100644 --- a/test/channel_test.js +++ b/test/channel_test.js @@ -36,11 +36,13 @@ var assert = require('assert'); var grpc = require('bindings')('grpc.node'); +var insecureCreds = grpc.Credentials.createInsecure(); + describe('channel', function() { describe('constructor', function() { it('should require a string for the first argument', function() { assert.doesNotThrow(function() { - new grpc.Channel('hostname'); + new grpc.Channel('hostname', insecureCreds); }); assert.throws(function() { new grpc.Channel(); @@ -49,38 +51,46 @@ describe('channel', function() { new grpc.Channel(5); }); }); - it('should accept an object for the second parameter', function() { + it('should accept a credential for the second argument', function() { assert.doesNotThrow(function() { - new grpc.Channel('hostname', {}); + new grpc.Channel('hostname', insecureCreds); }); assert.throws(function() { new grpc.Channel('hostname', 5); }); }); + it('should accept an object for the third argument', function() { + assert.doesNotThrow(function() { + new grpc.Channel('hostname', insecureCreds, {}); + }); + assert.throws(function() { + new grpc.Channel('hostname', insecureCreds, 'abc'); + }); + }); it('should only accept objects with string or int values', function() { assert.doesNotThrow(function() { - new grpc.Channel('hostname', {'key' : 'value'}); + new grpc.Channel('hostname', insecureCreds,{'key' : 'value'}); }); assert.doesNotThrow(function() { - new grpc.Channel('hostname', {'key' : 5}); + new grpc.Channel('hostname', insecureCreds, {'key' : 5}); }); assert.throws(function() { - new grpc.Channel('hostname', {'key' : null}); + new grpc.Channel('hostname', insecureCreds, {'key' : null}); }); assert.throws(function() { - new grpc.Channel('hostname', {'key' : new Date()}); + new grpc.Channel('hostname', insecureCreds, {'key' : new Date()}); }); }); }); describe('close', function() { it('should succeed silently', function() { - var channel = new grpc.Channel('hostname', {}); + var channel = new grpc.Channel('hostname', insecureCreds, {}); assert.doesNotThrow(function() { channel.close(); }); }); it('should be idempotent', function() { - var channel = new grpc.Channel('hostname', {}); + var channel = new grpc.Channel('hostname', insecureCreds, {}); assert.doesNotThrow(function() { channel.close(); channel.close(); @@ -89,7 +99,7 @@ describe('channel', function() { }); describe('getTarget', function() { it('should return a string', function() { - var channel = new grpc.Channel('localhost', {}); + var channel = new grpc.Channel('localhost', insecureCreds, {}); assert.strictEqual(typeof channel.getTarget(), 'string'); }); }); diff --git a/test/end_to_end_test.js b/test/end_to_end_test.js index 5d3baf82..9133ceca 100644 --- a/test/end_to_end_test.js +++ b/test/end_to_end_test.js @@ -57,6 +57,8 @@ function multiDone(done, count) { }; } +var insecureCreds = grpc.Credentials.createInsecure(); + describe('end-to-end', function() { var server; var channel; @@ -64,7 +66,7 @@ describe('end-to-end', function() { server = new grpc.Server(); var port_num = server.addHttp2Port('0.0.0.0:0'); server.start(); - channel = new grpc.Channel('localhost:' + port_num); + channel = new grpc.Channel('localhost:' + port_num, insecureCreds); }); after(function() { server.shutdown(); diff --git a/test/health_test.js b/test/health_test.js index bb700cc4..93b068be 100644 --- a/test/health_test.js +++ b/test/health_test.js @@ -56,7 +56,8 @@ describe('Health Checking', function() { before(function() { var port_num = healthServer.bind('0.0.0.0:0'); healthServer.start(); - healthClient = new health.Client('localhost:' + port_num); + healthClient = new health.Client('localhost:' + port_num, + grpc.Credentials.createInsecure()); }); after(function() { healthServer.shutdown(); diff --git a/test/math_client_test.js b/test/math_client_test.js index f2751857..1bd85ca4 100644 --- a/test/math_client_test.js +++ b/test/math_client_test.js @@ -53,7 +53,8 @@ describe('Math client', function() { before(function(done) { var port_num = server.bind('0.0.0.0:0'); server.start(); - math_client = new math.Math('localhost:' + port_num); + math_client = new math.Math('localhost:' + port_num, + grpc.Credentials.createInsecure()); done(); }); after(function() { diff --git a/test/surface_test.js b/test/surface_test.js index 9005cbd5..d63f48d6 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -124,7 +124,7 @@ describe('Echo service', function() { }); var port = server.bind('localhost:0'); var Client = surface_client.makeProtobufClientConstructor(echo_service); - client = new Client('localhost:' + port); + client = new Client('localhost:' + port, grpc.Credentials.createInsecure()); server.start(); }); after(function() { @@ -169,7 +169,8 @@ describe('Generic client and server', function() { var port = server.bind('localhost:0'); server.start(); var Client = grpc.makeGenericClientConstructor(string_service_attrs); - client = new Client('localhost:' + port); + client = new Client('localhost:' + port, + grpc.Credentials.createInsecure()); }); after(function() { server.shutdown(); @@ -216,7 +217,7 @@ describe('Echo metadata', function() { }); var port = server.bind('localhost:0'); var Client = surface_client.makeProtobufClientConstructor(test_service); - client = new Client('localhost:' + port); + client = new Client('localhost:' + port, grpc.Credentials.createInsecure()); server.start(); }); after(function() { @@ -338,7 +339,7 @@ describe('Other conditions', function() { }); port = server.bind('localhost:0'); var Client = surface_client.makeProtobufClientConstructor(test_service); - client = new Client('localhost:' + port); + client = new Client('localhost:' + port, grpc.Credentials.createInsecure()); server.start(); }); after(function() { @@ -383,7 +384,8 @@ describe('Other conditions', function() { }; var Client = surface_client.makeClientConstructor(test_service_attrs, 'TestService'); - misbehavingClient = new Client('localhost:' + port); + misbehavingClient = new Client('localhost:' + port, + grpc.Credentials.createInsecure()); }); it('should respond correctly to a unary call', function(done) { misbehavingClient.unary(badArg, function(err, data) { @@ -603,7 +605,7 @@ describe('Cancelling surface client', function() { }); var port = server.bind('localhost:0'); var Client = surface_client.makeProtobufClientConstructor(mathService); - client = new Client('localhost:' + port); + client = new Client('localhost:' + port, grpc.Credentials.createInsecure()); server.start(); }); after(function() { From eda1e5fb016ea0098c3ccb59253aa9a5336a861c Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 27 Jul 2015 15:54:13 -0700 Subject: [PATCH 16/41] Added test that credential channel argument is required --- test/channel_test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/channel_test.js b/test/channel_test.js index 3cb43334..c991d7b2 100644 --- a/test/channel_test.js +++ b/test/channel_test.js @@ -51,13 +51,16 @@ describe('channel', function() { new grpc.Channel(5); }); }); - it('should accept a credential for the second argument', function() { + it('should require a credential for the second argument', function() { assert.doesNotThrow(function() { new grpc.Channel('hostname', insecureCreds); }); assert.throws(function() { new grpc.Channel('hostname', 5); }); + assert.throws(function() { + new grpc.Channel('hostname'); + }); }); it('should accept an object for the third argument', function() { assert.doesNotThrow(function() { From d03da0cd75a1c223402ded660cf1cd533ac857da Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 27 Jul 2015 16:13:28 -0700 Subject: [PATCH 17/41] Made binding a server to a port insecurely explicit --- examples/math_server.js | 2 +- examples/route_guide_server.js | 2 +- examples/stock_server.js | 2 +- ext/server.cc | 44 +++++++++++++--------------------- ext/server.h | 1 - ext/server_credentials.cc | 18 ++++++++++---- ext/server_credentials.h | 1 + interop/interop_server.js | 4 +++- src/server.js | 6 +---- test/call_test.js | 3 ++- test/end_to_end_test.js | 3 ++- test/health_test.js | 3 ++- test/math_client_test.js | 3 ++- test/server_test.js | 19 ++++++++------- test/surface_test.js | 12 ++++++---- 15 files changed, 62 insertions(+), 61 deletions(-) diff --git a/examples/math_server.js b/examples/math_server.js index b1f8a632..31892c65 100644 --- a/examples/math_server.js +++ b/examples/math_server.js @@ -115,7 +115,7 @@ server.addProtoService(math.Math.service, { }); if (require.main === module) { - server.bind('0.0.0.0:50051'); + server.bind('0.0.0.0:50051', grpc.ServerCredentials.createInsecure()); server.start(); } diff --git a/examples/route_guide_server.js b/examples/route_guide_server.js index 70044a32..bb8e79b5 100644 --- a/examples/route_guide_server.js +++ b/examples/route_guide_server.js @@ -239,7 +239,7 @@ function getServer() { if (require.main === module) { // If this is run as a script, start a server on an unused port var routeServer = getServer(); - routeServer.bind('0.0.0.0:50051'); + routeServer.bind('0.0.0.0:50051', grpc.ServerCredentials.createInsecure()); var argv = parseArgs(process.argv, { string: 'db_path' }); diff --git a/examples/stock_server.js b/examples/stock_server.js index f2eb6ad4..dfcfe30e 100644 --- a/examples/stock_server.js +++ b/examples/stock_server.js @@ -80,7 +80,7 @@ stockServer.addProtoService(examples.Stock.service, { }); if (require.main === module) { - stockServer.bind('0.0.0.0:50051'); + stockServer.bind('0.0.0.0:50051', grpc.ServerCredentials.createInsecure()); stockServer.listen(); } diff --git a/ext/server.cc b/ext/server.cc index 8554fce7..04fabc87 100644 --- a/ext/server.cc +++ b/ext/server.cc @@ -136,10 +136,6 @@ void Server::Init(Handle exports) { tpl, "addHttp2Port", NanNew(AddHttp2Port)->GetFunction()); - NanSetPrototypeTemplate( - tpl, "addSecureHttp2Port", - NanNew(AddSecureHttp2Port)->GetFunction()); - NanSetPrototypeTemplate(tpl, "start", NanNew(Start)->GetFunction()); @@ -246,45 +242,37 @@ NAN_METHOD(Server::RequestCall) { } NAN_METHOD(Server::AddHttp2Port) { - NanScope(); - if (!HasInstance(args.This())) { - return NanThrowTypeError("addHttp2Port can only be called on a Server"); - } - if (!args[0]->IsString()) { - return NanThrowTypeError("addHttp2Port's argument must be a String"); - } - Server *server = ObjectWrap::Unwrap(args.This()); - if (server->wrapped_server == NULL) { - return NanThrowError("addHttp2Port cannot be called on a shut down Server"); - } - NanReturnValue(NanNew(grpc_server_add_http2_port( - server->wrapped_server, *NanUtf8String(args[0])))); -} - -NAN_METHOD(Server::AddSecureHttp2Port) { NanScope(); if (!HasInstance(args.This())) { return NanThrowTypeError( - "addSecureHttp2Port can only be called on a Server"); + "addHttp2Port can only be called on a Server"); } if (!args[0]->IsString()) { return NanThrowTypeError( - "addSecureHttp2Port's first argument must be a String"); + "addHttp2Port's first argument must be a String"); } if (!ServerCredentials::HasInstance(args[1])) { return NanThrowTypeError( - "addSecureHttp2Port's second argument must be ServerCredentials"); + "addHttp2Port's second argument must be ServerCredentials"); } Server *server = ObjectWrap::Unwrap(args.This()); if (server->wrapped_server == NULL) { return NanThrowError( - "addSecureHttp2Port cannot be called on a shut down Server"); + "addHttp2Port cannot be called on a shut down Server"); } - ServerCredentials *creds = ObjectWrap::Unwrap( + ServerCredentials *creds_object = ObjectWrap::Unwrap( args[1]->ToObject()); - NanReturnValue(NanNew(grpc_server_add_secure_http2_port( - server->wrapped_server, *NanUtf8String(args[0]), - creds->GetWrappedServerCredentials()))); + grpc_server_credentials *creds = creds_object->GetWrappedServerCredentials(); + int port; + if (creds == NULL) { + port = grpc_server_add_http2_port(server->wrapped_server, + *NanUtf8String(args[0])); + } else { + port = grpc_server_add_secure_http2_port(server->wrapped_server, + *NanUtf8String(args[0]), + creds); + } + NanReturnValue(NanNew(port)); } NAN_METHOD(Server::Start) { diff --git a/ext/server.h b/ext/server.h index 5b4b18a0..faab7e34 100644 --- a/ext/server.h +++ b/ext/server.h @@ -66,7 +66,6 @@ class Server : public ::node::ObjectWrap { static NAN_METHOD(New); static NAN_METHOD(RequestCall); static NAN_METHOD(AddHttp2Port); - static NAN_METHOD(AddSecureHttp2Port); static NAN_METHOD(Start); static NAN_METHOD(Shutdown); static NanCallback *constructor; diff --git a/ext/server_credentials.cc b/ext/server_credentials.cc index 66aaa330..51cdbcde 100644 --- a/ext/server_credentials.cc +++ b/ext/server_credentials.cc @@ -73,6 +73,8 @@ void ServerCredentials::Init(Handle exports) { Handle ctr = tpl->GetFunction(); ctr->Set(NanNew("createSsl"), NanNew(CreateSsl)->GetFunction()); + ctr->Set(NanNew("createInsecure"), + NanNew(CreateInsecure)->GetFunction()); constructor = new NanCallback(ctr); exports->Set(NanNew("ServerCredentials"), ctr); } @@ -85,9 +87,6 @@ bool ServerCredentials::HasInstance(Handle val) { Handle ServerCredentials::WrapStruct( grpc_server_credentials *credentials) { NanEscapableScope(); - if (credentials == NULL) { - return NanEscapeScope(NanNull()); - } const int argc = 1; Handle argv[argc] = { NanNew(reinterpret_cast(credentials))}; @@ -138,8 +137,17 @@ NAN_METHOD(ServerCredentials::CreateSsl) { return NanThrowTypeError("createSsl's third argument must be a Buffer"); } key_cert_pair.cert_chain = ::node::Buffer::Data(args[2]); - NanReturnValue(WrapStruct( - grpc_ssl_server_credentials_create(root_certs, &key_cert_pair, 1))); + grpc_server_credentials *creds = + grpc_ssl_server_credentials_create(root_certs, &key_cert_pair, 1); + if (creds == NULL) { + NanReturnNull(); + } + NanReturnValue(WrapStruct(creds)); +} + +NAN_METHOD(ServerCredentials::CreateInsecure) { + NanScope(); + NanReturnValue(WrapStruct(NULL)); } } // namespace node diff --git a/ext/server_credentials.h b/ext/server_credentials.h index 80747504..63903f66 100644 --- a/ext/server_credentials.h +++ b/ext/server_credentials.h @@ -63,6 +63,7 @@ class ServerCredentials : public ::node::ObjectWrap { static NAN_METHOD(New); static NAN_METHOD(CreateSsl); + static NAN_METHOD(CreateInsecure); static NanCallback *constructor; // Used for typechecking instances of this javascript class static v8::Persistent fun_tpl; diff --git a/interop/interop_server.js b/interop/interop_server.js index 505c6bb5..ece22cce 100644 --- a/interop/interop_server.js +++ b/interop/interop_server.js @@ -161,7 +161,7 @@ function handleHalfDuplex(call) { function getServer(port, tls) { // TODO(mlumish): enable TLS functionality var options = {}; - var server_creds = null; + var server_creds; if (tls) { var key_path = path.join(__dirname, '../test/data/server1.key'); var pem_path = path.join(__dirname, '../test/data/server1.pem'); @@ -171,6 +171,8 @@ function getServer(port, tls) { server_creds = grpc.ServerCredentials.createSsl(null, key_data, pem_data); + } else { + server_creds = grpc.ServerCredentials.createInsecure(); } var server = new grpc.Server(options); server.addProtoService(testProto.TestService.service, { diff --git a/src/server.js b/src/server.js index e876313d..fac013f4 100644 --- a/src/server.js +++ b/src/server.js @@ -673,11 +673,7 @@ Server.prototype.bind = function(port, creds) { if (this.started) { throw new Error('Can\'t bind an already running server to an address'); } - if (creds) { - return this._server.addSecureHttp2Port(port, creds); - } else { - return this._server.addHttp2Port(port); - } + return this._server.addHttp2Port(port, creds); }; /** diff --git a/test/call_test.js b/test/call_test.js index 942c31ac..4f183949 100644 --- a/test/call_test.js +++ b/test/call_test.js @@ -53,7 +53,8 @@ describe('call', function() { var server; before(function() { server = new grpc.Server(); - var port = server.addHttp2Port('localhost:0'); + var port = server.addHttp2Port('localhost:0', + grpc.ServerCredentials.createInsecure()); server.start(); channel = new grpc.Channel('localhost:' + port); }); diff --git a/test/end_to_end_test.js b/test/end_to_end_test.js index 5d3baf82..bb8ad625 100644 --- a/test/end_to_end_test.js +++ b/test/end_to_end_test.js @@ -62,7 +62,8 @@ describe('end-to-end', function() { var channel; before(function() { server = new grpc.Server(); - var port_num = server.addHttp2Port('0.0.0.0:0'); + var port_num = server.addHttp2Port('0.0.0.0:0', + grpc.ServerCredentials.createInsecure()); server.start(); channel = new grpc.Channel('localhost:' + port_num); }); diff --git a/test/health_test.js b/test/health_test.js index bb700cc4..fa23dc3e 100644 --- a/test/health_test.js +++ b/test/health_test.js @@ -54,7 +54,8 @@ describe('Health Checking', function() { new health.Implementation(statusMap)); var healthClient; before(function() { - var port_num = healthServer.bind('0.0.0.0:0'); + var port_num = healthServer.bind('0.0.0.0:0', + grpc.ServerCredentials.createInsecure()); healthServer.start(); healthClient = new health.Client('localhost:' + port_num); }); diff --git a/test/math_client_test.js b/test/math_client_test.js index f2751857..567faf9c 100644 --- a/test/math_client_test.js +++ b/test/math_client_test.js @@ -51,7 +51,8 @@ var server = require('../examples/math_server.js'); describe('Math client', function() { before(function(done) { - var port_num = server.bind('0.0.0.0:0'); + var port_num = server.bind('0.0.0.0:0', + grpc.ServerCredentials.createInsecure()); server.start(); math_client = new math.Math('localhost:' + port_num); done(); diff --git a/test/server_test.js b/test/server_test.js index 9c7bb465..a9df4390 100644 --- a/test/server_test.js +++ b/test/server_test.js @@ -59,16 +59,11 @@ describe('server', function() { it('should bind to an unused port', function() { var port; assert.doesNotThrow(function() { - port = server.addHttp2Port('0.0.0.0:0'); + port = server.addHttp2Port('0.0.0.0:0', + grpc.ServerCredentials.createInsecure()); }); assert(port > 0); }); - }); - describe('addSecureHttp2Port', function() { - var server; - before(function() { - server = new grpc.Server(); - }); it('should bind to an unused port with ssl credentials', function() { var port; var key_path = path.join(__dirname, '../test/data/server1.key'); @@ -77,16 +72,22 @@ describe('server', function() { var pem_data = fs.readFileSync(pem_path); var creds = grpc.ServerCredentials.createSsl(null, key_data, pem_data); assert.doesNotThrow(function() { - port = server.addSecureHttp2Port('0.0.0.0:0', creds); + port = server.addHttp2Port('0.0.0.0:0', creds); }); assert(port > 0); }); }); + describe('addSecureHttp2Port', function() { + var server; + before(function() { + server = new grpc.Server(); + }); + }); describe('listen', function() { var server; before(function() { server = new grpc.Server(); - server.addHttp2Port('0.0.0.0:0'); + server.addHttp2Port('0.0.0.0:0', grpc.ServerCredentials.createInsecure()); }); after(function() { server.shutdown(); diff --git a/test/surface_test.js b/test/surface_test.js index 9005cbd5..fd326e44 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -47,6 +47,8 @@ var mathService = math_proto.lookup('math.Math'); var _ = require('lodash'); +var server_insecure_creds = grpc.ServerCredentials.createInsecure(); + describe('File loader', function() { it('Should load a proto file by default', function() { assert.doesNotThrow(function() { @@ -122,7 +124,7 @@ describe('Echo service', function() { callback(null, call.request); } }); - var port = server.bind('localhost:0'); + var port = server.bind('localhost:0', server_insecure_creds); var Client = surface_client.makeProtobufClientConstructor(echo_service); client = new Client('localhost:' + port); server.start(); @@ -166,7 +168,7 @@ describe('Generic client and server', function() { callback(null, _.capitalize(call.request)); } }); - var port = server.bind('localhost:0'); + var port = server.bind('localhost:0', server_insecure_creds); server.start(); var Client = grpc.makeGenericClientConstructor(string_service_attrs); client = new Client('localhost:' + port); @@ -214,7 +216,7 @@ describe('Echo metadata', function() { }); } }); - var port = server.bind('localhost:0'); + var port = server.bind('localhost:0', server_insecure_creds); var Client = surface_client.makeProtobufClientConstructor(test_service); client = new Client('localhost:' + port); server.start(); @@ -336,7 +338,7 @@ describe('Other conditions', function() { }); } }); - port = server.bind('localhost:0'); + port = server.bind('localhost:0', server_insecure_creds); var Client = surface_client.makeProtobufClientConstructor(test_service); client = new Client('localhost:' + port); server.start(); @@ -601,7 +603,7 @@ describe('Cancelling surface client', function() { 'fib': function(stream) {}, 'sum': function(stream) {} }); - var port = server.bind('localhost:0'); + var port = server.bind('localhost:0', server_insecure_creds); var Client = surface_client.makeProtobufClientConstructor(mathService); client = new Client('localhost:' + port); server.start(); From c6d3cb0dfec6b39f9097288734dd57120950bda0 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 28 Jul 2015 15:18:57 -0700 Subject: [PATCH 18/41] Wrap connectivity API, expose it to client as waitForReady --- ext/channel.cc | 80 +++++++++++++++++++++++++ ext/channel.h | 2 + ext/completion_queue_async_worker.cc | 2 +- ext/node_grpc.cc | 19 ++++++ src/client.js | 41 +++++++++++++ test/channel_test.js | 88 +++++++++++++++++++++++++++- test/constant_test.js | 19 ++++++ test/surface_test.js | 76 ++++++++++++++++++++++++ 8 files changed, 323 insertions(+), 4 deletions(-) diff --git a/ext/channel.cc b/ext/channel.cc index c43b55f1..fa99c10d 100644 --- a/ext/channel.cc +++ b/ext/channel.cc @@ -33,12 +33,17 @@ #include +#include "grpc/support/log.h" + #include #include #include "grpc/grpc.h" #include "grpc/grpc_security.h" +#include "call.h" #include "channel.h" +#include "completion_queue_async_worker.h" #include "credentials.h" +#include "timeval.h" namespace grpc { namespace node { @@ -51,11 +56,31 @@ using v8::Handle; using v8::HandleScope; using v8::Integer; using v8::Local; +using v8::Number; using v8::Object; using v8::Persistent; using v8::String; using v8::Value; +class ConnectivityStateOp : public Op { + public: + Handle GetNodeValue() const { + return NanNew(new_state); + } + + bool ParseOp(Handle value, grpc_op *out, + shared_ptr resources) { + return true; + } + + grpc_connectivity_state new_state; + + protected: + std::string GetTypeString() const { + return "new_state"; + } +}; + NanCallback *Channel::constructor; Persistent Channel::fun_tpl; @@ -78,6 +103,12 @@ void Channel::Init(Handle exports) { NanNew(Close)->GetFunction()); NanSetPrototypeTemplate(tpl, "getTarget", NanNew(GetTarget)->GetFunction()); + NanSetPrototypeTemplate( + tpl, "getConnectivityState", + NanNew(GetConnectivityState)->GetFunction()); + NanSetPrototypeTemplate( + tpl, "watchConnectivityState", + NanNew(WatchConnectivityState)->GetFunction()); NanAssignPersistent(fun_tpl, tpl); Handle ctr = tpl->GetFunction(); constructor = new NanCallback(ctr); @@ -196,5 +227,54 @@ NAN_METHOD(Channel::GetTarget) { NanReturnValue(NanNew(grpc_channel_get_target(channel->wrapped_channel))); } +NAN_METHOD(Channel::GetConnectivityState) { + NanScope(); + if (!HasInstance(args.This())) { + return NanThrowTypeError( + "getConnectivityState can only be called on Channel objects"); + } + Channel *channel = ObjectWrap::Unwrap(args.This()); + int try_to_connect = (int)args[0]->Equals(NanTrue()); + NanReturnValue(grpc_channel_check_connectivity_state(channel->wrapped_channel, + try_to_connect)); +} + +NAN_METHOD(Channel::WatchConnectivityState) { + NanScope(); + if (!HasInstance(args.This())) { + return NanThrowTypeError( + "watchConnectivityState can only be called on Channel objects"); + } + if (!args[0]->IsUint32()) { + return NanThrowTypeError( + "watchConnectivityState's first argument must be a channel state"); + } + if (!(args[1]->IsNumber() || args[1]->IsDate())) { + return NanThrowTypeError( + "watchConnectivityState's second argument must be a date or a number"); + } + if (!args[2]->IsFunction()) { + return NanThrowTypeError( + "watchConnectivityState's third argument must be a callback"); + } + grpc_connectivity_state last_state = + static_cast(args[0]->Uint32Value()); + double deadline = args[1]->NumberValue(); + Handle callback_func = args[2].As(); + NanCallback *callback = new NanCallback(callback_func); + Channel *channel = ObjectWrap::Unwrap(args.This()); + ConnectivityStateOp *op = new ConnectivityStateOp(); + unique_ptr ops(new OpVec()); + ops->push_back(unique_ptr(op)); + grpc_channel_watch_connectivity_state( + channel->wrapped_channel, last_state, &op->new_state, + MillisecondsToTimespec(deadline), CompletionQueueAsyncWorker::GetQueue(), + new struct tag(callback, + ops.release(), + shared_ptr(nullptr))); + CompletionQueueAsyncWorker::Next(); + NanReturnUndefined(); +} + } // namespace node } // namespace grpc diff --git a/ext/channel.h b/ext/channel.h index 6725ebb0..c4e83a32 100644 --- a/ext/channel.h +++ b/ext/channel.h @@ -67,6 +67,8 @@ class Channel : public ::node::ObjectWrap { static NAN_METHOD(New); static NAN_METHOD(Close); static NAN_METHOD(GetTarget); + static NAN_METHOD(GetConnectivityState); + static NAN_METHOD(WatchConnectivityState); static NanCallback *constructor; static v8::Persistent fun_tpl; diff --git a/ext/completion_queue_async_worker.cc b/ext/completion_queue_async_worker.cc index 1215c97e..c45e303f 100644 --- a/ext/completion_queue_async_worker.cc +++ b/ext/completion_queue_async_worker.cc @@ -65,7 +65,7 @@ void CompletionQueueAsyncWorker::Execute() { result = grpc_completion_queue_next(queue, gpr_inf_future(GPR_CLOCK_REALTIME)); if (!result.success) { - SetErrorMessage("The batch encountered an error"); + SetErrorMessage("The asnyc function encountered an error"); } } diff --git a/ext/node_grpc.cc b/ext/node_grpc.cc index 4e31cbaa..331ccb60 100644 --- a/ext/node_grpc.cc +++ b/ext/node_grpc.cc @@ -159,12 +159,31 @@ void InitOpTypeConstants(Handle exports) { op_type->Set(NanNew("RECV_CLOSE_ON_SERVER"), RECV_CLOSE_ON_SERVER); } +void InitConnectivityStateConstants(Handle exports) { + NanScope(); + Handle channel_state = NanNew(); + exports->Set(NanNew("connectivityState"), channel_state); + Handle IDLE(NanNew(GRPC_CHANNEL_IDLE)); + channel_state->Set(NanNew("IDLE"), IDLE); + Handle CONNECTING(NanNew(GRPC_CHANNEL_CONNECTING)); + channel_state->Set(NanNew("CONNECTING"), CONNECTING); + Handle READY(NanNew(GRPC_CHANNEL_READY)); + channel_state->Set(NanNew("READY"), READY); + Handle TRANSIENT_FAILURE( + NanNew(GRPC_CHANNEL_TRANSIENT_FAILURE)); + channel_state->Set(NanNew("TRANSIENT_FAILURE"), TRANSIENT_FAILURE); + Handle FATAL_FAILURE( + NanNew(GRPC_CHANNEL_FATAL_FAILURE)); + channel_state->Set(NanNew("FATAL_FAILURE"), FATAL_FAILURE); +} + void init(Handle exports) { NanScope(); grpc_init(); InitStatusConstants(exports); InitCallErrorConstants(exports); InitOpTypeConstants(exports); + InitConnectivityStateConstants(exports); grpc::node::Call::Init(exports); grpc::node::Channel::Init(exports); diff --git a/src/client.js b/src/client.js index f843669b..39392dff 100644 --- a/src/client.js +++ b/src/client.js @@ -551,6 +551,47 @@ exports.makeClientConstructor = function(methods, serviceName) { this.updateMetadata = updateMetadata; } + /** + * Wait for the client to be ready. The callback will be called when the + * client has successfully connected to the server, and it will be called + * with an error if the attempt to connect to the server has unrecoverablly + * failed or if the deadline expires. This function does not automatically + * attempt to initiate the connection, so the callback will not be called + * unless you also start a method call or call $tryConnect. + * @param {(Date|Number)} deadline When to stop waiting for a connection. Pass + * Infinity to wait forever. + * @param {function(Error)} callback The callback to call when done attempting + * to connect. + */ + Client.prototype.$waitForReady = function(deadline, callback) { + var self = this; + var checkState = function(err, result) { + if (err) { + callback(new Error('Failed to connect before the deadline')); + } + var new_state = result.new_state; + console.log(result); + if (new_state === grpc.connectivityState.READY) { + callback(); + } else if (new_state === grpc.connectivityState.FATAL_FAILURE) { + callback(new Error('Failed to connect to server')); + } else { + self.channel.watchConnectivityState(new_state, deadline, checkState); + } + }; + checkState(null, {new_state: this.channel.getConnectivityState()}); + }; + + /** + * Attempt to connect to the server. That will happen automatically if + * you try to make a method call with this client, so this function should + * only be used if you want to know that you have a connection before making + * any calls. + */ + Client.prototype.$tryConnect = function() { + this.channel.getConnectivityState(true); + }; + _.each(methods, function(attrs, name) { var method_type; if (attrs.requestStream) { diff --git a/test/channel_test.js b/test/channel_test.js index 3e61d3bb..feb3e672 100644 --- a/test/channel_test.js +++ b/test/channel_test.js @@ -36,6 +36,27 @@ var assert = require('assert'); var grpc = require('bindings')('grpc.node'); +/** + * This is used for testing functions with multiple asynchronous calls that + * can happen in different orders. This should be passed the number of async + * function invocations that can occur last, and each of those should call this + * function's return value + * @param {function()} done The function that should be called when a test is + * complete. + * @param {number} count The number of calls to the resulting function if the + * test passes. + * @return {function()} The function that should be called at the end of each + * sequence of asynchronous functions. + */ +function multiDone(done, count) { + return function() { + count -= 1; + if (count <= 0) { + done(); + } + }; +} + describe('channel', function() { describe('constructor', function() { it('should require a string for the first argument', function() { @@ -73,14 +94,16 @@ describe('channel', function() { }); }); describe('close', function() { + var channel; + beforeEach(function() { + channel = new grpc.Channel('hostname', {}); + }); it('should succeed silently', function() { - var channel = new grpc.Channel('hostname', {}); assert.doesNotThrow(function() { channel.close(); }); }); it('should be idempotent', function() { - var channel = new grpc.Channel('hostname', {}); assert.doesNotThrow(function() { channel.close(); channel.close(); @@ -88,9 +111,68 @@ describe('channel', function() { }); }); describe('getTarget', function() { + var channel; + beforeEach(function() { + channel = new grpc.Channel('hostname', {}); + }); it('should return a string', function() { - var channel = new grpc.Channel('localhost', {}); assert.strictEqual(typeof channel.getTarget(), 'string'); }); }); + describe('getConnectivityState', function() { + var channel; + beforeEach(function() { + channel = new grpc.Channel('hostname', {}); + }); + it('should return IDLE for a new channel', function() { + assert.strictEqual(channel.getConnectivityState(), + grpc.connectivityState.IDLE); + }); + }); + describe('watchConnectivityState', function() { + var channel; + beforeEach(function() { + channel = new grpc.Channel('localhost', {}); + }); + afterEach(function() { + channel.close(); + }); + it('should time out if called alone', function(done) { + var old_state = channel.getConnectivityState(); + var deadline = new Date(); + deadline.setSeconds(deadline.getSeconds() + 1); + channel.watchConnectivityState(old_state, deadline, function(err, value) { + assert(err); + done(); + }); + }); + it('should complete if a connection attempt is forced', function(done) { + var old_state = channel.getConnectivityState(); + var deadline = new Date(); + deadline.setSeconds(deadline.getSeconds() + 1); + channel.watchConnectivityState(old_state, deadline, function(err, value) { + assert.ifError(err); + assert.notEqual(value.new_state, old_state); + done(); + }); + channel.getConnectivityState(true); + }); + it('should complete twice if called twice', function(done) { + done = multiDone(done, 2); + var old_state = channel.getConnectivityState(); + var deadline = new Date(); + deadline.setSeconds(deadline.getSeconds() + 1); + channel.watchConnectivityState(old_state, deadline, function(err, value) { + assert.ifError(err); + assert.notEqual(value.new_state, old_state); + done(); + }); + channel.watchConnectivityState(old_state, deadline, function(err, value) { + assert.ifError(err); + assert.notEqual(value.new_state, old_state); + done(); + }); + channel.getConnectivityState(true); + }); + }); }); diff --git a/test/constant_test.js b/test/constant_test.js index ecc98ec4..93bf0c8a 100644 --- a/test/constant_test.js +++ b/test/constant_test.js @@ -78,6 +78,19 @@ var callErrorNames = [ 'INVALID_FLAGS' ]; +/** + * List of all connectivity state names + * @const + * @type {Array.} + */ +var connectivityStateNames = [ + 'IDLE', + 'CONNECTING', + 'READY', + 'TRANSIENT_FAILURE', + 'FATAL_FAILURE' +]; + describe('constants', function() { it('should have all of the status constants', function() { for (var i = 0; i < statusNames.length; i++) { @@ -91,4 +104,10 @@ describe('constants', function() { 'call error missing: ' + callErrorNames[i]); } }); + it('should have all of the connectivity states', function() { + for (var i = 0; i < connectivityStateNames.length; i++) { + assert(grpc.connectivityState.hasOwnProperty(connectivityStateNames[i]), + 'connectivity status missing: ' + connectivityStateNames[i]); + } + }); }); diff --git a/test/surface_test.js b/test/surface_test.js index 98f9b15b..4711658e 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -47,6 +47,27 @@ var mathService = math_proto.lookup('math.Math'); var _ = require('lodash'); +/** + * This is used for testing functions with multiple asynchronous calls that + * can happen in different orders. This should be passed the number of async + * function invocations that can occur last, and each of those should call this + * function's return value + * @param {function()} done The function that should be called when a test is + * complete. + * @param {number} count The number of calls to the resulting function if the + * test passes. + * @return {function()} The function that should be called at the end of each + * sequence of asynchronous functions. + */ +function multiDone(done, count) { + return function() { + count -= 1; + if (count <= 0) { + done(); + } + }; +} + describe('File loader', function() { it('Should load a proto file by default', function() { assert.doesNotThrow(function() { @@ -110,6 +131,61 @@ describe('Server.prototype.addProtoService', function() { }); }); }); +describe('Client#$waitForReady', function() { + var server; + var port; + var Client; + var client; + before(function() { + server = new grpc.Server(); + port = server.bind('localhost:0'); + server.start(); + Client = surface_client.makeProtobufClientConstructor(mathService); + }); + beforeEach(function() { + client = new Client('localhost:' + port); + }); + after(function() { + server.shutdown(); + }); + it('should complete when a call is initiated', function(done) { + client.$waitForReady(Infinity, function(error) { + assert.ifError(error); + done(); + }); + var call = client.div({}, function(err, response) {}); + call.cancel(); + }); + it('should complete if $tryConnect is called', function(done) { + client.$waitForReady(Infinity, function(error) { + assert.ifError(error); + done(); + }); + client.$tryConnect(); + }); + it('should complete if called more than once', function(done) { + done = multiDone(done, 2); + client.$waitForReady(Infinity, function(error) { + assert.ifError(error); + done(); + }); + client.$waitForReady(Infinity, function(error) { + assert.ifError(error); + done(); + }); + client.$tryConnect(); + }); + it('should complete if called when already ready', function(done) { + client.$waitForReady(Infinity, function(error) { + assert.ifError(error); + client.$waitForReady(Infinity, function(error) { + assert.ifError(error); + done(); + }); + }); + client.$tryConnect(); + }); +}); describe('Echo service', function() { var server; var client; From 288bdb4eabdea544b9f264e4f6a8e9009b56cffd Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jul 2015 10:09:36 -0700 Subject: [PATCH 19/41] Ensure that client generated methods don't conflict with other properties --- src/client.js | 27 +++++++++++++++------------ test/surface_test.js | 20 +++++++++++++++++++- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/client.js b/src/client.js index f843669b..405e2be6 100644 --- a/src/client.js +++ b/src/client.js @@ -236,7 +236,7 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { deadline = Infinity; } var emitter = new EventEmitter(); - var call = new grpc.Call(this.channel, method, deadline); + var call = new grpc.Call(this.$channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } @@ -246,7 +246,7 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { emitter.getPeer = function getPeer() { return call.getPeer(); }; - this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { + this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); callback(error); @@ -309,12 +309,12 @@ function makeClientStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.channel, method, deadline); + var call = new grpc.Call(this.$channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientWritableStream(call, serialize); - this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { + this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); callback(error); @@ -383,12 +383,12 @@ function makeServerStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.channel, method, deadline); + var call = new grpc.Call(this.$channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientReadableStream(call, deserialize); - this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { + this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); stream.emit('error', error); @@ -455,12 +455,12 @@ function makeBidiStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.channel, method, deadline); + var call = new grpc.Call(this.$channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientDuplexStream(call, serialize, deserialize); - this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { + this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); stream.emit('error', error); @@ -545,14 +545,17 @@ exports.makeClientConstructor = function(methods, serviceName) { options = {}; } options['grpc.primary_user_agent'] = 'grpc-node/' + version; - this.channel = new grpc.Channel(address, options); - this.server_address = address.replace(/\/$/, ''); - this.auth_uri = this.server_address + '/' + serviceName; - this.updateMetadata = updateMetadata; + this.$channel = new grpc.Channel(address, options); + this.$server_address = address.replace(/\/$/, ''); + this.$auth_uri = this.$server_address + '/' + serviceName; + this.$updateMetadata = updateMetadata; } _.each(methods, function(attrs, name) { var method_type; + if (_.startsWith(name, '$')) { + throw new Error('Method names cannot start with $'); + } if (attrs.requestStream) { if (attrs.responseStream) { method_type = 'bidi'; diff --git a/test/surface_test.js b/test/surface_test.js index 98f9b15b..1afdb330 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -110,6 +110,24 @@ describe('Server.prototype.addProtoService', function() { }); }); }); +describe('Client constructor building', function() { + var illegal_service_attrs = { + $method : { + path: '/illegal/$method', + requestStream: false, + responseStream: false, + requestSerialize: _.identity, + requestDeserialize: _.identity, + responseSerialize: _.identity, + responseDeserialize: _.identity + } + }; + it('Should reject method names starting with $', function() { + assert.throws(function() { + grpc.makeGenericClientConstructor(illegal_service_attrs); + }, /\$/); + }); +}); describe('Echo service', function() { var server; var client; @@ -344,7 +362,7 @@ describe('Other conditions', function() { server.shutdown(); }); it('channel.getTarget should be available', function() { - assert.strictEqual(typeof client.channel.getTarget(), 'string'); + assert.strictEqual(typeof client.$channel.getTarget(), 'string'); }); describe('Server recieving bad input', function() { var misbehavingClient; From c451627d3620713661d97c39311be4888a3ff865 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jul 2015 10:11:07 -0700 Subject: [PATCH 20/41] Revert "Ensure that client generated methods don't conflict with other properties" This reverts commit 6f34bad568e6db5f78a908fa63bd08a1304366fa. --- src/client.js | 27 ++++++++++++--------------- test/surface_test.js | 20 +------------------- 2 files changed, 13 insertions(+), 34 deletions(-) diff --git a/src/client.js b/src/client.js index 405e2be6..f843669b 100644 --- a/src/client.js +++ b/src/client.js @@ -236,7 +236,7 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { deadline = Infinity; } var emitter = new EventEmitter(); - var call = new grpc.Call(this.$channel, method, deadline); + var call = new grpc.Call(this.channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } @@ -246,7 +246,7 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { emitter.getPeer = function getPeer() { return call.getPeer(); }; - this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { + this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); callback(error); @@ -309,12 +309,12 @@ function makeClientStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.$channel, method, deadline); + var call = new grpc.Call(this.channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientWritableStream(call, serialize); - this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { + this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); callback(error); @@ -383,12 +383,12 @@ function makeServerStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.$channel, method, deadline); + var call = new grpc.Call(this.channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientReadableStream(call, deserialize); - this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { + this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); stream.emit('error', error); @@ -455,12 +455,12 @@ function makeBidiStreamRequestFunction(method, serialize, deserialize) { if (deadline === undefined) { deadline = Infinity; } - var call = new grpc.Call(this.$channel, method, deadline); + var call = new grpc.Call(this.channel, method, deadline); if (metadata === null || metadata === undefined) { metadata = {}; } var stream = new ClientDuplexStream(call, serialize, deserialize); - this.$updateMetadata(this.$auth_uri, metadata, function(error, metadata) { + this.updateMetadata(this.auth_uri, metadata, function(error, metadata) { if (error) { call.cancel(); stream.emit('error', error); @@ -545,17 +545,14 @@ exports.makeClientConstructor = function(methods, serviceName) { options = {}; } options['grpc.primary_user_agent'] = 'grpc-node/' + version; - this.$channel = new grpc.Channel(address, options); - this.$server_address = address.replace(/\/$/, ''); - this.$auth_uri = this.$server_address + '/' + serviceName; - this.$updateMetadata = updateMetadata; + this.channel = new grpc.Channel(address, options); + this.server_address = address.replace(/\/$/, ''); + this.auth_uri = this.server_address + '/' + serviceName; + this.updateMetadata = updateMetadata; } _.each(methods, function(attrs, name) { var method_type; - if (_.startsWith(name, '$')) { - throw new Error('Method names cannot start with $'); - } if (attrs.requestStream) { if (attrs.responseStream) { method_type = 'bidi'; diff --git a/test/surface_test.js b/test/surface_test.js index 1afdb330..98f9b15b 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -110,24 +110,6 @@ describe('Server.prototype.addProtoService', function() { }); }); }); -describe('Client constructor building', function() { - var illegal_service_attrs = { - $method : { - path: '/illegal/$method', - requestStream: false, - responseStream: false, - requestSerialize: _.identity, - requestDeserialize: _.identity, - responseSerialize: _.identity, - responseDeserialize: _.identity - } - }; - it('Should reject method names starting with $', function() { - assert.throws(function() { - grpc.makeGenericClientConstructor(illegal_service_attrs); - }, /\$/); - }); -}); describe('Echo service', function() { var server; var client; @@ -362,7 +344,7 @@ describe('Other conditions', function() { server.shutdown(); }); it('channel.getTarget should be available', function() { - assert.strictEqual(typeof client.$channel.getTarget(), 'string'); + assert.strictEqual(typeof client.channel.getTarget(), 'string'); }); describe('Server recieving bad input', function() { var misbehavingClient; From 46d1b4143c1cc41c1c46ff3d46724e7c1a6912a9 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jul 2015 16:33:52 -0700 Subject: [PATCH 21/41] Eliminated some redundant checks in the Node interop client --- interop/interop_client.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/interop/interop_client.js b/interop/interop_client.js index e810e68e..d2f51296 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -69,9 +69,6 @@ function zeroBuffer(size) { function emptyUnary(client, done) { var call = client.emptyCall({}, function(err, resp) { assert.ifError(err); - }); - call.on('status', function(status) { - assert.strictEqual(status.code, grpc.status.OK); if (done) { done(); } @@ -96,9 +93,6 @@ function largeUnary(client, done) { assert.ifError(err); assert.strictEqual(resp.payload.type, 'COMPRESSABLE'); assert.strictEqual(resp.payload.body.length, 314159); - }); - call.on('status', function(status) { - assert.strictEqual(status.code, grpc.status.OK); if (done) { done(); } @@ -115,9 +109,6 @@ function clientStreaming(client, done) { var call = client.streamingInputCall(function(err, resp) { assert.ifError(err); assert.strictEqual(resp.aggregated_payload_size, 74922); - }); - call.on('status', function(status) { - assert.strictEqual(status.code, grpc.status.OK); if (done) { done(); } @@ -308,9 +299,6 @@ function authTest(expected_user, scope, client, done) { assert.strictEqual(resp.payload.body.length, 314159); assert.strictEqual(resp.username, expected_user); assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); - }); - call.on('status', function(status) { - assert.strictEqual(status.code, grpc.status.OK); if (done) { done(); } @@ -344,9 +332,6 @@ function oauth2Test(expected_user, scope, per_rpc, client, done) { assert.ifError(err); assert.strictEqual(resp.username, expected_user); assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); - }); - call.on('status', function(status) { - assert.strictEqual(status.code, grpc.status.OK); if (done) { done(); } @@ -358,7 +343,6 @@ function oauth2Test(expected_user, scope, per_rpc, client, done) { client.updateMetadata = updateMetadata; makeTestCall(null, {}); } - }); }); } From eb6b0ede34c523212d2198c0ec6d205f7fb54c54 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 31 Jul 2015 17:01:47 -0700 Subject: [PATCH 22/41] Update wrappers, tests to new create_call() --- ext/call.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ext/call.cc b/ext/call.cc index dc45c8d8..e4451c36 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -511,8 +511,9 @@ NAN_METHOD(Call::New) { double deadline = args[2]->NumberValue(); grpc_channel *wrapped_channel = channel->GetWrappedChannel(); grpc_call *wrapped_call = grpc_channel_create_call( - wrapped_channel, CompletionQueueAsyncWorker::GetQueue(), *method, - channel->GetHost(), MillisecondsToTimespec(deadline)); + wrapped_channel, NULL, GRPC_INHERIT_DEFAULTS, + CompletionQueueAsyncWorker::GetQueue(), *method, channel->GetHost(), + MillisecondsToTimespec(deadline)); call = new Call(wrapped_call); args.This()->SetHiddenValue(NanNew("channel_"), channel_object); } From 70c8bfd09e3adc67043c00f99ada83a6ac9b9a0a Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 3 Aug 2015 08:06:50 -0700 Subject: [PATCH 23/41] Implement cancellation propagation, define auth propagation --- ext/call.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/call.cc b/ext/call.cc index e4451c36..fe585a0d 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -511,7 +511,7 @@ NAN_METHOD(Call::New) { double deadline = args[2]->NumberValue(); grpc_channel *wrapped_channel = channel->GetWrappedChannel(); grpc_call *wrapped_call = grpc_channel_create_call( - wrapped_channel, NULL, GRPC_INHERIT_DEFAULTS, + wrapped_channel, NULL, GRPC_PROPAGATE_DEFAULTS, CompletionQueueAsyncWorker::GetQueue(), *method, channel->GetHost(), MillisecondsToTimespec(deadline)); call = new Call(wrapped_call); From 1451651e8c3716ea2c50ea46499f5400cd378b40 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 3 Aug 2015 10:42:22 -0700 Subject: [PATCH 24/41] Rename grpc_server_add_http2_port to grpc_server_add_insecure_http2_port --- ext/server.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/server.cc b/ext/server.cc index 04fabc87..1dc179db 100644 --- a/ext/server.cc +++ b/ext/server.cc @@ -265,8 +265,8 @@ NAN_METHOD(Server::AddHttp2Port) { grpc_server_credentials *creds = creds_object->GetWrappedServerCredentials(); int port; if (creds == NULL) { - port = grpc_server_add_http2_port(server->wrapped_server, - *NanUtf8String(args[0])); + port = grpc_server_add_insecure_http2_port(server->wrapped_server, + *NanUtf8String(args[0])); } else { port = grpc_server_add_secure_http2_port(server->wrapped_server, *NanUtf8String(args[0]), From 8551cd9559b48f120a6144aa3b932956f86abf92 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 3 Aug 2015 10:56:00 -0700 Subject: [PATCH 25/41] Disabled deprecation warnings in Node build --- binding.gyp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/binding.gyp b/binding.gyp index 6ba23338..734dc841 100644 --- a/binding.gyp +++ b/binding.gyp @@ -11,7 +11,8 @@ '-pedantic', '-g', '-zdefs', - '-Werror' + '-Werror', + '-Wno-error=deprecated-declarations' ], 'ldflags': [ '-g' From ad3334967aa9ca57e271b117c2c7dcf73ac34ba7 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 3 Aug 2015 15:17:53 -0700 Subject: [PATCH 26/41] Exposed host parameter in Call constructor, don't save it in Channel object --- ext/call.cc | 14 ++++++++++++-- ext/channel.cc | 25 ++++++------------------- ext/channel.h | 6 +----- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/ext/call.cc b/ext/call.cc index dc45c8d8..ab177e85 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -510,9 +510,19 @@ NAN_METHOD(Call::New) { NanUtf8String method(args[1]); double deadline = args[2]->NumberValue(); grpc_channel *wrapped_channel = channel->GetWrappedChannel(); - grpc_call *wrapped_call = grpc_channel_create_call( + grpc_call *wrapped_call; + if (args[3]->IsString()) { + NanUtf8String host_override(args[3]); + wrapped_call = grpc_channel_create_call( wrapped_channel, CompletionQueueAsyncWorker::GetQueue(), *method, - channel->GetHost(), MillisecondsToTimespec(deadline)); + *host_override, MillisecondsToTimespec(deadline)); + } else if (args[3]->IsUndefined() || args[3]->IsNull()) { + wrapped_call = grpc_channel_create_call( + wrapped_channel, CompletionQueueAsyncWorker::GetQueue(), *method, + NULL, MillisecondsToTimespec(deadline)); + } else { + return NanThrowTypeError("Call's fourth argument must be a string"); + } call = new Call(wrapped_call); args.This()->SetHiddenValue(NanNew("channel_"), channel_object); } diff --git a/ext/channel.cc b/ext/channel.cc index d02ed956..457a58c0 100644 --- a/ext/channel.cc +++ b/ext/channel.cc @@ -59,14 +59,12 @@ using v8::Value; NanCallback *Channel::constructor; Persistent Channel::fun_tpl; -Channel::Channel(grpc_channel *channel, NanUtf8String *host) - : wrapped_channel(channel), host(host) {} +Channel::Channel(grpc_channel *channel) : wrapped_channel(channel) {} Channel::~Channel() { if (wrapped_channel != NULL) { grpc_channel_destroy(wrapped_channel); } - delete host; } void Channel::Init(Handle exports) { @@ -91,8 +89,6 @@ bool Channel::HasInstance(Handle val) { grpc_channel *Channel::GetWrappedChannel() { return this->wrapped_channel; } -char *Channel::GetHost() { return **this->host; } - NAN_METHOD(Channel::New) { NanScope(); @@ -103,8 +99,7 @@ NAN_METHOD(Channel::New) { } grpc_channel *wrapped_channel; // Owned by the Channel object - NanUtf8String *host = new NanUtf8String(args[0]); - NanUtf8String *host_override = NULL; + NanUtf8String host(args[0]); grpc_credentials *creds; if (!Credentials::HasInstance(args[1])) { return NanThrowTypeError( @@ -116,12 +111,9 @@ NAN_METHOD(Channel::New) { grpc_channel_args *channel_args_ptr; if (args[2]->IsUndefined()) { channel_args_ptr = NULL; - wrapped_channel = grpc_insecure_channel_create(**host, NULL); + wrapped_channel = grpc_insecure_channel_create(*host, NULL); } else if (args[2]->IsObject()) { Handle args_hash(args[2]->ToObject()->Clone()); - if (args_hash->HasOwnProperty(NanNew(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG))) { - host_override = new NanUtf8String(args_hash->Get(NanNew(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG))); - } Handle keys(args_hash->GetOwnPropertyNames()); grpc_channel_args channel_args; channel_args.num_args = keys->Length(); @@ -153,20 +145,15 @@ NAN_METHOD(Channel::New) { return NanThrowTypeError("Channel expects a string and an object"); } if (creds == NULL) { - wrapped_channel = grpc_insecure_channel_create(**host, channel_args_ptr); + wrapped_channel = grpc_insecure_channel_create(*host, channel_args_ptr); } else { wrapped_channel = - grpc_secure_channel_create(creds, **host, channel_args_ptr); + grpc_secure_channel_create(creds, *host, channel_args_ptr); } if (channel_args_ptr != NULL) { free(channel_args_ptr->args); } - Channel *channel; - if (host_override == NULL) { - channel = new Channel(wrapped_channel, host); - } else { - channel = new Channel(wrapped_channel, host_override); - } + Channel *channel = new Channel(wrapped_channel); channel->Wrap(args.This()); NanReturnValue(args.This()); } else { diff --git a/ext/channel.h b/ext/channel.h index 6725ebb0..e2182cb4 100644 --- a/ext/channel.h +++ b/ext/channel.h @@ -53,11 +53,8 @@ class Channel : public ::node::ObjectWrap { /* Returns the grpc_channel struct that this object wraps */ grpc_channel *GetWrappedChannel(); - /* Return the hostname that this channel connects to */ - char *GetHost(); - private: - explicit Channel(grpc_channel *channel, NanUtf8String *host); + explicit Channel(grpc_channel *channel); ~Channel(); // Prevent copying @@ -71,7 +68,6 @@ class Channel : public ::node::ObjectWrap { static v8::Persistent fun_tpl; grpc_channel *wrapped_channel; - NanUtf8String *host; }; } // namespace node From dff79ef1d848b630e2f7fd6e86164cbf14cf7595 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 3 Aug 2015 15:18:23 -0700 Subject: [PATCH 27/41] Added host override option for RPCs. Added optional params object --- examples/perf_test.js | 2 +- interop/interop_client.js | 3 +- src/client.js | 59 ++++++++++++++++++++------------------- test/call_test.js | 5 ++++ test/end_to_end_test.js | 8 ------ 5 files changed, 39 insertions(+), 38 deletions(-) diff --git a/examples/perf_test.js b/examples/perf_test.js index 0f38725f..214b9384 100644 --- a/examples/perf_test.js +++ b/examples/perf_test.js @@ -63,7 +63,7 @@ function runTest(iterations, callback) { var timeDiff = process.hrtime(startTime); intervals[i] = timeDiff[0] * 1000000 + timeDiff[1] / 1000; next(i+1); - }, {}, deadline); + }, {}, {deadline: deadline}); } } next(0); diff --git a/interop/interop_client.js b/interop/interop_client.js index 236b3661..68f47477 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -268,7 +268,7 @@ function cancelAfterFirstResponse(client, done) { function timeoutOnSleepingServer(client, done) { var deadline = new Date(); deadline.setMilliseconds(deadline.getMilliseconds() + 1); - var call = client.fullDuplexCall(null, deadline); + var call = client.fullDuplexCall(null, {deadline: deadline}); call.write({ payload: {body: zeroBuffer(27182)} }); @@ -409,6 +409,7 @@ function runTest(address, host_override, test_case, tls, test_ca, done) { creds = grpc.Credentials.createSsl(ca_data); if (host_override) { options['grpc.ssl_target_name_override'] = host_override; + options['grpc.default_authority'] = host_override; } } else { creds = grpc.Credentials.createInsecure(); diff --git a/src/client.js b/src/client.js index b2b44237..87c7690d 100644 --- a/src/client.js +++ b/src/client.js @@ -207,6 +207,25 @@ ClientReadableStream.prototype.getPeer = getPeer; ClientWritableStream.prototype.getPeer = getPeer; ClientDuplexStream.prototype.getPeer = getPeer; +/** + * Get a call object built with the provided options. Keys for options are + * 'deadline', which takes a date or number, and 'host', which takes a string + * and overrides the hostname to connect to. + * @param {Object} options Options map. + */ +function getCall(channel, method, options) { + var deadline; + var host; + if (options) { + deadline = options.deadline; + host = options.host; + } + if (deadline === undefined) { + deadline = Infinity; + } + return new grpc.Call(channel, method, deadline, host); +} + /** * Get a function that can make unary requests to the specified method. * @param {string} method The name of the method to request @@ -226,17 +245,13 @@ function makeUnaryRequestFunction(method, serialize, deserialize) { * response is received * @param {array=} metadata Array of metadata key/value pairs to add to the * call - * @param {(number|Date)=} deadline The deadline for processing this request. - * Defaults to infinite future + * @param {Object=} options Options map * @return {EventEmitter} An event emitter for stream related events */ - function makeUnaryRequest(argument, callback, metadata, deadline) { + function makeUnaryRequest(argument, callback, metadata, options) { /* jshint validthis: true */ - if (deadline === undefined) { - deadline = Infinity; - } var emitter = new EventEmitter(); - var call = new grpc.Call(this.channel, method, deadline); + var call = getCall(this.channel, method, options); if (metadata === null || metadata === undefined) { metadata = {}; } @@ -300,16 +315,12 @@ function makeClientStreamRequestFunction(method, serialize, deserialize) { * response is received * @param {array=} metadata Array of metadata key/value pairs to add to the * call - * @param {(number|Date)=} deadline The deadline for processing this request. - * Defaults to infinite future + * @param {Object=} options Options map * @return {EventEmitter} An event emitter for stream related events */ - function makeClientStreamRequest(callback, metadata, deadline) { + function makeClientStreamRequest(callback, metadata, options) { /* jshint validthis: true */ - if (deadline === undefined) { - deadline = Infinity; - } - var call = new grpc.Call(this.channel, method, deadline); + var call = getCall(this.channel, method, options); if (metadata === null || metadata === undefined) { metadata = {}; } @@ -374,16 +385,12 @@ function makeServerStreamRequestFunction(method, serialize, deserialize) { * serialize * @param {array=} metadata Array of metadata key/value pairs to add to the * call - * @param {(number|Date)=} deadline The deadline for processing this request. - * Defaults to infinite future + * @param {Object} options Options map * @return {EventEmitter} An event emitter for stream related events */ - function makeServerStreamRequest(argument, metadata, deadline) { + function makeServerStreamRequest(argument, metadata, options) { /* jshint validthis: true */ - if (deadline === undefined) { - deadline = Infinity; - } - var call = new grpc.Call(this.channel, method, deadline); + var call = getCall(this.channel, method, options); if (metadata === null || metadata === undefined) { metadata = {}; } @@ -446,16 +453,12 @@ function makeBidiStreamRequestFunction(method, serialize, deserialize) { * @this {SurfaceClient} Client object. Must have a channel member. * @param {array=} metadata Array of metadata key/value pairs to add to the * call - * @param {(number|Date)=} deadline The deadline for processing this request. - * Defaults to infinite future + * @param {Options} options Options map * @return {EventEmitter} An event emitter for stream related events */ - function makeBidiStreamRequest(metadata, deadline) { + function makeBidiStreamRequest(metadata, options) { /* jshint validthis: true */ - if (deadline === undefined) { - deadline = Infinity; - } - var call = new grpc.Call(this.channel, method, deadline); + var call = getCall(this.channel, method, options); if (metadata === null || metadata === undefined) { metadata = {}; } diff --git a/test/call_test.js b/test/call_test.js index 48d859a8..8d0f20b0 100644 --- a/test/call_test.js +++ b/test/call_test.js @@ -84,6 +84,11 @@ describe('call', function() { new grpc.Call(channel, 'method', 0); }); }); + it('should accept an optional fourth string parameter', function() { + assert.doesNotThrow(function() { + new grpc.Call(channel, 'method', new Date(), 'host_override'); + }); + }); it('should fail with a closed channel', function() { var local_channel = new grpc.Channel('hostname', insecureCreds); local_channel.close(); diff --git a/test/end_to_end_test.js b/test/end_to_end_test.js index ea41dfc2..7574d98b 100644 --- a/test/end_to_end_test.js +++ b/test/end_to_end_test.js @@ -74,8 +74,6 @@ describe('end-to-end', function() { }); it('should start and end a request without error', function(complete) { var done = multiDone(complete, 2); - var deadline = new Date(); - deadline.setSeconds(deadline.getSeconds() + 3); var status_text = 'xyz'; var call = new grpc.Call(channel, 'dummy_method', @@ -126,8 +124,6 @@ describe('end-to-end', function() { }); it('should successfully send and receive metadata', function(complete) { var done = multiDone(complete, 2); - var deadline = new Date(); - deadline.setSeconds(deadline.getSeconds() + 3); var status_text = 'xyz'; var call = new grpc.Call(channel, 'dummy_method', @@ -184,8 +180,6 @@ describe('end-to-end', function() { var req_text = 'client_request'; var reply_text = 'server_response'; var done = multiDone(complete, 2); - var deadline = new Date(); - deadline.setSeconds(deadline.getSeconds() + 3); var status_text = 'success'; var call = new grpc.Call(channel, 'dummy_method', @@ -241,8 +235,6 @@ describe('end-to-end', function() { it('should send multiple messages', function(complete) { var done = multiDone(complete, 2); var requests = ['req1', 'req2']; - var deadline = new Date(); - deadline.setSeconds(deadline.getSeconds() + 3); var status_text = 'xyz'; var call = new grpc.Call(channel, 'dummy_method', From 2a4657d30a0fb703d31d942ababebe7ed2de1d70 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Fri, 7 Aug 2015 01:40:49 +0200 Subject: [PATCH 28/41] Working on node. --- ext/call.cc | 6 +++--- ext/channel.cc | 4 ++-- ext/completion_queue_async_worker.cc | 4 ++-- ext/server.cc | 14 +++++++------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/ext/call.cc b/ext/call.cc index 15c9b2d9..4dd70813 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -510,7 +510,7 @@ NAN_METHOD(Call::New) { grpc_channel *wrapped_channel = channel->GetWrappedChannel(); grpc_call *wrapped_call = grpc_channel_create_call( wrapped_channel, CompletionQueueAsyncWorker::GetQueue(), *method, - channel->GetHost(), MillisecondsToTimespec(deadline)); + channel->GetHost(), MillisecondsToTimespec(deadline), NULL); call = new Call(wrapped_call); args.This()->SetHiddenValue(NanNew("channel_"), channel_object); } @@ -587,7 +587,7 @@ NAN_METHOD(Call::StartBatch) { NanCallback *callback = new NanCallback(callback_func); grpc_call_error error = grpc_call_start_batch( call->wrapped_call, &ops[0], nops, new struct tag( - callback, op_vector.release(), resources)); + callback, op_vector.release(), resources), NULL); if (error != GRPC_CALL_OK) { return NanThrowError("startBatch failed", error); } @@ -601,7 +601,7 @@ NAN_METHOD(Call::Cancel) { return NanThrowTypeError("cancel can only be called on Call objects"); } Call *call = ObjectWrap::Unwrap(args.This()); - grpc_call_error error = grpc_call_cancel(call->wrapped_call); + grpc_call_error error = grpc_call_cancel(call->wrapped_call, NULL); if (error != GRPC_CALL_OK) { return NanThrowError("cancel failed", error); } diff --git a/ext/channel.cc b/ext/channel.cc index d37bf763..15f121e8 100644 --- a/ext/channel.cc +++ b/ext/channel.cc @@ -103,7 +103,7 @@ NAN_METHOD(Channel::New) { NanUtf8String *host = new NanUtf8String(args[0]); NanUtf8String *host_override = NULL; if (args[1]->IsUndefined()) { - wrapped_channel = grpc_channel_create(**host, NULL); + wrapped_channel = grpc_channel_create(**host, NULL, NULL); } else if (args[1]->IsObject()) { grpc_credentials *creds = NULL; Handle args_hash(args[1]->ToObject()->Clone()); @@ -148,7 +148,7 @@ NAN_METHOD(Channel::New) { } } if (creds == NULL) { - wrapped_channel = grpc_channel_create(**host, &channel_args); + wrapped_channel = grpc_channel_create(**host, &channel_args, NULL); } else { wrapped_channel = grpc_secure_channel_create(creds, **host, &channel_args); diff --git a/ext/completion_queue_async_worker.cc b/ext/completion_queue_async_worker.cc index 1215c97e..4501e848 100644 --- a/ext/completion_queue_async_worker.cc +++ b/ext/completion_queue_async_worker.cc @@ -63,7 +63,7 @@ CompletionQueueAsyncWorker::~CompletionQueueAsyncWorker() {} void CompletionQueueAsyncWorker::Execute() { result = - grpc_completion_queue_next(queue, gpr_inf_future(GPR_CLOCK_REALTIME)); + grpc_completion_queue_next(queue, gpr_inf_future(GPR_CLOCK_REALTIME), NULL); if (!result.success) { SetErrorMessage("The batch encountered an error"); } @@ -85,7 +85,7 @@ void CompletionQueueAsyncWorker::Init(Handle exports) { NanScope(); current_threads = 0; waiting_next_calls = 0; - queue = grpc_completion_queue_create(); + queue = grpc_completion_queue_create(NULL); } void CompletionQueueAsyncWorker::HandleOKCallback() { diff --git a/ext/server.cc b/ext/server.cc index 34cde9ff..a123da85 100644 --- a/ext/server.cc +++ b/ext/server.cc @@ -113,8 +113,8 @@ class NewCallOp : public Op { }; Server::Server(grpc_server *server) : wrapped_server(server) { - shutdown_queue = grpc_completion_queue_create(); - grpc_server_register_completion_queue(server, shutdown_queue); + shutdown_queue = grpc_completion_queue_create(NULL); + grpc_server_register_completion_queue(server, shutdown_queue, NULL); } Server::~Server() { @@ -162,7 +162,7 @@ void Server::ShutdownServer() { this->shutdown_queue, NULL); grpc_completion_queue_pluck(this->shutdown_queue, NULL, - gpr_inf_future(GPR_CLOCK_REALTIME)); + gpr_inf_future(GPR_CLOCK_REALTIME), NULL); this->wrapped_server = NULL; } } @@ -180,7 +180,7 @@ NAN_METHOD(Server::New) { grpc_server *wrapped_server; grpc_completion_queue *queue = CompletionQueueAsyncWorker::GetQueue(); if (args[0]->IsUndefined()) { - wrapped_server = grpc_server_create(NULL); + wrapped_server = grpc_server_create(NULL, NULL); } else if (args[0]->IsObject()) { Handle args_hash(args[0]->ToObject()); Handle keys(args_hash->GetOwnPropertyNames()); @@ -209,12 +209,12 @@ NAN_METHOD(Server::New) { return NanThrowTypeError("Arg values must be strings"); } } - wrapped_server = grpc_server_create(&channel_args); + wrapped_server = grpc_server_create(&channel_args, NULL); free(channel_args.args); } else { return NanThrowTypeError("Server expects an object"); } - grpc_server_register_completion_queue(wrapped_server, queue); + grpc_server_register_completion_queue(wrapped_server, queue, NULL); Server *server = new Server(wrapped_server); server->Wrap(args.This()); NanReturnValue(args.This()); @@ -237,7 +237,7 @@ NAN_METHOD(Server::RequestCall) { CompletionQueueAsyncWorker::GetQueue(), CompletionQueueAsyncWorker::GetQueue(), new struct tag(new NanCallback(args[0].As()), ops.release(), - shared_ptr(nullptr))); + shared_ptr(NULL))); if (error != GRPC_CALL_OK) { return NanThrowError("requestCall failed", error); } From 5cbc7dd6c9835c1ae5b828b09e75e92b7618d86b Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Wed, 12 Aug 2015 01:10:54 +0200 Subject: [PATCH 29/41] Reverted unintended change. --- ext/server.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/server.cc b/ext/server.cc index 53342c00..8e396448 100644 --- a/ext/server.cc +++ b/ext/server.cc @@ -233,7 +233,7 @@ NAN_METHOD(Server::RequestCall) { CompletionQueueAsyncWorker::GetQueue(), CompletionQueueAsyncWorker::GetQueue(), new struct tag(new NanCallback(args[0].As()), ops.release(), - shared_ptr(NULL))); + shared_ptr(nullptr))); if (error != GRPC_CALL_OK) { return NanThrowError("requestCall failed", error); } From 33591d5e19f654dfd14b64106617266251ae7fa0 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 11 Aug 2015 17:30:05 -0700 Subject: [PATCH 30/41] Fixed lint errors --- interop/interop_client.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/interop/interop_client.js b/interop/interop_client.js index 55d243ca..27f6c19c 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -67,7 +67,7 @@ function zeroBuffer(size) { * primarily for use with mocha */ function emptyUnary(client, done) { - var call = client.emptyCall({}, function(err, resp) { + client.emptyCall({}, function(err, resp) { assert.ifError(err); if (done) { done(); @@ -89,7 +89,7 @@ function largeUnary(client, done) { body: zeroBuffer(271828) } }; - var call = client.unaryCall(arg, function(err, resp) { + client.unaryCall(arg, function(err, resp) { assert.ifError(err); assert.strictEqual(resp.payload.type, 'COMPRESSABLE'); assert.strictEqual(resp.payload.body.length, 314159); @@ -293,7 +293,7 @@ function authTest(expected_user, scope, client, done) { fill_username: true, fill_oauth_scope: true }; - var call = client.unaryCall(arg, function(err, resp) { + client.unaryCall(arg, function(err, resp) { assert.ifError(err); assert.strictEqual(resp.payload.type, 'COMPRESSABLE'); assert.strictEqual(resp.payload.body.length, 314159); @@ -328,7 +328,7 @@ function oauth2Test(expected_user, scope, per_rpc, client, done) { }; var makeTestCall = function(error, client_metadata) { assert.ifError(error); - var call = client.unaryCall(arg, function(err, resp) { + client.unaryCall(arg, function(err, resp) { assert.ifError(err); assert.strictEqual(resp.username, expected_user); assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); From 1e65e15085512221153ab2d2935f43988d8b062b Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 12 Aug 2015 10:48:39 -0700 Subject: [PATCH 31/41] Fixed failing cloud-to-prod auth interop tests --- interop/interop_client.js | 6 ++++-- src/client.js | 11 +++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/interop/interop_client.js b/interop/interop_client.js index 221d69e2..6152d445 100644 --- a/interop/interop_client.js +++ b/interop/interop_client.js @@ -298,7 +298,9 @@ function authTest(expected_user, scope, client, done) { assert.strictEqual(resp.payload.type, 'COMPRESSABLE'); assert.strictEqual(resp.payload.body.length, 314159); assert.strictEqual(resp.username, expected_user); - assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); + if (scope) { + assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); + } if (done) { done(); } @@ -335,7 +337,7 @@ function oauth2Test(expected_user, scope, per_rpc, client, done) { if (done) { done(); } - }); + }, client_metadata); }; if (per_rpc) { updateMetadata('', {}, makeTestCall); diff --git a/src/client.js b/src/client.js index b2b44237..a253c860 100644 --- a/src/client.js +++ b/src/client.js @@ -523,7 +523,7 @@ var requester_makers = { * requestSerialize: function to serialize request objects * responseDeserialize: function to deserialize response objects * @param {Object} methods An object mapping method names to method attributes - * @param {string} serviceName The name of the service + * @param {string} serviceName The fully qualified name of the service * @return {function(string, Object)} New client constructor */ exports.makeClientConstructor = function(methods, serviceName) { @@ -548,8 +548,10 @@ exports.makeClientConstructor = function(methods, serviceName) { } options['grpc.primary_user_agent'] = 'grpc-node/' + version; this.channel = new grpc.Channel(address, credentials, options); - this.server_address = address.replace(/\/$/, ''); - this.auth_uri = this.server_address + '/' + serviceName; + // Extract the DNS name from the address string + address = address.replace(/(\w+:\/\/)?([^:]+)(:\d+)?\/?$/, '$2'); + this.server_address = address; + this.auth_uri = 'https://' + this.server_address + '/' + serviceName; this.updateMetadata = updateMetadata; } @@ -587,7 +589,8 @@ exports.makeClientConstructor = function(methods, serviceName) { */ exports.makeProtobufClientConstructor = function(service) { var method_attrs = common.getProtobufServiceAttrs(service, service.name); - var Client = exports.makeClientConstructor(method_attrs); + var Client = exports.makeClientConstructor( + method_attrs, common.fullyQualifiedName(service)); Client.service = service; return Client; }; From 5cb7853f37631d2e96249662538d7ea569f417f5 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Wed, 12 Aug 2015 20:07:54 +0200 Subject: [PATCH 32/41] Fixing merge failures. --- ext/channel.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/channel.cc b/ext/channel.cc index 3ff310f1..45d0d09e 100644 --- a/ext/channel.cc +++ b/ext/channel.cc @@ -145,7 +145,8 @@ NAN_METHOD(Channel::New) { return NanThrowTypeError("Channel expects a string and an object"); } if (creds == NULL) { - wrapped_channel = grpc_insecure_channel_create(*host, channel_args_ptr); + wrapped_channel = grpc_insecure_channel_create(*host, channel_args_ptr, + NULL); } else { wrapped_channel = grpc_secure_channel_create(creds, *host, channel_args_ptr); From 7970af5e1b681aa6a983e3b1753781a447c7a8c9 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 12 Aug 2015 11:32:54 -0700 Subject: [PATCH 33/41] Fix scheme matching in auth URI --- src/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.js b/src/client.js index a253c860..4e631456 100644 --- a/src/client.js +++ b/src/client.js @@ -549,7 +549,7 @@ exports.makeClientConstructor = function(methods, serviceName) { options['grpc.primary_user_agent'] = 'grpc-node/' + version; this.channel = new grpc.Channel(address, credentials, options); // Extract the DNS name from the address string - address = address.replace(/(\w+:\/\/)?([^:]+)(:\d+)?\/?$/, '$2'); + address = address.replace(/(\w+:\/\/+)?([^:]+)(:\d+)?\/?$/, '$2'); this.server_address = address; this.auth_uri = 'https://' + this.server_address + '/' + serviceName; this.updateMetadata = updateMetadata; From 02b7005b49380bacaba21dc112586c17e4303cf7 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 12 Aug 2015 11:54:41 -0700 Subject: [PATCH 34/41] Clarified address regex --- src/client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client.js b/src/client.js index 4e631456..bbb45382 100644 --- a/src/client.js +++ b/src/client.js @@ -548,8 +548,8 @@ exports.makeClientConstructor = function(methods, serviceName) { } options['grpc.primary_user_agent'] = 'grpc-node/' + version; this.channel = new grpc.Channel(address, credentials, options); - // Extract the DNS name from the address string - address = address.replace(/(\w+:\/\/+)?([^:]+)(:\d+)?\/?$/, '$2'); + // Remove the optional DNS scheme, trailing port, and trailing backslash + address = address.replace(/^(dns:\/{3})?([^:\/]+)(:\d+)?\/?$/, '$2'); this.server_address = address; this.auth_uri = 'https://' + this.server_address + '/' + serviceName; this.updateMetadata = updateMetadata; From d71afbc48bf9616d414cfcd250956e3f0810979b Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 13 Aug 2015 11:00:13 -0700 Subject: [PATCH 35/41] Fixed typo --- ext/completion_queue_async_worker.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/completion_queue_async_worker.cc b/ext/completion_queue_async_worker.cc index c45e303f..b3e8b960 100644 --- a/ext/completion_queue_async_worker.cc +++ b/ext/completion_queue_async_worker.cc @@ -65,7 +65,7 @@ void CompletionQueueAsyncWorker::Execute() { result = grpc_completion_queue_next(queue, gpr_inf_future(GPR_CLOCK_REALTIME)); if (!result.success) { - SetErrorMessage("The asnyc function encountered an error"); + SetErrorMessage("The async function encountered an error"); } } From 46a84765018ee8019b364a897fc3da907cf74c45 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 13 Aug 2015 11:24:34 -0700 Subject: [PATCH 36/41] Modified watchState functions to match C API --- ext/channel.cc | 25 ++----------------------- src/client.js | 22 +++++----------------- test/surface_test.js | 13 ++++++------- 3 files changed, 13 insertions(+), 47 deletions(-) diff --git a/ext/channel.cc b/ext/channel.cc index aa1ca031..907178f1 100644 --- a/ext/channel.cc +++ b/ext/channel.cc @@ -62,25 +62,6 @@ using v8::Persistent; using v8::String; using v8::Value; -class ConnectivityStateOp : public Op { - public: - Handle GetNodeValue() const { - return NanNew(new_state); - } - - bool ParseOp(Handle value, grpc_op *out, - shared_ptr resources) { - return true; - } - - grpc_connectivity_state new_state; - - protected: - std::string GetTypeString() const { - return "new_state"; - } -}; - NanCallback *Channel::constructor; Persistent Channel::fun_tpl; @@ -252,12 +233,10 @@ NAN_METHOD(Channel::WatchConnectivityState) { Handle callback_func = args[2].As(); NanCallback *callback = new NanCallback(callback_func); Channel *channel = ObjectWrap::Unwrap(args.This()); - ConnectivityStateOp *op = new ConnectivityStateOp(); unique_ptr ops(new OpVec()); - ops->push_back(unique_ptr(op)); grpc_channel_watch_connectivity_state( - channel->wrapped_channel, last_state, &op->new_state, - MillisecondsToTimespec(deadline), CompletionQueueAsyncWorker::GetQueue(), + channel->wrapped_channel, last_state, MillisecondsToTimespec(deadline), + CompletionQueueAsyncWorker::GetQueue(), new struct tag(callback, ops.release(), shared_ptr(nullptr))); diff --git a/src/client.js b/src/client.js index f47e030f..0cd29e5b 100644 --- a/src/client.js +++ b/src/client.js @@ -562,9 +562,8 @@ exports.makeClientConstructor = function(methods, serviceName) { * Wait for the client to be ready. The callback will be called when the * client has successfully connected to the server, and it will be called * with an error if the attempt to connect to the server has unrecoverablly - * failed or if the deadline expires. This function does not automatically - * attempt to initiate the connection, so the callback will not be called - * unless you also start a method call or call $tryConnect. + * failed or if the deadline expires. This function will make the channel + * start connecting if it has not already done so. * @param {(Date|Number)} deadline When to stop waiting for a connection. Pass * Infinity to wait forever. * @param {function(Error)} callback The callback to call when done attempting @@ -572,12 +571,11 @@ exports.makeClientConstructor = function(methods, serviceName) { */ Client.prototype.$waitForReady = function(deadline, callback) { var self = this; - var checkState = function(err, result) { + var checkState = function(err) { if (err) { callback(new Error('Failed to connect before the deadline')); } - var new_state = result.new_state; - console.log(result); + var new_state = this.channel.getConnectivityState(true); if (new_state === grpc.connectivityState.READY) { callback(); } else if (new_state === grpc.connectivityState.FATAL_FAILURE) { @@ -586,17 +584,7 @@ exports.makeClientConstructor = function(methods, serviceName) { self.channel.watchConnectivityState(new_state, deadline, checkState); } }; - checkState(null, {new_state: this.channel.getConnectivityState()}); - }; - - /** - * Attempt to connect to the server. That will happen automatically if - * you try to make a method call with this client, so this function should - * only be used if you want to know that you have a connection before making - * any calls. - */ - Client.prototype.$tryConnect = function() { - this.channel.getConnectivityState(true); + checkState(); }; _.each(methods, function(attrs, name) { diff --git a/test/surface_test.js b/test/surface_test.js index f002c8b6..b0f3aa4b 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -149,6 +149,12 @@ describe('Client#$waitForReady', function() { after(function() { server.shutdown(); }); + it('should complete when called alone', function(done) { + client.$waitForReady(Infinity, function(error) { + assert.ifError(error); + done(); + }); + }); it('should complete when a call is initiated', function(done) { client.$waitForReady(Infinity, function(error) { assert.ifError(error); @@ -157,13 +163,6 @@ describe('Client#$waitForReady', function() { var call = client.div({}, function(err, response) {}); call.cancel(); }); - it('should complete if $tryConnect is called', function(done) { - client.$waitForReady(Infinity, function(error) { - assert.ifError(error); - done(); - }); - client.$tryConnect(); - }); it('should complete if called more than once', function(done) { done = multiDone(done, 2); client.$waitForReady(Infinity, function(error) { From cb408b39bc3e2e19688a7e0dc1387c74883bc681 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 13 Aug 2015 11:26:57 -0700 Subject: [PATCH 37/41] Further fixed connectivity tests --- src/client.js | 2 +- test/surface_test.js | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/client.js b/src/client.js index 0cd29e5b..d14713f3 100644 --- a/src/client.js +++ b/src/client.js @@ -575,7 +575,7 @@ exports.makeClientConstructor = function(methods, serviceName) { if (err) { callback(new Error('Failed to connect before the deadline')); } - var new_state = this.channel.getConnectivityState(true); + var new_state = self.channel.getConnectivityState(true); if (new_state === grpc.connectivityState.READY) { callback(); } else if (new_state === grpc.connectivityState.FATAL_FAILURE) { diff --git a/test/surface_test.js b/test/surface_test.js index b0f3aa4b..098905e7 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -139,7 +139,7 @@ describe('Client#$waitForReady', function() { var client; before(function() { server = new grpc.Server(); - port = server.bind('localhost:0'); + port = server.bind('localhost:0', grpc.ServerCredentials.createInsecure()); server.start(); Client = surface_client.makeProtobufClientConstructor(mathService); }); @@ -173,7 +173,6 @@ describe('Client#$waitForReady', function() { assert.ifError(error); done(); }); - client.$tryConnect(); }); it('should complete if called when already ready', function(done) { client.$waitForReady(Infinity, function(error) { @@ -183,7 +182,6 @@ describe('Client#$waitForReady', function() { done(); }); }); - client.$tryConnect(); }); }); describe('Echo service', function() { From 64035f9b3ba2181732d308718e3dd6486e38ea73 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 14 Aug 2015 10:35:43 -0700 Subject: [PATCH 38/41] Add parent call propagation API to Node library --- ext/call.cc | 20 ++++- ext/node_grpc.cc | 20 +++++ src/client.js | 7 +- src/server.js | 1 + test/constant_test.js | 18 +++++ test/surface_test.js | 183 +++++++++++++++++++++++++++++++++++++++++- 6 files changed, 244 insertions(+), 5 deletions(-) diff --git a/ext/call.cc b/ext/call.cc index c5c83133..5e187607 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -502,6 +502,22 @@ NAN_METHOD(Call::New) { return NanThrowTypeError( "Call's third argument must be a date or a number"); } + // These arguments are at the end because they are optional + grpc_call *parent_call = NULL; + if (Call::HasInstance(args[4])) { + Call *parent_obj = ObjectWrap::Unwrap(args[4]->ToObject()); + parent_call = parent_obj->wrapped_call; + } else if (!(args[4]->IsUndefined() || args[4]->IsNull())) { + return NanThrowTypeError( + "Call's fifth argument must be another call, if provided"); + } + gpr_uint32 propagate_flags = GRPC_PROPAGATE_DEFAULTS; + if (args[5]->IsUint32()) { + propagate_flags = args[5]->Uint32Value(); + } else if (!(args[5]->IsUndefined() || args[5]->IsNull())) { + return NanThrowTypeError( + "Call's fifth argument must be propagate flags, if provided"); + } Handle channel_object = args[0]->ToObject(); Channel *channel = ObjectWrap::Unwrap(channel_object); if (channel->GetWrappedChannel() == NULL) { @@ -514,12 +530,12 @@ NAN_METHOD(Call::New) { if (args[3]->IsString()) { NanUtf8String host_override(args[3]); wrapped_call = grpc_channel_create_call( - wrapped_channel, NULL, GRPC_PROPAGATE_DEFAULTS, + wrapped_channel, parent_call, propagate_flags, CompletionQueueAsyncWorker::GetQueue(), *method, *host_override, MillisecondsToTimespec(deadline)); } else if (args[3]->IsUndefined() || args[3]->IsNull()) { wrapped_call = grpc_channel_create_call( - wrapped_channel, NULL, GRPC_PROPAGATE_DEFAULTS, + wrapped_channel, parent_call, propagate_flags, CompletionQueueAsyncWorker::GetQueue(), *method, NULL, MillisecondsToTimespec(deadline)); } else { diff --git a/ext/node_grpc.cc b/ext/node_grpc.cc index 4e31cbaa..6c9bc873 100644 --- a/ext/node_grpc.cc +++ b/ext/node_grpc.cc @@ -159,12 +159,32 @@ void InitOpTypeConstants(Handle exports) { op_type->Set(NanNew("RECV_CLOSE_ON_SERVER"), RECV_CLOSE_ON_SERVER); } +void InitPropagateConstants(Handle exports) { + NanScope(); + Handle propagate = NanNew(); + exports->Set(NanNew("propagate"), propagate); + Handle DEADLINE(NanNew(GRPC_PROPAGATE_DEADLINE)); + propagate->Set(NanNew("DEADLINE"), DEADLINE); + Handle CENSUS_STATS_CONTEXT( + NanNew(GRPC_PROPAGATE_CENSUS_STATS_CONTEXT)); + propagate->Set(NanNew("CENSUS_STATS_CONTEXT"), CENSUS_STATS_CONTEXT); + Handle CENSUS_TRACING_CONTEXT( + NanNew(GRPC_PROPAGATE_CENSUS_TRACING_CONTEXT)); + propagate->Set(NanNew("CENSUS_TRACING_CONTEXT"), CENSUS_TRACING_CONTEXT); + Handle CANCELLATION( + NanNew(GRPC_PROPAGATE_CANCELLATION)); + propagate->Set(NanNew("CANCELLATION"), CANCELLATION); + Handle DEFAULTS(NanNew(GRPC_PROPAGATE_DEFAULTS)); + propagate->Set(NanNew("DEFAULTS"), DEFAULTS); +} + void init(Handle exports) { NanScope(); grpc_init(); InitStatusConstants(exports); InitCallErrorConstants(exports); InitOpTypeConstants(exports); + InitPropagateConstants(exports); grpc::node::Call::Init(exports); grpc::node::Channel::Init(exports); diff --git a/src/client.js b/src/client.js index 5cde4385..616b3969 100644 --- a/src/client.js +++ b/src/client.js @@ -216,14 +216,19 @@ ClientDuplexStream.prototype.getPeer = getPeer; function getCall(channel, method, options) { var deadline; var host; + var parent; + var propagate_flags; if (options) { deadline = options.deadline; host = options.host; + parent = _.get(options, 'parent.call'); + propagate_flags = options.propagate_flags; } if (deadline === undefined) { deadline = Infinity; } - return new grpc.Call(channel, method, deadline, host); + return new grpc.Call(channel, method, deadline, host, + parent, propagate_flags); } /** diff --git a/src/server.js b/src/server.js index 5c62f599..8b86173f 100644 --- a/src/server.js +++ b/src/server.js @@ -432,6 +432,7 @@ function handleUnary(call, handler, metadata) { }); emitter.metadata = metadata; waitForCancel(call, emitter); + emitter.call = call; var batch = {}; batch[grpc.opType.RECV_MESSAGE] = true; call.startBatch(batch, function(err, result) { diff --git a/test/constant_test.js b/test/constant_test.js index ecc98ec4..964fc60d 100644 --- a/test/constant_test.js +++ b/test/constant_test.js @@ -78,6 +78,18 @@ var callErrorNames = [ 'INVALID_FLAGS' ]; +/** + * List of all propagate flag names + * @const + * @type {Array.} + */ +var propagateFlagNames = [ + 'DEADLINE', + 'CENSUS_STATS_CONTEXT', + 'CENSUS_TRACING_CONTEXT', + 'CANCELLATION' +]; + describe('constants', function() { it('should have all of the status constants', function() { for (var i = 0; i < statusNames.length; i++) { @@ -91,4 +103,10 @@ describe('constants', function() { 'call error missing: ' + callErrorNames[i]); } }); + it('should have all of the propagate flags', function() { + for (var i = 0; i < propagateFlagNames.length; i++) { + assert(grpc.propagate.hasOwnProperty(propagateFlagNames[i]), + 'call error missing: ' + propagateFlagNames[i]); + } + }); }); diff --git a/test/surface_test.js b/test/surface_test.js index dda2f8d1..b8740af7 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -47,6 +47,27 @@ var mathService = math_proto.lookup('math.Math'); var _ = require('lodash'); +/** + * This is used for testing functions with multiple asynchronous calls that + * can happen in different orders. This should be passed the number of async + * function invocations that can occur last, and each of those should call this + * function's return value + * @param {function()} done The function that should be called when a test is + * complete. + * @param {number} count The number of calls to the resulting function if the + * test passes. + * @return {function()} The function that should be called at the end of each + * sequence of asynchronous functions. + */ +function multiDone(done, count) { + return function() { + count -= 1; + if (count <= 0) { + done(); + } + }; +} + var server_insecure_creds = grpc.ServerCredentials.createInsecure(); describe('File loader', function() { @@ -272,12 +293,14 @@ describe('Echo metadata', function() { }); }); describe('Other conditions', function() { + var test_service; + var Client; var client; var server; var port; before(function() { var test_proto = ProtoBuf.loadProtoFile(__dirname + '/test_service.proto'); - var test_service = test_proto.lookup('TestService'); + test_service = test_proto.lookup('TestService'); server = new grpc.Server(); server.addProtoService(test_service, { unary: function(call, cb) { @@ -339,7 +362,7 @@ describe('Other conditions', function() { } }); port = server.bind('localhost:0', server_insecure_creds); - var Client = surface_client.makeProtobufClientConstructor(test_service); + Client = surface_client.makeProtobufClientConstructor(test_service); client = new Client('localhost:' + port, grpc.Credentials.createInsecure()); server.start(); }); @@ -592,6 +615,162 @@ describe('Other conditions', function() { }); }); }); + describe('Call propagation', function() { + var proxy; + var proxy_impl; + beforeEach(function() { + proxy = new grpc.Server(); + proxy_impl = { + unary: function(call) {}, + clientStream: function(stream) {}, + serverStream: function(stream) {}, + bidiStream: function(stream) {} + }; + }); + afterEach(function() { + console.log('Shutting down server'); + proxy.shutdown(); + }); + describe('Cancellation', function() { + it('With a unary call', function(done) { + done = multiDone(done, 2); + proxy_impl.unary = function(parent, callback) { + client.unary(parent.request, function(err, value) { + try { + assert(err); + assert.strictEqual(err.code, grpc.status.CANCELLED); + } finally { + callback(err, value); + done(); + } + }, null, {parent: parent}); + call.cancel(); + }; + proxy.addProtoService(test_service, proxy_impl); + var proxy_port = proxy.bind('localhost:0', server_insecure_creds); + proxy.start(); + var proxy_client = new Client('localhost:' + proxy_port, + grpc.Credentials.createInsecure()); + var call = proxy_client.unary({}, function(err, value) { + done(); + }); + }); + it('With a client stream call', function(done) { + done = multiDone(done, 2); + proxy_impl.clientStream = function(parent, callback) { + client.clientStream(function(err, value) { + try { + assert(err); + assert.strictEqual(err.code, grpc.status.CANCELLED); + } finally { + callback(err, value); + done(); + } + }, null, {parent: parent}); + call.cancel(); + }; + proxy.addProtoService(test_service, proxy_impl); + var proxy_port = proxy.bind('localhost:0', server_insecure_creds); + proxy.start(); + var proxy_client = new Client('localhost:' + proxy_port, + grpc.Credentials.createInsecure()); + var call = proxy_client.clientStream(function(err, value) { + done(); + }); + }); + it('With a server stream call', function(done) { + done = multiDone(done, 2); + proxy_impl.serverStream = function(parent) { + var child = client.serverStream(parent.request, null, + {parent: parent}); + child.on('error', function(err) { + assert(err); + assert.strictEqual(err.code, grpc.status.CANCELLED); + done(); + }); + call.cancel(); + }; + proxy.addProtoService(test_service, proxy_impl); + var proxy_port = proxy.bind('localhost:0', server_insecure_creds); + proxy.start(); + var proxy_client = new Client('localhost:' + proxy_port, + grpc.Credentials.createInsecure()); + var call = proxy_client.serverStream({}); + call.on('error', function(err) { + done(); + }); + }); + it('With a bidi stream call', function(done) { + done = multiDone(done, 2); + proxy_impl.bidiStream = function(parent) { + var child = client.bidiStream(null, {parent: parent}); + child.on('error', function(err) { + assert(err); + assert.strictEqual(err.code, grpc.status.CANCELLED); + done(); + }); + call.cancel(); + }; + proxy.addProtoService(test_service, proxy_impl); + var proxy_port = proxy.bind('localhost:0', server_insecure_creds); + proxy.start(); + var proxy_client = new Client('localhost:' + proxy_port, + grpc.Credentials.createInsecure()); + var call = proxy_client.bidiStream(); + call.on('error', function(err) { + done(); + }); + }); + }); + describe('Deadline', function() { + it.skip('With a client stream call', function(done) { + done = multiDone(done, 2); + proxy_impl.clientStream = function(parent, callback) { + client.clientStream(function(err, value) { + try { + assert(err); + assert.strictEqual(err.code, grpc.status.DEADLINE_EXCEEDED); + } finally { + callback(err, value); + done(); + } + }, null, {parent: parent}); + }; + proxy.addProtoService(test_service, proxy_impl); + var proxy_port = proxy.bind('localhost:0', server_insecure_creds); + proxy.start(); + var proxy_client = new Client('localhost:' + proxy_port, + grpc.Credentials.createInsecure()); + var deadline = new Date(); + deadline.setSeconds(deadline.getSeconds() + 1); + var call = proxy_client.clientStream(function(err, value) { + done(); + }, null, {deadline: deadline}); + }); + it.skip('With a bidi stream call', function(done) { + done = multiDone(done, 2); + proxy_impl.bidiStream = function(parent) { + var child = client.bidiStream(null, {parent: parent}); + child.on('error', function(err) { + assert(err); + assert.strictEqual(err.code, grpc.status.DEADLINE_EXCEEDED); + done(); + }); + }; + proxy.addProtoService(test_service, proxy_impl); + var proxy_port = proxy.bind('localhost:0', server_insecure_creds); + proxy.start(); + var proxy_client = new Client('localhost:' + proxy_port, + grpc.Credentials.createInsecure()); + var deadline = new Date(); + deadline.setSeconds(deadline.getSeconds() + 1); + var call = proxy_client.bidiStream(null, {deadline: deadline}); + call.on('error', function(err) { + done(); + }); + }); + }); + }); }); describe('Cancelling surface client', function() { var client; From 4e0886684a3bca92a03a1e531ac4383754f9e3a6 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 14 Aug 2015 10:48:45 -0700 Subject: [PATCH 39/41] Made deadline tests work --- index.js | 5 +++++ test/constant_test.js | 3 ++- test/surface_test.js | 14 +++++++++----- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index b26ab35f..93c65ac5 100644 --- a/index.js +++ b/index.js @@ -134,6 +134,11 @@ exports.Server = server.Server; */ exports.status = grpc.status; +/** + * Propagate flag name to number mapping + */ +exports.propagate = grpc.propagate; + /** * Call error name to code number mapping */ diff --git a/test/constant_test.js b/test/constant_test.js index 964fc60d..e251dd34 100644 --- a/test/constant_test.js +++ b/test/constant_test.js @@ -87,7 +87,8 @@ var propagateFlagNames = [ 'DEADLINE', 'CENSUS_STATS_CONTEXT', 'CENSUS_TRACING_CONTEXT', - 'CANCELLATION' + 'CANCELLATION', + 'DEFAULTS' ]; describe('constants', function() { diff --git a/test/surface_test.js b/test/surface_test.js index b8740af7..5e731ea4 100644 --- a/test/surface_test.js +++ b/test/surface_test.js @@ -723,7 +723,10 @@ describe('Other conditions', function() { }); }); describe('Deadline', function() { - it.skip('With a client stream call', function(done) { + /* jshint bitwise:false */ + var deadline_flags = (grpc.propagate.DEFAULTS & + ~grpc.propagate.CANCELLATION); + it('With a client stream call', function(done) { done = multiDone(done, 2); proxy_impl.clientStream = function(parent, callback) { client.clientStream(function(err, value) { @@ -734,7 +737,7 @@ describe('Other conditions', function() { callback(err, value); done(); } - }, null, {parent: parent}); + }, null, {parent: parent, propagate_flags: deadline_flags}); }; proxy.addProtoService(test_service, proxy_impl); var proxy_port = proxy.bind('localhost:0', server_insecure_creds); @@ -743,14 +746,15 @@ describe('Other conditions', function() { grpc.Credentials.createInsecure()); var deadline = new Date(); deadline.setSeconds(deadline.getSeconds() + 1); - var call = proxy_client.clientStream(function(err, value) { + proxy_client.clientStream(function(err, value) { done(); }, null, {deadline: deadline}); }); - it.skip('With a bidi stream call', function(done) { + it('With a bidi stream call', function(done) { done = multiDone(done, 2); proxy_impl.bidiStream = function(parent) { - var child = client.bidiStream(null, {parent: parent}); + var child = client.bidiStream( + null, {parent: parent, propagate_flags: deadline_flags}); child.on('error', function(err) { assert(err); assert.strictEqual(err.code, grpc.status.DEADLINE_EXCEEDED); From 833652967cbaf4ce2bf634f9c7aedb2ca6b8f639 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 14 Aug 2015 11:09:04 -0700 Subject: [PATCH 40/41] Replaced remaining references to 'listen' with 'start' --- examples/perf_test.js | 2 +- examples/qps_test.js | 2 +- examples/route_guide_server.js | 2 +- examples/stock_server.js | 2 +- interop/interop_server.js | 2 +- test/server_test.js | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/examples/perf_test.js b/examples/perf_test.js index 214b9384..ba8fbf88 100644 --- a/examples/perf_test.js +++ b/examples/perf_test.js @@ -40,7 +40,7 @@ var interop_server = require('../interop/interop_server.js'); function runTest(iterations, callback) { var testServer = interop_server.getServer(0, false); - testServer.server.listen(); + testServer.server.start(); var client = new testProto.TestService('localhost:' + testServer.port, grpc.Credentials.createInsecure()); diff --git a/examples/qps_test.js b/examples/qps_test.js index 1ce4dbe0..ec968b85 100644 --- a/examples/qps_test.js +++ b/examples/qps_test.js @@ -60,7 +60,7 @@ var interop_server = require('../interop/interop_server.js'); */ function runTest(concurrent_calls, seconds, callback) { var testServer = interop_server.getServer(0, false); - testServer.server.listen(); + testServer.server.start(); var client = new testProto.TestService('localhost:' + testServer.port, grpc.Credentials.createInsecure()); diff --git a/examples/route_guide_server.js b/examples/route_guide_server.js index bb8e79b5..465b32f5 100644 --- a/examples/route_guide_server.js +++ b/examples/route_guide_server.js @@ -248,7 +248,7 @@ if (require.main === module) { throw err; } feature_list = JSON.parse(data); - routeServer.listen(); + routeServer.start(); }); } diff --git a/examples/stock_server.js b/examples/stock_server.js index dfcfe30e..12e54795 100644 --- a/examples/stock_server.js +++ b/examples/stock_server.js @@ -81,7 +81,7 @@ stockServer.addProtoService(examples.Stock.service, { if (require.main === module) { stockServer.bind('0.0.0.0:50051', grpc.ServerCredentials.createInsecure()); - stockServer.listen(); + stockServer.start(); } module.exports = stockServer; diff --git a/interop/interop_server.js b/interop/interop_server.js index ece22cce..1242a0f9 100644 --- a/interop/interop_server.js +++ b/interop/interop_server.js @@ -194,7 +194,7 @@ if (require.main === module) { }); var server_obj = getServer(argv.port, argv.use_tls === 'true'); console.log('Server attaching to port ' + argv.port); - server_obj.server.listen(); + server_obj.server.start(); } /** diff --git a/test/server_test.js b/test/server_test.js index a9df4390..20c9a07f 100644 --- a/test/server_test.js +++ b/test/server_test.js @@ -83,7 +83,7 @@ describe('server', function() { server = new grpc.Server(); }); }); - describe('listen', function() { + describe('start', function() { var server; before(function() { server = new grpc.Server(); @@ -92,7 +92,7 @@ describe('server', function() { after(function() { server.shutdown(); }); - it('should listen without error', function() { + it('should start without error', function() { assert.doesNotThrow(function() { server.start(); }); From dcd29a3c7a1fd495099c2e17e6c818110f05e5a1 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 14 Aug 2015 12:57:00 -0700 Subject: [PATCH 41/41] Fixed typo in argument error message --- ext/call.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/call.cc b/ext/call.cc index d62ce2aa..705c80ff 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -516,7 +516,7 @@ NAN_METHOD(Call::New) { propagate_flags = args[5]->Uint32Value(); } else if (!(args[5]->IsUndefined() || args[5]->IsNull())) { return NanThrowTypeError( - "Call's fifth argument must be propagate flags, if provided"); + "Call's sixth argument must be propagate flags, if provided"); } Handle channel_object = args[0]->ToObject(); Channel *channel = ObjectWrap::Unwrap(channel_object);