Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store different versions with the exact same filename but in different directories #1861

Closed
Silex opened this issue Feb 2, 2016 · 7 comments
Labels

Comments

@Silex
Copy link

Silex commented Feb 2, 2016

Hello

I'm using carrierwave from master.

It looks like we cannot have the exact same filename for different versions.

If I upload a file, say "hello.jpg" and the associated model has id 123, I want it to be saved that way on the disk (the model.id is irrelevant here, it can be anything as long as it results in the exact same filename):

/data/original/123.jpg
/data/version1/123.jpg
/data/version2/123.jpg

Here is what I tried:

class MyUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick

  storage :file

  def store_dir
    "/data/#{version_name || 'original'}"
  end

  def filename
    "#{model.id}.jpeg" if original_filename.present?
  end

  version :version1 do
    def full_filename(for_file)
      for_file
    end
  end

  version :version2 do
    def full_filename(for_file)
      for_file
    end
  end
end

It fails when creating the versions because I guess it deletes the same cache file twice (it works if I let the filename be different, with the version1_ or version2_ being prefixed).

Also, someone at http://stackoverflow.com/questions/31729932/how-to-customize-version-filename-in-carrierwave seems to have the same problem.

@Silex
Copy link
Author

Silex commented Feb 2, 2016

I just had an idea, maybe I could have store_dir return "/data" and full_filename return "version1/123.jpg". The idea is that the changing directory would be part of the filename instead of the store dir.

@thomasfedb
Copy link
Contributor

@Silex what's your usecase?

@Silex
Copy link
Author

Silex commented Feb 3, 2016

@thomasfedb: I store images, I need to store a "medium" version that is 800x600 and a "small" version that is 200x150. I want them to have this layout on the disk:

"/images/#{version_name || 'original'}/#{model.created_at.to_i}.jpeg"

That is, I want to have the following resulting files:

/images/original/1454397604.jpeg
/images/original/1454411423.jpeg
/images/original/1454412602.jpeg
/images/original/1454414406.jpeg
/images/original/1454415600.jpeg
/images/medium/1454397604.jpeg
/images/medium/1454411423.jpeg
/images/medium/1454412602.jpeg
/images/medium/1454414406.jpeg
/images/medium/1454415600.jpeg
/images/small/1454397604.jpeg
/images/small/1454411423.jpeg
/images/small/1454412602.jpeg
/images/small/1454414406.jpeg
/images/small/1454415600.jpeg

It's because I have another tool that requires a directory containing only full-res pictures, and I find it silly to have the medium/small versions with a different filename if they are in different directories.

@Silex
Copy link
Author

Silex commented Feb 3, 2016

I just found the hack needed for it to work:

class AlbumImageUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick

  storage :file

  def store_dir
    "/images/%s" % (version_name || 'original')
  end

  def filename
    "%s.jpeg" % model.created_at.to_i if original_filename.present?
  end

  def full_filename(for_file)
    if model.new_record?
      super(for_file)
    else
      for_file
    end
  end

  version :medium do
    process resize_to_fill: [800, 600]
  end

  version :small, from_version: :medium do
    process resize_to_fill: [200, 150]
  end
end

The idea is that carrierwave needs a different filename for the temporary thumbnails, but only when the record is not saved yet.

Basically, once the file is saved full_filename does not need to be different if store_dir is different... however before the file is saved, full_filename needs to be different. I guess that's because of some assumptions in the cache system (for example the cache system doesn't use store_dir, only /tmp, and in this case if the filename is the same one version overwrites the other).

A simple fix that'd remove the hack above could be to also use store_dir for the temporary directory... would you be interested in a PR? If not, should I create a new wiki entry with this?

@thomasfedb
Copy link
Contributor

We're always interested in new contributions @Silex. Glad you found a solution!

@Silex
Copy link
Author

Silex commented Feb 4, 2016

@thomasfedb: before I do, can you confirm that in full_filename, checking with cached? is the appropriate way to know wether you want a filename for the cache or for the store?

EDIT: hum, unfortunately it seems that cached? is not sufficient

Current's what's working is if for_file == original_filename.

@thomasfedb
Copy link
Contributor

Hi @Silex, you're going to have to investigate and read through the codebase.

I'm not a long-standing CarrierWave developer, just somebody who volunteered to come on to look after the issues and PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants