Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #333 from haines/method_missing

Refactor Decorator#method_missing
  • Loading branch information...
commit 33808da454cae45b63bb6e21d0b87bd0fe45dd9e 2 parents a9dedcd + b8ca040
@steveklabnik steveklabnik authored
Showing with 191 additions and 206 deletions.
  1. +10 −13 lib/draper/decorator.rb
  2. +2 −2 spec/draper/collection_decorator_spec.rb
  3. +126 −113 spec/draper/decorator_spec.rb
  4. +19 −39 spec/spec_helper.rb
  5. +12 −0 spec/support/action_controller.rb
  6. +5 −0 spec/support/{samples → }/active_record.rb
  7. 0  spec/support/{samples → decorators}/decorator_with_application_helper.rb
  8. +0 −2  spec/support/{samples → decorators}/namespaced_product_decorator.rb
  9. 0  spec/support/{samples → decorators}/non_active_model_product_decorator.rb
  10. +9 −0 spec/support/{samples → decorators}/product_decorator.rb
  11. 0  spec/support/{samples → decorators}/products_decorator.rb
  12. 0  spec/support/{samples → decorators}/some_thing_decorator.rb
  13. +0 −2  spec/support/{samples → decorators}/specific_product_decorator.rb
  14. 0  spec/support/{samples → decorators}/widget_decorator.rb
  15. +0 −2  spec/support/{samples → models}/namespaced_product.rb
  16. 0  spec/support/{samples → models}/non_active_model_product.rb
  17. +8 −0 spec/support/{samples → models}/product.rb
  18. 0  spec/support/{samples → models}/some_thing.rb
  19. 0  spec/support/{samples → models}/uninferrable_decorator_model.rb
  20. 0  spec/support/{samples → models}/widget.rb
  21. +0 −5 spec/support/samples/decorator.rb
  22. +0 −13 spec/support/samples/decorator_with_special_methods.rb
  23. +0 −2  spec/support/samples/finder_decorator.rb
  24. +0 −13 spec/support/samples/sequel_product.rb
