Ticket #3238: activerecord_monitor_changes_fixed.diff

activerecord/test/attribute_methods_test.rb

@@ -139 +139 @@
-    end
-  end
-
+  def test_attribute_changed_without_inplace_modifications
+    # Turn off in place modifications  
+    Topic.track_inplace_modifications = false 
+  
+    # Test for mass attribute and creation 
+    topic = Topic.new(:title => "Test Topic", :content => "Is this thing on?",  
+                      :author_name => "Loren", :author_email_address => "lsegal@holycraplions.com") 
+    assert topic.title_changed? 
+    assert topic.author_email_address_changed? 
+    assert_equal 4, topic.changed_attributes.size 
+    topic.save 
+    assert !topic.attribute_changed?(:title) 
+    assert_equal 0, topic.changed_attributes.size 
+     
+    # Make sure inplace modifications don't log changes here 
+    topic.title << " - Some other stuff" 
+    assert_equal "Test Topic - Some other stuff", topic.title 
+    assert !topic.title_changed? 
+     
+    # Test for an update 
+    topic.title = "We changed the topic" 
+    assert topic.title_changed? 
+    assert_equal ["title"], topic.changed_attributes 
+    topic.save 
+    assert !topic.title_changed? 
+    assert_equal 0, topic.changed_attributes.size 
+     
+    # Test for an existing record update 
+    topic = Topic.find(:first) 
+    assert !topic.content_changed? 
+    assert_equal 0, topic.changed_attributes.size 
+    topic.content = "We changed the content this time." 
+    assert topic.content_changed? 
+    assert_equal ["content"], topic.changed_attributes 
+    topic.save 
+    assert !topic.content_changed? 
+    assert_equal 0, topic.changed_attributes.size 
+  
+    # Turn default behaviour back on 
+    Topic.track_inplace_modifications = true 
+  end
+  
+  def test_reset_changed_attributes 
+    topic = Topic.find(:first) 
+    topic.author_name = "Johnny" 
+    assert topic.author_name_changed? 
+    assert_equal ["author_name"], topic.changed_attributes 
+     
+    # Reload and check if attributes reset 
+    topic.reload 
+    assert !topic.author_name_changed? 
+  end 
+   
+  def test_attribute_changed_with_inplace_modifications 
+    # Test for mass attribute and creation 
+    topic = Topic.new(:title => "Test Topic", :content => "Is this thing on?",  
+                      :author_name => "FooBar", :author_email_address => "foo@bar.com") 
+
+    assert topic.class.track_inplace_modifications 
+                 
+    assert topic.title_changed? 
+    assert topic.content_changed? 
+    assert topic.author_name_changed? 
+    assert topic.author_email_address_changed? 
+    topic.save
+    assert !topic.title_changed? 
+    assert_equal 0, topic.changed_attributes.size 
+     
+    # Test for an update
+    topic.title << " - No, really." 
+    assert topic.title_changed? 
+    assert_equal ["title"], topic.changed_attributes 
+    topic.save 
+    assert !topic.title_changed? 
+    assert_equal 0, topic.changed_attributes.size
+     
+    # Test for an existing record update 
+    topic = Topic.find(:first) 
+    assert topic.track_inplace_modifications 
+    assert !topic.content_changed? 
+    assert_equal 0, topic.changed_attributes.size 
+    topic.content.gsub!(/./, '') 
+    assert topic.content_changed? 
+    assert_equal ["content"], topic.changed_attributes 
+    topic.save 
+    assert !topic.content_changed? 
+    assert_equal 0, topic.changed_attributes.size     
+  end
+
-  private
-  def time_related_columns_on_topic
-    Topic.columns.select{|c| [:time, :date, :datetime, :timestamp].include?(c.type)}.map(&:name)

activerecord/lib/active_record/attribute_methods.rb

@@ -76 +76 @@
-          unless instance_method_already_implemented?("#{name}?")
-            define_question_method(name)
-          end
+          
+          unless instance_method_already_implemented?("#{name}_changed?")
+            define_changed_method(name)
+          end
-        end
-      end
-
@@ -149 +153 @@
-        def define_write_method(attr_name)
-          evaluate_attribute_method attr_name, "def #{attr_name}=(new_value);write_attribute('#{attr_name}', new_value);end", "#{attr_name}="
-        end
-
+
+
+
+
+
-        # Evaluate the definition for an attribute related method
-        def evaluate_attribute_method(attr_name, method_definition, method_name=attr_name)
-
-          unless method_name.to_s == primary_key.to_s
-            generated_methods << method_name
-          end
@@ -211 +218 @@
-    # "2004-12-12" in a data column is cast to a date object, like Date.new(2004, 12, 12)).
-    def read_attribute(attr_name)
-      attr_name = attr_name.to_s
+      track_attribute(attr_name.to_s)
+
-      if !(value = @attributes[attr_name]).nil?
-        if column = column_for_attribute(attr_name)
-          if unserializable_attribute?(attr_name, column)
@@ -227 +236 @@
-    end
-
-    def read_attribute_before_type_cast(attr_name)
-
+
+
-    end
-
-    # Returns true if the attribute is of a text column and marked for serialization.
@@ -258 +268 @@
-      else
-        @attributes[attr_name] = value
-      end
+      
+      @attributes_changed[attr_name] = true unless @attributes_changed.frozen?
-    end
-
-
@@ -325 +337 @@
-      def attribute_before_type_cast(attribute_name)
-        read_attribute_before_type_cast(attribute_name)
-      end
+      
+      # Handle *_changed? for method_missing
+      def attribute_changed?(attribute_name)
+        attribute_changed?(attribute_name)
+      end
-  end
-end

