If remove_invalid plugin is written before derivative plugin, derivative is deleted if validation fails

#1

HI.
I have been using shrine and it is very helpful. Thanks to all contributors.

I have a question this time.
I found strange behavior like the topic title. Is this a bug? or is it a specification? code and reproduction procedure is as follows.

environment
shrine 3.1.0
rails 6.0.0

image_uploader.rb

require "image_processing/mini_magick"

class ImageUploader < Shrine
  # OK
  # plugin :derivatives
  # plugin :remove_invalid
  # plugin :validation

  # NG
  plugin :remove_invalid
  plugin :derivatives
  plugin :validation

  Attacher.derivatives do |original|
    magick = ImageProcessing::MiniMagick.source(original)

    { 
      small:  magick.resize_to_limit!(100, 100)
    }
  end

  Attacher.validate do
    # DEBUG: always fail
    # errors << "fail"
  end
end

shrine.rb

require "shrine"
require "shrine/storage/file_system"

Shrine.storages = { 
  cache: Shrine::Storage::FileSystem.new("public", prefix: "uploads/cache"), # temporary 
  store: Shrine::Storage::FileSystem.new("public", prefix: "uploads"),       # permanent 
}

class Shrine::Attacher
  def promote(*)
    create_derivatives
    super
  end
end

Shrine.plugin :activerecord

show.html.erb

<p id="notice"><%= notice %></p>

<p>
  <strong>Name:</strong>
  <%= @user.name %>
</p>
<p>
  <strong>image_url:</strong>
  <%= image_tag @user.image_url %>
</p>
<p>
  <strong>image(:small).url:</strong>
  <%= image_tag @user.image(:small).url %>
</p>

<%= link_to 'Edit', edit_user_path(@user) %> |
<%= link_to 'Back', users_path %>

Reproduction procedure

  1. Register model.
    1

  2. Check the derivatives display.(image(:small).url)

    2
    2806×776 7.87 KB

  3. Check that the original image and derivative are in storage.

    storage
    storage737×345 23.5 KB

  4. Uncomment # errors << "fail" in image_uploader.rb

  5. Update model.
    3

  6. Validation error occurs.
    4

  7. Derivatives are deleted.

    5
    5742×752 6.39 KB

    storage
    storage2
    storage2738×412 35.8 KB

all code is here

#2

Hi, thanks for reporting, and I’m glad you’re finding Shrine useful :smiley:

Yes, this is definitely a bug, and it’s coming from the plugin load order. A simple workaround would be to load the derivatives plugin before the remove_invalid plugin:

# ...
plugin :derivatives
plugin :remove_invalid
# ...

I will try to come up with a fix that will make the opposite order work as well.

1 Like
#3

Actually, that wouldn’t work as well, because remove_invalid plugin wouldn’t assign back the original derivatives on validation errors, as I forgot to handle that case. I will include the fix for this as well.

1 Like
#4

Thank you for your time.
I look forward to the fix.:slightly_smiling_face:

#5

I’ve just pushed a fix for the remove_invalid + derivatives incompatibility to master, would you mind giving it a spin?

The next version (which will include this fix) should be released by the end of this week.

#6

I tried the same procedure using the master branch (@15bd6322).
Derivatives were not deleted. Looks like it ’s okay.
Thank you for your nice work!

1 Like