Add support for eager loading all rich text associations at once by swanson · Pull Request #39397 · rails/rails

swanson · 2020-05-22T23:02:22Z

Summary

ActionText provides with_rich_text_#{name} helpers to make it easy to preload rich text associations. This works great for single rich text fields on a model, but if you have multiple rich text fields, it will cause multiple queries to the ActionText table to load each one.

Consider the case of a model with lots of dynamic, user-generated content

class Page < ApplicationRecord
  has_rich_text :header
  has_rich_text :sub_header
  has_rich_text :content
  has_rich_text :aside
  has_rich_text :footer
  ...
end

When trying to display all of the content for a Page using eager loading:

Page
  .with_rich_text_header
  .with_rich_text_sub_header
  .with_rich_text_content
  .with_rich_text_aside
  .with_rich_text_footer
  .find(params[:id])

Rails will run a total of 6 queries (one to load Page, six individual ActionText loads).

This PR adds a with_all_rich_text that uses eager_load and reflects on has_rich_text associations to bulk load all rich text associations.

Page.with_all_rich_text.find(params[:id])

Other Information

Per #37976, ActionText internals are currently in-flux but wanted to see if there was interest in adding this feature in a future release, or if it fits better as an application-specific helper.

actiontext/test/unit/model_test.rb

swanson · 2020-05-25T02:12:47Z

actiontext/test/unit/model_test.rb

@dhh it seems that ActiveRecord::TestCase is not easy to use outside of the activerecord suite. I think this is preferably to putting the helpers directly in the single test and seems to follow the convention used in: actionview/test/activerecord/relation_cache_test.rb.

Open to other ideas if you have them. I wasn't ready to tackle more structural changes to ActiveRecord::TestCase based on my current understanding of the overall test suite.

I tried fiddling around with this again today, but seems like the preference is to duplicate the query helpers vs making the existing AR::TestCase more accessible: #36031 (comment)

rails-bot · 2020-08-23T02:15:42Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

…once

swanson · 2020-11-24T16:54:49Z

@dhh I've merged master, squashed, and added a CHANGELOG entry. Anything else to tackle on this PR?

kaspth

Sweet, very close!

actiontext/test/test_helper.rb

actiontext/lib/action_text/attribute.rb

Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>

kaspth · 2020-11-24T17:19:58Z

Sweet, thank you!

…s run by ruby 3.0.0dev This pull request addresses the `ArgumentError: missing keywords: :sql, :name`, which has been reported at https://buildkite.com/rails/rails/builds/72990#c78d1e4c-5775-4758-9010-9ba71fdb6138 Follow up rails#39397 ```ruby $ ruby -v ruby 3.0.0dev (2020-11-24T22:48:19Z master 63ad55cd88) [x86_64-linux] $ cd actiontext $ bin/test test/unit/model_test.rb -n test_eager_loading Run options: -n test_eager_loading --seed 16493 E Error: ActionText::ModelTest#test_eager_loading: ArgumentError: missing keywords: :sql, :name /home/yahonda/src/github.com/rails/rails/actiontext/test/test_helper.rb:29:in `block in assert_queries' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/fanout.rb:186:in `finish' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/fanout.rb:63:in `block in finish' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/fanout.rb:63:in `each' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/fanout.rb:63:in `finish' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/instrumenter.rb:45:in `finish_with_state' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/instrumenter.rb:30:in `instrument' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:688:in `log' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/sqlite3/database_statements.rb:46:in `exec_query' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:536:in `select_prepared' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:67:in `select_all' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:103:in `select_all' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/querying.rb:47:in `find_by_sql' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/relation.rb:850:in `block in exec_queries' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/relation.rb:868:in `skip_query_cache_if_necessary' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/relation.rb:835:in `exec_queries' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/relation.rb:638:in `load' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/relation.rb:249:in `records' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/relation.rb:244:in `to_ary' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/relation/finder_methods.rb:553:in `find_nth_with_limit' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/relation/finder_methods.rb:538:in `find_nth' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/relation/finder_methods.rb:122:in `first' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/relation/finder_methods.rb:153:in `last' /home/yahonda/src/github.com/rails/rails/actiontext/test/unit/model_test.rb:93:in `block (2 levels) in <class:ModelTest>' /home/yahonda/src/github.com/rails/rails/actiontext/test/test_helper.rb:33:in `assert_queries' /home/yahonda/src/github.com/rails/rails/actiontext/test/unit/model_test.rb:93:in `block in <class:ModelTest>' Error: ActionText::ModelTest#test_eager_loading: ArgumentError: missing keywords: :sql, :name /home/yahonda/src/github.com/rails/rails/actiontext/test/test_helper.rb:29:in `block in assert_queries' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/fanout.rb:186:in `finish' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/fanout.rb:63:in `block in finish' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/fanout.rb:63:in `each' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/fanout.rb:63:in `finish' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/instrumenter.rb:45:in `finish_with_state' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications/instrumenter.rb:30:in `instrument' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:688:in `log' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/sqlite3/database_statements.rb:98:in `exec_rollback_db_transaction' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:378:in `rollback_db_transaction' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:22:in `rollback_db_transaction' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb:205:in `rollback' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb:302:in `block in rollback_transaction' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb:300:in `rollback_transaction' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:328:in `rollback_transaction' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/test_fixtures.rb:168:in `block in teardown_fixtures' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/test_fixtures.rb:167:in `each' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/test_fixtures.rb:167:in `teardown_fixtures' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/test_fixtures.rb:16:in `after_teardown' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:51:in `after_teardown' bin/test test/unit/model_test.rb:90 Finished in 0.117256s, 8.5284 runs/s, 0.0000 assertions/s. 1 runs, 0 assertions, 0 failures, 1 errors, 0 skips $ ``` Co-authored-by: Ryuta Kamizono <kamipo@gmail.com>

