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

Incorrect url generated for versions of different format #378

Closed
jcarlson opened this issue Jun 25, 2011 · 4 comments
Closed

Incorrect url generated for versions of different format #378

jcarlson opened this issue Jun 25, 2011 · 4 comments

Comments

@jcarlson
Copy link
Contributor

I have an uploader that converts a version to a different format and applies some additional transformations. Initially, everything works, but after loading the model instance from the database, the uploader isn't generating the correct path or url for the version.

Here is my uploader class:


class PhotoUploader < CarrierWave::Uploader::Base

  include CarrierWave::RMagick

  storage :file

  # Override the directory where uploaded files will be stored.
  def store_dir
    "uploads/photos/#{model.id}"
  end

  # Large format version, suitable for the standard photo viewer page
  version :large do

    def store_dir
      "uploads/photos/#{model.id}/versions"
    end

    # Resize down to viewer's width and max height
    process :resize_to_fit => [640, 1280]

    # Reduce quality of version for web storage
    process :quality => [50]

    version :reflected do

      process :reflection

      def filename
        super.chomp(File.extname(super)) + '.png'
      end

    end

  end

  private

  def quality(q=50)
    manipulate! {|img| img.write(current_path) {self.quality=q}}
  end

  def reflection
    manipulate!(:format => "png") do |img|
      img = img.wet_floor 0.4, 0.7
      img.write(current_path) {self.quality=50}
    end
  end

end

And here's what happens when I reload the instance (using rails console):

>> p = Photo.create :image => File.open('/Users/jcarlson/Desktop/DSC_0171.jpg')
=> #<Photo id: 20, release_date: "2011-06-21", image: "dsc_0171.jpg", created_at: "2011-06-25 20:17:58", updated_at: "2011-06-25 20:17:58">
>> p.image.large.reflected.url
=> "/uploads/photos/20/versions/large_reflected_dsc_0171.png"
>> p = Photo.find(20)
=> #<Photo id: 20, release_date: "2011-06-21", image: "dsc_0171.jpg", created_at: "2011-06-25 20:17:58", updated_at: "2011-06-25 20:17:58">
>> p.image.large.reflected.url
=> "/uploads/photos/20/versions/large_reflected_dsc_0171.jpg"
>> 

I haven't quite figured out what's going wrong, but clearly the custom filename method on the "reflected" version is not being accounted for. If I check the versions filename, I get something like this:

>> p.image.large.reflected.filename
TypeError: can't convert nil into String
    from /Users/jcarlson/Projects/living5to9.com/app/uploaders/photo_uploader.rb:32:in `extname'
    from /Users/jcarlson/Projects/living5to9.com/app/uploaders/photo_uploader.rb:32:in `filename'
    from (irb):70
>> 

Oops! What's nil here? Calls to super?

@jcarlson
Copy link
Contributor Author

Note that the uploader expects the "reflected" version to have an extension of ".jpg" instead of the correct ".png" extension?

@trevorturk
Copy link
Contributor

If you're able to figure it out, a failing test case would be great to see.

@jcarlson
Copy link
Contributor Author

So on further investigating, it looks like this isn't so much a defect as it is lack of clarity. What's happening is when you store an image, the "filename" method is called (and in the case of my class above) given a chance to mutate the filename as it will be written to the store and subsequently stored in the database. So it works as expected when creating new images. However, on retrieving an image, the "identifier" stored in the database is used to construct even the version's path. The "filename" method is not called during retrieval to allow the same mutation to occur, thus the problem.

Fortunately, it looks like the whole problem can be avoided by overwriting "full_filename" instead of "filename" inside my versions. This method is invoked on both store! and retrieve_from_store! and thus allows the identifier stored in the database to be mutated properly. Here is a revised copy of my Uploader that works correctly:


class PhotoUploader < CarrierWave::Uploader::Base

  include CarrierWave::RMagick

  storage :file

  # ... clutter omitted for clarity...

  version :large do

    process :resize_to_fit => [640,640]

    version :reflection do
      process :reflect

      def full_filename(for_file)
        super(for_file).chomp(File.extname(super(for_file))) + '.png'
      end

    end

  end

  def reflect
    manipulate! do |img|
      #img = img.wet_floor 0.4, 0.7
      img.flip!
    end
  end

end

While testing, I did discover that for some reason, versions are handled differently when the uploader is mounted versus not mounted, but I will open a new issue to address that.

@trevorturk
Copy link
Contributor

Awesome - glad you figured this out. If you think an addition to the wiki or readme is called for, please do pitch in. Thanks!

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

No branches or pull requests

2 participants