FIX: refactor ImageSizer.resize

FIX: refactor ImageSizer.resize

reverts 140d9c2

From 0eacd45ab15cbd20ed9f444fd447886a7fc6dccb Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Thu, 29 Nov 2018 15:28:45 -0500
Subject: [PATCH] FIX: refactor ImageSizer.resize

reverts 140d9c2

diff --git a/app/models/upload.rb b/app/models/upload.rb
index 802be43..bf3d415 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -128,9 +128,7 @@ class Upload < ActiveRecord::Base
       end
 
     self.width, self.height = size = FastImage.new(path).size
-    if !size.blank?
-      self.thumbnail_width, self.thumbnail_height = ImageSizer.resize(*size)
-    end
+    self.thumbnail_width, self.thumbnail_height = ImageSizer.resize(*size)
     nil
   end
 
diff --git a/lib/image_sizer.rb b/lib/image_sizer.rb
index ee368ce..6e16464 100644
--- a/lib/image_sizer.rb
+++ b/lib/image_sizer.rb
@@ -1,7 +1,7 @@
 module ImageSizer
 
   # Resize an image to the aspect ratio we want
-  def self.resize(width, height, opts = {})
+  def self.resize(width = nil, height = nil, opts = {})
     return if width.blank? || height.blank?
 
     max_width = (opts[:max_width] || SiteSetting.max_image_width).to_f
diff --git a/spec/components/image_sizer_spec.rb b/spec/components/image_sizer_spec.rb
index b013267..f3c0960 100644
--- a/spec/components/image_sizer_spec.rb
+++ b/spec/components/image_sizer_spec.rb
@@ -24,6 +24,10 @@ describe ImageSizer do
     expect(ImageSizer.resize('100', '101')).to eq([100, 101])
   end
 
+  it 'returns nil if no width or height are provided' do
+    expect(ImageSizer.resize).to eq(nil)
+  end
+
   describe 'when larger than the maximum width' do
 
     before do

GitHub

1 Like

I am marking this for a followup, this is somewhat complex to deal with but…

If we ask for a thumbnail and the upload is missing we should raise an exception that says something along the lines of “sorry, I can not get resize this upload cause it does not exist” upload id, etc

The disadvantage of blindly doing this, this deep in the code is that it will go unnoticed

Then the site serializer that is asking for thumbnails and thumbnail sizes can handle the exception you raise there and log a warning to the logs:

“Sorry I ignored the image for the category bob cause it is broken, upload id is 183”

This means that we can cleanly handle this situation without breaking sites and have fantastic instrumentation.

2 Likes