posts: factor out post edit logic.

Factor out most of the tag edit logic from the Post class to a new
PostEdit class. The PostEdit class contains the logic for parsing tags
and metatags from the tag edit string, and for determining which tags
were added or removed by the edit.

Fixes various bugs caused by not calculating the set of added or removed
tags correctly, for example when tag category prefixes were used (e.g.
`copy:touhou`) or when the same tag was added and removed in the same
edit (e.g. `touhou -touhou`).

Fixes #5123: Tag categorization prefixes bypass deprecation check
Fixes #5126: Negating a deprecated tag will still cause the warning to show
Fixes #3477: Remove tag validator triggering on tag category changes
Fixes #4848: newpool: metatag doesn't parse correctly

app/logical/post_edit.rb

@@ -0,0 +1,166 @@
+# frozen_string_literal: true
+
+# A PostEdit represents a tag edit being performed on a post. It contains most
+# of the logic for performing a tag edit, including methods for parsing the tag
+# string into tags and metatags, methods for determining which tags were added
+# or removed by the edit, and methods for calculating the final list of tags.
+class PostEdit
+  extend Memoist
+
+  Tag = Struct.new(:name, :negated, keyword_init: true)
+  Metatag = Struct.new(:name, :value, keyword_init: true)
+
+  # Metatags that change the tag's category: `art:bkub`, `char:chen`, `copy:touhou`, `gen:1girl`, `meta:animated`.
+  CATEGORIZATION_METATAGS = TagCategory.mapping.keys
+
+  # Pre-metatags affect the post itself, so they must be applied before the post is saved.
+  PRE_METATAGS = %w[parent -parent rating source] + CATEGORIZATION_METATAGS
+
+  # Post-metatags rely on the post's ID, so they must be applied after the post is saved to ensure the ID has been created.
+  POST_METATAGS = %w[newpool pool -pool favgroup -favgroup fav -fav child -child upvote downvote disapproved status -status]
+
+  METATAGS = PRE_METATAGS + POST_METATAGS
+  METATAG_NAME_REGEX = /(#{METATAGS.join("|")}):/io
+
+  private attr_reader :post, :current_tag_names, :old_tag_names, :new_tag_string, :parser
+  private delegate :accept, :expect, :error, :skip, :zero_or_more, :one_of, to: :parser
+
+  # @param post [Post] The post being edited.
+  # @param current_tag_string [String] The space-separated list of tags currently on the post.
+  # @param old_tag_string [String] The space-separated list of tags the user saw before the edit.
+  # @param new_tag_string [String] The space-separated list of tags after the edit.
+  def initialize(post, current_tag_string, old_tag_string, new_tag_string)
+    @post = post
+    @current_tag_names = current_tag_string.to_s.split
+    @old_tag_names = old_tag_string.to_s.split
+    @new_tag_string = new_tag_string.to_s.gsub(/[[:space:]]/, " ").strip
+    @parser = StringParser.new(@new_tag_string)
+  end
+
+  concerning :HelperMethods do
+    # @return [Array<String>] The final list of tags on the post after the edit.
+    def tag_names
+      tag_names = current_tag_names + effective_added_tag_names - user_removed_tag_names
+      tag_names = post.add_automatic_tags(tag_names)
+      tag_names = ::Tag.convert_cosplay_tags(tag_names)
+      tag_names += ::Tag.automatic_tags_for(tag_names)
+      tag_names += TagImplication.tags_implied_by(tag_names).map(&:name)
+      tag_names.uniq.sort
+    end
+
+    # @return [Array<String>] The list of tags in the edited tag string, including regular tags and tags with a category prefix (e.g. `artist:bkub`)
+    def new_tag_names
+      tag_terms.reject(&:negated).map(&:name) + tag_categorization_terms.map(&:value)
+    end
+
+    # @return [Array<Tag>] The list of tags in the edited tag string. Includes negated and non-negated tags, but not tags with a category prefix.
+    def tag_terms
+      terms.grep(Tag)
+    end
+
+    # @return [Array<Metatag>] The list of metatags in the edited tag string.
+    def metatag_terms
+      terms.grep(Metatag)
+    end
+
+    # @return [Array<Metatag>] The list of pre-save metatags in the edit (metatags that are applied before the post is saved).
+    def pre_metatag_terms
+      metatag_terms.select { |term| term.name.in?(PRE_METATAGS) }
+    end
+
+    # @return [Array<Metatag>] The list of post-save metatags in the edit (metatags that are applied after the post is saved).
+    def post_metatag_terms
+      metatag_terms.select { |term| term.name.in?(POST_METATAGS) }
+    end
+
+    # @return [Array<Metatag>] The list of tags with a category prefix (e.g. `artist:bkub`).
+    def tag_categorization_terms
+      metatag_terms.select { |term| term.name.in?(TagCategory.categories) }
+    end
+
+    # @return [Array<String>] The list of tags actually added by the user, excluding invalid or deprecated tags.
+    def effective_added_tag_names
+      user_added_tag_names - invalid_added_tags.map(&:name) - deprecated_added_tag_names
+    end
+
+    # @return [Array<String>] The list of tags the user is trying to add. Includes tags that won't
+    #   actually be added, such as invalid or deprecated tags. Does not include tags not explicitly
+    #   added by the user, such as implied or automatic tags.
+    def user_added_tag_names
+      TagAlias.to_aliased(new_tag_names - old_tag_names - user_removed_tag_names).uniq.sort
+    end
+
+    # @return [Array<String>] The list of tags the user is trying to remove. Includes tags that
+    #   won't actually be removed, such as implied tags, automatic tags, and nonexistent tags.
+    def user_removed_tag_names
+      (explicit_removed_tag_names + implicit_removed_tag_names).uniq.sort
+    end
+
+    # @return [Array<String>] The list of tags explicitly removed using the '-' operator (e.g. `-tagme`).
+    def explicit_removed_tag_names
+      TagAlias.to_aliased(tag_terms.select(&:negated).map(&:name))
+    end
+
+    # @return [Array<String>] The list of tags implicitly removed by being deleted from the tag string (e.g. `1girl tagme` => `1girl`)
+    def implicit_removed_tag_names
+      old_tag_names - new_tag_names
+    end
+
+    # @return [Array<Tag>] The list of user-added tags that have invalid names.
+    def invalid_added_tags
+      user_added_tag_names.map { |name| ::Tag.new(name: name) }.select { |tag| tag.invalid?(:name) }
+    end
+
+    # @return [Array<String>] The list of user-added tags that are deprecated.
+    def deprecated_added_tag_names
+      ::Tag.deprecated.where(name: user_added_tag_names).map(&:name)
+    end
+  end
+
+  concerning :ParserMethods do
+    # @return [Array<Tag, Metatag>] The list of tags and metatags in the edit.
+    def terms
+      zero_or_more { skip(/[[:space:]]+/); term }
+    end
+
+    private def term
+      one_of([method(:tag), method(:metatag)])
+    end
+
+    private def tag
+      negated = accept("-").present?
+      error("Invalid tag name") if accept(METATAG_NAME_REGEX)
+      name = expect(/[^[:space:]]+/)
+
+      Tag.new(name: name.downcase, negated: negated)
+    end
+
+    private def metatag
+      name = expect(METATAG_NAME_REGEX)
+      value = quoted_string
+
+      name = name.delete_suffix(":").downcase
+      name = TagCategory.short_name_mapping.fetch(name, name) # 'art:bkub' => 'artist:bkub'
+      value = value.downcase unless name.in?(["newpool", "source"])
+      value = value.gsub(/[[:space:]]/, "_") unless name == "source"
+
+      Metatag.new(name: name, value: value)
+    end
+
+    private def quoted_string
+      if accept('"')
+        string = expect(/(\\"|[^"])*/).gsub(/\\"/, '"') # handle backslash escaped quotes
+        expect('"')
+        string
+      elsif accept("'")
+        string = expect(/(\\'|[^'])*/).gsub(/\\'/, "'") # handle backslash escaped quotes
+        expect("'")
+        string
+      else
+        expect(/(\\ |[^ ])*/).gsub(/\\ /, " ") # handle backslash escaped spaces
+      end
+    end
+  end
+
+  memoize :tag_names, :new_tag_names, :user_added_tag_names, :user_removed_tag_names, :invalid_added_tags, :deprecated_added_tag_names, :terms, :tag_terms, :metatag_terms
+end

app/logical/post_query_builder.rb

@@ -574,12 +574,6 @@ class PostQueryBuilder
       end
     end
 
-    # Parse a tag edit string into a list of strings, one per search term.
-    # @return [Array<String>] the list of terms
-    def parse_tag_edit
-      split_query
-    end
-
     class_methods do
       # Parse a simple string value into a Ruby type.
       # @param string [String] the value to parse

app/logical/string_parser.rb

@@ -33,6 +33,14 @@ class StringParser
     scanner.scan(pattern)
   end
 
+  # Skip over `pattern`, returning true if it was skipped or false if it wasn't.
+  #
+  # @param pattern [Regexp, String] The pattern to match.
+  # @return [Boolean] True if the pattern was skipped, false otherwise.
+  def skip(pattern)
+    scanner.scan(pattern) != nil
+  end
+
   # Try to match `pattern`, returning the string if it matched or raising an Error if it didn't.
   #
   # @param pattern [Regexp, String] The pattern to match.

app/models/post.rb

@@ -15,20 +15,21 @@ class Post < ApplicationRecord
   normalize :source, :normalize_source
   before_validation :merge_old_changes
   before_validation :normalize_tags
-  before_validation :parse_pixiv_id
   before_validation :blank_out_nonexistent_parents
   before_validation :remove_parent_loops
   validates :md5, uniqueness: { message: ->(post, _data) { "Duplicate of post ##{Post.find_by_md5(post.md5).id}" }}, on: :create
   validates :rating, presence: { message: "not selected" }
   validates :rating, inclusion: { in: %w[s q e], message: "must be S, Q, or E" }, if: -> { rating.present? }
   validates :source, length: { maximum: 1200 }
-  validate :added_tags_are_valid
-  validate :removed_tags_are_valid
-  validate :has_artist_tag
-  validate :has_copyright_tag
-  validate :has_enough_tags
   validate :post_is_not_its_own_parent
   validate :uploader_is_not_limited, on: :create
+  before_save :apply_pre_metatags
+  before_save :parse_pixiv_id
+  before_save :added_tags_are_valid
+  before_save :removed_tags_are_valid
+  before_save :has_artist_tag
+  before_save :has_copyright_tag
+  before_save :has_enough_tags
   before_save :update_tag_post_counts
   before_save :update_tag_category_counts
   before_create :autoban
@@ -55,7 +56,7 @@ class Post < ApplicationRecord
   has_many :favorites, dependent: :destroy
   has_many :replacements, class_name: "PostReplacement", :dependent => :destroy
 
-  attr_accessor :old_tag_string, :old_parent_id, :old_source, :old_rating, :has_constraints, :disable_versioning
+  attr_accessor :old_tag_string, :old_parent_id, :old_source, :old_rating, :has_constraints, :disable_versioning, :post_edit
 
   scope :pending, -> { where(is_pending: true) }
   scope :flagged, -> { where(is_flagged: true) }
@@ -320,7 +321,7 @@ class Post < ApplicationRecord
     end
 
     def tag_array_was
-      (tag_string_in_database.presence || tag_string_before_last_save || "").split
+      tag_string_was.split
     end
 
     def tags
@@ -362,21 +363,6 @@ class Post < ApplicationRecord
     end
 
     def merge_old_changes
-      @removed_tags = []
-
-      if old_tag_string
-        # If someone else committed changes to this post before we did,
-        # then try to merge the tag changes together.
-        current_tags = tag_string_was.split
-        new_tags = PostQueryBuilder.new(tag_string).parse_tag_edit
-        old_tags = old_tag_string.split
-
-        kept_tags = current_tags & new_tags
-        @removed_tags = old_tags - kept_tags
-
-        self.tag_string = ((current_tags + new_tags) - old_tags + (current_tags & new_tags)).uniq.sort.join(" ")
-      end
-
       if old_parent_id == ""
         old_parent_id = nil
       else
@@ -396,50 +382,8 @@ class Post < ApplicationRecord
     end
 
     def normalize_tags
-      normalized_tags = PostQueryBuilder.new(tag_string).parse_tag_edit
+      @post_edit = PostEdit.new(self, tag_string_was, old_tag_string || tag_string_was, tag_string)
-      normalized_tags = apply_casesensitive_metatags(normalized_tags)
+      self.tag_string = Tag.create_for_list(post_edit.tag_names).uniq.sort.join(" ")
-      normalized_tags = normalized_tags.map(&:downcase)
-      normalized_tags = filter_metatags(normalized_tags)
-      normalized_tags = TagAlias.to_aliased(normalized_tags)
-      normalized_tags = remove_negated_tags(normalized_tags)
-      normalized_tags = add_automatic_tags(normalized_tags)
-      normalized_tags = remove_invalid_tags(normalized_tags)
-      normalized_tags = Tag.convert_cosplay_tags(normalized_tags)
-      normalized_tags += Tag.create_for_list(Tag.automatic_tags_for(normalized_tags))
-      normalized_tags += TagImplication.tags_implied_by(normalized_tags).map(&:name)
-      normalized_tags -= added_deprecated_tags
-      normalized_tags = normalized_tags.compact.uniq.sort
-      normalized_tags = Tag.create_for_list(normalized_tags)
-      self.tag_string = normalized_tags.join(" ")
-    end
-
-    def remove_invalid_tags(tag_names)
-      invalid_tags = tag_names.map { |name| Tag.new(name: name) }.select { |tag| tag.invalid?(:name) }
-
-      invalid_tags.each do |tag|
-        tag.errors.messages.each do |_attribute, messages|
-          warnings.add(:base, "Couldn't add tag: #{messages.join(';')}")
-        end
-      end
-
-      tag_names - invalid_tags.map(&:name)
-    end
-
-    def added_deprecated_tags
-      added_deprecated_tags = added_tags.select(&:is_deprecated)
-      if added_deprecated_tags.present?
-        added_deprecated_tags_list = added_deprecated_tags.map { |t| "[[#{t.name}]]" }.to_sentence
-        warnings.add(:base, "The following tags are deprecated and could not be added: #{added_deprecated_tags_list}")
-      end
-
-      added_deprecated_tags.pluck(:name)
-    end
-
-    def remove_negated_tags(tags)
-      @negated_tags, tags = tags.partition {|x| x =~ /\A-/i}
-      @negated_tags = @negated_tags.map {|x| x[1..-1]}
-      @negated_tags = TagAlias.to_aliased(@negated_tags)
-      tags - @negated_tags
     end
 
     def add_automatic_tags(tags)
@@ -499,126 +443,87 @@ class Post < ApplicationRecord
       tags
     end
 
-    def apply_casesensitive_metatags(tags)
-      casesensitive_metatags, tags = tags.partition {|x| x =~ /\A(?:source):/i}
-      # Reuse the following metatags after the post has been saved
-      casesensitive_metatags += tags.select {|x| x =~ /\A(?:newpool):/i}
-      if !casesensitive_metatags.empty?
-        case casesensitive_metatags[-1]
-        when /^source:none$/i
-          self.source = ""
-
-        when /^source:"(.*)"$/i
-          self.source = $1
-
-        when /^source:(.*)$/i
-          self.source = $1
-
-        when /^newpool:(.+)$/i
-          pool = Pool.find_by_name($1)
-          if pool.nil?
-            Pool.create(name: $1, description: "This pool was automatically generated")
-          end
-        end
-      end
-
-      tags
-    end
-
-    def filter_metatags(tags)
-      @pre_metatags, tags = tags.partition {|x| x =~ /\A(?:rating|parent|-parent):/i}
-      tags = apply_categorization_metatags(tags)
-      @post_metatags, tags = tags.partition {|x| x =~ /\A(?:-pool|pool|newpool|fav|-fav|child|-child|-favgroup|favgroup|upvote|downvote|status|-status|disapproved):/i}
-      apply_pre_metatags
-      tags
-    end
-
-    def apply_categorization_metatags(tags)
-      tags.map do |x|
-        if x =~ Tag.categories.regexp
-          tag = Tag.find_or_create_by_name(x)
-          tag.name
-        else
-          x
-        end
-      end
-    end
-
     def apply_post_metatags
-      return unless @post_metatags
+      post_edit.post_metatag_terms.each do |metatag|
-
+        case [metatag.name, metatag.value]
-      @post_metatags.each do |tag|
+        in "-pool", /^\d+$/ => pool_id
-        case tag
+          pool = Pool.find_by_id(pool_id)
-        when /^-pool:(\d+)$/i
-          pool = Pool.find_by_id($1.to_i)
           pool&.remove!(self)
 
-        when /^-pool:(.+)$/i
+        in "-pool", name
-          pool = Pool.find_by_name($1)
+          pool = Pool.find_by_name(name)
           pool&.remove!(self)
 
-        when /^pool:(\d+)$/i
+        in "pool", /^\d+$/ => pool_id
-          pool = Pool.find_by_id($1.to_i)
+          pool = Pool.find_by_id(pool_id)
           pool&.add!(self)
 
-        when /^pool:(.+)$/i
+        in "pool", name
-          pool = Pool.find_by_name($1)
+          pool = Pool.find_by_name(name)
           pool&.add!(self)
 
-        when /^newpool:(.+)$/i
+        in "newpool", name
-          pool = Pool.find_by_name($1)
+          pool = Pool.find_by_name(name)
-          pool&.add!(self)
 
-        when /^fav:(.+)$/i
+          # XXX race condition
+          if pool.nil?
+            Pool.create!(name: name, description: "This pool was automatically generated", post_ids: [id])
+          else
+            pool.add!(self)
+          end
+
+        in "fav", name
           raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, Favorite).create?
           Favorite.create(post: self, user: CurrentUser.user)
 
