From acd062f2895ca419ed91e4b51943f43f38a343af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=BF=E3=81=A6=E3=82=8B=E3=81=9E?= Date: Tue, 30 Jun 2026 01:20:55 +0900 Subject: [PATCH] =?UTF-8?q?=E3=82=BF=E3=82=B0=E5=BE=AA=E7=92=B0=E3=81=AE?= =?UTF-8?q?=E7=A6=81=E6=AD=A2=20(#332)=20(#382)?= 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/382 --- backend/app/models/tag_implication.rb | 24 ++++++++++++++ backend/spec/models/tag_implication_spec.rb | 36 +++++++++++++++++++++ backend/spec/models/tag_spec.rb | 12 ++++++- backend/spec/requests/tag_children_spec.rb | 11 +++++++ backend/spec/requests/tags_spec.rb | 14 ++++++-- 5 files changed, 93 insertions(+), 4 deletions(-) create mode 100644 backend/spec/models/tag_implication_spec.rb diff --git a/backend/app/models/tag_implication.rb b/backend/app/models/tag_implication.rb index a629764..888713c 100644 --- a/backend/app/models/tag_implication.rb +++ b/backend/app/models/tag_implication.rb @@ -6,6 +6,7 @@ class TagImplication < ApplicationRecord validates :parent_tag_id, presence: true validate :parent_tag_mustnt_be_itself + validate :parent_tag_mustnt_create_cycle private @@ -14,4 +15,27 @@ class TagImplication < ApplicationRecord errors.add :parent_tag_id, '親タグは子タグと同一であってはなりません．' end end + + def parent_tag_mustnt_create_cycle + return if tag_id.blank? || parent_tag_id.blank? + return if errors[:parent_tag_id].present? + + seen = { } + stack = [parent_tag_id] + + until stack.empty? + current_id = stack.pop + next if seen[current_id] + + seen[current_id] = true + + if current_id == tag_id + errors.add :parent_tag_id, '親タグに子孫タグを指定すると循環します．' + errors.add :base, 'タグの親子関係が循環します．' + return + end + + stack.concat(TagImplication.where(tag_id: current_id).pluck(:parent_tag_id)) + end + end end diff --git a/backend/spec/models/tag_implication_spec.rb b/backend/spec/models/tag_implication_spec.rb new file mode 100644 index 0000000..a29cc1c --- /dev/null +++ b/backend/spec/models/tag_implication_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +RSpec.describe TagImplication, type: :model do + it 'rejects a parent tag that would create a cycle' do + child = create(:tag, name: 'tag_implication_cycle_child') + parent = create(:tag, name: 'tag_implication_cycle_parent') + + described_class.create!(tag: child, parent_tag: parent) + + implication = described_class.new(tag: parent, parent_tag: child) + + expect(implication).not_to be_valid + expect(implication.errors[:parent_tag_id]).to include( + '親タグに子孫タグを指定すると循環します．' + ) + expect(implication.errors[:base]).to be_present + end + + it 'terminates even when existing data already contains a cycle' do + child = create(:tag, name: 'tag_implication_existing_cycle_child') + parent = create(:tag, name: 'tag_implication_existing_cycle_parent') + ancestor = create(:tag, name: 'tag_implication_existing_cycle_ancestor') + + described_class.create!(tag: parent, parent_tag: ancestor) + described_class.insert_all!( + [ + { tag_id: ancestor.id, parent_tag_id: parent.id, + created_at: Time.current, updated_at: Time.current } + ] + ) + + implication = described_class.new(tag: child, parent_tag: parent) + + expect(implication).to be_valid + end +end diff --git a/backend/spec/models/tag_spec.rb b/backend/spec/models/tag_spec.rb index 014cdde..1b2549c 100644 --- a/backend/spec/models/tag_spec.rb +++ b/backend/spec/models/tag_spec.rb @@ -54,7 +54,17 @@ RSpec.describe Tag, type: :model do first = create(:tag, name: 'expand_cycle_first') second = create(:tag, name: 'expand_cycle_second') TagImplication.create!(tag: first, parent_tag: second) - TagImplication.create!(tag: second, parent_tag: first) + now = Time.current + TagImplication.insert_all!( + [ + { + tag_id: second.id, + parent_tag_id: first.id, + created_at: now, + updated_at: now + } + ] + ) expect(described_class.expand_parent_tags([first])).to contain_exactly(first, second) end diff --git a/backend/spec/requests/tag_children_spec.rb b/backend/spec/requests/tag_children_spec.rb index 8e1b91b..69bb10a 100644 --- a/backend/spec/requests/tag_children_spec.rb +++ b/backend/spec/requests/tag_children_spec.rb @@ -56,6 +56,17 @@ RSpec.describe "TagChildren", type: :request do expect(response).to have_http_status(:no_content) end + + it 'returns 422 and does not create relation when the new link makes a cycle' do + TagImplication.create!(tag: parent, parent_tag: child) + + expect { + do_request + }.not_to change(TagImplication, :count) + + expect(response).to have_http_status(:unprocessable_entity) + expect(TagImplication.where(tag: child, parent_tag: parent)).not_to exist + end end context "when Tag.find raises (invalid ids)" do diff --git a/backend/spec/requests/tags_spec.rb b/backend/spec/requests/tags_spec.rb index ccb28ab..0055364 100644 --- a/backend/spec/requests/tags_spec.rb +++ b/backend/spec/requests/tags_spec.rb @@ -797,7 +797,17 @@ RSpec.describe 'Tags API', type: :request do ) TagImplication.create!(tag: first, parent_tag: root_material) TagImplication.create!(tag: second, parent_tag: first) - TagImplication.create!(tag: first, parent_tag: second) + now = Time.current + TagImplication.insert_all!( + [ + { + tag_id: first.id, + parent_tag_id: second.id, + created_at: now, + updated_at: now + } + ] + ) get '/tags/with-depth', params: { parent: root_material.id } @@ -1364,8 +1374,6 @@ RSpec.describe 'Tags API', type: :request do end it 'parent_tags に指定すると循環する tag は 422 にする' do - pending '#332 で対応予定' - child = Tag.create!( tag_name: TagName.create!(name: 'put_cycle_child'), category: :general