From 686f1395da64d2848b7f442a38c9ba49b27cce64 Mon Sep 17 00:00:00 2001 From: Andrew Haines Date: Thu, 17 Jan 2013 13:02:43 +0000 Subject: [PATCH] Check for `decorated?` before using `source` in #== Closes #426 --- lib/draper/decoratable.rb | 10 ++--- lib/draper/decoratable/equality.rb | 14 +++++++ lib/draper/decorator.rb | 4 +- spec/draper/decoratable/equality_spec.rb | 10 +++++ spec/draper/decoratable_spec.rb | 27 +------------ spec/draper/decorator_spec.rb | 32 +++++++-------- .../shared_examples/decoratable_equality.rb | 40 +++++++++++++++++++ 7 files changed, 85 insertions(+), 52 deletions(-) create mode 100644 lib/draper/decoratable/equality.rb create mode 100644 spec/draper/decoratable/equality_spec.rb create mode 100644 spec/support/shared_examples/decoratable_equality.rb diff --git a/lib/draper/decoratable.rb b/lib/draper/decoratable.rb index 56536b58..3707134a 100644 --- a/lib/draper/decoratable.rb +++ b/lib/draper/decoratable.rb @@ -1,3 +1,5 @@ +require 'draper/decoratable/equality' + module Draper # Provides shortcuts to decorate objects directly, so you can do # `@product.decorate` instead of `ProductDecorator.new(@product)`. @@ -7,6 +9,7 @@ module Draper # plain old Ruby objects, you can include it manually. module Decoratable extend ActiveSupport::Concern + include Draper::Decoratable::Equality # Decorates the object using the inferred {#decorator_class}. # @param [Hash] options @@ -40,13 +43,6 @@ def decorated? false end - # Compares with possibly-decorated objects. - # - # @return [Boolean] - def ==(other) - super || (other.respond_to?(:source) && self == other.source) - end - module ClassMethods # Decorates a collection of objects. Used at the end of a scope chain. diff --git a/lib/draper/decoratable/equality.rb b/lib/draper/decoratable/equality.rb new file mode 100644 index 00000000..64679eb2 --- /dev/null +++ b/lib/draper/decoratable/equality.rb @@ -0,0 +1,14 @@ +module Draper + module Decoratable + module Equality + # Compares self with a possibly-decorated object. + # + # @return [Boolean] + def ==(other) + super || + other.respond_to?(:decorated?) && other.decorated? && + other.respond_to?(:source) && self == other.source + end + end + end +end diff --git a/lib/draper/decorator.rb b/lib/draper/decorator.rb index b7c46018..64165191 100755 --- a/lib/draper/decorator.rb +++ b/lib/draper/decorator.rb @@ -156,11 +156,11 @@ def decorated? true end - # Delegated to the source object. + # Compares the source with a possibly-decorated object. # # @return [Boolean] def ==(other) - source == (other.respond_to?(:source) ? other.source : other) + source.extend(Draper::Decoratable::Equality) == other end # Checks if `self.kind_of?(klass)` or `source.kind_of?(klass)` diff --git a/spec/draper/decoratable/equality_spec.rb b/spec/draper/decoratable/equality_spec.rb new file mode 100644 index 00000000..c2d96627 --- /dev/null +++ b/spec/draper/decoratable/equality_spec.rb @@ -0,0 +1,10 @@ +require 'spec_helper' +require 'support/shared_examples/decoratable_equality' + +module Draper + describe Decoratable::Equality do + describe "#==" do + it_behaves_like "decoration-aware #==", Object.new.extend(Decoratable::Equality) + end + end +end diff --git a/spec/draper/decoratable_spec.rb b/spec/draper/decoratable_spec.rb index 9f05cc6c..df3e033a 100644 --- a/spec/draper/decoratable_spec.rb +++ b/spec/draper/decoratable_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +require 'support/shared_examples/decoratable_equality' module Draper describe Decoratable do @@ -55,31 +56,7 @@ module Draper end describe "#==" do - it "is true for itself" do - product = Product.new - - expect(product == product).to be_true - end - - it "is false for another instance" do - product = Product.new - - expect(product == Product.new).to be_false - end - - it "is true for a decorated version of itself" do - product = Product.new - decorator = double(source: product) - - expect(product == decorator).to be_true - end - - it "is false for a decorated other instance" do - product = Product.new - decorator = double(source: Product.new) - - expect(product == decorator).to be_false - end + it_behaves_like "decoration-aware #==", Product.new end describe "#===" do diff --git a/spec/draper/decorator_spec.rb b/spec/draper/decorator_spec.rb index 7dd68da6..00278b11 100755 --- a/spec/draper/decorator_spec.rb +++ b/spec/draper/decorator_spec.rb @@ -354,37 +354,33 @@ module Draper end describe "#==" do - it "is true for itself" do - decorator = Decorator.new(Model.new) + it "ensures the source has a decoration-aware #==" do + source = Object.new + decorator = Decorator.new(source) - expect(decorator == decorator).to be_true + expect(source).not_to be_a_kind_of Draper::Decoratable::Equality + decorator == :something + expect(source).to be_a_kind_of Draper::Decoratable::Equality end - it "is true for another decorator with the same source" do + it "is true when source #== is true" do source = Model.new decorator = Decorator.new(source) + other = double(source: Model.new) - expect(decorator == OtherDecorator.new(source)).to be_true + source.should_receive(:==).with(other).and_return(true) + expect(decorator == other).to be_true end - it "is false for another decorator with a different source" do - decorator = Decorator.new(Model.new) - - expect(decorator == OtherDecorator.new(Model.new)).to be_false - end - - it "is true for the source object" do + it "is false when source #== is false" do source = Model.new decorator = Decorator.new(source) + other = double(source: Model.new) - expect(decorator == source).to be_true + source.should_receive(:==).with(other).and_return(false) + expect(decorator == other).to be_false end - it "is false for any other object" do - decorator = Decorator.new(Model.new) - - expect(decorator == Model.new).to be_false - end end describe "#===" do diff --git a/spec/support/shared_examples/decoratable_equality.rb b/spec/support/shared_examples/decoratable_equality.rb new file mode 100644 index 00000000..6e09d11d --- /dev/null +++ b/spec/support/shared_examples/decoratable_equality.rb @@ -0,0 +1,40 @@ +shared_examples_for "decoration-aware #==" do |subject| + it "is true for itself" do + expect(subject == subject).to be_true + end + + it "is false for another object" do + expect(subject == Object.new).to be_false + end + + it "is true for a decorated version of itself" do + decorated = double(source: subject, decorated?: true) + + expect(subject == decorated).to be_true + end + + it "is false for a decorated other object" do + decorated = double(source: Object.new, decorated?: true) + + expect(subject == decorated).to be_false + end + + it "is false for a decoratable object with a `source` association" do + decoratable = double(source: subject, decorated?: false) + + expect(subject == decoratable).to be_false + end + + it "is false for an undecoratable object with a `source` association" do + undecoratable = double(source: subject) + + expect(subject == undecoratable).to be_false + end + + it "is true for a multiply-decorated version of itself" do + decorated = double(source: subject, decorated?: true) + redecorated = double(source: decorated, decorated?: true) + + expect(subject == redecorated).to be_true + end +end