Merge pull request #428 from haines/issue_426

Check for `decorated?` before using `source` in #==

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.

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

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

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

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

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

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