Persist deduplicated filename with ActiveRecord

[krasnoukhov]

I noticed an issue that when using ActiveRecord and trying to update the mounted file with a different file of the same name it is deduplicated logically but that change is not persisted, resulting in current_path or url pointing to the "old" filename which does not exist (cause it was deleted). So deduplication works but it's not persisted. There was a spec for this case but in spec record was not reloaded (dirty) so it passed. Spec failure before code changes:

Failures:

  1) CarrierWave::ActiveRecord#mount_uploader removing old files normally should give a different name to new file and remove the old file
     Failure/Error: expect(@event.image.current_path).to eq public_path('uploads/old(2).jpeg')
     
       expected: "/Users/krasnoukhov/Projects/Simplepractice/carrierwave/spec/public/uploads/old(2).jpeg"
            got: "/Users/krasnoukhov/Projects/Simplepractice/carrierwave/spec/public/uploads/old.jpeg"
     
       (compared using ==)
     # ./spec/orm/activerecord_spec.rb:779:in `block (4 levels) in <top (required)>'

Changing to before_save makes write_identifier also kick in before record is persisted which makes issue go away. Not sure if before_save can have other complications, but it seems like it's working... The alternative would be to move this deduplicate call to happen before save:

carrierwave/lib/carrierwave/mounter.rb

Line 145 in 478eccc

uploader.deduplicate(existing_paths)

Another alternative would be to perform another save after store.

[krasnoukhov]

I tried this in our app and it does not seem to be a solution since we rely on model.id for filename so store must happen after save...

[krasnoukhov]

Ok I guess there's no way to call deduplicate at the other time. We need to call it at the store point because this is when everything is ready to check for duplication. But yes, deduplication can result in dirty value which needs to be persisted. So I've added a persistence mechanism and AR integration. Let me know your thoughts.

[mshibuya]

You're totally right, I'm really sorry that one of key features in the release 3.0 is completely broken.

But the #persist_uploader idea doesn't seem to be ideal, as it will incur an additional RDB access. Let me try to think about other solutions.

[krasnoukhov]

Closing in favor of #2679