From 1e8420464c3931626e782ae9bbece8efa9725ffe Mon Sep 17 00:00:00 2001 From: KMY Date: Tue, 13 May 2025 07:28:37 +0900 Subject: [PATCH] =?UTF-8?q?`StatusReferencesService`=E3=82=92=E5=BC=95?= =?UTF-8?q?=E7=94=A8=E3=81=AB=E9=96=A2=E4=B8=8E=E3=81=97=E3=81=AA=E3=81=84?= =?UTF-8?q?=E6=96=B0=E4=BB=95=E6=A7=98=E3=81=AB=E5=AF=BE=E5=BF=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/lib/activitypub/activity/create.rb | 4 +- .../process_status_update_service.rb | 2 +- app/services/process_references_service.rb | 55 ++------ app/workers/process_references_worker.rb | 4 +- .../process_references_service_spec.rb | 133 +++--------------- 5 files changed, 35 insertions(+), 163 deletions(-) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 5c15cb20db..b15e8097fe 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -653,7 +653,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def process_references! - ProcessReferencesService.call_service_without_error(@status, [], reference_uris, [quote].compact) + ProcessReferencesService.call_service_without_error(@status, [], reference_uris) end def free_friend_domain? @@ -665,6 +665,6 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def quote - @quote ||= nil # TODO: quote + @quote ||= @status_parser.quote_uri end end diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index fd9eca3b69..4bd825ab69 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -348,7 +348,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService def update_references! references = reference_uris - ProcessReferencesService.call_service_without_error(@status, [], references, [quote_url].compact) + ProcessReferencesService.call_service_without_error(@status, [], references) end def reference_uris diff --git a/app/services/process_references_service.rb b/app/services/process_references_service.rb index b901df1bbe..5bfd24cf6b 100644 --- a/app/services/process_references_service.rb +++ b/app/services/process_references_service.rb @@ -11,11 +11,10 @@ class ProcessReferencesService < BaseService QUOTEURL_EXP = /(QT|RN|RE)((:|;)?\s+|:|;)(#{URI::DEFAULT_PARSER.make_regexp(%w(http https))})/ MAX_REFERENCES = 5 - def call(status, reference_parameters, urls: nil, fetch_remote: true, no_fetch_urls: nil, quote_urls: nil) + def call(status, reference_parameters, urls: nil, fetch_remote: true, no_fetch_urls: nil) @status = status @reference_parameters = reference_parameters || [] - @quote_urls = quote_urls || [] - @urls = (urls - @quote_urls) || [] + @urls = urls || [] @no_fetch_urls = no_fetch_urls || [] @fetch_remote = fetch_remote @again = false @@ -42,8 +41,8 @@ class ProcessReferencesService < BaseService launch_worker if @again end - def self.need_process?(status, reference_parameters, urls, quote_urls) - reference_parameters.any? || (urls || []).any? || (quote_urls || []).any? || FormattingHelper.extract_status_plain_text(status).scan(REFURL_EXP).pluck(3).uniq.any? + def self.need_process?(status, reference_parameters, urls) + reference_parameters.any? || (urls || []).any? || FormattingHelper.extract_status_plain_text(status).scan(REFURL_EXP).pluck(3).uniq.any? end def self.extract_uris(text, remote: false) @@ -56,23 +55,17 @@ class ProcessReferencesService < BaseService text.scan(QUOTEURL_EXP).pick(3) end - def self.perform_worker_async(status, reference_parameters, urls, quote_urls) - return unless need_process?(status, reference_parameters, urls, quote_urls) + def self.call_service(status, reference_parameters, urls) + return unless need_process?(status, reference_parameters, urls) - ProcessReferencesWorker.perform_async(status.id, reference_parameters, urls, [], quote_urls || []) + ProcessReferencesService.new.call(status, reference_parameters || [], urls: urls || [], fetch_remote: false) end - def self.call_service(status, reference_parameters, urls, quote_urls = []) - return unless need_process?(status, reference_parameters, urls, quote_urls) - - ProcessReferencesService.new.call(status, reference_parameters || [], urls: urls || [], fetch_remote: false, quote_urls: quote_urls) - end - - def self.call_service_without_error(status, reference_parameters, urls, quote_urls = []) - return unless need_process?(status, reference_parameters, urls, quote_urls) + def self.call_service_without_error(status, reference_parameters, urls) + return unless need_process?(status, reference_parameters, urls) begin - ProcessReferencesService.new.call(status, reference_parameters || [], urls: urls || [], quote_urls: quote_urls) + ProcessReferencesService.new.call(status, reference_parameters || [], urls: urls || []) rescue true end @@ -119,7 +112,6 @@ class ProcessReferencesService < BaseService text = extract_status_plain_text(@status) url_to_attributes = @urls.index_with('BT') .merge(text.scan(REFURL_EXP).to_h { |result| [result[3], result[0]] }) - .merge(@quote_urls.index_with('QT')) url_to_statuses = fetch_statuses(url_to_attributes.keys.uniq) @@ -130,9 +122,9 @@ class ProcessReferencesService < BaseService status = url_to_statuses[url] if status.present? - quote = quote_attribute?(attribute) + quote_attribute?(attribute) - [status.id, !quote || quotable?(status) ? attribute : 'BT'] + [status.id, attribute] else [url, attribute] end @@ -170,10 +162,6 @@ class ProcessReferencesService < BaseService @referrable = StatusPolicy.new(@status.account, target_status).show? end - def quotable?(_target_status) - true # TODO: quote - end - def add_references return if @added_items.empty? @@ -185,12 +173,8 @@ class ProcessReferencesService < BaseService next if status.blank? attribute_type = @added_items[status_id] - quote_attribute?(attribute_type) @added_objects << @status.reference_objects.new(target_status: status, attribute_type: attribute_type) - # TODO: quote - # Quote.create(status: @status, approval_uri: approval_uri) if quote - status.increment_count!(:status_referred_by_count) @references_count += 1 @@ -216,9 +200,6 @@ class ProcessReferencesService < BaseService @status.reference_objects.where(target_status: @removed_items.keys).destroy_all - # TODO: quote - # @status.update!(quote_of_id: nil) if @status.quote_of_id.present? && @removed_items.key?(@status.quote_of_id) - statuses = Status.where(id: @added_items.keys).to_a @removed_items.each_key do |status_id| status = statuses.find { |s| s.id == status_id } @@ -238,20 +219,10 @@ class ProcessReferencesService < BaseService attribute_type = @changed_items[ref.target_status_id] ref.update!(attribute_type: attribute_type) - - # TODO: quote - # quote = quote_attribute?(attribute_type) - # quote_change = ref.quote != quote - # next unless quote_change - # if quote - # ref.status.update!(quote_of_id: ref.target_status.id) - # else - # ref.status.update!(quote_of_id: nil) - # end end end def launch_worker - ProcessReferencesWorker.perform_async(@status.id, @reference_parameters, @urls, @no_fetch_urls, @quote_urls) + ProcessReferencesWorker.perform_async(@status.id, @reference_parameters, @urls, @no_fetch_urls) end end diff --git a/app/workers/process_references_worker.rb b/app/workers/process_references_worker.rb index 26dfbae465..f082744857 100644 --- a/app/workers/process_references_worker.rb +++ b/app/workers/process_references_worker.rb @@ -5,8 +5,8 @@ class ProcessReferencesWorker sidekiq_options queue: 'pull', retry: 3 - def perform(status_id, ids, urls, no_fetch_urls = nil, quote_urls = nil) - ProcessReferencesService.new.call(Status.find(status_id), ids || [], urls: urls || [], no_fetch_urls: no_fetch_urls, quote_urls: quote_urls || []) + def perform(status_id, ids, urls, no_fetch_urls = nil) + ProcessReferencesService.new.call(Status.find(status_id), ids || [], urls: urls || [], no_fetch_urls: no_fetch_urls) rescue ActiveRecord::RecordNotFound true end diff --git a/spec/services/process_references_service_spec.rb b/spec/services/process_references_service_spec.rb index e027970103..f7b0976494 100644 --- a/spec/services/process_references_service_spec.rb +++ b/spec/services/process_references_service_spec.rb @@ -10,7 +10,6 @@ RSpec.describe ProcessReferencesService, type: :service do let(:status) { Fabricate(:status, account: account, text: text, visibility: visibility) } let(:target_status) { Fabricate(:status, account: Fabricate(:user).account, visibility: target_status_visibility) } let(:target_status_uri) { ActivityPub::TagManager.instance.uri_for(target_status) } - let(:quote_urls) { nil } def notify?(target_status_id = nil) target_status_id ||= target_status.id @@ -21,7 +20,7 @@ RSpec.describe ProcessReferencesService, type: :service do subject do target_status.account.user&.save - described_class.new.call(status, reference_parameters, urls: urls, fetch_remote: fetch_remote, quote_urls: quote_urls) + described_class.new.call(status, reference_parameters, urls: urls, fetch_remote: fetch_remote) status.reference_objects.pluck(:target_status_id, :attribute_type) end @@ -39,7 +38,7 @@ RSpec.describe ProcessReferencesService, type: :service do expect(notify?).to be true end - it 'not quote', :inline_jobs do + it 'not legacy quote', :inline_jobs do expect(status.quote).to be_nil end end @@ -80,96 +79,21 @@ RSpec.describe ProcessReferencesService, type: :service do end end - context 'with quote' do + context 'without kmyblue legacy quote' do let(:text) { "Hello QT #{target_status_uri}" } it 'post status', :inline_jobs do expect(subject.size).to eq 1 expect(subject.pluck(0)).to include target_status.id expect(subject.pluck(1)).to include 'QT' - # TODO: quote - # expect(status.quote).to_not be_nil - # expect(status.quote.id).to eq target_status.id + + # it's not kmyblue legacy quote + expect(status.quote).to be_nil + expect(notify?).to be true end end - context 'with quote as parameter only' do - let(:text) { 'Hello' } - let(:quote_urls) { [ActivityPub::TagManager.instance.uri_for(target_status)] } - - it 'post status', :inline_jobs do - expect(subject.size).to eq 1 - expect(subject.pluck(0)).to include target_status.id - expect(subject.pluck(1)).to include 'QT' - # TODO: quote - # expect(status.quote).to_not be_nil - # expect(status.quote.id).to eq target_status.id - expect(notify?).to be true - end - end - - context 'with quote as parameter and embed' do - let(:text) { "Hello QT #{target_status_uri}" } - let(:quote_urls) { [ActivityPub::TagManager.instance.uri_for(target_status)] } - - it 'post status', :inline_jobs do - expect(subject.size).to eq 1 - expect(subject.pluck(0)).to include target_status.id - expect(subject.pluck(1)).to include 'QT' - # TODO: quote - # expect(status.quote).to_not be_nil - # expect(status.quote.id).to eq target_status.id - expect(notify?).to be true - end - end - - context 'with quote as parameter but embed is not quote' do - let(:text) { "Hello RE #{target_status_uri}" } - let(:quote_urls) { [ActivityPub::TagManager.instance.uri_for(target_status)] } - - it 'post status', :inline_jobs do - expect(subject.size).to eq 1 - expect(subject.pluck(0)).to include target_status.id - expect(subject.pluck(1)).to include 'QT' - # TODO: quote - # expect(status.quote).to_not be_nil - # expect(status.quote.id).to eq target_status.id - expect(notify?).to be true - end - end - - context 'when quote is rejected' do - let(:text) { "Hello QT #{target_status_uri}" } - # let(:allow_quote) { false } - - it 'post status', :inline_jobs do - expect(subject.size).to eq 1 - expect(subject.pluck(0)).to include target_status.id - # TODO: quote - # expect(subject.pluck(1)).to include 'BT' - # expect(status.quote).to be_nil - expect(notify?).to be true - end - end - - context 'with quote and reference' do - let(:target_status2) { Fabricate(:status) } - let(:target_status2_uri) { ActivityPub::TagManager.instance.uri_for(target_status2) } - let(:text) { "Hello QT #{target_status_uri}\nBT #{target_status2_uri}" } - - it 'post status', :inline_jobs do - expect(subject.size).to eq 2 - expect(subject).to include [target_status.id, 'QT'] - expect(subject).to include [target_status2.id, 'BT'] - # TODO: quote - # expect(status.quote).to_not be_nil - # expect(status.quote.id).to eq target_status.id - expect(notify?).to be true - expect(notify?(target_status2.id)).to be true - end - end - context 'when url only' do let(:text) { "Hello #{target_status_uri}" } @@ -409,10 +333,17 @@ RSpec.describe ProcessReferencesService, type: :service do let(:text) { "QT #{target_status_uri}" } let(:new_text) { 'Hello' } + before do + Quote.create!(status: status, quoted_status: target_status) + end + it 'post status', :inline_jobs do expect(subject.size).to eq 0 - # TODO: quote - # expect(status.quote).to be_nil + + # doesn't remove original Mastodon's quote + expect(status.quote).to_not be_nil + expect(status.quote.quoted_status_id).to eq target_status.id + expect(notify?).to be false end end @@ -428,43 +359,13 @@ RSpec.describe ProcessReferencesService, type: :service do end end - context 'when change quote' do - let(:text) { "QT #{target_status_uri}" } - let(:new_text) { "QT #{target_status2_uri}" } - - it 'post status', :inline_jobs do - expect(subject.size).to eq 1 - expect(subject).to include target_status2.id - # TODO: quote - # expect(status.quote).to_not be_nil - # expect(status.quote.id).to eq target_status2.id - expect(notify?(target_status2.id)).to be true - end - end - - context 'when change quote to reference' do + context 'when change reference attribute' do let(:text) { "QT #{target_status_uri}" } let(:new_text) { "RT #{target_status_uri}" } it 'post status', :inline_jobs do expect(subject.size).to eq 1 expect(subject).to include target_status.id - # TODO: quote - # expect(status.quote).to be_nil - expect(notify?(target_status.id)).to be true - end - end - - context 'when change reference to quote' do - let(:text) { "RT #{target_status_uri}" } - let(:new_text) { "QT #{target_status_uri}" } - - it 'post status', :inline_jobs do - expect(subject.size).to eq 1 - expect(subject).to include target_status.id - # TODO: quote - # expect(status.quote).to_not be_nil - # expect(status.quote.id).to eq target_status.id expect(notify?(target_status.id)).to be true end end