Merge pull request rails#35969 from shioyama/avoid_anonymous_module_inclusion_in_acceptance_validator

Avoid LazilyDefinedAttributes being included multiple times in class, and remove methods after first use

Author: kamipo

activemodel/lib/active_model/validations/acceptance.rb

@@ -17,53 +17,66 @@ def validate_each(record, attribute, value)
       private
 
         def setup!(klass)
-          klass.include(LazilyDefineAttributes.new(AttributeDefinition.new(attributes)))
+          klass.include(LazilyDefineAttributes.new(klass)) unless klass.ancestors.any?(LazilyDefineAttributes)
         end
 
         def acceptable_option?(value)
           Array(options[:accept]).include?(value)
         end
 
         class LazilyDefineAttributes < Module
-          def initialize(attribute_definition)
+          attr_reader :removed
+
+          def initialize(klass)
+            @klass = klass
+            @removed = false
+            mod = self
+
             define_method(:respond_to_missing?) do |method_name, include_private = false|
-              super(method_name, include_private) || attribute_definition.matches?(method_name)
+              mod.define_attributes
+              super(method_name, include_private) || mod.matches?(method_name)
             end
 
             define_method(:method_missing) do |method_name, *args, &block|
-              if attribute_definition.matches?(method_name)
-                attribute_definition.define_on(self.class)
+              mod.define_attributes
+              if mod.matches?(method_name)
                 send(method_name, *args, &block)
               else
                 super(method_name, *args, &block)
               end
             end
           end
-        end
-
-        class AttributeDefinition
-          def initialize(attributes)
-            @attributes = attributes.map(&:to_s)
-          end
 
           def matches?(method_name)
-            attr_name = convert_to_reader_name(method_name)
-            attributes.include?(attr_name)
+            attr_name = method_name.to_s.chomp("=")
+            attributes.any? { |name| name.to_s == attr_name }
           end
 
-          def define_on(klass)
+          def define_attributes
             attr_readers = attributes.reject { |name| klass.attribute_method?(name) }
             attr_writers = attributes.reject { |name| klass.attribute_method?("#{name}=") }
-            klass.define_attribute_methods
-            klass.attr_reader(*attr_readers)
-            klass.attr_writer(*attr_writers)
+
+            remove_methods
+
+            attr_reader(*attr_readers)
+            attr_writer(*attr_writers)
+          end
+
+          def self.===(other)
+            super && !other.removed
           end
 
           private
-            attr_reader :attributes
+            attr_reader :klass
+
+            def attributes
+              @attributes ||= klass.validators.grep(AcceptanceValidator).flat_map(&:attributes).uniq
+            end
 
-            def convert_to_reader_name(method_name)
-              method_name.to_s.chomp("=")
+            def remove_methods
+              remove_method :respond_to_missing?
+              remove_method :method_missing
+              @removed = true
             end
         end
     end

activemodel/test/cases/validations/acceptance_validation_test.rb

@@ -91,11 +91,35 @@ def test_validates_acceptance_of_true
     assert_predicate klass.new(terms_of_service: true), :valid?
   end
 
-  private
+  def test_lazy_attribute_module_included_only_once
+    klass = define_test_class(Topic)
+    assert_difference -> { klass.ancestors.count }, 1 do
+      2.times { klass.validates_acceptance_of(:something_to_accept) }
+      2.times { klass.validates_acceptance_of(:something_else_to_accept) }
+    end
+  end
+
+  def test_lazy_attributes_module_included_again_if_needed
+    klass = define_test_class(Topic)
+    assert_difference -> { klass.ancestors.count }, 1 do
+      klass.validates_acceptance_of(:something_to_accept)
+    end
+    topic = klass.new
+    topic.something_to_accept
+    assert_difference -> { klass.ancestors.count }, 1 do
+      klass.validates_acceptance_of(:something_else_to_accept)
+    end
+    assert topic.respond_to?(:something_else_to_accept)
+  end
 
-    # Acceptance validator includes anonymous module into class, which cannot
-    # be cleared, so to avoid multiple inclusions we use a named subclass which
-    # we can remove in teardown.
+  def test_lazy_attributes_respond_to?
+    klass = define_test_class(Topic)
+    klass.validates_acceptance_of(:terms_of_service)
+    topic = klass.new
+    assert topic.respond_to?(:terms_of_service)
+  end
+
+  private
     def define_test_class(parent)
       self.class.const_set(:TestClass, Class.new(parent))
     end