Doublespeak: Only store original method once

If a method for a class is doubled more than once within the same test
run, the original implementation of that method will change from double
to double, even if the double is deactivated correctly. Say we have two
distinct tests that both double the same method. Here is how that method
will be overridden as it goes along:

    A) START: original method
    B) ACTIVATE (1st test): method is doubled
    C) DEACTIVATE (1st test): calls original method (A)
    D) ACTIVATE (2nd test): original method (C) stored; method is again
                            doubled
    E) DEACTIVATE (2nd test): calls original method (C)

With this commit, this changes to:

    A) START: original method
    B) ACTIVATE (1st test): method is doubled
    C) DEACTIVATE (1st test): calls original method (A)
    D) ACTIVATE (2nd test): original method not stored again; method is
                            again doubled
    E) DEACTIVATE (2nd test): calls original method (A)

lib/shoulda/matchers/doublespeak/double.rb

@@ -5,7 +5,8 @@ module Shoulda
       class Double
         attr_reader :calls
 
-        def initialize(klass, method_name, implementation)
+        def initialize(world, klass, method_name, implementation)
+          @world = world
           @klass = klass
           @method_name = method_name
           @implementation = implementation
@@ -41,17 +42,20 @@ module Shoulda
         end
 
         def call_original_method(call)
-          if original_method
-            original_method.bind(call.object).call(*call.args, &call.block)
+          unbound_method = world.original_method_for(klass, call.method_name)
+
+          if unbound_method
+            unbound_method.bind(call.object).call(*call.args, &call.block)
           end
         end
 
         protected
 
-        attr_reader :klass, :method_name, :implementation, :original_method
+        attr_reader :world, :klass, :method_name, :implementation,
+          :original_method
 
         def store_original_method
-          @original_method = klass.instance_method(method_name)
+          world.store_original_method_for(klass, method_name)
         end
 
         def replace_method_with_double
@@ -72,8 +76,10 @@ module Shoulda
         end
 
         def restore_original_method
-          original_method = @original_method
+          original_method = world.original_method_for(klass, method_name)
+
           klass.__send__(:remove_method, method_name)
+
           klass.__send__(:define_method, method_name) do |*args, &block|
             original_method.bind(self).call(*args, &block)
           end

lib/shoulda/matchers/doublespeak/double_collection.rb

@@ -3,7 +3,8 @@ module Shoulda
     module Doublespeak
       # @private
       class DoubleCollection
-        def initialize(klass)
+        def initialize(world, klass)
+          @world = world
           @klass = klass
           @doubles_by_method_name = {}
         end
@@ -40,12 +41,12 @@ module Shoulda
 
         protected
 
-        attr_reader :klass, :doubles_by_method_name
+        attr_reader :world, :klass, :doubles_by_method_name
 
         def register_double(method_name, implementation_type)
           implementation =
             DoubleImplementationRegistry.find(implementation_type)
-          double = Double.new(klass, method_name, implementation)
+          double = Double.new(world, klass, method_name, implementation)
           doubles_by_method_name[method_name] = double
           double
         end

lib/shoulda/matchers/doublespeak/world.rb

@@ -4,7 +4,19 @@ module Shoulda
       # @private
       class World
         def double_collection_for(klass)
-          double_collections_by_class[klass] ||= DoubleCollection.new(klass)
+          double_collections_by_class[klass] ||=
+            DoubleCollection.new(self, klass)
+        end
+
+        def store_original_method_for(klass, method_name)
+          original_methods_for_class(klass)[method_name] ||=
+            klass.instance_method(method_name)
+        end
+
+        def original_method_for(klass, method_name)
+          if original_methods_by_class.key?(klass)
+            original_methods_by_class[klass][method_name]
+          end
         end
 
         def with_doubles_activated
@@ -31,6 +43,14 @@ module Shoulda
         def double_collections_by_class
           @_double_collections_by_class ||= {}
         end
