jrgifford / delayed_paperclip

Process your Paperclip attachments in the background with delayed_job or Resque.
http://www.jstorimer.com/ruby/2010/01/30/delayed-paperclip.html
MIT License
404 stars 155 forks source link

"Missing" image gets deleted when the delayed conversion runs #64

Closed walterdavis closed 10 years ago

walterdavis commented 10 years ago

I had a really odd thing happen in development as I was trialing this gem. I started with the usual :default_url => "/images/:style/missing.png" set in Paperclip, but I had not yet added the :processing_image_url => "/images/original/processing.jpg" definition. When I uploaded an image, it initially showed the "missing" image, but when DJ ran the conversion process, the missing image was interpreted as being the actual file attached to the record, and so when Paperclip added the converted versions, it removed the missing image (deleted it from disk!) and replaced it with the newly-converted image. This happened a couple of times before I figured out what was going on.

ScotterC commented 10 years ago

Thanks for the issue. Can you post your full paperclip and delayed_paperclip configuration?

This sounds similar to this. Does it work properly without delayed paperclip?

walterdavis commented 10 years ago

I'll try to do that later today. I can't see how it would work incorrectly without delayed paperclip -- if the file is uploaded and the asset is saved in that same process, then the missing image wouldn't be linked at all.

Walter

On Nov 13, 2013, at 10:33 AM, Scott Carleton wrote:

Thanks for the issue. Can you post your full paperclip and delayed_paperclip configuration?

This sounds similar to this. Does it work properly without delayed paperclip?

— Reply to this email directly or view it on GitHub.

tombruijn commented 10 years ago

I'm having the same issue and it looks like it could only happen in delayed_paperclip. In my config the only thing referencing the missing files is delayed_paperclip.

This is my config:

class MediaBlock < ActiveRecord::Base

  # ...

  has_attached_file :cover,
    styles: { thumb: "50x50>", small: "480x240^", large: "480x480^" },
    convert_options: { small: "-gravity center -crop 440x240+0+0 +repage" }
  process_in_background :cover, processing_image_url: "/media-missing/:style/missing.png"

  # ...

end

I though it was a conflict of the media directory at first, but renaming it to media-missing and having the directory still clear is kind of strange. My media-missing directory contains missing images for the thumb, small, large and original styles. When it is cleared during the job the thumb, small and large directory are cleared while the original directory is removed completely.

I looked through the delayed_paperclip codebase, but couldn't find a place where this could easily happen accidentally. Maybe it's paperclip throwing them away thinking these files were previous uploads and then replacing them with new ones throwing the old ones away?

Note: I have paperclip configured with S3, as in the other issue, using the fog gem, but this happens in development (on my machine, without S3 configured) as well.

walterdavis commented 10 years ago

This was what I surmised as well.

Walter

On Dec 2, 2013, at 9:52 AM, Tom de Bruijn wrote:

Maybe it's paperclip thinking these files are uploads and then replaced with new one throwing the old ones away?

tombruijn commented 10 years ago

Sorry walter, thought you never tried it with the processing_image_url. Monkey patched it for now with this in my app:

def cover_url(style = :original)
  if cover_processing?
    ActionController::Base.helpers.asset_path("media-missing/#{style}/missing.png")
  else
    cover.url(style)
  end
end

media.cover.url(:thumb) => media.cover_url(:thumb)

So I'm not using the processing_image_url or missing images anymore.

walterdavis commented 10 years ago

This looks very useful. Thanks for the update. I ended up using the processing_image_url without the missing images in base paperclip, and that worked as well (although I lost the desired behavior of missing_url). Your way looks to be more correct for my purposes.

It really does seem that paperclip believes that the missing image is the real file owned by the model, and so as in a normal update, deletes it before attaching the delayed processed version. On the production app that I'm aiming this toward, we already use the keep deleted assets behavior, so it might still work there, but I think your technique is more expressive and thus more likely to land when I implement this feature for real.