From e477232e02600811185a29770015558b8a8f5cc9 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 6 May 2020 00:33:35 -0500 Subject: [PATCH] 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 | 77 ++++++++++++++++ app/logical/media_file/flash.rb | 6 ++ app/logical/media_file/image.rb | 6 ++ app/logical/media_file/ugoira.rb | 13 +++ app/logical/media_file/video.rb | 9 ++ app/logical/upload_service.rb | 2 +- app/logical/upload_service/preprocessor.rb | 4 +- app/logical/upload_service/utils.rb | 67 ++------------ test/unit/media_file_test.rb | 85 ++++++++++++++++++ test/unit/upload_service_test.rb | 100 ++------------------- 10 files changed, 215 insertions(+), 154 deletions(-) create mode 100644 app/logical/media_file.rb create mode 100644 app/logical/media_file/flash.rb create mode 100644 app/logical/media_file/image.rb create mode 100644 app/logical/media_file/ugoira.rb create mode 100644 app/logical/media_file/video.rb create mode 100644 test/unit/media_file_test.rb diff --git a/app/logical/media_file.rb b/app/logical/media_file.rb new file mode 100644 index 000000000..91ca4a95f --- /dev/null +++ b/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 diff --git a/app/logical/media_file/flash.rb b/app/logical/media_file/flash.rb new file mode 100644 index 000000000..d7f8dc024 --- /dev/null +++ b/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 diff --git a/app/logical/media_file/image.rb b/app/logical/media_file/image.rb new file mode 100644 index 000000000..e266f2577 --- /dev/null +++ b/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 diff --git a/app/logical/media_file/ugoira.rb b/app/logical/media_file/ugoira.rb new file mode 100644 index 000000000..7c9d30817 --- /dev/null +++ b/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 diff --git a/app/logical/media_file/video.rb b/app/logical/media_file/video.rb new file mode 100644 index 000000000..d11b08ad7 --- /dev/null +++ b/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 diff --git a/app/logical/upload_service.rb b/app/logical/upload_service.rb index 87ff1b672..c2473284c 100644 --- a/app/logical/upload_service.rb +++ b/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! diff --git a/app/logical/upload_service/preprocessor.rb b/app/logical/upload_service/preprocessor.rb index 3bb443f27..6c422bff3 100644 --- a/app/logical/upload_service/preprocessor.rb +++ b/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 diff --git a/app/logical/upload_service/utils.rb b/app/logical/upload_service/utils.rb index e8939122b..e0d91bb14 100644 --- a/app/logical/upload_service/utils.rb +++ b/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 - upload.file_ext = Utils.file_header_to_file_ext(file) - upload.file_size = file.size - upload.md5 = Digest::MD5.file(file.path).hexdigest + binding.pry if !file.is_a? Tempfile + media_file = MediaFile.open(file) - Utils.calculate_dimensions(upload, file) do |width, height| - upload.image_width = width - upload.image_height = height - end + upload.file = file + upload.file_ext = media_file.file_ext.to_s + upload.file_size = media_file.file_size + 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)}" diff --git a/test/unit/media_file_test.rb b/test/unit/media_file_test.rb new file mode 100644 index 000000000..2277b56e2 --- /dev/null +++ b/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 diff --git a/test/unit/upload_service_test.rb b/test/unit/upload_service_test.rb index d99360175..79dd9a2d7 100644 --- a/test/unit/upload_service_test.rb +++ b/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