+
+        def original_methods_by_class
+          @_original_methods_by_class ||= {}
+        end
+
+        def original_methods_for_class(klass)
+          original_methods_by_class[klass] ||= {}
+        end
       end
     end
   end

spec/unit/shoulda/matchers/doublespeak/double_collection_spec.rb

@@ -5,7 +5,7 @@ module Shoulda::Matchers::Doublespeak
     describe '#register_stub' do
       it 'calls DoubleImplementationRegistry.find correctly' do
         allow(DoubleImplementationRegistry).to receive(:find)
-        double_collection = described_class.new(:klass)
+        double_collection = described_class.new(build_world, :klass)
 
         double_collection.register_stub(:a_method)
 
@@ -13,24 +13,25 @@ module Shoulda::Matchers::Doublespeak
       end
 
       it 'calls Double.new correctly' do
+        world = build_world
         allow(DoubleImplementationRegistry).
           to receive(:find).
           and_return(:implementation)
         allow(Double).to receive(:new)
-        double_collection = described_class.new(:klass)
+        double_collection = described_class.new(world, :klass)
 
         double_collection.register_stub(:a_method)
 
         expect(Double).
           to have_received(:new).
-          with(:klass, :a_method, :implementation)
+          with(world, :klass, :a_method, :implementation)
       end
     end
 
     describe '#register_proxy' do
       it 'calls DoubleImplementationRegistry.find correctly' do
         allow(DoubleImplementationRegistry).to receive(:find)
-        double_collection = described_class.new(:klass)
+        double_collection = described_class.new(build_world, :klass)
 
         double_collection.register_proxy(:a_method)
 
@@ -40,24 +41,25 @@ module Shoulda::Matchers::Doublespeak
       end
 
       it 'calls Double.new correctly' do
+        world = build_world
         allow(DoubleImplementationRegistry).
           to receive(:find).
           and_return(:implementation)
         allow(Double).to receive(:new)
-        double_collection = described_class.new(:klass)
+        double_collection = described_class.new(world, :klass)
 
         double_collection.register_proxy(:a_method)
 
         expect(Double).
           to have_received(:new).
-          with(:klass, :a_method, :implementation)
+          with(world, :klass, :a_method, :implementation)
       end
     end
 
     describe '#activate' do
       it 'replaces all registered methods with doubles' do
         klass = create_class(first_method: 1, second_method: 2)
-        double_collection = described_class.new(klass)
+        double_collection = described_class.new(build_world, klass)
         double_collection.register_stub(:first_method)
         double_collection.register_stub(:second_method)
 
@@ -72,7 +74,7 @@ module Shoulda::Matchers::Doublespeak
     describe '#deactivate' do
       it 'restores the original methods that were doubled' do
         klass = create_class(first_method: 1, second_method: 2)
-        double_collection = described_class.new(klass)
+        double_collection = described_class.new(build_world, klass)
         double_collection.register_stub(:first_method)
         double_collection.register_stub(:second_method)
 
@@ -88,7 +90,7 @@ module Shoulda::Matchers::Doublespeak
     describe '#calls_to' do
       it 'returns all calls to the given method' do
         klass = create_class(a_method: nil)
-        double_collection = described_class.new(klass)
+        double_collection = described_class.new(build_world, klass)
         double_collection.register_stub(:a_method)
         double_collection.activate
 
@@ -108,7 +110,7 @@ module Shoulda::Matchers::Doublespeak
 
       it 'returns an empty array if the method has never been doubled' do
         klass = create_class
-        double_collection = described_class.new(klass)
+        double_collection = described_class.new(build_world, klass)
         expect(double_collection.calls_to(:non_existent_method)).to eq []
       end
     end
@@ -120,5 +122,9 @@ module Shoulda::Matchers::Doublespeak
         end
       end
     end
+
+    def build_world
+      Shoulda::Matchers::Doublespeak::World.new
+    end
   end
 end

spec/unit/shoulda/matchers/doublespeak/double_spec.rb

