From ef6198d39474fa4f102d400f13eb032818455428 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 18 Jul 2025 15:43:31 -0400 Subject: [PATCH] Make sure the image variants we use are preprocessed to avoid implicitly creating a variant during a GET to a read replica, which will raise an exception and cause the image to fail to load. This feels pretty brittle, to be honest, and we'll need to be careful in the future about creating and using new variants; we will probably need to backfill the new variant for existing images because we can't guarantee that they will be created implicitly/lazily if the GET lands on the replica. ref: https://fizzy.37signals.com/5986089/collections/7/cards/942 --- app/models/concerns/attachments.rb | 19 ++++++++++++++++++ .../blobs/web/_representation.html.erb | 8 +++----- app/views/events/event/_attachments.html.erb | 8 ++++---- config/initializers/action_text.rb | 20 +++++++++++++++++++ 4 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 config/initializers/action_text.rb diff --git a/app/models/concerns/attachments.rb b/app/models/concerns/attachments.rb index de0dcea70..f4328600e 100644 --- a/app/models/concerns/attachments.rb +++ b/app/models/concerns/attachments.rb @@ -1,6 +1,25 @@ module Attachments extend ActiveSupport::Concern + # The variants we use must all declared here so that we can preprocess them. + # + # If they are not preprocessed, then Rails will attempt to transform the image on-the-fly when + # they are first viewed, which may be on the read replica where writing to the database is not + # allowed. Chaos will ensue if that happens. + # + # These variants are patched into ActionText::RichText in config/initializers/action_text.rb + VARIANTS = { + # vipsthumbnail used to create sized image variants has a intent setting to manage colors during + # resize. By setting an invalid intent value the gif-incompatible intent filtering is skipped and + # the gif can be rendered with all its frame intact. + # + # Only `n` is accepted as an override, using the full parameter name `intent` doesn’t work. + # + # This was cargo-culted from know-it-all and I imagine it may be fixed at some point. + small: { loader: { n: -1 }, resize_to_limit: [ 800, 600 ] }, + large: { loader: { n: -1 }, resize_to_limit: [ 1024, 768 ] } + } + def attachments rich_text_record&.embeds || [] end diff --git a/app/views/active_storage/blobs/web/_representation.html.erb b/app/views/active_storage/blobs/web/_representation.html.erb index 7b1106367..b8247e376 100644 --- a/app/views/active_storage/blobs/web/_representation.html.erb +++ b/app/views/active_storage/blobs/web/_representation.html.erb @@ -1,4 +1,4 @@ -<% size = local_assigns[:in_gallery] ? [800, 600] : [1024, 768] %> +<% variant = Attachments::VARIANTS[local_assigns[:in_gallery] ? :small : :large] %> <% width = blob.metadata["width"] %> <% height = blob.metadata["height"] %> @@ -14,10 +14,8 @@ -<% elsif blob.variable? %> - <%= image_tag url_for(blob.variant(loader: { n: -1 }, resize_to_limit: size)), width: width, height: height %> -<% elsif blob.previewable? %> - <%= image_tag url_for(blob.preview(resize_to_limit: size)), width: width, height: height %> +<% elsif blob.variable? || blob.previewable? %> + <%= image_tag url_for(blob.variant(variant)), width: width, height: height %> <% else %> <%= blob.filename.extension&.downcase.presence || "unknown" %> <% end %> diff --git a/app/views/events/event/_attachments.html.erb b/app/views/events/event/_attachments.html.erb index 8cca03864..9a255e8cb 100644 --- a/app/views/events/event/_attachments.html.erb +++ b/app/views/events/event/_attachments.html.erb @@ -1,14 +1,14 @@ <% if eventable&.has_attachments? %> <% eventable.attachments.each do |attachment| %> - <% size = [800, 600] %> + <% variant = Attachments::VARIANTS[:small] %> <% width = attachment.metadata["width"] %> <% height = attachment.metadata["height"] %> <% if attachment.previewable? %> - <%= image_tag url_for(attachment.preview(resize_to_limit: size)), class: "attachment attachment--image", width: width, height: height %> + <%= image_tag url_for(attachment.preview(variant)), class: "attachment attachment--image", width: width, height: height %> <% elsif attachment.variable? %> - <%= image_tag url_for(attachment.variant(resize_to_limit: size)), class: "attachment attachment--image", width: width, height: height %> + <%= image_tag url_for(attachment.variant(variant)), class: "attachment attachment--image", width: width, height: height %> <% else %>
<%= attachment.filename.extension&.downcase.presence || "unknown" %> @@ -16,4 +16,4 @@ <% end %> <% end %> -<% end %> \ No newline at end of file +<% end %> diff --git a/config/initializers/action_text.rb b/config/initializers/action_text.rb new file mode 100644 index 000000000..b2724b1c7 --- /dev/null +++ b/config/initializers/action_text.rb @@ -0,0 +1,20 @@ +module ActionText + module Extensions + module RichText + extend ActiveSupport::Concern + + included do + # This overrides the default :embeds association! + has_many_attached :embeds do |attachable| + ::Attachments::VARIANTS.each do |variant_name, variant_options| + attachable.variant variant_name, variant_options.merge(preprocessed: true) + end + end + end + end + end +end + +ActiveSupport.on_load(:action_text_rich_text) do + include ActionText::Extensions::RichText +end