Merge pull request #438 from haines/issue_436

Store helper proxies instead of raw view contexts

Gemfile

@@ -8,7 +8,7 @@ end
 
 platforms :jruby do
   gem "minitest", ">= 3.0"
-  gem "activerecord-jdbcsqlite3-adapter", "~> 1.2.2.1"
+  gem "activerecord-jdbcsqlite3-adapter"
 end
 
 case ENV["RAILS_VERSION"]

lib/draper/helper_proxy.rb

@@ -3,15 +3,34 @@ module Draper
   # defined in your application.
   class HelperProxy
 
+    # @overload initialize(view_context)
+    def initialize(view_context = nil)
+      view_context ||= current_view_context # backwards compatibility
+
+      @view_context = view_context
+    end
+
     # Sends helper methods to the view context.
     def method_missing(method, *args, &block)
-      view_context.send(method, *args, &block)
+      self.class.define_proxy method
+      send(method, *args, &block)
     end
 
+    protected
+
+    attr_reader :view_context
+
     private
 
-    def view_context
-      Draper::ViewContext.current
+    def self.define_proxy(name)
+      define_method name do |*args, &block|
+        view_context.send(name, *args, &block)
+      end
+    end
+
+    def current_view_context
+      ActiveSupport::Deprecation.warn("wrong number of arguments (0 for 1) passed to Draper::HelperProxy.new", caller[1..-1])
+      Draper::ViewContext.current.view_context
     end
   end
 end

lib/draper/view_context.rb

@@ -21,19 +21,21 @@ def self.controller=(controller)
     end
 
     # Returns the current view context, or builds one if none is saved.
+    #
+    # @return [HelperProxy]
     def self.current
-      RequestStore.store[:current_view_context] ||= build
+      RequestStore.store.fetch(:current_view_context) { build! }
     end
 
     # Sets the current view context.
     def self.current=(view_context)
-      RequestStore.store[:current_view_context] = view_context
+      RequestStore.store[:current_view_context] = Draper::HelperProxy.new(view_context)
     end
 
     # Clears the saved controller and view context.
     def self.clear!
-      self.controller = nil
-      self.current = nil
+      RequestStore.store.delete :current_controller
+      RequestStore.store.delete :current_view_context
     end
 
     # Builds a new view context for usage in tests. See {test_strategy} for
@@ -42,6 +44,14 @@ def self.build
       build_strategy.call
     end
 
+    # Builds a new view context and sets it as the current view context.
+    #
+    # @return [HelperProxy]
+    def self.build!
+      # send because we want to return the HelperProxy returned from #current=
+      send :current=, build
+    end
+
     # Configures the strategy used to build view contexts in tests, which
     # defaults to `:full` if `test_strategy` has not been called. Evaluates
     # the block, if given, in the context of the view context's class.

lib/draper/view_helpers.rb

@@ -11,7 +11,7 @@ module ClassMethods
       #
       # @return [HelperProxy] the helpers proxy
       def helpers
-        @helpers ||= Draper::HelperProxy.new
+        Draper::ViewContext.current
       end
       alias_method :h, :helpers
 
@@ -22,7 +22,7 @@ def helpers
     #
     # @return [HelperProxy] the helpers proxy
     def helpers
-      self.class.helpers
+      Draper::ViewContext.current
     end
     alias_method :h, :helpers
 

spec/draper/helper_proxy_spec.rb

@@ -2,13 +2,41 @@
 
 module Draper
   describe HelperProxy do
-    it "proxies methods to the view context" do
-      view_context = double
-      ViewContext.stub(current: view_context)
-      helper_proxy = HelperProxy.new
+    describe "#initialize" do
+      it "sets the view context" do
+        view_context = double
+        helper_proxy = HelperProxy.new(view_context)
 
