URL 正規化 (#409) (#410)

Reviewed-on: #410
Co-authored-by: miteruzo <miteruzo@naver.com>
Co-committed-by: miteruzo <miteruzo@naver.com>

backend/spec/models/post_url_sanitisation_rule_spec.rb

@@ -0,0 +1,265 @@
+require 'rails_helper'
+
+RSpec.describe PostUrlSanitisationRule, type: :model do
+  before do
+    described_class.unscoped.delete_all
+  end
+
+  describe 'validations' do
+    it 'requires a source pattern' do
+      rule = described_class.new(priority: 10, source_pattern: nil, replacement: '')
+
+      expect(rule).to be_invalid
+      expect(rule.errors.details.fetch(:source_pattern)).to eq([{ error: :blank }])
+    end
+
+    it 'requires a unique source pattern' do
+      described_class.create!(priority: 10, source_pattern: 'source', replacement: 'first')
+      rule = described_class.new(
+        priority: 20,
+        source_pattern: 'source',
+        replacement: 'second'
+      )
+
+      expect(rule).to be_invalid
+      expect(rule.errors.details.fetch(:source_pattern)).to include(
+        error: :taken,
+        value: 'source'
+      )
+    end
+
+    it 'rejects an invalid regexp' do
+      rule = described_class.new(priority: 10, source_pattern: '[', replacement: '')
+
+      expect(rule).to be_invalid
+      expect(rule.errors[:source_pattern]).to include('変な正規表現だね〜（笑）')
+    end
+  end
+
+  describe '.sanitise' do
+    it 'applies each active rule once in priority order' do
+      described_class.create!(priority: 30, source_pattern: 'c', replacement: 'd')
+      described_class.create!(priority: 10, source_pattern: 'a', replacement: 'aa')
+      described_class.create!(priority: 20, source_pattern: 'aa', replacement: 'c')
+      discarded = described_class.create!(
+        priority: 5,
+        source_pattern: '.',
+        replacement: 'discarded'
+      )
+      discarded.discard!
+
+      expect(described_class.sanitise('a')).to eq('d')
+    end
+
+    it 'canonicalises the initial YouTube and Nico URL forms' do
+      create_initial_rules
+
+      urls = {
+        'https://youtu.be/abc123?si=share' => youtube_url('abc123'),
+        'https://www.youtube.com/live/abc123?t=10' => youtube_url('abc123'),
+        'https://youtube.com/shorts/abc123?feature=share' => youtube_url('abc123'),
+        'https://m.youtube.com/embed/abc123' => youtube_url('abc123'),
+        'https://youtube.com/watch?feature=share&v=abc123&t=10' => youtube_url('abc123'),
+        'https://nico.ms/sm123?from=share#fragment' => nico_url('sm123'),
+        'https://www.nicovideo.jp/watch/sm123?ref=share#fragment' => nico_url('sm123')
+      }
+
+      urls.each do |url, canonical_url|
+        expect(described_class.sanitise(url)).to eq(canonical_url)
+      end
+    end
+  end
+
+  describe '.apply!' do
+    it 'locks posts in ID order and updates through temporary URLs' do
+      first = create(:post, url: 'https://example.com/source/1')
+      second = create(:post, url: 'https://example.com/source/2')
+      create_source_rule
+      sql = capture_sql { described_class.apply! }
+
+      lock_sql = sql.find { _1.match?(/SELECT .*posts.*FOR UPDATE/i) }
+      expect(lock_sql).to match(/ORDER BY .*posts.*id.* ASC/i)
+      expect(sql.grep(/post-url-sanitising\.invalid/).size).to eq(2)
+      expect(first.reload.url).to eq('https://example.com/canonical/1')
+      expect(second.reload.url).to eq('https://example.com/canonical/2')
+    end
+
+    it 'loads and compiles rules only once' do
+      create(:post, url: 'https://example.com/source/1')
+      create(:post, url: 'https://example.com/source/2')
+      create_source_rule
+
+      expect(described_class).to receive(:rules).once.and_call_original
+
+      described_class.apply!
+    end
+
+    it 'does not change post versions, version_no, or updated_at' do
+      post = create(:post, url: 'https://example.com/source/1')
+      post.update_columns(version_no: 7, updated_at: 1.day.ago)
+      original_updated_at = post.reload.updated_at
+      create_source_rule
+
+      expect { described_class.apply! }.not_to change(PostVersion, :count)
+
+      post.reload
+      expect(post.url).to eq('https://example.com/canonical/1')
+      expect(post.version_no).to eq(7)
+      expect(post.updated_at).to eq(original_updated_at)
+    end
+
+    invalid_urls = {
+      'a blank URL' => '',
+      'an unparseable URL' => 'https://[',
+      'a non-HTTP URL' => 'ftp://example.com/file',
+      'an HTTP URL without a host' => 'https:/path'
+    }
+
+    invalid_urls.each do |description, sanitised_url|
+      it "rolls back every update when sanitisation produces #{ description }" do
+        valid_post = create(:post, url: 'https://example.com/source/1')
+        invalid_post = create(:post, url: 'https://example.com/invalid')
+        create_source_rule
+        described_class.create!(
+          priority: 20,
+          source_pattern: '\\Ahttps://example\\.com/invalid\\z',
+          replacement: sanitised_url
+        )
+
+        expect { described_class.apply! }
+            .to raise_error(described_class::InvalidUrlError) { |error|
+          expect(error.invalid_rows).to eq([
+            { post_id: invalid_post.id,
+              original_url: 'https://example.com/invalid',
+              sanitised_url: }
+          ])
+        }
+        expect(valid_post.reload.url).to eq('https://example.com/source/1')
+        expect(invalid_post.reload.url).to eq('https://example.com/invalid')
+      end
+    end
+
+    it 'rolls back every update when sanitisation produces a URL longer than 768 characters' do
+      path = 'a' * 375
+      original_url = "https://example.com/#{ path }"
+      sanitised_url = "https://example.com/#{ path }#{ path }"
+      valid_post = create(:post, url: 'https://example.com/source/1')
+      invalid_post = create(:post, url: original_url)
+      create_source_rule
+      described_class.create!(
+        priority: 20,
+        source_pattern: '\\Ahttps://example\\.com/(a+)\\z',
+        replacement: 'https://example.com/\\1\\1'
+      )
+
+      expect { described_class.apply! }
+          .to raise_error(described_class::InvalidUrlError) { |error|
+        expect(error.invalid_rows).to eq([
+          { post_id: invalid_post.id,
+            original_url:,
+            sanitised_url: }
+        ])
+      }
+      expect(valid_post.reload.url).to eq('https://example.com/source/1')
+      expect(invalid_post.reload.url).to eq(original_url)
+    end
+
+    it 'rejects sanitised URL collisions case-insensitively without changing posts' do
+      source = create(:post, url: 'https://example.com/source/Foo')
+      canonical = create(:post, url: 'https://example.com/canonical/foo')
+      create_source_rule
+
+      expect { described_class.apply! }
+          .to raise_error(described_class::UrlConflictError) { |error|
+        expect(error.conflicts).to contain_exactly(
+          { url: 'https://example.com/canonical/Foo',
+            post_id: source.id,
+            original_url: 'https://example.com/source/Foo' },
+          { url: 'https://example.com/canonical/foo',
+            post_id: canonical.id,
+            original_url: 'https://example.com/canonical/foo' }
+        )
+      }
+      expect(source.reload.url).to eq('https://example.com/source/Foo')
+      expect(canonical.reload.url).to eq('https://example.com/canonical/foo')
+      expect(Post.count).to eq(2)
+    end
+
+    it 'converts a persisted URL constraint race into UrlConflictError' do
+      post = create(:post, url: 'https://example.com/source/1')
+      create_source_rule
+      conflict = { url: 'https://example.com/canonical/1',
+                   post_id: post.id,
+                   original_url: post.url }
+      database_error = ActiveRecord::RecordNotUnique.new('duplicate URL')
+      allow_any_instance_of(ActiveRecord::Relation)
+          .to receive(:update_all).and_raise(database_error)
+      allow(described_class).to receive(:build_persisted_conflicts).and_return([conflict])
+
+      expect { described_class.apply! }
+          .to raise_error(described_class::UrlConflictError) { |error|
+        expect(error.conflicts).to eq([conflict])
+        expect(error.cause).to equal(database_error)
+      }
+      expect(post.reload.url).to eq('https://example.com/source/1')
+    end
+
+    it 're-raises an unidentified RecordNotUnique error' do
+      post = create(:post, url: 'https://example.com/source/1')
+      create_source_rule
+      database_error = ActiveRecord::RecordNotUnique.new('another unique constraint')
+      allow_any_instance_of(ActiveRecord::Relation)
+          .to receive(:update_all).and_raise(database_error)
+      allow(described_class).to receive(:build_persisted_conflicts).and_return([])
+
+      expect { described_class.apply! }.to raise_error(database_error)
+      expect(post.reload.url).to eq('https://example.com/source/1')
+    end
+  end
+
+  def capture_sql
+    statements = []
+    subscriber = lambda do |_name, _start, _finish, _id, payload|
+      binds = payload.fetch(:binds, []).map { _1.value_for_database.to_s }
+      statements << ([payload.fetch(:sql)] + binds).join(' ')
+    end
+    ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') { yield }
+    statements
+  end
+
+  def create_source_rule
+    described_class.create!(
+      priority: 10,
+      source_pattern: '\\Ahttps://example\\.com/source/(.+)\\z',
+      replacement: 'https://example.com/canonical/\\1'
+    )
+  end
+
+  def create_initial_rules
+    rules = [
+      ['\\Ahttps?://youtu\\.be/([^/?#]+)(?:[?#].*)?\\z', youtube_url('\\1')],
+      ['\\Ahttps?://(?:www\\.|m\\.)?youtube\\.com/live/([^/?#]+)(?:[?#].*)?\\z',
+       youtube_url('\\1')],
+      ['\\Ahttps?://(?:www\\.|m\\.)?youtube\\.com/shorts/([^/?#]+)(?:[?#].*)?\\z',
+       youtube_url('\\1')],
+      ['\\Ahttps?://(?:www\\.|m\\.)?youtube\\.com/embed/([^/?#]+)(?:[?#].*)?\\z',
+       youtube_url('\\1')],
+      ['\\Ahttps?://(?:www\\.|m\\.)?youtube\\.com/watch\\?(?:[^#&]+&)*' \
+       'v=([^&#]+)(?:[&#].*)?\\z', youtube_url('\\1')],
+      ['\\Ahttps?://nico\\.ms/([^/?#]+)(?:[?#].*)?\\z', nico_url('\\1')],
+      ['\\Ahttps?://(?:www\\.)?nicovideo\\.jp/watch/([^?#/]+)(?:[?#].*)?\\z',
+       nico_url('\\1')]
+    ]
+    rules.each_with_index do |(source_pattern, replacement), index|
+      described_class.create!(
+        priority: (index + 1) * 10,
+        source_pattern:,
+        replacement:
+      )
+    end
+  end
+
+  def youtube_url(video_id) = "https://www.youtube.com/watch?v=#{ video_id }"
+
+  def nico_url(video_id) = "https://www.nicovideo.jp/watch/#{ video_id }"
+end