Permalink
Browse files

Further encapsulate dirty checking on `Attribute`

We can skip the allocation of a full `AttributeSet` by changing the
semantics of how we structure things. Instead of comparing two separate
`AttributeSet` objects, and `Attribute` is now a singly linked list of
every change that has happened to it. Since the attribute objects are
immutable, to apply the changes we simply need to copy the head of the
list.

It's worth noting that this causes one subtle change in the behavior of
AR. When a record is saved successfully, the `before_type_cast` version
of everything will be what was sent to the database. I honestly think
these semantics make more sense, as we could have just as easily had the
DB do `RETURNING *` and updated the record with those if we had things
like timestamps implemented at the DB layer.

This brings our performance closer to 4.2, but we're still not quite
there.
  • Loading branch information...
sgrif committed Sep 28, 2015
1 parent 9db73a2 commit 07723c23a7dc570beae73c074ad37227e3e8a06e
@@ -5,8 +5,8 @@ def from_database(name, value, type)
FromDatabase.new(name, value, type)
end

def from_user(name, value, type)
FromUser.new(name, value, type)
def from_user(name, value, type, original_attribute = nil)
FromUser.new(name, value, type, original_attribute)
end

def with_cast_value(name, value, type)
@@ -26,10 +26,11 @@ def uninitialized(name, type)

# This method should not be called directly.
# Use #from_database or #from_user
def initialize(name, value_before_type_cast, type)
def initialize(name, value_before_type_cast, type, original_attribute = nil)
@name = name
@value_before_type_cast = value_before_type_cast
@type = type
@original_attribute = original_attribute
end

def value
@@ -38,21 +39,33 @@ def value
@value
end

def original_value
if assigned?
original_attribute.original_value
else
type_cast(value_before_type_cast)
end
end

def value_for_database
type.serialize(value)
end

def changed_from?(old_value)
type.changed?(old_value, value, value_before_type_cast)
def changed?
changed_from_assignment? || changed_in_place?
end

def changed_in_place?
has_been_read? && type.changed_in_place?(original_value_for_database, value)
end

def changed_in_place_from?(old_value)
has_been_read? && type.changed_in_place?(old_value, value)
def forgetting_assignment
with_value_from_database(value_for_database)
end

def with_value_from_user(value)
type.assert_valid_value(value)
self.class.from_user(name, value, type)
self.class.from_user(name, value, type, self)
end

def with_value_from_database(value)
@@ -64,7 +77,7 @@ def with_cast_value(value)
end

def with_type(type)
self.class.new(name, value_before_type_cast, type)
self.class.new(name, value_before_type_cast, type, original_attribute)
end

def type_cast(*)
@@ -97,16 +110,39 @@ def hash

protected

attr_reader :original_attribute
alias_method :assigned?, :original_attribute

def initialize_dup(other)
if defined?(@value) && @value.duplicable?
@value = @value.dup
end
end

def changed_from_assignment?
assigned? && type.changed?(original_value, value, value_before_type_cast)
end

def original_value_for_database
if assigned?
original_attribute.original_value_for_database
else
_original_value_for_database
end
end

def _original_value_for_database
value_for_database
end

class FromDatabase < Attribute # :nodoc:
def type_cast(value)
type.deserialize(value)
end

def _original_value_for_database
value_before_type_cast
end
end

class FromUser < Attribute # :nodoc:
@@ -4,8 +4,7 @@ module ActiveRecord
class Attribute # :nodoc:
class UserProvidedDefault < FromUser
def initialize(name, value, type, database_default)
super(name, value, type)
@database_default = database_default
super(name, value, type, database_default)
end

def type_cast(value)
@@ -16,17 +15,9 @@ def type_cast(value)
end
end

def changed_from?(old_value)
super || changed_from?(database_default.value)
end

def with_type(type)
self.class.new(name, value_before_type_cast, type, database_default)
self.class.new(name, value_before_type_cast, type, original_attribute)
end

protected

attr_reader :database_default
end
end
end
@@ -41,13 +41,15 @@ def reload(*)

def init_internals
super
@mutation_tracker = AttributeMutationTracker.new(@attributes, @attributes.dup)
@mutation_tracker = AttributeMutationTracker.new(@attributes)
end

