Move InternalMetadata to an independent object

Followup to #45908 to match the same behavior as SchemaMigration

Previously, InternalMetadata inherited from ActiveRecord::Base. This is
problematic for multiple databases and resulted in building the code in
AbstractAdapter that was previously there. Rather than hacking around
the fact that InternalMetadata inherits from Base, this PR makes
InternalMetadata an independent object. Then each connection can get it's
own InternalMetadata object. This change required defining the methods
that InternalMetadata was depending on ActiveRecord::Base for (ex
create!). I reimplemented only the methods called by the framework as
this class is no-doc's so it doesn't need to implement anything beyond
that. Now each connection gets it's own InternalMetadata object which
stores the connection.

This change also required adding a NullInternalMetadata class for cases
when we don't have a connection yet but still need to copy migrations
from the MigrationContext. Ultimately I think this is a little weird -
we need to do so much work to pick up a set of files? Maybe something to
explore in the future.

Aside from removing the hack we added back in #36439 this change will
enable my work to stop clobbering and depending directly on
Base.connection in the rake tasks. While working on this I discovered
that we always have a ActiveRecord::InternalMetadata because the
connection is always on Base in the rake tasks. This will free us up
to do less hacky stuff in the migrations and tasks.

Both schema migration and internal metadata are blockers to removing
`Base.connection` and `Base.establish_connection` from rake tasks, work
that is required to drop the reliance on `Base.connection` which will
enable more robust (and correct) sharding behavior in Rails..

activerecord/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Move `ActiveRecord::InternalMetadata` to an independent object.
+
+    `ActiveRecord::InternalMetadata` no longer inherits from `ActiveRecord::Base` and is now an independent object that should be instantiated with a `connection`. This class is private and should not be used by applications directly. If you want to interact with the schema migrations table, please access it on the connection directly, for example: `ActiveRecord::Base.connection.schema_migration`.
+
+    *Eileen M. Uchitelle*
+
 *   Deprecate quoting `ActiveSupport::Duration` as an integer
 
     Using ActiveSupport::Duration as an interpolated bind parameter in a SQL

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -197,21 +197,7 @@ def schema_migration # :nodoc:
       end
 
       def internal_metadata # :nodoc:
-        @internal_metadata ||= begin
-                                conn = self
-                                connection_name = conn.pool.pool_config.connection_name
-
-                                return ActiveRecord::InternalMetadata if connection_name == "ActiveRecord::Base"
-
-                                internal_metadata_name = "#{connection_name}::InternalMetadata"
-
-                                Class.new(ActiveRecord::InternalMetadata) do
-                                  define_singleton_method(:name) { internal_metadata_name }
-                                  define_singleton_method(:to_s) { internal_metadata_name }
-
-                                  self.connection_specification_name = connection_name
-                                end
-                              end
+        InternalMetadata.new(self)
       end
 
       def prepared_statements?

activerecord/lib/active_record/internal_metadata.rb

@@ -9,58 +9,135 @@ module ActiveRecord
   #
   # This is enabled by default. To disable this functionality set
   # `use_metadata_table` to false in your database configuration.
-  class InternalMetadata < ActiveRecord::Base # :nodoc:
-    self.record_timestamps = true
+  class InternalMetadata # :nodoc:
+    class NullInternalMetadata
+    end
 
-    class << self
-      def enabled?
-        connection.use_metadata_table?
-      end
+    attr_reader :connection, :arel_table
 
-      def primary_key
-        "key"
-      end
+    def initialize(connection)
+      @connection = connection
+      @arel_table = Arel::Table.new(table_name)
+    end
+
+    def enabled?
+      connection.use_metadata_table?
+    end
+
+    def primary_key
+      "key"
+    end
+
+    def value_key
+      "value"
+    end
 
-      def table_name
-        "#{table_name_prefix}#{internal_metadata_table_name}#{table_name_suffix}"
+    def table_name
+      "#{ActiveRecord::Base.table_name_prefix}#{ActiveRecord::Base.internal_metadata_table_name}#{ActiveRecord::Base.table_name_suffix}"
+    end
+
+    def []=(key, value)
+      return unless enabled?
+
+      update_or_create_entry(key, value)
+    end
+
+    def [](key)
+      return unless enabled?
+
+      if entry = select_entry(key)
+        entry[value_key]
       end
+    end
+
+    def delete_all_entries
+      dm = Arel::DeleteManager.new(arel_table)
+
+      connection.delete(dm, "#{self} Destroy")
+    end
+
+    def count
+      sm = Arel::SelectManager.new(arel_table)
+      sm.project(*Arel::Nodes::Count.new([Arel.star]))
+      connection.select_values(sm).first
+    end
+
+    def create_table_and_set_flags(environment, schema_sha1 = nil)
+      create_table
+      update_or_create_entry(:environment, environment)
+      update_or_create_entry(:schema_sha1, schema_sha1) if schema_sha1
+    end
 
