From 4bf8ffc6516312e68fb0d2b4ac97505f8d0cf192 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Thu, 12 Jun 2014 15:13:28 -0600 Subject: Detect in-place changes on mutable AR attributes We have several mutable types on Active Record now. (Serialized, JSON, HStore). We need to be able to detect if these have been modified in place. --- .../lib/active_record/attribute_methods/dirty.rb | 83 ++++++++++++++++++++-- .../attribute_methods/serialization.rb | 17 ----- .../connection_adapters/postgresql/oid/hstore.rb | 6 +- .../connection_adapters/postgresql/oid/json.rb | 6 +- activerecord/lib/active_record/type.rb | 1 + activerecord/lib/active_record/type/mutable.rb | 16 +++++ activerecord/lib/active_record/type/serialized.rb | 6 +- activerecord/lib/active_record/type/value.rb | 4 ++ 8 files changed, 104 insertions(+), 35 deletions(-) create mode 100644 activerecord/lib/active_record/type/mutable.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index be438aba95..6a5c057384 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -38,12 +38,39 @@ module ActiveRecord end end - def initialize_dup(other) # :nodoc: - super - init_changed_attributes - end + def initialize_dup(other) # :nodoc: + super + init_changed_attributes + end + + def changed? + super || changed_in_place.any? + end + + def changed + super | changed_in_place + end + + def attribute_changed?(attr_name, options = {}) + result = super + # We can't change "from" something in place. Only setters can define + # "from" and "to" + result ||= changed_in_place?(attr_name) unless options.key?(:from) + result + end + + def changes_applied + super + store_original_raw_attributes + end + + def reset_changes + super + original_raw_attributes.clear + end + + private - private def initialize_internals_callback super init_changed_attributes @@ -65,11 +92,20 @@ module ActiveRecord old_value = old_attribute_value(attr) - result = super(attr, value) + result = super + store_original_raw_attribute(attr) save_changed_attribute(attr, old_value) result end + def raw_write_attribute(attr, value) + attr = attr.to_s + + result = super + original_raw_attributes[attr] = value + result + end + def save_changed_attribute(attr, old_value) if attribute_changed?(attr) changed_attributes.delete(attr) unless _field_changed?(attr, old_value) @@ -105,6 +141,41 @@ module ActiveRecord raw_value = read_attribute_before_type_cast(attr) column_for_attribute(attr).changed?(old_value, new_value, raw_value) end + + def changed_in_place + self.class.attribute_names.select do |attr_name| + changed_in_place?(attr_name) + end + end + + def changed_in_place?(attr_name) + type = type_for_attribute(attr_name) + old_value = original_raw_attribute(attr_name) + value = read_attribute(attr_name) + type.changed_in_place?(old_value, value) + end + + def original_raw_attribute(attr_name) + original_raw_attributes.fetch(attr_name) do + read_attribute_before_type_cast(attr_name) + end + end + + def original_raw_attributes + @original_raw_attributes ||= {} + end + + def store_original_raw_attribute(attr_name) + type = type_for_attribute(attr_name) + value = type.type_cast_for_database(read_attribute(attr_name)) + original_raw_attributes[attr_name] = value + end + + def store_original_raw_attributes + attribute_names.each do |attr| + store_original_raw_attribute(attr) + end + end end end end diff --git a/activerecord/lib/active_record/attribute_methods/serialization.rb b/activerecord/lib/active_record/attribute_methods/serialization.rb index 60debb7d18..cec50f62a3 100644 --- a/activerecord/lib/active_record/attribute_methods/serialization.rb +++ b/activerecord/lib/active_record/attribute_methods/serialization.rb @@ -50,8 +50,6 @@ module ActiveRecord # serialize :preferences, Hash # end def serialize(attr_name, class_name_or_coder = Object) - include Behavior - coder = if [:load, :dump].all? { |x| class_name_or_coder.respond_to?(x) } class_name_or_coder else @@ -67,21 +65,6 @@ module ActiveRecord self.serialized_attributes = serialized_attributes.merge(attr_name.to_s => coder) end end - - - # This is only added to the model when serialize is called, which - # ensures we do not make things slower when serialization is not used. - module Behavior # :nodoc: - extend ActiveSupport::Concern - - def should_record_timestamps? - super || (self.record_timestamps && (attributes.keys & self.class.serialized_attributes.keys).present?) - end - - def keys_for_partial_write - super | (attributes.keys & self.class.serialized_attributes.keys) - end - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb index 88de816d4f..0dd4b65333 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb @@ -3,14 +3,12 @@ module ActiveRecord module PostgreSQL module OID # :nodoc: class Hstore < Type::Value + include Type::Mutable + def type :hstore end - def type_cast_from_user(value) - type_cast_from_database(type_cast_for_database(value)) - end - def type_cast_from_database(value) ConnectionAdapters::PostgreSQLColumn.string_to_hstore(value) end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb index b4fed1bcab..d1347c7bb5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb @@ -3,14 +3,12 @@ module ActiveRecord module PostgreSQL module OID # :nodoc: class Json < Type::Value + include Type::Mutable + def type :json end - def type_cast_from_user(value) - type_cast_from_database(type_cast_for_database(value)) - end - def type_cast_from_database(value) ConnectionAdapters::PostgreSQLColumn.string_to_json(value) end diff --git a/activerecord/lib/active_record/type.rb b/activerecord/lib/active_record/type.rb index e9b827886a..f1384e0bb2 100644 --- a/activerecord/lib/active_record/type.rb +++ b/activerecord/lib/active_record/type.rb @@ -1,3 +1,4 @@ +require 'active_record/type/mutable' require 'active_record/type/numeric' require 'active_record/type/time_value' require 'active_record/type/value' diff --git a/activerecord/lib/active_record/type/mutable.rb b/activerecord/lib/active_record/type/mutable.rb new file mode 100644 index 0000000000..64cf4b9b93 --- /dev/null +++ b/activerecord/lib/active_record/type/mutable.rb @@ -0,0 +1,16 @@ +module ActiveRecord + module Type + module Mutable + def type_cast_from_user(value) + type_cast_from_database(type_cast_for_database(value)) + end + + # +raw_old_value+ will be the `_before_type_cast` version of the + # value (likely a string). +new_value+ will be the current, type + # cast value. + def changed_in_place?(raw_old_value, new_value) # :nodoc: + raw_old_value != type_cast_for_database(new_value) + end + end + end +end diff --git a/activerecord/lib/active_record/type/serialized.rb b/activerecord/lib/active_record/type/serialized.rb index e6d84eadc0..ebde14634c 100644 --- a/activerecord/lib/active_record/type/serialized.rb +++ b/activerecord/lib/active_record/type/serialized.rb @@ -1,6 +1,8 @@ module ActiveRecord module Type class Serialized < SimpleDelegator # :nodoc: + include Mutable + attr_reader :subtype, :coder def initialize(subtype, coder) @@ -17,10 +19,6 @@ module ActiveRecord end end - def type_cast_from_user(value) - type_cast_from_database(type_cast_for_database(value)) - end - def type_cast_for_database(value) return if value.nil? unless is_default_value?(value) diff --git a/activerecord/lib/active_record/type/value.rb b/activerecord/lib/active_record/type/value.rb index efcdab1c0e..b34d1697cd 100644 --- a/activerecord/lib/active_record/type/value.rb +++ b/activerecord/lib/active_record/type/value.rb @@ -60,6 +60,10 @@ module ActiveRecord old_value != new_value end + def changed_in_place?(*) # :nodoc: + false + end + private # Takes an input from the database, or from attribute setters, # and casts it to a type appropriate for this object. This method -- cgit v1.2.3