Fix: 一部サーバーの投稿が参照引用できない問題・参照引用時、フェッチを基本しないよう変更 (#360)

app/lib/activitypub/activity/create.rb

@@ -544,7 +544,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
     url = quote
 
     if url.present?
-      ResolveURLService.new.call(url, on_behalf_of: @account, local_only: true).present?
+      ActivityPub::TagManager.instance.uri_to_resource(url, Status)&.local?
     else
       false
     end

app/lib/activitypub/tag_manager.rb

@@ -208,7 +208,7 @@ class ActivityPub::TagManager
     uri_to_resource(uri, Account)
   end
 
-  def uri_to_resource(uri, klass)
+  def uri_to_resource(uri, klass, url: false)
     return if uri.nil?
 
     if local_uri?(uri)
@@ -221,7 +221,9 @@ class ActivityPub::TagManager
     elsif OStatus::TagManager.instance.local_id?(uri)
       klass.find_by(id: OStatus::TagManager.instance.unique_tag_to_local_id(uri, klass.to_s))
     else
-      klass.find_by(uri: uri.split('#').first)
+      resource   = klass.find_by(uri: uri.split('#').first)
+      resource ||= klass.where('uri != url').find_by(url: uri.split('#').first) if url
+      resource
     end
   rescue ActiveRecord::RecordNotFound
     nil

app/services/process_references_service.rb

@@ -141,11 +141,20 @@ class ProcessReferencesService < BaseService
   end
 
   def url_to_status(url)
-    ResolveURLService.new.call(url, on_behalf_of: @status.account, fetch_remote: @fetch_remote && @no_fetch_urls.exclude?(url))
+    status   = ActivityPub::TagManager.instance.uri_to_resource(url, Status, url: true)
+    status ||= ResolveURLService.new.call(url, on_behalf_of: @status.account) if @fetch_remote && @no_fetch_urls.exclude?(url)
+    referrable?(status) ? status : nil
+  end
+
+  def referrable?(target_status)
+    return false if target_status.nil?
+    return @referrable if defined?(@referrable)
+
+    @referrable = StatusPolicy.new(@status.account, target_status).show?
   end
 
   def quotable?(target_status)
-    target_status.account.allow_quote? && (!@status.local? || StatusPolicy.new(@status.account, target_status).quote?)
+    target_status.account.allow_quote? && StatusPolicy.new(@status.account, target_status).quote?
   end
 
   def add_references

app/services/resolve_url_service.rb

@@ -6,16 +6,15 @@ class ResolveURLService < BaseService
 
   USERNAME_STATUS_RE = %r{/@(?<username>#{Account::USERNAME_RE})/(?<status_id>[0-9]+)\Z}
 
-  def call(url, on_behalf_of: nil, fetch_remote: true, local_only: false)
+  def call(url, on_behalf_of: nil)
     @url          = url
     @on_behalf_of = on_behalf_of
-    @fetch_remote = fetch_remote
 
     if local_url?
       process_local_url
-    elsif !local_only && fetch_remote && !fetched_resource.nil?
+    elsif !fetched_resource.nil?
       process_url
-    elsif !local_only
+    else
       process_url_from_db
     end
   end
@@ -38,7 +37,7 @@ class ResolveURLService < BaseService
       return account unless account.nil?
     end
 
-    return unless @on_behalf_of.present? && (!@fetch_remote || [401, 403, 404].include?(fetch_resource_service.response_code))
+    return unless @on_behalf_of.present? && [401, 403, 404].include?(fetch_resource_service.response_code)
 
     # It may happen that the resource is a private toot, and thus not fetchable,
     # but we can return the toot if we already know about it.

db/migrate/20231214225249_index_to_statuses_url.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class IndexToStatusesURL < ActiveRecord::Migration[7.1]
+  disable_ddl_transaction!
+
+  def up
+    add_index :statuses, :url, name: :index_statuses_on_url, algorithm: :concurrently, opclass: :text_pattern_ops, where: 'url IS NOT NULL AND url <> uri'
+  end
+
+  def down
+    remove_index :statuses, name: :index_statuses_on_url
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2023_12_12_225737) do
+ActiveRecord::Schema[7.1].define(version: 2023_12_14_225249) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -1247,6 +1247,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_12_12_225737) do
     t.index ["quote_of_id", "account_id"], name: "index_statuses_on_quote_of_id_and_account_id"
     t.index ["reblog_of_id", "account_id"], name: "index_statuses_on_reblog_of_id_and_account_id"
     t.index ["uri"], name: "index_statuses_on_uri", unique: true, opclass: :text_pattern_ops, where: "(uri IS NOT NULL)"
+    t.index ["url"], name: "index_statuses_on_url", opclass: :text_pattern_ops, where: "((url IS NOT NULL) AND ((url)::text <> (uri)::text))"
   end
 
   create_table "statuses_tags", primary_key: ["tag_id", "status_id"], force: :cascade do |t|

spec/services/process_references_service_spec.rb

@@ -258,10 +258,7 @@ RSpec.describe ProcessReferencesService, type: :service do
       end
       let(:text) { 'BT:https://example.com/test_post' }
 
-      before do
+      shared_examples 'reference once' do |uri, url|
-        stub_request(:get, 'https://example.com/test_post').to_return(status: 200, body: Oj.dump(object_json), headers: { 'Content-Type' => 'application/activity+json' })
-      end
-
         it 'reference it' do
           expect(subject.size).to eq 1
           expect(subject[0][1]).to eq 'BT'
@@ -269,7 +266,49 @@ RSpec.describe ProcessReferencesService, type: :service do
           status = Status.find_by(id: subject[0][0])
           expect(status).to_not be_nil
           expect(status.id).to eq remote_status.id
-        expect(status.url).to eq 'https://example.com/test_post'
+          expect(status.uri).to eq uri
+          expect(status.url).to eq url
+        end
+      end
+
+      before do
+        stub_request(:get, 'https://example.com/test_post').to_return(status: 200, body: Oj.dump(object_json), headers: { 'Content-Type' => 'application/activity+json' })
+      end
+
+      it_behaves_like 'reference once', 'https://example.com/test_post', 'https://example.com/test_post'
+
+      context 'when uri and url is difference and url is not accessable' do
+        let(:remote_status) { Fabricate(:status, account: account, uri: 'https://example.com/test_post', url: 'https://example.com/test_post_ohagi', text: 'Lorem ipsum') }
+        let(:text) { 'BT:https://example.com/test_post_ohagi' }
+        let(:object_json) do
+          {
+            id: 'https://example.com/test_post',
+            url: 'https://example.com/test_post_ohagi',
+            to: ActivityPub::TagManager::COLLECTIONS[:public],
+            '@context': ActivityPub::TagManager::CONTEXT,
+            type: 'Note',
+            actor: account.uri,
+            attributedTo: account.uri,
+            content: 'Lorem ipsum',
+          }
+        end
+
+        before do
+          stub_request(:get, 'https://example.com/test_post_ohagi').to_return(status: 404)
+        end
+
+        it_behaves_like 'reference once', 'https://example.com/test_post', 'https://example.com/test_post_ohagi'
+
+        it 'do not request to uri' do
+          subject
+          expect(a_request(:get, 'https://example.com/test_post_ohagi')).to_not have_been_made
+        end
+
+        context 'when url and uri is specified at the same time' do
+          let(:text) { 'BT:https://example.com/test_post_ohagi BT:https://example.com/test_post' }
+
+          it_behaves_like 'reference once', 'https://example.com/test_post', 'https://example.com/test_post_ohagi'
+        end
       end
     end
   end