#409

backend/app/models/post.rb

@@ -59,7 +59,7 @@ class Post < ApplicationRecord
 
   before_validation :normalise_url, if: :will_save_change_to_url?
 
-  validates :url, presence: true, uniqueness: true
+  validates :url, presence: true, uniqueness: true, length: { maximum: 768 }
   validates :video_ms, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true
 
   validate :validate_original_created_range

backend/app/models/post_url_sanitisation_rule.rb

@@ -1,6 +1,16 @@
 class PostUrlSanitisationRule < ApplicationRecord
   include Discard::Model
 
+  class InvalidUrlError < StandardError
+    attr_reader :invalid_rows
+
+    def initialize(invalid_rows)
+      @invalid_rows = invalid_rows
+      ids = invalid_rows.map { _1.fetch(:post_id) }.join(', ')
+      super("post URL sanitisation produced invalid URLs for posts #{ ids }")
+    end
+  end
+
   class UrlConflictError < StandardError
     attr_reader :conflicts
 
@@ -20,33 +30,37 @@ class PostUrlSanitisationRule < ApplicationRecord
   validate :source_pattern_must_be_regexp
 
   class << self
-    def sanitise(url) =
-        rules.reduce(url.dup) { |value, (pattern, replacement)| value.sub(pattern, replacement) }
+    def sanitise(url) = sanitise_with_rules(url, rules)
 
     def apply!
-      rewrites = Post.order(:id).pluck(:id, :url).map do |post_id, original_url|
-        { post_id:,
-          original_url:,
-          sanitised_url: sanitise(original_url) }
-      end
-
-      conflicts = rewrites
-                      .group_by { _1.fetch(:sanitised_url) }
-                      .values
-                      .filter { _1.size > 1 }
-                      .flatten
-                      .map { { url: _1.fetch(:sanitised_url),
-                               post_id: _1.fetch(:post_id),
-                               original_url: _1.fetch(:original_url) } }
-
-      raise UrlConflictError.new(conflicts) if conflicts.present?
-
-      changed = rewrites.filter { _1.fetch(:original_url) != _1.fetch(:sanitised_url) }
-      return if changed.empty?
-
-      token = SecureRandom.hex(6)
+      rewrites = nil
 
       Post.transaction do
+        compiled_rules = rules
+
+        rewrites = Post.order(:id)
+                       .lock('FOR UPDATE')
+                       .pluck(:id, :url)
+                       .map do |post_id, original_url|
+          { post_id:,
+            original_url:,
+            sanitised_url: sanitise_with_rules(original_url, compiled_rules) }
+        end
+
+        invalid_rows = rewrites.filter { invalid_sanitised_url?(_1.fetch(:sanitised_url)) }
+                               .map { { post_id: _1.fetch(:post_id),
+                                        original_url: _1.fetch(:original_url),
+                                        sanitised_url: _1.fetch(:sanitised_url) } }
+        raise InvalidUrlError.new(invalid_rows) if invalid_rows.present?
+
+        conflicts = build_conflicts(rewrites)
+        raise UrlConflictError.new(conflicts) if conflicts.present?
+
+        changed = rewrites.filter { _1.fetch(:original_url) != _1.fetch(:sanitised_url) }
+        return if changed.empty?
+
+        token = SecureRandom.hex(6)
+
         changed.each do |row|
           Post.where(id: row.fetch(:post_id))
               .update_all(url: temporary_url_for(row.fetch(:post_id), token))
@@ -57,19 +71,91 @@ class PostUrlSanitisationRule < ApplicationRecord
               .update_all(url: row.fetch(:sanitised_url))
         end
       end
+    rescue ActiveRecord::RecordNotUnique => error
+      conflicts = build_persisted_conflicts(rewrites)
+      raise error if conflicts.empty?
+
+      raise UrlConflictError.new(conflicts), cause: error
     end
 
     private
 
     def rules = kept.order(:priority).map { |r| [Regexp.new(r.source_pattern), r.replacement] }
 
+    def sanitise_with_rules(url, compiled_rules)
+      compiled_rules.reduce(url.dup) do |value, (pattern, replacement)|
+        value.sub(pattern, replacement)
+      end
+    end
+
     def temporary_url_for(post_id, token) =
         "https://post-url-sanitising.invalid/#{ token }/#{ post_id }"
+
+    def invalid_sanitised_url?(url)
+      return true if url.blank?
+      return true if url.length > 768
+
+      parsed = URI.parse(url)
+      return true if !(parsed in URI::HTTP)
+      return true if parsed.host.blank?
+
+      false
+    rescue URI::InvalidURIError
+      true
+    end
+
+    def build_conflicts(rewrites)
+      rewrites
+          .group_by { _1.fetch(:sanitised_url).downcase }
+          .values
+          .filter { _1.size > 1 }
+          .flatten
+          .map { { url: _1.fetch(:sanitised_url),
+                   post_id: _1.fetch(:post_id),
+                   original_url: _1.fetch(:original_url) } }
+    end
+
+    def build_persisted_conflicts(rewrites)
+      return [] if rewrites.blank?
+
+      target_rows = rewrites.filter { _1.fetch(:original_url) != _1.fetch(:sanitised_url) }
+      target_keys = target_rows.map { _1.fetch(:sanitised_url).downcase }.uniq
+      return [] if target_keys.empty?
+
+      target_pairs = target_rows.to_h do |row|
+        [row.fetch(:post_id), row.fetch(:sanitised_url).downcase]
+      end
+
+      persisted_rows = Post.order(:id)
+                           .where('LOWER(url) IN (?)', target_keys)
+                           .pluck(:id, :url)
+                           .reject { |post_id, original_url| target_pairs[post_id] == original_url.downcase }
+                           .map { |post_id, original_url|
+        { url: original_url,
+          post_id:,
+          original_url:,
+          conflict_key: original_url.downcase }
+      }
+
+      target_conflicts = target_rows.map { { url: _1.fetch(:sanitised_url),
+                                             post_id: _1.fetch(:post_id),
+                                             original_url: _1.fetch(:original_url),
+                                             conflict_key: _1.fetch(:sanitised_url).downcase } }
+
+      (persisted_rows + target_conflicts)
+          .group_by { _1.fetch(:conflict_key) }
+          .values
+          .filter { _1.size > 1 }
+          .flatten
+          .map { _1.except(:conflict_key) }
+    end
   end
 
   private
 
   def source_pattern_must_be_regexp
+    return if source_pattern.blank?
+
     Regexp.new(source_pattern)
   rescue RegexpError
     errors.add :source_pattern, '変な正規表現だね〜（笑）'