From 432698ef2befb7a736a54bd69c1a8d646b9eb727 Mon Sep 17 00:00:00 2001 From: aaron Date: Fri, 18 Dec 2020 13:58:11 -0700 Subject: [PATCH] Fix `SELECT COUNT` queries when rendering ActiveRecord collections (#40870) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix `SELECT COUNT` queries when rendering ActiveRecord collections Fixes #40837 When rendering collections, calling `size` when the collection is an ActiveRecord relation causes unwanted `SELECT COUNT(*)` queries. This change ensures the collection is an array before getting the size, and also loads the relation for any further array inspections. * Test queries when rendering relation collections * Add `length` support to partial collection iterator Allows getting the size of a relation without duplicating records, but still loads the relation. The length method existence needs to be checked because you can pass in an `Enumerator`, which does not respond to `length`. * Ensure unsubscribed from notifications after tests [Rafael Mendonça França + aar0nr] --- .../renderer/collection_renderer.rb | 6 +++- .../partial_rendering_query_test.rb | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 actionview/test/activerecord/partial_rendering_query_test.rb diff --git a/actionview/lib/action_view/renderer/collection_renderer.rb b/actionview/lib/action_view/renderer/collection_renderer.rb index ad4d7d2fc6..97b0a2cea1 100644 --- a/actionview/lib/action_view/renderer/collection_renderer.rb +++ b/actionview/lib/action_view/renderer/collection_renderer.rb @@ -47,6 +47,10 @@ module ActionView def size @collection.size end + + def length + @collection.respond_to?(:length) ? @collection.length : size + end end class SameCollectionIterator < CollectionIterator # :nodoc: @@ -144,7 +148,7 @@ module ActionView "render_collection.action_view", identifier: identifier, layout: layout && layout.virtual_path, - count: collection.size + count: collection.length ) do |payload| spacer = if @options.key?(:spacer_template) spacer_template = find_template(@options[:spacer_template], @locals.keys) diff --git a/actionview/test/activerecord/partial_rendering_query_test.rb b/actionview/test/activerecord/partial_rendering_query_test.rb new file mode 100644 index 0000000000..053aba6f1c --- /dev/null +++ b/actionview/test/activerecord/partial_rendering_query_test.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require "active_record_unit" + +class PartialRenderingQueryTest < ActiveRecordTestCase + def setup + @view = ActionView::Base + .with_empty_template_cache + .with_view_paths(ActionController::Base.view_paths, {}) + + @queries = [] + + @subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*, payload| + @queries << payload[:sql] unless %w[ SCHEMA TRANSACTION ].include?(payload[:name]) + end + end + + def teardown + ActiveSupport::Notifications.unsubscribe(@subscriber) + end + + def test_render_with_relation_collection + @view.render partial: "topics/topic", collection: Topic.all + + assert_equal 1, @queries.size + assert_equal 'SELECT "topics".* FROM "topics"', @queries[0] + end +end