uploads: factor out remaining image methods to MediaFile.

app/logical/media_file.rb

@@ -89,6 +89,22 @@ class MediaFile
     file_ext == :swf
   end
 
+  def is_corrupt?
+    false
+  end
+
+  def is_animated?
+    is_video?
+  end
+
+  def has_audio?
+    false
+  end
+
+  def duration
+    0.0
+  end
+
   def preview(width, height, **options)
     nil
   end
@@ -97,5 +113,5 @@ class MediaFile
     nil
   end
 
-  memoize :dimensions, :file_ext, :file_size, :md5
+  memoize :dimensions, :file_ext, :file_size, :md5, :is_corrupt?, :is_animated?
 end

app/logical/media_file/image.rb

@@ -20,6 +20,17 @@ class MediaFile::Image < MediaFile
     [0, 0]
   end
 
+  def is_corrupt?
+    image.stats
+    false
+  rescue Vips::Error
+    true
+  end
+
+  def is_animated?
+    is_animated_gif? || is_animated_png?
+  end
+
   # https://github.com/jcupitt/libvips/wiki/HOWTO----Image-shrinking
   # http://jcupitt.github.io/libvips/API/current/Using-vipsthumbnail.md.html
   def preview(width, height)
@@ -40,6 +51,14 @@ class MediaFile::Image < MediaFile
 
   private
 
+  def is_animated_gif?
+    file_ext == :gif && image.get("n-pages") > 1
+  end
+
+  def is_animated_png?
+    file_ext == :png && APNGInspector.new(file.path).inspect!.animated?
+  end
+
   def image
     @image ||= Vips::Image.new_from_file(file.path, fail: true)
   end

app/logical/media_file/ugoira.rb

@@ -1,5 +1,4 @@
 class MediaFile::Ugoira < MediaFile
-  extend Memoist
   class Error < StandardError; end
   attr_reader :frame_data
 

app/logical/media_file/video.rb

@@ -1,10 +1,12 @@
 class MediaFile::Video < MediaFile
-  extend Memoist
-
   def dimensions
     [video.width, video.height]
   end
 
+  def duration
+    video.duration
+  end
+
   def preview(max_width, max_height)
     preview_frame.preview(max_width, max_height)
   end
@@ -13,6 +15,10 @@ class MediaFile::Video < MediaFile
     preview_frame.crop(max_width, max_height)
   end
 
+  def has_audio?
+    video.audio_channels.present?
+  end
+
   private
 
   def video

app/logical/upload_service/utils.rb

@@ -2,14 +2,6 @@ class UploadService
   module Utils
     module_function
 
-    def corrupt?(filename)
-      image = Vips::Image.new_from_file(filename, fail: true)
-      image.stats
-      false
-    rescue Vips::Error
-      true
-    end
-
     def distribute_files(file, record, type, original_post_id: nil)
       # need to do this for hybrid storage manager
       post = Post.new
@@ -51,7 +43,7 @@ class UploadService
       upload.image_height = media_file.height
 
       upload.validate!(:file)
-      upload.tag_string = "#{upload.tag_string} #{Utils.automatic_tags(upload, file)}"
+      upload.tag_string = "#{upload.tag_string} #{Utils.automatic_tags(media_file)}"
 
       preview_file, crop_file, sample_file = Utils.generate_resizes(media_file)
 
@@ -67,38 +59,11 @@ class UploadService
       end
     end
 
-    # these methods are only really used during upload processing even
+    def automatic_tags(media_file)
-    # though logically they belong on upload. post can rely on the
-    # automatic tag that's added.
-    def is_animated_gif?(upload, file)
-      return false if upload.file_ext != "gif"
-
-      # Check whether the gif has multiple frames by trying to load the second frame.
-      result = Vips::Image.gifload(file.path, page: 1) rescue $ERROR_INFO
-      if result.is_a?(Vips::Image)
-        true
-      elsif result.is_a?(Vips::Error) && result.message =~ /too few frames in GIF file/
-        false
-      else
-        raise result
-      end
-    end
-
-    def is_animated_png?(upload, file)
-      upload.file_ext == "png" && APNGInspector.new(file.path).inspect!.animated?
-    end
-
-    def is_video_with_audio?(upload, file)
-      return false if !upload.is_video? # avoid ffprobe'ing the file if it's not a video (issue #3826)
-      video = FFMPEG::Movie.new(file.path)
-      video.audio_channels.present?
-    end
-
-    def automatic_tags(upload, file)
       tags = []