-        when /^-fav:(.+)$/i
+        in "-fav", name
           raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, Favorite).create?
           Favorite.destroy_by(post: self, user: CurrentUser.user)
 
-        when /^(up|down)vote:(.+)$/i
+        in "upvote", name
-          score = ($1 == "up" ? 1 : -1)
+          vote!(1, CurrentUser.user)
-          vote!(score, CurrentUser.user)
 
-        when /^status:active$/i
+        in "downvote", name
+          vote!(-1, CurrentUser.user)
+
+        in "status", "active"
           raise User::PrivilegeError unless CurrentUser.is_approver?
           approvals.create!(user: CurrentUser.user)
 
-        when /^status:banned$/i
+        in "status", "banned"
           raise User::PrivilegeError unless CurrentUser.is_approver?
           ban!
 
-        when /^-status:banned$/i
+        in "-status", "banned"
           raise User::PrivilegeError unless CurrentUser.is_approver?
           unban!
 
-        when /^disapproved:(.+)$/i
+        in "disapproved", reason
           raise User::PrivilegeError unless CurrentUser.is_approver?
-          disapprovals.create!(user: CurrentUser.user, reason: $1.downcase)
+          disapprovals.create!(user: CurrentUser.user, reason: reason.downcase)
 
-        when /^child:none$/i
+        in "child", "none"
           children.each do |post|
             post.update!(parent_id: nil)
           end
 
