BugFix: Ensure callbacks only run once per instance. (#1712)

CallbacksObserver refactored to ignore an individual
callback that has already been run on the given instance.

EXAMPLE

factory :user do
  name { "Jane" }

  trait :trait_a do
    trait_b
  end

  trait :trait_b do
    after(:build) { |user| user.name += " Doe" }
  end
end

user = build(:user, :trait_a, :trait_b)

BEFORE:
user.name #=> "Jane Doe Doe"

AFTER:
user.name #=> "Jane Doe"

Author: CodeMeister

lib/factory_bot/callbacks_observer.rb

@@ -4,11 +4,15 @@ class CallbacksObserver
     def initialize(callbacks, evaluator)
       @callbacks = callbacks
       @evaluator = evaluator
+      @completed = []
     end
 
     def update(name, result_instance)
       callbacks_by_name(name).each do |callback|
-        callback.run(result_instance, @evaluator)
+        if !completed?(result_instance, callback)
+          callback.run(result_instance, @evaluator)
+          record_completion!(result_instance, callback)
+        end
       end
     end
 
@@ -17,5 +21,19 @@ def update(name, result_instance)
     def callbacks_by_name(name)
       @callbacks.select { |callback| callback.name == name }
     end
+
+    def completed?(instance, callback)
+      key = completion_key_for(instance, callback)
+      @completed.include?(key)
+    end
+
+    def record_completion!(instance, callback)
+      key = completion_key_for(instance, callback)
+      @completed << key
+    end
+
+    def completion_key_for(instance, callback)
+      "#{instance.object_id}-#{callback.object_id}"
+    end
   end
 end

spec/acceptance/callbacks_spec.rb

@@ -16,6 +16,15 @@
           after(:stub) { |user| user.last_name = "Double-Stubby" }
           after(:build) { |user| user.first_name = "Child-Buildy" }
         end
+
+        factory :user_with_multi_called_callbacks, class: :user do
+          first_name { "Jane" }
+          trait(:alias_2) { alias_1 }
+          trait(:alias_1) { surname }
+          trait :surname do
+            after(:build) { |user| user.first_name += " Doe" }
+          end
+        end
       end
     end
 
@@ -45,6 +54,15 @@
       user = FactoryBot.build(:user_with_inherited_callbacks)
       expect(user.first_name).to eq "Child-Buildy"
     end
+
+    it "only runs each callback once per instance" do
+      user_1 = FactoryBot.build(:user_with_multi_called_callbacks, :surname, :alias_1, :alias_2)
+      user_2 = FactoryBot.build(:user_with_multi_called_callbacks, :alias_1, :alias_2, :surname)
+      user_3 = FactoryBot.build(:user_with_multi_called_callbacks, :alias_2, :surname, :alias_1)
+      expect(user_1.first_name).to eq "Jane Doe"
+      expect(user_2.first_name).to eq "Jane Doe"
+      expect(user_3.first_name).to eq "Jane Doe"
+    end
   end # with strategy callbacks
 
   context "with before(:all) and after(:all) included" do