Plugin: "remove_invalid" doesn't clear _data column.

[james-em]

Brief Description

When assigning an invalid file with the plugin "remove_invalid" enabled, it removes the file but doesn't clear the _data column.

Actual behavior

model.file = ...invalid file

model.file
=> nil (OK)

model.file_data
=> {"id"=>"...", ... } (NOT OK)

model.save(validate: false)
=> It actually save the invalid file!!

Expected behavior

model.file_data
=> nil

model.save(validate: false)
=> Don't save the file

System configuration

Ruby version: 3.1.0

Shrine version: 3.4.0

Workaround monkey patching

module MonkeyPatchRemoveInvalidPlugin
  extend ActiveSupport::Concern

  included do
    alias_method :old_deassign, :deassign

    def deassign
      old_deassign
      record.send("#{attribute}=", nil)
    end
  end
end

Shrine::Plugins::RemoveInvalid::AttacherMethods.include MonkeyPatchRemoveInvalidPlugin

Testing

context "with valid file" do
      let(:file) { Rack::Test::UploadedFile.new("spec/fixtures/files/valid.png", "image/png") }

      it { expect(model.file).to be_present }
      it { expect(model.file_data).to be_present }
      it { expect(model).to be_valid }
    end

    context "with invalid file" do
      let(:file) { Rack::Test::UploadedFile.new("spec/fixtures/files/invalid.txt", "text/plain") }

      it { expect(model.file).to be_blank }
      it { expect(model.file_data).to be_blank } # <== Monkey Patching fixes this one 
      it { expect(model).not_to be_valid }
    end

[janko]

Could you post a self-contained script reproducing the issue from the template included in the contributing guide?

[james-em]

@janko

The bug will not be fixed?

[benkoshy]

@janko

The bug will not be fixed?

While the code samples given above are useful, if copied into the following template: https://github.com/shrinerb/shrine/blob/master/SELF_CONTAINED_EXAMPLE.md - it will be inordinately helpful.

[james-em]

@janko The bug will not be fixed?

While the code samples given above are useful, if copied into the following template: https://github.com/shrinerb/shrine/blob/master/SELF_CONTAINED_EXAMPLE.md - it will be inordinately helpful.

require "active_record"
require "shrine"
require "shrine/storage/memory"
require "down"

Shrine.storages = {
  cache: Shrine::Storage::Memory.new,
  store: Shrine::Storage::Memory.new,
}

Shrine.plugin :activerecord           # loads Active Record integration
Shrine.plugin :cached_attachment_data # enables retaining cached file across form redisplays
Shrine.plugin :restore_cached_data    # extracts metadata for assigned cached files
Shrine.plugin :determine_mime_type    # determine and store the actual MIME type of the file analyzed from file content
Shrine.plugin :validation_helpers, default_messages: {
  max_size: ->(max) { I18n.t("errors.file.max_size") },
  mime_type_inclusion: ->(list) { I18n.t("errors.file.mime_type_inclusion", list: list) }
}


class MyUploader < Shrine
  Attacher.validate do
    validate_mime_type(
      %w[application/pdf],
      message: I18n.t("activerecord.errors.messages.invalid_mime_type")
    )
    validate_max_size 2 * 1024 * 1024 * 1024 # 2GB
  end
end

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.connection.create_table(:posts) { |t| t.text :image_data }

class Post < ActiveRecord::Base
  include MyUploader::Attachment(:image)
end

puts "\n====== Without remove_invalid ======\n"

post = Post.new(image: Down.download("https://avatars.githubusercontent.com/u/63726983"))

puts "Image presence: #{post.image.present? ? "yes" : "no"}"
puts "Image Data: #{post.image_data}"
puts(post.valid? ? "Post is valid" : "Post is not valid")

puts("Saving: #{post.save ? "yes" : "no"}")


puts "\n\n====== With remove_invalid ======\n"

Shrine.plugin :remove_invalid         # remove and delete files that failed validation

post = Post.new(image: Down.download("https://avatars.githubusercontent.com/u/63726983"))

puts "Image presence: #{post.image.present? ? "yes" : "no"}"
puts "Image Data: #{post.image_data}"
puts(post.valid? ? "Post is valid" : "Post is not valid")

puts("Saving: #{post.save ? "yes" : "no"}")

source 'https://rubygems.org' do
    gem "activerecord"
    gem "sqlite3"
    gem "down"
    gem "shrine"
end

Output

====== Without remove_invalid ======
Image presence: yes
Image Data: {"id":"2a36893c33c72860814cda65893be303","storage":"cache","metadata":{"filename":"63726983","size":1540,"mime_type":"image/png"}}
Post is not valid
Saving: no


====== With remove_invalid ======
Image presence: no
Image Data: {"id":"a057c4be23af093520f126c1e30b4c3c","storage":"cache","metadata":{"filename":"63726983","size":1540,"mime_type":"image/png"}}
Post is not valid
Saving: no

[janko]

Thanks for providing a reproducible example, I will investigate.