-        when /^-child:(.+)$/i
+        in "-child", ids
-          children.search(id: $1).each do |post|
+          children.search(id: ids).each do |post|
             post.update!(parent_id: nil)
           end
 
-        when /^child:(.+)$/i
+        in "child", ids
-          Post.search(id: $1).where.not(id: id).limit(10).each do |post|
+          Post.search(id: ids).where.not(id: id).limit(10).each do |post|
             post.update!(parent_id: id)
           end
 
-        when /^-favgroup:(.+)$/i
+        in "-favgroup", name
-          favgroup = FavoriteGroup.find_by_name_or_id!($1, CurrentUser.user)
+          favgroup = FavoriteGroup.find_by_name_or_id!(name, CurrentUser.user)
           raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, favgroup).update?
           favgroup&.remove!(self)
 
-        when /^favgroup:(.+)$/i
+        in "favgroup", name
-          favgroup = FavoriteGroup.find_by_name_or_id!($1, CurrentUser.user)
+          favgroup = FavoriteGroup.find_by_name_or_id!(name, CurrentUser.user)
           raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, favgroup).update?
           favgroup&.add!(self)
 
@@ -627,26 +532,36 @@ class Post < ApplicationRecord
     end
 
     def apply_pre_metatags
-      return unless @pre_metatags
+      post_edit.pre_metatag_terms.each do |metatag|
-
+        case [metatag.name, metatag.value]
-      @pre_metatags.each do |tag|
+        in "parent", ("none" | "0")
-        case tag
-        when /^parent:none$/i, /^parent:0$/i
           self.parent_id = nil
 
