From b4a38c68b497f70233a126ac75ea0af56c3f0967 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 31 Mar 2017 13:38:48 -0500 Subject: [PATCH 1/9] upload.rb: separate out post creation from upload processing. --- app/models/upload.rb | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/app/models/upload.rb b/app/models/upload.rb index bcf4e1672..fc19c8b0d 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -105,7 +105,7 @@ class Upload < ActiveRecord::Base end module ConversionMethods - def process_once + def process_upload CurrentUser.scoped(uploader, uploader_ip_addr) do update_attribute(:status, "processing") self.source = strip_source @@ -129,16 +129,19 @@ class Upload < ActiveRecord::Base move_file validate_md5_confirmation_after_move save - post = convert_to_post - post.distribute_files - if post.save - User.where(id: CurrentUser.id).update_all("post_upload_count = post_upload_count + 1") - create_artist_commentary(post) if include_artist_commentary? - ugoira_service.save_frame_data(post) if is_ugoira? - update_attributes(:status => "completed", :post_id => post.id) - else - update_attribute(:status, "error: " + post.errors.full_messages.join(", ")) - end + end + end + + def create_post_from_upload + post = convert_to_post + post.distribute_files + if post.save + User.where(id: CurrentUser.id).update_all("post_upload_count = post_upload_count + 1") + create_artist_commentary(post) if include_artist_commentary? + ugoira_service.save_frame_data(post) if is_ugoira? + update_attributes(:status => "completed", :post_id => post.id) + else + update_attribute(:status, "error: " + post.errors.full_messages.join(", ")) end end @@ -146,7 +149,8 @@ class Upload < ActiveRecord::Base @tries ||= 0 return if !force && status =~ /processing|completed|error/ - process_once + process_upload + create_post_from_upload rescue Timeout::Error, Net::HTTP::Persistent::Error => x if @tries > 3 From e2b988a562a3660494b47169d908511ffbfd697e Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 31 Mar 2017 13:39:31 -0500 Subject: [PATCH 2/9] post replacement: add Post#replace! method. --- app/models/post.rb | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/app/models/post.rb b/app/models/post.rb index b5daed3bf..9fb84a4e7 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1393,6 +1393,35 @@ class Post < ActiveRecord::Base Post.expire_cache_for_all(tag_array) ModAction.log("undeleted post ##{id}") end + + def replace!(url) + transaction do + upload = Upload.create!(source: url, rating: self.rating, tag_string: self.tag_string) + upload.process_upload + upload.update(status: "completed", post_id: id) + + self.md5 = upload.md5 + self.file_ext = upload.file_ext + self.image_width = upload.image_width + self.image_height = upload.image_height + self.file_size = upload.file_size + self.source = upload.source + self.tag_string = upload.tag_string + + ModAction.log(<<-EOS.strip_heredoc) + replaced post ##{id}: #{image_width_was}x#{image_height_was} (#{file_size_was.to_formatted_s(:human_size)} #{file_ext_was.upcase}) -> #{image_width}x#{image_height} (#{file_size.to_formatted_s(:human_size)} #{file_ext.upcase}) + source: #{source_was} -> #{source} + md5: "#{md5_was}":[/data/#{md5_was}.#{file_ext_was}] -> "#{md5}":[/data/#{md5}.#{file_ext}] + EOS + + save! + end + + # point of no return: these things can't be rolled back, so we do them + # only after the transaction successfully commits. + distribute_files + update_iqdb_async + end end module VersionMethods From ca01539c4b86ecb3cb0d80565a75f76021b63e39 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 2 May 2017 19:47:47 -0500 Subject: [PATCH 3/9] post replacement: delete old files after image is replaced. --- app/helpers/delayed_jobs_helper.rb | 6 +++++ app/models/post.rb | 39 ++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/app/helpers/delayed_jobs_helper.rb b/app/helpers/delayed_jobs_helper.rb index 8b90b3512..13d75c5f3 100644 --- a/app/helpers/delayed_jobs_helper.rb +++ b/app/helpers/delayed_jobs_helper.rb @@ -61,6 +61,9 @@ module DelayedJobsHelper when "Pool#update_category_pseudo_tags_for_posts" "update pool category pseudo tags for posts" + when "Post.delete_files" + "delete old files" + else h(job.name) end @@ -122,6 +125,9 @@ module DelayedJobsHelper when "Pool#update_category_pseudo_tags_for_posts" %{#{h(job.payload_object.name)}} + when "Post.delete_files" + %{post ##{job.payload_object.args.first}} + else h(job.handler) end diff --git a/app/models/post.rb b/app/models/post.rb index 9fb84a4e7..9de7bbc0b 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -7,6 +7,8 @@ class Post < ActiveRecord::Base class RevertError < Exception ; end class SearchError < Exception ; end + DELETION_GRACE_PERIOD = 3.days + before_validation :initialize_uploader, :on => :create before_validation :merge_old_changes before_validation :normalize_tags @@ -29,7 +31,6 @@ class Post < ActiveRecord::Base after_save :expire_essential_tag_string_cache after_destroy :remove_iqdb_async after_destroy :delete_files - after_destroy :delete_remote_files after_commit :update_iqdb_async, :on => :create after_commit :notify_pubsub @@ -60,24 +61,31 @@ class Post < ActiveRecord::Base attr_accessor :old_tag_string, :old_parent_id, :old_source, :old_rating, :has_constraints, :disable_versioning, :view_count module FileMethods + extend ActiveSupport::Concern + + module ClassMethods + def delete_files(post_id, file_path, large_file_path, preview_file_path) + # the large file and the preview don't necessarily exist. if so errors will be ignored. + FileUtils.rm_f(file_path) + FileUtils.rm_f(large_file_path) + FileUtils.rm_f(preview_file_path) + + RemoteFileManager.new(file_path).delete + RemoteFileManager.new(large_file_path).delete + RemoteFileManager.new(preview_file_path).delete + end + end + + def delete_files + Post.delete_files(id, file_path, large_file_path, preview_file_path) + end + def distribute_files RemoteFileManager.new(file_path).distribute RemoteFileManager.new(preview_file_path).distribute if has_preview? RemoteFileManager.new(large_file_path).distribute if has_large? end - def delete_remote_files - RemoteFileManager.new(file_path).delete - RemoteFileManager.new(preview_file_path).delete if has_preview? - RemoteFileManager.new(large_file_path).delete if has_large? - end - - def delete_files - FileUtils.rm_f(file_path) - FileUtils.rm_f(large_file_path) - FileUtils.rm_f(preview_file_path) - end - def file_path_prefix Rails.env == "test" ? "test." : "" end @@ -1400,6 +1408,11 @@ class Post < ActiveRecord::Base upload.process_upload upload.update(status: "completed", post_id: id) + # 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. + Post.delay(queue: "default", run_at: Time.now + DELETION_GRACE_PERIOD).delete_files(id, file_path, large_file_path, preview_file_path) + self.md5 = upload.md5 self.file_ext = upload.file_ext self.image_width = upload.image_width From dd920b3ffc778e26606fac968d02d31ea6a6585b Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 2 May 2017 19:57:44 -0500 Subject: [PATCH 4/9] post replacement: disallow unhandled cases. --- app/models/post.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/app/models/post.rb b/app/models/post.rb index 9de7bbc0b..71135d769 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1403,6 +1403,21 @@ class Post < ActiveRecord::Base end def replace!(url) + # TODO for posts with notes we need to rescale the notes if the dimensions change. + if notes.size > 0 + raise NotImplementedError.new("Replacing images with notes not yet supported.") + end + + # TODO for ugoiras we need to replace the frame data. + if is_ugoira? + raise NotImplementedError.new("Replacing ugoira images not yet supported.") + end + + # TODO images hosted on s3 need to be deleted from s3 instead of the local filesystem. + if Danbooru.config.use_s3_proxy?(self) + raise NotImplementedError.new("Replacing S3 hosted images not yet supported.") + end + transaction do upload = Upload.create!(source: url, rating: self.rating, tag_string: self.tag_string) upload.process_upload From df7cd67a7d62076cc191e2698f58a0c8dc73d973 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 31 Mar 2017 13:40:06 -0500 Subject: [PATCH 5/9] post replacement: add POST /moderator/post/posts/replace endpoint. --- app/controllers/moderator/post/posts_controller.rb | 13 ++++++++++++- config/routes.rb | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/controllers/moderator/post/posts_controller.rb b/app/controllers/moderator/post/posts_controller.rb index 94ceb1166..10e7e701d 100644 --- a/app/controllers/moderator/post/posts_controller.rb +++ b/app/controllers/moderator/post/posts_controller.rb @@ -1,10 +1,12 @@ module Moderator module Post class PostsController < ApplicationController - before_filter :approver_only, :only => [:delete, :undelete, :move_favorites, :ban, :unban, :confirm_delete, :confirm_move_favorites, :confirm_ban] + before_filter :approver_only, :only => [:delete, :undelete, :move_favorites, :replace, :ban, :unban, :confirm_delete, :confirm_move_favorites, :confirm_ban] before_filter :admin_only, :only => [:expunge] skip_before_filter :api_check + respond_to :html, :json, :xml + def confirm_delete @post = ::Post.find(params[:id]) end @@ -35,6 +37,15 @@ module Moderator redirect_to(post_path(@post)) end + def replace + @post = ::Post.find(params[:id]) + @post.replace!(params[:post][:source]) + + respond_with(@post) do |format| + format.html { redirect_to(@post) } + end + end + def expunge @post = ::Post.find(params[:id]) @post.expunge! diff --git a/config/routes.rb b/config/routes.rb index 45cddb0fc..3d2b95e0b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,6 +30,7 @@ Rails.application.routes.draw do member do get :confirm_delete post :expunge + post :replace post :delete post :undelete get :confirm_move_favorites From 4e841c4ea5d968cb52aaa49823f56ce6b64ad18a Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 31 Mar 2017 13:37:57 -0500 Subject: [PATCH 6/9] post replacement: add "Replace Image" dialog to post sidebar. --- app/assets/javascripts/posts.js | 27 +++++++++++++++++++ .../moderator/post/posts/_replace.html.erb | 11 ++++++++ .../moderator/post/posts/replace.html.erb | 5 ++++ .../posts/partials/show/_options.html.erb | 2 ++ app/views/posts/show.html.erb | 4 +++ 5 files changed, 49 insertions(+) create mode 100644 app/views/moderator/post/posts/_replace.html.erb create mode 100644 app/views/moderator/post/posts/replace.html.erb diff --git a/app/assets/javascripts/posts.js b/app/assets/javascripts/posts.js index 2ba3c2320..fc303a44f 100644 --- a/app/assets/javascripts/posts.js +++ b/app/assets/javascripts/posts.js @@ -23,6 +23,7 @@ this.initialize_post_image_resize_links(); this.initialize_post_image_resize_to_window_link(); this.initialize_similar(); + this.initialize_replace_image_dialog(); if (Danbooru.meta("always-resize-images") === "true") { $("#image-resize-to-window-link").click(); @@ -606,6 +607,32 @@ e.preventDefault(); }); } + + Danbooru.Post.initialize_replace_image_dialog = function() { + $("#replace-image-dialog").dialog({ + autoOpen: false, + width: 700, + modal: true, + buttons: { + "Submit": function() { + $("#replace-image-dialog form").submit(); + $(this).dialog("close"); + }, + "Cancel": function() { + $(this).dialog("close"); + } + } + }); + + $('#replace-image-dialog form').submit(function() { + $('#replace-image-dialog').dialog('close'); + }); + + $("#replace-image").click(function(e) { + e.preventDefault(); + $("#replace-image-dialog").dialog("open"); + }); + }; })(); $(document).ready(function() { diff --git a/app/views/moderator/post/posts/_replace.html.erb b/app/views/moderator/post/posts/_replace.html.erb new file mode 100644 index 000000000..7b43d6e0c --- /dev/null +++ b/app/views/moderator/post/posts/_replace.html.erb @@ -0,0 +1,11 @@ +<%= simple_form_for(@post, url: replace_moderator_post_post_path, method: :post) do |f| %> +

