From 0e6c358701fdb0eb1e5279ea3fc3d6ca59760db3 Mon Sep 17 00:00:00 2001 From: Albert Yi Date: Wed, 6 Jun 2018 14:55:27 -0700 Subject: [PATCH] add drag and drop file uploads w/async processing [skip ci] --- app/assets/javascripts/uploads.js | 2 +- app/assets/stylesheets/specific/uploads.scss | 31 +++ app/controllers/uploads_controller.rb | 12 +- app/logical/upload_service.rb | 187 +++++++++++++++++-- app/models/post_replacement.rb | 123 ------------ app/views/uploads/new.html.erb | 61 +++++- config/routes.rb | 1 + test/models/upload_service_test.rb | 107 ++++++++++- test/unit/post_replacement_test.rb | 6 +- 9 files changed, 375 insertions(+), 155 deletions(-) diff --git a/app/assets/javascripts/uploads.js b/app/assets/javascripts/uploads.js index ad511a1b6..406739e4a 100644 --- a/app/assets/javascripts/uploads.js +++ b/app/assets/javascripts/uploads.js @@ -33,7 +33,7 @@ Danbooru.Upload.initialize_submit = function() { $("#form").submit(function(e) { var error_messages = []; - if (($("#upload_file").val() === "") && ($("#upload_source").val() === "")) { + if (($("#upload_file").val() === "") && ($("#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/assets/stylesheets/specific/uploads.scss b/app/assets/stylesheets/specific/uploads.scss index 49b71d0b1..f32d49fd6 100644 --- a/app/assets/stylesheets/specific/uploads.scss +++ b/app/assets/stylesheets/specific/uploads.scss @@ -34,6 +34,37 @@ div#c-uploads { div.field_with_errors { display: inline; } + + #filedropzone { + border: 4px dashed #DDD; + padding: 10px; + max-width: 700px; + min-height: 50px; + + &.error { + background-color: #f2dede; + } + + &.success { + background-color: #dff0d8; + } + } + + .dz-preview { + margin-bottom: 1em; + } + + .dz-progress { + height: 20px; + width: 300px; + border: 1px solid #CCC; + + .dz-upload { + background-color: #F5F5FF; + display: block; + height: 20px; + } + } } div#a-index { diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 62f9081f4..e6afe9dbf 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -1,10 +1,13 @@ class UploadsController < ApplicationController before_action :member_only, except: [:index, :show] respond_to :html, :xml, :json, :js + skip_before_action :verify_authenticity_token, only: [:preprocess] def new @upload_notice_wiki = WikiPage.titled(Danbooru.config.upload_notice_wiki_page).first - @upload, @post, @source, @normalized_url, @remote_size = UploadService::ControllerHelper.prepare(params[:url], params[:ref]) + @upload, @post, @source, @normalized_url, @remote_size = UploadService::ControllerHelper.prepare( + url: params[:url], ref: params[:ref] + ) respond_with(@upload) end @@ -39,6 +42,13 @@ class UploadsController < ApplicationController end end + def preprocess + @upload, @post, @source, @normalized_url, @remote_size = UploadService::ControllerHelper.prepare( + url: params[:url], file: params[:file], ref: params[:ref] + ) + render body: nil + end + def create @service = UploadService.new(upload_params) @upload = @service.start! diff --git a/app/logical/upload_service.rb b/app/logical/upload_service.rb index 7509cdcba..3b27d9f03 100644 --- a/app/logical/upload_service.rb +++ b/app/logical/upload_service.rb @@ -1,9 +1,11 @@ class UploadService module ControllerHelper - def self.prepare(url, ref = nil) + def self.prepare(url: nil, file: nil, ref: nil) upload = Upload.new if url + # this gets called from UploadsController#new so we need + # to preprocess async Preprocessor.new(source: url).delay(queue: "default").start!(CurrentUser.user.id) download = Downloads::File.new(url) @@ -21,6 +23,9 @@ class UploadService end return [upload, post, source, normalized_url, remote_size] + elsif file + # this gets called via XHR so we can process sync + Preprocessor.new(file: file).start!((CurrentUser.user.id)) end return [upload] @@ -216,12 +221,26 @@ class UploadService params[:source] end + def md5 + params[:md5_confirmation] + end + def in_progress? - Upload.where(status: "preprocessing", source: source).exists? + if source.present? + Upload.where(status: "preprocessing", source: source).exists? + elsif md5.present? + Upload.where(status: "preprocessing", md5: md5).exists? + else + false + end end def predecessor - Upload.where(status: ["preprocessed", "preprocessing"], source: source).first + if source.present? + Upload.where(status: ["preprocessed", "preprocessing"], source: source).first + elsif md5.present? + Upload.where(status: ["preprocessed", "preprocessing"], md5: md5).first + end end def completed? @@ -229,20 +248,22 @@ class UploadService end def start!(uploader_id) - if !Utils.is_downloadable?(source) - return - end + if source.present? + if !Utils.is_downloadable?(source) + return + end - if Post.where(source: source).exists? - return - end + if Post.where(source: source).exists? + return + end - if Upload.where(source: source, status: "completed").exists? - return - end + if Upload.where(source: source, status: "completed").exists? + return + end - if Upload.where(source: source).where("status like ?", "error%").exists? - return + if Upload.where(source: source).where("status like ?", "error%").exists? + return + end end params[:rating] ||= "q" @@ -254,13 +275,17 @@ class UploadService upload.update(status: "preprocessing") begin - file = download_from_source(source, referer_url: upload.referer_url) do |context| - upload.downloaded_source = context[:downloaded_source] - upload.source = context[:source] + if source.present? + file = download_from_source(source, referer_url: upload.referer_url) do |context| + upload.downloaded_source = context[:downloaded_source] + upload.source = context[:source] - if context[:ugoira] - upload.context = { ugoira: context[:ugoira] } + if context[:ugoira] + upload.context = { ugoira: context[:ugoira] } + end end + elsif params[:file].present? + file = params[:file] end Utils.process_file(upload, file) @@ -306,6 +331,128 @@ class UploadService end end + class Replacer + attr_reader :post, :replacement + + def initialize(post:, replacement:) + @post = post + @replacement = replacement + end + + def comment_replacement_message(post, replacement) + %("#{replacement.creator.name}":[/users/#{replacement.creator.id}] replaced this post with a new image:\n\n#{replacement_message(post, replacement)}) + end + + def replacement_message(post, replacement) + linked_source = linked_source(replacement.replacement_url) + linked_source_was = linked_source(post.source_was) + + <<-EOS.strip_heredoc + [table] + [tbody] + [tr] + [th]Old[/th] + [td]#{linked_source_was}[/td] + [td]#{post.md5_was}[/td] + [td]#{post.file_ext_was}[/td] + [td]#{post.image_width_was} x #{post.image_height_was}[/td] + [td]#{post.file_size_was.human_size(precision: 4)}[/td] + [/tr] + [tr] + [th]New[/th] + [td]#{linked_source}[/td] + [td]#{post.md5}[/td] + [td]#{post.file_ext}[/td] + [td]#{post.image_width} x #{post.image_height}[/td] + [td]#{post.file_size.human_size(precision: 4)}[/td] + [/tr] + [/tbody] + [/table] + EOS + end + + def linked_source(source) + return nil if source.nil? + + # truncate long sources in the middle: "www.pixiv.net...lust_id=23264933" + truncated_source = source.gsub(%r{\Ahttps?://}, "").truncate(64, omission: "...#{source.last(32)}") + + if source =~ %r{\Ahttps?://}i + %("#{truncated_source}":[#{source}]) + else + truncated_source + end + end + + def suggested_tags_for_removal + tags = post.tag_array.select { |tag| Danbooru.config.remove_tag_after_replacement?(tag) } + tags = tags.map { |tag| "-#{tag}" } + tags.join(" ") + end + + def process! + preprocessor = Preprocessor.new( + rating: post.rating, + tag_string: replacement.tags, + source: replacement.replacement_url, + file: replacement.replacement_file + ) + upload = preprocessor.start!(CurrentUser.id) + upload = preprocessor.finish! + md5_changed = upload.md5 != post.md5 + + if replacement.replacement_file.present? + replacement.replacement_url = "file://#{replacement.replacement_file.original_filename}" + elsif upload.downloaded_source.present? + replacement.replacement_url = upload.downloaded_source + end + + if md5_changed + post.queue_delete_files(PostReplacement::DELETION_GRACE_PERIOD) + end + + replacement.file_ext = upload.file_ext + replacement.file_size = upload.file_size + replacement.image_height = upload.image_height + replacement.image_width = upload.image_width + replacement.md5 = upload.md5 + + post.md5 = upload.md5 + post.file_ext = upload.file_ext + post.image_width = upload.image_width + post.image_height = upload.image_height + post.file_size = upload.file_size + post.source = upload.source + post.tag_string = upload.tag_string + + rescale_notes(post) + update_ugoira_frame_data(post, upload) + + if md5_changed + CurrentUser.as(User.system) do + post.comments.create!(body: comment_replacement_message(post, replacement), do_not_bump_post: true) + end + end + + replacement.save! + post.save! + end + + def rescale_notes(post) + x_scale = post.image_width.to_f / post.image_width_was.to_f + y_scale = post.image_height.to_f / post.image_height_was.to_f + + post.notes.each do |note| + note.rescale!(x_scale, y_scale) + end + end + + def update_ugoira_frame_data(post, upload) + post.pixiv_ugoira_frame_data.destroy if post.pixiv_ugoira_frame_data.present? + upload.ugoira_service.save_frame_data(post) if post.is_ugoira? + end + end + attr_reader :params, :post, :upload def initialize(params) @@ -370,7 +517,7 @@ class UploadService @post = convert_to_post(upload) @post.save! - @upload.update(status: "error: " + @post.errors.full_messages.join(", ")) + upload.update(status: "error: " + @post.errors.full_messages.join(", ")) if upload.context && upload.context["ugoira"] PixivUgoiraFrameData.create( diff --git a/app/models/post_replacement.rb b/app/models/post_replacement.rb index 8ea4046ce..2174d0972 100644 --- a/app/models/post_replacement.rb +++ b/app/models/post_replacement.rb @@ -23,77 +23,6 @@ class PostReplacement < ApplicationRecord undo_replacement.process! end - def process! - upload = nil - - transaction do - upload = Upload.create!( - file: replacement_file, - source: replacement_url, - rating: post.rating, - tag_string: self.tags, - replaced_post: post, - ) - - upload.process_upload - upload.update(status: "completed", post_id: post.id) - md5_changed = (upload.md5 != post.md5) - - if replacement_file.present? - self.replacement_url = "file://#{replacement_file.original_filename}" - else - self.replacement_url = upload.downloaded_source - end - - # queue the deletion *before* updating the post so that we use the old - # md5/file_ext to delete the old files. if saving the post fails, - # this is rolled back so the job won't run. - if md5_changed - post.queue_delete_files(DELETION_GRACE_PERIOD) - end - - self.file_ext = upload.file_ext - self.file_size = upload.file_size - self.image_height = upload.image_height - self.image_width = upload.image_width - self.md5 = upload.md5 - - post.md5 = upload.md5 - post.file_ext = upload.file_ext - post.image_width = upload.image_width - post.image_height = upload.image_height - post.file_size = upload.file_size - post.source = final_source.presence || upload.source - post.tag_string = upload.tag_string - - rescale_notes - update_ugoira_frame_data(upload) - - if md5_changed - CurrentUser.as(User.system) do - post.comments.create!(body: comment_replacement_message, do_not_bump_post: true) - end - end - - save! - post.save! - end - - # point of no return: these things can't be rolled back, so we do them - # only after the transaction successfully commits. - upload.distribute_files(post) - post.update_iqdb_async - end - - def rescale_notes - x_scale = post.image_width.to_f / post.image_width_was.to_f - y_scale = post.image_height.to_f / post.image_height_was.to_f - - post.notes.each do |note| - note.rescale!(x_scale, y_scale) - end - end - def update_ugoira_frame_data(upload) post.pixiv_ugoira_frame_data.destroy if post.pixiv_ugoira_frame_data.present? upload.ugoira_service.save_frame_data(post) if post.is_ugoira? @@ -127,57 +56,5 @@ class PostReplacement < ApplicationRecord end end - module PresenterMethods - def comment_replacement_message - %("#{creator.name}":[/users/#{creator.id}] replaced this post with a new image:\n\n#{replacement_message}) - end - - def replacement_message - linked_source = linked_source(replacement_url) - linked_source_was = linked_source(post.source_was) - - <<-EOS.strip_heredoc - [table] - [tbody] - [tr] - [th]Old[/th] - [td]#{linked_source_was}[/td] - [td]#{post.md5_was}[/td] - [td]#{post.file_ext_was}[/td] - [td]#{post.image_width_was} x #{post.image_height_was}[/td] - [td]#{post.file_size_was.to_s(:human_size, precision: 4)}[/td] - [/tr] - [tr] - [th]New[/th] - [td]#{linked_source}[/td] - [td]#{post.md5}[/td] - [td]#{post.file_ext}[/td] - [td]#{post.image_width} x #{post.image_height}[/td] - [td]#{post.file_size.to_s(:human_size, precision: 4)}[/td] - [/tr] - [/tbody] - [/table] - EOS - end - - def linked_source(source) - # truncate long sources in the middle: "www.pixiv.net...lust_id=23264933" - truncated_source = source.gsub(%r{\Ahttps?://}, "").truncate(64, omission: "...#{source.last(32)}") - - if source =~ %r{\Ahttps?://}i - %("#{truncated_source}":[#{source}]) - else - truncated_source - end - end - - def suggested_tags_for_removal - tags = post.tag_array.select { |tag| Danbooru.config.remove_tag_after_replacement?(tag) } - tags = tags.map { |tag| "-#{tag}" } - tags.join(" ") - end - end - - include PresenterMethods extend SearchMethods end diff --git a/app/views/uploads/new.html.erb b/app/views/uploads/new.html.erb index d392321e8..7fa2d59ec 100644 --- a/app/views/uploads/new.html.erb +++ b/app/views/uploads/new.html.erb @@ -21,6 +21,7 @@ <%= hidden_field_tag :url, params[:url] %> <%= hidden_field_tag :ref, params[:ref] %> <%= hidden_field_tag :normalized_url, @normalized_url %> + <%= f.hidden_field :md5_confirmation %> <%= f.hidden_field :referer_url, :value => @source.try(:referer_url) %> <% if CurrentUser.can_upload_free? %> @@ -37,6 +38,10 @@ <%= f.file_field :file, :size => 50 %> +
+ Drag and drop a file here +
+
<%= f.label :source %> <% if params[:url].present? %> @@ -112,9 +117,9 @@ <%= button_tag "Related tags", :id => "related-tags-button", :type => "button", :class => "ui-button ui-widget ui-corner-all sub gradient" %> - <% TagCategory.related_button_list.each do |category| %> - <%= button_tag "#{TagCategory.related_button_mapping[category]}", :id => "related-#{category}-button", :type => "button", :class => "ui-button ui-widget ui-corner-all sub gradient" %> - <% end %> + <% TagCategory.related_button_list.each do |category| %> + <%= button_tag "#{TagCategory.related_button_mapping[category]}", :id => "related-#{category}-button", :type => "button", :class => "ui-button ui-widget ui-corner-all sub gradient" %> + <% end %>
@@ -143,4 +148,54 @@ Upload - <%= Danbooru.config.app_name %> <% end %> +<% content_for(:html_header) do %> + + + +<% end %> + <%= render "uploads/secondary_links" %> diff --git a/config/routes.rb b/config/routes.rb index b6d9c5584..2a0664b0b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -279,6 +279,7 @@ Rails.application.routes.draw do resource :tag_implication_request, :only => [:new, :create] resources :uploads do collection do + post :preprocess get :batch get :image_proxy end diff --git a/test/models/upload_service_test.rb b/test/models/upload_service_test.rb index 9423bf9c4..aa37fa3d0 100644 --- a/test/models/upload_service_test.rb +++ b/test/models/upload_service_test.rb @@ -326,7 +326,7 @@ class UploadServiceTest < ActiveSupport::TestCase should "work for a jpeg" do @service = subject.new(source: @jpeg) - @upload = @service.start! + @upload = @service.start!(CurrentUser.id) assert_equal("preprocessed", @upload.status) assert_not_nil(@upload.md5) assert_equal("jpg", @upload.file_ext) @@ -339,7 +339,7 @@ class UploadServiceTest < ActiveSupport::TestCase should "work for an ugoira" do @service = subject.new(source: @ugoira) - @upload = @service.start! + @upload = @service.start!(CurrentUser.id) assert_equal("preprocessed", @upload.status) assert_not_nil(@upload.md5) assert_equal("zip", @upload.file_ext) @@ -351,7 +351,7 @@ class UploadServiceTest < ActiveSupport::TestCase should "work for a video" do @service = subject.new(source: @video) - @upload = @service.start! + @upload = @service.start!(CurrentUser.id) assert_equal("preprocessed", @upload.status) assert_not_nil(@upload.md5) assert_equal("mp4", @upload.file_ext) @@ -368,7 +368,7 @@ class UploadServiceTest < ActiveSupport::TestCase should "leave the upload in an error state" do @service = subject.new(source: @video) - @upload = @service.start! + @upload = @service.start!(CurrentUser.id) assert_match(/error:/, @upload.status) end end @@ -376,6 +376,103 @@ 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") + travel_to(1.month.ago) do + @user = FactoryBot.create(:user) + end + as_user do + @post = FactoryBot.create(:post) + @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 + 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) + @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 "#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 + end + end + end + context "#start!" do subject { UploadService } @@ -450,7 +547,7 @@ class UploadServiceTest < ActiveSupport::TestCase context "with a preprocessed predecessor" do setup do - @predecessor = FactoryBot.create(:source_upload, status: "preprocessed", source: @source, image_height: 0, image_width: 0, file_ext: "jpg") + @predecessor = FactoryBot.create(:source_upload, status: "preprocessed", source: @source, file_size: 0, md5: "something", image_height: 0, image_width: 0, file_ext: "jpg") @tags = 'hello world' end diff --git a/test/unit/post_replacement_test.rb b/test/unit/post_replacement_test.rb index 7d96a07c4..128407dba 100644 --- a/test/unit/post_replacement_test.rb +++ b/test/unit/post_replacement_test.rb @@ -27,8 +27,9 @@ class PostReplacementTest < ActiveSupport::TestCase context "Replacing" do setup do CurrentUser.scoped(@uploader, "127.0.0.2") do - upload = FactoryBot.create(:jpg_upload, as_pending: "0", tag_string: "lowres tag1") - upload.process! + attributes = FactoryBot.attributes_for(:jpg_upload, as_pending: "0", tag_string: "lowres tag1") + service = UploadService.new(attributes) + upload = service.start! @post = upload.post end end @@ -65,6 +66,7 @@ class PostReplacementTest < ActiveSupport::TestCase end should "create a post replacement record" do + binding.pry assert_equal(@post.id, PostReplacement.last.post_id) end