Change Builder from a class to a module and only build app once

Previously, OmniAuth::Builder was a subclass of Rack::Builder.
Rack::Builder#call calls #to_app, rebuilding the Rack application on
each request, and, as it says in the Rack::Builder comments, this may
have a negative impact on performance.

This commit changes OmniAuth::Builder from a class to a module and
defines a ::new method that extends a Rack::Builder instance with that
module, instance_evals any block, and returns the value of #to_app, the
built application.

Using OmniAuth::Builder will now only build the application once.

For users who have been subclassing OmniAuth::Builder, a fix would be
subclassing Rack::Builder directly and including OmniAuth::Builder:

    class MyBuilder < Rack::Builder
      include OmniAuth::Builder
      # ...
    end

Resolves omniauth#1083.

lib/omniauth/builder.rb

@@ -1,5 +1,13 @@
+require 'rack/builder'
+
 module OmniAuth
-  class Builder < ::Rack::Builder
+  module Builder
+    def self.new app = nil, &block
+      builder = Rack::Builder.new(app).extend(self)
+      builder.instance_eval(&block) if block_given?
+      builder.to_app
+    end
+
     def on_failure(&block)
       OmniAuth.config.on_failure = block
     end
@@ -39,9 +47,5 @@ def provider(klass, *args, **opts, &block)
 
       use middleware, *args, **options.merge(opts), &block
     end
-
-    def call(env)
-      to_app.call(env)
-    end
   end
 end

spec/omniauth/builder_spec.rb

@@ -3,25 +3,25 @@
 describe OmniAuth::Builder do
   describe '#provider' do
     it 'translates a symbol to a constant' do
-      expect(OmniAuth::Strategies).to receive(:const_get).with('MyStrategy', false).and_return(Class.new)
-      OmniAuth::Builder.new(nil) do
+      expect(OmniAuth::Strategies).to receive(:const_get).with('MyStrategy', false).and_return(Struct.new :app)
+      OmniAuth::Builder.new(proc {}) do
         provider :my_strategy
       end
     end
 
     it 'accepts a class' do
-      class ExampleClass; end
+      ExampleClass = Struct.new :app
 
       expect do
-        OmniAuth::Builder.new(nil) do
+        OmniAuth::Builder.new(proc {}) do
           provider ::ExampleClass
         end
       end.not_to raise_error
     end
 
     it "raises a helpful LoadError message if it can't find the class" do
       expect do
-        OmniAuth::Builder.new(nil) do
+        OmniAuth::Builder.new(proc {}) do
           provider :lorax
         end
       end.to raise_error(LoadError, 'Could not find matching strategy for :lorax. You may need to install an additional gem (such as omniauth-lorax).')
@@ -31,7 +31,7 @@ class ExampleClass; end
       class MyStrategy; end
 
       expect do
-        OmniAuth::Builder.new(nil) do
+        OmniAuth::Builder.new(proc {}) do
           provider :my_strategy
         end
       end.to raise_error(LoadError, 'Could not find matching strategy for :my_strategy. You may need to install an additional gem (such as omniauth-my_strategy).')
@@ -40,21 +40,25 @@ class MyStrategy; end
 
   describe '#options' do
     it 'merges provided options in' do
-      k = Class.new
-      b = OmniAuth::Builder.new(nil)
-      expect(b).to receive(:use).with(k, :foo => 'bar', :baz => 'tik')
+      k = Struct.new :app, :options
+      app = proc {}
+      expect(k).to receive(:new).with(app, :foo => 'bar', :baz => 'tik')
 
-      b.options :foo => 'bar'
-      b.provider k, :baz => 'tik'
+      OmniAuth::Builder.new(app) do
+        options :foo => 'bar'
+        provider k, :baz => 'tik'
+      end
     end
 
     it 'adds an argument if no options are provided' do
-      k = Class.new
-      b = OmniAuth::Builder.new(nil)
-      expect(b).to receive(:use).with(k, :foo => 'bar')
+      k = Struct.new :app, :options
+      app = proc {}
+      expect(k).to receive(:new).with(app, :foo => 'bar')
 
-      b.options :foo => 'bar'
-      b.provider k
+      OmniAuth::Builder.new(app) do
+        options :foo => 'bar'
+        provider k
+      end
     end
   end
 
@@ -63,7 +67,9 @@ class MyStrategy; end
       prok = proc {}
 
       with_config_reset(:on_failure) do
-        OmniAuth::Builder.new(nil).on_failure(&prok)
+        OmniAuth::Builder.new(proc {}) do
+          on_failure(&prok)
+        end
 
         expect(OmniAuth.config.on_failure).to eq(prok)
       end
@@ -75,7 +81,9 @@ class MyStrategy; end
       prok = proc {}
 
       with_config_reset(:before_options_phase) do
-        OmniAuth::Builder.new(nil).before_options_phase(&prok)
+        OmniAuth::Builder.new(proc {}) do
+          before_options_phase(&prok)
+        end
 
         expect(OmniAuth.config.before_options_phase).to eq(prok)
       end
@@ -87,7 +95,9 @@ class MyStrategy; end
       prok = proc {}
 
       with_config_reset(:before_request_phase) do
-        OmniAuth::Builder.new(nil).before_request_phase(&prok)
+        OmniAuth::Builder.new(proc {}) do
+          before_request_phase(&prok)
+        end
 
         expect(OmniAuth.config.before_request_phase).to eq(prok)
       end
@@ -99,7 +109,9 @@ class MyStrategy; end
       prok = proc {}
 
       with_config_reset(:before_callback_phase) do
-        OmniAuth::Builder.new(nil).before_callback_phase(&prok)
+        OmniAuth::Builder.new(proc {}) do
+          before_callback_phase(&prok)
+        end
 
         expect(OmniAuth.config.before_callback_phase).to eq(prok)
       end
@@ -111,7 +123,9 @@ class MyStrategy; end
       prok = proc {}
       allow(OmniAuth).to receive(:configure).and_call_original
 
-      OmniAuth::Builder.new(nil).configure(&prok)
+      OmniAuth::Builder.new(proc {}) do
+        configure(&prok)
+      end
 
       expect(OmniAuth).to have_received(:configure) do |&block|
         expect(block).to eq(prok)