Merge pull request #10093 from murgatroid99/node_server_method_name_flexibility

Node add service: allow exact match to name in proto file, improve error reporting
pull/10134/head
Michael Lumish 8 years ago committed by GitHub
commit 8f134272f5
  1. 1
      src/node/src/common.js
  2. 13
      src/node/src/server.js
  3. 26
      src/node/test/surface_test.js

@ -149,6 +149,7 @@ exports.getProtobufServiceAttrs = function getProtobufServiceAttrs(service,
return _.camelCase(method.name);
}), _.map(service.children, function(method) {
return {
originalName: method.name,
path: prefix + method.name,
requestStream: method.requestStream,
responseStream: method.responseStream,

@ -755,9 +755,16 @@ Server.prototype.addService = function(service, implementation) {
}
var impl;
if (implementation[name] === undefined) {
common.log(grpc.logVerbosity.ERROR, 'Method handler for ' +
attrs.path + ' expected but not provided');
impl = defaultHandler[method_type];
/* Handle the case where the method is passed with the name exactly as
written in the proto file, instead of using JavaScript function
naming style */
if (implementation[attrs.originalName] === undefined) {
common.log(grpc.logVerbosity.ERROR, 'Method handler ' + name + ' for ' +
attrs.path + ' expected but not provided');
impl = defaultHandler[method_type];
} else {
impl = _.bind(implementation[attrs.originalName], implementation);
}
} else {
impl = _.bind(implementation[name], implementation);
}

@ -143,6 +143,32 @@ describe('Server.prototype.addProtoService', function() {
server.addProtoService(mathService, dummyImpls);
});
});
it('Should allow method names as originally written', function() {
var altDummyImpls = {
'Div': function() {},
'DivMany': function() {},
'Fib': function() {},
'Sum': function() {}
};
assert.doesNotThrow(function() {
server.addProtoService(mathService, altDummyImpls);
});
});
it('Should have a conflict between name variations', function() {
/* This is really testing that both name variations are actually used,
by checking that the method actually gets registered, for the
corresponding function, in both cases */
var altDummyImpls = {
'Div': function() {},
'DivMany': function() {},
'Fib': function() {},
'Sum': function() {}
};
server.addProtoService(mathService, altDummyImpls);
assert.throws(function() {
server.addProtoService(mathService, dummyImpls);
});
});
it('Should fail if the server has been started', function() {
server.start();
assert.throws(function() {

Loading…
Cancel
Save