uploads: factor out image dimension and filetype detection code.

* Add MediaFile abstraction. A MediaFile represents an image or video file.
* Move filetype detection and dimension parsing code from uploads to MediaFile.

app/logical/media_file.rb

@@ -0,0 +1,77 @@
+class MediaFile
+  extend Memoist
+  attr_accessor :file
+
+  # delegate all File methods to `file`.
+  delegate *(File.instance_methods - MediaFile.instance_methods), to: :file
+
+  def self.open(file)
+    file = Kernel.open(file, "r", binmode: true) unless file.respond_to?(:read)
+
+    case file_ext(file)
+    when :jpg, :gif, :png
+      MediaFile::Image.new(file)
+    when :swf
+      MediaFile::Flash.new(file)
+    when :webm, :mp4
+      MediaFile::Video.new(file)
+    when :zip
+      MediaFile::Ugoira.new(file)
+    else
+      MediaFile.new(file)
+    end
+  end
+
+  def self.file_ext(file)
+    header = file.pread(16, 0)
+
+    case header
+    when /\A\xff\xd8/n
+      :jpg
+    when /\AGIF87a/, /\AGIF89a/
+      :gif
+    when /\A\x89PNG\r\n\x1a\n/n
+      :png
+    when /\ACWS/, /\AFWS/, /\AZWS/
+      :swf
+    when /\x1a\x45\xdf\xa3/n
+      :webm
+    when /\A....ftyp(?:isom|3gp5|mp42|MSNV|avc1)/
+      :mp4
+    when /\APK\x03\x04/
+      :zip
+    else
+      :bin
+    end
+  end
+
+  def initialize(file)
+    @file = file
+  end
+
+  def dimensions
+    [0, 0]
+  end
+
+  def width
+    dimensions.first
+  end
+
+  def height
+    dimensions.second
+  end
+
+  def md5
+    Digest::MD5.file(file.path).hexdigest
+  end
+
+  def file_ext
+    MediaFile.file_ext(file)
+  end
+
+  def file_size
+    file.size
+  end
+
+  memoize :dimensions, :file_ext, :file_size, :md5
+end

app/logical/media_file/flash.rb

@@ -0,0 +1,6 @@
+class MediaFile::Flash < MediaFile::Image
+  def dimensions
+    image_size = ImageSpec.new(file.path)
+    [image_size.width, image_size.height]
+  end
+end

app/logical/media_file/image.rb

@@ -0,0 +1,6 @@
+class MediaFile::Image < MediaFile
+  def dimensions
+    image_size = ImageSpec.new(file.path)
+    [image_size.width, image_size.height]
+  end
+end

app/logical/media_file/ugoira.rb

@@ -0,0 +1,13 @@
+class MediaFile::Ugoira < MediaFile
+  def dimensions
+    tempfile = Tempfile.new
+    folder = Zip::File.new(file.path)
+    folder.first.extract(tempfile.path) { true }
+
+    image_file = MediaFile.open(tempfile)
+    image_file.dimensions
+  ensure
+    image_file.close
+    tempfile.close!
+  end
+end

app/logical/media_file/video.rb

@@ -0,0 +1,9 @@
+class MediaFile::Video < MediaFile
+  def dimensions
+    [video.width, video.height]
+  end
+
+  def video
+    @video ||= FFMPEG::Movie.new(file.path)
+  end
+end

app/logical/upload_service.rb

@@ -48,7 +48,7 @@ class UploadService
 
       @upload.update(status: "processing")
 
-      @upload.file = Utils.get_file_for_upload(@upload, file: @upload.file)
+      @upload.file = Utils.get_file_for_upload(@upload, file: @upload.file&.tempfile)
       Utils.process_file(upload, @upload.file)
 
       @upload.save!

app/logical/upload_service/preprocessor.rb

@@ -90,7 +90,7 @@ class UploadService
       begin
         upload.update(status: "preprocessing")
 
-        file = Utils.get_file_for_upload(upload, file: params[:file])
+        file = Utils.get_file_for_upload(upload, file: params[:file]&.tempfile)
         Utils.process_file(upload, file, original_post_id: original_post_id)
 
         upload.rating = params[:rating]
@@ -116,7 +116,7 @@ class UploadService
       # if a file was uploaded after the preprocessing occurred,
       # then process the file and overwrite whatever the preprocessor
       # did
