From cbd4b7719a05c87f5e8db3dfbe9ff62f4a8cacac Mon Sep 17 00:00:00 2001 From: KMY Date: Sun, 1 Oct 2023 09:49:42 +0900 Subject: [PATCH] =?UTF-8?q?=E5=BC=95=E7=94=A8API=E3=82=92=E4=BD=9C?= =?UTF-8?q?=E6=88=90=E3=80=81=E3=81=A4=E3=81=84=E3=81=A7=E3=81=AB=E3=83=96?= =?UTF-8?q?=E3=83=AD=E3=83=83=E3=82=AF=E7=8A=B6=E6=B3=81=E3=82=92=E5=BC=95?= =?UTF-8?q?=E7=94=A8API=E3=81=AB=E5=8F=8D=E6=98=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../v1/statuses/emoji_reactions_controller.rb | 2 +- app/models/status.rb | 10 ++-- app/models/status_reference.rb | 1 + app/policies/status_policy.rb | 4 ++ .../status_relationships_presenter.rb | 7 ++- app/serializers/rest/status_serializer.rb | 23 ++++++++- app/services/process_references_service.rb | 39 ++++++++++----- app/workers/process_references_worker.rb | 4 +- ...30233930_add_quote_to_status_references.rb | 24 +++++++++ db/schema.rb | 3 +- .../status_reference_fabricator.rb | 8 +++ spec/models/status_spec.rb | 49 +++++++++++++++++++ spec/policies/status_policy_spec.rb | 42 ++++++++++++++++ .../process_references_service_spec.rb | 4 ++ 14 files changed, 195 insertions(+), 25 deletions(-) create mode 100644 db/migrate/20230930233930_add_quote_to_status_references.rb create mode 100644 spec/fabricators/status_reference_fabricator.rb diff --git a/app/controllers/api/v1/statuses/emoji_reactions_controller.rb b/app/controllers/api/v1/statuses/emoji_reactions_controller.rb index 4dc4bd92c8aa56..f437576d1b8abc 100644 --- a/app/controllers/api/v1/statuses/emoji_reactions_controller.rb +++ b/app/controllers/api/v1/statuses/emoji_reactions_controller.rb @@ -31,7 +31,7 @@ def destroy end render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new( - [@status], current_account.id, emoji_reactions_map: { @status.id => false } + [@status], current_account.id ) rescue Mastodon::NotPermittedError not_found diff --git a/app/models/status.rb b/app/models/status.rb index c16c1a7c4f7f17..0111f3d7050abb 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -228,7 +228,7 @@ def reblog? end def quote - reference_objects.where(attribute_type: 'QT').first&.target_status + @quote ||= reference_objects.where(quote: true).first&.target_status end def within_realtime_window? @@ -480,12 +480,12 @@ def mutes_map(conversation_ids, account_id) ConversationMute.select('conversation_id').where(conversation_id: conversation_ids).where(account_id: account_id).each_with_object({}) { |m, h| h[m.conversation_id] = true } end - def pins_map(status_ids, account_id) - StatusPin.select('status_id').where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |p, h| h[p.status_id] = true } + def blocks_map(account_ids, account_id) + Block.where(account_id: account_id, target_account_id: account_ids).each_with_object({}) { |b, h| h[b.target_account_id] = true } end - def emoji_reactions_map(status_ids, account_id) - EmojiReaction.select('status_id').where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |e, h| h[e.status_id] = true } + def pins_map(status_ids, account_id) + StatusPin.select('status_id').where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |p, h| h[p.status_id] = true } end def emoji_reaction_allows_map(status_ids, account_id) diff --git a/app/models/status_reference.rb b/app/models/status_reference.rb index 8d5d6eba8b9961..919338929965b3 100644 --- a/app/models/status_reference.rb +++ b/app/models/status_reference.rb @@ -10,6 +10,7 @@ # created_at :datetime not null # updated_at :datetime not null # attribute_type :string +# quote :boolean default(FALSE), not null # class StatusReference < ApplicationRecord diff --git a/app/policies/status_policy.rb b/app/policies/status_policy.rb index 301ec4fdc48213..b30d48e37443e0 100644 --- a/app/policies/status_policy.rb +++ b/app/policies/status_policy.rb @@ -40,6 +40,10 @@ def emoji_reaction? show? && !blocking_author? end + def quote? + %i(public public_unlisted unlisted).include?(record.visibility.to_sym) && show? && !blocking_author? + end + def destroy? owned? end diff --git a/app/presenters/status_relationships_presenter.rb b/app/presenters/status_relationships_presenter.rb index 9e55742403dc0b..b2c5e8c26989d9 100644 --- a/app/presenters/status_relationships_presenter.rb +++ b/app/presenters/status_relationships_presenter.rb @@ -3,8 +3,8 @@ class StatusRelationshipsPresenter PINNABLE_VISIBILITIES = %w(public public_unlisted unlisted login private).freeze - attr_reader :reblogs_map, :favourites_map, :mutes_map, :pins_map, - :bookmarks_map, :filters_map, :emoji_reactions_map, :attributes_map, :emoji_reaction_allows_map + attr_reader :reblogs_map, :favourites_map, :mutes_map, :pins_map, :blocks_map, + :bookmarks_map, :filters_map, :attributes_map, :emoji_reaction_allows_map def initialize(statuses, current_account_id = nil, **options) @current_account_id = current_account_id @@ -16,7 +16,6 @@ def initialize(statuses, current_account_id = nil, **options) @mutes_map = {} @pins_map = {} @filters_map = {} - @emoji_reactions_map = {} @emoji_reaction_allows_map = nil else statuses = statuses.compact @@ -29,8 +28,8 @@ def initialize(statuses, current_account_id = nil, **options) @favourites_map = Status.favourites_map(status_ids, current_account_id).merge(options[:favourites_map] || {}) @bookmarks_map = Status.bookmarks_map(status_ids, current_account_id).merge(options[:bookmarks_map] || {}) @mutes_map = Status.mutes_map(conversation_ids, current_account_id).merge(options[:mutes_map] || {}) + @blocks_map = Status.blocks_map(conversation_ids, current_account_id).merge(options[:blocks_map] || {}) @pins_map = Status.pins_map(pinnable_status_ids, current_account_id).merge(options[:pins_map] || {}) - @emoji_reactions_map = Status.emoji_reactions_map(status_ids, current_account_id).merge(options[:emoji_reactions_map] || {}) @emoji_reaction_allows_map = Status.emoji_reaction_allows_map(status_ids, current_account_id).merge(options[:emoji_reaction_allows_map] || {}) @attributes_map = options[:attributes_map] || {} end diff --git a/app/serializers/rest/status_serializer.rb b/app/serializers/rest/status_serializer.rb index 3603026f39bd03..0987c3e0895c89 100644 --- a/app/serializers/rest/status_serializer.rb +++ b/app/serializers/rest/status_serializer.rb @@ -5,7 +5,7 @@ class REST::StatusSerializer < ActiveModel::Serializer attributes :id, :created_at, :in_reply_to_id, :in_reply_to_account_id, :sensitive, :spoiler_text, :visibility, :visibility_ex, :limited_scope, :language, - :uri, :url, :replies_count, :reblogs_count, :searchability, :markdown, + :uri, :url, :replies_count, :reblogs_count, :searchability, :markdown, :quote_id, :status_reference_ids, :status_references_count, :status_referred_by_count, :favourites_count, :emoji_reactions, :emoji_reactions_count, :reactions, :edited_at @@ -33,6 +33,23 @@ class REST::StatusSerializer < ActiveModel::Serializer has_one :preview_card, key: :card, serializer: REST::PreviewCardSerializer has_one :preloadable_poll, key: :poll, serializer: REST::PollSerializer + class QuotedStatusSerializer < REST::StatusSerializer + attribute :blocked, if: :current_user? + + def quote + nil + end + + def blocked + if relationships + relationships.blocks_map[object.account_id] || false + else + current_user.account.blocking?(object.account_id) + end + end + end + belongs_to :quote, serializer: QuotedStatusSerializer + def id object.id.to_s end @@ -159,6 +176,10 @@ def reactions end end + def quote_id + object.quote&.id + end + def reblogged if relationships relationships.reblogs_map[object.id] || false diff --git a/app/services/process_references_service.rb b/app/services/process_references_service.rb index faf5965a7aef05..e1ac835bd39fdc 100644 --- a/app/services/process_references_service.rb +++ b/app/services/process_references_service.rb @@ -10,14 +10,17 @@ class ProcessReferencesService < BaseService REFURL_EXP = /(RT|QT|BT|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) + def call(status, reference_parameters, urls: nil, fetch_remote: true, no_fetch_urls: nil, quote_urls: nil) @status = status @reference_parameters = reference_parameters || [] - @urls = urls || [] + @quote_urls = quote_urls || [] + @urls = (urls - @quote_urls) || [] @no_fetch_urls = no_fetch_urls || [] @fetch_remote = fetch_remote @again = false + @attributes = {} + with_redis_lock("process_status_refs:#{@status.id}") do @references_count = old_references.size @@ -28,7 +31,7 @@ def call(status, reference_parameters, urls: nil, fetch_remote: true, no_fetch_u @status.save! end - + quote_ur create_notifications! end @@ -42,23 +45,23 @@ 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.perform_worker_async(status, reference_parameters, urls) + def self.perform_worker_async(status, reference_parameters, urls, quote_urls) return unless need_process?(status, reference_parameters, urls) Rails.cache.write("status_reference:#{status.id}", true, expires_in: 10.minutes) - ProcessReferencesWorker.perform_async(status.id, reference_parameters, urls, []) + ProcessReferencesWorker.perform_async(status.id, reference_parameters, urls, [], quote_urls || []) end - def self.call_service(status, reference_parameters, urls) + def self.call_service(status, reference_parameters, urls, quote_urls) return unless need_process?(status, reference_parameters, urls) - ProcessReferencesService.new.call(status, reference_parameters || [], urls: urls || [], fetch_remote: false) + ProcessReferencesService.new.call(status, reference_parameters || [], urls: urls || [], fetch_remote: false, quote_urls: quote_urls) end private def references - @references ||= @reference_parameters + scan_text! + @references ||= @reference_parameters + scan_text! + quote_status_ids end def old_references @@ -88,12 +91,24 @@ def fetch_statuses!(urls) target_urls = urls + @urls target_urls.map do |url| - status = ResolveURLService.new.call(url, on_behalf_of: @status.account, fetch_remote: @fetch_remote && @no_fetch_urls.exclude?(url)) + status = url_to_status(url) @no_fetch_urls << url if !@fetch_remote && status.present? && status.local? status end end + def url_to_status(url) + ResolveURLService.new.call(url, on_behalf_of: @status.account, fetch_remote: @fetch_remote && @no_fetch_urls.exclude?(url)) + end + + def quote_status_ids + @quote_status_ids ||= @quote_urls.filter_map { |url| url_to_status(url) }.map(&:id) + end + + def quotable?(target_status) + StatusPolicy.new(@status.account, target_status).quote? + end + def add_references return if added_references.empty? @@ -101,7 +116,9 @@ def add_references statuses = Status.where(id: added_references) statuses.each do |status| - @added_objects << @status.reference_objects.new(target_status: status, attribute_type: @attributes[status.id]) + attribute_type = quote_status_ids.include?(status.id) ? 'QT' : @attributes[status.id] + attribute_type = 'BT' unless quotable?(status) + @added_objects << @status.reference_objects.new(target_status: status, attribute_type: attribute_type, quote: attribute_type.casecmp('QT').zero?) status.increment_count!(:status_referred_by_count) @references_count += 1 @@ -133,6 +150,6 @@ def remove_old_references end def launch_worker - ProcessReferencesWorker.perform_async(@status.id, @reference_parameters, @urls, @no_fetch_urls) + ProcessReferencesWorker.perform_async(@status.id, @reference_parameters, @urls, @no_fetch_urls, @quote_urls) end end diff --git a/app/workers/process_references_worker.rb b/app/workers/process_references_worker.rb index f082744857ad61..26dfbae465aeb6 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) - ProcessReferencesService.new.call(Status.find(status_id), ids || [], urls: urls || [], no_fetch_urls: no_fetch_urls) + 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 || []) rescue ActiveRecord::RecordNotFound true end diff --git a/db/migrate/20230930233930_add_quote_to_status_references.rb b/db/migrate/20230930233930_add_quote_to_status_references.rb new file mode 100644 index 00000000000000..f2bd6cd48d8402 --- /dev/null +++ b/db/migrate/20230930233930_add_quote_to_status_references.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require Rails.root.join('lib', 'mastodon', 'migration_helpers') + +class AddQuoteToStatusReferences < ActiveRecord::Migration[7.0] + include Mastodon::MigrationHelpers + + disable_ddl_transaction! + + class StatusReference < ApplicationRecord; end + + def up + safety_assured do + add_column_with_default :status_references, :quote, :boolean, default: false, allow_null: false + StatusReference.where(attribute_type: 'QT').update_all(quote: true) # rubocop:disable Rails/SkipsModelValidations + end + end + + def down + safety_assured do + remove_column :status_references, :quote + end + end +end diff --git a/db/schema.rb b/db/schema.rb index e17320cdaa0637..3801693f8561e4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_09_23_103430) do +ActiveRecord::Schema[7.0].define(version: 2023_09_30_233930) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1143,6 +1143,7 @@ t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.string "attribute_type" + t.boolean "quote", default: false, null: false t.index ["status_id"], name: "index_status_references_on_status_id" t.index ["target_status_id"], name: "index_status_references_on_target_status_id" end diff --git a/spec/fabricators/status_reference_fabricator.rb b/spec/fabricators/status_reference_fabricator.rb new file mode 100644 index 00000000000000..0eff89c14ba727 --- /dev/null +++ b/spec/fabricators/status_reference_fabricator.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +Fabricator(:status_reference) do + status { Fabricate.build(:status) } + target_status { Fabricate.build(:status) } + attribute_type 'BT' + quote false +end diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index 64623075943b37..58433a701954d7 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -217,6 +217,39 @@ end end + describe '#quote' do + let(:target_status) { Fabricate(:status) } + let(:quote) { true } + + before do + Fabricate(:status_reference, status: subject, target_status: target_status, quote: quote) + end + + context 'when quoting single' do + it 'get quote' do + expect(subject.quote).to_not be_nil + expect(subject.quote.id).to eq target_status.id + end + end + + context 'when multiple quotes' do + it 'get quote' do + target2 = Fabricate(:status) + Fabricate(:status_reference, status: subject, quote: quote) + expect(subject.quote).to_not be_nil + expect([target_status.id, target2.id].include?(subject.quote.id)).to be true + end + end + + context 'when no quote but reference' do + let(:quote) { false } + + it 'get quote' do + expect(subject.quote).to be_nil + end + end + end + describe '#content' do it 'returns the text of the status if it is not a reblog' do expect(subject.content).to eql subject.text @@ -324,6 +357,22 @@ end end + describe '.blocks_map' do + subject { described_class.blocks_map([status.account.id], account) } + + let(:status) { Fabricate(:status) } + let(:account) { Fabricate(:account) } + + it 'returns a hash' do + expect(subject).to be_a Hash + end + + it 'contains true value' do + account.block!(status.account) + expect(subject[status.account.id]).to be true + end + end + describe '.favourites_map' do subject { described_class.favourites_map([status], account) } diff --git a/spec/policies/status_policy_spec.rb b/spec/policies/status_policy_spec.rb index 271c70804bb96b..3bdc2084d8a7be 100644 --- a/spec/policies/status_policy_spec.rb +++ b/spec/policies/status_policy_spec.rb @@ -167,6 +167,48 @@ end end + context 'with the permission of emoji_reaction?' do + permissions :emoji_reaction? do + it 'grants access when viewer is not blocked' do + follow = Fabricate(:follow) + status.account = follow.target_account + + expect(subject).to permit(follow.account, status) + end + + it 'denies when viewer is blocked' do + block = Fabricate(:block) + status.account = block.target_account + + expect(subject).to_not permit(block.account, status) + end + end + end + + context 'with the permission of quote?' do + permissions :quote? do + it 'grants access when viewer is not blocked' do + follow = Fabricate(:follow) + status.account = follow.target_account + + expect(subject).to permit(follow.account, status) + end + + it 'denies when viewer is blocked' do + block = Fabricate(:block) + status.account = block.target_account + + expect(subject).to_not permit(block.account, status) + end + + it 'denies when private visibility' do + status.visibility = :private + + expect(subject).to_not permit(Fabricate(:account), status) + end + end + end + context 'with the permission of update?' do permissions :update? do it 'grants access if owner' do diff --git a/spec/services/process_references_service_spec.rb b/spec/services/process_references_service_spec.rb index c41144a2aaacfa..305df945845438 100644 --- a/spec/services/process_references_service_spec.rb +++ b/spec/services/process_references_service_spec.rb @@ -35,6 +35,10 @@ def notify?(target_status_id = nil) expect(subject.pluck(1)).to include 'RT' expect(notify?).to be true end + + it 'not quote' do + expect(status.quote).to be_nil + end end context 'when multiple references' do