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

recreate_versions! is changing the original file content #2480

Closed
feliperaul opened this issue May 17, 2020 · 3 comments
Closed

recreate_versions! is changing the original file content #2480

feliperaul opened this issue May 17, 2020 · 3 comments

Comments

@feliperaul
Copy link

I'm on the latest version 2.1.0.

When I invoke recreate_versions!, my original version is being re-processed, it's file size is increasing (from 385kb to 451kb) and, of course, it's Sha-256 is changing as well.

Coincidently #2479 is also the most recent issue opened, but it only tackles a modification in the original file's modified timestamp, while at this issue I'm pointing that the original file is having it's content changed as well.

Carrierwave has a test for this (https://github.com/carrierwaveuploader/carrierwave/pull/2020/files), but somehow it's not preventing this bug.

I'm pasting my entire uploader below, because maybe it helps with debugging:

class PictureUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick
  include OurApp::ImageProcessing

  storage :file

  # Override the directory where uploaded files will be stored.
  # This is a sensible default for uploaders that are meant to be mounted:
  def store_dir
    "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{id_partition}/#{model.id}"
  end

  def id_partition
    case id = model.id
    when Integer
      ("%012d" % id).scan(/\d{3}/).join("/")
    else
      nil
    end
  end

  def seo_filename
    title_words = model.property.title_br[0..30].split(" ")
    title_words.pop(1) if title_words.size > 1 
    "#{title_words.join(" ").parameterize}-#{model.property.try(:location).try(:city).try(:parameterize) || ""}-#{model.property.try(:location).try(:beach).try(:parameterize) || ""}.jpg"
  end

  def filename
    if original_filename
      if model && model.read_attribute(mounted_as).present?
        model.read_attribute(mounted_as)
      else
        seo_filename
      end
    end
  end

  def cache_dir
     "#{Rails.root}/tmp/uploads"
  end

  process :fix_exif_rotation
  process :set_original_dimensions

  process convert: 'jpg'
  
  process resize_to_fill: [1280, 720], if: :is_landscape?
  process resize_to_fit: [1280, 720], if: :is_portrait?

  version :watermark do
    process :watermark_image
  end

  version :mobile do
    process resize_to_fill: [720, 405], if: :is_landscape?
    process resize_to_fit: [720, 405], if: :is_portrait?
  end

  version :thumb do
    process resize_to_fill: [470, 265], if: :is_landscape?
    process resize_to_fit: [470, 265], if: :is_portrait?
  end


  def extension_whitelist
    %w(jpg jpeg png heic jfif)
  end

  protected

  def watermark_image

    second_image = MiniMagick::Image.open("#{Rails.root}/public/logo.png")

    manipulate! do |img|
      img.composite(second_image) do |c|
        c.compose "Over"    # OverCompositeOp
        c.gravity "Center" # copy second_image onto first_image from (20, 20)
        c.dissolve 35
      end
    end

  end

end
@victorpolko
Copy link

victorpolko commented Feb 20, 2022

@feliperaul
I was able to reproduce your issue, and I had a similar problem.
I couldn't figure out a way to fix this after reading into the carrierwave's source files, that's too abstract for my current level.

But I was able to get around the issue using common sense: carrierwave's main purpose is to process and store the file you uploaded, and the versions is only a feature, so all the process operations outside version macro are first applied to the original file (which could be implied from here).

My fix was to move all process macros inside all versions. Yes, it's not that DRY, but after that my cropping/rotating didn't change the original file contents (I checked the image's size).

Regarding your example, it would be something like this:

...

def cache_dir
  "#{Rails.root}/tmp/uploads"
end

version :watermark do
  process :fix_exif_rotation
  process :set_original_dimensions

  process convert: 'jpg'

  process resize_to_fill: [1280, 720], if: :is_landscape?
  process resize_to_fit: [1280, 720], if: :is_portrait?
  
  process :watermark_image
end

version :mobile do
  process :fix_exif_rotation
  process :set_original_dimensions

  process convert: 'jpg'

  process resize_to_fill: [1280, 720], if: :is_landscape?
  process resize_to_fit: [1280, 720], if: :is_portrait?
  
  process resize_to_fill: [720, 405], if: :is_landscape?
  process resize_to_fit: [720, 405], if: :is_portrait?
end

version :thumb do
  process :fix_exif_rotation
  process :set_original_dimensions

  process convert: 'jpg'

  process resize_to_fill: [1280, 720], if: :is_landscape?
  process resize_to_fit: [1280, 720], if: :is_portrait?
  
  process resize_to_fill: [470, 265], if: :is_landscape?
  process resize_to_fit: [470, 265], if: :is_portrait?
end

# two empty lines here? 
def extension_whitelist
...

To DRY things up you could use a class method defined before the calls:

def self.common_processing
  process :fix_exif_rotation
  process :set_original_dimensions

  process convert: 'jpg'

  process resize_to_fill: [1280, 720], if: :is_landscape?
  process resize_to_fit: [1280, 720], if: :is_portrait?
end

...

version :watermark do
  common_processing

  process :watermark_image
end

# etc.

Please have a go with this and let us know if it worked for you.
My guess it's only applicable when you want to operate with your versions only, leaving the original file really untouched.


In case you still want to process the original file at the upload, an approach I could think of regarding all said before about the process calls would be to use Conditional Processing feature, only processing outside the versions in case it's the first upload: you could use some flag or maybe you already store some dimensions in the DB using your OurApp::ImageProcessing#set_original_dimensions, and you can come up with some appropriate just_uploaded? method.

Something like this:

...

def cache_dir
  "#{Rails.root}/tmp/uploads"
end

process :fix_exif_rotation, if: :just_uploaded?
process :set_original_dimensions, if: :just_uploaded?

process convert: 'jpg', if: :just_uploaded?

process resize_to_fill: [1280, 720], if: :just_uploaded_and_landscape?
process resize_to_fit: [1280, 720], if: :just_uploaded_and_portrait?

version :watermark do
  process :watermark_image
end

...

@juanjzv
Copy link

juanjzv commented Aug 3, 2023

Hello, @mshibuya, I was wondering if there's a workaround to have the corrected behaviour back. I'm trying to upgrade from a pre 3.0 version of carrierwave and I depend on having a way to re-process the original file.

I have an uploader that can rotate existing images by processing it, here is a simplified version of it:

class ImageUploader < CarrierWave::Uploader::Base
  ...
  process :process_rotate
  ...

  version :thumb do
    # specific version processing here
  end

  def rotate(degrees)
    @degrees = degrees
  end

  private

  def process_rotate
    unless @degrees.to_i.zero?
      manipulate! do |img|
        img.rotate(@degrees.to_i)
      end
    end
  end
end

And I use it like this:

record.image.rotate(90)
record.recreate_versions!

I can't just add the processing method to each version because images need to be able to rotate based on the current rotation state and because the versions are created from the original file (not rotated) it won't work.

Is there any way that I can reprocess the original file and its versions?

@herbertia
Copy link

@juanjzv did you find a solution? We are experiencing the same issue and cannot find a workaround.
@mshibuya do you know if there is an option to say I want to update the original file or not?

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

5 participants