def initialize_dup(other) # :nodoc:
super
@mutation_tracker = AttributeMutationTracker.new(@attributes,
self.class._default_attributes.deep_dup)
@attributes = self.class._default_attributes.map do |attr|
attr.with_value_from_user(@attributes.fetch_value(attr.name))
end
@mutation_tracker = AttributeMutationTracker.new(@attributes)
end

def changes_applied
@@ -122,7 +124,8 @@ def keys_for_partial_write
end

def store_original_attributes
@mutation_tracker = @mutation_tracker.now_tracking(@attributes)
@attributes = @attributes.map(&:forgetting_assignment)
@mutation_tracker = AttributeMutationTracker.new(@attributes)
end

def previous_mutation_tracker
@@ -1,64 +1,48 @@
module ActiveRecord
class AttributeMutationTracker # :nodoc:
def initialize(attributes, original_attributes)
def initialize(attributes)
@attributes = attributes
@original_attributes = original_attributes
end

def changed_values
attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result|
if changed?(attr_name)
result[attr_name] = original_attributes.fetch_value(attr_name)
result[attr_name] = attributes[attr_name].original_value
end
end
end

def changes
attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result|
if changed?(attr_name)
result[attr_name] = [original_attributes.fetch_value(attr_name), attributes.fetch_value(attr_name)]
result[attr_name] = [attributes[attr_name].original_value, attributes.fetch_value(attr_name)]
end
end
end

def changed?(attr_name)
attr_name = attr_name.to_s
modified?(attr_name) || changed_in_place?(attr_name)
attributes[attr_name].changed?
end

def changed_in_place?(attr_name)
original_database_value = original_attributes[attr_name].value_before_type_cast
attributes[attr_name].changed_in_place_from?(original_database_value)
attributes[attr_name].changed_in_place?
end

def forget_change(attr_name)
attr_name = attr_name.to_s
original_attributes[attr_name] = attributes[attr_name].dup
end

def now_tracking(new_attributes)
AttributeMutationTracker.new(new_attributes, clean_copy_of(new_attributes))
attributes[attr_name] = attributes[attr_name].forgetting_assignment
end

protected

attr_reader :attributes, :original_attributes
attr_reader :attributes

private

def attr_names
attributes.keys
end

def modified?(attr_name)
attributes[attr_name].changed_from?(original_attributes.fetch_value(attr_name))
end

def clean_copy_of(attributes)
attributes.map do |attr|
attr.with_value_from_database(attr.value_for_database)
end
end
end

class NullMutationTracker # :nodoc:
@@ -542,9 +542,6 @@ def test_converted_values_are_returned_after_assignment

developer.save!

assert_equal "50000", developer.salary_before_type_cast
assert_equal 1337, developer.name_before_type_cast

assert_equal 50000, developer.salary
assert_equal "1337", developer.name
end
@@ -182,12 +182,49 @@ def assert_valid_value(*)
assert attribute.has_been_read?
end

test "an attribute is not changed if it hasn't been assigned or mutated" do
attribute = Attribute.from_database(:foo, 1, Type::Value.new)

refute attribute.changed?
end

test "an attribute is changed if it's been assigned a new value" do
attribute = Attribute.from_database(:foo, 1, Type::Value.new)
changed = attribute.with_value_from_user(2)

assert changed.changed?
end

test "an attribute is not changed if it's assigned the same value" do
attribute = Attribute.from_database(:foo, 1, Type::Value.new)
unchanged = attribute.with_value_from_user(1)

refute unchanged.changed?
end

test "an attribute can not be mutated if it has not been read,
and skips expensive calculations" do
type_which_raises_from_all_methods = Object.new
attribute = Attribute.from_database(:foo, "bar", type_which_raises_from_all_methods)

assert_not attribute.changed_in_place_from?("bar")
assert_not attribute.changed_in_place?
end

test "an attribute is changed if it has been mutated" do
attribute = Attribute.from_database(:foo, "bar", Type::String.new)
attribute.value << "!"

assert attribute.changed_in_place?
assert attribute.changed?
end

test "an attribute can forget its changes" do
attribute = Attribute.from_database(:foo, "bar", Type::String.new)
changed = attribute.with_value_from_user("foo")
forgotten = changed.forgetting_assignment

assert changed.changed? # sanity check
refute forgotten.changed?
end

test "with_value_from_user validates the value" do

0 comments on commit 07723c2

Please sign in to comment.