activerecord/lib/active_record/base.rb

@@ -419 +419 @@
-    # Defaults to false. Set to true if you're writing a threaded application.
-    cattr_accessor :allow_concurrency, :instance_writer => false
-    @@allow_concurrency = false
+    
+    # Determines whether or not to track in place modifications of attributes 
+    # Turning this feature on will raise memory usage of each ActiveRecord object 
+    # and take a performance hit for each attribute read, but will allow for 
+    # more ruby-like syntax such as: 
+    #    
+    #   t = Topic.find_first 
+    #   t.title << " Extra information" 
+    #   t.title.gsub!('Extra', '') 
+    # 
+    # This feature can be left off and you will still have the ability to use the above  
+    # code, but you will not be able to guarantee proper results from attribute_changed? 
+    # and can cause problems with record saving. The default value is true. 
+    cattr_accessor :track_inplace_modifications 
+    @@track_inplace_modifications = true
-
+
-    # Specifies the format to use when dumping the database schema with Rails'
-    # Rakefile.  If :sql, the schema is dumped as (potentially database-
-    # specific) SQL statements.  If :ruby, the schema is dumped as an
@@ -1331 +1347 @@
-            end
-
-          object.instance_variable_set("@attributes", record)
+          object.instance_eval("reset_changed_attributes") 
-          object.instance_variable_set("@attributes_cache", Hash.new)
-
-          if object.respond_to_without_attributes?(:after_find)
@@ -1928 +1945 @@
-        @attributes = attributes_from_column_definition
-        @attributes_cache = {}
-        @new_record = true
+        reset_changed_attributes
-        ensure_proper_type
-        self.attributes = attributes unless attributes.nil?
-        self.class.send(:scope, :create).each { |att,value| self.send("#{att}=", value) } if self.class.send(:scoped?, :create)
@@ -1975 +1993 @@
-      # * No record exists: Creates a new record with values matching those of the object attributes.
-      # * A record does exist: Updates the record with values matching those of the object attributes.
-      def save
+        freeze_changed_attributes
-        create_or_update
+        reset_changed_attributes
-      end
-
-      # Attempts to save the record, but instead of just returning false if it couldn't happen, it raises a
-      # RecordNotSaved exception
-      def save!
+        freeze_changed_attributes
-        create_or_update || raise(RecordNotSaved)
+        reset_changed_attributes
-      end
-
-      # Deletes the record in the database and freezes this instance to reflect that no changes should
@@ -2088 +2110 @@
-      def reload(options = nil)
-        clear_aggregation_cache
-        clear_association_cache
+        reset_changed_attributes
-        @attributes.update(self.class.find(self.id, options).instance_variable_get('@attributes'))
-        @attributes_cache = {}
-        self
@@ -2178 +2201 @@
-        @attributes.has_key?(attr_name.to_s)
-      end
-
+      # Returns true if the given attribute has been modified 
+      def attribute_changed?(attr_name)
+        attr_name = attr_name.to_s unless attr_name.is_a?(String) 
+        if @attributes_changed[attr_name].kind_of?(TrueClass) 
+          true  
+        elsif @attributes_changed.has_key?(attr_name) 
+          @attributes_changed[attr_name] != @attributes[attr_name] 
+        else 
+          false 
+        end 
+      end 
+        
+      # Returns an array of attributes modified since last database read or save.
+      def changed_attributes 
+        @attributes_changed.keys.collect {|key| attribute_changed?(key) ? key : nil }.compact 
+      end
+      
-      # Returns an array of names for the attributes available on this object sorted alphabetically.
-      def attribute_names
-        @attributes.keys.sort
@@ -2248 +2288 @@
-      # Updates the associated record with values matching those of the instance attributes.
-      # Returns the number of affected rows.
-      def update
-
+
+
-        return 0 if quoted_attributes.empty?
-        connection.update(
-          "UPDATE #{self.class.quoted_table_name} " +
-
+ 
-          "WHERE #{connection.quote_column_name(self.class.primary_key)} = #{quote_value(id)}",
-          "#{self.class.name} Update"
-        )
@@ -2337 +2378 @@
-        default << 'id' unless self.class.primary_key.eql? 'id'
-        default
-      end
+      
+      # Resets attributes that have been modified since last database access or save.
+      def reset_changed_attributes 
+        @attributes_changed = {}
+      end 
+       
+      # Freeze the changed attributes before a save so successive reads won't alter data before 
+      # it goes into the table. 
+      def freeze_changed_attributes 
+        @attributes_changed.freeze 
+      end 
+       
+      # Keeps track of attributes when a value is read for pre-emptive caching of data  
+      # so that we can monitor if the value was changed, but only when track_inplace_modifications 
+      # is on.
+      def track_attribute(attr_name) 
+        return if @attributes_changed.frozen? || !track_inplace_modifications    
+        attr_name = attr_name.to_s unless attr_name.is_a? String 
+        @attributes_changed[attr_name] = calculate_attribute_hash(attr_name)
+      end 
+       
+      # Calculates the hash value for the attribute's data. Only used when track_inplace_modifications 
+      # is on. The current scheme uses the cloned attribute as a 'hash' 
+      def calculate_attribute_hash(attr_name) 
+        @attributes[attr_name.to_s].to_s.clone 
+      end
-
-      # Returns a copy of the attributes hash where all the values have been safely quoted for use in
-
-
+
+
+
-        quoted = attributes.inject({}) do |result, (name, value)|
-
-
+
+
+
+
-          end
-          result
-        end