diff --git a/app/javascript/src/javascripts/uploads.js b/app/javascript/src/javascripts/uploads.js index 904318aa1..ea59af4f3 100644 --- a/app/javascript/src/javascripts/uploads.js +++ b/app/javascript/src/javascripts/uploads.js @@ -35,7 +35,7 @@ Upload.initialize_submit = function() { Upload.validate_upload = function (e) { var error_messages = []; - if (($("#upload_file").val() === "") && ($("#upload_source").val() === "") && $("#upload_md5_confirmation").val() === "") { + if (($("#upload_file").val() === "") && !/^https?:\/\//i.test($("#upload_source").val()) && $("#upload_md5_confirmation").val() === "") { error_messages.push("Must choose file or specify source"); } if (!$("#upload_rating_s").prop("checked") && !$("#upload_rating_q").prop("checked") && !$("#upload_rating_e").prop("checked") && diff --git a/app/logical/upload_service.rb b/app/logical/upload_service.rb index 42ac90a4c..1974726b9 100644 --- a/app/logical/upload_service.rb +++ b/app/logical/upload_service.rb @@ -48,13 +48,8 @@ class UploadService @upload.update(status: "processing") - if @upload.file.nil? && @upload.source_url.present? - @upload.file = Utils.download_for_upload(@upload) - end - - if @upload.file.present? - Utils.process_file(upload, @upload.file) - end + @upload.file = Utils.get_file_for_upload(@upload, file: @upload.file) + Utils.process_file(upload, @upload.file) @upload.save! @post = create_post_from_upload(@upload) diff --git a/app/logical/upload_service/preprocessor.rb b/app/logical/upload_service/preprocessor.rb index 77fef8b96..a9d0802cf 100644 --- a/app/logical/upload_service/preprocessor.rb +++ b/app/logical/upload_service/preprocessor.rb @@ -91,12 +91,7 @@ class UploadService begin upload.update(status: "preprocessing") - if params[:file].present? - file = params[:file] - elsif upload.source_url.present? - file = Utils.download_for_upload(upload) - end - + file = Utils.get_file_for_upload(upload, file: params[:file]) Utils.process_file(upload, file, original_post_id: original_post_id) upload.rating = params[:rating] diff --git a/app/logical/upload_service/utils.rb b/app/logical/upload_service/utils.rb index 73f26cc07..93270815d 100644 --- a/app/logical/upload_service/utils.rb +++ b/app/logical/upload_service/utils.rb @@ -206,7 +206,10 @@ class UploadService tags.join(" ") end - def download_for_upload(upload) + def get_file_for_upload(upload, file: nil) + return file if file.present? + raise RuntimeError, "No file or source URL provided" if upload.source_url.blank? + attempts = 0 begin diff --git a/app/models/upload.rb b/app/models/upload.rb index b775a2a51..481afc1be 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -115,11 +115,11 @@ class Upload < ApplicationRecord end def is_duplicate? - status =~ /duplicate: \d+/ + status.match?(/duplicate: \d+/) end def is_errored? - status =~ /error:/ + status.match?(/error:/) end def sanitized_status diff --git a/test/models/upload_service_test.rb b/test/models/upload_service_test.rb index eaa51bb16..f7ae19355 100644 --- a/test/models/upload_service_test.rb +++ b/test/models/upload_service_test.rb @@ -25,7 +25,7 @@ class UploadServiceTest < ActiveSupport::TestCase context "::Utils" do subject { UploadService::Utils } - context "#download_for_upload" do + context "#get_file_for_upload" do context "for a non-source site" do setup do @source = "https://upload.wikimedia.org/wikipedia/commons/c/c5/Moraine_Lake_17092005.jpg" @@ -34,7 +34,7 @@ class UploadServiceTest < ActiveSupport::TestCase end should "work on a jpeg" do - file = subject.download_for_upload(@upload) + file = subject.get_file_for_upload(@upload) assert_operator(File.size(file.path), :>, 0) @@ -59,7 +59,7 @@ class UploadServiceTest < ActiveSupport::TestCase should "retry three times" do DanbooruImageResizer.expects(:validate_shell).times(4).returns(false) assert_raise(UploadService::Utils::CorruptFileError) do - subject.download_for_upload(@mock_upload) + subject.get_file_for_upload(@mock_upload) end end end @@ -73,7 +73,7 @@ class UploadServiceTest < ActiveSupport::TestCase should "work on an ugoira url" do begin - file = subject.download_for_upload(@upload) + file = subject.get_file_for_upload(@upload) assert_operator(File.size(file.path), :>, 0) @@ -95,7 +95,7 @@ class UploadServiceTest < ActiveSupport::TestCase should "work on an ugoira url" do begin - file = subject.download_for_upload(@upload) + file = subject.get_file_for_upload(@upload) assert_not_nil(@upload.context["ugoira"]) assert_operator(File.size(file.path), :>, 0) @@ -1108,6 +1108,27 @@ class UploadServiceTest < ActiveSupport::TestCase assert_equal(source2, @upload.source) end end + + context "without a file or a source url" do + should "fail gracefully" do + service = subject.new(source: "blah", rating: "s") + + assert_nothing_raised { @upload = service.start! } + assert_equal(true, @upload.is_errored?) + assert_match(/No file or source URL provided/, @upload.status) + end + end + + context "with both a file and a source url" do + should "upload the file and set the source field to the given source" do + service = subject.new(file: upload_file("test/files/test.jpg"), source: "http://www.example.com", rating: "s") + + assert_nothing_raised { @upload = service.start! } + assert_equal(true, @upload.is_completed?) + assert_equal("ecef68c44edb8a0d6a3070b5f8e8ee76", @upload.md5) + assert_equal("http://www.example.com", @upload.source) + end + end end context "#create_post_from_upload" do