From c61e356475cf5df85f067a951abf404b93795f19 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 27 May 2024 05:49:44 -0400 Subject: [PATCH] Add `Status::MEDIA_ATTACHMENTS_LIMIT` configuration constant (#30433) --- app/lib/activitypub/activity/create.rb | 4 ++-- app/models/status.rb | 2 ++ app/serializers/rest/instance_serializer.rb | 2 +- app/serializers/rest/v1/instance_serializer.rb | 2 +- app/services/activitypub/process_status_update_service.rb | 2 +- app/services/post_status_service.rb | 4 ++-- app/services/update_status_service.rb | 4 ++-- spec/requests/api/v2/instance_spec.rb | 2 +- spec/services/post_status_service_spec.rb | 5 +++-- 9 files changed, 15 insertions(+), 12 deletions(-) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 85195f4c3..7ec7e84bd 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -110,7 +110,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def process_status_params @status_parser = ActivityPub::Parser::StatusParser.new(@json, followers_collection: @account.followers_url, object: @object) - attachment_ids = process_attachments.take(4).map(&:id) + attachment_ids = process_attachments.take(Status::MEDIA_ATTACHMENTS_LIMIT).map(&:id) @params = { uri: @status_parser.uri, @@ -260,7 +260,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity as_array(@object['attachment']).each do |attachment| media_attachment_parser = ActivityPub::Parser::MediaAttachmentParser.new(attachment) - next if media_attachment_parser.remote_url.blank? || media_attachments.size >= 4 + next if media_attachment_parser.remote_url.blank? || media_attachments.size >= Status::MEDIA_ATTACHMENTS_LIMIT begin media_attachment = MediaAttachment.create( diff --git a/app/models/status.rb b/app/models/status.rb index 72a8d6c40..9d09fa5fe 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -39,6 +39,8 @@ class Status < ApplicationRecord include Status::SnapshotConcern include Status::ThreadingConcern + MEDIA_ATTACHMENTS_LIMIT = 4 + rate_limit by: :account, family: :statuses self.discard_column = :deleted_at diff --git a/app/serializers/rest/instance_serializer.rb b/app/serializers/rest/instance_serializer.rb index 42b73f438..8df79db6c 100644 --- a/app/serializers/rest/instance_serializer.rb +++ b/app/serializers/rest/instance_serializer.rb @@ -59,7 +59,7 @@ class REST::InstanceSerializer < ActiveModel::Serializer statuses: { max_characters: StatusLengthValidator::MAX_CHARS, - max_media_attachments: 4, + max_media_attachments: Status::MEDIA_ATTACHMENTS_LIMIT, characters_reserved_per_url: StatusLengthValidator::URL_PLACEHOLDER_CHARS, }, diff --git a/app/serializers/rest/v1/instance_serializer.rb b/app/serializers/rest/v1/instance_serializer.rb index fdf939cfc..636925b97 100644 --- a/app/serializers/rest/v1/instance_serializer.rb +++ b/app/serializers/rest/v1/instance_serializer.rb @@ -64,7 +64,7 @@ class REST::V1::InstanceSerializer < ActiveModel::Serializer statuses: { max_characters: StatusLengthValidator::MAX_CHARS, - max_media_attachments: 4, + max_media_attachments: Status::MEDIA_ATTACHMENTS_LIMIT, characters_reserved_per_url: StatusLengthValidator::URL_PLACEHOLDER_CHARS, }, diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index fb2b33114..1dbed27f2 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -73,7 +73,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService as_array(@json['attachment']).each do |attachment| media_attachment_parser = ActivityPub::Parser::MediaAttachmentParser.new(attachment) - next if media_attachment_parser.remote_url.blank? || @next_media_attachments.size > 4 + next if media_attachment_parser.remote_url.blank? || @next_media_attachments.size > Status::MEDIA_ATTACHMENTS_LIMIT begin media_attachment = previous_media_attachments.find { |previous_media_attachment| previous_media_attachment.remote_url == media_attachment_parser.remote_url } diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 22a6a24af..83a931817 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -130,9 +130,9 @@ class PostStatusService < BaseService return end - raise Mastodon::ValidationError, I18n.t('media_attachments.validations.too_many') if @options[:media_ids].size > 4 || @options[:poll].present? + raise Mastodon::ValidationError, I18n.t('media_attachments.validations.too_many') if @options[:media_ids].size > Status::MEDIA_ATTACHMENTS_LIMIT || @options[:poll].present? - @media = @account.media_attachments.where(status_id: nil).where(id: @options[:media_ids].take(4).map(&:to_i)) + @media = @account.media_attachments.where(status_id: nil).where(id: @options[:media_ids].take(Status::MEDIA_ATTACHMENTS_LIMIT).map(&:to_i)) raise Mastodon::ValidationError, I18n.t('media_attachments.validations.images_and_video') if @media.size > 1 && @media.find(&:audio_or_video?) raise Mastodon::ValidationError, I18n.t('media_attachments.validations.not_ready') if @media.any?(&:not_processed?) diff --git a/app/services/update_status_service.rb b/app/services/update_status_service.rb index cdfe28365..dc7d177e2 100644 --- a/app/services/update_status_service.rb +++ b/app/services/update_status_service.rb @@ -69,9 +69,9 @@ class UpdateStatusService < BaseService def validate_media! return [] if @options[:media_ids].blank? || !@options[:media_ids].is_a?(Enumerable) - raise Mastodon::ValidationError, I18n.t('media_attachments.validations.too_many') if @options[:media_ids].size > 4 || @options[:poll].present? + raise Mastodon::ValidationError, I18n.t('media_attachments.validations.too_many') if @options[:media_ids].size > Status::MEDIA_ATTACHMENTS_LIMIT || @options[:poll].present? - media_attachments = @status.account.media_attachments.where(status_id: [nil, @status.id]).where(scheduled_status_id: nil).where(id: @options[:media_ids].take(4).map(&:to_i)).to_a + media_attachments = @status.account.media_attachments.where(status_id: [nil, @status.id]).where(scheduled_status_id: nil).where(id: @options[:media_ids].take(Status::MEDIA_ATTACHMENTS_LIMIT).map(&:to_i)).to_a raise Mastodon::ValidationError, I18n.t('media_attachments.validations.images_and_video') if media_attachments.size > 1 && media_attachments.find(&:audio_or_video?) raise Mastodon::ValidationError, I18n.t('media_attachments.validations.not_ready') if media_attachments.any?(&:not_processed?) diff --git a/spec/requests/api/v2/instance_spec.rb b/spec/requests/api/v2/instance_spec.rb index c5c6a26f4..5c464f09a 100644 --- a/spec/requests/api/v2/instance_spec.rb +++ b/spec/requests/api/v2/instance_spec.rb @@ -45,7 +45,7 @@ describe 'Instances' do ), statuses: include( max_characters: StatusLengthValidator::MAX_CHARS, - max_media_attachments: 4 # TODO, move to constant somewhere + max_media_attachments: Status::MEDIA_ATTACHMENTS_LIMIT ), polls: include( max_options: PollValidator::MAX_OPTIONS diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index 18891bf11..11bf4c30e 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -228,14 +228,15 @@ RSpec.describe PostStatusService do expect(media.reload.status).to be_nil end - it 'does not allow attaching more than 4 files' do + it 'does not allow attaching more files than configured limit' do + stub_const('Status::MEDIA_ATTACHMENTS_LIMIT', 1) account = Fabricate(:account) expect do subject.call( account, text: 'test status update', - media_ids: Array.new(5) { Fabricate(:media_attachment, account: account) }.map(&:id) + media_ids: Array.new(2) { Fabricate(:media_attachment, account: account) }.map(&:id) ) end.to raise_error( Mastodon::ValidationError,