Attribute assignment and type casting has nothing to do with columns

It's finally finished!!!!!!! The reason the Attributes API was kept
private in 4.2 was due to some publicly visible implementation details.
It was previously implemented by overloading `columns` and
`columns_hash`, to make them return column objects which were modified
with the attribute information.

This meant that those methods LIED! We didn't change the database
schema. We changed the attribute information on the class. That is
wrong! It should be the other way around, where schema loading just
calls the attributes API for you. And now it does!

Yes, this means that there is nothing that happens in automatic schema
loading that you couldn't manually do yourself. (There's still some
funky cases where we hit the connection adapter that I need to handle,
before we can turn off automatic schema detection entirely.)

There were a few weird test failures caused by this that had to be
fixed. The main source came from the fact that the attribute methods are
now defined in terms of `attribute_names`, which has a clause like
`return [] unless table_exists?`. I don't *think* this is an issue,
since the only place this caused failures were in a fake adapter which
didn't override `table_exists?`.

Additionally, there were a few cases where tests were failing because a
migration was run, but the model was not reloaded. I'm not sure why
these started failing from this change, I might need to clear an
additional cache in `reload_schema_from_cache`. Again, since this is not
normal usage, and it's expected that `reset_column_information` will be
called after the table is modified, I don't think it's a problem.

Still, test failures that were unrelated to the change are worrying, and
I need to dig into them further.

Finally, I spent a lot of time debugging issues with the mutex used in
`define_attribute_methods`. I think we can just remove that method
entirely, and define the attribute methods *manually* in the call to
`define_attribute`, which would simplify the code *tremendously*.

Ok. now to make this damn thing public, and work on moving it up to
Active Model.

15 files changed, 153 insertions, 113 deletions

diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb
index 48b50d9017..91886f1324 100644
--- a/activerecord/lib/active_record/attribute.rb
+++ b/activerecord/lib/active_record/attribute.rb
@@ -66,6 +66,10 @@ module ActiveRecord
       self.class.with_cast_value(name, value, type)
     end
 
+    def with_type(type)
+      self.class.new(name, value_before_type_cast, type)
+    end
+
     def type_cast(*)
       raise NotImplementedError
     end
@@ -137,6 +141,10 @@ module ActiveRecord
         nil
       end
 
