Merge pull request #385 from murgatroid99/generic_client_interceptor_fix

Fix generic client interceptor resolution

Author: murgatroid99

packages/grpc-native-core/src/client.js

@@ -380,19 +380,18 @@ function Client(address, credentials, options) {
   options['grpc.primary_user_agent'] += 'grpc-node/' + version;
 
   // Resolve interceptor options and assign interceptors to each method
-  var interceptor_providers = options.interceptor_providers || [];
-  var interceptors = options.interceptors || [];
-  if (interceptor_providers.length && interceptors.length) {
+  if (_.isArray(options.interceptor_providers) && _.isArray(options.interceptors)) {
     throw new client_interceptors.InterceptorConfigurationError(
       'Both interceptors and interceptor_providers were passed as options ' +
       'to the client constructor. Only one of these is allowed.');
   }
+  self.$interceptors = options.interceptors || [];
+  self.$interceptor_providers = options.interceptor_providers || [];
   _.each(self.$method_definitions, function(method_definition, method_name) {
     self[method_name].interceptors = client_interceptors
-      .resolveInterceptorProviders(interceptor_providers, method_definition)
-      .concat(interceptors);
+      .resolveInterceptorProviders(self.$interceptor_providers, method_definition)
+      .concat(self.$interceptors);
   });
-
   // Exclude interceptor options which have already been consumed
   var channel_options = _.omit(options,
      ['interceptors', 'interceptor_providers']);
@@ -403,6 +402,21 @@ function Client(address, credentials, options) {
 
 exports.Client = Client;
 
+Client.prototype.resolveCallInterceptors = function(method_definition, interceptors, interceptor_providers) {
+  if (_.isArray(interceptors) && _.isArray(interceptor_providers)) {
+    throw new client_interceptors.InterceptorConfigurationError(
+      'Both interceptors and interceptor_providers were passed as call ' +
+      'options. Only one of these is allowed.');
+  }
+  if (_.isArray(interceptors) || _.isArray(interceptor_providers)) {
+    interceptors = interceptors || [];
+    interceptor_providers = interceptor_providers || [];
+    return client_interceptors.resolveInterceptorProviders(interceptor_providers, method_definition).concat(interceptors);
+  } else {
+    return client_interceptors.resolveInterceptorProviders(this.$interceptor_providers, method_definition).concat(this.$interceptors);
+  }
+}
+
 /**
  * @callback grpc.Client~requestCallback
  * @param {?grpc~ServiceError} error The error, if the call
@@ -454,10 +468,6 @@ Client.prototype.makeUnaryRequest = function(path, serialize, deserialize,
     throw new Error('Argument mismatch in makeUnaryRequest');
   }
 
-  var method_name = this.$method_names[path];
-  var constructor_interceptors = this[method_name] ?
-    this[method_name].interceptors :
-    null;
   var method_definition = options.method_definition = {
     path: path,
     requestStream: false,
@@ -471,7 +481,7 @@ Client.prototype.makeUnaryRequest = function(path, serialize, deserialize,
   var intercepting_call = client_interceptors.getInterceptingCall(
     method_definition,
     options,
-    constructor_interceptors,
+    Client.prototype.resolveCallInterceptors.call(this, method_definition, options.interceptors, options.interceptor_providers),
     this.$channel,
     callback
   );
@@ -533,10 +543,6 @@ Client.prototype.makeClientStreamRequest = function(path, serialize,
     throw new Error('Argument mismatch in makeClientStreamRequest');
   }
 
-  var method_name = this.$method_names[path];
-  var constructor_interceptors = this[method_name] ?
-    this[method_name].interceptors :
-    null;
   var method_definition = options.method_definition = {
     path: path,
     requestStream: true,
@@ -550,7 +556,7 @@ Client.prototype.makeClientStreamRequest = function(path, serialize,
   var intercepting_call = client_interceptors.getInterceptingCall(
     method_definition,
     options,
-    constructor_interceptors,
+    Client.prototype.resolveCallInterceptors.call(this, method_definition, options.interceptors, options.interceptor_providers),
     this.$channel,
     callback
   );
@@ -595,10 +601,6 @@ Client.prototype.makeServerStreamRequest = function(path, serialize,
     throw new Error('Argument mismatch in makeServerStreamRequest');
   }
 
-  var method_name = this.$method_names[path];
-  var constructor_interceptors = this[method_name] ?
-    this[method_name].interceptors :
-    null;
   var method_definition = options.method_definition = {
     path: path,
     requestStream: false,
@@ -613,7 +615,7 @@ Client.prototype.makeServerStreamRequest = function(path, serialize,
   var intercepting_call = client_interceptors.getInterceptingCall(
     method_definition,
     options,
-    constructor_interceptors,
+    Client.prototype.resolveCallInterceptors.call(this, method_definition, options.interceptors, options.interceptor_providers),
     this.$channel,
     emitter
   );
@@ -655,10 +657,6 @@ Client.prototype.makeBidiStreamRequest = function(path, serialize,
     throw new Error('Argument mismatch in makeBidiStreamRequest');
   }
 
-  var method_name = this.$method_names[path];
-  var constructor_interceptors = this[method_name] ?
-    this[method_name].interceptors :
-    null;
   var method_definition = options.method_definition = {
     path: path,
     requestStream: true,
@@ -673,7 +671,7 @@ Client.prototype.makeBidiStreamRequest = function(path, serialize,
   var intercepting_call = client_interceptors.getInterceptingCall(
     method_definition,
     options,
-    constructor_interceptors,
+    Client.prototype.resolveCallInterceptors.call(this, method_definition, options.interceptors, options.interceptor_providers),
     this.$channel,
     emitter
   );

packages/grpc-native-core/src/client_interceptors.js

@@ -367,32 +367,6 @@ var resolveInterceptorProviders = function(providers, method_definition) {
   return interceptors;
 };
 
-/**
- * Resolves interceptor options at call invocation time
- * @param {grpc.Client~CallOptions} options The call options passed to a gRPC
- * call.
- * @param {Interceptor[]} [options.interceptors]
- * @param {InterceptorProvider[]} [options.interceptor_providers]
- * @param {grpc~MethodDefinition} method_definition
- * @return {null|function[]}
- */
-var resolveInterceptorOptions = function(options, method_definition) {
-  var provided = resolveInterceptorProviders(options.interceptor_providers,
-    method_definition);
-  if (_.isArray(options.interceptors) && _.isArray(provided)) {
-    throw new InterceptorConfigurationError(
-      'Both interceptors and interceptor_providers were passed as options ' +
-      'to the call invocation. Only one of these is allowed.');
-  }
-  if (_.isArray(options.interceptors)) {
-    return options.interceptors;
-  }
-  if (_.isArray(provided)) {
-    return provided;
-  }
-  return null;
-};
-
 /**
  * A chainable gRPC call proxy which will delegate to an optional requester
  * object. By default, interceptor methods will chain to next_call. If a
@@ -1360,17 +1334,12 @@ function getLastListener(method_definition, emitter, callback) {
  *
  * @param {grpc~MethodDefinition} method_definition
  * @param {grpc.Client~CallOptions} options
- * @param {Interceptor[]} constructor_interceptors
+ * @param {Interceptor[]} interceptors
  * @param {grpc.Channel} channel
  * @param {function|EventEmitter} responder
  */
 function getInterceptingCall(method_definition, options,
-                             constructor_interceptors, channel, responder) {
-  var interceptors = _processInterceptorLayers(
-    options,
-    constructor_interceptors,
-    method_definition
-  );
+                             interceptors, channel, responder) {
   var last_interceptor = _getLastInterceptor(method_definition, channel,
                                             responder);
   var all_interceptors = interceptors.concat(last_interceptor);
@@ -1416,29 +1385,6 @@ function _buildChain(interceptors, options) {
   return new InterceptingCall(chain);
 }
 
-/**
- * Process call options and the interceptor override layers to get the final set
- * of interceptors.
- * @private
- * @param {grpc.Client~CallOptions} call_options The options passed to the gRPC
- * call.
- * @param {Interceptor[]} constructor_interceptors Interceptors passed to the
- * client constructor.
- * @param {grpc~MethodDefinition} method_definition Details of the RPC method.
- * @return {Interceptor[]|null} The final set of interceptors.
- */
-function _processInterceptorLayers(call_options,
-                                   constructor_interceptors,
-                                   method_definition) {
-  var calltime_interceptors = resolveInterceptorOptions(call_options,
-    method_definition);
-  var interceptor_overrides = [
-    calltime_interceptors,
-    constructor_interceptors
-  ];
-  return _resolveInterceptorOverrides(interceptor_overrides);
-}
-
 /**
  * Wraps a plain listener object in an InterceptingListener if it isn't an
  * InterceptingListener already.
@@ -1464,23 +1410,6 @@ function _isInterceptingListener(listener) {
   return listener && listener.constructor.name === 'InterceptingListener';
 }
 
-/**
- * Chooses the first valid array of interceptors or returns null.
- * @param {Interceptor[][]} interceptor_lists A list of interceptor lists in
- * descending override priority order.
- * @return {Interceptor[]|null} The resulting interceptors
- * @private
- */
-function _resolveInterceptorOverrides(interceptor_lists) {
-  for (var i = 0; i < interceptor_lists.length; i++) {
-    var interceptor_list = interceptor_lists[i];
-    if (_.isArray(interceptor_list)) {
-      return interceptor_list;
-    }
-  }
-  return null;
-}
-
 exports.resolveInterceptorProviders = resolveInterceptorProviders;
 
 exports.InterceptingCall = InterceptingCall;