@@ -11,14 +11,24 @@ module Shoulda::Matchers::Doublespeak
         implementation.singleton_class.__send__(:define_method, :returns) do |&block|
           actual_block = block
         end
-        double = described_class.new(:klass, :a_method, implementation)
+        double = described_class.new(
+          :a_world,
+          :klass,
+          :a_method,
+          implementation
+        )
         double.to_return(&sent_block)
         expect(actual_block).to eq sent_block
       end
 
       it 'tells its implementation to return the given value' do
         implementation = build_implementation
-        double = described_class.new(:klass, :a_method, implementation)
+        double = described_class.new(
+          :a_world,
+          :klass,
+          :a_method,
+          implementation
+        )
         double.to_return(:implementation)
 
         expect(implementation).to have_received(:returns).with(:implementation)
@@ -32,7 +42,12 @@ module Shoulda::Matchers::Doublespeak
         implementation.singleton_class.__send__(:define_method, :returns) do |&block|
           actual_block = block
         end
-        double = described_class.new(:klass, :a_method, implementation)
+        double = described_class.new(
+          :a_world,
+          :klass,
+          :a_method,
+          implementation
+        )
         double.to_return(:value, &sent_block)
         expect(actual_block).to eq sent_block
       end
@@ -44,7 +59,12 @@ module Shoulda::Matchers::Doublespeak
         method_name = :a_method
         klass = create_class(method_name => :some_return_value)
         instance = klass.new