-        when /^-parent:(\d+)$/i
+        in "-parent", /^\d+$/ => new_parent_id
-          if parent_id == $1.to_i
+          if parent_id == new_parent_id.to_i
             self.parent_id = nil
           end
 
-        when /^parent:(\d+)$/i
+        in "parent", /^\d+$/ => new_parent_id
-          if $1.to_i != id && Post.exists?(["id = ?", $1.to_i])
+          if new_parent_id.to_i != id && Post.exists?(new_parent_id)
-            self.parent_id = $1.to_i
+            self.parent_id = new_parent_id.to_i
             remove_parent_loops
           end
 
-        when /^rating:([qse])/i
+        in "rating", /\A([qse])/i
-          self.rating = $1
+          self.rating = $1.downcase
+
+        in "source", "none"
+          self.source = ""
+
+        in "source", value
+          self.source = value
+
+        in category, name if category.in?(PostEdit::CATEGORIZATION_METATAGS)
+          Tag.find_or_create_by_name("#{category}:#{name}", creator: CurrentUser.user)
+
+        else
+          nil
 
         end
       end
@@ -1514,10 +1429,22 @@ class Post < ApplicationRecord
           warnings.add(:base, "Artist [[#{tag.name}]] requires an artist entry. \"Create new artist entry\":[#{new_artist_path}]")
         end
       end