-      Utils.process_file(pred, pred.file) if pred.file.present?
+      Utils.process_file(pred, pred.file.tempfile) if pred.file.present?
 
       pred.status = "completed"
       pred.save

app/logical/upload_service/utils.rb

@@ -2,27 +2,6 @@ class UploadService
   module Utils
     module_function
 
-    def file_header_to_file_ext(file)
-      case File.read(file.path, 16)
-      when /^\xff\xd8/n
-        "jpg"
-      when /^GIF87a/, /^GIF89a/
-        "gif"
-      when /^\x89PNG\r\n\x1a\n/n
-        "png"
-      when /^CWS/, /^FWS/, /^ZWS/
-        "swf"
-      when /^\x1a\x45\xdf\xa3/n
-        "webm"
-      when /^....ftyp(?:isom|3gp5|mp42|MSNV|avc1)/
-        "mp4"
-      when /^PK\x03\x04/
-        "zip"
-      else
-        "bin"
-      end
-    end
-
     def corrupt?(filename)
       image = Vips::Image.new_from_file(filename, fail: true)
       image.stats
@@ -31,36 +10,6 @@ class UploadService
       true
     end
 
-    def calculate_ugoira_dimensions(source_path)
-      folder = Zip::File.new(source_path)
-      Tempfile.open("ugoira-dim-") do |tempfile|
-        folder.first.extract(tempfile.path) { true }
-        image_size = ImageSpec.new(tempfile.path)
-        return [image_size.width, image_size.height]
-      end
-    end
-
-    def calculate_dimensions(upload, file)
-      if upload.is_video?
-        video = FFMPEG::Movie.new(file.path)
-        yield(video.width, video.height)
-
-      elsif upload.is_ugoira?
-        w, h = calculate_ugoira_dimensions(file.path)
-        yield(w, h)
-
-      elsif upload.is_image? || upload.is_flash?
-        image_size = ImageSpec.new(file.path)
-        yield(image_size.width, image_size.height)
-
-      elsif upload.file_ext == "bin"
-        yield(0, 0)
-
-      else
-        raise ArgumentError, "unhandled file type (#{upload.file_ext})" # should not happen
-      end
-    end
-
     def distribute_files(file, record, type, original_post_id: nil)
       # need to do this for hybrid storage manager
       post = Post.new
@@ -120,15 +69,15 @@ class UploadService
     end
 
     def process_file(upload, file, original_post_id: nil)
-      upload.file = file
+      binding.pry if !file.is_a? Tempfile
-      upload.file_ext = Utils.file_header_to_file_ext(file)
+      media_file = MediaFile.open(file)
-      upload.file_size = file.size
-      upload.md5 = Digest::MD5.file(file.path).hexdigest
 
-      Utils.calculate_dimensions(upload, file) do |width, height|
+      upload.file = file
-        upload.image_width = width
+      upload.file_ext = media_file.file_ext.to_s
-        upload.image_height = height
+      upload.file_size = media_file.file_size
-      end
+      upload.md5 = media_file.md5
+      upload.image_width = media_file.width
+      upload.image_height = media_file.height
 
       upload.validate!(:file)
       upload.tag_string = "#{upload.tag_string} #{Utils.automatic_tags(upload, file)}"

test/unit/media_file_test.rb

