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

"undefined method `copy' for nil" on save #2742

Closed ssugiyama closed 1 month ago

ssugiyama commented 6 months ago

Due to 4c65b393cd85b66bc256d04363cf3e3a97c8fd64, the following pseudo code begins to raise an error

class  ImageUploader < CarrierWave::Uploader::Base
  storage :fog 
  cache_storage :fog # maybe :file is alright
end

class Foo < ApplicationRecord
  mount_uploader :image, ImageUploader
end

...
class FooController < ApplicationController
... 
  def confirm
    @foo = Foo.new(image: params[:image], image_cache: params[:image_cache])
  end

  def complete
    @foo = Foo.new(image: params[:image], image_cache: params[:image_cache])
    @foo.save
  end
...
end

when complete method is called from confirm form which has f.hidden_field :image_cache and f.hidden_field :image , foo.save raises the following error.

undefined method `copy' for nil
        "/usr/local/bundle/bundler/gems/carrierwave-287e9a1924cb/lib/carrierwave/storage/fog.rb:444:in `copy_to'",
        "/usr/local/bundle/bundler/gems/carrierwave-287e9a1924cb/lib/carrierwave/storage/fog.rb:330:in `store'",
        "/usr/local/bundle/bundler/gems/carrierwave-287e9a1924cb/lib/carrierwave/storage/fog.rb:86:in `store!'",
        "/usr/local/bundle/bundler/gems/carrierwave-287e9a1924cb/lib/carrierwave/uploader/store.rb:97:in `block in store!'",
        "/usr/local/bundle/bundler/gems/carrierwave-287e9a1924cb/lib/carrierwave/uploader/callbacks.rb:15:in `with_callbacks'"

Due to 4c65b39, params[:image] turns to be the same as params[:image_cache]. Maybe this the reason of the behavior.

When the error was raised, the cache store seemed to have been already deleted.

mshibuya commented 2 months ago

@ssugiyama Thank you for reporting. I agree that this is better to be improved, but I want to understand one thing. The expected usage is to have either f.hidden_field :image_cache or f.hidden_field :image, not both. What led you to to do so? Was there any instruction or something?

ssugiyama commented 1 month ago

@mshibuya sorry for delay in my response

I make the entire form invisible, showing the image to users outside the form. That's why both image_cache and image are hidden fields. Maybe there's no need to worry about that as it's not essential.​​​​​​​​​​​​​​​​

mshibuya commented 1 month ago

Ah I meant, having both of :image_cache and :image is unexpected use case. If you have followed the old way you should have only :image_cache. And with new way I'd recommend only having :image (mainly due to https://github.com/carrierwaveuploader/carrierwave/pull/2401).

So the question is, what's the reason you ended up having :image_cache and :image together?

ssugiyama commented 1 month ago

I see! I just followed the old way. I will fix it to follow the current way.

ssugiyama commented 1 month ago

Sorry for misreading you comment.

This is a legacy code of our project. I have misunderstood this is the correct way.

ssugiyama commented 1 month ago

Maybe, we referred to this description. Do you mean the description is obsolete? @mshibuya

mshibuya commented 1 month ago

What's there is file_field, not hidden_field. If what you referred to was that one you should have misunderstood something. Anyway closing this, as this seems to be a very exceptional case. Thank you for reporting.