Merge pull request #2704 from murgatroid99/node_method_name_conflicts

Ensure that client generated methods don't conflict with other properties

examples/node/greeter_client.js

@@ -37,7 +37,8 @@ var grpc = require('grpc');
 var hello_proto = grpc.load(PROTO_PATH).helloworld;
 
 function main() {
-  var client = new hello_proto.Greeter('localhost:50051');
+  var client = new hello_proto.Greeter('localhost:50051',
+                                       grpc.Credentials.createInsecure());
   var user;
   if (process.argv.length >= 3) {
     user = process.argv[2];

examples/node/greeter_server.js

@@ -36,8 +36,6 @@ var PROTO_PATH = __dirname + '/helloworld.proto';
 var grpc = require('grpc');
 var hello_proto = grpc.load(PROTO_PATH).helloworld;
 
-var Server = grpc.buildServer([hello_proto.Greeter.service]);
-
 /**
  * Implements the SayHello RPC method.
  */
@@ -50,14 +48,10 @@ function sayHello(call, callback) {
  * sample server port
  */
 function main() {
-  var server = new Server({
-    "helloworld.Greeter": {
-      sayHello: sayHello
-    }
-  });
-
-  server.bind('0.0.0.0:50051');
-  server.listen();
+  var server = new grpc.Server();
+  server.addProtoService(hello_proto.Greeter.service, {sayHello: sayHello});
+  server.bind('0.0.0.0:50051', grpc.ServerCredentials.createInsecure());
+  server.start();
 }
 
 main();

examples/node/package.json

@@ -3,7 +3,7 @@
   "version": "0.5.0",
   "dependencies": {
     "async": "^0.9.0",
-    "grpc": "~0.9.0",
+    "grpc": "~0.11.0",
     "minimist": "^1.1.0",
     "underscore": "^1.8.2"
   }

examples/node/route_guide/route_guide_client.js

@@ -34,7 +34,8 @@ var path = require('path');
 var _ = require('underscore');
 var grpc = require('grpc');
 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;
 

examples/node/route_guide/route_guide_server.js

@@ -34,8 +34,6 @@ var _ = require('underscore');
 var grpc = require('grpc');
 var examples = grpc.load(__dirname + '/route_guide.proto').examples;
 
-var Server = grpc.buildServer([examples.RouteGuide.service]);
-
 var COORD_FACTOR = 1e7;
 
 /**
@@ -222,27 +220,27 @@ function routeChat(call) {
  * @return {Server} The new server object
  */
 function getServer() {
-  return new Server({
-    'examples.RouteGuide' : {
-      getFeature: getFeature,
-      listFeatures: listFeatures,
-      recordRoute: recordRoute,
-      routeChat: routeChat
-    }
+  var server = new grpc.Server();
+  server.addProtoService(examples.RouteGuide.service, {
+    getFeature: getFeature,
+    listFeatures: listFeatures,
+    recordRoute: recordRoute,
+    routeChat: routeChat
   });
+  return server;
 }
 
 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'
   });
   fs.readFile(path.resolve(argv.db_path), function(err, data) {
     if (err) throw err;
     feature_list = JSON.parse(data);
-    routeServer.listen();
+    routeServer.start();
   });
 }
 

src/node/index.js

@@ -164,3 +164,13 @@ exports.ServerCredentials = grpc.ServerCredentials;
  * @see module:src/client.makeClientConstructor
  */
 exports.makeGenericClientConstructor = client.makeClientConstructor;
+
+/**
+ * @see module:src/client.getClientChannel
+ */
+exports.getClientChannel = client.getClientChannel;
+
+/**
+ * @see module:src/client.waitForClientReady
+ */
+exports.waitForClientReady = client.waitForClientReady;

src/node/interop/interop_client.js

@@ -285,7 +285,7 @@ function authTest(expected_user, scope, client, done) {
     if (credential.createScopedRequired() && scope) {
       credential = credential.createScoped(scope);
     }
-    client.updateMetadata = grpc.getGoogleAuthDelegate(credential);
+    client.$updateMetadata = grpc.getGoogleAuthDelegate(credential);
     var arg = {
       response_type: 'COMPRESSABLE',
       response_size: 314159,
@@ -338,7 +338,7 @@ function oauth2Test(expected_user, scope, per_rpc, client, done) {
       if (per_rpc) {
         updateMetadata('', {}, makeTestCall);
       } else {
-        client.updateMetadata = updateMetadata;
+        client.$updateMetadata = updateMetadata;
         makeTestCall(null, {});
       }
     });

src/node/src/client.js

@@ -32,7 +32,7 @@
  */
 
 /**
- * Server module
+ * Client module
  * @module
  */
 
@@ -270,7 +270,7 @@ function makeUnaryRequestFunction(method, serialize, deserialize) {
   function makeUnaryRequest(argument, callback, metadata, options) {
     /* jshint validthis: true */
     var emitter = new EventEmitter();
-    var call = getCall(this.channel, method, options);
+    var call = getCall(this.$channel, method, options);
     if (metadata === null || metadata === undefined) {
       metadata = new Metadata();
     } else {
@@ -282,7 +282,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);
@@ -364,14 +364,14 @@ function makeClientStreamRequestFunction(method, serialize, deserialize) {
    */
   function makeClientStreamRequest(callback, metadata, options) {
     /* jshint validthis: true */
-    var call = getCall(this.channel, method, options);
+    var call = getCall(this.$channel, method, options);
     if (metadata === null || metadata === undefined) {
       metadata = new Metadata();
     } else {
       metadata = metadata.clone();
     }
     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);
@@ -455,14 +455,14 @@ function makeServerStreamRequestFunction(method, serialize, deserialize) {
    */
   function makeServerStreamRequest(argument, metadata, options) {
     /* jshint validthis: true */
-    var call = getCall(this.channel, method, options);
+    var call = getCall(this.$channel, method, options);
     if (metadata === null || metadata === undefined) {
       metadata = new Metadata();
     } else {
       metadata = metadata.clone();
     }
     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);
@@ -533,14 +533,14 @@ function makeBidiStreamRequestFunction(method, serialize, deserialize) {
    */
   function makeBidiStreamRequest(metadata, options) {
     /* jshint validthis: true */
-    var call = getCall(this.channel, method, options);
+    var call = getCall(this.$channel, method, options);
     if (metadata === null || metadata === undefined) {
       metadata = new Metadata();
     } else {
       metadata = metadata.clone();
     }
     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);
@@ -631,45 +631,21 @@ exports.makeClientConstructor = function(methods, serviceName) {
       options = {};
     }
     options['grpc.primary_user_agent'] = 'grpc-node/' + version;
-    this.channel = new grpc.Channel(address, credentials, options);
+    /* Private fields use $ as a prefix instead of _ because it is an invalid
+     * prefix of a method name */
+    this.$channel = new grpc.Channel(address, credentials, options);
     // 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;
+    this.$server_address = address;
+    this.$auth_uri = 'https://' + this.server_address + '/' + 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 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
-   *     to connect.
-   */
-  Client.prototype.$waitForReady = function(deadline, callback) {
-    var self = this;
-    var checkState = function(err) {
-      if (err) {
-        callback(new Error('Failed to connect before the deadline'));
-      }
-      var new_state = self.channel.getConnectivityState(true);
-      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();
-  };
-
   _.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';
@@ -694,6 +670,44 @@ exports.makeClientConstructor = function(methods, serviceName) {
   return Client;
 };
 
+/**
+ * Return the underlying channel object for the specified client
+ * @param {Client} client
+ * @return {Channel} The channel
+ */
+exports.getClientChannel = function(client) {
+  return client.$channel;
+};
+
+/**
+ * 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 will make the channel
+ * start connecting if it has not already done so.
+ * @param {Client} client The client to wait on
+ * @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.
+ */
+exports.waitForClientReady = function(client, deadline, callback) {
+  var checkState = function(err) {
+    if (err) {
+      callback(new Error('Failed to connect before the deadline'));
+    }
+    var new_state = client.$channel.getConnectivityState(true);
+    if (new_state === grpc.connectivityState.READY) {
+      callback();
+    } else if (new_state === grpc.connectivityState.FATAL_FAILURE) {
+      callback(new Error('Failed to connect to server'));
+    } else {
+      client.$channel.watchConnectivityState(new_state, deadline, checkState);
+    }
+  };
+  checkState();
+};
+
 /**
  * Creates a constructor for clients for the given service
  * @param {ProtoBuf.Reflect.Service} service The service to generate a client

src/node/test/surface_test.js

@@ -133,7 +133,25 @@ describe('Server.prototype.addProtoService', function() {
     });
   });
 });
-describe('Client#$waitForReady', 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('waitForClientReady', function() {
   var server;
   var port;
   var Client;
@@ -151,13 +169,13 @@ describe('Client#$waitForReady', function() {
     server.forceShutdown();
   });
   it('should complete when called alone', function(done) {
-    client.$waitForReady(Infinity, function(error) {
+    grpc.waitForClientReady(client, Infinity, function(error) {
       assert.ifError(error);
       done();
     });
   });
   it('should complete when a call is initiated', function(done) {
-    client.$waitForReady(Infinity, function(error) {
+    grpc.waitForClientReady(client, Infinity, function(error) {
       assert.ifError(error);
       done();
     });
@@ -166,19 +184,19 @@ describe('Client#$waitForReady', function() {
   });
   it('should complete if called more than once', function(done) {
     done = multiDone(done, 2);
-    client.$waitForReady(Infinity, function(error) {
+    grpc.waitForClientReady(client, Infinity, function(error) {
       assert.ifError(error);
       done();
     });
-    client.$waitForReady(Infinity, function(error) {
+    grpc.waitForClientReady(client, Infinity, function(error) {
       assert.ifError(error);
       done();
     });
   });
   it('should complete if called when already ready', function(done) {
-    client.$waitForReady(Infinity, function(error) {
+    grpc.waitForClientReady(client, Infinity, function(error) {
       assert.ifError(error);
-      client.$waitForReady(Infinity, function(error) {
+      grpc.waitForClientReady(client, Infinity, function(error) {
         assert.ifError(error);
         done();
       });
@@ -426,7 +444,8 @@ describe('Other conditions', function() {
     server.forceShutdown();
   });
   it('channel.getTarget should be available', function() {
-    assert.strictEqual(typeof client.channel.getTarget(), 'string');
+    assert.strictEqual(typeof grpc.getClientChannel(client).getTarget(),
+                       'string');
   });
   describe('Server recieving bad input', function() {
     var misbehavingClient;