diff --git a/backend/app/controllers/tags_controller.rb b/backend/app/controllers/tags_controller.rb index 7fdcc22..8613bd0 100644 --- a/backend/app/controllers/tags_controller.rb +++ b/backend/app/controllers/tags_controller.rb @@ -302,10 +302,9 @@ class TagsController < ApplicationController NicoTagVersionRecorder.record!(tag:, event_type:, created_by_user:) else TagVersionRecorder.record!(tag:, event_type:, created_by_user:) - if tag.has_wiki - WikiVersionRecorder.record!(page: tag.tag_name.wiki_page, - event_type: :update, - created_by_user:) + wiki_page = tag.tag_name.wiki_page + if wiki_page&.wiki_versions&.exists? + WikiVersionRecorder.record!(page: wiki_page, event_type: :update, created_by_user:) end end end diff --git a/backend/app/models/wiki_page.rb b/backend/app/models/wiki_page.rb index 83a7f1b..b725706 100644 --- a/backend/app/models/wiki_page.rb +++ b/backend/app/models/wiki_page.rb @@ -17,6 +17,7 @@ class WikiPage < ApplicationRecord belongs_to :tag_name validates :tag_name, presence: true + validates :body, presence: true def title = tag_name.name @@ -26,11 +27,6 @@ class WikiPage < ApplicationRecord def current_revision = wiki_revisions.order(id: :desc).first - def body - rev = current_revision - rev.body if rev&.content? - end - def resolve_redirect limit: 10 page = self visited = Set.new diff --git a/backend/app/services/wiki/commit.rb b/backend/app/services/wiki/commit.rb index f3e325d..4f22fc3 100644 --- a/backend/app/services/wiki/commit.rb +++ b/backend/app/services/wiki/commit.rb @@ -7,6 +7,31 @@ module Wiki ; end + def self.create_content! tag_name:, body:, created_by_user:, message: nil + normalised = normalise_body(body) + + page = WikiPage.new(tag_name:, + body: normalised, + created_user: created_by_user, + updated_user: created_by_user) + + if normalised.blank? + page.errors.add(:body, :blank) + raise ActiveRecord::RecordInvalid, page + end + + ActiveRecord::Base.transaction do + page.save! + + new(page:, created_user: created_by_user).content!( + body: normalised, + message:, + base_revision_id: nil) + + page + end + end + def self.content! page:, body:, created_user:, message: nil, base_revision_id: nil new(page:, created_user:).content!(body:, message:, base_revision_id:) end @@ -21,7 +46,12 @@ module Wiki end def content! body:, message:, base_revision_id: - normalised = normalise_body(body).gsub(/\n+$/, '') + normalised = self.class.normalise_body(body) + if normalised.blank? + @page.errors.add(:body, :blank) + raise ActiveRecord::RecordInvalid, @page + end + lines = split_lines(normalised) line_shas = lines.map { |line| Digest::SHA256.hexdigest(line) } @@ -45,11 +75,12 @@ module Wiki @page.update!(body: normalised) WikiVersionRecorder.record!( - page: { title: @page.title, body: normalised }, + page: @page, event_type: @page.wiki_versions.exists? ? :update : :create, + reason: message, created_by_user: @created_user) tag = @page.tag_name.tag - if tag + if tag&.tag_versions&.exists? TagVersionRecorder.record!(tag:, event_type: :update, created_by_user: @created_user) end @@ -74,14 +105,15 @@ module Wiki def redirect!(redirect_page:, message:, base_revision_id:) = raise '廃止しました.' - private - - def normalise_body body + def self.normalise_body body s = body.to_s s.gsub!(/\r\n?/, "\n") s.encode('UTF-8', invalid: :replace, undef: :replace, replace: '🖕') + s.gsub(/\n+$/, '') end + private + def split_lines(body) = body.split("\n") def upsert_lines! lines, line_shas diff --git a/backend/app/services/wiki_version_recorder.rb b/backend/app/services/wiki_version_recorder.rb index a9bebe4..f6aa09e 100644 --- a/backend/app/services/wiki_version_recorder.rb +++ b/backend/app/services/wiki_version_recorder.rb @@ -1,9 +1,10 @@ class WikiVersionRecorder < VersionRecorder - def self.record! page:, event_type:, created_by_user: + def self.record! page:, event_type:, reason: nil, created_by_user: new(page:, event_type:, created_by_user:).record! end - def initialize page:, event_type:, created_by_user: + def initialize page:, event_type:, reason: nil, created_by_user: + @reason = reason super(record: page, event_type:, created_by_user:) end @@ -13,5 +14,8 @@ class WikiVersionRecorder < VersionRecorder def version_association = :wiki_versions def record_key = :wiki_page - def snapshot_attributes = { title: @record.title, body: @record.body } + def snapshot_attributes = { + title: @record.title, + body: @record.body, + reason: @reason } end diff --git a/backend/spec/factories/wiki_pages.rb b/backend/spec/factories/wiki_pages.rb index b4f1496..7f5f41a 100644 --- a/backend/spec/factories/wiki_pages.rb +++ b/backend/spec/factories/wiki_pages.rb @@ -3,5 +3,7 @@ FactoryBot.define do title { "TestPage" } association :created_user, factory: :user association :updated_user, factory: :user + + body { ' ' } end end diff --git a/backend/spec/models/tag_spec.rb b/backend/spec/models/tag_spec.rb index 1dee732..1a1714b 100644 --- a/backend/spec/models/tag_spec.rb +++ b/backend/spec/models/tag_spec.rb @@ -107,11 +107,13 @@ RSpec.describe Tag, type: :model do context 'when the source tag_name has a wiki_page' do let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_tag) } let!(:wiki_page) do - WikiPage.create!( - tag_name: source_tag_name, - created_user: create_admin_user!, - updated_user: create_admin_user! - ) + admin = create_admin_user! + + Wiki::Commit.create_content!( + tag_name: source_tag_name, + body: 'source wiki body', + created_by_user: admin, + message: 'init') end it 'rolls back the transaction' do diff --git a/backend/spec/requests/tag_versions_spec.rb b/backend/spec/requests/tag_versions_spec.rb index f1d92e7..0175704 100644 --- a/backend/spec/requests/tag_versions_spec.rb +++ b/backend/spec/requests/tag_versions_spec.rb @@ -214,5 +214,34 @@ RSpec.describe 'TagVersions API', type: :request do expect(versions.size).to eq(1) expect(versions.first['version_no']).to eq(2) end + + it 'includes tag versions created by wiki updates' do + wiki_tag_name = TagName.create!(name: 'tag_versions_from_wiki') + wiki_tag = Tag.create!(tag_name: wiki_tag_name, category: :general) + + wiki_page = + Wiki::Commit.create_content!( + tag_name: wiki_tag_name, + body: 'before', + created_by_user: member, + message: 'init') + + Wiki::Commit.content!( + page: wiki_page, + body: 'after', + created_user: member, + message: 'edit', + base_revision_id: wiki_page.current_revision.id) + + get '/tags/versions', params: { id: wiki_tag.id } + + expect(response).to have_http_status(:ok) + + versions = json.fetch('versions') + + expect(json.fetch('count')).to eq(2) + expect(versions.map { |v| v['event_type'] }).to eq(['update', 'create']) + expect(versions.map { |v| v['name']['current'] }.uniq).to eq(['tag_versions_from_wiki']) + end end end diff --git a/backend/spec/requests/tags_spec.rb b/backend/spec/requests/tags_spec.rb index d3bc594..e186582 100644 --- a/backend/spec/requests/tags_spec.rb +++ b/backend/spec/requests/tags_spec.rb @@ -876,6 +876,105 @@ RSpec.describe 'Tags API', type: :request do expect(system_tag.reload.name).to eq(old_name) expect(system_tag.category).to eq(old_category) end + + it 'wiki を持つ tag を更新すると wiki version も作成する' do + wiki_page = + Wiki::Commit.create_content!( + tag_name: tag.tag_name, + body: 'wiki body before', + created_by_user: member_user, + message: 'init') + + Wiki::Commit.content!( + page: wiki_page, + body: 'wiki body before', + created_user: member_user, + message: 'init' + ) + + expect { + put "/tags/#{ tag.id }", params: { + name: 'put_wiki_version_tag', + category: 'meme', + aliases: 'unko', + parent_tags: '', + } + } + .to change(TagVersion, :count).by(2) + .and change(WikiVersion, :count).by(1) + + expect(response).to have_http_status(:ok) + + version = wiki_page.reload.wiki_versions.order(:version_no).last + + expect(version).to have_attributes( + event_type: 'update', + title: 'put_wiki_version_tag', + body: 'wiki body before', + created_by_user_id: member_user.id + ) + end + + it '別名を他 tag から奪った場合、奪はれた側の tag version も作成する' do + pending '#329 で対応予定' + + old_owner = Tag.create!( + tag_name: TagName.create!(name: 'put_alias_old_owner'), + category: :general + ) + stolen_alias = TagName.create!( + name: 'put_stolen_alias', + canonical: old_owner.tag_name + ) + + expect(old_owner.tag_name.aliases.map(&:name)).to include('put_stolen_alias') + + expect { + put "/tags/#{ tag.id }", params: { + name: 'spec_tag', + category: 'general', + aliases: 'unko put_stolen_alias', + parent_tags: '', + } + } + .to change { tag.reload.tag_versions.count }.by(2) + .and change { old_owner.reload.tag_versions.count }.by(2) + + expect(response).to have_http_status(:ok) + + expect(stolen_alias.reload.canonical).to eq(tag.tag_name) + expect(old_owner.reload.tag_name.aliases.map(&:name)).not_to include('put_stolen_alias') + + old_owner_versions = old_owner.tag_versions.order(:version_no) + + expect(old_owner_versions.first.event_type).to eq('create') + expect(old_owner_versions.first.aliases.split).to include('put_stolen_alias') + + expect(old_owner_versions.second.event_type).to eq('update') + expect(old_owner_versions.second.aliases.split).not_to include('put_stolen_alias') + end + + it 'parent_tags に指定すると循環する tag は 422 にする' do + pending '#332 で対応予定' + + child = Tag.create!( + tag_name: TagName.create!(name: 'put_cycle_child'), + category: :general + ) + + TagImplication.create!(tag: child, parent_tag: tag) + + put "/tags/#{ tag.id }", params: { + name: 'spec_tag', + category: 'general', + aliases: 'unko', + parent_tags: child.name, + } + + expect(response).to have_http_status(:unprocessable_entity) + + expect(TagImplication.where(tag:, parent_tag: child)).not_to exist + end end end end diff --git a/backend/spec/requests/wiki_spec.rb b/backend/spec/requests/wiki_spec.rb index 4541501..020e412 100644 --- a/backend/spec/requests/wiki_spec.rb +++ b/backend/spec/requests/wiki_spec.rb @@ -8,7 +8,12 @@ RSpec.describe 'Wiki API', type: :request do let!(:tn) { TagName.create!(name: 'spec_wiki_title') } let!(:page) do - WikiPage.create!(tag_name: tn, created_user: user, updated_user: user).tap do |p| + WikiPage.create!( + tag_name: tn, + body: 'init', + created_user: user, + updated_user: user + ).tap do |p| Wiki::Commit.content!(page: p, body: 'init', created_user: user, message: 'init') end end @@ -99,6 +104,17 @@ RSpec.describe 'Wiki API', type: :request do end .to change(WikiPage, :count).by(1) .and change(WikiRevision, :count).by(1) + .and change(WikiVersion, :count).by(1) + + version = page.wiki_versions.order(:version_no).last + + expect(version).to have_attributes( + version_no: 1, + event_type: 'create', + title: 'TestPage', + body: "a\nb\nc", + created_by_user_id: member.id + ) expect(response).to have_http_status(:created) @@ -135,6 +151,41 @@ RSpec.describe 'Wiki API', type: :request do # "a" の WikiLine が増殖しない(1行のはず) expect(WikiLine.where(body: 'a').count).to eq(1) end + + it 'deduplicates duplicated new lines before upsert' do + duplicated = 'duplicated_line_for_wiki_line_upsert_spec' + + post endpoint, + params: { title: 'DuplicateNewLine', body: "#{ duplicated }\n#{ duplicated }" }, + headers: auth_headers(member) + + expect(response).to have_http_status(:created) + + page = WikiPage.find(json.fetch('id')) + rev = page.current_revision + + expect(rev.lines_count).to eq(2) + expect(WikiLine.where(body: duplicated).count).to eq(1) + expect(rev.wiki_lines.where(body: duplicated).count).to eq(1) + expect(rev.wiki_revision_lines.count).to eq(2) + end + + it 'normalises CRLF and strips trailing newlines' do + post endpoint, + params: { title: 'NormalisedBody', body: "a\r\nb\r\n\r\n", message: 'normalise' }, + headers: auth_headers(member) + + expect(response).to have_http_status(:created) + + page = WikiPage.find(json.fetch('id')) + rev = page.current_revision + version = page.wiki_versions.order(:version_no).last + + expect(page.body).to eq("a\nb") + expect(version.body).to eq("a\nb") + expect(rev.lines_count).to eq(2) + expect(rev.wiki_lines.order('wiki_revision_lines.position').map(&:body)).to eq(['a', 'b']) + end end end @@ -146,17 +197,14 @@ RSpec.describe 'Wiki API', type: :request do { 'X-Transfer-Code' => user.inheritance_code } end - #let!(:page) { create(:wiki_page, title: 'TestPage') } - let!(:page) do - build(:wiki_page, title: 'TestPage').tap do |p| - puts p.errors.full_messages unless p.valid? - p.save! - end - end + let!(:test_tag_name) { TagName.create!(name: 'TestPage') } - before do - # 初期版を 1 つ作っておく(更新が“2版目”になるように) - Wiki::Commit.content!(page: page, body: "a\nb", created_user: member, message: 'init') + let!(:page) do + Wiki::Commit.create_content!( + tag_name: test_tag_name, + body: "a\nb", + created_by_user: member, + message: 'init') end context 'when not logged in' do @@ -200,7 +248,18 @@ RSpec.describe 'Wiki API', type: :request do put "/wiki/#{page.id}", params: { title: 'TestPage', body: "x\ny", message: 'edit', base_revision_id: current_id }, headers: auth_headers(member) - end.to change(WikiRevision, :count).by(1) + end + .to change(WikiRevision, :count).by(1) + .and change(WikiVersion, :count).by(1) + + version = page.wiki_versions.order(:version_no).last + + expect(version).to have_attributes( + event_type: 'update', + title: 'TestPage', + body: "x\ny", + created_by_user_id: member.id + ) expect(response).to have_http_status(:ok) @@ -211,6 +270,48 @@ RSpec.describe 'Wiki API', type: :request do expect(page.body).to eq("x\ny") expect(rev.base_revision_id).to eq(current_id) end + + it 'wiki page に対応する tag があれば tag version も作成する' do + linked_tag_name = TagName.create!(name: 'wiki_linked_tag_for_version') + linked_tag = Tag.create!(tag_name: linked_tag_name, category: :general) + linked_page = WikiPage.create!( + tag_name: linked_tag_name, + body: 'before', + created_user: member, + updated_user: member + ) + + Wiki::Commit.content!( + page: linked_page, + body: 'before', + created_user: member, + message: 'init' + ) + + current_id = linked_page.wiki_revisions.maximum(:id) + + expect { + put "/wiki/#{ linked_page.id }", + params: { + title: 'wiki_linked_tag_for_version', + body: 'after', + message: 'edit', + base_revision_id: current_id, + }, + headers: auth_headers(member) + } + .to change(WikiRevision, :count).by(1) + .and change(WikiVersion, :count).by(1) + .and change { linked_tag.reload.tag_versions.count }.by(1) + + expect(response).to have_http_status(:ok) + + version = linked_tag.reload.tag_versions.order(:version_no).last + + expect(version.event_type).to eq('update') + expect(version.name).to eq('wiki_linked_tag_for_version') + expect(version.created_by_user_id).to eq(member.id) + end end # TODO: コンフリクト未実装のため,実装したらコメント外す. @@ -263,11 +364,11 @@ RSpec.describe 'Wiki API', type: :request do before do # 追加で検索ヒット用 TagName.create!(name: 'spec_wiki_title_2') - WikiPage.create!(tag_name: TagName.find_by!(name: 'spec_wiki_title_2'), + WikiPage.create!(tag_name: TagName.find_by!(name: 'spec_wiki_title_2'), body: 'init', created_user: user, updated_user: user) TagName.create!(name: 'unrelated_title') - WikiPage.create!(tag_name: TagName.find_by!(name: 'unrelated_title'), + WikiPage.create!(tag_name: TagName.find_by!(name: 'unrelated_title'), body: 'init', created_user: user, updated_user: user) end @@ -329,7 +430,7 @@ RSpec.describe 'Wiki API', type: :request do it 'returns empty array when page has no revisions and filtered by id' do # 別ページを作って revision 無し tn2 = TagName.create!(name: 'spec_no_rev') - p2 = WikiPage.create!(tag_name: tn2, created_user: user, updated_user: user) + p2 = WikiPage.create!(tag_name: tn2, body: 'init', created_user: user, updated_user: user) get "/wiki/changes?id=#{p2.id}" expect(response).to have_http_status(:ok) @@ -398,29 +499,27 @@ RSpec.describe 'Wiki API', type: :request do expect(json['older_revision_id']).to eq(rev_a.id) expect(json['newer_revision_id']).to eq(page.current_revision.id) end + end - it 'returns 422 when "to" is redirect revision' do - # redirect revision を作る - tn2 = TagName.create!(name: 'redirect_target') - target = WikiPage.create!(tag_name: tn2, created_user: user, updated_user: user) - - Wiki::Commit.redirect!(page: page, redirect_page: target, created_user: user, message: 'redir') - redirect_rev = page.current_revision - expect(redirect_rev).to be_redirect - - get "/wiki/#{page.id}/diff?from=#{rev_a.id}&to=#{redirect_rev.id}" - expect(response).to have_http_status(:unprocessable_entity) - end - - it 'returns 422 when "from" is redirect revision' do - tn2 = TagName.create!(name: 'redirect_target2') - target = WikiPage.create!(tag_name: tn2, created_user: user, updated_user: user) - - Wiki::Commit.redirect!(page: page, redirect_page: target, created_user: user, message: 'redir2') - redirect_rev = page.current_revision + describe 'Wiki::Commit.redirect!' do + it 'raises because redirect revisions are deprecated' do + target_tag_name = TagName.create!(name: 'redirect_deprecated_target') + target = WikiPage.create!( + tag_name: target_tag_name, + body: 'target', + created_user: user, + updated_user: user + ) - get "/wiki/#{page.id}/diff?from=#{redirect_rev.id}&to=#{rev_b.id}" - expect(response).to have_http_status(:unprocessable_entity) + expect { + Wiki::Commit.redirect!( + page: page, + redirect_page: target, + created_user: user, + message: 'redirect', + base_revision_id: page.current_revision.id + ) + }.to raise_error(RuntimeError, '廃止しました.') end end end diff --git a/backend/spec/services/wiki/commit_spec.rb b/backend/spec/services/wiki/commit_spec.rb new file mode 100644 index 0000000..d8513c5 --- /dev/null +++ b/backend/spec/services/wiki/commit_spec.rb @@ -0,0 +1,120 @@ +require 'rails_helper' + +RSpec.describe Wiki::Commit do + let(:user) { create_member_user! } + + def create_page(title: 'commit_spec_page', body: 'initial body') + tag_name = TagName.create!(name: title) + + Wiki::Commit.create_content!( + tag_name:, + body:, + created_by_user: user, + message: 'init') + end + + describe '.content!' do + it 'stores normalised body in wiki_pages and wiki_versions' do + page = create_page(title: 'commit_normalised_page') + + described_class.content!( + page:, + body: "a\r\nb\r\n\r\n", + created_user: user, + message: 'init' + ) + + page.reload + version = page.wiki_versions.order(:version_no).last + + expect(page.body).to eq("a\nb") + expect(version.body).to eq("a\nb") + expect(page.current_revision.lines_count).to eq(2) + end + + it 'deduplicates duplicated missing wiki lines before upsert' do + page = create_page(title: 'commit_duplicate_line_page') + duplicated = 'commit_duplicate_line' + + described_class.content!( + page:, + body: "#{ duplicated }\n#{ duplicated }", + created_user: user, + message: 'init' + ) + + page.reload + + expect(WikiLine.where(body: duplicated).count).to eq(1) + expect(page.current_revision.lines_count).to eq(2) + expect(page.current_revision.wiki_revision_lines.count).to eq(2) + end + + it 'raises conflict when base_revision_id is stale' do + page = create_page(title: 'commit_conflict_page') + + first = described_class.content!( + page:, + body: 'first', + created_user: user, + message: 'first' + ) + + described_class.content!( + page:, + body: 'second', + created_user: user, + message: 'second', + base_revision_id: first.id + ) + + expect { + described_class.content!( + page:, + body: 'third', + created_user: user, + message: 'third', + base_revision_id: first.id + ) + }.to raise_error(Wiki::Commit::Conflict) + end + + it 'records tag version when page has corresponding tag' do + tag_name = TagName.create!(name: 'commit_linked_tag') + tag = Tag.create!(tag_name:, category: :general) + page = WikiPage.create!( + tag_name:, + body: '', + created_user: user, + updated_user: user + ) + + expect { + described_class.content!( + page:, + body: 'body', + created_user: user, + message: 'init' + ) + } + .to change(WikiVersion, :count).by(1) + .and change { tag.reload.tag_versions.count }.by(1) + end + end + + describe '.redirect!' do + it 'raises because redirect revisions are deprecated' do + page = create_page(title: 'commit_redirect_source') + target = create_page(title: 'commit_redirect_target') + + expect { + described_class.redirect!( + page:, + redirect_page: target, + created_user: user, + message: 'redirect' + ) + }.to raise_error(RuntimeError, '廃止しました.') + end + end +end