-      view_context.should_receive(:foo).with("bar")
-      helper_proxy.foo("bar")
+        expect(helper_proxy.send(:view_context)).to be view_context
+      end
+    end
+
+    describe "#method_missing" do
+      protect_class HelperProxy
+
+      it "proxies methods to the view context" do
+        view_context = double
+        helper_proxy = HelperProxy.new(view_context)
+
+        view_context.stub(:foo).and_return{|arg| arg}
+        expect(helper_proxy.foo(:passed)).to be :passed
+      end
+
+      it "passes blocks" do
+        view_context = double
+        helper_proxy = HelperProxy.new(view_context)
+
+        view_context.stub(:foo).and_return{|&block| block.call}
+        expect(helper_proxy.foo{:yielded}).to be :yielded
+      end
+
+      it "defines the method for better performance" do
+        helper_proxy = HelperProxy.new(double(foo: "bar"))
+
+        expect(HelperProxy.instance_methods).not_to include :foo
+        helper_proxy.foo
+        expect(HelperProxy.instance_methods).to include :foo
+      end
     end
   end
 end

spec/draper/view_context_spec.rb

@@ -7,8 +7,8 @@ module Draper
       let(:controller) { Class.new(base) { include ViewContext } }
 
       it "saves the superclass's view context" do
+        ViewContext.should_receive(:current=).with(:controller_view_context)
         controller.new.view_context
-        expect(ViewContext.current).to be :controller_view_context
       end
 
       it "returns the superclass's view context" do
@@ -19,6 +19,7 @@ module Draper
     describe ".controller" do
       it "returns the stored controller from RequestStore" do
         RequestStore.stub store: {current_controller: :stored_controller}
+
         expect(ViewContext.controller).to be :stored_controller
       end
     end
@@ -36,23 +37,39 @@ module Draper
     describe ".current" do
       it "returns the stored view context from RequestStore" do
         RequestStore.stub store: {current_view_context: :stored_view_context}
+
         expect(ViewContext.current).to be :stored_view_context
       end
 
-      it "falls back to building a view context" do
-        RequestStore.stub store: {}
-        ViewContext.should_receive(:build).and_return(:new_view_context)
-        expect(ViewContext.current).to be :new_view_context
+      context "when no view context is stored" do
+        it "builds a view context" do
+          RequestStore.stub store: {}
+          ViewContext.stub build_strategy: ->{ :new_view_context }
+          HelperProxy.stub(:new).with(:new_view_context).and_return(:new_helper_proxy)
+
+          expect(ViewContext.current).to be :new_helper_proxy
+        end
+
+        it "stores the built view context" do
+          store = {}
+          RequestStore.stub store: store
+          ViewContext.stub build_strategy: ->{ :new_view_context }
+          HelperProxy.stub(:new).with(:new_view_context).and_return(:new_helper_proxy)
+
+          ViewContext.current
+          expect(store[:current_view_context]).to be :new_helper_proxy
+        end
       end
     end
 
     describe ".current=" do
-      it "stores a view context in RequestStore" do
+      it "stores a helper proxy for the view context in RequestStore" do
         store = {}
         RequestStore.stub store: store
+        HelperProxy.stub(:new).with(:stored_view_context).and_return(:stored_helper_proxy)
 
         ViewContext.current = :stored_view_context
-        expect(store[:current_view_context]).to be :stored_view_context
+        expect(store[:current_view_context]).to be :stored_helper_proxy
       end
     end
 
@@ -62,18 +79,38 @@ module Draper
         RequestStore.stub store: store
 
         ViewContext.clear!
-        expect(store[:current_controller]).to be_nil
-        expect(store[:current_view_context]).to be_nil
+        expect(store).not_to have_key :current_controller
+        expect(store).not_to have_key :current_view_context
       end
     end
 
     describe ".build" do
-      it "calls the build strategy" do
+      it "returns a new view context using the build strategy" do
         ViewContext.stub build_strategy: ->{ :new_view_context }
+
         expect(ViewContext.build).to be :new_view_context
       end
     end
 
