lardawge/carrierwave_backgrounder

`process_in_background` was broken by commit on Aug 8

Closed this issue · 6 comments

Qqwy commented

This particular line: 9cd5dd5#diff-5ff69b5ab5e3c197030848e9435e88c012107aa8ae086fb780b220aa589120b5L16

is paramount to make process_in_background work.
It was removed in that commit, which means that currently the process_in_background flow of this gem is broken.

lardawge commented

There are tests running that do not support that theory. Feel free to write a failing spec showing this to be the case and I will be happy to fix it.

  • 👍1
jannikolai commented

I just wanted to create an own ticket, but luckily there is just in time a ticket and I do not feel alone any more :)
I've created a demo app to demonstrate it: https://github.com/jannikolai/carrierwave_backgrounder_example/commits/main

Thumb generation works fine, as long as carrierwave_backgrounder is not in use. After activating it and using process_in_background the thumb generation is broken. It just copies the file with a "thumb_" prefix.

jannikolai commented

I just extended the process_in_background_spec.rb with a stupid test regarding the file sizes to be different:

    it 'creates the versions' do
      version_paths = admin.avatar.versions.keys.map { |key| admin.avatar.send(key).current_path }
      version_paths.each { |path| expect(File.exist? path).to be(true) }
      file_sizes = version_paths.map { | path| File.size path }
      unique_file_sizes = file_sizes.uniq
      expect(unique_file_sizes).to eql(file_sizes)
    end

Readding the line @Qqwy identified makes the test pass:
record.send(:"process_#{column}_upload=", true)

  • ❤️1
lardawge commented

@Qqwy, Thanks for the PR.
It looks like the files are getting created. However, they are not getting resized, just copied over and renamed. Correct?

Qqwy commented

It looks like the files are getting created. However, they are not getting resized, just copied over and renamed. Correct?

Indeed, in the minified example as executed in the specs this is the case. And this seems to also be the exact situation encountered by @jannikolai.

In the Rails app I encountered the problem in, we were not even seeing files for the versions being created at all. But I have not been able to minify that exact problem. The difference in behaviour might be becaues we're scheduling jobs using Active Job->Delayed Job->Delayed Job Active Record whereas the carrierwave_backgrounder's test suite uses Sidekiq. Or maybe there is another small difference between our app and carrierwave_backgrounder's spec/support/dummy_app. Regardless, this fix resolves the underlying issue (process_#{column}_upload not being set resulting in proceed_with_versioning? being false and thus the background job also using the no-op version of the overridden methods) either way.

Thank you for merging the PR! 💚

Looking forward to the next bugfix release 😊

lardawge commented

Fix has been release in 1.0.2.

Thank you both for getting this sorted. Much appriciated.

  • ❤️1