Skip to content

Commit

Permalink
Change how AttributeSet::Builder receives its defaults
Browse files Browse the repository at this point in the history
There are two concerns which are both being combined into one here, but
both have the same goal. There are certain attributes which we want to
always consider initialized. Previously, they were handled separately.
The primary key (which is assumed to be backed by a database column)
needs to be initialized, because there is a ton of code in Active Record
that assumes `foo.id` will never raise. Additionally, we want attributes
which aren't backed by a database column to always be initialized, since
we would never receive a database value for them.

Ultimately these two concerns can be combined into one. The old
implementation hid a lot of inherent complexity, and is hard to optimize
from the outside. We can simplify things significantly by just passing
in a hash.

This has slightly different semantics from the old behavior, in that
`Foo.select(:bar).first.id` will return the default value for the
primary key, rather than `nil` unconditionally -- however, the default
value is always `nil` in practice.
  • Loading branch information
sgrif authored and kamipo committed Feb 5, 2018
1 parent fcb5ae3 commit 1fd6750
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 20 deletions.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/attribute_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

module ActiveRecord
class AttributeSet # :nodoc:
delegate :each_value, :fetch, to: :attributes
delegate :each_value, :fetch, :except, to: :attributes

def initialize(attributes)
@attributes = attributes
Expand Down
28 changes: 14 additions & 14 deletions activerecord/lib/active_record/attribute_set/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,30 @@
module ActiveRecord
class AttributeSet # :nodoc:
class Builder # :nodoc:
attr_reader :types, :always_initialized, :default
attr_reader :types, :default_attributes

def initialize(types, always_initialized = nil, &default)
def initialize(types, default_attributes = {})
@types = types
@always_initialized = always_initialized
@default = default
@default_attributes = default_attributes
end

def build_from_database(values = {}, additional_types = {})
if always_initialized && !values.key?(always_initialized)
values[always_initialized] = nil
end

attributes = LazyAttributeHash.new(types, values, additional_types, &default)
attributes = LazyAttributeHash.new(types, values, additional_types, default_attributes)
AttributeSet.new(attributes)
end
end
end

class LazyAttributeHash # :nodoc:
delegate :transform_values, :each_key, :each_value, :fetch, to: :materialize
delegate :transform_values, :each_key, :each_value, :fetch, :except, to: :materialize

def initialize(types, values, additional_types, &default)
def initialize(types, values, additional_types, default_attributes)
@types = types
@values = values
@additional_types = additional_types
@materialized = false
@delegate_hash = {}
@default = default || proc {}
@default_attributes = default_attributes
end

def key?(key)
Expand Down Expand Up @@ -94,7 +89,7 @@ def marshal_load(delegate_hash)
# Workaround for Ruby 2.2 "private attribute?" warning.
protected

attr_reader :types, :values, :additional_types, :delegate_hash, :default
attr_reader :types, :values, :additional_types, :delegate_hash, :default_attributes

def materialize
unless @materialized
Expand All @@ -117,7 +112,12 @@ def assign_default_value(name)
if value_present
delegate_hash[name] = Attribute.from_database(name, value, type)
elsif types.key?(name)
delegate_hash[name] = default.call(name) || Attribute.uninitialized(name, type)
attr = default_attributes[name]
if attr
delegate_hash[name] = attr.dup
else
delegate_hash[name] = Attribute.uninitialized(name, type)
end
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions activerecord/lib/active_record/model_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -332,11 +332,11 @@ def table_exists?
end

def attributes_builder # :nodoc:
@attributes_builder ||= AttributeSet::Builder.new(attribute_types, primary_key) do |name|
unless columns_hash.key?(name)
_default_attributes[name].dup
end
unless defined?(@attributes_builder) && @attributes_builder
defaults = _default_attributes.except(*(column_names - [primary_key]))
@attributes_builder = AttributeSet::Builder.new(attribute_types, defaults)
end
@attributes_builder
end

def columns_hash # :nodoc:
Expand Down
3 changes: 2 additions & 1 deletion activerecord/test/cases/attribute_set_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ class AttributeSetTest < ActiveRecord::TestCase
end

test "the primary_key is always initialized" do
builder = AttributeSet::Builder.new({ foo: Type::Integer.new }, :foo)
defaults = { foo: Attribute.from_user(:foo, nil, nil) }
builder = AttributeSet::Builder.new({ foo: Type::Integer.new }, defaults)
attributes = builder.build_from_database

assert attributes.key?(:foo)
Expand Down

0 comments on commit 1fd6750

Please sign in to comment.