+
+      post_edit.invalid_added_tags.each do |tag|
+        tag.errors.messages.each do |_attribute, messages|
+          warnings.add(:base, "Couldn't add tag: #{messages.join(';')}")
+        end
+      end
+
+      deprecated_tags = post_edit.deprecated_added_tag_names
+      if deprecated_tags.present?
+        tag_list = deprecated_tags.map { |tag| "[[#{tag}]]" }.to_sentence
+        warnings.add(:base, "The following tags are deprecated and could not be added: #{tag_list}")
+      end
     end
 
     def removed_tags_are_valid
-      attempted_removed_tags = @removed_tags + @negated_tags
+      attempted_removed_tags = post_edit.user_removed_tag_names
       unremoved_tags = tag_array & attempted_removed_tags
 
       if unremoved_tags.present?

test/functional/posts_controller_test.rb

@@ -90,7 +90,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
         end
 
         should "render for an artist tag" do
-          create(:post, tag_string: "artist:bkub", rating: "s")
+          as(@user) { create(:post, tag_string: "artist:bkub", rating: "s") }
           get posts_path, params: { tags: "bkub" }
           assert_response :success
           assert_select "#show-excerpt-link", count: 1, text: "Artist"
@@ -131,7 +131,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
         end
 
         should "render for a tag with a wiki page" do
