From 14b1208dd313f643f72aa2d92874198342e9fe14 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Sun, 22 Jun 2014 16:25:40 -0600 Subject: [PATCH] Encapsulate the creation of `Attribute` objects This will make it less painful to add additional properties, which should persist across writes, such as `name`. Conflicts: activerecord/lib/active_record/attribute_set.rb --- activerecord/lib/active_record/attribute.rb | 28 +++++++++------ .../active_record/attribute_methods/write.rb | 5 ++- .../lib/active_record/attribute_set.rb | 10 +++++- .../active_record/attribute_set/builder.rb | 2 +- activerecord/lib/active_record/core.rb | 4 +-- activerecord/test/cases/attribute_set_test.rb | 36 ++++++++++++++++++- activerecord/test/cases/attribute_test.rb | 26 ++++++++++++++ 7 files changed, 92 insertions(+), 19 deletions(-) diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb index 6c0b81b3fe..13c8bc3676 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activerecord/lib/active_record/attribute.rb @@ -41,6 +41,14 @@ module ActiveRecord type.changed_in_place?(old_value, value) end + def with_value_from_user(value) + self.class.from_user(value, type) + end + + def with_value_from_database(value) + self.class.from_database(value, type) + end + def type_cast raise NotImplementedError end @@ -69,18 +77,13 @@ module ActiveRecord end end - class Null # :nodoc: - class << self - attr_reader :value, :value_before_type_cast, :value_for_database + class NullAttribute < Attribute # :nodoc: + def initialize + super(nil, Type::Value.new) + end - def changed_from?(*) - false - end - alias changed_in_place_from? changed_from? - - def initialized? - true - end + def value + nil end end @@ -98,5 +101,8 @@ module ActiveRecord false end end + private_constant :FromDatabase, :FromUser, :NullAttribute, :Uninitialized + + Null = NullAttribute.new # :nodoc: end end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index 246a2cd8ba..94fce2db60 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -69,16 +69,15 @@ module ActiveRecord def write_attribute_with_type_cast(attr_name, value, should_type_cast) attr_name = attr_name.to_s attr_name = self.class.primary_key if attr_name == 'id' && self.class.primary_key - type = type_for_attribute(attr_name) unless has_attribute?(attr_name) || self.class.columns_hash.key?(attr_name) raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{attr_name}'" end if should_type_cast - @attributes[attr_name] = Attribute.from_user(value, type) + @attributes.write_from_user(attr_name, value) else - @attributes[attr_name] = Attribute.from_database(value, type) + @attributes.write_from_database(attr_name, value) end value diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb index 68382756a4..65e15b16dd 100644 --- a/activerecord/lib/active_record/attribute_set.rb +++ b/activerecord/lib/active_record/attribute_set.rb @@ -2,7 +2,7 @@ require 'active_record/attribute_set/builder' module ActiveRecord class AttributeSet # :nodoc: - delegate :[], :[]=, to: :attributes + delegate :[], to: :attributes delegate :keys, to: :initialized_attributes def initialize(attributes) @@ -31,6 +31,14 @@ module ActiveRecord end end + def write_from_database(name, value) + attributes[name] = self[name].with_value_from_database(value) + end + + def write_from_user(name, value) + attributes[name] = self[name].with_value_from_user(value) + end + def freeze @attributes.freeze super diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index d91720d5e1..f9cf9c1809 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -7,7 +7,7 @@ module ActiveRecord @types = types end - def build_from_database(values, additional_types = {}) + def build_from_database(values = {}, additional_types = {}) attributes = build_attributes_from_values(values, additional_types) add_uninitialized_attributes(attributes) AttributeSet.new(attributes) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 10b9e27b7d..c434b0d40a 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -322,7 +322,7 @@ module ActiveRecord def initialize_dup(other) # :nodoc: pk = self.class.primary_key @attributes = @attributes.dup - @attributes[pk] = Attribute.from_database(nil, type_for_attribute(pk)) + @attributes.write_from_database(pk, nil) run_callbacks(:initialize) unless _initialize_callbacks.empty? @@ -522,7 +522,7 @@ module ActiveRecord def init_internals pk = self.class.primary_key unless @attributes.include?(pk) - @attributes[pk] = Attribute.from_database(nil, type_for_attribute(pk)) + @attributes.write_from_database(pk, nil) end @aggregation_cache = {} diff --git a/activerecord/test/cases/attribute_set_test.rb b/activerecord/test/cases/attribute_set_test.rb index df35a7b384..35caa0ddab 100644 --- a/activerecord/test/cases/attribute_set_test.rb +++ b/activerecord/test/cases/attribute_set_test.rb @@ -35,7 +35,7 @@ module ActiveRecord attributes[:bar].value duped = attributes.dup - duped[:foo] = Attribute.from_database(2, Type::Integer.new) + duped.write_from_database(:foo, 2) duped[:bar].value << 'bar' assert_equal 1, attributes[:foo].value @@ -120,6 +120,40 @@ module ActiveRecord assert_nil attributes.fetch_value(:bar) end + class MyType + def type_cast_from_user(value) + return if value.nil? + value + " from user" + end + + def type_cast_from_database(value) + return if value.nil? + value + " from database" + end + end + + test "write_from_database sets the attribute with database typecasting" do + builder = AttributeSet::Builder.new(foo: MyType.new) + attributes = builder.build_from_database + + assert_nil attributes.fetch_value(:foo) + + attributes.write_from_database(:foo, "value") + + assert_equal "value from database", attributes.fetch_value(:foo) + end + + test "write_from_user sets the attribute with user typecasting" do + builder = AttributeSet::Builder.new(foo: MyType.new) + attributes = builder.build_from_database + + assert_nil attributes.fetch_value(:foo) + + attributes.write_from_user(:foo, "value") + + assert_equal "value from user", attributes.fetch_value(:foo) + end + def attributes_with_uninitialized_key builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Float.new) builder.build_from_database(foo: '1.1') diff --git a/activerecord/test/cases/attribute_test.rb b/activerecord/test/cases/attribute_test.rb index 57dd2e9a5e..0adf9545b8 100644 --- a/activerecord/test/cases/attribute_test.rb +++ b/activerecord/test/cases/attribute_test.rb @@ -99,5 +99,31 @@ module ActiveRecord attribute = Attribute.from_database('a value', @type) attribute.dup end + + class MyType + def type_cast_from_user(value) + value + " from user" + end + + def type_cast_from_database(value) + value + " from database" + end + end + + test "with_value_from_user returns a new attribute with the value from the user" do + old = Attribute.from_database("old", MyType.new) + new = old.with_value_from_user("new") + + assert_equal "old from database", old.value + assert_equal "new from user", new.value + end + + test "with_value_from_database returns a new attribute with the value from the database" do + old = Attribute.from_user("old", MyType.new) + new = old.with_value_from_database("new") + + assert_equal "old from user", old.value + assert_equal "new from database", new.value + end end end