Replace Image

+ +

+ Delete the current image and replace it with another one, keeping + everything else in the post intact. This is meant for upgrading + lower-quality images, such as image samples, to higher-quality versions. +

+ + <%= f.input :source, label: "New Source", input_html: { value: "" } %> +<% end %> diff --git a/app/views/moderator/post/posts/replace.html.erb b/app/views/moderator/post/posts/replace.html.erb new file mode 100644 index 000000000..3ef8b5d68 --- /dev/null +++ b/app/views/moderator/post/posts/replace.html.erb @@ -0,0 +1,5 @@ +
+
+ <%= render "moderator/post/posts/replace" %> +
+
diff --git a/app/views/posts/partials/show/_options.html.erb b/app/views/posts/partials/show/_options.html.erb index 8776640a9..b047cde99 100644 --- a/app/views/posts/partials/show/_options.html.erb +++ b/app/views/posts/partials/show/_options.html.erb @@ -55,6 +55,8 @@
  • <%= link_to "Expunge", expunge_moderator_post_post_path(:post_id => post.id), :remote => true, :method => :post, :id => "expunge", :data => {:confirm => "This will permanently delete this post (meaning the file will be deleted). Are you sure you want to delete this post?"} %>
    • <% end %> +
  • <%= link_to "Replace Image", replace_moderator_post_post_path(:post_id => post.id), :id => "replace-image" %>
    • +
  • <%= link_to "Mobile version", mobile_post_path(post) %>
    • <% end %> <% end %> diff --git a/app/views/posts/show.html.erb b/app/views/posts/show.html.erb index 07e89ba36..d121e60b9 100644 --- a/app/views/posts/show.html.erb +++ b/app/views/posts/show.html.erb @@ -121,6 +121,10 @@ <%= render "post_appeals/new", post_appeal: @post.appeals.new %> + + From 5f2219a005feb5db8ced295ef9bf513312428e27 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 2 May 2017 20:11:47 -0500 Subject: [PATCH 7/9] post replacement: add tests. --- test/unit/post_test.rb | 84 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index ceb50527d..68f45764a 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -1588,6 +1588,90 @@ class PostTest < ActiveSupport::TestCase end end + context "Replacing: " do + setup do + Delayed::Worker.delay_jobs = true # don't delete the old images right away + Danbooru.config.stubs(:use_s3_proxy?).returns(false) # don't fail on post ids < 10000 + + @uploader = FactoryGirl.create(:user, created_at: 2.weeks.ago, can_upload_free: true) + @replacer = FactoryGirl.create(:user, created_at: 2.weeks.ago, can_approve_posts: true) + CurrentUser.user = @replacer + CurrentUser.ip_addr = "127.0.0.1" + + CurrentUser.scoped(@uploader, "127.0.0.2") do + upload = FactoryGirl.create(:jpg_upload, as_pending: "0") + upload.process! + @post = upload.post + end + end + + teardown do + Delayed::Worker.delay_jobs = false + end + + context "replacing a post from a generic source" do + setup do + @post.replace!("https://www.google.com/intl/en_ALL/images/logo.gif") + @upload = Upload.last + @mod_action = ModAction.last + end + + should "correctly update the attributes" do + assert_equal(@post.id, @upload.post.id) + assert_equal("completed", @upload.status) + + assert_equal(276, @post.image_width) + assert_equal(110, @post.image_height) + assert_equal(8558, @post.file_size) + assert_equal("gif", @post.file_ext) + assert_equal("e80d1c59a673f560785784fb1ac10959", @post.md5) + assert_equal("e80d1c59a673f560785784fb1ac10959", Digest::MD5.file(@post.file_path).hexdigest) + assert_equal("https://www.google.com/intl/en_ALL/images/logo.gif", @post.source) + end + + should "not change the post status or uploader" do + assert_equal("127.0.0.2", @post.uploader_ip_addr.to_s) + assert_equal(@uploader.id, @post.uploader_id) + assert_equal(false, @post.is_pending) + end + + should "log a mod action" do + assert_match(/replaced post ##{@post.id}/, @mod_action.description) + end + end + + context "replacing a post with a pixiv html source" do + should "replace with the full size image" do + @post.replace!("https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") + + 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_path).hexdigest) + assert_equal("https://i.pximg.net/img-original/img/2017/04/04/08/54/15/62247350_p0.png", @post.source) + end + + should "delete the old files after three days" do + old_file_path, old_preview_file_path, old_large_file_path = @post.file_path, @post.preview_file_path, @post.large_file_path + @post.replace!("https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") + + assert(File.exists?(old_file_path)) + assert(File.exists?(old_preview_file_path)) + assert(File.exists?(old_large_file_path)) + + Timecop.travel(Time.now + Post::DELETION_GRACE_PERIOD + 1.day) do + Delayed::Worker.new.work_off + end + + assert_not(File.exists?(old_file_path)) + assert_not(File.exists?(old_preview_file_path)) + assert_not(File.exists?(old_large_file_path)) + end + end + end + context "Searching:" do setup do mock_pool_archive_service! From 038e40ec98cbd14e7dc1672ba366389f5a360247 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 4 May 2017 12:30:27 -0500 Subject: [PATCH 8/9] post replacement: increase grace period to 30 days. --- app/models/post.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/post.rb b/app/models/post.rb index 71135d769..deb29b043 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -7,7 +7,7 @@ class Post < ActiveRecord::Base class RevertError < Exception ; end class SearchError < Exception ; end - DELETION_GRACE_PERIOD = 3.days + DELETION_GRACE_PERIOD = 30.days before_validation :initialize_uploader, :on => :create before_validation :merge_old_changes From d40da8c5c94049342debbd3003d3037fcb9c72b8 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 5 May 2017 16:10:43 -0500 Subject: [PATCH 9/9] post replacement: leave a system comment after replacement. --- app/models/post.rb | 9 ++---- app/presenters/post_presenter.rb | 49 ++++++++++++++++++++++++++++++++ test/unit/post_test.rb | 11 +++++++ 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index deb29b043..7bb198d09 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1402,7 +1402,7 @@ class Post < ActiveRecord::Base ModAction.log("undeleted post ##{id}") end - def replace!(url) + def replace!(url, replacer = CurrentUser.user) # TODO for posts with notes we need to rescale the notes if the dimensions change. if notes.size > 0 raise NotImplementedError.new("Replacing images with notes not yet supported.") @@ -1436,11 +1436,8 @@ class Post < ActiveRecord::Base self.source = upload.source self.tag_string = upload.tag_string - ModAction.log(<<-EOS.strip_heredoc) - replaced post ##{id}: #{image_width_was}x#{image_height_was} (#{file_size_was.to_formatted_s(:human_size)} #{file_ext_was.upcase}) -> #{image_width}x#{image_height} (#{file_size.to_formatted_s(:human_size)} #{file_ext.upcase}) - source: #{source_was} -> #{source} - md5: "#{md5_was}":[/data/#{md5_was}.#{file_ext_was}] -> "#{md5}":[/data/#{md5}.#{file_ext}] - EOS + comments.create!({creator: User.system, body: presenter.comment_replacement_message(replacer), do_not_bump_post: true}, without_protection: true) + ModAction.log(presenter.modaction_replacement_message) save! end diff --git a/app/presenters/post_presenter.rb b/app/presenters/post_presenter.rb index eaee5e70e..69d74bb33 100644 --- a/app/presenters/post_presenter.rb +++ b/app/presenters/post_presenter.rb @@ -279,4 +279,53 @@ class PostPresenter < Presenter pool_html << "" pool_html end + + def comment_replacement_message(replacer = CurrentUser.user) + "@#{replacer.name} replaced this post with a new image:\n\n#{replacement_message}" + end + + def modaction_replacement_message + "replaced post ##{@post.id}:\n\n#{replacement_message}" + end + + def replacement_message + linked_source = linked_source(@post.source) + 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 + +protected + + 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 end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 68f45764a..13939b714 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -1593,6 +1593,9 @@ class PostTest < ActiveSupport::TestCase Delayed::Worker.delay_jobs = true # don't delete the old images right away Danbooru.config.stubs(:use_s3_proxy?).returns(false) # don't fail on post ids < 10000 + @system = FactoryGirl.create(:user, created_at: 2.weeks.ago) + Danbooru.config.stubs(:system_user).returns(@system) + @uploader = FactoryGirl.create(:user, created_at: 2.weeks.ago, can_upload_free: true) @replacer = FactoryGirl.create(:user, created_at: 2.weeks.ago, can_approve_posts: true) CurrentUser.user = @replacer @@ -1638,6 +1641,14 @@ class PostTest < ActiveSupport::TestCase should "log a mod action" do assert_match(/replaced post ##{@post.id}/, @mod_action.description) end + + should "leave a system comment" do + comment = @post.comments.last + + assert_not_nil(comment) + assert_equal(User.system.id, comment.creator_id) + assert_match(/@#{@replacer.name} replaced this post/, comment.body) + end end context "replacing a post with a pixiv html source" do