From a5f589f9e0b574f052fe95c0484b5521d567e99f Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 5 Nov 2021 05:46:50 -0500 Subject: [PATCH] aliases/implications: change automatic retirement rules. Change the rules for automatically retiring aliases and implications: * Retire aliases to tags that are empty, or that are for a general or artist tag that hasn't received any new posts in the last two years. * Retire implications from tags that are empty. * Don't retire aliases or implications for character, copyright, or meta tags any more, unless the tags are empty. --- .../tag_relationship_retirement_service.rb | 47 ++++---- app/models/tag_alias.rb | 2 + app/models/tag_implication.rb | 2 + app/models/tag_relationship.rb | 10 +- .../jobs/retire_tag_relationships_job_test.rb | 105 ++++++++++++++++++ ...ag_relationship_retirement_service_test.rb | 62 ----------- 6 files changed, 136 insertions(+), 92 deletions(-) create mode 100644 test/jobs/retire_tag_relationships_job_test.rb delete mode 100644 test/unit/tag_relationship_retirement_service_test.rb diff --git a/app/logical/tag_relationship_retirement_service.rb b/app/logical/tag_relationship_retirement_service.rb index 4d7917a5d8e..d088ced37e5 100644 --- a/app/logical/tag_relationship_retirement_service.rb +++ b/app/logical/tag_relationship_retirement_service.rb @@ -1,6 +1,9 @@ -# Removes tag aliases and implications if they haven't had any new uploads in -# the last two years. Runs weekly. Posts a message to the forum when aliases or -# implications are retired. +# Removes inactive aliases and implications. 'Inactive' means aliases to empty +# tags, implications from empty tags, and gentag and artist aliases that +# haven't had any new posts in the last two years. +# +# Runs weekly. Posts a message to the forum when aliases or implications are +# retired. # # @see DanbooruMaintenance#weekly module TagRelationshipRetirementService @@ -8,20 +11,15 @@ module TagRelationshipRetirementService THRESHOLD = 2.years - def forum_topic_title - "Retired tag aliases & implications" - end - - def forum_topic_body - "This topic deals with tag relationships created two or more years ago that have not been used since. They will be retired. This topic will be updated as an automated system retires expired relationships." - end + FORUM_TOPIC_TITLE = "Retired tag aliases & implications" + FORUM_TOPIC_BODY = "This topic deals with tag relationships created two or more years ago that have not been used since. They will be retired. This topic will be updated as an automated system retires expired relationships." def forum_topic - topic = ForumTopic.where(title: forum_topic_title).first + topic = ForumTopic.where(title: FORUM_TOPIC_TITLE).first if topic.nil? CurrentUser.scoped(User.system) do - topic = ForumTopic.create!(creator: User.system, title: forum_topic_title, category_id: 1) - ForumPost.create!(creator: User.system, body: forum_topic_body, topic: topic) + topic = ForumTopic.create!(creator: User.system, title: FORUM_TOPIC_TITLE, category_id: 1) + ForumPost.create!(creator: User.system, body: FORUM_TOPIC_BODY, topic: topic) end end topic @@ -30,26 +28,23 @@ def forum_topic def find_and_retire! messages = [] - [TagAlias, TagImplication].each do |model| - each_candidate(model) do |rel| - rel.update(status: "retired") - messages << rel.retirement_message - end + inactive_relationships.each do |rel| + rel.update!(status: "retired") + messages << "The #{rel.relationship} [[#{rel.antecedent_name}]] -> [[#{rel.consequent_name}]] has been retired." end updater = ForumUpdater.new(forum_topic) updater.update(messages.sort.join("\n")) end - def each_candidate(model) - model.active.where("created_at < ?", THRESHOLD.ago).find_each do |rel| - if is_unused?(rel.consequent_name) - yield(rel) - end - end + def inactive_relationships + (inactive_aliases + TagAlias.active.empty + TagImplication.active.empty).uniq end - def is_unused?(name) - !Post.raw_tag_match(name).exists?(["created_at > ?", THRESHOLD.ago]) + def inactive_aliases + aliases = TagAlias.general.or(TagAlias.artist).active.where("tag_aliases.created_at < ?", THRESHOLD.ago) + aliases.select do |tag_alias| + !tag_alias.consequent_tag.posts.exists?(["created_at > ?", THRESHOLD.ago]) + end end end diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index e0eb31fafc2..5b76473bb29 100644 --- a/app/models/tag_alias.rb +++ b/app/models/tag_alias.rb @@ -6,6 +6,8 @@ class TagAlias < TagRelationship before_create :delete_conflicting_relationships + scope :empty, -> { joins(:consequent_tag).merge(Tag.empty) } + def self.to_aliased(names) names = Array(names).map(&:to_s) return [] if names.empty? diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index d1062e92788..d8892cd2bc6 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -15,6 +15,8 @@ class TagImplication < TagRelationship validate :meets_tag_size_requirements, on: :request validate :has_wiki_page, on: :request + scope :empty, -> { joins(:antecedent_tag).merge(Tag.empty) } + concerning :HierarchyMethods do class_methods do def ancestors_of(names) diff --git a/app/models/tag_relationship.rb b/app/models/tag_relationship.rb index 58dfd17e494..f1124cae366 100644 --- a/app/models/tag_relationship.rb +++ b/app/models/tag_relationship.rb @@ -16,6 +16,12 @@ class TagRelationship < ApplicationRecord scope :deleted, -> {where(status: "deleted")} scope :retired, -> {where(status: "retired")} + # TagAlias.artist, TagAlias.general, TagAlias.character, TagAlias.copyright, TagAlias.meta + # TagImplication.artist, TagImplication.general, TagImplication.character, TagImplication.copyright, TagImplication.meta + TagCategory.categories.each do |category| + scope category, -> { joins(:consequent_tag).where(consequent_tag: { category: TagCategory.mapping[category] }) } + end + before_validation :normalize_names validates :status, inclusion: { in: STATUSES } validates :antecedent_name, presence: true @@ -103,10 +109,6 @@ def relationship # "TagAlias" -> "tag alias", "TagImplication" -> "tag implication" self.class.name.underscore.tr("_", " ") end - - def retirement_message - "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] has been retired." - end end def antecedent_and_consequent_are_different diff --git a/test/jobs/retire_tag_relationships_job_test.rb b/test/jobs/retire_tag_relationships_job_test.rb new file mode 100644 index 00000000000..c9b5ab4903c --- /dev/null +++ b/test/jobs/retire_tag_relationships_job_test.rb @@ -0,0 +1,105 @@ +require 'test_helper' + +class RetireTagRelationshipsJobTest < ActiveJob::TestCase + context "RetireTagRelationshipsJob" do + should "create a new forum topic if one doesn't already exist" do + create(:tag_alias, created_at: 3.years.ago, antecedent_name: "0", consequent_name: "1") + RetireTagRelationshipsJob.perform_now + + assert_equal(true, ForumTopic.exists?(title: TagRelationshipRetirementService::FORUM_TOPIC_TITLE)) + assert_equal(true, ForumPost.exists?(body: TagRelationshipRetirementService::FORUM_TOPIC_BODY)) + end + + should "retire inactive gentag and artist aliases" do + ta0 = create(:tag_alias, created_at: 3.years.ago, antecedent_name: "0", consequent_name: "artist") + ta1 = create(:tag_alias, created_at: 3.years.ago, antecedent_name: "1", consequent_name: "general") + ta2 = create(:tag_alias, created_at: 3.years.ago, antecedent_name: "2", consequent_name: "character") + ta3 = create(:tag_alias, created_at: 3.years.ago, antecedent_name: "3", consequent_name: "copyright") + ta4 = create(:tag_alias, created_at: 3.years.ago, antecedent_name: "4", consequent_name: "meta") + + as(User.system) do + create(:post, created_at: 3.years.ago, tag_string: "art:artist") + create(:post, created_at: 3.years.ago, tag_string: "gen:general") + create(:post, created_at: 3.years.ago, tag_string: "char:character") + create(:post, created_at: 3.years.ago, tag_string: "copy:copyright") + create(:post, created_at: 3.years.ago, tag_string: "meta:meta") + end + + RetireTagRelationshipsJob.perform_now + + assert_equal(true, ta0.reload.is_retired?) + assert_equal(true, ta1.reload.is_retired?) + assert_equal(false, ta2.reload.is_retired?) + assert_equal(false, ta3.reload.is_retired?) + assert_equal(false, ta4.reload.is_retired?) + end + + should "not retire old aliases with recent posts" do + ta0 = create(:tag_alias, created_at: 3.years.ago, antecedent_name: "0", consequent_name: "artist") + ta1 = create(:tag_alias, created_at: 3.years.ago, antecedent_name: "1", consequent_name: "general") + ta2 = create(:tag_alias, created_at: 3.years.ago, antecedent_name: "2", consequent_name: "character") + ta3 = create(:tag_alias, created_at: 3.years.ago, antecedent_name: "3", consequent_name: "copyright") + ta4 = create(:tag_alias, created_at: 3.years.ago, antecedent_name: "4", consequent_name: "meta") + + as(User.system) do + create(:post, created_at: 1.week.ago, tag_string: "art:artist") + create(:post, created_at: 1.week.ago, tag_string: "gen:general") + create(:post, created_at: 1.week.ago, tag_string: "char:character") + create(:post, created_at: 1.week.ago, tag_string: "copy:copyright") + create(:post, created_at: 1.week.ago, tag_string: "meta:meta") + end + + RetireTagRelationshipsJob.perform_now + + assert_equal(false, ta0.reload.is_retired?) + assert_equal(false, ta1.reload.is_retired?) + assert_equal(false, ta2.reload.is_retired?) + assert_equal(false, ta3.reload.is_retired?) + assert_equal(false, ta4.reload.is_retired?) + end + + should "retire empty aliases" do + create(:tag, name: "artist", post_count: 0, category: Tag.categories.artist) + create(:tag, name: "general", post_count: 0, category: Tag.categories.general) + create(:tag, name: "character", post_count: 0, category: Tag.categories.character) + create(:tag, name: "copyright", post_count: 0, category: Tag.categories.copyright) + create(:tag, name: "meta", post_count: 0, category: Tag.categories.meta) + + ta0 = create(:tag_alias, antecedent_name: "0", consequent_name: "artist") + ta1 = create(:tag_alias, antecedent_name: "1", consequent_name: "general") + ta2 = create(:tag_alias, antecedent_name: "2", consequent_name: "character") + ta3 = create(:tag_alias, antecedent_name: "3", consequent_name: "copyright") + ta4 = create(:tag_alias, antecedent_name: "4", consequent_name: "meta") + + RetireTagRelationshipsJob.perform_now + + assert_equal(true, ta0.reload.is_retired?) + assert_equal(true, ta1.reload.is_retired?) + assert_equal(true, ta2.reload.is_retired?) + assert_equal(true, ta3.reload.is_retired?) + assert_equal(true, ta4.reload.is_retired?) + end + + should "retire empty implications" do + create(:tag, name: "artist", post_count: 0, category: Tag.categories.artist) + create(:tag, name: "general", post_count: 0, category: Tag.categories.general) + create(:tag, name: "character", post_count: 0, category: Tag.categories.character) + create(:tag, name: "copyright", post_count: 0, category: Tag.categories.copyright) + create(:tag, name: "meta", post_count: 0, category: Tag.categories.meta) + + ta0 = create(:tag_implication, antecedent_name: "artist", consequent_name: "1") + ta1 = create(:tag_implication, antecedent_name: "general", consequent_name: "1") + ta2 = create(:tag_implication, antecedent_name: "character", consequent_name: "1") + ta3 = create(:tag_implication, antecedent_name: "copyright", consequent_name: "1") + ta4 = create(:tag_implication, antecedent_name: "meta", consequent_name: "1") + + RetireTagRelationshipsJob.perform_now + + assert_equal(true, ta0.reload.is_retired?) + assert_equal(true, ta1.reload.is_retired?) + assert_equal(true, ta2.reload.is_retired?) + assert_equal(true, ta3.reload.is_retired?) + assert_equal(true, ta4.reload.is_retired?) + end + end +end diff --git a/test/unit/tag_relationship_retirement_service_test.rb b/test/unit/tag_relationship_retirement_service_test.rb deleted file mode 100644 index a8c9432be72..00000000000 --- a/test/unit/tag_relationship_retirement_service_test.rb +++ /dev/null @@ -1,62 +0,0 @@ -require 'test_helper' - -class TagRelationshipRetirementServiceTest < ActiveSupport::TestCase - context ".forum_topic" do - subject { TagRelationshipRetirementService } - - should "create a new topic if one doesn't already exist" do - assert_difference(-> { ForumTopic.count }) do - subject.forum_topic - end - end - - should "create a new post if one doesn't already exist" do - assert_difference(-> { ForumPost.count }) do - subject.forum_topic - end - end - end - - context ".each_candidate" do - subject { TagRelationshipRetirementService } - - setup do - subject.stubs(:is_unused?).returns(true) - - @new_alias = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb") - @old_alias = create(:tag_alias, antecedent_name: "ccc", consequent_name: "ddd", created_at: 3.years.ago) - end - - should "find old tag relationships" do - subject.each_candidate(TagAlias) do |rel| - assert_equal(@old_alias, rel) - end - end - - should "not find new tag relationships" do - subject.each_candidate(TagAlias) do |rel| - assert_not_equal(@new_alias, rel) - end - end - end - - context ".is_unused?" do - subject { TagRelationshipRetirementService } - - setup do - @new_alias = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb") - @new_post = create(:post, tag_string: "bbb") - - @old_alias = create(:tag_alias, antecedent_name: "ccc", consequent_name: "ddd", created_at: 3.years.ago) - @old_post = create(:post, tag_string: "ddd", created_at: 3.years.ago) - end - - should "return true if no recent post exists" do - assert(subject.is_unused?("ddd")) - end - - should "return false if a recent post exists" do - refute(subject.is_unused?("bbb")) - end - end -end