Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QueryCache not working in Capybara transactional tests #36587

Closed
engwan opened this issue Jul 2, 2019 · 4 comments · Fixed by #36618
Closed

QueryCache not working in Capybara transactional tests #36587

engwan opened this issue Jul 2, 2019 · 4 comments · Fixed by #36618
Assignees
Milestone

Comments

@engwan
Copy link

engwan commented Jul 2, 2019

Steps to reproduce

When transactional_tests are enabled, requests to the Capybara web server don't have query cache enabled.

In the controller, ActiveRecord::Base.connection.query_cache_enabled is false

Expected behavior

The connection has query cache enabled

Actual behavior

The query cache middleware enables query caching on the pool. But since we have transactional_tests, when we get a connection, we return the shared connection (based on @lock_thread) instead of checking out a connection from the pool.

I think this means https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb#L13 would not be triggered, causing query cache to not be enabled for the request

Additional information

We want query cache enabled in test so that it is closer to what happens in development / production. We have tests that check for the number of uncached queries that are now failing after enabling transactional_tests.

System configuration

Rails version: 5.1.7

Ruby version: 2.6.3

@engwan
Copy link
Author

engwan commented Jul 2, 2019

cc @eileencodes I'm not sure if this is intended behavior?

@engwan
Copy link
Author

engwan commented Jul 4, 2019

Having this patch fixes the issue:

module ActiveRecord
  module ConnectionAdapters
    module QueryCache
      module ConnectionPoolConfiguration
        def enable_query_cache!
          @query_cache_enabled[connection_cache_key(@lock_thread || Thread.current)] = true
          connection.enable_query_cache! if active_connection?
        end

        def disable_query_cache!
          @query_cache_enabled.delete connection_cache_key(@lock_thread || Thread.current)
          connection.disable_query_cache! if active_connection?
        end

        def query_cache_enabled
          @query_cache_enabled[connection_cache_key(@lock_thread || Thread.current)]
        end
      end
    end

    class ConnectionPool
      def active_connection?
        @thread_cached_conns[connection_cache_key(@lock_thread || Thread.current)]
      end
    end
  end
end

@eileencodes
Copy link
Member

Do you want to send a PR @engwan?

@eileencodes eileencodes self-assigned this Jul 5, 2019
@eileencodes eileencodes added this to the 6.0.0 milestone Jul 5, 2019
@engwan
Copy link
Author

engwan commented Jul 5, 2019

Sure. I'll look at adding tests for this and send a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants