Merge pull request rails#49378 from Shopify/ar-delay-version-check

Delay Adapter#check_version to #configure_connection

Author: byroot

actioncable/test/subscription_adapter/postgresql_test.rb

@@ -23,7 +23,7 @@ def setup
     ActiveRecord::Base.establish_connection database_config
 
     begin
-      ActiveRecord::Base.connection
+      ActiveRecord::Base.connection.connect!
     rescue
       @rx_adapter = @tx_adapter = nil
       skip "Couldn't connect to PostgreSQL: #{database_config.inspect}"

activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb

@@ -243,7 +243,7 @@ def with_connection
 
       # Returns true if a connection has already been opened.
       def connected?
-        synchronize { @connections.any? }
+        synchronize { @connections.any?(&:connected?) }
       end
 
       # Returns an array containing the connections currently in the pool.
@@ -682,7 +682,6 @@ def remove_connection_from_thread_cache(conn, owner_thread = conn.owner)
         def new_connection
           connection = Base.public_send(db_config.adapter_method, db_config.configuration_hash)
           connection.pool = self
-          connection.check_version
           connection
         rescue ConnectionNotEstablished => ex
           raise ex.set_pool(self)

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -668,6 +668,13 @@ def check_all_foreign_keys_valid!
 
       # CONNECTION MANAGEMENT ====================================
 
+      # Checks whether the connection to the database was established. This doesn't
+      # include checking whether the database is actually capable of responding, i.e.
+      # whether the connection is stale.
+      def connected?
+        !@raw_connection.nil?
+      end
+
       # Checks whether the connection to the database is still active. This includes
       # checking whether the database is actually capable of responding, i.e. whether
       # the connection isn't stale.
@@ -1215,6 +1222,7 @@ def build_result(columns:, rows:, column_types: {})
         # Implementations may assume this method will only be called while
         # holding @lock (or from #initialize).
         def configure_connection
+          check_version
         end
 
         def default_prepared_statements

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -870,6 +870,7 @@ def supports_rename_column?
         end
 
         def configure_connection
+          super
           variables = @config.fetch(:variables, {}).stringify_keys
 
           # Increase timeout so the server doesn't disconnect us.

activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb

@@ -130,6 +130,10 @@ def quote_string(string)
       # CONNECTION MANAGEMENT ====================================
       #++
 
+      def connected?
+        !(@raw_connection.nil? || @raw_connection.closed?)
+      end
+
       def active?
         !!@raw_connection&.ping
       end

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -337,6 +337,10 @@ def initialize(...)
         @use_insert_returning = @config.key?(:insert_returning) ? self.class.type_cast_config_to_boolean(@config[:insert_returning]) : true
       end
 
+      def connected?
+        !(@raw_connection.nil? || @raw_connection.finished?)
+      end
+
       # Is this connection alive and ready for queries?
       def active?
         @lock.synchronize do
@@ -616,7 +620,9 @@ def use_insert_returning?
 
       # Returns the version of the connected PostgreSQL server.
       def get_database_version # :nodoc:
-        valid_raw_connection.server_version
+        with_raw_connection do |conn|
+          conn.server_version
+        end
       end
       alias :postgresql_version :database_version
 
@@ -998,6 +1004,8 @@ def reconnect
         # Configures the encoding, verbosity, schema search path, and time zone of the connection.
         # This is called by #connect and should not be called manually.
         def configure_connection
+          super
+
           if @config[:encoding]
             @raw_connection.set_client_encoding(@config[:encoding])
           end

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -196,10 +196,12 @@ def supports_concurrent_connections?
         !@memory_database
       end
 
-      def active?
-        @raw_connection && !@raw_connection.closed?
+      def connected?
+        !(@raw_connection.nil? || @raw_connection.closed?)
       end
 
+      alias_method :active?, :connected?
+
       def return_value_after_insert?(column) # :nodoc:
         column.auto_populated?
       end
@@ -723,6 +725,8 @@ def configure_connection
             end
           end
 
+          super
+
           # Enforce foreign key constraints
           # https://www.sqlite.org/pragma.html#pragma_foreign_keys
           # https://www.sqlite.org/foreignkeys.html

activerecord/lib/active_record/connection_adapters/trilogy_adapter.rb

@@ -131,6 +131,10 @@ def quote_string(string)
         end
       end
 
+      def connected?
+        !(@raw_connection.nil? || @raw_connection.closed?)
+      end
+
       def active?
         connection&.ping || false
       rescue ::Trilogy::Error

activerecord/lib/active_record/tasks/sqlite_database_tasks.rb

@@ -70,6 +70,7 @@ def connection
 
         def establish_connection(config = db_config)
           ActiveRecord::Base.establish_connection(config)
+          connection.connect!
         end
 
         def run_cmd(cmd, args, out)

activerecord/lib/active_record/test_fixtures.rb

@@ -158,6 +158,7 @@ def setup_fixtures(config = ActiveRecord::Base)
           if connection_name
             begin
               connection = ActiveRecord::Base.connection_handler.retrieve_connection(connection_name, shard: shard)
+              connection.connect! # eagerly validate the connection
             rescue ConnectionNotEstablished
               connection = nil
             end