dcangulo · 2020-11-25T05:39:04Z

@swanson Is this implemented in ActiveStorage? I think the same approach can be used if not.

swanson · 2020-12-08T15:38:47Z

@swanson Is this implemented in ActiveStorage? I think the same approach can be used if not.

It's not, only ActionText at the moment.

…ds on an attachment at once Currently Active Storage [variant tracking](rails#37901) runs a query for each attachment to check for a variant. Regular Rails n+1 prevention techniques (`includes`) cannot prevent this. This PR adds `with_all_variant_records`, as well as allowing `includes` to work as expected on Active Storage attachments. ```ruby user.vlogs.with_all_variant_records.map do |vlog| vlog.representation(resize: "100x100").processed end ``` Fixes several of the comments in rails#37901 In implementing this, I borrowed heavily from the style of rails#39397

Docs for rails#40842 + rails#39397 [ci skip]

…ds on an attachment at once Currently Active Storage [variant tracking](rails/rails#37901) runs a query for each attachment to check for a variant. Regular Rails n+1 prevention techniques (`includes`) cannot prevent this. This PR adds `with_all_variant_records`, as well as allowing `includes` to work as expected on Active Storage attachments. ```ruby user.vlogs.with_all_variant_records.map do |vlog| vlog.representation(resize: "100x100").processed end ``` Fixes several of the comments in rails/rails#37901 In implementing this, I borrowed heavily from the style of rails/rails#39397

Docs for rails/rails#40842 + rails/rails#39397 [ci skip]

rails-bot bot added the actiontext label May 22, 2020

swanson commented May 22, 2020

View reviewed changes

actiontext/test/unit/model_test.rb Outdated Show resolved Hide resolved

swanson changed the title ~~Spike out eager loading all rich text associations~~ Add support for eager loading all rich text associations at once May 22, 2020

dhh self-assigned this May 23, 2020

swanson commented May 23, 2020

View reviewed changes

actiontext/test/unit/model_test.rb Outdated Show resolved Hide resolved

swanson commented May 25, 2020

View reviewed changes

rails-bot bot added the stale label Aug 23, 2020

jonathanhefner added pinned and removed stale labels Aug 23, 2020

Add with_all_rich_text method to eager load all RichText models at …

5df83ae

…once

swanson force-pushed the multi-actiontext-eager-loading branch from 75295c7 to 5df83ae Compare November 24, 2020 16:53

kaspth reviewed Nov 24, 2020

View reviewed changes

actiontext/test/test_helper.rb Outdated Show resolved Hide resolved

actiontext/lib/action_text/attribute.rb Outdated Show resolved Hide resolved

matt swanson and others added 2 commits November 24, 2020 12:04

Update actiontext/test/test_helper.rb

e37d423

Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>

Update actiontext/lib/action_text/attribute.rb

5bcff18

Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>

kaspth merged commit 9b6459a into rails:master Nov 24, 2020

yahonda mentioned this pull request Nov 25, 2020

Address "ArgumentError: missing keywords: :sql, :name" with ruby 3.0.0dev #40684

Merged

ghiculescu mentioned this pull request Dec 14, 2020

Add support for eager loading Active Storage variants #40842

Merged

swanson deleted the multi-actiontext-eager-loading branch December 29, 2020 03:42

ghiculescu added a commit to ghiculescu/rails that referenced this pull request Jun 14, 2021

Document Active Storage & Action Text eager loading

7f348eb

Docs for rails#40842 + rails#39397 [ci skip]

ghiculescu mentioned this pull request Jun 14, 2021

Document Active Storage & Action Text eager loading #42477

Merged

jyoun-godaddy pushed a commit to jyoun-godaddy/activestorage that referenced this pull request Jul 5, 2022

Document Active Storage & Action Text eager loading

3366eb2

Docs for rails/rails#40842 + rails/rails#39397 [ci skip]

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for eager loading all rich text associations at once#39397

Add support for eager loading all rich text associations at once#39397
kaspth merged 3 commits intorails:masterfrom
swanson:multi-actiontext-eager-loading

swanson commented May 22, 2020

Uh oh!

Uh oh!

Uh oh!

swanson May 25, 2020

Uh oh!

swanson Nov 24, 2020

Uh oh!

rails-bot bot commented Aug 23, 2020

Uh oh!

swanson commented Nov 24, 2020

Uh oh!

kaspth left a comment

Uh oh!

Uh oh!

Uh oh!

kaspth commented Nov 24, 2020

Uh oh!

dcangulo commented Nov 25, 2020

Uh oh!

swanson commented Dec 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

swanson commented May 22, 2020

Summary

Other Information

Uh oh!

Uh oh!

Uh oh!

swanson May 25, 2020

Choose a reason for hiding this comment

Uh oh!

swanson Nov 24, 2020

Choose a reason for hiding this comment

Uh oh!

rails-bot bot commented Aug 23, 2020

Uh oh!

swanson commented Nov 24, 2020

Uh oh!

kaspth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kaspth commented Nov 24, 2020

Uh oh!

dcangulo commented Nov 25, 2020

Uh oh!

swanson commented Dec 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants