From 38650b5671cce01f3a501cb1053b7846bb42e7cc Mon Sep 17 00:00:00 2001 From: miteruzo Date: Tue, 14 Jul 2026 12:38:02 +0900 Subject: [PATCH] =?UTF-8?q?URL=20=E6=AD=A3=E8=A6=8F=E5=8C=96=20(#409)=20(#?= =?UTF-8?q?410)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-on: http://git.miteruzo.com/miteruzo/btrc-hub/pulls/410 Co-authored-by: miteruzo Co-committed-by: miteruzo --- backend/app/models/post.rb | 6 +- .../app/models/post_url_sanitisation_rule.rb | 163 +++++++++++ ...0000_create_post_url_sanitisation_rules.rb | 61 ++++ backend/db/schema.rb | 12 +- backend/db/seeds.rb | 37 +++ backend/spec/db/seeds_spec.rb | 65 +++++ backend/spec/models/post_spec.rb | 68 +++++ .../models/post_url_sanitisation_rule_spec.rb | 265 ++++++++++++++++++ backend/spec/tasks/nico_export_spec.rb | 2 +- 9 files changed, 674 insertions(+), 5 deletions(-) create mode 100644 backend/app/models/post_url_sanitisation_rule.rb create mode 100644 backend/db/migrate/20260713000000_create_post_url_sanitisation_rules.rb create mode 100644 backend/spec/db/seeds_spec.rb create mode 100644 backend/spec/models/post_spec.rb create mode 100644 backend/spec/models/post_url_sanitisation_rule_spec.rb diff --git a/backend/app/models/post.rb b/backend/app/models/post.rb index cdfbaef..d22dd8d 100644 --- a/backend/app/models/post.rb +++ b/backend/app/models/post.rb @@ -57,9 +57,9 @@ class Post < ApplicationRecord attribute :version_no, :integer, default: 1 - before_validation :normalise_url + 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 @@ -180,7 +180,7 @@ class Post < ApplicationRecord u.host = u.host.downcase if u.host u.path = u.path.sub(/\/\Z/, '') if u.path.present? - self.url = u.to_s + self.url = PostUrlSanitisationRule.sanitise(u.to_s) rescue URI::InvalidURIError ; end diff --git a/backend/app/models/post_url_sanitisation_rule.rb b/backend/app/models/post_url_sanitisation_rule.rb new file mode 100644 index 0000000..e449cf6 --- /dev/null +++ b/backend/app/models/post_url_sanitisation_rule.rb @@ -0,0 +1,163 @@ +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 + + def initialize(conflicts) + @conflicts = conflicts + urls = conflicts.map { _1.fetch(:url) }.uniq.join(', ') + super("post URL sanitisation conflicts detected for #{ urls }") + end + end + + self.primary_key = :priority + + default_scope -> { kept } + + validates :source_pattern, presence: true, uniqueness: true + + validate :source_pattern_must_be_regexp + + class << self + def sanitise(url) = sanitise_with_rules(url, rules) + + def apply! + 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)) + end + + changed.each do |row| + Post.where(id: row.fetch(:post_id)) + .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, '変な正規表現だね〜（笑）' + end +end diff --git a/backend/db/migrate/20260713000000_create_post_url_sanitisation_rules.rb b/backend/db/migrate/20260713000000_create_post_url_sanitisation_rules.rb new file mode 100644 index 0000000..77a2b0b --- /dev/null +++ b/backend/db/migrate/20260713000000_create_post_url_sanitisation_rules.rb @@ -0,0 +1,61 @@ +class CreatePostUrlSanitisationRules < ActiveRecord::Migration[8.0] + class PostUrlSanitisationRule < ActiveRecord::Base + self.table_name = 'post_url_sanitisation_rules' + end + + def up + create_table :post_url_sanitisation_rules, id: :integer, primary_key: :priority do |t| + t.string :source_pattern, null: false + t.string :replacement, null: false + t.timestamps + t.datetime :discarded_at + + t.index :source_pattern, unique: true + t.index :discarded_at + end + + now = Time.current + + PostUrlSanitisationRule.insert_all!([ + { priority: 10, + source_pattern: '\Ahttps?://youtu\.be/([^/?#]+)(?:[?#].*)?\z', + replacement: 'https://www.youtube.com/watch?v=\1', + created_at: now, + updated_at: now }, + { priority: 20, + source_pattern: '\Ahttps?://(?:www\.|m\.)?youtube\.com/live/([^/?#]+)(?:[?#].*)?\z', + replacement: 'https://www.youtube.com/watch?v=\1', + created_at: now, + updated_at: now }, + { priority: 30, + source_pattern: '\Ahttps?://(?:www\.|m\.)?youtube\.com/shorts/([^/?#]+)(?:[?#].*)?\z', + replacement: 'https://www.youtube.com/watch?v=\1', + created_at: now, + updated_at: now }, + { priority: 40, + source_pattern: '\Ahttps?://(?:www\.|m\.)?youtube\.com/embed/([^/?#]+)(?:[?#].*)?\z', + replacement: 'https://www.youtube.com/watch?v=\1', + created_at: now, + updated_at: now }, + { priority: 50, + source_pattern: + '\Ahttps?://(?:www\.|m\.)?youtube\.com/watch\?(?:[^#&]+&)*v=([^&#]+)(?:[&#].*)?\z', + replacement: 'https://www.youtube.com/watch?v=\1', + created_at: now, + updated_at: now }, + { priority: 60, + source_pattern: '\Ahttps?://nico\.ms/([^/?#]+)(?:[?#].*)?\z', + replacement: 'https://www.nicovideo.jp/watch/\1', + created_at: now, + updated_at: now }, + { priority: 70, + source_pattern: '\Ahttps?://(?:www\.)?nicovideo\.jp/watch/([^?#/]+)(?:[?#].*)?\z', + replacement: 'https://www.nicovideo.jp/watch/\1', + created_at: now, + updated_at: now }]) + end + + def down + drop_table :post_url_sanitisation_rules + end +end diff --git a/backend/db/schema.rb b/backend/db/schema.rb index eb54633..c515aee 100644 --- a/backend/db/schema.rb +++ b/backend/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_07_05_000000) do +ActiveRecord::Schema[8.0].define(version: 2026_07_13_000000) do create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -331,6 +331,16 @@ ActiveRecord::Schema[8.0].define(version: 2026_07_05_000000) do t.index ["tag_id"], name: "index_post_tags_on_tag_id" end + create_table "post_url_sanitisation_rules", primary_key: "priority", id: :integer, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.string "source_pattern", null: false + t.string "replacement", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.datetime "discarded_at" + t.index ["discarded_at"], name: "index_post_url_sanitisation_rules_on_discarded_at" + t.index ["source_pattern"], name: "index_post_url_sanitisation_rules_on_source_pattern", unique: true + end + create_table "post_versions", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "post_id", null: false t.integer "version_no", null: false diff --git a/backend/db/seeds.rb b/backend/db/seeds.rb index 067d073..c88f60a 100644 --- a/backend/db/seeds.rb +++ b/backend/db/seeds.rb @@ -8,6 +8,43 @@ # MovieGenre.find_or_create_by!(name: genre_name) # end +post_url_sanitisation_rules = [ + { priority: 10, + source_pattern: '\Ahttps?://youtu\.be/([^/?#]+)(?:[?#].*)?\z', + replacement: 'https://www.youtube.com/watch?v=\1' }, + { priority: 20, + source_pattern: '\Ahttps?://(?:www\.|m\.)?youtube\.com/live/([^/?#]+)(?:[?#].*)?\z', + replacement: 'https://www.youtube.com/watch?v=\1' }, + { priority: 30, + source_pattern: '\Ahttps?://(?:www\.|m\.)?youtube\.com/shorts/([^/?#]+)(?:[?#].*)?\z', + replacement: 'https://www.youtube.com/watch?v=\1' }, + { priority: 40, + source_pattern: '\Ahttps?://(?:www\.|m\.)?youtube\.com/embed/([^/?#]+)(?:[?#].*)?\z', + replacement: 'https://www.youtube.com/watch?v=\1' }, + { priority: 50, + source_pattern: + '\Ahttps?://(?:www\.|m\.)?youtube\.com/watch\?(?:[^#&]+&)*v=([^&#]+)(?:[&#].*)?\z', + replacement: 'https://www.youtube.com/watch?v=\1' }, + { priority: 60, + source_pattern: '\Ahttps?://nico\.ms/([^/?#]+)(?:[?#].*)?\z', + replacement: 'https://www.nicovideo.jp/watch/\1' }, + { priority: 70, + source_pattern: '\Ahttps?://(?:www\.)?nicovideo\.jp/watch/([^?#/]+)(?:[?#].*)?\z', + replacement: 'https://www.nicovideo.jp/watch/\1' } +] + +post_url_sanitisation_rule_scope = PostUrlSanitisationRule.unscoped + +post_url_sanitisation_rules.each do |attributes| + priority = attributes.fetch(:priority) + source_pattern = attributes.fetch(:source_pattern) + + next if post_url_sanitisation_rule_scope.exists?(priority:) + next if post_url_sanitisation_rule_scope.exists?(source_pattern:) + + post_url_sanitisation_rule_scope.create!(attributes) +end + material_sync_source_uri = ENV['MATERIAL_SYNC_SOURCE_URI'] material_sync_source_file_id = ENV['MATERIAL_SYNC_SOURCE_FILE_ID'] diff --git a/backend/spec/db/seeds_spec.rb b/backend/spec/db/seeds_spec.rb new file mode 100644 index 0000000..57eba67 --- /dev/null +++ b/backend/spec/db/seeds_spec.rb @@ -0,0 +1,65 @@ +require 'rails_helper' + +RSpec.describe 'database seeds' do + before do + PostUrlSanitisationRule.unscoped.delete_all + end + + it 'registers the initial post URL sanitisation rules' do + load_seeds + + 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') + } + + expect(PostUrlSanitisationRule.count).to eq(7) + urls.each do |url, canonical_url| + expect(PostUrlSanitisationRule.sanitise(url)).to eq(canonical_url) + end + end + + it 'does not overwrite or restore an existing rule' do + rule = PostUrlSanitisationRule.create!( + priority: 10, + source_pattern: '\Ahttps://example\.com/custom\z', + replacement: 'https://example.com/replacement' + ) + rule.discard! + original_attributes = rule.reload.attributes + + 2.times { load_seeds } + + persisted_rule = PostUrlSanitisationRule.unscoped.find(10) + expect(persisted_rule.attributes).to eq(original_attributes) + expect(PostUrlSanitisationRule.unscoped.count).to eq(7) + end + + it 'does not duplicate a rule moved to another priority' do + source_pattern = '\Ahttps?://youtu\.be/([^/?#]+)(?:[?#].*)?\z' + PostUrlSanitisationRule.create!( + priority: 80, + source_pattern:, + replacement: 'https://example.com/custom/\1' + ) + + load_seeds + + rules = PostUrlSanitisationRule.unscoped + expect(rules.where(source_pattern:).count).to eq(1) + expect(rules.find(80).replacement).to eq('https://example.com/custom/\1') + end + + def load_seeds + load Rails.root.join('db/seeds.rb') + 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 diff --git a/backend/spec/models/post_spec.rb b/backend/spec/models/post_spec.rb new file mode 100644 index 0000000..627b28f --- /dev/null +++ b/backend/spec/models/post_spec.rb @@ -0,0 +1,68 @@ +require 'rails_helper' + +RSpec.describe Post, type: :model do + before do + PostUrlSanitisationRule.unscoped.delete_all + end + + describe 'URL normalisation' do + it 'normalises the HTTP URL before applying sanitisation rules' do + PostUrlSanitisationRule.create!( + priority: 10, + source_pattern: '\\Ahttps://example\\.com/videos/([^/]+)\\z', + replacement: 'https://example.com/watch/\\1' + ) + + post = described_class.create!( + title: 'normalised URL', + url: ' https://EXAMPLE.com/videos/123/ ' + ) + + expect(post.url).to eq('https://example.com/watch/123') + end + + it 'does not normalise an unchanged URL when another attribute changes' do + post = create(:post) + post.update_column(:url, 'https://EXAMPLE.com/unchanged/') + + post.update!(title: 'updated title') + + expect(post.reload.url).to eq('https://EXAMPLE.com/unchanged/') + end + + it 'validates the sanitised URL length' do + path = 'a' * 375 + + PostUrlSanitisationRule.create!( + priority: 10, + source_pattern: '\\Ahttps://example\\.com/(a+)\\z', + replacement: 'https://example.com/\\1\\1' + ) + + post = described_class.new( + title: 'long URL', + url: "https://example.com/#{ path }" + ) + + expect(post).to be_invalid + expect(post.errors.details.fetch(:url)).to include( + error: :too_long, + count: 768 + ) + end + + it 'validates uniqueness after sanitisation' do + PostUrlSanitisationRule.create!( + priority: 10, + source_pattern: '\\Ahttps://example\\.com/alias\\z', + replacement: 'https://example.com/canonical' + ) + create(:post, url: 'https://example.com/canonical') + + post = described_class.new(title: 'duplicate URL', url: 'https://example.com/alias') + + expect(post).to be_invalid + expect(post.errors.details.fetch(:url)).to include(error: :taken, value: post.url) + end + end +end diff --git a/backend/spec/models/post_url_sanitisation_rule_spec.rb b/backend/spec/models/post_url_sanitisation_rule_spec.rb new file mode 100644 index 0000000..bf5f7df --- /dev/null +++ b/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 diff --git a/backend/spec/tasks/nico_export_spec.rb b/backend/spec/tasks/nico_export_spec.rb index e6ed87c..0221f99 100644 --- a/backend/spec/tasks/nico_export_spec.rb +++ b/backend/spec/tasks/nico_export_spec.rb @@ -62,7 +62,7 @@ RSpec.describe 'nico:export' do it 'deduplicates video ids' do create_post('https://www.nicovideo.jp/watch/sm12345') - create_post('https://www.nicovideo.jp/watch/sm12345?from=1') + create_post('https://sp.nicovideo.jp/watch/sm12345') expect(Open3).to receive(:capture3) do |_env, *args, **_kwargs| expect(args.drop(3)).to eq(['sm12345'])