Merge pull request #1393 from ridiculous/lazy-middleware-stack

Middleware can be inserted before or after defaults

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).
 

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
 

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

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