Multi database: define reading_request? in resolver

The current multi-database middleware defines [`reading_request?`](b925880914/activerecord/lib/active_record/middleware/database_selector.rb (L74)) which is true for GET and HEAD requests. If a request is not a `reading_request?`, it won't be sent to a replica via this middleware becuase the resolver's [`write`](b925880914/activerecord/lib/active_record/middleware/database_selector/resolver.rb (L44)) method will be called.

There may be use cases where you want to send POST/PUT/DELETE/<whatever> requests to the replica. An example of this is if you're providing a GraphQL server, you want all the queries to be sent as [POST requests](https://graphql.org/learn/serving-over-http/#post-request), even if they are reads. Currently to support this you need to either

- Make `Resolver#write` do more checks, or
- Override the middleware class / provide your own

This PR moves `reading_request?` from the middleware class to the resolver class. Since users are already encouraged to provide a resolver, it makes sense for the resolver to be able to determine if a request is a read request or not. The default implementation still just checks the request method, but you can change this to do whatever you like. A GraphQL example might look like:

```ruby
def reading_request?(request)
  graphql_read = request.post? && request.path == "/graphql" && !request.params[:query]&.include?("mutation")

  graphql_read || super
end
```
This commit is contained in:
Alex Ghiculescu 2022-04-23 11:55:09 +10:00
parent b925880914
commit 439119c8c5
4 changed files with 31 additions and 5 deletions

@ -1,3 +1,12 @@
* Allow overriding `reading_request?` in `DatabaseSelector::Resolver`
The default implementation checks if a request is a `get?` or `head?`,
but you can now change it to anything you like. If the method returns true,
`Resolver#read` gets called meaning the request could be served by the
replica database.
*Alex Ghiculescu*
* Remove `ActiveRecord.legacy_connection_handling`.
*Eileen M. Uchitelle*

@ -71,7 +71,7 @@ def select_database(request, &blk)
context = context_klass.call(request)
resolver = resolver_klass.call(context, options)
response = if reading_request?(request)
response = if resolver.reading_request?(request)
resolver.read(&blk)
else
resolver.write(&blk)
@ -80,10 +80,6 @@ def select_database(request, &blk)
resolver.update_context(response)
response
end
def reading_request?(request)
request.get? || request.head?
end
end
end
end

@ -48,6 +48,10 @@ def update_context(response)
context.save(response)
end
def reading_request?(request)
request.get? || request.head?
end
private
def read_from_primary(&blk)
ActiveRecord::Base.connected_to(role: ActiveRecord.writing_role, prevent_writes: true) do

@ -297,5 +297,22 @@ def test_the_middleware_chooses_reading_role_with_GET_request
assert_equal [200, {}, ["body"]], middleware.call("REQUEST_METHOD" => "GET")
end
class ReadonlyResolver < ActiveRecord::Middleware::DatabaseSelector::Resolver
def reading_request?(request)
true
end
end
def test_the_middleware_chooses_reading_role_with_POST_request_if_resolver_tells_it_to
middleware = ActiveRecord::Middleware::DatabaseSelector.new(lambda { |env|
assert ActiveRecord::Base.connected_to?(role: :reading)
[200, {}, ["body"]]
}, ReadonlyResolver)
cache = ActiveSupport::Cache::MemoryStore.new
middleware = ActionDispatch::Session::CacheStore.new(middleware, cache: cache, key: "_session_id")
assert_equal [200, {}, ["body"]], middleware.call("REQUEST_METHOD" => "POST")
end
end
end