@@ -0,0 +1,85 @@
+require 'test_helper'
+
+class MediaFileTest < ActiveSupport::TestCase
+  context "#dimensions" do
+    should "determine the correct dimensions for a jpeg file" do
+      assert_equal([500, 335], MediaFile.open("test/files/test.jpg").dimensions)
+      assert_equal([668, 996], MediaFile.open("test/files/test-blank.jpg").dimensions)
+      assert_equal([529, 600], MediaFile.open("test/files/test-exif-small.jpg").dimensions)
+      assert_equal([1356, 911], MediaFile.open("test/files/test-large.jpg").dimensions)
+    end
+
+    should "determine the correct dimensions for a png file" do
+      assert_equal([768, 1024], MediaFile.open("test/files/test.png").dimensions)
+      assert_equal([150, 150], MediaFile.open("test/files/apng/normal_apng.png").dimensions)
+      assert_equal([85, 62], MediaFile.open("test/files/alpha.png").dimensions)
+    end
+
+    should "determine the correct dimensions for a gif file" do
+      assert_equal([400, 400], MediaFile.open("test/files/test.gif").dimensions)
+      assert_equal([86, 52], MediaFile.open("test/files/test-animated-86x52.gif").dimensions)
+      assert_equal([32, 32], MediaFile.open("test/files/test-static-32x32.gif").dimensions)
+    end
+
+    should "determine the correct dimensions for a webm file" do
+      assert_equal([512, 512], MediaFile.open("test/files/test-512x512.webm").dimensions)
+    end
+
+    should "determine the correct dimensions for a mp4 file" do
+      assert_equal([300, 300], MediaFile.open("test/files/test-300x300.mp4").dimensions)
+    end
+
+    should "determine the correct dimensions for a ugoira file" do
+      assert_equal([60, 60], MediaFile.open("test/files/valid_ugoira.zip").dimensions)
+    end
+
+    should "determine the correct dimensions for a flash file" do
+      assert_equal([607, 756], MediaFile.open("test/files/compressed.swf").dimensions)
+    end
+  end
+
+  context "#file_ext" do
+    should "determine the correct extension for a jpeg file" do
+      assert_equal(:jpg, MediaFile.open("test/files/test.jpg").file_ext)
+      assert_equal(:jpg, MediaFile.open("test/files/test-blank.jpg").file_ext)
+      assert_equal(:jpg, MediaFile.open("test/files/test-exif-small.jpg").file_ext)
+      assert_equal(:jpg, MediaFile.open("test/files/test-large.jpg").file_ext)
+    end
+
+    should "determine the correct extension for a png file" do
+      assert_equal(:png, MediaFile.open("test/files/test.png").file_ext)
+      assert_equal(:png, MediaFile.open("test/files/apng/normal_apng.png").file_ext)
+      assert_equal(:png, MediaFile.open("test/files/alpha.png").file_ext)
+    end
+
+    should "determine the correct extension for a gif file" do
+      assert_equal(:gif, MediaFile.open("test/files/test.gif").file_ext)
+      assert_equal(:gif, MediaFile.open("test/files/test-animated-86x52.gif").file_ext)
+      assert_equal(:gif, MediaFile.open("test/files/test-static-32x32.gif").file_ext)
+    end
+
+    should "determine the correct extension for a webm file" do
+      assert_equal(:webm, MediaFile.open("test/files/test-512x512.webm").file_ext)
+    end
+
+    should "determine the correct extension for a mp4 file" do
+      assert_equal(:mp4, MediaFile.open("test/files/test-300x300.mp4").file_ext)
+    end
+
+    should "determine the correct extension for a ugoira file" do
+      assert_equal(:zip, MediaFile.open("test/files/valid_ugoira.zip").file_ext)
+    end
+
+    should "determine the correct extension for a flash file" do
+      assert_equal(:swf, MediaFile.open("test/files/compressed.swf").file_ext)
+    end
+  end
+
+  should "determine the correct md5 for a jpeg file" do
+    assert_equal("ecef68c44edb8a0d6a3070b5f8e8ee76", MediaFile.open("test/files/test.jpg").md5)
+  end
+
+  should "determine the correct filesize for a jpeg file" do
+    assert_equal(28086, MediaFile.open("test/files/test.jpg").file_size)
+  end
+end

test/unit/upload_service_test.rb

@@ -77,105 +77,21 @@ class UploadServiceTest < ActiveSupport::TestCase
       end
     end
 
