ニジラー情報で他ニジラー・タグの持つ情報を奪へてしまふ問題 (#406) #407

オープン
みてるぞfeature/406 から main への 2 コミットのマージを希望してゐます
2個のファイルの変更198行の追加7行の削除
+115 -7
ファイルの表示
@@ -200,14 +200,45 @@ class TagsController < ApplicationController
.find_by(id: params[:id]) .find_by(id: params[:id])
return head :not_found unless tag return head :not_found unless tag
rows = normalise_deerjikist_rows(tag)
return if performed?
ApplicationRecord.transaction do ApplicationRecord.transaction do
tag.deerjikists = [] tag.lock!
params[:_json].each.with_index do |item, i|
platform = item[:platform] requested_keys = rows.map { |row| [row[:platform], row[:code]] }.uniq
code = normalise_deerjikist_code(platform, item[:code]) row_indexes_by_key = rows_by_key(rows)
deerjikist = Deerjikist.find_or_initialize_by(platform:, code:) locked_deerjikists = lock_deerjikists_for_tag_update(tag.id, requested_keys)
deerjikist.tag = tag current_deerjikists = locked_deerjikists.filter { |deerjikist|
render_deerjikist_form_record_invalid(deerjikist, i) unless deerjikist.save deerjikist.tag_id == tag.id
}
requested_deerjikists = locked_deerjikists.filter { |deerjikist|
row_indexes_by_key.key?([deerjikist.platform, deerjikist.code])
}
render_deerjikist_conflicts(requested_deerjikists, row_indexes_by_key, tag)
raise ActiveRecord::Rollback if performed?
requested_keys_set = requested_keys.to_set
current_deerjikists.each do |deerjikist|
key = [deerjikist.platform, deerjikist.code]
deerjikist.destroy! unless requested_keys_set.include?(key)
end
existing_keys = requested_deerjikists.to_h { |deerjikist|
[[deerjikist.platform, deerjikist.code], true]
}
requested_keys.each do |platform, code|
next if existing_keys[[platform, code]]
deerjikist = Deerjikist.new(platform:, code:, tag:)
row_index = row_indexes_by_key[[platform, code]].first
begin
render_deerjikist_form_record_invalid(deerjikist, row_index) unless deerjikist.save
rescue ActiveRecord::RecordNotUnique
conflicts = lock_deerjikists_for_tag_update(tag.id, [[platform, code]])
render_deerjikist_conflicts(conflicts, row_indexes_by_key, tag)
end
raise ActiveRecord::Rollback if performed? raise ActiveRecord::Rollback if performed?
end end
end end
@@ -653,6 +684,7 @@ class TagsController < ApplicationController
end end
def normalise_deerjikist_code platform, code def normalise_deerjikist_code platform, code
code = code.to_s
return code if platform != 'youtube' || code[0] != '@' return code if platform != 'youtube' || code[0] != '@'
url = "https://www.youtube.com/#{ code }" url = "https://www.youtube.com/#{ code }"
@@ -669,6 +701,82 @@ class TagsController < ApplicationController
nil nil
end end
def normalise_deerjikist_rows tag
rows = []
params[:_json].each.with_index do |item, index|
platform = item[:platform]
unless Deerjikist.platforms.key?(platform)
render_deerjikist_platform_invalid(index)
return rows
end
code = normalise_deerjikist_code(platform, item[:code])
deerjikist = Deerjikist.new(platform:, code:, tag:)
unless deerjikist.valid?
render_deerjikist_form_record_invalid(deerjikist, index)
return rows
end
rows << { index:, platform:, code: }
end
rows
end
def lock_deerjikists_for_tag_update tag_id, keys
clauses = ['tag_id = ?']
values = [tag_id]
keys.each do |platform, code|
clauses << '(platform = ? AND code = ?)'
values << platform << code
end
Deerjikist
.where(clauses.join(' OR '), *values)
.order(:platform, :code)
.lock
.to_a
end
def render_deerjikist_conflicts deerjikists, row_indexes_by_key, tag
conflicts = deerjikists.filter { |deerjikist| deerjikist.tag_id != tag.id }
return if conflicts.empty?
tag_names_by_id = Tag
.joins(:tag_name)
.where(id: conflicts.map(&:tag_id).uniq)
.pluck('tags.id', 'tag_names.name')
.to_h
fields = { }
conflicts.each do |deerjikist|
message = "この情報は既に「#{ tag_names_by_id[deerjikist.tag_id] }」に紐づいてゐます."
row_indexes_by_key[[deerjikist.platform, deerjikist.code]].each do |index|
field = :"deerjikists.#{ index }.code"
fields[field] ||= []
fields[field] << message
end
end
render_validation_error fields:
end
def render_deerjikist_platform_invalid index
render_validation_error fields: {
:"deerjikists.#{ index }.platform" => ['値が不正です.'],
}
end
def rows_by_key rows
rows.each_with_object({ }) do |row, result|
key = [row[:platform], row[:code]]
result[key] ||= []
result[key] << row[:index]
end
end
def render_deerjikist_form_record_invalid deerjikist, index def render_deerjikist_form_record_invalid deerjikist, index
fields = { } fields = { }
+83
ファイルの表示
@@ -226,6 +226,14 @@ RSpec.describe 'Tags deerjikists API', type: :request do
[platform2, code2], [platform2, code2],
) )
end end
it 'locks the tag before replacing the complete list' do
expect_any_instance_of(Tag).to receive(:lock!).and_call_original
do_request
expect(response).to have_http_status(:ok)
end
end end
context 'when tag already has deerjikists' do context 'when tag already has deerjikists' do
@@ -299,6 +307,81 @@ RSpec.describe 'Tags deerjikists API', type: :request do
end end
end end
context 'when platform is outside the enum' do
let(:payload) do
[
{ platform: 'invalid', code: code1 },
]
end
it 'returns 422 with an indexed platform error without changing the list' do
Deerjikist.create!(platform: platform1, code: code1, tag: tag)
expect {
do_request
}.not_to change { Deerjikist.where(tag: tag).map { |d| [d.platform, d.code] } }
expect(response).to have_http_status(:unprocessable_entity)
expect(json.fetch('errors')).to include(
'deerjikists.0.platform' => [be_present],
)
end
end
context 'when a requested deerjikist belongs to another tag' do
let!(:other_tag) { create(:tag, category: :deerjikist) }
let!(:owned_deerjikist) do
Deerjikist.create!(platform: platform1, code: code1, tag: tag)
end
let!(:conflicting_deerjikist) do
Deerjikist.create!(platform: platform2, code: code2, tag: other_tag)
end
let(:payload) do
[
{ platform: 'nico', code: 'new-code' },
{ platform: platform2, code: code2 },
]
end
before do
other_tag.tag_name.update!(name: 'existing-deerjikist')
end
it 'returns an indexed 422 error and rolls back the complete replacement' do
expect {
do_request
}.not_to change { Deerjikist.order(:platform, :code).pluck(:platform, :code, :tag_id) }
expect(response).to have_http_status(:unprocessable_entity)
expect(json.fetch('errors')).to include(
'deerjikists.1.code' => [include('existing-deerjikist')],
)
expect(owned_deerjikist.reload.tag_id).to eq(tag.id)
expect(conflicting_deerjikist.reload.tag_id).to eq(other_tag.id)
end
end
context 'when a requested deerjikist already belongs to the same tag' do
let!(:existing_deerjikist) do
Deerjikist.create!(platform: platform1, code: code1, tag: tag)
end
let(:payload) do
[
{ platform: platform1, code: code1 },
]
end
it 'keeps the existing row' do
expect {
do_request
}.not_to change { existing_deerjikist.reload.created_at }
expect(response).to have_http_status(:ok)
expect(Deerjikist.where(tag: tag).pluck(:platform, :code))
.to eq([[platform1, code1]])
end
end
context 'when youtube code is handle' do context 'when youtube code is handle' do
let(:channel_id) { 'UCabcdefghijklmnopqrstuv' } let(:channel_id) { 'UCabcdefghijklmnopqrstuv' }
let(:payload) do let(:payload) do