From 4836804cae6f87ed0c6e8b2771f956379ccb5822 Mon Sep 17 00:00:00 2001 From: Albert Yi Date: Mon, 25 Jun 2018 14:53:06 -0700 Subject: [PATCH] bubble up upload validation errors during replacement restore missing UploadService::Replacer tests addresses #3761 --- app/logical/upload_service.rb | 6 +- test/models/upload_service_test.rb | 394 +++++++++++++++++++++++++++++ 2 files changed, 397 insertions(+), 3 deletions(-) diff --git a/app/logical/upload_service.rb b/app/logical/upload_service.rb index 3b0307cf9..2a5f54f3c 100644 --- a/app/logical/upload_service.rb +++ b/app/logical/upload_service.rb @@ -311,16 +311,16 @@ class UploadService if Utils.is_downloadable?(source) CurrentUser.as_system do if Post.tag_match("source:#{source}").exists? - return + raise ActiveRecord::RecordNotUnique.new("A post with source #{source} already exists") end end if Upload.where(source: source, status: "completed").exists? - return + raise ActiveRecord::RecordNotUnique.new("A completed upload with source #{source} already exists") end if Upload.where(source: source).where("status like ?", "error%").exists? - return + raise ActiveRecord::RecordNotUnique.new("An errored upload with source #{source} already exists") end end diff --git a/test/models/upload_service_test.rb b/test/models/upload_service_test.rb index 52fddc9a8..9107bda06 100644 --- a/test/models/upload_service_test.rb +++ b/test/models/upload_service_test.rb @@ -376,6 +376,400 @@ class UploadServiceTest < ActiveSupport::TestCase end end + context "::Replacer" do + context "for a file replacement" do + setup do + @new_file = upload_file("test/files/test.jpg") + @old_file = upload_file("test/files/test.png") + travel_to(1.month.ago) do + @user = FactoryBot.create(:user) + end + as_user do + @post = FactoryBot.create(:post, md5: Digest::MD5.hexdigest(@old_file.read)) + @old_md5 = @post.md5 + @post.stubs(:queue_delete_files) + @replacement = FactoryBot.create(:post_replacement, post: @post, replacement_url: "", replacement_file: @new_file) + end + end + + subject { UploadService::Replacer.new(post: @post, replacement: @replacement) } + + context "#process!" do + should "create a new upload" do + assert_difference(-> { Upload.count }) do + as_user { subject.process! } + end + end + + should "create a comment" do + assert_difference(-> { @post.comments.count }) do + as_user { subject.process! } + @post.reload + end + end + + should "not create a new post" do + assert_difference(-> { Post.count }, 0) do + as_user { subject.process! } + end + end + + should "update the post's MD5" do + assert_changes(-> { @post.md5 }) do + as_user { subject.process! } + @post.reload + end + end + + should "preserve the old values" do + as_user { subject.process! } + assert_equal(1500, @replacement.image_width_was) + assert_equal(1000, @replacement.image_height_was) + assert_equal(2000, @replacement.file_size_was) + assert_equal("jpg", @replacement.file_ext_was) + assert_equal(@old_md5, @replacement.md5_was) + end + + should "record the new values" do + as_user { subject.process! } + assert_equal(500, @replacement.image_width) + assert_equal(335, @replacement.image_height) + assert_equal(28086, @replacement.file_size) + assert_equal("jpg", @replacement.file_ext) + assert_equal("ecef68c44edb8a0d6a3070b5f8e8ee76", @replacement.md5) + end + + should "correctly update the attributes" do + as_user { subject.process! } + assert_equal(500, @post.image_width) + assert_equal(335, @post.image_height) + assert_equal(28086, @post.file_size) + assert_equal("jpg", @post.file_ext) + assert_equal("ecef68c44edb8a0d6a3070b5f8e8ee76", @post.md5) + assert(File.exists?(@post.file.path)) + end + end + + context "a post with the same file" do + should "not raise a duplicate error" do + upload_file("test/files/test.png") do |file| + assert_nothing_raised do + as_user { @post.replace!(replacement_file: file, replacement_url: "") } + end + end + end + + should "not queue a deletion or log a comment" do + upload_file("test/files/test.png") do |file| + assert_no_difference(-> { @post.comments.count }) do + as_user { @post.replace!(replacement_file: file, replacement_url: "") } + @post.reload + end + end + end + end + end + + context "for a source replacement" do + setup do + @new_url = "https://upload.wikimedia.org/wikipedia/commons/c/c5/Moraine_Lake_17092005.jpg" + travel_to(1.month.ago) do + @user = FactoryBot.create(:user) + end + as_user do + @post = FactoryBot.create(:post, uploader_ip_addr: "127.0.0.2") + @post.stubs(:queue_delete_files) + @replacement = FactoryBot.create(:post_replacement, post: @post, replacement_url: @new_url) + end + end + + subject { UploadService::Replacer.new(post: @post, replacement: @replacement) } + + context "when an upload with the same source already exists" do + setup do + @post = FactoryBot.create(:post, source: @new_url) + end + + should "throw an error" do + assert_raises(ActiveRecord::RecordNotUnique) do + as_user { @post.replace!(replacement_url: @new_url) } + end + end + end + + context "a post when given a final_source" do + should "change the source to the final_source" do + replacement_url = "http://data.tumblr.com/afed9f5b3c33c39dc8c967e262955de2/tumblr_orwwptNBCE1wsfqepo1_raw.png" + final_source = "https://noizave.tumblr.com/post/162094447052" + + as_user { @post.replace!(replacement_url: replacement_url, final_source: final_source) } + + assert_equal(final_source, @post.source) + end + end + + context "a post when replaced with a HTML source" do + should "record the image URL as the replacement URL, not the HTML source" do + skip "Twitter key not set" unless Danbooru.config.twitter_api_key + replacement_url = "https://twitter.com/nounproject/status/540944400767922176" + image_url = "https://pbs.twimg.com/media/B4HSEP5CUAA4xyu.png:orig" + as_user { @post.replace!(replacement_url: replacement_url) } + + assert_equal(image_url, @post.replacements.last.replacement_url) + end + end + + context "#undo!" do + setup do + @user = travel_to(1.month.ago) { FactoryBot.create(:user) } + as_user do + @post = FactoryBot.create(:post, source: "https://www.google.com/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png") + @post.stubs(:queue_delete_files) + @post.replace!(replacement_url: "https://danbooru.donmai.us/data/preview/download.png", tags: "-tag1 tag2") + end + + @replacement = @post.replacements.last + end + + should "update the attributes" do + as_user do + subject.undo! + end + + assert_equal("lowres tag2", @post.tag_string) + assert_equal(272, @post.image_width) + assert_equal(92, @post.image_height) + assert_equal(5969, @post.file_size) + assert_equal("png", @post.file_ext) + assert_equal("8f9327db2597fa57d2f42b4a6c5a9855", @post.md5) + assert_equal("8f9327db2597fa57d2f42b4a6c5a9855", Digest::MD5.file(@post.file).hexdigest) + assert_equal("https://www.google.com/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png", @post.source) + end + end + + context "#process!" do + should "create a new upload" do + assert_difference(-> { Upload.count }) do + as_user { subject.process! } + end + end + + should "create a comment" do + assert_difference(-> { @post.comments.count }) do + as_user { subject.process! } + @post.reload + end + end + + should "not create a new post" do + assert_difference(-> { Post.count }, 0) do + as_user { subject.process! } + end + end + + should "update the post's MD5" do + assert_changes(-> { @post.md5 }) do + as_user { subject.process! } + @post.reload + end + end + + should "update the post's source" do + assert_changes(-> { @post.source }, nil, from: @post.source, to: @new_url) do + as_user { subject.process! } + @post.reload + end + end + + should "not change the post status or uploader" do + assert_no_changes(-> { {ip_addr: @post.uploader_ip_addr.to_s, uploader: @post.uploader_id, pending: @post.is_pending?} }) do + as_user { subject.process! } + @post.reload + end + end + + should "leave a system comment" do + as_user { subject.process! } + comment = @post.comments.last + assert_not_nil(comment) + assert_equal(User.system.id, comment.creator_id) + assert_match(/replaced this post/, comment.body) + end + end + + context "a post with a pixiv html source" do + setup do + Delayed::Worker.delay_jobs = true + end + + teardown do + Delayed::Worker.delay_jobs = false + end + + should "replace with the full size image" do + begin + as_user do + @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") + end + + assert_equal(80, @post.image_width) + assert_equal(82, @post.image_height) + assert_equal(16275, @post.file_size) + assert_equal("png", @post.file_ext) + assert_equal("4ceadc314938bc27f3574053a3e1459a", @post.md5) + assert_equal("4ceadc314938bc27f3574053a3e1459a", Digest::MD5.file(@post.file).hexdigest) + assert_equal("https://i.pximg.net/img-original/img/2017/04/04/08/54/15/62247350_p0.png", @post.replacements.last.replacement_url) + assert_equal("https://i.pximg.net/img-original/img/2017/04/04/08/54/15/62247350_p0.png", @post.source) + rescue Net::OpenTimeout + skip "Remote connection to Pixiv failed" + end + end + + should "delete the old files after thirty days" do + begin + @post.unstub(:queue_delete_files) + FileUtils.expects(:rm_f).times(3) + + as_user { @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") } + + travel_to((PostReplacement::DELETION_GRACE_PERIOD + 1).days.from_now) do + Delayed::Worker.new.work_off + end + rescue Net::OpenTimeout + skip "Remote connection to Pixiv failed" + end + end + end + + context "a post that is replaced by a ugoira" do + should "save the frame data" do + skip "ffmpeg not installed" unless check_ffmpeg + begin + as_user { @post.replace!(replacement_url: "http://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") } + @post.reload + + assert_equal(80, @post.image_width) + assert_equal(82, @post.image_height) + assert_equal(2804, @post.file_size) + assert_equal("zip", @post.file_ext) + assert_equal("cad1da177ef309bf40a117c17b8eecf5", @post.md5) + assert_equal("cad1da177ef309bf40a117c17b8eecf5", Digest::MD5.file(@post.file).hexdigest) + + assert_equal("https://i.pximg.net/img-zip-ugoira/img/2017/04/04/08/57/38/62247364_ugoira1920x1080.zip", @post.source) + assert_equal([{"delay"=>125, "file"=>"000000.jpg"}, {"delay"=>125,"file"=>"000001.jpg"}], @post.pixiv_ugoira_frame_data.data) + rescue Net::OpenTimeout + skip "Remote connection to Pixiv failed" + end + end + end + + context "a post that is replaced to another file then replaced back to the original file" do + setup do + Delayed::Worker.delay_jobs = true + end + + teardown do + Delayed::Worker.delay_jobs = false + end + + should "not delete the original files" do + begin + FileUtils.expects(:rm_f).never + + as_user do + @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") + @post.reload + @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") + @post.reload + Upload.destroy_all + @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") + end + + assert_nothing_raised { @post.file(:original) } + assert_nothing_raised { @post.file(:preview) } + + travel_to((PostReplacement::DELETION_GRACE_PERIOD + 1).days.from_now) do + Delayed::Worker.new.work_off + end + + assert_nothing_raised { @post.file(:original) } + assert_nothing_raised { @post.file(:preview) } + rescue Net::OpenTimeout + skip "Remote connection to Pixiv failed" + end + end + end + + context "two posts that have had their files swapped" do + setup do + Delayed::Worker.delay_jobs = true + + as_user do + @post1 = FactoryBot.create(:post) + @post2 = FactoryBot.create(:post) + end + end + + teardown do + Delayed::Worker.delay_jobs = false + end + + should "not delete the still active files" do + # swap the images between @post1 and @post2. + begin + as_user do + @post1.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") + @post2.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") + @post2.replace!(replacement_url: "https://www.google.com/intl/en_ALL/images/logo.gif") + Upload.destroy_all + @post1.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") + @post2.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") + end + + Timecop.travel(Time.now + PostReplacement::DELETION_GRACE_PERIOD + 1.day) do + Delayed::Worker.new.work_off + end + + assert_nothing_raised { @post1.file(:original) } + assert_nothing_raised { @post2.file(:original) } + rescue Net::OpenTimeout + skip "Remote connection to Pixiv failed" + end + end + end + + context "a post with notes" do + setup do + Note.any_instance.stubs(:merge_version?).returns(false) + + as_user do + @post.update(image_width: 160, image_height: 164) + @note = @post.notes.create(x: 80, y: 82, width: 80, height: 82, body: "test") + @note.reload + end + end + + should "rescale the notes" do + assert_equal([80, 82, 80, 82], [@note.x, @note.y, @note.width, @note.height]) + + begin + assert_difference(-> { @note.versions.count }) do + # replacement image is 80x82, so we're downscaling by 50% (160x164 -> 80x82). + as_user do + @post.replace!(replacement_url: "https://upload.wikimedia.org/wikipedia/commons/c/c5/Moraine_Lake_17092005.jpg") + end + @note.reload + end + + assert_equal([1024, 768, 1024, 768], [@note.x, @note.y, @note.width, @note.height]) + rescue Net::OpenTimeout + skip "Remote connection to Pixiv failed" + end + end + end + end + end + context "#start!" do subject { UploadService }