URL 正規化 (#409) #410
@@ -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']
|
||||
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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'])
|
||||
|
||||
新しい課題から参照
ユーザをブロックする