-        double = described_class.new(klass, method_name, implementation)
+        double = described_class.new(
+          build_world,
+          klass,
+          method_name,
+          implementation
+        )
         args = [:any, :args]
         block = -> {}
         call = MethodCall.new(
@@ -66,10 +86,17 @@ module Shoulda::Matchers::Doublespeak
 
     describe '#deactivate' do
       it 'restores the original method after being doubled' do
-        implementation = build_implementation
         klass = create_class(a_method: 42)
+        world = build_world(
+          original_method_for: klass.instance_method(:a_method)
+        )
         instance = klass.new
-        double = described_class.new(klass, :a_method, implementation)
+        double = described_class.new(
+          world,
+          klass,
+          :a_method,
+          build_implementation
+        )
 
         double.activate
         double.deactivate
@@ -77,10 +104,18 @@ module Shoulda::Matchers::Doublespeak
       end
 
       it 'still restores the original method if #activate was called twice' do
-        implementation = build_implementation
-        klass = create_class(a_method: 42)
+        method_name = :a_method
+        klass = create_class(method_name => 42)
+        world = build_world(
+          original_method_for: klass.instance_method(:a_method)
+        )
         instance = klass.new
-        double = described_class.new(klass, :a_method, implementation)
+        double = described_class.new(
+          world,
+          klass,
+          :a_method,
+          build_implementation
+        )
 
         double.activate
         double.activate
@@ -89,10 +124,14 @@ module Shoulda::Matchers::Doublespeak
       end
 
       it 'does nothing if the method has not been doubled' do
-        implementation = build_implementation
         klass = create_class(a_method: 42)
         instance = klass.new
-        double = described_class.new(klass, :a_method, implementation)
+        double = described_class.new(
+          :a_world,
+          klass,
+          :a_method,
+          build_implementation
+        )
 
         double.deactivate
         expect(instance.a_method).to eq 42
@@ -101,7 +140,12 @@ module Shoulda::Matchers::Doublespeak
 
     describe '#record_call' do
       it 'adds the given call to the list of calls' do
-        double = described_class.new(:a_klass, :a_method, :an_implementation)
+        double = described_class.new(
+          :a_world,
+          :a_klass,
+          :a_method,
+          :an_implementation
+        )
         double.record_call(:some_call)
         expect(double.calls.last).to eq :some_call
       end
@@ -109,35 +153,82 @@ module Shoulda::Matchers::Doublespeak
 
     describe '#call_original_method' do
       it 'binds the stored method object to the given object and calls it with the given args and block' do
-        klass = create_class
-        instance = klass.new
-        actual_args = actual_block = method_called = nil
         expected_args = [:one, :two, :three]
         expected_block = -> { }
+        actual_args = actual_block = method_called = nil
+        method_name = :a_method
+        klass = create_class
+        klass.__send__(:define_method, method_name) do |*args, &block|
+          actual_args = args
+          actual_block = block
+          method_called = true
+        end
+        world = build_world(
+          original_method_for: klass.instance_method(method_name)
+        )
+        instance = klass.new
         call = double('call',
           object: instance,
+          method_name: method_name,
           args: expected_args,
           block: expected_block
         )
-        double = described_class.new(klass, :a_method, :an_implementation)
-
-        klass.__send__(:define_method, :a_method) do |*args, &block|
-          actual_args = expected_args
-          actual_block = expected_block
-          method_called = true
-        end
+        double = described_class.new(
+          world,
+          klass,
+          method_name,
+          :an_implementation
+        )
 
         double.activate
         double.call_original_method(call)
 
-        expect(expected_args).to eq actual_args
-        expect(expected_block).to eq actual_block
+        expect(actual_args).to eq expected_args
+        expect(actual_block).to eq expected_block
         expect(method_called).to eq true
       end
 
       it 'does nothing if no method has been stored' do
-        double = described_class.new(:klass, :a_method, :an_implementation)
-        expect { double.call_original_method(:a_call) }.not_to raise_error
+        method_name = :a_method
+        world = build_world(original_method_for: nil)
+        call = double('call', method_name: method_name)
+        double = described_class.new(
+          world,
+          :klass,
+          method_name,
+          :an_implementation
+        )
+        expect { double.call_original_method(call) }.not_to raise_error
+      end
+
+      it 'does not store the original method multiple times when a method is doubled multiple times' do
+        world = Shoulda::Matchers::Doublespeak::World.new
+        klass = create_class(a_method: :some_return_value)
+        method_name = :a_method
+        doubles = 2.times.map do
+          described_class.new(
+            world,
+            klass,
+            method_name,
+            build_implementation
+          )
+        end
+        instance = klass.new
+
+        doubles[0].activate
+
+        was_called = false
+        klass.__send__(:define_method, method_name) do
+          was_called = true
+        end
+
+        doubles[1].activate
+
+        doubles.each(&:deactivate)
+
+        instance.__send__(method_name)
+
+        expect(was_called).to be false
       end
     end
 
@@ -152,5 +243,13 @@ module Shoulda::Matchers::Doublespeak
     def build_implementation
       double('implementation', returns: nil, call: nil)
     end
+
+    def build_world(methods = {})
+      defaults = {
+        original_method_for: nil,
+        store_original_method_for: nil
+      }
+      double('world', defaults.merge(methods))
+    end
   end
 end

spec/unit/shoulda/matchers/doublespeak/world_spec.rb

@@ -10,16 +10,19 @@ module Shoulda::Matchers::Doublespeak
         world.double_collection_for(:klass)
         world.double_collection_for(:klass)
 
-        expect(DoubleCollection).to have_received(:new).with(:klass).once
+        expect(DoubleCollection).
+          to have_received(:new).
+          with(world, :klass).
+          once
       end
 
       it 'returns the created DoubleCollection' do
+        world = described_class.new
         double_collection = build_double_collection
         allow(DoubleCollection).
           to receive(:new).
-          with(:klass).
+          with(world, :klass).
           and_return(double_collection)
-        world = described_class.new
 
         expect(world.double_collection_for(:klass)).to be double_collection
       end
@@ -40,7 +43,7 @@ module Shoulda::Matchers::Doublespeak
         double_collections.zip(klasses).each do |double_collection, klass|
           allow(DoubleCollection).
             to receive(:new).
-            with(klass).
+            with(world, klass).
             and_return(double_collection)
           world.double_collection_for(klass)
         end