shrinerb / shrine

File Attachment toolkit for Ruby applications
https://shrinerb.com
MIT License
3.19k stars 275 forks source link

Derivatives depends on `store_key` proc, but it is only defined in the `default_storage` plugin #698

Closed prem-prakash closed 2 months ago

prem-prakash commented 2 months ago

https://github.com/shrinerb/shrine/blob/1ce6da4be0ca62ecd5776f107fcd0d657b773f28/lib/shrine/plugins/derivatives.rb#L19-L21

I am having an issue where the data for derivatives is being registered with two storage keys, one empty and another with the "store". Because of that the JSON is not parsed and it ends up being treated as a string, so I got values like this in the database field:

"\"{\\\"id\\\": \\\"observation_photos/edited_images/00c5e118-1f2d-4506-bee0-b7eedc2dc25c/original/data\\\", \\\"storage\\\":
 \\\"store\\\", \\\"metadata\\\": {\\\"size\\\": 300808, \\\"filename\\\": \\\"data\\\", \\\"mime_type\\\": \\\"image/jpeg\\\", 
\\\"updated_at\\\": \\\"2024-07-20T16:17:59Z\\\"}, \\\"derivatives\\\": {\\\"small\\\": {\\\"id\\\": 
\\\"observation_photos/edited_images/00c5e118-1f2d-4506-bee0-b7eedc2dc25c/small/data\\\", \\\"storage\\\": \\\", 
\\\"storage\\\": \\\"store\\\", \\\"metadata\\\": {}}}}\""

Note that there 2 storage keys, one without value and another one with store.

So I get this error when trying to update the record:

Screenshot 2024-08-25 at 12 14 36

I am not sure the problem is related to the fact that derivatives plugin depends on store_key proc, but it is only defined in the default_storage plugin.

This issue is not affecting all the records, it is affecting a very small portion, so this behaviour is for sure derived from some business logic of my application.

# config/initializers/shrine.rb

require "shrine"

case Rails.env
when "production", "development", "staging"
  require "shrine/storage/s3_aws_sdk_v2"

  s3_options = {
    bucket: ENV.fetch("S3_BUCKET_NAME"),
    region: ENV.fetch("AWS_REGION"),
    access_key_id: ENV.fetch("AWS_ACCESS_KEY_ID"),
    secret_access_key: ENV.fetch("AWS_SECRET_ACCESS_KEY")
  }

  Shrine.storages = {
    cache: Shrine::Storage::S3.new(public: true, **s3_options, prefix: "cache"),
    store: Shrine::Storage::S3.new(public: true, **s3_options)
  }

  Shrine.plugin :instrumentation,
                log_subscriber: lambda { |event|
                                  Shrine.logger.info(JSON.generate(
                                                       name: event.name,
                                                       duration: event.duration,
                                                       **event.payload
                                                     ))
                                }
when "test"
  require "shrine/storage/memory"

  Shrine.storages = {
    cache: Shrine::Storage::Memory.new,
    store: Shrine::Storage::Memory.new,
  }
else
  raise StandardError.new("Invalid environment #{Rails.env}. Shrine needs a valid environment.")
end

Shrine.plugin :activerecord
Shrine.plugin :cached_attachment_data # for retaining the cached file across form redisplays
Shrine.plugin :restore_cached_data # re-extract metadata when attaching a cached file
Shrine.plugin :derivatives
Shrine.plugin :determine_mime_type
Shrine.plugin :pretty_location
Shrine.plugin :url_options, store: { host: "https://#{ENV["ASSET_HOST"]}" }
Shrine.plugin :type_predicates, mime: :mini_mime
Shrine.plugin :keep_files

module PaperclipShrineSynchronization
  def self.included(model)
    model.before_save do
      # Paperclip::AttachmentRegistry.each_definition is a function that loops
      # trought attachment definitions for each model
      #
      # https://github.com/thoughtbot/paperclip/blob/v4.3.1/lib/paperclip/attachment_registry.rb#L45
      #
      # this code will break when paperclip is removed
      #
      Paperclip::AttachmentRegistry.definitions_for(self.class).each_key do |name|
        write_shrine_data(name) if changes.key?(:"#{name}_file_name")
      end
    end
  end

  def write_shrine_data(name)
    attachment = send(name)
    attacher   = Shrine::Attacher.from_model(self, name)

    if attachment.size.present?
      attacher.set shrine_file(attachment)

      attachment.styles.each do |style_name, style|
        attacher.merge_derivatives(style_name => shrine_file(style))
      end
    else
      attacher.set nil
    end
  end

  private

  def shrine_file(object)
    if object.is_a?(Paperclip::Attachment)
      Shrine::UploadedFile.new(
        storage: :store,
        id: object.path.delete_prefix("/"),
        metadata: {
          "size" => object.size,
          "filename" => object.original_filename,
          "mime_type" => object.content_type,
          "updated_at" => calculate_updated_at_for_shrine_file(object).utc.iso8601
        }
      )
    else
      Shrine::UploadedFile.new(
        storage: :store,
        id: object.attachment.path(object.name).delete_prefix("/"),
        metadata: {},
      )
    end
  end

  def calculate_updated_at_for_shrine_file(object)
    object.instance.try("#{object.name}_updated_at") || object.instance.updated_at || Time.current
  end
end
janko commented 2 months ago

Could you post a minimal self-contained example reproducing the issue? You can use this template as a starting point.

prem-prakash commented 2 months ago

@janko unfortunately I haven't been able to reproduce the problem yet, it's happening randomly, or rather, without an identified cause. I'll dig deeper into the research and come back with a reproducible example.