ActiveRecord validations are ignored

[re.vkol...@gmail.com]

Hi!

Thanks for the great library, I find it the best of the Ruby attachments libraries.

I met some strange behavior of my models after including Shrine attachment. There is a short self-contained example of this problem:

```ruby

# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do

  gem 'activerecord', '5.2.3'

  gem 'mysql2'

  gem 'fastimage'

  gem 'shrine', '2.16.0'

end

require 'active_record'

require 'shrine'

require 'shrine/storage/file_system'

ActiveRecord::Base.establish_connection('mysql2://ro...@127.0.0.1/shrine_test')

ActiveRecord::Base.connection.create_table(:fake_users) do |table|

  table.text :name

  table.text :image_data

end unless ActiveRecord::Base.connection.table_exists?(:fake_users)

Shrine.plugin :activerecord

Shrine.storages = {

  cache: Shrine::Storage::FileSystem.new(__dir__, prefix: 'assets'),

  store: Shrine::Storage::FileSystem.new(__dir__, prefix: 'assets')

}

class Uploader < ::Shrine

end

class FakeUser < ActiveRecord::Base

  include Uploader::Attachment.new(:image)

  validates :name, length: { maximum: 6 }

end

FakeUser.destroy_all

FakeUser.create(name: 'short', image: File.open('image.jpg'))

user = FakeUser.first

user.save # it's required

puts user.reload.name # => short

user.image = File.open('image.jpg')

user.name = 'too_long'

FakeUser.transaction { user.save } # transaction is required

puts user.reload.name # => too_long

```

It escapes AR validations! But how and why? And it happens only with Shrine attachment included into the model.

[Janko Marohnić]

Hi Vasily,

Sorry, your report fell off my radar, but I guess that late is still better then never :P

Thank you for the inline comments, they really helped me track this down. This appears to be a bug in Active Record (still present in 6.0.0.rc2), where under specific circumstances shown in your script, the `after_commit on: :update` callback gets called on `ActiveRecord::Base#save` even when validation failed.

Here is the script that shows this behaviour without Shrine:

  require "bundler/inline"

  gemfile(true) do

    gem "activerecord", "6.0.0.rc2"

    gem "sqlite3"

  end

  require "active_record"

  ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")

  ActiveRecord::Base.connection.create_table(:users) do |t|

    t.text :name

  end

  class User < ActiveRecord::Base

    validates :name, length: { maximum: 6 }

    after_commit on: :update do

      puts "after_commit"

    end

  end

  User.create(name: "short")

  user = User.first

  user.save

  user.name = "too_long"

  User.transaction { user.save }

The "after_commit" message should be printed only once (for the first `user.save`), but it's printed also for the second `user.save` that's wrapped inside the transaction. As you indicated, if I remove

  user = User.first

  user.save

or if I remove

  User.transaction { ... }

the bug doesn't occur. I'll file an issue on rails/rails, and we'll see what they say.

I don't see any easy workaround on the Shrine side for this. Shrine does use `save(validate: false)` in the after_commit, but it's needed for scenarios where people want to save the record with validation errors (see #55. And it wouldn't entirely fix the problem, as file promotion would still happen, even if the changes won't be persisted. So, I'll have to categorize it as "needs to be fixed in Rails".

Hope that helps.

Kind regards,

Janko

--
You received this message because you are subscribed to the Google Groups "Shrine" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ruby-shrine...@googlegroups.com.
To post to this group, send email to ruby-...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ruby-shrine/6c0a9a92-72ba-432d-9548-82407f0e955f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.