refactored a bit & tested thumbnails creation

app/models/optimized_image.rb

@@ -1,15 +1,39 @@
+require "digest/sha1"
+
 class OptimizedImage < ActiveRecord::Base
   belongs_to :upload
 
-  def self.create_for(upload_id, path)
+  def self.create_for(upload, width=nil, height=nil)
-    image_info = FastImage.new(path)
+    @image_sorcery_loaded ||= require "image_sorcery"
-    OptimizedImage.new({
+
-      upload_id: upload_id,
+    original_path = "#{Rails.root}/public#{upload.url}"
-      sha: Digest::SHA1.file(path).hexdigest,
+    # create a temp file with the same extension as the original
-      ext: File.extname(path),
+    temp_file = Tempfile.new(["discourse", File.extname(original_path)])
+    temp_path = temp_file.path
+
+    # do the resize when there is both dimensions
+    if width && height && ImageSorcery.new(original_path).convert(temp_path, resize: "#{width}x#{height}")
+      image_info = FastImage.new(temp_path)
+      thumbnail = OptimizedImage.new({
+        upload_id: upload.id,
+        sha: Digest::SHA1.file(temp_path).hexdigest,
+        ext: File.extname(temp_path),
         width: image_info.size[0],
         height: image_info.size[1]
       })
+      # make sure the directory exists
+      FileUtils.mkdir_p Pathname.new(thumbnail.path).dirname
+      # move the temp file to the right location
+      File.open(thumbnail.path, "wb") do |f|
+        f.write temp_file.read
+      end
+    end
+
+    # close && remove temp file
+    temp_file.close
+    temp_file.unlink
+
+    thumbnail
   end
 
   def url

app/models/upload.rb

@@ -32,22 +32,8 @@ class Upload < ActiveRecord::Base
     return unless SiteSetting.create_thumbnails?
     return unless width > SiteSetting.auto_link_images_wider_than
     return if has_thumbnail?
-    @image_sorcery_loaded ||= require "image_sorcery"
+    thumbnail = OptimizedImage.create_for(self, width, height)
-    original_path = "#{Rails.root}/public#{url}"
+    optimized_images << thumbnail if thumbnail
-    temp_file = Tempfile.new(["discourse", File.extname(original_path)])
-    if ImageSorcery.new(original_path).convert(temp_file.path, resize: "#{width}x#{height}")
-      thumbnail = OptimizedImage.create_for(id, temp_file.path)
-      optimized_images << thumbnail
-      # make sure the directory exists
-      FileUtils.mkdir_p Pathname.new(thumbnail.path).dirname
-      # move the temp file to the right location
-      File.open(thumbnail.path, "wb") do |f|
-        f.write temp_file.read
-      end
-    end
-    # close && remove temp file if it exists
-    temp_file.close
-    temp_file.unlink
   end
 
   def self.create_for(user_id, file)

spec/components/cooked_post_processor_spec.rb

@@ -63,7 +63,6 @@ describe CookedPostProcessor do
 
       before do
         FastImage.stubs(:size).returns([123, 456])
-        ImageSorcery.any_instance.stubs(:convert).returns(false)
         creator = PostCreator.new(user, raw: Fabricate.build(:post_with_images).raw, topic_id: topic.id)
         @post = creator.create
       end

spec/fabricators/upload_fabricator.rb

@@ -0,0 +1,8 @@
+Fabricator(:upload) do
+  user
+  original_filename "uploaded.jpg"
+  filesize 1234
+  width 100
+  height 200
+  url  "/uploads/default/123456789.jpg"
+end

spec/models/optimized_image_spec.rb

@@ -4,4 +4,28 @@ describe OptimizedImage do
 
   it { should belong_to :upload }
 
+  let(:upload) { build(:upload) }
+  let(:oi) { OptimizedImage.create_for(upload, 100, 100) }
+
+  describe ".create_for" do
+
+    before(:each) do
+      ImageSorcery.any_instance.stubs(:convert).returns(true)
+      FastImage.any_instance.stubs(:size).returns([244, 66])
+      # make sure we don't hit the filesystem
+      FileUtils.stubs(:mkdir_p)
+      File.stubs(:open)
+    end
+
+    it "works" do
+      Tempfile.any_instance.expects(:close).once
+      Tempfile.any_instance.expects(:unlink).once
+      oi.sha.should == "da39a3ee5e6b4b0d3255bfef95601890afd80709"
+      oi.ext.should == ".jpg"
+      oi.width.should == 244
+      oi.height.should == 66
+    end
+
+  end
+
 end

spec/models/post_alert_observer_spec.rb

@@ -5,7 +5,6 @@ describe PostAlertObserver do
 
   before do
     ActiveRecord::Base.observers.enable :post_alert_observer
-    ImageSorcery.any_instance.stubs(:convert).returns(false)
   end
 
   let!(:evil_trout) { Fabricate(:evil_trout) }