+      def with_type(type)
+        self.class.with_cast_value(name, nil, type)
+      end
+
       def with_value_from_database(value)
         raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{name}`"
       end
diff --git a/activerecord/lib/active_record/attribute_decorators.rb b/activerecord/lib/active_record/attribute_decorators.rb
index 5b96623b6e..7d0ae32411 100644
--- a/activerecord/lib/active_record/attribute_decorators.rb
+++ b/activerecord/lib/active_record/attribute_decorators.rb
@@ -15,7 +15,7 @@ module ActiveRecord
       end
 
       def decorate_matching_attribute_types(matcher, decorator_name, &block)
-        clear_caches_calculated_from_columns
+        reload_schema_from_cache
         decorator_name = decorator_name.to_s
 
         # Create new hashes so we don't modify parent classes
@@ -24,10 +24,11 @@ module ActiveRecord
 
       private
 
-      def add_user_provided_columns(*)
-        super.map do |column|
-          decorated_type = attribute_type_decorations.apply(column.name, column.cast_type)
-          column.with_type(decorated_type)
+      def load_schema!
+        super
+        attribute_types.each do |name, type|
+          decorated_type = attribute_type_decorations.apply(name, type)
+          define_attribute(name, decorated_type)
         end
       end
     end
diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index 6de71896ee..9d58a19304 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -83,7 +83,7 @@ module ActiveRecord
         generated_attribute_methods.synchronize do
           return false if @attribute_methods_generated
           superclass.define_attribute_methods unless self == base_class
-          super(column_names)
+          super(attribute_names)
           @attribute_methods_generated = true
         end
         true
@@ -185,7 +185,7 @@ module ActiveRecord
       #   # => ["id", "created_at", "updated_at", "name", "age"]
       def attribute_names
         @attribute_names ||= if !abstract_class? && table_exists?
-            column_names
+            attribute_types.keys
           else
             []
           end
diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index 06d87ee01e..7ba907f786 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -131,10 +131,8 @@ module ActiveRecord
         partial_writes? ? super(keys_for_partial_write) : super
       end
 
-      # Serialized attributes should always be written in case they've been
-      # changed in place.
       def keys_for_partial_write
-        changed & persistable_attribute_names
+        changed & self.class.column_names
       end
 
       def _field_changed?(attr, old_value)
diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb
index 0c9793d470..9142317646 100644
--- a/activerecord/lib/active_record/attribute_set.rb
+++ b/activerecord/lib/active_record/attribute_set.rb
@@ -10,6 +10,10 @@ module ActiveRecord
       attributes[name] || Attribute.null(name)
     end
 
+    def []=(name, value)
+      attributes[name] = value
+    end
+
     def values_before_type_cast
       attributes.transform_values(&:value_before_type_cast)
     end
@@ -49,7 +53,7 @@ module ActiveRecord
     end
 
     def initialize_dup(_)
-      @attributes = attributes.dup
+      @attributes = attributes.deep_dup
       super
     end
 
diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb
index faf5d632ec..c1b69092bb 100644
--- a/activerecord/lib/active_record/attributes.rb
+++ b/activerecord/lib/active_record/attributes.rb
@@ -5,12 +5,8 @@ module ActiveRecord
     Type = ActiveRecord::Type
 
     included do
-      class_attribute :user_provided_columns, instance_accessor: false # :internal:
-      class_attribute :user_provided_defaults, instance_accessor: false # :internal:
-      self.user_provided_columns = {}
-      self.user_provided_defaults = {}
-
-      delegate :persistable_attribute_names, to: :class
+      class_attribute :user_provided_types, instance_accessor: false # :internal:
+      self.user_provided_types = {}
     end
 
     module ClassMethods # :nodoc:
@@ -77,70 +73,44 @@ module ActiveRecord
       #
       #   store_listing = StoreListing.new(price_in_cents: '$10.00')
       #   store_listing.price_in_cents # => 1000
-      def attribute(name, cast_type, options = {})
+      def attribute(name, cast_type, **options)
         name = name.to_s
-        clear_caches_calculated_from_columns
-        # Assign a new hash to ensure that subclasses do not share a hash
-        self.user_provided_columns = user_provided_columns.merge(name => cast_type)
-
-        if options.key?(:default)
-          self.user_provided_defaults = user_provided_defaults.merge(name => options[:default])
-        end
-      end
+        reload_schema_from_cache
 
-      # Returns an array of column objects for the table associated with this class.
-      def columns
-        @columns ||= add_user_provided_columns(connection.schema_cache.columns(table_name))
+        self.user_provided_types = user_provided_types.merge(name => [cast_type, options])
       end
 
-      # Returns a hash of column objects for the table associated with this class.
-      def columns_hash
-        @columns_hash ||= Hash[columns.map { |c| [c.name, c] }]
+      def define_attribute(
+        name,
+        cast_type,
+        default: NO_DEFAULT_PROVIDED,
+        user_provided_default: true
+      )
+        attribute_types[name] = cast_type
+        define_default_attribute(name, default, cast_type, from_user: user_provided_default)
       end
 
-      def persistable_attribute_names # :nodoc:
-        @persistable_attribute_names ||= connection.schema_cache.columns_hash(table_name).keys
-      end
-
-      def reset_column_information # :nodoc:
+      def load_schema!
         super
-        clear_caches_calculated_from_columns
+        user_provided_types.each do |name, (type, options)|
+          define_attribute(name, type, **options)
+        end
       end
 
       private
 
-      def add_user_provided_columns(schema_columns)
-        existing_columns = schema_columns.map do |column|
-          new_type = user_provided_columns[column.name]
-          if new_type
-            column.with_type(new_type)
-          else
-            column
-          end
-        end
+      NO_DEFAULT_PROVIDED = Object.new # :nodoc:
+      private_constant :NO_DEFAULT_PROVIDED
 
-        existing_column_names = existing_columns.map(&:name)
-        new_columns = user_provided_columns.except(*existing_column_names).map do |(name, type)|
-          connection.new_column(name, nil, type)
+      def define_default_attribute(name, value, type, from_user:)
+        if value == NO_DEFAULT_PROVIDED
+          default_attribute = _default_attributes[name].with_type(type)
+        elsif from_user
+          default_attribute = Attribute.from_user(name, value, type)
+        else
+          default_attribute = Attribute.from_database(name, value, type)
         end
-
-        existing_columns + new_columns
-      end
-
-      def clear_caches_calculated_from_columns
-        @arel_table = nil
-        @attributes_builder = nil
-        @column_names = nil
-        @column_types = nil
-        @columns = nil
-        @columns_hash = nil
-        @content_columns = nil
-        @default_attributes = nil
-        @persistable_attribute_names = nil
-      end
-
-      def raw_default_values
-        super.merge(user_provided_defaults)
+        _default_attributes[name] = default_attribute
       end
     end
   end
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index e68b2c399c..4416217897 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -210,7 +210,7 @@ module ActiveRecord
         elsif !connected?
           "#{super} (call '#{super}.connection' to establish a connection)"
         elsif table_exists?
-          attr_list = column_types.map { |name, type| "#{name}: #{type.type}" } * ', '
+          attr_list = attribute_types.map { |name, type| "#{name}: #{type.type}" } * ', '
           "#{super}(#{attr_list})"
         else
           "#{super}(Table doesn't exist)"
diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb
index fd1e22349b..24098f72dc 100644
--- a/activerecord/lib/active_record/inheritance.rb
+++ b/activerecord/lib/active_record/inheritance.rb
@@ -192,7 +192,7 @@ module ActiveRecord
       # If this is a StrongParameters hash, and access to inheritance_column is not permitted,
       # this will ignore the inheritance column and return nil
       def subclass_from_attributes?(attrs)
-        columns_hash.include?(inheritance_column) && attrs.is_a?(Hash)
+        attribute_names.include?(inheritance_column) && attrs.is_a?(Hash)
       end
 
       def subclass_from_attributes(attrs)
diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb
index 6f2b65c137..008cda46cd 100644
--- a/activerecord/lib/active_record/locking/optimistic.rb
+++ b/activerecord/lib/active_record/locking/optimistic.rb
@@ -144,7 +144,7 @@ module ActiveRecord
 
         # Set the column to use for optimistic locking. Defaults to +lock_version+.
         def locking_column=(value)
-          clear_caches_calculated_from_columns
+          reload_schema_from_cache
           @locking_column = value.to_s
         end
 
diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb
index af0b667262..293db1c57f 100644
--- a/activerecord/lib/active_record/model_schema.rb
+++ b/activerecord/lib/active_record/model_schema.rb
@@ -217,28 +217,37 @@ module ActiveRecord
       end
 
       def attributes_builder # :nodoc:
-        @attributes_builder ||= AttributeSet::Builder.new(column_types, primary_key)
+        @attributes_builder ||= AttributeSet::Builder.new(attribute_types, primary_key)
       end
 
-      def column_types # :nodoc:
-        @column_types ||= columns_hash.transform_values(&:cast_type).tap do |h|
-          h.default = Type::Value.new
-        end
+      def columns_hash # :nodoc:
+        load_schema
+        @columns_hash
+      end
+
+      def columns
+        load_schema
+        @columns ||= columns_hash.values
+      end
+
+      def attribute_types # :nodoc:
+        load_schema
+        @attribute_types ||= Hash.new(Type::Value.new)
       end
 
       def type_for_attribute(attr_name) # :nodoc:
-        column_types[attr_name]
+        attribute_types[attr_name]
       end
 
       # Returns a hash where the keys are column names and the values are
       # default values when instantiating the AR object for this table.
       def column_defaults
+        load_schema
         _default_attributes.to_hash
       end
 
       def _default_attributes # :nodoc:
-        @default_attributes ||= attributes_builder.build_from_database(
-          raw_default_values)
+        @default_attributes ||= AttributeSet.new({})
       end
 
       # Returns an array of column names as strings.
@@ -281,19 +290,53 @@ module ActiveRecord
       def reset_column_information
         connection.clear_cache!
         undefine_attribute_methods
-        connection.schema_cache.clear_table_cache!(table_name) if table_exists?
+        connection.schema_cache.clear_table_cache!(table_name)
 
-        @arel_engine        = nil
-        @arel_table         = nil
-        @column_names       = nil
-        @column_types       = nil
-        @content_columns    = nil
-        @default_attributes = nil
-        @inheritance_column = nil unless defined?(@explicit_inheritance_column) && @explicit_inheritance_column
+        reload_schema_from_cache
       end
 
       private
 
+      def schema_loaded?
+        defined?(@columns_hash) && @columns_hash
+      end
+
+      def load_schema
+        unless schema_loaded?
+          load_schema!
+        end
+      end
+
+      def load_schema!
+        @columns_hash = connection.schema_cache.columns_hash(table_name)
+        @columns_hash.each do |name, column|
+          define_attribute(
+            name,
+            column.cast_type,
+            default: column.default,
+            user_provided_default: false
+          )
+        end
+      end
+
+      def reload_schema_from_cache
+        @arel_engine = nil
+        @arel_table = nil
+        @column_names = nil
+        @attribute_types = nil
+        @content_columns = nil
+        @default_attributes = nil
+        @inheritance_column = nil unless defined?(@explicit_inheritance_column) && @explicit_inheritance_column
+        @attributes_builder = nil
+        @column_names = nil
+        @attribute_types = nil
+        @columns = nil
+        @columns_hash = nil
+        @content_columns = nil
+        @default_attributes = nil
+        @attribute_names = nil
+      end
+
       # Guesses the table name, but does not decorate it with prefix and suffix information.
       def undecorated_table_name(class_name = base_class.name)
         table_name = class_name.to_s.demodulize.underscore
@@ -317,10 +360,6 @@ module ActiveRecord
           base.table_name
         end
       end
-
-      def raw_default_values
-        columns_hash.transform_values(&:default)
-      end
     end
   end
 end
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index c3c4d7f1ce..63e0d2fc21 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -167,7 +167,7 @@ module ActiveRecord
           columns_hash.key?(cn) ? arel_table[cn] : cn
         }
         result = klass.connection.select_all(relation.arel, nil, bound_attributes)
-        result.cast_values(klass.column_types)
+        result.cast_values(klass.attribute_types)
       end
     end
 
diff --git a/activerecord/test/active_record/connection_adapters/fake_adapter.rb b/activerecord/test/active_record/connection_adapters/fake_adapter.rb
index 64cde143a1..ad9279aaca 100644
--- a/activerecord/test/active_record/connection_adapters/fake_adapter.rb
+++ b/activerecord/test/active_record/connection_adapters/fake_adapter.rb
@@ -22,7 +22,7 @@ module ActiveRecord
       end
 
       def primary_key(table)
-        @primary_keys[table]
+        @primary_keys[table] || "id"
       end
 
       def merge_column(table_name, name, sql_type = nil, options = {})
@@ -38,6 +38,10 @@ module ActiveRecord
         @columns[table_name]
       end
 
+      def table_exists?(*)
+        true
+      end
+
       def active?
         true
       end
diff --git a/activerecord/test/cases/attribute_methods/read_test.rb b/activerecord/test/cases/attribute_methods/read_test.rb
index e38b32d7fc..74e556211b 100644
--- a/activerecord/test/cases/attribute_methods/read_test.rb
+++ b/activerecord/test/cases/attribute_methods/read_test.rb
@@ -17,7 +17,7 @@ module ActiveRecord
 
           include ActiveRecord::AttributeMethods
 
-          def self.column_names
+          def self.attribute_names
             %w{ one two three }
           end
 
@@ -25,11 +25,11 @@ module ActiveRecord
           end
 
           def self.columns
-            column_names.map { FakeColumn.new(name) }
+            attribute_names.map { FakeColumn.new(name) }
           end
 
           def self.columns_hash
-            Hash[column_names.map { |name|
+            Hash[attribute_names.map { |name|
               [name, FakeColumn.new(name)]
             }]
           end
@@ -39,13 +39,13 @@ module ActiveRecord
       def test_define_attribute_methods
         instance = @klass.new
 
-        @klass.column_names.each do |name|
+        @klass.attribute_names.each do |name|
           assert !instance.methods.map(&:to_s).include?(name)
         end
 
         @klass.define_attribute_methods
 
-        @klass.column_names.each do |name|
+        @klass.attribute_names.each do |name|
           assert instance.methods.map(&:to_s).include?(name), "#{name} is not defined"
         end
       end
diff --git a/activerecord/test/cases/attributes_test.rb b/activerecord/test/cases/attributes_test.rb
index 4ddf6e7ba0..6f331c5985 100644
--- a/activerecord/test/cases/attributes_test.rb
+++ b/activerecord/test/cases/attributes_test.rb
@@ -50,8 +50,8 @@ module ActiveRecord
     end
 
     test "overloaded properties with limit" do
-      assert_equal 50, OverloadedType.columns_hash['overloaded_string_with_limit'].limit
-      assert_equal 255, UnoverloadedType.columns_hash['overloaded_string_with_limit'].limit
+      assert_equal 50, OverloadedType.type_for_attribute('overloaded_string_with_limit').limit
+      assert_equal 255, UnoverloadedType.type_for_attribute('overloaded_string_with_limit').limit
     end
 
     test "nonexistent attribute" do
@@ -87,29 +87,42 @@ module ActiveRecord
       assert_equal 4.4, data.overloaded_float
     end
 
-    test "overloading properties does not change column order" do
-      column_names = OverloadedType.column_names
-      assert_equal %w(id overloaded_float unoverloaded_float overloaded_string_with_limit string_with_default non_existent_decimal), column_names
+    test "overloading properties does not attribute method order" do
+      attribute_names = OverloadedType.attribute_names
+      assert_equal %w(id overloaded_float unoverloaded_float overloaded_string_with_limit string_with_default non_existent_decimal), attribute_names
     end
 
     test "caches are cleared" do
       klass = Class.new(OverloadedType)
 
-      assert_equal 6, klass.columns.length
-      assert_not klass.columns_hash.key?('wibble')
-      assert_equal 6, klass.column_types.length
+      assert_equal 6, klass.attribute_types.length
       assert_equal 6, klass.column_defaults.length
-      assert_not klass.column_names.include?('wibble')
-      assert_equal 5, klass.content_columns.length
+      assert_not klass.attribute_types.include?('wibble')
 
       klass.attribute :wibble, Type::Value.new
 
-      assert_equal 7, klass.columns.length
-      assert klass.columns_hash.key?('wibble')
-      assert_equal 7, klass.column_types.length
+      assert_equal 7, klass.attribute_types.length
       assert_equal 7, klass.column_defaults.length
-      assert klass.column_names.include?('wibble')
-      assert_equal 6, klass.content_columns.length
+      assert klass.attribute_types.include?('wibble')
+    end
+
+    test "the given default value is cast from user" do
+      custom_type = Class.new(Type::Value) do
+        def type_cast_from_user(*)
+          "from user"
+        end
+
+        def type_cast_from_database(*)
+          "from database"
+        end
+      end
+
+      klass = Class.new(OverloadedType) do
+        attribute :wibble, custom_type.new, default: "default"
+      end
+      model = klass.new
+
+      assert_equal "from user", model.wibble
     end
   end
 end
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index a5d7cc2dff..6ea8b93be7 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -162,6 +162,7 @@ class MigrationTest < ActiveRecord::TestCase
 
     assert !BigNumber.table_exists?
     GiveMeBigNumbers.up
+    BigNumber.reset_column_information
 
     assert BigNumber.create(
       :bank_balance => 1586.43,
@@ -397,6 +398,7 @@ class MigrationTest < ActiveRecord::TestCase
     Thing.reset_table_name
     Thing.reset_sequence_name
     WeNeedThings.up
+    Thing.reset_column_information
 
     assert Thing.create("content" => "hello world")
     assert_equal "hello world", Thing.first.content
@@ -416,6 +418,7 @@ class MigrationTest < ActiveRecord::TestCase
     ActiveRecord::Base.table_name_suffix = '_suffix'
     Reminder.reset_table_name
     Reminder.reset_sequence_name
+    Reminder.reset_column_information
     WeNeedReminders.up
     assert Reminder.create("content" => "hello world", "remind_at" => Time.now)
     assert_equal "hello world", Reminder.first.content