View
23 lib/draper/decorator.rb
@@ -157,21 +157,12 @@ def present?
end
def method_missing(method, *args, &block)
- super unless allow?(method)
-
- if source.respond_to?(method)
- self.class.send :define_method, method do |*args, &blokk|
- source.send method, *args, &blokk
- end
-
- send method, *args, &block
+ if allow?(method) && source.respond_to?(method)
+ self.class.define_proxy(method)
+ send(method, *args, &block)
else
super
end
-
- rescue NoMethodError => no_method_error
- super if no_method_error.name == method
- raise no_method_error
end
# For ActiveModel compatibilty
@@ -184,7 +175,13 @@ def to_param
source.to_param
end
- private
+ private
+
+ def self.define_proxy(method)
+ define_method(method) do |*args, &block|
+ source.send(method, *args, &block)
+ end
+ end
def self.security
@security ||= Security.new
View
4 spec/draper/collection_decorator_spec.rb
@@ -136,11 +136,11 @@
describe ".helpers" do
it "returns a HelperProxy" do
- Decorator.helpers.should be_a Draper::HelperProxy
+ subject.class.helpers.should be_a Draper::HelperProxy
end
it "is aliased to .h" do
- Decorator.h.should be Decorator.helpers
+ subject.class.h.should be subject.class.helpers
end
end
View
239 spec/draper/decorator_spec.rb
@@ -2,9 +2,9 @@
describe Draper::Decorator do
before { ApplicationController.new.view_context }
- subject { Decorator.new(source) }
- let(:source){ Product.new }
- let(:non_active_model_source){ NonActiveModelProduct.new }
+ subject { decorator_class.new(source) }
+ let(:decorator_class) { Draper::Decorator }
+ let(:source) { Product.new }
describe "#initialize" do
it "sets the source" do
@@ -12,7 +12,7 @@
end
it "stores options" do
- decorator = Decorator.new(source, some: "options")
+ decorator = decorator_class.new(source, some: "options")
decorator.options.should == {some: "options"}
end
@@ -106,7 +106,7 @@
describe "#localize" do
before { subject.helpers.should_receive(:localize).with(:an_object, {some: "options"}) }
- it "delegates to helpers" do
+ it "delegates to #helpers" do
subject.localize(:an_object, some: "options")
end
@@ -117,16 +117,17 @@
describe ".helpers" do
it "returns a HelperProxy" do
- Decorator.helpers.should be_a Draper::HelperProxy
+ subject.class.helpers.should be_a Draper::HelperProxy
end
it "is aliased to .h" do
- Decorator.h.should be Decorator.helpers
+ subject.class.h.should be subject.class.helpers
end
end
describe ".decorates_association" do
- before { subject.class.decorates_association :similar_products, with: ProductDecorator }
+ let(:decorator_class) { Class.new(ProductDecorator) }
+ before { decorator_class.decorates_association :similar_products, with: ProductDecorator }
describe "overridden association method" do
let(:decorated_association) { ->{} }
@@ -151,7 +152,7 @@
end
describe ".decorates_associations" do
- subject { Decorator }
+ subject { decorator_class }
it "decorates each of the associations" do
subject.should_receive(:decorates_association).with(:similar_products, {})
@@ -210,73 +211,119 @@
end
end
- describe "method selection" do
- it "echos the methods of the wrapped class" do
- source.methods.each do |method|
- subject.respond_to?(method.to_sym, true).should be_true
- end
+ describe "#to_param" do
+ it "proxies to the source" do
+ source.stub(:to_param).and_return(42)
+ subject.to_param.should == 42
+ end
+ end
+
+ describe "#==" do
+ it "compares the decorated models" do
+ other = Draper::Decorator.new(source)
+ subject.should == other
end
+ end
+
+ describe "#respond_to?" do
+ let(:decorator_class) { Class.new(ProductDecorator) }
- it "not override a defined method with a source method" do
- DecoratorWithApplicationHelper.new(source).length.should == "overridden"
+ it "returns true for its own methods" do
+ subject.should respond_to :awesome_title
end
- it "not copy the .class, .inspect, or other existing methods" do
- source.class.should_not == subject.class
- source.inspect.should_not == subject.inspect
- source.to_s.should_not == subject.to_s
+ it "returns true for the source's methods" do
+ subject.should respond_to :title
end
- context "when an ActiveModel descendant" do
- it "always proxy to_param if it is not defined on the decorator itself" do
- source.stub(:to_param).and_return(1)
- Draper::Decorator.new(source).to_param.should == 1
+ context "with include_private" do
+ it "returns true for its own private methods" do
+ subject.respond_to?(:awesome_private_title, true).should be_true
end
- it "always proxy id if it is not defined on the decorator itself" do
- source.stub(:id).and_return(123456789)
- Draper::Decorator.new(source).id.should == 123456789
+ it "returns true for the source's private methods" do
+ subject.respond_to?(:private_title, true).should be_true
end
+ end
- it "always proxy errors if it is not defined on the decorator itself" do
- Draper::Decorator.new(source).errors.should be_an_instance_of ActiveModel::Errors
- end
+ context "with method security" do
+ it "respects allows" do
+ subject.class.allows :hello_world
- it "never proxy to_param if it is defined on the decorator itself" do
- source.stub(:to_param).and_return(1)
- DecoratorWithSpecialMethods.new(source).to_param.should == "foo"
+ subject.should respond_to :hello_world
+ subject.should_not respond_to :goodnight_moon
end
- it "never proxy id if it is defined on the decorator itself" do
- source.stub(:id).and_return(123456789)
- DecoratorWithSpecialMethods.new(source).id.should == 1337
+ it "respects denies" do
+ subject.class.denies :goodnight_moon
+
+ subject.should respond_to :hello_world
+ subject.should_not respond_to :goodnight_moon
end
- it "never proxy errors if it is defined on the decorator itself" do
- DecoratorWithSpecialMethods.new(source).errors.should be_an_instance_of Array
+ it "respects denies_all" do
+ subject.class.denies_all
+
+ subject.should_not respond_to :hello_world
+ subject.should_not respond_to :goodnight_moon
end
end
end
- it "wrap source methods so they still accept blocks" do
- subject.block{"marker"}.should == "marker"
- end
+ describe "method proxying" do
+ let(:decorator_class) { Class.new(ProductDecorator) }
- describe "#==" do
- it "compares the decorated models" do
- other = Draper::Decorator.new(source)
- subject.should == other
+ it "does not proxy methods that are defined on the decorator" do
+ subject.overridable.should be :overridden
end
- end
- describe "#respond_to?" do
- # respond_to? is called by some proxies (id, to_param, errors).
- # This is, why I stub it this way.
- it "delegates to the decorated model" do
- other = Draper::Decorator.new(source)
- source.stub(:respond_to?).and_return(false)
- source.should_receive(:respond_to?).with(:whatever, true).once.and_return("mocked")
- subject.respond_to?(:whatever, true).should == "mocked"
+ it "does not proxy methods inherited from Object" do
+ subject.inspect.should_not be source.inspect
+ end
+
+ it "proxies missing methods that exist on the source" do
+ source.stub(:hello_world).and_return(:proxied)
+ subject.hello_world.should be :proxied
+ end
+
+ it "adds proxied methods to the decorator when they are used" do
+ subject.methods.should_not include :hello_world
+ subject.hello_world
+ subject.methods.should include :hello_world
+ end
+
+ it "passes blocks to proxied methods" do
+ subject.block{"marker"}.should == "marker"
+ end
+
+ it "does not confuse Kernel#Array" do
+ Array(subject).should be_a Array
+ end
+
+ context "with method security" do
+ it "respects allows" do
+ source.stub(:hello_world, :goodnight_moon).and_return(:proxied)
+ subject.class.allows :hello_world
+
+ subject.hello_world.should be :proxied
+ expect{subject.goodnight_moon}.to raise_error NameError
+ end
+
+ it "respects denies" do
+ source.stub(:hello_world, :goodnight_moon).and_return(:proxied)
+ subject.class.denies :goodnight_moon
+
+ subject.hello_world.should be :proxied
+ expect{subject.goodnight_moon}.to raise_error NameError
+ end
+
+ it "respects denies_all" do
+ source.stub(:hello_world, :goodnight_moon).and_return(:proxied)
+ subject.class.denies_all
+
+ expect{subject.hello_world}.to raise_error NameError
+ expect{subject.goodnight_moon}.to raise_error NameError
+ end
end
end
@@ -302,64 +349,26 @@
end
context "in a Rails application" do
- let(:decorator){ DecoratorWithApplicationHelper.decorate(Object.new) }
+ let(:decorator_class) { DecoratorWithApplicationHelper }
it "has access to ApplicationHelper helpers" do
- decorator.uses_hello_world.should == "Hello, World!"
+ subject.uses_hello_world.should == "Hello, World!"
end
it "is able to use the content_tag helper" do
- decorator.sample_content.to_s.should == "<span>Hello, World!</span>"
+ subject.sample_content.to_s.should == "<span>Hello, World!</span>"
end
it "is able to use the link_to helper" do
- decorator.sample_link.should == "<a href=\"/World\">Hello</a>"
+ subject.sample_link.should == "<a href=\"/World\">Hello</a>"
end
- it "is able to use the pluralize helper" do
- decorator.sample_truncate.should == "Once..."
+ it "is able to use the truncate helper" do
+ subject.sample_truncate.should == "Once..."
end
it "is able to access html_escape, a private method" do
- decorator.sample_html_escaped_text.should == '&lt;script&gt;danger&lt;/script&gt;'
- end
- end
-
- describe "#method_missing" do
- context "with an isolated decorator class" do
- let(:decorator_class) { Class.new(Decorator) }
- subject{ decorator_class.new(source) }
-
- context "when #hello_world is called again" do
- it "proxies method directly after first hit" do
- subject.methods.should_not include(:hello_world)
- subject.hello_world
- subject.methods.should include(:hello_world)
- end
- end
-
- context "when #hello_world is called for the first time" do
- it "hits method missing" do
- subject.should_receive(:method_missing)
- subject.hello_world
- end
- end
- end
-
- context "when the delegated method calls a non-existant method" do
- it "should not try to delegate to non-existant methods to not confuse Kernel#Array" do
- Array(subject).should be_kind_of(Array)
- end
-
- it "raises the correct NoMethodError" do
- begin
- subject.some_action
- rescue NoMethodError => e
- e.name.should_not == :some_action
- else
- fail("No exception raised")
- end
- end
+ subject.sample_html_escaped_text.should == '&lt;script&gt;danger&lt;/script&gt;'
end
end
@@ -390,24 +399,28 @@
end
end
- context "with for: symbol" do
- it "sets the finder class" do
- FinderDecorator.has_finders for: :product
- FinderDecorator.finder_class.should be Product
+ context "with :for option" do
+ subject { Class.new(Draper::Decorator) }
+
+ context "with a symbol" do
+ it "sets the finder class" do
+ subject.has_finders for: :product
+ subject.finder_class.should be Product
+ end
end
- end
- context "with for: string" do
- it "sets the finder class" do
- FinderDecorator.has_finders for: "some_thing"
- FinderDecorator.finder_class.should be SomeThing
+ context "with a string" do
+ it "sets the finder class" do
+ subject.has_finders for: "some_thing"
+ subject.finder_class.should be SomeThing
+ end
end
- end
- context "with for: class" do
- it "sets the finder_class" do
- FinderDecorator.has_finders for: Namespace::Product
- FinderDecorator.finder_class.should be Namespace::Product
+ context "with a class" do
+ it "sets the finder_class" do
+ subject.has_finders for: Namespace::Product
+ subject.finder_class.should be Namespace::Product
+ end
end
end
end
View
58 spec/spec_helper.rb
@@ -3,49 +3,29 @@
require 'rails'
require 'action_view'
-Bundler.require
-
-require './spec/support/samples/active_record'
-require './spec/support/samples/decorator'
-require './spec/support/samples/decorator_with_application_helper'
-require './spec/support/samples/decorator_with_special_methods'
-require './spec/support/samples/finder_decorator'
-require './spec/support/samples/namespaced_product'
-require './spec/support/samples/namespaced_product_decorator'
-require './spec/support/samples/non_active_model_product'
-require './spec/support/samples/non_active_model_product_decorator'
-require './spec/support/samples/product'
-require './spec/support/samples/product_decorator'
-require './spec/support/samples/products_decorator'
-require './spec/support/samples/sequel_product'
-require './spec/support/samples/specific_product_decorator'
-require './spec/support/samples/some_thing'
-require './spec/support/samples/some_thing_decorator'
-require './spec/support/samples/uninferrable_decorator_model'
-require './spec/support/samples/widget'
-require './spec/support/samples/widget_decorator'
-
require 'action_controller'
require 'action_controller/test_case'
-module ActionController
- class Base
- Draper::System.setup_action_controller(self)
- end
-end
-
-module ActiveRecord
- class Relation
- end
-end
-
-class ApplicationController < ActionController::Base
- def hello_world
- "Hello, World!"
- end
- helper_method :hello_world
-end
+Bundler.require
+require './spec/support/active_record'
+require './spec/support/action_controller'
+
+require './spec/support/models/product'
+require './spec/support/models/namespaced_product'
+require './spec/support/models/non_active_model_product'
+require './spec/support/models/widget'
+require './spec/support/models/some_thing'
+require './spec/support/models/uninferrable_decorator_model'
+
+require './spec/support/decorators/product_decorator'
+require './spec/support/decorators/namespaced_product_decorator'
+require './spec/support/decorators/non_active_model_product_decorator'
+require './spec/support/decorators/widget_decorator'
+require './spec/support/decorators/specific_product_decorator'
+require './spec/support/decorators/products_decorator'
+require './spec/support/decorators/some_thing_decorator'
+require './spec/support/decorators/decorator_with_application_helper'
class << Rails
undef application # Avoid silly Rails bug: https://github.com/rails/rails/pull/6429
View
12 spec/support/action_controller.rb
@@ -0,0 +1,12 @@
+module ActionController
+ class Base
+ Draper::System.setup_action_controller(self)
+ end
+end
+
+class ApplicationController < ActionController::Base
+ def hello_world
+ "Hello, World!"
+ end
+ helper_method :hello_world
+end
View
5 spec/support/samples/active_record.rb → spec/support/active_record.rb
@@ -15,3 +15,8 @@ def self.limit
end
end
+
+module ActiveRecord
+ class Relation
+ end
+end
View
0  ...port/samples/decorator_with_application_helper.rb → ...t/decorators/decorator_with_application_helper.rb
File renamed without changes
View
2  spec/support/samples/namespaced_product_decorator.rb → ...upport/decorators/namespaced_product_decorator.rb
@@ -1,5 +1,3 @@
-require './spec/support/samples/namespaced_product'
-
module Namespace
class ProductDecorator < Draper::Decorator
has_finders
View
0  ...ort/samples/non_active_model_product_decorator.rb → .../decorators/non_active_model_product_decorator.rb
File renamed without changes
View
9 spec/support/samples/product_decorator.rb → spec/support/decorators/product_decorator.rb
@@ -5,6 +5,15 @@ def awesome_title
"Awesome Title"
end
+ def overridable
+ :overridden
+ end
+
def self.my_class_method
end
+
+ private
+
+ def awesome_private_title
+ end
end
View
0  spec/support/samples/products_decorator.rb → spec/support/decorators/products_decorator.rb
File renamed without changes
View
0  spec/support/samples/some_thing_decorator.rb → spec/support/decorators/some_thing_decorator.rb
File renamed without changes
View
2  spec/support/samples/specific_product_decorator.rb → .../support/decorators/specific_product_decorator.rb
@@ -1,4 +1,2 @@
-require './spec/support/samples/product_decorator'
-
class SpecificProductDecorator < ProductDecorator
end
View
0  spec/support/samples/widget_decorator.rb → spec/support/decorators/widget_decorator.rb
File renamed without changes
View
2  spec/support/samples/namespaced_product.rb → spec/support/models/namespaced_product.rb
@@ -1,5 +1,3 @@
-require './spec/support/samples/product'
-
module Namespace
class Product < ActiveRecord::Base
include Draper::Decoratable
View
0  spec/support/samples/non_active_model_product.rb → spec/support/models/non_active_model_product.rb
File renamed without changes
View
8 spec/support/samples/product.rb → spec/support/models/product.rb
@@ -77,4 +77,12 @@ def poro_previous_version
Product.new
end
+ def overridable
+ :overridable
+ end
+
+ private
+
+ def private_title
+ end
end
View
0  spec/support/samples/some_thing.rb → spec/support/models/some_thing.rb
File renamed without changes
View
0  spec/support/samples/uninferrable_decorator_model.rb → spec/support/models/uninferrable_decorator_model.rb
File renamed without changes
View
0  spec/support/samples/widget.rb → spec/support/models/widget.rb
File renamed without changes
View
5 spec/support/samples/decorator.rb
@@ -1,5 +0,0 @@
-class Decorator < Draper::Decorator
- def self.own_class_method
- "own class method"
- end
-end
View
13 spec/support/samples/decorator_with_special_methods.rb
@@ -1,13 +0,0 @@
-class DecoratorWithSpecialMethods < Draper::Decorator
- def to_param
- "foo"
- end
-
- def id
- 1337
- end
-
- def errors
- ["omg errors!"]
- end
-end
View
2  spec/support/samples/finder_decorator.rb
@@ -1,2 +0,0 @@
-class FinderDecorator < Draper::Decorator
-end
View
13 spec/support/samples/sequel_product.rb
@@ -1,13 +0,0 @@
-module Sequel
- class Model
- def each
- end
- end
-end
-
-class SequelProduct < Sequel::Model
- def some_attribute
- "hello"
- end
-end
-
Please sign in to comment.
Something went wrong with that request. Please try again.