Clarify which private methods of DatabaseSelector are a 'public/stable' interface

rubyonrails-core
1

Reposting my question on Part 8: Multi db improvements, Adds basic automatic database switching to Rails by eileencodes · Pull Request #35073 · rails/rails · GitHub

ActiveRecord::Middleware::DatabaseSelector defines reading_request? as private method, yet it appear to be the correct way to handle my use case: I want to specify that a particular route is always connected to the leader.

Meanwhile, here’s what I did on Rails 6.0

config/application.rb

    require Rails.root.join 'lib', 'extensions', 'active_record', 'middleware_database_selector_patch'
    ActiveRecord::Middleware::DatabaseSelector.prepend(MiddlewareDatabaseSelectorPatch)
    if enable_request_db_selection
      config.active_record.database_selector = { delay: 2.seconds }
      config.active_record.database_resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver
      config.active_record.database_resolver_context = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session
    end

where my extension is defined as

module MiddlewareDatabaseSelectorPatch

  def self.prepended(base)
    super
    base.class_eval do
      class_attribute :leader_only_paths, instance_writer: false, default: Set.new
    end
  end

  private

  # https://github.com/rails/rails/blob/6-0-stable/activerecord/lib/active_record/middleware/database_selector.rb
  def reading_request?(request)
    if (is_reading = super)
      is_leader_only_path = leader_only_paths.any? {|leader_only_path|
        request.path&.start_with?(leader_only_path)
      }
      !is_leader_only_path
    else
      is_reading
    end
  end
end

so in config/routes.rb I can

  ActiveRecord::Middleware::DatabaseSelector.leader_only_paths << "/pghero"

I’m open to making a code or docs PR to Rails if core is interested

2 Likes
2

re @eileencodes comment here:

All the classes are designed to be overwritten.

I appreciate that they were written in this way, but I’m a bit confused by this advice. For DatabaseSelector::Resolver and DatabaseSelector::Resolver::Session it’s clear how you should overwrite them and then reference them in your app’s config. But for DatabaseSelector it’s not. I get that this is a power user feature anyway, but we went back and forth plenty of times and the right way to configure it, and don’t really love where we ended up. Rails can help make these decisions for us.

I can think of two ways to make it more clear:

  1. Make config.active_record.database_selector accept an instance of DatabaseSelector (or a subclass), rather than a config hash. Then you can put your config in that class if you want to.
  2. Move reading_request?(request) into DatabaseSelector::Resolver. Currently it lives in DatabaseSelector. Moving it will remove the main reason you’d want to overwrite DatabaseSelector. (This was my original idea, there’s a few more notes about why in my PR comment.)

I think #1 is a more comprehensive solution but it requires changing a public API. #2 seems like an easier win - I’d be happy & confident to do a PR for it.

1 Like