diff --git a/Gemfile b/Gemfile index 48c3c5269..55086cc65 100644 --- a/Gemfile +++ b/Gemfile @@ -59,6 +59,7 @@ gem 'google-api-client' gem 'cityhash' gem 'bigquery', :git => "https://github.com/abronte/BigQuery.git", :ref => "b92b4e0b54574e3fde7ad910f39a67538ed387ad" gem 'memcache_mock' +gem 'memoist' # needed for looser jpeg header compat gem 'ruby-imagespec', :require => "image_spec", :git => "https://github.com/r888888888/ruby-imagespec.git", :branch => "exif-fixes" diff --git a/Gemfile.lock b/Gemfile.lock index 53b147376..c53f268dc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -405,6 +405,7 @@ DEPENDENCIES mechanize memcache-client memcache_mock + memoist mocha net-sftp net-ssh diff --git a/app/controllers/post_versions_controller.rb b/app/controllers/post_versions_controller.rb index af484f500..3b955d283 100644 --- a/app/controllers/post_versions_controller.rb +++ b/app/controllers/post_versions_controller.rb @@ -3,7 +3,7 @@ class PostVersionsController < ApplicationController rescue_from ActiveRecord::StatementInvalid, :with => :rescue_exception def index - @post_versions = PostVersion.search(params[:search]).order("updated_at desc, id desc").paginate(params[:page], :limit => params[:limit], :search_count => params[:search]) + @post_versions = PostArchive.search(params[:search]).order("updated_at desc, id desc").paginate(params[:page], :limit => params[:limit], :search_count => params[:search]) respond_with(@post_versions) do |format| format.xml do render :xml => @post_versions.to_xml(:root => "post-versions") @@ -15,7 +15,7 @@ class PostVersionsController < ApplicationController end def undo - @post_version = PostVersion.find(params[:id]) + @post_version = PostArchive.find(params[:id]) if @post_version.post.visible? @post_version.undo! diff --git a/app/logical/bulk_revert.rb b/app/logical/bulk_revert.rb index e95ea806a..b5929ab2c 100644 --- a/app/logical/bulk_revert.rb +++ b/app/logical/bulk_revert.rb @@ -27,7 +27,7 @@ class BulkRevert end def find_post_versions - q = PostVersion.where("true") + q = PostArchive.where("true") if constraints[:user_name] constraints[:user_id] = User.find_by_name(constraints[:user_name]).try(:id) diff --git a/app/logical/moderator/ip_addr_search.rb b/app/logical/moderator/ip_addr_search.rb index eccbf11ab..c47216c04 100644 --- a/app/logical/moderator/ip_addr_search.rb +++ b/app/logical/moderator/ip_addr_search.rb @@ -34,7 +34,7 @@ module Moderator add_row(sums, Hash[User.where(last_ip_addr: ip_addrs).collect { |user| [user, 1] }]) add_row_id(sums, PoolArchive.where(updater_ip_addr: ip_addrs).group(:updater_id).count) if PoolArchive.enabled? - add_row_id(sums, PostVersion.where(updater_ip_addr: ip_addrs).group(:updater_id).count) + add_row_id(sums, PostArchive.where(updater_ip_addr: ip_addrs).group(:updater_id).count) if PostArchive.enabled? sums end @@ -52,7 +52,7 @@ module Moderator add_row(sums, ArtistVersion.where(updater: users).group(:updater_ip_addr).count) add_row(sums, NoteVersion.where(updater: users).group(:updater_ip_addr).count) add_row(sums, PoolArchive.where(updater_id: users.map(&:id)).group(:updater_ip_addr).count) if PoolArchive.enabled? - add_row(sums, PostVersion.where(updater_id: users.map(&:id)).group(:updater_ip_addr).count) + add_row(sums, PostArchive.where(updater_id: users.map(&:id)).group(:updater_ip_addr).count) if PostArchive.enabled? add_row(sums, WikiPageVersion.where(updater: users).group(:updater_ip_addr).count) add_row(sums, Comment.where(creator: users).group(:ip_addr).count) add_row(sums, Dmail.where(from: users).group(:creator_ip_addr).count) diff --git a/app/logical/reports/post_versions.rb b/app/logical/reports/post_versions.rb index 9c4da0bd4..5e58c66a0 100644 --- a/app/logical/reports/post_versions.rb +++ b/app/logical/reports/post_versions.rb @@ -13,7 +13,7 @@ module Reports end def mock_version(row) - PostVersion.new.tap do |x| + PostArchive.new.tap do |x| x.id = row["f"][0]["v"] x.post_id = row["f"][1]["v"] x.updated_at = Time.at(row["f"][2]["v"].to_f) diff --git a/app/logical/user_revert.rb b/app/logical/user_revert.rb index d97f25fae..ba104d691 100644 --- a/app/logical/user_revert.rb +++ b/app/logical/user_revert.rb @@ -15,13 +15,13 @@ class UserRevert end def validate! - if PostVersion.where(updater_id: user_id).count > THRESHOLD + if PostArchive.where(updater_id: user_id).count > THRESHOLD raise TooManyChangesError.new("This user has too many changes to be reverted") end end def revert_post_changes - PostVersion.where(updater_id: user_id).find_each do |x| + PostArchive.where(updater_id: user_id).find_each do |x| x.undo! end end diff --git a/app/models/post.rb b/app/models/post.rb index 059527a73..77237f812 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -42,7 +42,6 @@ class Post < ActiveRecord::Base has_one :pixiv_ugoira_frame_data, :class_name => "PixivUgoiraFrameData", :dependent => :destroy has_many :flags, :class_name => "PostFlag", :dependent => :destroy has_many :appeals, :class_name => "PostAppeal", :dependent => :destroy - has_many :versions, lambda {order("post_versions.updated_at ASC, post_versions.id ASC")}, :class_name => "PostVersion", :dependent => :destroy has_many :votes, :class_name => "PostVote", :dependent => :destroy has_many :notes, :dependent => :destroy has_many :comments, lambda {includes(:creator, :updater).order("comments.id")}, :dependent => :destroy @@ -1414,12 +1413,16 @@ class Post < ActiveRecord::Base end module VersionMethods + def versions + if PostArchive.enabled? + PostArchive.where(post_id: id).order("updated_at ASC, id asc") + else + raise "Archive service not configured" + end + end + def create_version(force = false) if new_record? || rating_changed? || source_changed? || parent_id_changed? || tag_string_changed? || force - if merge_version? - delete_previous_version - end - create_new_version end end @@ -1431,19 +1434,7 @@ class Post < ActiveRecord::Base def create_new_version User.where(id: CurrentUser.id).update_all("post_update_count = post_update_count + 1") - CurrentUser.reload - - versions.create( - :rating => rating, - :source => source, - :tags => tag_string, - :parent_id => parent_id - ) - end - - def delete_previous_version - prev = versions.last - prev.destroy + PostArchive.queue(self) if PostArchive.enabled? end def revert_to(target) diff --git a/app/models/post_archive.rb b/app/models/post_archive.rb index 1cc456331..b9af6d72e 100644 --- a/app/models/post_archive.rb +++ b/app/models/post_archive.rb @@ -1,4 +1,6 @@ class PostArchive < ActiveRecord::Base + extend Memoist + def self.enabled? Danbooru.config.aws_sqs_archives_url.present? end @@ -6,49 +8,283 @@ class PostArchive < ActiveRecord::Base establish_connection (ENV["ARCHIVE_DATABASE_URL"] || "archive_#{Rails.env}".to_sym) if enabled? self.table_name = "post_versions" - def self.calculate_version(post_id, updated_at, version_id) - if updated_at.to_i == Time.zone.parse("2007-03-14T19:38:12Z").to_i - # Old post versions which didn't have updated_at set correctly - 1 + PostVersion.where("post_id = ? and updated_at = ? and id < ?", post_id, updated_at, version_id).count - else - 1 + PostVersion.where("post_id = ? and updated_at < ?", post_id, updated_at).count - end - end - - def self.export(version_id = 9096768) - PostVersion.where("id > ?", version_id).find_each do |version| - previous = version.previous - tags = version.tags.scan(/\S+/) - - if previous - prev_tags = previous.tags.scan(/\S+/) - added_tags = tags - previous.tags.scan(/\S+/) - removed_tags = previous.tags.scan(/\S+/) - tags + module SearchMethods + def for_user(user_id) + if user_id + where("updater_id = ?", user_id) else - added_tags = tags - removed_tags = [] + where("false") + end + end + + def for_user_name(name) + user_id = User.name_to_id(name) + for_user(user_id) + end + + def search(params) + q = where("true") + params = {} if params.blank? + + if params[:updater_name].present? + q = q.for_user_name(params[:updater_name]) end - rating_changed = previous.nil? || version.rating != previous.try(:rating) - parent_changed = previous.nil? || version.parent_id != previous.try(:parent_id) - source_changed = previous.nil? || version.source != previous.try(:source) - create( - post_id: version.post_id, - tags: version.tags, - added_tags: added_tags, - removed_tags: removed_tags, - updater_id: version.updater_id, - updater_ip_addr: version.updater_ip_addr.to_s, - updated_at: version.updated_at, - version: calculate_version(version.post_id, version.updated_at, version.id), - rating: version.rating, - rating_changed: rating_changed, - parent_id: version.parent_id, - parent_changed: parent_changed, - source: version.source, - source_changed: source_changed - ) - puts "inserted #{version.id}" + if params[:updater_id].present? + q = q.where("updater_id = ?", params[:updater_id].to_i) + end + + if params[:post_id].present? + q = q.where("post_id = ?", params[:post_id].to_i) + end + + if params[:start_id].present? + q = q.where("id <= ?", params[:start_id].to_i) + end + + q end end + + module ArchiveServiceMethods + extend ActiveSupport::Concern + + class_methods do + def sqs_service + SqsService.new(Danbooru.config.aws_sqs_archives_url) + end + + def queue(post) + # queue updates to sqs so that if archives goes down for whatever reason it won't + # block post updates + raise "Archive service is not configured" if !enabled? + + json = { + "post_id" => post.id, + "rating" => post.rating, + "parent_id" => post.parent_id, + "source" => post.source, + "updater_id" => CurrentUser.id, + "updater_ip_addr" => CurrentUser.ip_addr.to_s, + "updated_at" => post.updated_at.try(:iso8601), + "created_at" => post.created_at.try(:iso8601), + "tags" => post.tag_string + } + msg = "add post version\n#{json.to_json}" + sqs_service.send_message(msg) + end + + def export_to_archives(version_id = 1451061) + PostVersion.where("id > ?", version_id).find_each do |version| + previous = version.previous + tags = version.tags.scan(/\S+/) + version_number = if version.updated_at.to_i == Time.zone.parse("2007-03-14T19:38:12Z").to_i + # Old post versions which didn't have updated_at set correctly + 1 + PostVersion.where("post_id = ? and updated_at = ? and id < ?", version.post_id, version.updated_at, version.id).count + else + 1 + PostVersion.where("post_id = ? and updated_at < ?", version.post_id, version.updated_at).count + end + + if previous + prev_tags = previous.tags.scan(/\S+/) + added_tags = tags - prev_tags + removed_tags = prev_tags - tags + else + added_tags = tags + removed_tags = [] + end + + rating_changed = previous.nil? || version.rating != previous.rating + parent_changed = previous.nil? || version.parent_id != previous.parent_id + source_changed = previous.nil? || version.source != previous.source + create( + post_id: version.post_id, + tags: version.tags, + added_tags: added_tags, + removed_tags: removed_tags, + updater_id: version.updater_id, + updater_ip_addr: version.updater_ip_addr.to_s, + updated_at: version.updated_at, + version: version_number, + rating: version.rating, + rating_changed: rating_changed, + parent_id: version.parent_id, + parent_changed: parent_changed, + source: version.source, + source_changed: source_changed + ) + puts "inserted #{version.id}" + end + end + end + end + + extend SearchMethods + include ArchiveServiceMethods + + def tag_array + tags.scan(/\S+/) + end + + def presenter + PostVersionPresenter.new(self) + end + + def reload + flush_cache + super + end + + def post + Post.where(id: post_id).first + end + + def previous + PostArchive.where("post_id = ? and version < ?", post_id, version).order("version desc").first + end + + def updater + User.find(updater_id) + end + + def diff(version = nil) + if post.nil? + latest_tags = tag_array + else + latest_tags = post.tag_array + latest_tags << "rating:#{post.rating}" if post.rating.present? + latest_tags << "parent:#{post.parent_id}" if post.parent_id.present? + latest_tags << "source:#{post.source}" if post.source.present? + end + + new_tags = tag_array + new_tags << "rating:#{rating}" if rating.present? + new_tags << "parent:#{parent_id}" if parent_id.present? + new_tags << "source:#{source}" if source.present? + + old_tags = version.present? ? version.tag_array : [] + if version.present? + old_tags << "rating:#{version.rating}" if version.rating.present? + old_tags << "parent:#{version.parent_id}" if version.parent_id.present? + old_tags << "source:#{version.source}" if version.source.present? + end + + added_tags = new_tags - old_tags + removed_tags = old_tags - new_tags + + return { + :added_tags => added_tags, + :removed_tags => removed_tags, + :obsolete_added_tags => added_tags - latest_tags, + :obsolete_removed_tags => removed_tags & latest_tags, + :unchanged_tags => new_tags & old_tags, + } + end + + def changes + delta = { + :added_tags => added_tags, + :removed_tags => removed_tags + } + + latest_tags = post.tag_array + latest_tags << "rating:#{post.rating}" if post.rating.present? + latest_tags << "parent:#{post.parent_id}" if post.parent_id.present? + latest_tags << "source:#{post.source}" if post.source.present? + + if parent_changed? + delta[:added_tags] << "parent:#{parent_id}" + + if previous + delta[:removed_tags] << "parent:#{previous.parent_id}" + end + end + + if rating_changed? + delta[:added_tags] << "rating:#{rating}" + + if previous + delta[:removed_tags] << "rating:#{previous.rating}" + end + end + + if source_changed? + delta[:added_tags] << "source:#{source}" + + if previous + delta[:removed_tags] << "source:#{previous.source}" + end + end + + delta[:obsolete_added_tags] = delta[:added_tags] - latest_tags + delta[:obsolete_removed_tags] = delta[:removed_tags] & latest_tags + + if previous + delta[:unchanged_tags] = tag_array & previous.tag_array + end + + delta + end + + def added_tags_with_fields + changes[:added_tags].join(" ") + end + + def removed_tags_with_fields + changes[:removed_tags].join(" ") + end + + def obsolete_added_tags + changes[:obsolete_added_tags].join(" ") + end + + def obsolete_removed_tags + changes[:obsolete_removed_tags].join(" ") + end + + def unchanged_tags + changes[:unchanged_tags].join(" ") + end + + def truncated_source + source.gsub(/^http:\/\//, "").sub(/\/.+/, "") + end + + def undo + added = changes[:added_tags_with_fields] - changes[:obsolete_added_tags] + removed = changes[:removed_tags_with_fields] - changes[:obsolete_removed_tags] + + added.each do |tag| + if tag =~ /^source:/ + post.source = "" + elsif tag =~ /^parent:/ + post.parent_id = nil + else + escaped_tag = Regexp.escape(tag) + post.tag_string = post.tag_string.sub(/(?:\A| )#{escaped_tag}(?:\Z| )/, " ").strip + end + end + removed.each do |tag| + if tag =~ /^source:(.+)$/ + post.source = $1 + else + post.tag_string = "#{post.tag_string} #{tag}".strip + end + end + end + + def undo! + undo + post.save! + end + + def updater + User.find_by_id(updater_id) + end + + def method_attributes + super + [:added_tags, :removed_tags, :obsolete_added_tags, :obsolete_removed_tags, :unchanged_tags, :updater_name] + end + + memoize :previous, :post, :tag_array, :changes, :added_tags_with_fields, :removed_tags_with_fields, :obsolete_removed_tags, :obsolete_added_tags, :unchanged_tags end diff --git a/app/models/post_version.rb b/app/models/post_version.rb index e3c1be962..2006da241 100644 --- a/app/models/post_version.rb +++ b/app/models/post_version.rb @@ -61,17 +61,6 @@ class PostVersion < ActiveRecord::Base super end - def sequence_for_post - versions = PostVersion.where(:post_id => post_id).order("updated_at desc, id desc") - diffs = [] - versions.each_index do |i| - if i < versions.size - 1 - diffs << versions[i].diff(versions[i + 1]) - end - end - return diffs - end - def diff(version) latest_tags = post.tag_array latest_tags << "rating:#{post.rating}" if post.rating.present? diff --git a/test/functional/moderator/dashboards_controller_test.rb b/test/functional/moderator/dashboards_controller_test.rb index dbdff63fd..6259601e9 100644 --- a/test/functional/moderator/dashboards_controller_test.rb +++ b/test/functional/moderator/dashboards_controller_test.rb @@ -53,7 +53,7 @@ module Moderator end should "render" do - assert_equal(1, PostVersion.count) + assert_equal(1, PostArchive.count) get :show, {}, {:user_id => @admin.id} assert_response :success end diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index 01da6f29f..d78111855 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -144,12 +144,12 @@ class PostsControllerTest < ActionController::TestCase context "revert action" do setup do - @post.stubs(:merge_version?).returns(false) + PostArchive.sqs_service.stubs(:merge?).returns(false) @post.update_attributes(:tag_string => "zzz") end should "work" do - @version = @post.versions(true).first + @version = @post.versions.first assert_equal("aaaa", @version.tags) post :revert, {:id => @post.id, :version_id => @version.id}, {:user_id => @user.id} assert_redirected_to post_path(@post) diff --git a/test/helpers/post_archive_test_helper.rb b/test/helpers/post_archive_test_helper.rb new file mode 100644 index 000000000..7a0a19453 --- /dev/null +++ b/test/helpers/post_archive_test_helper.rb @@ -0,0 +1,45 @@ +module PostArchiveTestHelper + def setup + super + + mock_post_archive_service! + start_post_archive_transaction + end + + def teardown + super + + rollback_post_archive_transaction + end + + def mock_post_archive_service! + mock_sqs_service = Class.new do + def send_message(msg) + _, json = msg.split(/\n/) + json = JSON.parse(json) + json.delete("created_at") + json["version"] = 1 + PostArchive.where(post_id: json["post_id"]).count + prev = PostArchive.where(post_id: json["post_id"]).order("id desc").first + if merge?(prev, json) + prev.update_columns(json) + else + PostArchive.create(json) + end + end + + def merge?(prev, json) + prev && (prev.updater_id == json["updater_id"]) && (prev.updated_at >= 1.hour.ago) + end + end + + PostArchive.stubs(:sqs_service).returns(mock_sqs_service.new) + end + + def start_post_archive_transaction + PostArchive.connection.begin_transaction joinable: false + end + + def rollback_post_archive_transaction + PostArchive.connection.rollback_transaction + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index d428a5de4..75496a920 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -11,6 +11,7 @@ end require File.expand_path('../../config/environment', __FILE__) require 'rails/test_help' require 'cache' +require 'helpers/post_archive_test_helper' Dir[File.expand_path(File.dirname(__FILE__) + "/factories/*.rb")].each {|file| require file} @@ -24,6 +25,26 @@ if defined?(MEMCACHE) Object.send(:remove_const, :MEMCACHE) end +class ActiveSupport::TestCase + include UploadTestMethods + include PostArchiveTestHelper +end + +class ActionController::TestCase + include UploadTestMethods + include PostArchiveTestHelper + + def assert_authentication_passes(action, http_method, role, params, session) + __send__(http_method, action, params, session.merge(:user_id => @users[role].id)) + assert_response :success + end + + def assert_authentication_fails(action, http_method, role) + __send__(http_method, action, params, session.merge(:user_id => @users[role].id)) + assert_redirected_to(new_sessions_path) + end +end + MEMCACHE = MemcacheMock.new Delayed::Worker.delay_jobs = false diff --git a/test/unit/note_test.rb b/test/unit/note_test.rb index d2a392162..081f46fbd 100644 --- a/test/unit/note_test.rb +++ b/test/unit/note_test.rb @@ -107,11 +107,14 @@ class NoteTest < ActiveSupport::TestCase setup do @post = FactoryGirl.create(:post, :image_width => 1000, :image_height => 1000) @note = FactoryGirl.create(:note, :post => @post) + @note.stubs(:merge_version?).returns(false) end should "increment the updater's note_update_count" do - assert_difference("CurrentUser.note_update_count", 1) do + @user.reload + assert_difference("@user.note_update_count", 1) do @note.update_attributes(:body => "zzz") + @user.reload end end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 0ac525442..bfd21d421 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -1055,7 +1055,7 @@ class PostTest < ActiveSupport::TestCase context "that has been updated" do should "create a new version if it's the first version" do - assert_difference("PostVersion.count", 1) do + assert_difference("PostArchive.count", 1) do post = FactoryGirl.create(:post) end end @@ -1063,7 +1063,7 @@ class PostTest < ActiveSupport::TestCase should "create a new version if it's been over an hour since the last update" do post = FactoryGirl.create(:post) Timecop.travel(6.hours.from_now) do - assert_difference("PostVersion.count", 1) do + assert_difference("PostArchive.count", 1) do post.update_attributes(:tag_string => "zzz") end end @@ -1071,15 +1071,16 @@ class PostTest < ActiveSupport::TestCase should "merge with the previous version if the updater is the same user and it's been less than an hour" do post = FactoryGirl.create(:post) - assert_difference("PostVersion.count", 0) do + assert_difference("PostArchive.count", 0) do post.update_attributes(:tag_string => "zzz") end assert_equal("zzz", post.versions.last.tags) end should "increment the updater's post_update_count" do + PostArchive.sqs_service.stubs(:merge?).returns(false) post = FactoryGirl.create(:post, :tag_string => "aaa bbb ccc") - post.stubs(:merge_version?).returns(false) + CurrentUser.reload assert_difference("CurrentUser.post_update_count", 1) do post.update_attributes(:tag_string => "zzz") @@ -2289,8 +2290,8 @@ class PostTest < ActiveSupport::TestCase context "a post that has been updated" do setup do - @post = FactoryGirl.create(:post, :rating => "q", :tag_string => "aaa", :source => nil) - @post.stubs(:merge_version?).returns(false) + @post = FactoryGirl.create(:post, :rating => "q", :tag_string => "aaa") + PostArchive.sqs_service.stubs(:merge?).returns(false) @post.update_attributes(:tag_string => "aaa bbb ccc ddd") @post.update_attributes(:tag_string => "bbb xxx yyy", :source => "xyz") @post.update_attributes(:tag_string => "bbb mmm yyy", :source => "abc") diff --git a/test/unit/post_version_test.rb b/test/unit/post_version_test.rb index cd9f45a01..c89a64dd6 100644 --- a/test/unit/post_version_test.rb +++ b/test/unit/post_version_test.rb @@ -18,28 +18,21 @@ class PostVersionTest < ActiveSupport::TestCase context "that has multiple versions: " do setup do + PostArchive.sqs_service.stubs(:merge?).returns(false) @post = FactoryGirl.create(:post, :tag_string => "1") - @post.stubs(:merge_version?).returns(false) - @post.stubs(:tag_string_changed?).returns(true) @post.update_attributes(:tag_string => "1 2") @post.update_attributes(:tag_string => "2 3") end context "a version record" do setup do - @version = PostVersion.last + @version = PostArchive.last end should "know its previous version" do assert_not_nil(@version.previous) assert_equal("1 2", @version.previous.tags) end - - should "know the seuqence of all versions for the post" do - assert_equal(2, @version.sequence_for_post.size) - assert_equal(%w(3), @version.sequence_for_post[0][:added_tags]) - assert_equal(%w(2), @version.sequence_for_post[1][:added_tags]) - end end end @@ -75,9 +68,8 @@ class PostVersionTest < ActiveSupport::TestCase context "that has been updated" do setup do - @parent = FactoryGirl.create(:post) + PostArchive.sqs_service.stubs(:merge?).returns(false) @post = FactoryGirl.create(:post, :tag_string => "aaa bbb ccc", :rating => "q", :source => "xyz") - @post.stubs(:merge_version?).returns(false) @post.update_attributes(:tag_string => "bbb ccc xxx", :source => "") end