-      tags << "video_with_sound" if is_video_with_audio?(upload, file)
+      tags << "video_with_sound" if media_file.has_audio?
-      tags << "animated_gif" if is_animated_gif?(upload, file)
+      tags << "animated_gif" if media_file.file_ext == :gif && media_file.is_animated?
-      tags << "animated_png" if is_animated_png?(upload, file)
+      tags << "animated_png" if media_file.file_ext == :png && media_file.is_animated?
       tags.join(" ")
     end
 

app/models/upload.rb

@@ -1,5 +1,3 @@
-require "tmpdir"
-
 class Upload < ApplicationRecord
   class Error < StandardError; end
 
@@ -19,8 +17,8 @@ class Upload < ApplicationRecord
     end
 
     def validate_integrity(record)
-      if record.file_ext.in?(["jpg", "gif", "png"]) && UploadService::Utils.corrupt?(record.file.path)
+      if record.media_file.is_corrupt?
-        record.errors[:file] << "File is corrupted"
+        record.errors[:file] << "is corrupted"
       end
     end
 
@@ -55,7 +53,7 @@ class Upload < ApplicationRecord
     end
 
     def validate_video_duration(record)
-      if record.is_video? && record.video.duration > 120
+      if record.media_file.is_video? && record.media_file.duration > 120
         record.errors[:base] << "video must not be longer than 2 minutes"
       end
     end
@@ -99,27 +97,11 @@ class Upload < ApplicationRecord
     end
   end
 
-  module FileMethods
+  concerning :FileMethods do
     def media_file
       @media_file ||= MediaFile.open(file, frame_data: context.to_h.dig("ugoira", "frame_data"))
     end
 
-    def is_image?
-      %w(jpg gif png).include?(file_ext)
-    end
-
-    def is_flash?
-      %w(swf).include?(file_ext)
-    end
-
-    def is_video?
-      %w(webm mp4).include?(file_ext)
-    end
-
-    def is_ugoira?
-      %w(zip).include?(file_ext)
-    end
-
     def delete_files
       # md5 is blank if the upload errored out before downloading the file.
       if is_completed? || md5.blank? || Upload.where(md5: md5).exists? || Post.where(md5: md5).exists?
@@ -136,7 +118,7 @@ class Upload < ApplicationRecord
     end
   end
 
-  module StatusMethods
+  concerning :StatusMethods do
     def is_pending?
       status == "pending"
     end
@@ -178,7 +160,7 @@ class Upload < ApplicationRecord
     end
   end
 
-  module SourceMethods
+  concerning :SourceMethods do
     def source=(source)
       source = source.unicode_normalize(:nfc)
 
@@ -196,13 +178,7 @@ class Upload < ApplicationRecord
     end
   end
 
-  module VideoMethods
+  concerning :SearchMethods do
-    def video
-      @video ||= FFMPEG::Movie.new(file.path)
-    end
-  end
-
-  module SearchMethods
     def search(params)
       q = super
 
@@ -234,22 +210,12 @@ class Upload < ApplicationRecord
     end
   end
 
-  include FileMethods
-  include StatusMethods
-  include VideoMethods
-  extend SearchMethods
-  include SourceMethods
-
   def assign_rating_from_tags
     if rating = PostQueryBuilder.new(tag_string).find_metatag(:rating)
       self.rating = rating.downcase.first
     end
   end
 
-  def presenter
-    @presenter ||= UploadPresenter.new(self)
-  end
-
   def upload_as_pending?
     as_pending.to_s.truthy?
   end

test/unit/upload_service_test.rb

@@ -843,7 +843,6 @@ class UploadServiceTest < ActiveSupport::TestCase
 
         upload = upload_from_file(@bad_jpeg_path)
         assert_match(/corrupt/, upload.status)
-        assert_equal(true, UploadService::Utils.corrupt?(@bad_jpeg_path))
       end
 
       should "fail for a corrupted gif" do
@@ -851,7 +850,6 @@ class UploadServiceTest < ActiveSupport::TestCase
 
         upload = upload_from_file(@bad_gif_path)
         assert_match(/corrupt/, upload.status)
-        assert_equal(true, UploadService::Utils.corrupt?(@bad_gif_path))
       end
 
       # https://schaik.com/pngsuite/pngsuite_xxx_png.html
@@ -860,7 +858,6 @@ class UploadServiceTest < ActiveSupport::TestCase
 
         upload = upload_from_file(@bad_png_path)
         assert_match(/corrupt/, upload.status)
-        assert_equal(true, UploadService::Utils.corrupt?(@bad_png_path))
       end
     end
   end