-    context ".calculate_ugoira_dimensions" do
-      context "for a valid ugoira file" do
-        setup do
-          @path = "test/files/valid_ugoira.zip"
-        end
-
-        should "extract the dimensions" do
-          w, h = subject.calculate_ugoira_dimensions(@path)
-          assert_operator(w, :>, 0)
-          assert_operator(h, :>, 0)
-        end
-      end
-
-      context "for an invalid ugoira file" do
-        setup do
-          @path = "test/files/invalid_ugoira.zip"
-        end
-
-        should "raise an error" do
-          assert_raises(ImageSpec::Error) do
-            subject.calculate_ugoira_dimensions(@path)
-          end
-        end
-      end
-    end
-
-    context ".calculate_dimensions" do
-      context "for an ugoira" do
-        setup do
-          @file = File.open("test/files/valid_ugoira.zip", "rb")
-          @upload = Upload.new(file_ext: "zip")
-        end
-
-        teardown do
-          @file.close
-        end
-
-        should "return the dimensions" do
-          subject.expects(:calculate_ugoira_dimensions).once.returns([60, 60])
-          subject.calculate_dimensions(@upload, @file) do |w, h|
-            assert_operator(w, :>, 0)
-            assert_operator(h, :>, 0)
-          end
-        end
-      end
-
-      context "for a video" do
-        setup do
-          @file = File.open("test/files/test-300x300.mp4", "rb")
-          @upload = Upload.new(file_ext: "mp4")
-        end
-
-        teardown do
-          @file.close
-        end
-
-        should "return the dimensions" do
-          subject.calculate_dimensions(@upload, @file) do |w, h|
-            assert_operator(w, :>, 0)
-            assert_operator(h, :>, 0)
-          end
-        end
-      end
-
-      context "for an image" do
-        setup do
-          @file = File.open("test/files/test.jpg", "rb")
-          @upload = Upload.new(file_ext: "jpg")
-        end
-
-        teardown do
-          @file.close
-        end
-
-        should "find the dimensions" do
-          subject.calculate_dimensions(@upload, @file) do |w, h|
-            assert_operator(w, :>, 0)
-            assert_operator(h, :>, 0)
-          end
-        end
-      end
-    end
-
     context ".process_file" do
       setup do
         @upload = FactoryBot.build(:jpg_upload)
-        @file = @upload.file
       end
 
       context "with an original_post_id" do
         should "run" do
           subject.expects(:distribute_files).times(3)
-          subject.process_file(@upload, @file, original_post_id: 12345)
+          subject.process_file(@upload, @upload.file.tempfile, original_post_id: 12345)
         end
       end
 
       should "run" do
         subject.expects(:distribute_files).times(3)
-        subject.process_file(@upload, @file)
+        subject.process_file(@upload, @upload.file.tempfile)
         assert_equal("jpg", @upload.file_ext)
         assert_equal(28086, @upload.file_size)
         assert_equal("ecef68c44edb8a0d6a3070b5f8e8ee76", @upload.md5)
@@ -188,7 +104,7 @@ class UploadServiceTest < ActiveSupport::TestCase
       context "for an ugoira" do
         setup do
           context = UGOIRA_CONTEXT
-          @file = File.open("test/fixtures/ugoira.zip", "rb")
+          @file = upload_file("test/fixtures/ugoira.zip")
           @upload = mock
           @upload.stubs(:is_video?).returns(false)
           @upload.stubs(:is_ugoira?).returns(true)
@@ -220,7 +136,7 @@ class UploadServiceTest < ActiveSupport::TestCase
 
         context "for an mp4" do
           setup do
-            @file = File.open("test/files/test-300x300.mp4", "rb")
+            @file = upload_file("test/files/test-300x300.mp4")
             @upload = mock
             @upload.stubs(:is_video?).returns(true)
             @upload.stubs(:is_ugoira?).returns(false)
@@ -243,7 +159,7 @@ class UploadServiceTest < ActiveSupport::TestCase
 
         context "for a webm" do
           setup do
-            @file = File.open("test/files/test-512x512.webm", "rb")
+            @file = upload_file("test/files/test-512x512.webm")
             @upload = mock
             @upload.stubs(:is_video?).returns(true)
             @upload.stubs(:is_ugoira?).returns(false)
@@ -281,7 +197,7 @@ class UploadServiceTest < ActiveSupport::TestCase
 
         context "for a jpeg" do
           setup do
-            @file = File.open("test/files/test.jpg", "rb")
+            @file = upload_file("test/files/test.jpg")
           end
 
           should "generate a preview" do
@@ -298,7 +214,7 @@ class UploadServiceTest < ActiveSupport::TestCase
 
         context "for a png" do
           setup do
-            @file = File.open("test/files/test.png", "rb")
+            @file = upload_file("test/files/test.png")
           end
 
           should "generate a preview" do
@@ -315,7 +231,7 @@ class UploadServiceTest < ActiveSupport::TestCase
 
         context "for a gif" do
           setup do
-            @file = File.open("test/files/test.png", "rb")
+            @file = upload_file("test/files/test.png")
           end
 
           should "generate a preview" do