+    describe ".build!" do
+      it "returns a helper proxy for the new view context" do
+        ViewContext.stub build_strategy: ->{ :new_view_context }
+        HelperProxy.stub(:new).with(:new_view_context).and_return(:new_helper_proxy)
+
+        expect(ViewContext.build!).to be :new_helper_proxy
+      end
+
+      it "stores the helper proxy" do
+        store = {}
+        RequestStore.stub store: store
+        ViewContext.stub build_strategy: ->{ :new_view_context }
+        HelperProxy.stub(:new).with(:new_view_context).and_return(:new_helper_proxy)
+
+        ViewContext.build!
+        expect(store[:current_view_context]).to be :new_helper_proxy
+      end
+    end
+
     describe ".build_strategy" do
       it "defaults to full" do
         expect(ViewContext.build_strategy).to be_a ViewContext::BuildStrategy::Full

spec/dummy/spec/decorators/helpers_spec.rb

@@ -12,4 +12,10 @@
   it "can access helpers through `h`" do
     expect(h.content_tag(:p, "Help!")).to eq "<p>Help!</p>"
   end
+
+  it "gets the same helper object as a decorator" do
+    decorator = Draper::Decorator.new(Object.new)
+
+    expect(helpers).to be decorator.helpers
+  end
 end

spec/dummy/test/decorators/minitest/helpers_test.rb

@@ -12,4 +12,10 @@
   it "can access helpers through `h`" do
     assert_equal "<p>Help!</p>", h.content_tag(:p, "Help!")
   end
+
+  it "gets the same helper object as a decorator" do
+    decorator = Draper::Decorator.new(Object.new)
+
+    assert_same decorator.helpers, helpers
+  end
 end

spec/dummy/test/decorators/test_unit/helpers_test.rb

@@ -12,4 +12,10 @@ def test_access_helpers_through_helpers
   def test_access_helpers_through_h
     assert_equal "<p>Help!</p>", h.content_tag(:p, "Help!")
   end
+
+  def test_same_helper_object_as_decorators
+    decorator = Draper::Decorator.new(Object.new)
+
+    assert_same decorator.helpers, helpers
+  end
 end

spec/support/shared_examples/view_helpers.rb

@@ -1,39 +1,39 @@
 shared_examples_for "view helpers" do |subject|
   describe "#helpers" do
-    it "returns the class's helpers" do
-      expect(subject.helpers).to be subject.class.helpers
+    it "returns the current view context" do
+      Draper::ViewContext.stub current: :current_view_context
+      expect(subject.helpers).to be :current_view_context
     end
 
     it "is aliased to #h" do
-      expect(subject.h).to be subject.helpers
+      Draper::ViewContext.stub current: :current_view_context
+      expect(subject.h).to be :current_view_context
     end
   end
 
   describe "#localize" do
     it "delegates to #helpers" do
+      subject.stub helpers: double
       subject.helpers.should_receive(:localize).with(:an_object, some: "parameter")
       subject.localize(:an_object, some: "parameter")
     end
 
     it "is aliased to #l" do
+      subject.stub helpers: double
       subject.helpers.should_receive(:localize).with(:an_object, some: "parameter")
       subject.l(:an_object, some: "parameter")
     end
   end
 
   describe ".helpers" do
-    it "returns a HelperProxy" do
-      expect(subject.class.helpers).to be_a Draper::HelperProxy
-    end
-
-    it "memoizes" do
-      helpers = subject.class.helpers
-
-      expect(subject.class.helpers).to be helpers
+    it "returns the current view context" do
+      Draper::ViewContext.stub current: :current_view_context
+      expect(subject.class.helpers).to be :current_view_context
     end
 
     it "is aliased to .h" do
-      expect(subject.class.h).to be subject.class.helpers
+      Draper::ViewContext.stub current: :current_view_context
+      expect(subject.class.h).to be :current_view_context
     end
   end
 end