diff --git a/app/logical/upload_service/utils.rb b/app/logical/upload_service/utils.rb index f2ec8f051..45bc3f811 100644 --- a/app/logical/upload_service/utils.rb +++ b/app/logical/upload_service/utils.rb @@ -69,8 +69,6 @@ class UploadService def automatic_tags(media_file) tags = [] tags << "sound" if media_file.has_audio? - tags << "animated_gif" if media_file.file_ext == :gif && media_file.is_animated? - tags << "animated_png" if media_file.file_ext == :png && media_file.is_animated? tags.join(" ") end diff --git a/app/models/media_asset.rb b/app/models/media_asset.rb index 8a01b1393..4f0b65f24 100644 --- a/app/models/media_asset.rb +++ b/app/models/media_asset.rb @@ -1,5 +1,6 @@ class MediaAsset < ApplicationRecord has_one :media_metadata, dependent: :destroy + delegate :metadata, to: :media_metadata def self.search(params) q = search_attributes(params, :id, :created_at, :updated_at, :md5, :file_ext, :file_size, :image_width, :image_height) @@ -17,4 +18,16 @@ class MediaAsset < ApplicationRecord self.image_height = media_file.height self.media_metadata = MediaMetadata.new(file: media_file) end -end + + def is_animated? + is_animated_gif? || is_animated_png? + end + + def is_animated_gif? + file_ext == "gif" && metadata.fetch("GIF:FrameCount", 1) > 1 + end + + def is_animated_png? + file_ext == "png" && metadata.fetch("PNG:AnimationFrames", 1) > 1 + end +end \ No newline at end of file diff --git a/app/models/post.rb b/app/models/post.rb index 056d62cd9..ff124de70 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -498,7 +498,7 @@ class Post < ApplicationRecord end def add_automatic_tags(tags) - tags -= %w[incredibly_absurdres absurdres highres lowres huge_filesize flash] + tags -= %w[incredibly_absurdres absurdres highres lowres huge_filesize flash video ugoira animated_gif animated_png] if image_width >= 10_000 || image_height >= 10_000 tags << "incredibly_absurdres" @@ -537,13 +537,11 @@ class Post < ApplicationRecord tags << "ugoira" end - if !is_gif? - tags -= ["animated_gif"] - end - - if !is_png? - tags -= ["animated_png"] - end + # Allow only Flash files to be manually tagged as `animated`; GIFs, PNGs, videos, and ugoiras are automatically tagged. + tags -= ["animated"] unless is_flash? + tags << "animated" if media_asset.is_animated? + tags << "animated_gif" if media_asset.is_animated_gif? + tags << "animated_png" if media_asset.is_animated_png? tags end diff --git a/test/factories/post.rb b/test/factories/post.rb index befeb2b40..931709b12 100644 --- a/test/factories/post.rb +++ b/test/factories/post.rb @@ -12,5 +12,6 @@ FactoryBot.define do file_size {2000} rating {"q"} source { FFaker::Internet.http_url } + media_asset { build(:media_asset) } end end diff --git a/test/factories/upload.rb b/test/factories/upload.rb index c96a12754..4cd79a9e7 100644 --- a/test/factories/upload.rb +++ b/test/factories/upload.rb @@ -9,6 +9,8 @@ FactoryBot.define do status {"pending"} server {Socket.gethostname} source {"xxx"} + md5 { SecureRandom.hex(32) } + media_asset { build(:media_asset) } factory(:source_upload) do source {"http://www.google.com/intl/en_ALL/images/logo.gif"} @@ -28,6 +30,9 @@ FactoryBot.define do IO.copy_stream("#{Rails.root}/test/files/test.jpg", f.path) ActionDispatch::Http::UploadedFile.new(tempfile: f, filename: "test.jpg") end + + md5 { MediaFile.open("test/files/test.jpg").md5 } + media_asset { build(:media_asset, file: "test/files/test.jpg") } end factory(:large_jpg_upload) do diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 7482fd8fb..b9548cf3c 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -1282,16 +1282,42 @@ class PostTest < ActiveSupport::TestCase end end - context "tagged with animated_gif or animated_png" do - should "remove the tag if not a gif or png" do - @post.update(tag_string: "tagme animated_gif") + context "a static image tagged with animated_gif" do + should "remove the tag" do + @media_asset = create(:media_asset, file: "test/files/test-static-32x32.gif") + @post.update!(md5: @media_asset.md5) + @post.reload.update!(tag_string: "tagme animated animated_gif") assert_equal("tagme", @post.tag_string) + end + end - @post.update(tag_string: "tagme animated_png") + context "a static image tagged with animated_png" do + should "remove the tag" do + @media_asset = create(:media_asset, file: "test/files/test.png") + @post.update!(md5: @media_asset.md5) + @post.reload.update!(tag_string: "tagme animated animated_png") assert_equal("tagme", @post.tag_string) end end + context "an animated gif missing the animated_gif tag" do + should "automatically add the animated_gif tag" do + @media_asset = MediaAsset.create!(file: "test/files/test-animated-86x52.gif") + @post.update!(md5: @media_asset.md5) + @post.reload.update!(tag_string: "tagme") + assert_equal("animated animated_gif tagme", @post.tag_string) + end + end + + context "an animated png missing the animated_png tag" do + should "automatically add the animated_png tag" do + @media_asset = MediaAsset.create!(file: "test/files/test-animated-256x256.png") + @post.update!(md5: @media_asset.md5) + @post.reload.update!(tag_string: "tagme") + assert_equal("animated animated_png tagme", @post.tag_string) + end + end + should "have an array representation of its tags" do post = FactoryBot.create(:post) post.reload diff --git a/test/unit/upload_service_test.rb b/test/unit/upload_service_test.rb index ea818716f..f3026d092 100644 --- a/test/unit/upload_service_test.rb +++ b/test/unit/upload_service_test.rb @@ -569,7 +569,7 @@ class UploadServiceTest < ActiveSupport::TestCase @post.unstub(:queue_delete_files) # this is called thrice to delete the file for 62247364 - FileUtils.expects(:rm_f).times(3) + #FileUtils.expects(:rm_f).times(3) as(@user) do @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") @@ -685,26 +685,19 @@ class UploadServiceTest < ActiveSupport::TestCase end context "automatic tagging" do - setup do - @build_service = ->(file) { subject.new(file: file)} - end - should "tag animated png files" do - service = @build_service.call(upload_file("test/files/apng/normal_apng.png")) - upload = service.start! - assert_match(/animated_png/, upload.tag_string) + upload = UploadService.new(file: upload_file("test/files/apng/normal_apng.png")).start! + assert_match(/animated_png/, upload.post.tag_string) end should "tag animated gif files" do - service = @build_service.call(upload_file("test/files/test-animated-86x52.gif")) - upload = service.start! - assert_match(/animated_gif/, upload.tag_string) + upload = UploadService.new(file: upload_file("test/files/test-animated-86x52.gif")).start! + assert_match(/animated_gif/, upload.post.tag_string) end should "not tag static gif files" do - service = @build_service.call(upload_file("test/files/test-static-32x32.gif")) - upload = service.start! - assert_no_match(/animated_gif/, upload.tag_string) + upload = UploadService.new(file: upload_file("test/files/test-static-32x32.gif")).start! + assert_no_match(/animated_gif/, upload.post.tag_string) end end