diff --git a/backend/app/controllers/tags_controller.rb b/backend/app/controllers/tags_controller.rb index cb07ddd..a940f0b 100644 --- a/backend/app/controllers/tags_controller.rb +++ b/backend/app/controllers/tags_controller.rb @@ -200,14 +200,45 @@ class TagsController < ApplicationController .find_by(id: params[:id]) return head :not_found unless tag + rows = normalise_deerjikist_rows(tag) + return if performed? + ApplicationRecord.transaction do - tag.deerjikists = [] - params[:_json].each.with_index do |item, i| - platform = item[:platform] - code = normalise_deerjikist_code(platform, item[:code]) - deerjikist = Deerjikist.find_or_initialize_by(platform:, code:) - deerjikist.tag = tag - render_deerjikist_form_record_invalid(deerjikist, i) unless deerjikist.save + tag.lock! + + requested_keys = rows.map { |row| [row[:platform], row[:code]] }.uniq + row_indexes_by_key = rows_by_key(rows) + locked_deerjikists = lock_deerjikists_for_tag_update(tag.id, requested_keys) + current_deerjikists = locked_deerjikists.filter { |deerjikist| + 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? end end @@ -653,6 +684,7 @@ class TagsController < ApplicationController end def normalise_deerjikist_code platform, code + code = code.to_s return code if platform != 'youtube' || code[0] != '@' url = "https://www.youtube.com/#{ code }" @@ -669,6 +701,82 @@ class TagsController < ApplicationController nil 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 fields = { } diff --git a/backend/spec/requests/tags_deerjikists_spec.rb b/backend/spec/requests/tags_deerjikists_spec.rb index 7bbf224..e0d3a3e 100644 --- a/backend/spec/requests/tags_deerjikists_spec.rb +++ b/backend/spec/requests/tags_deerjikists_spec.rb @@ -226,6 +226,14 @@ RSpec.describe 'Tags deerjikists API', type: :request do [platform2, code2], ) 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 context 'when tag already has deerjikists' do @@ -299,6 +307,81 @@ RSpec.describe 'Tags deerjikists API', type: :request do 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 let(:channel_id) { 'UCabcdefghijklmnopqrstuv' } let(:payload) do