From d17fb7013116767fc5c7d5eef63218bd8c45b023 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 9 Mar 2022 09:06:17 +0100 Subject: Change how changes to media attachments are stored for edits (#17696) * Change how changes to media attachments are stored for edits Fix not being able to re-order media attachments * Fix not broadcasting updates when polls/media is changed through ActivityPub * Various fixes and improvements * Update app/models/report.rb Co-authored-by: Claire * Add tracking of media attachment description changes * Change poll in status edit to have a structure closer to the real one Co-authored-by: Claire --- app/models/status.rb | 63 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 25 deletions(-) (limited to 'app/models/status.rb') diff --git a/app/models/status.rb b/app/models/status.rb index af3e645dc..db10eedc2 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -3,28 +3,29 @@ # # Table name: statuses # -# id :bigint(8) not null, primary key -# uri :string -# text :text default(""), not null -# created_at :datetime not null -# updated_at :datetime not null -# in_reply_to_id :bigint(8) -# reblog_of_id :bigint(8) -# url :string -# sensitive :boolean default(FALSE), not null -# visibility :integer default("public"), not null -# spoiler_text :text default(""), not null -# reply :boolean default(FALSE), not null -# language :string -# conversation_id :bigint(8) -# local :boolean -# account_id :bigint(8) not null -# application_id :bigint(8) -# in_reply_to_account_id :bigint(8) -# poll_id :bigint(8) -# deleted_at :datetime -# edited_at :datetime -# trendable :boolean +# id :bigint(8) not null, primary key +# uri :string +# text :text default(""), not null +# created_at :datetime not null +# updated_at :datetime not null +# in_reply_to_id :bigint(8) +# reblog_of_id :bigint(8) +# url :string +# sensitive :boolean default(FALSE), not null +# visibility :integer default("public"), not null +# spoiler_text :text default(""), not null +# reply :boolean default(FALSE), not null +# language :string +# conversation_id :bigint(8) +# local :boolean +# account_id :bigint(8) not null +# application_id :bigint(8) +# in_reply_to_account_id :bigint(8) +# poll_id :bigint(8) +# deleted_at :datetime +# edited_at :datetime +# trendable :boolean +# ordered_media_attachment_ids :bigint(8) is an Array # class Status < ApplicationRecord @@ -211,11 +212,14 @@ class Status < ApplicationRecord public_visibility? || unlisted_visibility? end - def snapshot!(media_attachments_changed: false, account_id: nil, at_time: nil) + def snapshot!(account_id: nil, at_time: nil) edits.create!( text: text, spoiler_text: spoiler_text, - media_attachments_changed: media_attachments_changed, + sensitive: sensitive, + ordered_media_attachment_ids: ordered_media_attachment_ids || media_attachments.pluck(:id), + media_descriptions: ordered_media_attachments.map(&:description), + poll_options: preloadable_poll&.options, account_id: account_id || self.account_id, created_at: at_time || edited_at ) @@ -228,7 +232,7 @@ class Status < ApplicationRecord alias sign? distributable? def with_media? - media_attachments.any? + ordered_media_attachments.any? end def with_preview_card? @@ -252,6 +256,15 @@ class Status < ApplicationRecord @emojis = CustomEmoji.from_text(fields.join(' '), account.domain) end + def ordered_media_attachments + if ordered_media_attachment_ids.nil? + media_attachments + else + map = media_attachments.index_by(&:id) + ordered_media_attachment_ids.map { |media_attachment_id| map[media_attachment_id] } + end + end + def replies_count status_stat&.replies_count || 0 end -- cgit From b2cd34474b58b8a1f5e01ba73d236551dd0a878f Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 9 Mar 2022 20:06:51 +0100 Subject: Add rate limit for editing (#17728) --- app/controllers/api/v1/statuses_controller.rb | 1 + app/models/status.rb | 5 +++-- app/models/status_edit.rb | 4 ++++ app/services/activitypub/process_status_update_service.rb | 4 ++-- app/services/update_status_service.rb | 2 +- 5 files changed, 11 insertions(+), 5 deletions(-) (limited to 'app/models/status.rb') diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index f48aeb945..3fe137bfd 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -10,6 +10,7 @@ class Api::V1::StatusesController < Api::BaseController before_action :set_thread, only: [:create] override_rate_limit_headers :create, family: :statuses + override_rate_limit_headers :update, family: :statuses # This API was originally unlimited, pagination cannot be introduced without # breaking backwards-compatibility. Arbitrarily high number to cover most diff --git a/app/models/status.rb b/app/models/status.rb index db10eedc2..12daee2de 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -212,7 +212,7 @@ class Status < ApplicationRecord public_visibility? || unlisted_visibility? end - def snapshot!(account_id: nil, at_time: nil) + def snapshot!(account_id: nil, at_time: nil, rate_limit: true) edits.create!( text: text, spoiler_text: spoiler_text, @@ -221,7 +221,8 @@ class Status < ApplicationRecord media_descriptions: ordered_media_attachments.map(&:description), poll_options: preloadable_poll&.options, account_id: account_id || self.account_id, - created_at: at_time || edited_at + created_at: at_time || edited_at, + rate_limit: rate_limit ) end diff --git a/app/models/status_edit.rb b/app/models/status_edit.rb index 94a387c36..6da9b4b85 100644 --- a/app/models/status_edit.rb +++ b/app/models/status_edit.rb @@ -17,6 +17,8 @@ # class StatusEdit < ApplicationRecord + include RateLimitable + self.ignored_columns = %w( media_attachments_changed ) @@ -26,6 +28,8 @@ class StatusEdit < ApplicationRecord delegate :id, :type, :url, :preview_url, :remote_url, :preview_remote_url, :text_url, :meta, :blurhash, to: :media_attachment end + rate_limit by: :account, family: :statuses + belongs_to :status belongs_to :account, optional: true diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 11afa894f..1260c0482 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -216,13 +216,13 @@ class ActivityPub::ProcessStatusUpdateService < BaseService return if @status.edits.any? - @status.snapshot!(at_time: @status.created_at) + @status.snapshot!(at_time: @status.created_at, rate_limit: false) end def create_edit! return unless significant_changes? - @status.snapshot!(account_id: @account.id) + @status.snapshot!(account_id: @account.id, rate_limit: false) end def skip_download? diff --git a/app/services/update_status_service.rb b/app/services/update_status_service.rb index 1c63ab656..055e5968d 100644 --- a/app/services/update_status_service.rb +++ b/app/services/update_status_service.rb @@ -131,7 +131,7 @@ class UpdateStatusService < BaseService return if @status.edits.any? - @status.snapshot!(at_time: @status.created_at) + @status.snapshot!(at_time: @status.created_at, rate_limit: false) end def create_edit! -- cgit From 2a56a890dabb7cffd1dc14cf8a7aea9cccc7ab09 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 9 Mar 2022 20:49:14 +0100 Subject: Fix rare race condition when rebloged status is deleted (#17693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix rare race condition when rebloged status is deleted * Use INSERT INTO … SELECT --- app/models/status.rb | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) (limited to 'app/models/status.rb') diff --git a/app/models/status.rb b/app/models/status.rb index 12daee2de..f2c55090b 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -379,6 +379,63 @@ class Status < ApplicationRecord super || build_status_stat end + # Hack to use a "INSERT INTO ... SELECT ..." query instead of "INSERT INTO ... VALUES ..." query + def self._insert_record(values) + if values.is_a?(Hash) && values['reblog_of_id'].present? + primary_key = self.primary_key + primary_key_value = nil + + if primary_key + primary_key_value = values[primary_key] + + if !primary_key_value && prefetch_primary_key? + primary_key_value = next_sequence_value + values[primary_key] = primary_key_value + end + end + + # The following line is where we differ from stock ActiveRecord implementation + im = _compile_reblog_insert(values) + + # Since we are using SELECT instead of VALUES, a non-error `nil` return is possible. + # For our purposes, it's equivalent to a foreign key constraint violation + result = connection.insert(im, "#{self} Create", primary_key || false, primary_key_value) + raise ActiveRecord::InvalidForeignKey, "(reblog_of_id)=(#{values['reblog_of_id']}) is not present in table \"statuses\"" if result.nil? + + result + else + super + end + end + + def self._compile_reblog_insert(values) + # This is somewhat equivalent to the following code of ActiveRecord::Persistence: + # `arel_table.compile_insert(_substitute_values(values))` + # The main difference is that we use a `SELECT` instead of a `VALUES` clause, + # which means we have to build the `SELECT` clause ourselves and do a bit more + # manual work. + + # Instead of using Arel::InsertManager#values, we are going to use Arel::InsertManager#select + im = Arel::InsertManager.new + im.into(arel_table) + + binds = [] + reblog_bind = nil + values.each do |name, value| + attr = arel_table[name] + bind = predicate_builder.build_bind_attribute(attr.name, value) + + im.columns << attr + binds << bind + + reblog_bind = bind if name == 'reblog_of_id' + end + + im.select(arel_table.where(arel_table[:id].eq(reblog_bind)).where(arel_table[:deleted_at].eq(nil)).project(*binds)) + + im + end + private def update_status_stat!(attrs) -- cgit