diff --git a/app/logical/tag_relationship_retirement_service.rb b/app/logical/tag_relationship_retirement_service.rb index 4d7917a5d..d088ced37 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 @@ module TagRelationshipRetirementService 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 e0eb31faf..5b76473bb 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 d1062e927..d8892cd2b 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 58dfd17e4..f1124cae3 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 @@ class TagRelationship < ApplicationRecord # "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 000000000..c9b5ab490 --- /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 a8c9432be..000000000 --- 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