Add multi-database support to Statesman. (#522)

Statesman currently relies heavily on ActiveRecord::Base, either explicitly
or implicitly, when querying database connections. This doesn't play well
when we have models or transitions which live on a different database, since
it forces us to open a query to the primary connection pool.

This is a series of changes to allow Statesman to use the context it has
available for the model or transition class, and make use of the appropriate
connection.

Co-authored-by: Amey Kusurkar <amey1000@gmail.com>

lib/generators/statesman/generator_helpers.rb

@@ -52,7 +52,7 @@ def configuration
     end
 
     def database_supports_partial_indexes?
-      Statesman::Adapters::ActiveRecord.database_supports_partial_indexes?
+      Statesman::Adapters::ActiveRecord.database_supports_partial_indexes?(klass.constantize)
     end
 
     def metadata_default_value

lib/statesman.rb

@@ -34,10 +34,8 @@ def self.storage_adapter
     @storage_adapter || Adapters::Memory
   end
 
-  def self.mysql_gaplock_protection?
-    return @mysql_gaplock_protection unless @mysql_gaplock_protection.nil?
-
-    @mysql_gaplock_protection = config.mysql_gaplock_protection?
+  def self.mysql_gaplock_protection?(connection)
+    config.mysql_gaplock_protection?(connection)
   end
 
   def self.config

lib/statesman/adapters/active_record.rb

@@ -7,19 +7,15 @@ module Adapters
     class ActiveRecord
       JSON_COLUMN_TYPES = %w[json jsonb].freeze
 
-      def self.database_supports_partial_indexes?
+      def self.database_supports_partial_indexes?(model)
         # Rails 3 doesn't implement `supports_partial_index?`
-        if ::ActiveRecord::Base.connection.respond_to?(:supports_partial_index?)
-          ::ActiveRecord::Base.connection.supports_partial_index?
+        if model.connection.respond_to?(:supports_partial_index?)
+          model.connection.supports_partial_index?
         else
-          ::ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
+          model.connection.adapter_name.casecmp("postgresql").zero?
         end
       end
 
-      def self.adapter_name
-        ::ActiveRecord::Base.connection.adapter_name.downcase
-      end
-
       def initialize(transition_class, parent_model, observer, options = {})
         serialized = serialized?(transition_class)
         column_type = transition_class.columns_hash["metadata"].sql_type
@@ -88,10 +84,10 @@ def create_transition(from, to, metadata)
           default_transition_attributes(to, metadata),
         )
 
-        ::ActiveRecord::Base.transaction(requires_new: true) do
+        transition_class.transaction(requires_new: true) do
           @observer.execute(:before, from, to, transition)
 
-          if mysql_gaplock_protection?
+          if mysql_gaplock_protection?(transition_class.connection)
             # We save the transition first with most_recent falsy, then mark most_recent
             # true after to avoid letting MySQL acquire a next-key lock which can cause
             # deadlocks.
@@ -130,8 +126,8 @@ def default_transition_attributes(to, metadata)
       end
 
       def add_after_commit_callback(from, to, transition)
