rails: eager load doesn't work as expected in Action Text

Steps to reproduce

create rich texts with attachments
access index action and watch logs

class Message < ApplicationRecord
  has_rich_text :content
end

class MessagesController < ApplicationController
  def index
    @messages = Message.with_rich_text_content_and_embeds
  end
end

<% @messages.each do |message| %>
  <%= message.content %>  
<% end %>

Expected behavior

Avoid N+1

Actual behavior

The query fetch blobs is executed as many times as the number of blobs. Because ActionText::Content has attachments as GlobalID and fetch it directly without using associations.

I think it is confusing behavior so it should be documented at least.

System configuration

Rails version:

6.0.0.rc1

Ruby version:

2.6.3

About this issue

Original URL
State: open
Created 5 years ago
Reactions: 5
Comments: 16 (10 by maintainers)

Commits related to this issue

Update Documentation - add method which eliminates N + 1 queries — committed to rails/rails by benkoshy 4 years ago

Most upvoted comments

Short explaination for the cause:

ActionText doesn’t load the blobs via relationship. It loads the blobs in following steps:

It parses the content into a tree structure.
It goes through each attachment node and find a way to display them.
If a node is a file attachment, it takes its sgid attribute and uses a gem called globalid to find the blob.

https://github.com/rails/rails/blob/99650550e8dbd7a2f3248d5a0231dda92f58b111/actiontext/lib/action_text/attachable.rb#L22-L26

And it’s essentially like

ActiveStorage::Blob.find(decoded_id)

Since blobs are found individually when going through each attachment element, it gets 0 benefit from the eager loading scope. In fact, in some cases, it’s even worse to use those scopes because it creates extra queries to eager load the data we couldn’t use.

I’m preparing a blog post which will explain how ActionText deal with uploaded files, which would help understanding the cause. However, I don’t think this is an issue that can be fixed in short time. It requries some redesign and more discussions.

(Update: here’s the link of the post I mentioned above. And I’m working on a potential fix right now)

st0012 on Aug 15, 2019

@antoniobg no any feedback atm. would you like to take a look at my spike PR for this issue? https://github.com/rails/rails/pull/36945

st0012 on Oct 3, 2019

@willnet I’ve opened a PR to demonstrate my solution for this issue, can you take a look?

st0012 on Aug 15, 2019