From 9a522d9fefc018d0d146d2358804d5c6698a9ccd Mon Sep 17 00:00:00 2001 From: Albert Yi Date: Mon, 2 Jul 2018 13:26:31 -0700 Subject: [PATCH] for upload service, move md5 validation to upload/post. exclude self when doing a self replacement. --- app/logical/upload_service.rb | 15 ++++++--------- test/models/upload_service_test.rb | 28 +++++++++++++++++++++------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/app/logical/upload_service.rb b/app/logical/upload_service.rb index 2a5f54f3c..ceb664bac 100644 --- a/app/logical/upload_service.rb +++ b/app/logical/upload_service.rb @@ -168,11 +168,6 @@ class UploadService upload.file_size = file.size upload.md5 = Digest::MD5.file(file.path).hexdigest - if Post.where(md5: upload.md5).exists? - # abort early if this post already exists - raise Upload::Error.new("Post with MD5 #{upload.md5} already exists") - end - Utils.calculate_dimensions(upload, file) do |width, height| upload.image_width = width upload.image_height = height @@ -271,9 +266,10 @@ class UploadService end class Preprocessor - attr_reader :params + attr_reader :params, :original_post_id def initialize(params) + @original_post_id = params.delete(:original_post_id) || -1 @params = params end @@ -310,7 +306,7 @@ class UploadService def start!(uploader_id) if Utils.is_downloadable?(source) CurrentUser.as_system do - if Post.tag_match("source:#{source}").exists? + if Post.tag_match("source:#{source}").where.not(id: original_post_id).exists? raise ActiveRecord::RecordNotUnique.new("A post with source #{source} already exists") end end @@ -328,8 +324,8 @@ class UploadService params[:tag_string] ||= "tagme" CurrentUser.as(User.find(uploader_id)) do + upload = Upload.create!(params) begin - upload = Upload.create!(params) upload.update(status: "preprocessing") if source.present? @@ -429,7 +425,8 @@ class UploadService rating: post.rating, tag_string: replacement.tags, source: replacement.replacement_url, - file: replacement.replacement_file + file: replacement.replacement_file, + original_post_id: post.id ) upload = preprocessor.start!(CurrentUser.id) return if upload.is_errored? diff --git a/test/models/upload_service_test.rb b/test/models/upload_service_test.rb index 9107bda06..de1eeefd4 100644 --- a/test/models/upload_service_test.rb +++ b/test/models/upload_service_test.rb @@ -472,12 +472,14 @@ class UploadServiceTest < ActiveSupport::TestCase context "for a source replacement" do setup do - @new_url = "https://upload.wikimedia.org/wikipedia/commons/c/c5/Moraine_Lake_17092005.jpg" + @new_url = "https://raikou1.donmai.us/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg" + @new_md5 = "d34e4cf0a437a5d65f8e82b7bcd02606" 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_md5 = "710fd9cba4ef37260f9152ffa9d154d8" + @post = FactoryBot.create(:post, source: "https://raikou1.donmai.us/71/0f/#{@post_md5}.png", file_ext: "png", md5: @post_md5, uploader_ip_addr: "127.0.0.2") @post.stubs(:queue_delete_files) @replacement = FactoryBot.create(:post_replacement, post: @post, replacement_url: @new_url) end @@ -485,22 +487,34 @@ class UploadServiceTest < ActiveSupport::TestCase subject { UploadService::Replacer.new(post: @post, replacement: @replacement) } - context "when an upload with the same source already exists" do + context "when replacing with its own source" do + should "work" do + as_user { @post.replace!(replacement_url: @post.source) } + assert_equal(@post_md5, @post.md5) + assert_match(/#{@post_md5}/, @post.file_path) + end + end + + context "when an upload with the same MD5 already exists" do setup do - @post = FactoryBot.create(:post, source: @new_url) + @post.update(md5: @new_md5) + as_user do + @post2 = FactoryBot.create(:post) + @post2.stubs(:queue_delete_files) + end end should "throw an error" do assert_raises(ActiveRecord::RecordNotUnique) do - as_user { @post.replace!(replacement_url: @new_url) } + as_user { @post2.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" + replacement_url = "https://raikou1.donmai.us/fd/b4/fdb47f79fb8da82e66eeb1d84a1cae8d.jpg" + final_source = "https://raikou1.donmai.us/71/0f/710fd9cba4ef37260f9152ffa9d154d8.png" as_user { @post.replace!(replacement_url: replacement_url, final_source: final_source) }