Looks interesting @ghiculescu!

This will allow to do Product.where(id: [...]).with_attached_image.with_all_variant_records ? (And will query once each table: products, attachments, blobs and variant_records?)

@vizcay good idea. Just updated accordingly. Product.where(id: [...]).with_attached_image is all you need, see this test.

does this work if the storage is aws s3?

@rickyPanzer this should work with any storage backend. It doesn't actually interact with it the storage service at all. If you're using tracked variants, that will allow you to store in your DB a pointer to a variant in the storage backend - this PR enables you to eagerly load those pointers. But both this and variant tracking itself should be backend agnostic.

Just coded against this PR last night, works great! I hope it gets merged in

@georgeclaghorn can I please ask for a review here?

Some news about it? I think that's a great feature and could help a lot for pages with lots of images on AS

@VinyLimaZ @rickyPanzer @vizcay can you share what use cases (/sample code) you have in mind for this feature? It would help me ensure that the current implementation will do what you need.

Is there any news about this? I was trying to do something like this:

@addresses.includes(user: [user_information: [avatar_attachment: [:blob, :variant_record]]])

I want to include the thumbnail variant so I don't have to request one by one in the serializer

Thanks!

Would love to keep this PR alive. @ghiculescu I've got a page that shows a few hundred thumbnails (50px variant of original) and there doesn't seem to be a way to avoid the N+1 unless I store the thumbnails as a separate attachment. Hope this can get in.

I'd love to keep it alive too! I think it's ready to go and the build is green, I'm just not sure how to get it merged in 🤷‍♂️

Just coded against this PR last night, works great! I hope it gets merged in

@rickyPanzer would you mind sharing how you monkeypatched the record method? I'm trying to code against this too.

I'd love to keep it alive too! I think it's ready to go and the build is green, I'm just not sure how to get it merged in 🤷‍♂️

@ghiculescu looks like there is one failing test on the current build, looks like an unrelated test may need to be updated?

@tosbourn looks like it was unrelated. I've rebased against main which should fix it 🤞

Update: all green.

I posted a blog this morning for those who want to monkeypatch an existing Rails install to gain this functionality. See the "Extra credit" section

@wbharding you might like the with_all_variant_records scope.

Any chance you can share the context around your calling code?

I think the Active Storage Way (tm) would be to do <%= image_tag feature.screenshot.variant(gravity: "Southeast", pointsize: "30", fill: "#999", draw: "text 0,0 'NoteApps.info'") %> in your view. Once the page is rendered, the browser will make a subsequent request to ActiveStorage::Representations::RedirectController which is where .processed.url is called.

I suspect one of the reasons this PR is being held up is because you only get N+1s if you do feature.screenshot.variant(gravity: "Southeast", pointsize: "30", fill: "#999", draw: "text 0,0 'NoteApps.info'").processed.url in your view/controller/etc. So I think it would be good to collect use cases for where this is necessary, to help the Rails team see the value of this PR.

Hey @ghiculescu sure thing! Really appreciate the work that you've been putting into figuring out the solution to this and getting it accepted.

The use case I describe in the blog post is for my site noteapps.info, where there are a few reasons that we are using processed.url:

We are running a site on Heroku (high variance in response time, high variance in transit between database and query) that is trying to render up to hundreds of images per page load, like this page, where any checkbox icon you hover on pops a screenshot. Since our screenshot URLs use a small watermark, and are rendered within a presenter (no access to typical view methods, all data returned as JSON), making a call with the deep include syntax and your improvement to record is the only way for us to avoid potentially tens of separate requests being made to our underpowered Heroku app server when browser goes to render all of the tens of images.

Another example is the app-specific page, where we need to look up about 150 images for a particular note taking app. If it weren't for processed.url, then scrolling down this page would deluge our app server in requests that became database connections and spiked our request volume. Being able to grab all of the variant blobs in the controller means that even with 150 images that contain variants (the text watermark), the page still has reasonable ActiveRecord performance characteristics and no N+1 queries.

Finally, we cache all of our pages as aggressively as possible with Cloudflare. Calculating the processed URLs before rendering the page ensures that these cached pages can be rendered and have all of their images loaded with no calls being made directly to our Heroku server. ✨

@pixeltrix Is there any chance to have a backport? Rails 7 seems very far and this would be a life-safer for us.

@collimarco I’d suggest running Rails against the main branch, it’s much less scary than you’d think and means you’ll basically get all the stuff that’s gonna be in Rails 7.

Ouch... I'm testing this PR in our application and it does not fix the N+1 issue for us :(

This is the code:

category.items.with_attached_image.each do |item|
  puts "https://cdn.example.com/" + item.image.variant({ resize: "100x100" }).key
end

• If inside the loop we call variant.processed we have only 1 query
• If inside the loop we call variant.key (or variant.processed.key) we have N+1 queries

This is the log output for each iteration if you call variant.key:

ActiveStorage::Attachment Load (0.3ms)  SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_id" = $1 AND "active_storage_attachments"."record_type" = $2 AND "active_storage_attachments"."name" = $3 LIMIT $4  [["record_id", 10], ["record_type", "ActiveStorage::VariantRecord"], ["name", "image"], ["LIMIT", 1]]
ActiveStorage::Blob Load (0.3ms)  SELECT "active_storage_blobs".* FROM "active_storage_blobs" WHERE "active_storage_blobs"."id" = $1 LIMIT $2  [["id", 100], ["LIMIT", 1]]

Basically ActiveStorage::Attachment and ActiveStorage::Blob are loaded again, even though they are already available.

#42981 fixes that ^

Ouch... I'm testing this PR in our application and it does not fix the N+1 issue for us :(

This is the code:

category.items.with_attached_image.each do |item|
  puts "https://cdn.example.com/" + item.image.variant({ resize: "100x100" }).key
end

• If inside the loop we call variant.processed we have only 1 query
• If inside the loop we call variant.key (or variant.processed.key) we have N+1 queries

This is the log output for each iteration if you call variant.key:

ActiveStorage::Attachment Load (0.3ms)  SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_id" = $1 AND "active_storage_attachments"."record_type" = $2 AND "active_storage_attachments"."name" = $3 LIMIT $4  [["record_id", 10], ["record_type", "ActiveStorage::VariantRecord"], ["name", "image"], ["LIMIT", 1]]
ActiveStorage::Blob Load (0.3ms)  SELECT "active_storage_blobs".* FROM "active_storage_blobs" WHERE "active_storage_blobs"."id" = $1 LIMIT $2  [["id", 100], ["LIMIT", 1]]

Basically ActiveStorage::Attachment and ActiveStorage::Blob are loaded again, even though they are already available.

I observe se exact same behavior on my end. This code does not seems to fix this N+1 issue. I'm running Rails '7.0.4' on my project. Is there anything I can do to help testing it ?