-        ::ActiveRecord::Base.connection.add_transaction_record(
-          ActiveRecordAfterCommitWrap.new do
+        transition_class.connection.add_transaction_record(
+          ActiveRecordAfterCommitWrap.new(transition_class.connection) do
             @observer.execute(:after_commit, from, to, transition)
           end,
         )
@@ -144,17 +140,19 @@ def transitions_for_parent
       # Sets the given transition most_recent = t while unsetting the most_recent of any
       # previous transitions.
       def update_most_recents(most_recent_id = nil)
-        update = build_arel_manager(::Arel::UpdateManager)
+        update = build_arel_manager(::Arel::UpdateManager, transition_class)
         update.table(transition_table)
         update.where(most_recent_transitions(most_recent_id))
         update.set(build_most_recents_update_all_values(most_recent_id))
 
         # MySQL will validate index constraints across the intermediate result of an
         # update. This means we must order our update to deactivate the previous
         # most_recent before setting the new row to be true.
-        update.order(transition_table[:most_recent].desc) if mysql_gaplock_protection?
+        if mysql_gaplock_protection?(transition_class.connection)
+          update.order(transition_table[:most_recent].desc)
+        end
 
-        ::ActiveRecord::Base.connection.update(update.to_sql)
+        transition_class.connection.update(update.to_sql(transition_class))
       end
 
       def most_recent_transitions(most_recent_id = nil)
@@ -223,7 +221,7 @@ def most_recent_value(most_recent_id)
         if most_recent_id
           Arel::Nodes::Case.new.
             when(transition_table[:id].eq(most_recent_id)).then(db_true).
-            else(not_most_recent_value).to_sql
+            else(not_most_recent_value).to_sql(transition_class)
         else
           Arel::Nodes::SqlLiteral.new(not_most_recent_value)
         end
@@ -233,11 +231,11 @@ def most_recent_value(most_recent_id)
       # change in Arel as we move into Rails >6.0.
       #
       # https://github.com/rails/rails/commit/7508284800f67b4611c767bff9eae7045674b66f
-      def build_arel_manager(manager)
+      def build_arel_manager(manager, engine)
         if manager.instance_method(:initialize).arity.zero?
           manager.new
         else
-          manager.new(::ActiveRecord::Base)
+          manager.new(engine)
         end
       end
 
@@ -258,7 +256,7 @@ def transition_conflict_error?(err)
       end
 
       def unique_indexes
-        ::ActiveRecord::Base.connection.
+        transition_class.connection.
           indexes(transition_class.table_name).
           select do |index|
             next unless index.unique
@@ -329,16 +327,16 @@ def default_timezone
         ::ActiveRecord::Base.default_timezone
       end
 
-      def mysql_gaplock_protection?
-        Statesman.mysql_gaplock_protection?
+      def mysql_gaplock_protection?(connection)
+        Statesman.mysql_gaplock_protection?(connection)
       end
 
       def db_true
-        ::ActiveRecord::Base.connection.quote(type_cast(true))
+        transition_class.connection.quote(type_cast(true))
       end
 
       def db_false
-        ::ActiveRecord::Base.connection.quote(type_cast(false))
+        transition_class.connection.quote(type_cast(false))
       end
 
       def db_null
@@ -348,7 +346,7 @@ def db_null
       # Type casting against a column is deprecated and will be removed in Rails 6.2.
       # See https://github.com/rails/arel/commit/6160bfbda1d1781c3b08a33ec4955f170e95be11
       def type_cast(value)
-        ::ActiveRecord::Base.connection.type_cast(value)
+        transition_class.connection.type_cast(value)
       end
 
       # Check whether the `most_recent` column allows null values. If it doesn't, set old
@@ -368,9 +366,9 @@ def not_most_recent_value(db_cast: true)
     end
 
     class ActiveRecordAfterCommitWrap
-      def initialize(&block)
+      def initialize(connection, &block)
         @callback = block
-        @connection = ::ActiveRecord::Base.connection
+        @connection = connection
       end
 
       def self.trigger_transactional_callbacks?

lib/statesman/adapters/active_record_queries.rb

@@ -153,7 +153,7 @@ def most_recent_transition_alias
         end
 
         def db_true
-          ::ActiveRecord::Base.connection.quote(true)
+          model.connection.quote(true)
         end
       end
     end

lib/statesman/config.rb

@@ -15,17 +15,10 @@ def storage_adapter(adapter_class)
       @adapter_class = adapter_class
     end
 
-    def mysql_gaplock_protection?
-      return @mysql_gaplock_protection unless @mysql_gaplock_protection.nil?
-
+    def mysql_gaplock_protection?(connection)
       # If our adapter class suggests we're using mysql, enable gaplock protection by
       # default.
-      enable_mysql_gaplock_protection if mysql_adapter?(adapter_class)
-      @mysql_gaplock_protection
-    end
-
-    def enable_mysql_gaplock_protection
-      @mysql_gaplock_protection = true
+      mysql_adapter?(connection)
     end
 
     private
@@ -34,7 +27,7 @@ def mysql_adapter?(adapter_class)
       adapter_name = adapter_name(adapter_class)
       return false unless adapter_name
 
-      adapter_name.start_with?("mysql")
+      adapter_name.downcase.start_with?("mysql")
     end
 
     def adapter_name(adapter_class)

lib/tasks/statesman.rake

@@ -21,8 +21,8 @@ namespace :statesman do
     batch_size = 500
 
     parent_class.find_in_batches(batch_size: batch_size) do |models|
-      ActiveRecord::Base.transaction(requires_new: true) do
-        if Statesman::Adapters::ActiveRecord.database_supports_partial_indexes?
+      transition_class.transaction(requires_new: true) do
+        if Statesman::Adapters::ActiveRecord.database_supports_partial_indexes?(transition_class)
           # Set all transitions' most_recent to FALSE
           transition_class.where(parent_fk => models.map(&:id)).
             update_all(most_recent: false, updated_at: updated_at)

spec/generators/statesman/active_record_transition_generator_spec.rb

@@ -4,6 +4,13 @@
 require "generators/statesman/active_record_transition_generator"
 
 describe Statesman::ActiveRecordTransitionGenerator, type: :generator do
+  before do
+    stub_const("Bacon", Class.new(ActiveRecord::Base))
+    stub_const("BaconTransition", Class.new(ActiveRecord::Base))
+    stub_const("Yummy::Bacon", Class.new(ActiveRecord::Base))
+    stub_const("Yummy::BaconTransition", Class.new(ActiveRecord::Base))
+  end
+
   it_behaves_like "a generator" do
     let(:migration_name) { "db/migrate/create_bacon_transitions.rb" }
   end

spec/generators/statesman/migration_generator_spec.rb

@@ -4,6 +4,11 @@
 require "generators/statesman/migration_generator"
 
 describe Statesman::MigrationGenerator, type: :generator do
+  before do
+    stub_const("Yummy::Bacon", Class.new(ActiveRecord::Base))
+    stub_const("Yummy::BaconTransition", Class.new(ActiveRecord::Base))
+  end
+
   it_behaves_like "a generator" do
     let(:migration_name) { "db/migrate/add_statesman_to_bacon_transitions.rb" }
   end

spec/spec_helper.rb

@@ -5,13 +5,14 @@
 require "mysql2"
 require "pg"
 require "active_record"
+require "active_record/database_configurations"
 # We have to include all of Rails to make rspec-rails work
 require "rails"
 require "action_view"
 require "action_dispatch"
 require "action_controller"
 require "rspec/rails"
-require "support/active_record"
+require "support/exactly_query_databases"
 require "rspec/its"
 require "pry"
 
@@ -28,10 +29,31 @@ def connection_failure
   if config.exclusion_filter[:active_record]
     puts "Skipping ActiveRecord tests"
   else
-    # Connect to the database for activerecord tests
-    db_conn_spec = ENV["DATABASE_URL"]
-    db_conn_spec ||= { adapter: "sqlite3", database: ":memory:" }
-    ActiveRecord::Base.establish_connection(db_conn_spec)
+    current_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call
+
+    # We have to parse this to a hash since ActiveRecord::Base.configurations
+    # will only consider a single URL config.
+    url_config = if ENV["DATABASE_URL"]
+                   ActiveRecord::DatabaseConfigurations::ConnectionUrlResolver.
+                     new(ENV["DATABASE_URL"]).to_hash.merge({ sslmode: "disable" })
+                 end
+
+    db_config = {
+      current_env => {
+        primary: url_config || {
+          adapter: "sqlite3",
+          database: "/tmp/statesman.db",
+        },
+        secondary: url_config || {
+          adapter: "sqlite3",
+          database: "/tmp/statesman.db",
+        },
+      },
+    }
+
+    # Connect to the primary database for activerecord tests.
+    ActiveRecord::Base.configurations = db_config
+    ActiveRecord::Base.establish_connection(:primary)
 
     db_adapter = ActiveRecord::Base.connection.adapter_name
     puts "Running with database adapter '#{db_adapter}'"
@@ -40,6 +62,8 @@ def connection_failure
     ActiveRecord::Migration.verbose = false
   end
 
+  # Since our primary and secondary connections point to the same database, we don't
+  # need to worry about applying these actions to both.
   config.before(:each, :active_record) do
     tables = %w[
       my_active_record_models
@@ -53,6 +77,7 @@ def connection_failure
     ]
     tables.each do |table_name|
       sql = "DROP TABLE IF EXISTS #{table_name};"
+
       ActiveRecord::Base.connection.execute(sql)
     end
 
@@ -84,3 +109,6 @@ def prepare_sti_transitions_table
     end
   end
 end
+
+# We have to require this after the databases are configured.
+require "support/active_record"