-      def []=(key, value)
-        return unless enabled?
+    # Creates an internal metadata table with columns +key+ and +value+
+    def create_table
+      return unless enabled?
 
-        find_or_initialize_by(key: key).update!(value: value)
+      unless connection.table_exists?(table_name)
+        connection.create_table(table_name, id: false) do |t|
+          t.string :key, **connection.internal_string_options_for_primary_key
+          t.string :value
+          t.timestamps
+        end
       end
+    end
 
-      def [](key)
-        return unless enabled?
+    def drop_table
+      return unless enabled?
 
-        where(key: key).pick(:value)
+      connection.drop_table table_name, if_exists: true
+    end
+
+    def table_exists?
+      connection.schema_cache.data_source_exists?(table_name)
+    end
+
+    private
+      def update_or_create_entry(key, value)
+        entry = select_entry(key)
+
+        if entry
+          update_entry(key, value)
+        else
+          create_entry(key, value)
+        end
       end
 
-      def create_table_and_set_flags(environment, schema_sha1 = nil)
-        create_table
-        self[:environment] = environment
-        self[:schema_sha1] = schema_sha1 if schema_sha1
+      def current_time
+        connection.default_timezone == :utc ? Time.now.utc : Time.now
       end
 
-      # Creates an internal metadata table with columns +key+ and +value+
-      def create_table
-        return unless enabled?
+      def create_entry(key, value)
+        im = Arel::InsertManager.new(arel_table)
+        im.insert [
+          [arel_table[primary_key], key],
+          [arel_table[value_key], value],
+          [arel_table[:created_at], current_time],
+          [arel_table[:updated_at], current_time]
+        ]
 
-        unless connection.table_exists?(table_name)
-          connection.create_table(table_name, id: false) do |t|
-            t.string :key, **connection.internal_string_options_for_primary_key
-            t.string :value
-            t.timestamps
-          end
-        end
+        connection.insert(im, "#{self} Create", primary_key, key)
       end
 
-      def drop_table
-        return unless enabled?
+      def update_entry(key, new_value)
+        um = Arel::UpdateManager.new(arel_table)
+        um.set [
+          [arel_table[value_key], new_value],
+          [arel_table[:updated_at], current_time]
+        ]
+
+        um.where(arel_table[primary_key].eq(key))
 
-        connection.drop_table table_name, if_exists: true
+        connection.update(um, "#{self} Update")
+      end
+
+      def select_entry(key)
+        sm = Arel::SelectManager.new(arel_table)
+        sm.project(Arel::Nodes::SqlLiteral.new("*"))
+        sm.where(arel_table[primary_key].eq(Arel::Nodes::BindParam.new(key)))
+        sm.order(arel_table[primary_key].asc)
+        sm.limit = 1
+
+        connection.select_all(sm).first
       end
-    end
   end
 end

activerecord/lib/active_record/migration.rb

@@ -952,11 +952,13 @@ def copy(destination, sources, options = {})
       copied = []
 
       FileUtils.mkdir_p(destination) unless File.exist?(destination)
+      schema_migration = SchemaMigration::NullSchemaMigration.new
+      internal_metadata = InternalMetadata::NullInternalMetadata.new
 
-      destination_migrations = ActiveRecord::MigrationContext.new(destination, SchemaMigration::NullSchemaMigration.new).migrations
+      destination_migrations = ActiveRecord::MigrationContext.new(destination, schema_migration, internal_metadata).migrations
       last = destination_migrations.last
       sources.each do |scope, path|
-        source_migrations = ActiveRecord::MigrationContext.new(path, SchemaMigration::NullSchemaMigration.new).migrations
+        source_migrations = ActiveRecord::MigrationContext.new(path, schema_migration, internal_metadata).migrations
 
         source_migrations.each do |migration|
           source = File.binread(migration.filename)
@@ -1081,16 +1083,22 @@ def load_migration
   class MigrationContext
     attr_reader :migrations_paths, :schema_migration, :internal_metadata
 
-    def initialize(migrations_paths, schema_migration = nil, internal_metadata = InternalMetadata)
+    def initialize(migrations_paths, schema_migration = nil, internal_metadata = nil)
       if schema_migration == SchemaMigration
         schema_migration = SchemaMigration.new(ActiveRecord::Base.connection)
 
         ActiveSupport::Deprecation.warn("SchemaMigration no longer inherits from ActiveRecord::Base. Please instaniate a new SchemaMigration object with the desired connection, ie `ActiveRecord::SchemaMigration.new(ActiveRecord::Base.connection)`.")
       end
 
+      if internal_metadata == InternalMetadata
+        internal_metadata = InternalMetadata.new(ActiveRecord::Base.connection)
+
+        ActiveSupport::Deprecation.warn("InternalMetadata no longer inherits from ActiveRecord::Base. Please instaniate a new InternalMetadata object with the desired connection, ie `ActiveRecord::InternalMetadata.new(ActiveRecord::Base.connection)`.")
+      end
+
       @migrations_paths = migrations_paths
       @schema_migration = schema_migration || SchemaMigration.new(ActiveRecord::Base.connection)
