From 6f34bad568e6db5f78a908fa63bd08a1304366fa Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jul 2015 10:09:36 -0700 Subject: [PATCH 1/4] Ensure that client generated methods don't conflict with other properties --- src/node/src/client.js | 27 +++++++++++++++------------ src/node/test/surface_test.js | 20 +++++++++++++++++++- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/node/src/client.js b/src/node/src/client.js index f843669bd0f..405e2be693b 100644 --- a/src/node/src/client.js +++ b/src/node/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/src/node/test/surface_test.js b/src/node/test/surface_test.js index 98f9b15bfc4..1afdb33089b 100644 --- a/src/node/test/surface_test.js +++ b/src/node/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 440ed0318ec0118411c1288b293cf2593cb37d4f Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jul 2015 10:11:07 -0700 Subject: [PATCH 2/4] Revert "Ensure that client generated methods don't conflict with other properties" This reverts commit 6f34bad568e6db5f78a908fa63bd08a1304366fa. --- src/node/src/client.js | 27 ++++++++++++--------------- src/node/test/surface_test.js | 20 +------------------- 2 files changed, 13 insertions(+), 34 deletions(-) diff --git a/src/node/src/client.js b/src/node/src/client.js index 405e2be693b..f843669bd0f 100644 --- a/src/node/src/client.js +++ b/src/node/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/src/node/test/surface_test.js b/src/node/test/surface_test.js index 1afdb33089b..98f9b15bfc4 100644 --- a/src/node/test/surface_test.js +++ b/src/node/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 a43c14fef868964767694f6b297304f1408bcb40 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 30 Jul 2015 13:31:23 -0700 Subject: [PATCH 3/4] Make ruby tests see changes to C core --- src/ruby/ext/grpc/extconf.rb | 6 ++---- tools/run_tests/build_ruby.sh | 2 ++ tools/run_tests/run_tests.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ruby/ext/grpc/extconf.rb b/src/ruby/ext/grpc/extconf.rb index 7972272e2df..803f8fb5501 100644 --- a/src/ruby/ext/grpc/extconf.rb +++ b/src/ruby/ext/grpc/extconf.rb @@ -88,10 +88,8 @@ else else grpc_lib_dir = File.join(File.join(grpc_root, 'libs'), grpc_config) end - unless File.exist?(File.join(grpc_lib_dir, 'libgrpc.a')) - print "Building internal gRPC\n" - system("make -C #{grpc_root} static_c CONFIG=#{grpc_config}") - end + print "Building internal gRPC\n" + system("make -C #{grpc_root} static_c CONFIG=#{grpc_config}") $CFLAGS << ' -I' + File.join(grpc_root, 'include') $LDFLAGS << ' -L' + grpc_lib_dir raise 'gpr not found' unless have_library('gpr', 'gpr_now') diff --git a/tools/run_tests/build_ruby.sh b/tools/run_tests/build_ruby.sh index b6c4e32b7ec..259f336ef22 100755 --- a/tools/run_tests/build_ruby.sh +++ b/tools/run_tests/build_ruby.sh @@ -36,5 +36,7 @@ export GRPC_CONFIG=${CONFIG:-opt} # change to grpc's ruby directory cd $(dirname $0)/../../src/ruby +rm -rf ./tmp + bundle install rake compile:grpc diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 28980c14a82..38da1f043af 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -253,7 +253,7 @@ class RubyLanguage(object): environ=_FORCE_ENVIRON_FOR_WRAPPERS)] def make_targets(self): - return ['run_dep_checks'] + return ['static_c'] def build_steps(self): return [['tools/run_tests/build_ruby.sh']] From 94be2a8db1b537cea63afc1aabc3f1b8a321840b Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 30 Jul 2015 13:34:02 -0700 Subject: [PATCH 4/4] Revert changes to extconf.rb --- src/ruby/ext/grpc/extconf.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ruby/ext/grpc/extconf.rb b/src/ruby/ext/grpc/extconf.rb index 803f8fb5501..7972272e2df 100644 --- a/src/ruby/ext/grpc/extconf.rb +++ b/src/ruby/ext/grpc/extconf.rb @@ -88,8 +88,10 @@ else else grpc_lib_dir = File.join(File.join(grpc_root, 'libs'), grpc_config) end - print "Building internal gRPC\n" - system("make -C #{grpc_root} static_c CONFIG=#{grpc_config}") + unless File.exist?(File.join(grpc_lib_dir, 'libgrpc.a')) + print "Building internal gRPC\n" + system("make -C #{grpc_root} static_c CONFIG=#{grpc_config}") + end $CFLAGS << ' -I' + File.join(grpc_root, 'include') $LDFLAGS << ' -L' + grpc_lib_dir raise 'gpr not found' unless have_library('gpr', 'gpr_now')