Ticket #3238: changed_attributes2.patch

test/base_test.rb

@@ -1104 +1104 @@
-    developers = Developer.find(:all, :order => 'id')
-    assert_equal 10, developers.size
-  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")
+    topic.class.track_inplace_modifications = true
+    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
+
-  # FIXME: this test ought to run, but it needs to run sandboxed so that it
-  # doesn't b0rk the current test environment by undefing everything.
-  #

lib/active_record/base.rb

@@ -323 +323 @@
-    cattr_accessor :generate_read_methods
-    @@generate_read_methods = true
-    
+    # 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 
@@ -906 +921 @@
-            end
-
-          object.instance_variable_set("@attributes", record)
+          object.instance_eval("reset_changed_attributes")
-          object
-        end
-
@@ -1189 +1205 @@
-      def initialize(attributes = nil)
-        @attributes = attributes_from_column_definition
-        @new_record = true
+        reset_changed_attributes
-        ensure_proper_type
-        self.attributes = attributes unless attributes.nil?
-        yield self if block_given?
@@ -1228 +1245 @@
-      # * A record does exist: Updates the record with values matching those of the object attributes.
-      def save
-        raise ActiveRecord::ReadOnlyRecord if readonly?
+        freeze_changed_attributes
-        create_or_update
+        reset_changed_attributes
-      end
-
-      # Deletes the record in the database and freezes this instance to reflect that no changes should
@@ -1311 +1330 @@
-      def reload
-        clear_aggregation_cache
-        clear_association_cache
+        reset_changed_attributes
-        @attributes.update(self.class.find(self.id).instance_variable_get('@attributes'))
-        self
-      end
@@ -1365 +1385 @@
-      def has_attribute?(attr_name)
-        @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
@@ -1406 +1443 @@
-          return false if self.class.read_methods.include?(attr_name)
-        elsif @attributes.include?(method_name = method.to_s)
-          return true
-
+|_changed\?
-          return true if @attributes.include?(md.pre_match)
-        end
-        # super must be called at the end of the method, because the inherited respond_to?
@@ -1438 +1475 @@
-
-      # Updates the associated record with values matching those of the instance attributes.
-      def update
+        return true if changed_attributes.empty?
-        connection.update(
-          "UPDATE #{self.class.table_name} " +
-
+, true
-          "WHERE #{self.class.primary_key} = #{quote(id)}",
-          "#{self.class.name} Update"
-        )
@@ -1488 +1526 @@
-          read_attribute(method_name)
-        elsif self.class.primary_key.to_s == method_name
-          id
-
+|_changed\?
-          attribute_name, method_type = md.pre_match, md.to_s
-          if @attributes.include?(attribute_name)
-            case method_type
@@ -1498 +1536 @@
-                query_attribute(attribute_name)
-              when '_before_type_cast'
-                read_attribute_before_type_cast(attribute_name)
+              when '_changed?'
+                attribute_changed?(attribute_name)
-            end
-          else
-            super
@@ -1511 +1551 @@
-      # "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) if track_inplace_modifications
-        if !(value = @attributes[attr_name]).nil?
-          if column = column_for_attribute(attr_name)
-            if unserializable_attribute?(attr_name, column)
@@ -1527 +1568 @@
-      end
-
-      def read_attribute_before_type_cast(attr_name)
+        track_attribute(attr_name) if track_inplace_modifications
-        @attributes[attr_name]
-      end
-
@@ -1552 +1594 @@
-        end
-
-        begin
-
+
+
-        rescue SyntaxError => err
-          self.class.read_methods.delete(attr_name)
-          if logger
@@ -1589 +1632 @@
-        else
-          @attributes[attr_name] = value
-        end
+        @attributes_changed[attr_name] = true unless @attributes_changed.frozen?
-      end
-
-      def convert_number_column_value(value)
@@ -1637 +1681 @@
-        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 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 copy of the attributes hash where all the values have been safely quoted for use in
-
-
+
+
+
-        attributes.inject({}) do |quoted, (name, value)|
-
-
+
+
+
+
-          end
-          quoted
-        end