-      @internal_metadata = internal_metadata
+      @internal_metadata = internal_metadata || InternalMetadata.new(ActiveRecord::Base.connection)
     end
 
     # Runs the migrations in the +migrations_path+.
@@ -1263,8 +1271,9 @@ class << self
       # For cases where a table doesn't exist like loading from schema cache
       def current_version
         schema_migration = SchemaMigration.new(ActiveRecord::Base.connection)
+        internal_metadata = InternalMetadata.new(ActiveRecord::Base.connection)
 
-        MigrationContext.new(migrations_paths, schema_migration, InternalMetadata).current_version
+        MigrationContext.new(migrations_paths, schema_migration, internal_metadata).current_version
       end
     end
 

activerecord/test/cases/active_record_schema_test.rb

@@ -69,7 +69,6 @@ def test_schema_define
   def test_schema_define_with_table_name_prefix
     old_table_name_prefix = ActiveRecord::Base.table_name_prefix
     ActiveRecord::Base.table_name_prefix = "nep_"
-    @internal_metadata.reset_table_name
     ActiveRecord::Schema.define(version: 7) do
       create_table :fruits do |t|
         t.column :color, :string
@@ -81,7 +80,6 @@ def test_schema_define_with_table_name_prefix
     assert_equal 7, @connection.migration_context.current_version
   ensure
     ActiveRecord::Base.table_name_prefix = old_table_name_prefix
-    @internal_metadata.reset_table_name
   end
 
   def test_schema_raises_an_error_for_invalid_column_type

activerecord/test/cases/adapters/postgresql/extension_migration_test.rb

@@ -27,7 +27,6 @@ def setup
 
     ActiveRecord::Base.table_name_prefix = "p_"
     ActiveRecord::Base.table_name_suffix = "_s"
-    @connection.internal_metadata.reset_table_name
 
     @connection.schema_migration.delete_all_versions rescue nil
     ActiveRecord::Migration.verbose = false
@@ -39,7 +38,6 @@ def teardown
 
     ActiveRecord::Base.table_name_prefix = @old_table_name_prefix
     ActiveRecord::Base.table_name_suffix = @old_table_name_suffix
-    @connection.internal_metadata.reset_table_name
 
     super
   end

activerecord/test/cases/migration_test.rb

@@ -81,7 +81,7 @@ def setup
 
   def test_passing_a_schema_migration_class_to_migration_context_is_deprecated
     migrations_path = MIGRATIONS_ROOT + "/valid"
-    migrator = assert_deprecated { ActiveRecord::MigrationContext.new(migrations_path, ActiveRecord::SchemaMigration) }
+    migrator = assert_deprecated { ActiveRecord::MigrationContext.new(migrations_path, ActiveRecord::SchemaMigration, ActiveRecord::InternalMetadata) }
     migrator.up
 
     assert_equal 3, migrator.current_version
@@ -671,21 +671,20 @@ def test_schema_migrations_table_name
   def test_internal_metadata_table_name
     original_internal_metadata_table_name = ActiveRecord::Base.internal_metadata_table_name
 
-    assert_equal "ar_internal_metadata", ActiveRecord::InternalMetadata.table_name
+    assert_equal "ar_internal_metadata", @internal_metadata.table_name
     ActiveRecord::Base.table_name_prefix = "p_"
     ActiveRecord::Base.table_name_suffix = "_s"
     Reminder.reset_table_name
-    assert_equal "p_ar_internal_metadata_s", ActiveRecord::InternalMetadata.table_name
+    assert_equal "p_ar_internal_metadata_s", @internal_metadata.table_name
     ActiveRecord::Base.internal_metadata_table_name = "changed"
     Reminder.reset_table_name
-    assert_equal "p_changed_s", ActiveRecord::InternalMetadata.table_name
+    assert_equal "p_changed_s", @internal_metadata.table_name
     ActiveRecord::Base.table_name_prefix = ""
     ActiveRecord::Base.table_name_suffix = ""
     Reminder.reset_table_name
-    assert_equal "changed", ActiveRecord::InternalMetadata.table_name
+    assert_equal "changed", @internal_metadata.table_name
   ensure
     ActiveRecord::Base.internal_metadata_table_name = original_internal_metadata_table_name
-    ActiveRecord::InternalMetadata.reset_table_name
     Reminder.reset_table_name
   end
 
@@ -714,7 +713,7 @@ def test_internal_metadata_stores_environment
   end
 
   def test_internal_metadata_stores_environment_when_migration_fails
-    @internal_metadata.delete_all
+    @internal_metadata.delete_all_entries
     current_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call
 
     migration = Class.new(ActiveRecord::Migration::Current) {
@@ -730,7 +729,7 @@ def migrate(x)
   end
 
   def test_internal_metadata_stores_environment_when_other_data_exists
-    @internal_metadata.delete_all
+    @internal_metadata.delete_all_entries
     @internal_metadata[:foo] = "bar"
 
     current_env     = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call