From 24024d239c0ed9bf79134ba2a0dabf95fb8a724a Mon Sep 17 00:00:00 2001 From: Ryan Buckley Date: Wed, 11 May 2016 16:39:17 +0800 Subject: [PATCH] Middleware can be inserted before or after default middlewares --- CHANGELOG.md | 1 + lib/grape/endpoint.rb | 57 ++++++++++++++--------------- lib/grape/middleware/stack.rb | 29 +++++++++++---- spec/grape/middleware/stack_spec.rb | 39 +++++++++++++++++--- 4 files changed, 83 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6905ea0df0..3cbac7de73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ #### Features +* [#1393](https://github.com/ruby-grape/grape/pull/1393): Middleware can be inserted before or after default Grape middleware - [@ridiculous](https://github.com/ridiculous). * [#1390](https://github.com/ruby-grape/grape/pull/1390): Allow inserting middleware at arbitrary points in the middleware stack - [@Rosa](https://github.com/Rosa). * [#1366](https://github.com/ruby-grape/grape/pull/1366): Store `message_key` on `Grape::Exceptions::Validation` - [@mkou](https://github.com/mkou). diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 67268995c8..4accf0a9d4 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -245,42 +245,39 @@ def run end def build_stack - ms = Grape::Middleware::Stack.new - - ms.use Rack::Head - ms.use Grape::Middleware::Error, - format: namespace_inheritable(:format), - content_types: namespace_stackable_with_hash(:content_types), - default_status: namespace_inheritable(:default_error_status), - rescue_all: namespace_inheritable(:rescue_all), - default_error_formatter: namespace_inheritable(:default_error_formatter), - error_formatters: namespace_stackable_with_hash(:error_formatters), - rescue_options: namespace_stackable_with_hash(:rescue_options) || {}, - rescue_handlers: namespace_stackable_with_hash(:rescue_handlers) || {}, - base_only_rescue_handlers: namespace_stackable_with_hash(:base_only_rescue_handlers) || {}, - all_rescue_handler: namespace_inheritable(:all_rescue_handler) - - ms.merge_with(namespace_stackable(:middleware) || []) + stack = Grape::Middleware::Stack.new + + stack.use Rack::Head + stack.use Grape::Middleware::Error, + format: namespace_inheritable(:format), + content_types: namespace_stackable_with_hash(:content_types), + default_status: namespace_inheritable(:default_error_status), + rescue_all: namespace_inheritable(:rescue_all), + default_error_formatter: namespace_inheritable(:default_error_formatter), + error_formatters: namespace_stackable_with_hash(:error_formatters), + rescue_options: namespace_stackable_with_hash(:rescue_options) || {}, + rescue_handlers: namespace_stackable_with_hash(:rescue_handlers) || {}, + base_only_rescue_handlers: namespace_stackable_with_hash(:base_only_rescue_handlers) || {}, + all_rescue_handler: namespace_inheritable(:all_rescue_handler) + + stack.concat namespace_stackable(:middleware) if namespace_inheritable(:version) - ms.use Grape::Middleware::Versioner.using(namespace_inheritable(:version_options)[:using]), - versions: namespace_inheritable(:version) ? namespace_inheritable(:version).flatten : nil, - version_options: namespace_inheritable(:version_options), - prefix: namespace_inheritable(:root_prefix) + stack.use Grape::Middleware::Versioner.using(namespace_inheritable(:version_options)[:using]), + versions: namespace_inheritable(:version) ? namespace_inheritable(:version).flatten : nil, + version_options: namespace_inheritable(:version_options), + prefix: namespace_inheritable(:root_prefix) end - ms.use Grape::Middleware::Formatter, - format: namespace_inheritable(:format), - default_format: namespace_inheritable(:default_format) || :txt, - content_types: namespace_stackable_with_hash(:content_types), - formatters: namespace_stackable_with_hash(:formatters), - parsers: namespace_stackable_with_hash(:parsers) - - builder = Rack::Builder.new - ms.build(builder) + stack.use Grape::Middleware::Formatter, + format: namespace_inheritable(:format), + default_format: namespace_inheritable(:default_format) || :txt, + content_types: namespace_stackable_with_hash(:content_types), + formatters: namespace_stackable_with_hash(:formatters), + parsers: namespace_stackable_with_hash(:parsers) + builder = stack.build builder.run ->(env) { env[Grape::Env::API_ENDPOINT].run } - builder.to_app end diff --git a/lib/grape/middleware/stack.rb b/lib/grape/middleware/stack.rb index 9454af29cf..847e8b639e 100644 --- a/lib/grape/middleware/stack.rb +++ b/lib/grape/middleware/stack.rb @@ -32,10 +32,11 @@ def inspect include Enumerable - attr_accessor :middlewares + attr_accessor :middlewares, :others def initialize @middlewares = [] + @others = [] end def each @@ -72,25 +73,37 @@ def use(*args, &block) middlewares.push(middleware) end - def merge_with(other) - other.each do |operation, *args| - block = args.pop if args.last.is_a?(Proc) - block ? send(operation, *args, &block) : send(operation, *args) + def merge_with(middleware_specs) + middleware_specs.each do |operation, *args| + if args.last.is_a?(Proc) + public_send(operation, *args, &args.pop) + else + public_send(operation, *args) + end end end - def build(builder) + # @return [Rack::Builder] the builder object with our middlewares applied + def build(builder = Rack::Builder.new) + others.shift(others.size).each(&method(:merge_with)) middlewares.each do |m| m.block ? builder.use(m.klass, *m.args, &m.block) : builder.use(m.klass, *m.args) end + builder + end + + # @description Add middlewares with :use operation to the stack. Store others with :insert_* operation for later + # @param [Array] other_specs An array of middleware specifications (e.g. [[:use, klass], [:insert_before, *args]]) + def concat(other_specs) + @others << Array(other_specs).reject { |o| o.first == :use } + merge_with Array(other_specs).select { |o| o.first == :use } end protected def assert_index(index, where) i = index.is_a?(Integer) ? index : middlewares.index(index) - raise "No such middleware to insert #{where}: #{index.inspect}" unless i - i + i || raise("No such middleware to insert #{where}: #{index.inspect}") end end end diff --git a/spec/grape/middleware/stack_spec.rb b/spec/grape/middleware/stack_spec.rb index a871d8aff6..a837c2123f 100644 --- a/spec/grape/middleware/stack_spec.rb +++ b/spec/grape/middleware/stack_spec.rb @@ -12,6 +12,9 @@ def initialize(&block) end end + let(:proc) { ->() {} } + let(:others) { [[:use, StackSpec::BarMiddleware], [:insert_before, StackSpec::BarMiddleware, StackSpec::BlockMiddleware, proc]] } + subject { Grape::Middleware::Stack.new } before do @@ -33,7 +36,6 @@ def initialize(&block) end it 'pushes a middleware class with block arguments onto the stack' do - proc = ->() {} expect { subject.use StackSpec::BlockMiddleware, &proc } .to change { subject.size }.by(1) expect(subject.last).to eq(StackSpec::BlockMiddleware) @@ -80,15 +82,42 @@ def initialize(&block) end describe '#merge_with' do - let(:proc) { ->() {} } - let(:other) { [[:use, StackSpec::BarMiddleware], [:insert_before, StackSpec::BarMiddleware, StackSpec::BlockMiddleware, proc]] } - it 'applies a collection of operations and middlewares' do - expect { subject.merge_with(other) } + expect { subject.merge_with(others) } .to change { subject.size }.by(2) expect(subject[0]).to eq(StackSpec::FooMiddleware) expect(subject[1]).to eq(StackSpec::BlockMiddleware) expect(subject[2]).to eq(StackSpec::BarMiddleware) end end + + describe '#build' do + it 'returns a rack builder instance' do + expect(subject.build).to be_instance_of(Rack::Builder) + end + + context 'when @others are present' do + let(:others) { [[:insert_after, Grape::Middleware::Formatter, StackSpec::BarMiddleware]] } + + it 'applies the middleware specs stored in @others' do + subject.concat others + subject.use Grape::Middleware::Formatter + subject.build + expect(subject[0]).to eq StackSpec::FooMiddleware + expect(subject[1]).to eq Grape::Middleware::Formatter + expect(subject[2]).to eq StackSpec::BarMiddleware + end + end + end + + describe '#concat' do + it 'adds non :use specs to @others' do + expect { subject.concat others }.to change(subject, :others).from([]).to([[others.last]]) + end + + it 'calls +merge_with+ with the :use specs' do + expect(subject).to receive(:merge_with).with [[:use, StackSpec::BarMiddleware]] + subject.concat others + end + end end