Fix deduplicated filename not being persisted

[mshibuya]

I'm sorry that the deduplication feature introduced in the 3.0 release is completely broken, as reported by #2677. This PR is another attempt to fix it.

The approach is to move the deduplication logic from #store! to #write_identifier, which is invoked on before_save thus allowing the modified filename(identifier) to be persisted. To work around the limitation that the deduplication timing can't be made earlier because #store_dir relies on the model's id, the scope of deduplication was narrowed down only to filename, instead of whole #store_path. This sometimes can lead to unnecessary deduplication in a case like a user decided to use a custom #store_dir to handle deduplication by their own, but even if that happens it does not harm data consistency, so I think it's acceptable.

@krasnoukhov I want your opinion on this, what do you think?

[krasnoukhov]

Looks good to me. I ran our app CI with this commit and there are no failures (previously there was a bunch because of missing file error)

[Aiderlei]

Thanks for fixing this! When can I except this to be released?

[mshibuya]

Just released 3.0.2, which includes this fix.

[Aiderlei]

@mshibuya Thanks sir! That was fast 🚀

[mrudult]

@mshibuya Shouldn't this deduplicated filename feature be optional? We use S3 as our storage and S3 automatically replaces the file if the filename is already present in the directory. So #1855 was never really an issue on S3.

[ps231402]

I still have the issue with 3.0.2.

[mshibuya]

@mrudult I'm afraid that you may be missing something.

automatically replaces the file if the filename is already present in the directory

This is the problem itself. As described in the issue, if a user uploads two different files with the same filename results in one overwrites the other, ending up with only 1 file persisted.
It is also problematic for a singular uploader (#mount_uploader, without 's'). If a user updates a record but the transaction rolls back, the existing file still be overwritten because the file is stored on #after_save (see #2546). That's why we need the deduplication feature to be in place.

If you don't like the default deduplicated filenames, you can choose to override #deduplicated_filename with your own way like

class YourUploader < CarrierWave::Uploader::Base
  ...

  def deduplicated_filename
    ...
  end
end

[mshibuya]

@ps231402 Just saying so isn't very helpful. Please show me what you did and what you got, like a Rails console output.