From 01e0d32722808cc4e4c0e0a823cfc748b0b285bc Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 5 Jun 2018 11:14:58 -0700 Subject: [PATCH] Fix generic client interceptor resolution --- packages/grpc-native-core/src/client.js | 50 ++++++------- .../src/client_interceptors.js | 75 +------------------ 2 files changed, 26 insertions(+), 99 deletions(-) diff --git a/packages/grpc-native-core/src/client.js b/packages/grpc-native-core/src/client.js index e5ce8a3e1..83d7b750d 100644 --- a/packages/grpc-native-core/src/client.js +++ b/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 ); diff --git a/packages/grpc-native-core/src/client_interceptors.js b/packages/grpc-native-core/src/client_interceptors.js index 4f45b7529..ddd147218 100644 --- a/packages/grpc-native-core/src/client_interceptors.js +++ b/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;