Fix dup mounter cache behavior

carrierwaveuploader / carrierwave

Classier solution for file uploads for Rails, Sinatra and other Ruby web frameworks

https://github.com/carrierwaveuploader/carrierwave

8.78k stars 1.66k forks source link

Fix dup mounter cache behavior #2706

Closed rajyan closed 1 year ago

rajyan commented 1 year ago

dup should not prevent uploading

fixes https://github.com/carrierwaveuploader/carrierwave/issues/2700

rajyan commented 1 year ago

I'm very confused with the results, but these test fails inconsistently...

rajyan commented 1 year ago

I'm not sure yet with

If this test is expressing the problem in the issue
Why the tests are failing randomly

~~Also, testing locally with counter_culture, I still could not upload an image like explained in https://github.com/carrierwaveuploader/carrierwave/issues/2700.~~

rajyan commented 1 year ago

I'm very confused with the results, but these test fails inconsistently...

This was simply cause by test fixture setup... The first commit passes, because 'test.jpg' is uploaded by another test and the uploaded files isn't cleaned.

I would fix up them.

rajyan commented 1 year ago

All problems are resolved. I believe this pull request now fixes https://github.com/carrierwaveuploader/carrierwave/issues/2700

rajyan commented 1 year ago

@mshibuya

The last commit in https://github.com/carrierwaveuploader/carrierwave/pull/2690 broke the @_mounters cache, because the dup is called after nil is set. Sorry that I couldn't catch it.

mshibuya commented 1 year ago

Looks great except for the tiny issue I commented! This is my mistake as I didn't understand the invocation order of CarrierWave::Mount::Extension#initialize_dup and CarrierWave::ActiveRecord 's #initialize_dup correctly... 😓

mshibuya commented 1 year ago

Thank you so much!