-          create(:post, tag_string: "char:fumimi", rating: "s")
+          as(@user) { create(:post, tag_string: "char:fumimi", rating: "s") }
           get posts_path, params: { tags: "fumimi" }
           assert_response :success
           assert_select "#show-excerpt-link", count: 1, text: "Wiki"

test/functional/related_tags_controller_test.rb

@@ -3,7 +3,7 @@ require 'test_helper'
 class RelatedTagsControllerTest < ActionDispatch::IntegrationTest
   context "The related tags controller" do
     setup do
-      create(:post, tag_string: "copy:touhou")
+      as(create(:user)) { create(:post, tag_string: "copy:touhou") }
     end
 
     context "show action" do

test/unit/post_test.rb

@@ -3,9 +3,8 @@ require 'test_helper'
 class PostTest < ActiveSupport::TestCase
   def self.assert_invalid_tag(tag_name)
     should "not allow '#{tag_name}' to be tagged" do
-      post = build(:post, tag_string: "touhou #{tag_name}")
+      post = create(:post, tag_string: "touhou #{tag_name}")
 
-      assert(post.valid?)
       assert_equal("touhou", post.tag_string)
       assert_equal(1, post.warnings[:base].grep(/Couldn't add tag/).count)
     end
@@ -479,20 +478,47 @@ class PostTest < ActiveSupport::TestCase
         end
       end
 
+      context "tagged with a tag string containing newlines" do
+        should "not include the newlines in the tags" do
+          @post.update!(tag_string: "bkub\r\ntouhou\r\nchen  inaba_tewi\nhonk_honk\n")
+
+          assert_equal("bkub chen honk_honk inaba_tewi touhou", @post.tag_string)
+        end
+      end
+
       context "tagged with a deprecated tag" do
         should "not remove the tag if the tag was already in the post" do
           bad_tag = create(:tag, name: "bad_tag")
-          old_post = FactoryBot.create(:post, tag_string: "bad_tag")
+          old_post = create(:post, tag_string: "bad_tag")
           bad_tag.update!(is_deprecated: true)
           old_post.update!(tag_string: "asd bad_tag")
-          assert_equal("asd bad_tag", old_post.reload.tag_string)
 
+          assert_equal("asd bad_tag", old_post.reload.tag_string)
+          assert_no_match(/The following tags are deprecated and could not be added: \[\[a_bad_tag\]\]/, @post.warnings.full_messages.join)
         end
 
         should "not add the tag if it is being added" do
           create(:tag, name: "a_bad_tag", is_deprecated: true)
           @post.update!(tag_string: "asd a_bad_tag")
+
           assert_equal("asd", @post.reload.tag_string)
+          assert_match(/The following tags are deprecated and could not be added: \[\[a_bad_tag\]\]/, @post.warnings.full_messages.join)
+        end
+
+        should "not add the tag when it contains a category prefix" do
+          create(:tag, name: "a_bad_tag", is_deprecated: true)
+          @post.update!(tag_string: "asd char:a_bad_tag")
+
+          assert_equal("asd", @post.reload.tag_string)
+          assert_match(/The following tags are deprecated and could not be added: \[\[a_bad_tag\]\]/, @post.warnings.full_messages.join)
+        end
+
+        should "not warn about the tag being deprecated when the tag is added and removed in the same edit" do
+          create(:tag, name: "a_bad_tag", is_deprecated: true)
+          @post.update!(tag_string: "asd a_bad_tag -a_bad_tag")
+
+          assert_equal("asd", @post.reload.tag_string)
+          assert_no_match(/The following tags are deprecated and could not be added: \[\[a_bad_tag\]\]/, @post.warnings.full_messages.join)
         end
       end
 
@@ -591,6 +617,14 @@ class PostTest < ActiveSupport::TestCase
             assert_nil(@post.parent_id)
           end
 
+          should "clear the parent with parent:0" do
+            @post.update(parent_id: @parent.id)
+            assert_equal(@parent.id, @post.parent_id)
+
+            @post.update(tag_string: "parent:0")
+            assert_nil(@post.parent_id)
+          end
+
           should "clear the parent with -parent:1234" do
             @post.update(:parent_id => @parent.id)
             assert_equal(@parent.id, @post.parent_id)
@@ -620,78 +654,74 @@ class PostTest < ActiveSupport::TestCase
         end
 
         context "for a pool" do
-          context "on creation" do
+          should "add the post to the pool by id" do
-            setup do
+            @pool = create(:pool)
-              @pool = FactoryBot.create(:pool)
+            @post = create(:post, tag_string: "aaa pool:#{@pool.id}")
-              @post = FactoryBot.create(:post, :tag_string => "aaa pool:#{@pool.id}")
+            assert_equal([@post.id], @pool.reload.post_ids)
-            end
-
-            should "add the post to the pool" do
-              @post.reload
-              @pool.reload
-              assert_equal([@post.id], @pool.post_ids)
-            end
           end
 
-          context "negated" do
+          should "remove the post from the pool by id" do
-            setup do
+            @pool = create(:pool, post_ids: [@post.id])
-              @pool = FactoryBot.create(:pool)
+            @post.update!(tag_string: "aaa -pool:#{@pool.id}")
-              @post = FactoryBot.create(:post, :tag_string => "aaa")
-              @pool.add!(@post)
-              @post.tag_string = "aaa -pool:#{@pool.id}"
-              @post.save
-            end
 
-            should "remove the post from the pool" do
+            assert_equal([], @pool.reload.post_ids)
-              @post.reload
-              @pool.reload
-              assert_equal([], @pool.post_ids)
-            end
           end
 
-          context "id" do
+          should "add the post to the pool by name" do
-            setup do
+            @pool = create(:pool, name: "abc")
-              @pool = FactoryBot.create(:pool)
+            @post.update(tag_string: "aaa pool:abc")
-              @post.update(tag_string: "aaa pool:#{@pool.id}")
-            end
 
-            should "add the post to the pool" do
+            assert_equal([@post.id], @pool.reload.post_ids)
-              @post.reload
-              @pool.reload
-              assert_equal([@post.id], @pool.post_ids)
-            end
           end
 
-          context "name" do
+          should "remove the post from the pool by name" do
-            context "that exists" do
+            @pool = create(:pool, name: "abc", post_ids: [@post.id])
-              setup do
+            @post.update(tag_string: "aaa -pool:abc")
-                @pool = FactoryBot.create(:pool, :name => "abc")
-                @post.update(tag_string: "aaa pool:abc")
-              end
 
-              should "add the post to the pool" do
+            assert_equal([], @pool.reload.post_ids)
-                @post.reload
+          end
-                @pool.reload
+        end
-                assert_equal([@post.id], @pool.post_ids)
-              end
-            end
 
-            context "that doesn't exist" do
+        context "for the newpool: metatag" do
-              should "create a new pool and add the post to that pool" do
+          should "create a new pool and add the post to that pool" do
-                @post.update(tag_string: "aaa newpool:abc")
+            @post.update(tag_string: "aaa newpool:abc")
-                @pool = Pool.find_by_name("abc")
+            @pool = Pool.find_by_name("abc")
-                @post.reload
-                assert_not_nil(@pool)
-                assert_equal([@post.id], @pool.post_ids)
-              end
-            end
 
-            context "with special characters" do
+            assert_not_nil(@pool)
-              should "not strip '%' from the name" do
+            assert_equal([@post.id], @pool.post_ids)
-                @post.update(tag_string: "aaa newpool:ichigo_100%")
+          end
-                assert(Pool.exists?(name: "ichigo_100%"))
+
-              end
+          should "not strip special characters from the name" do
-            end
+            @post.update(tag_string: "aaa newpool:ichigo_100%")
+
+            assert(Pool.exists?(name: "ichigo_100%"))
+          end
+
+          should "parse a double-quoted name" do
+            @post.update(tag_string: 'aaa newpool:"foo bar baz" bbb')
+            @pool = Pool.find_by_name("foo_bar_baz")
+
+            assert_not_nil(@pool)
+            assert_equal([@post.id], @pool.post_ids)
+            assert_equal("aaa bbb", @post.tag_string)
+          end
+
+          should "parse a single-quoted name" do
+            @post.update(tag_string: "aaa newpool:'foo bar baz' bbb")
+            @pool = Pool.find_by_name("foo_bar_baz")
+
+            assert_not_nil(@pool)
+            assert_equal([@post.id], @pool.post_ids)
+            assert_equal("aaa bbb", @post.tag_string)
+          end
+
+          should "parse a name with backslash-escaped spaces" do
+            @post.update(tag_string: "aaa newpool:foo\\ bar\\ baz bbb")
+            @pool = Pool.find_by_name("foo_bar_baz")
+
+            assert_not_nil(@pool)
+            assert_equal([@post.id], @pool.post_ids)
+            assert_equal("aaa bbb", @post.tag_string)
           end
         end
 
@@ -876,8 +906,21 @@ class PostTest < ActiveSupport::TestCase
           end
 
           should 'set the source with source:"foo bar baz"' do
-            @post.update(:tag_string => 'source:"foo bar baz"')
+            @post.update(tag_string: 'aaa source:"foo bar baz" bbb')
             assert_equal("foo bar baz", @post.source)
+            assert_equal("aaa bbb", @post.tag_string)
+          end
+
+          should "set the source with source:'foo bar baz'" do
+            @post.update(tag_string: "aaa source:'foo bar baz' bbb")
+            assert_equal("foo bar baz", @post.source)
+            assert_equal("aaa bbb", @post.tag_string)
+          end
+
+          should "set the source with source:foo\\ bar\\ baz" do
+            @post.update(tag_string: "aaa source:foo\\ bar\\ baz bbb")
+            assert_equal("foo bar baz", @post.source)
+            assert_equal("aaa bbb", @post.tag_string)
           end
 
           should 'strip the source with source:"  foo bar baz  "' do
@@ -893,6 +936,7 @@ class PostTest < ActiveSupport::TestCase
 
           should "set the pixiv id with source:https://img18.pixiv.net/img/evazion/14901720.png" do
             @post.update(:tag_string => "source:https://img18.pixiv.net/img/evazion/14901720.png")
+            assert_equal("https://img18.pixiv.net/img/evazion/14901720.png", @post.source)
             assert_equal(14901720, @post.pixiv_id)
           end
 

test/unit/related_tag_calculator_test.rb

@@ -45,7 +45,7 @@ class RelatedTagCalculatorTest < ActiveSupport::TestCase
       end
 
       should "calculate the most frequent tags with a category constraint" do
-        create(:post, tag_string: "aaa bbb art:ccc copy:ddd")
+        as(@user) { create(:post, tag_string: "aaa bbb art:ccc copy:ddd") }
         create(:post, tag_string: "aaa bbb ccc")
         create(:post, tag_string: "aaa bbb")