From 9074c1fac930db91dbf7fb6aad4b73f628b11e94 Mon Sep 17 00:00:00 2001 From: Jonny Saunders Date: Sun, 27 Oct 2024 21:55:18 -0700 Subject: [PATCH] Use `likes` and `shares` totalItems on status creations and updates (#32620) --- app/lib/activitypub/activity/create.rb | 13 ++++ app/lib/activitypub/parser/status_parser.rb | 8 +++ app/models/status.rb | 26 +++++++- app/models/status_stat.rb | 27 ++++++-- app/serializers/rest/status_serializer.rb | 4 +- .../process_status_update_service.rb | 15 +++++ ..._untrusted_reblogs_count_to_status_stat.rb | 8 +++ db/schema.rb | 4 +- spec/lib/activitypub/activity/create_spec.rb | 26 ++++++++ spec/lib/activitypub/activity/update_spec.rb | 64 +++++++++++++++++++ spec/models/status_spec.rb | 50 +++++++++++++++ spec/requests/api/v1/trends/statuses_spec.rb | 36 +++++++++++ .../rest/status_serializer_spec.rb | 55 ++++++++++++++++ spec/spec_helper.rb | 4 +- 14 files changed, 326 insertions(+), 14 deletions(-) create mode 100644 db/migrate/20241022214312_add_untrusted_favourites_count_and_untrusted_reblogs_count_to_status_stat.rb create mode 100644 spec/serializers/rest/status_serializer_spec.rb diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index d04f7226a..85a66c685 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -53,6 +53,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity ApplicationRecord.transaction do @status = Status.create!(@params) attach_tags(@status) + attach_counts(@status) end resolve_thread(@status) @@ -166,6 +167,18 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end end + def attach_counts(status) + likes = @status_parser.favourites_count + shares = @status_parser.reblogs_count + return if likes.nil? && shares.nil? + + status.status_stat.tap do |status_stat| + status_stat.untrusted_reblogs_count = shares unless shares.nil? + status_stat.untrusted_favourites_count = likes unless likes.nil? + status_stat.save if status_stat.changed? + end + end + def process_tags return if @object['tag'].nil? diff --git a/app/lib/activitypub/parser/status_parser.rb b/app/lib/activitypub/parser/status_parser.rb index 2940aea44..3d2be3c66 100644 --- a/app/lib/activitypub/parser/status_parser.rb +++ b/app/lib/activitypub/parser/status_parser.rb @@ -93,6 +93,14 @@ class ActivityPub::Parser::StatusParser lang.presence && NORMALIZED_LOCALE_NAMES.fetch(lang.downcase.to_sym, lang) end + def favourites_count + @object.dig(:likes, :totalItems) + end + + def reblogs_count + @object.dig(:shares, :totalItems) + end + private def raw_language_code diff --git a/app/models/status.rb b/app/models/status.rb index e0630733d..f0a4f50ff 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -303,12 +303,34 @@ class Status < ApplicationRecord status_stat&.favourites_count || 0 end + # Reblogs count received from an external instance + def untrusted_reblogs_count + status_stat&.untrusted_reblogs_count unless local? + end + + # Favourites count received from an external instance + def untrusted_favourites_count + status_stat&.untrusted_favourites_count unless local? + end + def increment_count!(key) - update_status_stat!(key => public_send(key) + 1) + if key == :favourites_count && !untrusted_favourites_count.nil? + update_status_stat!(favourites_count: favourites_count + 1, untrusted_favourites_count: untrusted_favourites_count + 1) + elsif key == :reblogs_count && !untrusted_reblogs_count.nil? + update_status_stat!(reblogs_count: reblogs_count + 1, untrusted_reblogs_count: untrusted_reblogs_count + 1) + else + update_status_stat!(key => public_send(key) + 1) + end end def decrement_count!(key) - update_status_stat!(key => [public_send(key) - 1, 0].max) + if key == :favourites_count && !untrusted_favourites_count.nil? + update_status_stat!(favourites_count: [favourites_count - 1, 0].max, untrusted_favourites_count: [untrusted_favourites_count - 1, 0].max) + elsif key == :reblogs_count && !untrusted_reblogs_count.nil? + update_status_stat!(reblogs_count: [reblogs_count - 1, 0].max, untrusted_reblogs_count: [untrusted_reblogs_count - 1, 0].max) + else + update_status_stat!(key => [public_send(key) - 1, 0].max) + end end def trendable? diff --git a/app/models/status_stat.rb b/app/models/status_stat.rb index 47aa14477..14a02071a 100644 --- a/app/models/status_stat.rb +++ b/app/models/status_stat.rb @@ -4,18 +4,24 @@ # # Table name: status_stats # -# id :bigint(8) not null, primary key -# status_id :bigint(8) not null -# replies_count :bigint(8) default(0), not null -# reblogs_count :bigint(8) default(0), not null -# favourites_count :bigint(8) default(0), not null -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# status_id :bigint(8) not null +# replies_count :bigint(8) default(0), not null +# reblogs_count :bigint(8) default(0), not null +# favourites_count :bigint(8) default(0), not null +# created_at :datetime not null +# updated_at :datetime not null +# untrusted_favourites_count :bigint(8) +# untrusted_reblogs_count :bigint(8) # class StatusStat < ApplicationRecord belongs_to :status, inverse_of: :status_stat + before_validation :clamp_untrusted_counts + + MAX_UNTRUSTED_COUNT = 100_000_000 + def replies_count [attributes['replies_count'], 0].max end @@ -27,4 +33,11 @@ class StatusStat < ApplicationRecord def favourites_count [attributes['favourites_count'], 0].max end + + private + + def clamp_untrusted_counts + self.untrusted_favourites_count = untrusted_favourites_count.to_i.clamp(0, MAX_UNTRUSTED_COUNT) if untrusted_favourites_count.present? + self.untrusted_reblogs_count = untrusted_reblogs_count.to_i.clamp(0, MAX_UNTRUSTED_COUNT) if untrusted_reblogs_count.present? + end end diff --git a/app/serializers/rest/status_serializer.rb b/app/serializers/rest/status_serializer.rb index e17e8c823..e108c789c 100644 --- a/app/serializers/rest/status_serializer.rb +++ b/app/serializers/rest/status_serializer.rb @@ -84,11 +84,11 @@ class REST::StatusSerializer < ActiveModel::Serializer end def reblogs_count - relationships&.attributes_map&.dig(object.id, :reblogs_count) || object.reblogs_count + object.untrusted_reblogs_count || relationships&.attributes_map&.dig(object.id, :reblogs_count) || object.reblogs_count end def favourites_count - relationships&.attributes_map&.dig(object.id, :favourites_count) || object.favourites_count + object.untrusted_favourites_count || relationships&.attributes_map&.dig(object.id, :favourites_count) || object.favourites_count end def favourited diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 141ad24e9..1c7584b76 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -43,6 +43,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService update_poll! update_immediate_attributes! update_metadata! + update_counts! create_edits! end @@ -62,6 +63,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService with_redis_lock("create:#{@uri}") do update_poll!(allow_significant_changes: false) queue_poll_notifications! + update_counts! end end @@ -239,6 +241,19 @@ class ActivityPub::ProcessStatusUpdateService < BaseService end end + def update_counts! + likes = @status_parser.favourites_count + shares = @status_parser.reblogs_count + return if likes.nil? && shares.nil? + + @status.status_stat.tap do |status_stat| + status_stat.untrusted_reblogs_count = shares unless shares.nil? + status_stat.untrusted_favourites_count = likes unless likes.nil? + + status_stat.save if status_stat.changed? + end + end + def expected_type? equals_or_includes_any?(@json['type'], %w(Note Question)) end diff --git a/db/migrate/20241022214312_add_untrusted_favourites_count_and_untrusted_reblogs_count_to_status_stat.rb b/db/migrate/20241022214312_add_untrusted_favourites_count_and_untrusted_reblogs_count_to_status_stat.rb new file mode 100644 index 000000000..e34caff24 --- /dev/null +++ b/db/migrate/20241022214312_add_untrusted_favourites_count_and_untrusted_reblogs_count_to_status_stat.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddUntrustedFavouritesCountAndUntrustedReblogsCountToStatusStat < ActiveRecord::Migration[7.1] + def change + add_column :status_stats, :untrusted_favourites_count, :bigint, null: true + add_column :status_stats, :untrusted_reblogs_count, :bigint, null: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 5f7b3a330..bea9ad46b 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.1].define(version: 2024_10_14_010506) do +ActiveRecord::Schema[7.1].define(version: 2024_10_22_214312) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1008,6 +1008,8 @@ ActiveRecord::Schema[7.1].define(version: 2024_10_14_010506) do t.bigint "favourites_count", default: 0, null: false t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false + t.bigint "untrusted_favourites_count" + t.bigint "untrusted_reblogs_count" t.index ["status_id"], name: "index_status_stats_on_status_id", unique: true end diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb index bdc8fd9d5..9482a3095 100644 --- a/spec/lib/activitypub/activity/create_spec.rb +++ b/spec/lib/activitypub/activity/create_spec.rb @@ -928,6 +928,32 @@ RSpec.describe ActivityPub::Activity::Create do expect(poll.votes.first).to be_nil end end + + context 'with counts' do + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum', + likes: { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar', '/likes'].join, + type: 'Collection', + totalItems: 50, + }, + shares: { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar', '/shares'].join, + type: 'Collection', + totalItems: 100, + }, + } + end + + it 'uses the counts from the created object' do + status = sender.statuses.first + expect(status.untrusted_favourites_count).to eq 50 + expect(status.untrusted_reblogs_count).to eq 100 + end + end end context 'when object URI uses bearcaps' do diff --git a/spec/lib/activitypub/activity/update_spec.rb b/spec/lib/activitypub/activity/update_spec.rb index 87e96d2d1..b829f3a5a 100644 --- a/spec/lib/activitypub/activity/update_spec.rb +++ b/spec/lib/activitypub/activity/update_spec.rb @@ -115,5 +115,69 @@ RSpec.describe ActivityPub::Activity::Update do expect(status.edited_at).to be_nil end end + + context 'with a Note object' do + let(:updated) { nil } + let(:favourites) { 50 } + let(:reblogs) { 100 } + + let!(:status) { Fabricate(:status, uri: 'https://example.com/statuses/poll', account: sender) } + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Update', + actor: sender.uri, + object: { + type: 'Note', + id: status.uri, + content: 'Foo', + updated: updated, + likes: { + id: "#{status.uri}/likes", + type: 'Collection', + totalItems: favourites, + }, + shares: { + id: "#{status.uri}/shares", + type: 'Collection', + totalItems: reblogs, + }, + }, + }.with_indifferent_access + end + + shared_examples 'updates counts' do + it 'updates the reblog count' do + expect(status.untrusted_reblogs_count).to eq reblogs + end + + it 'updates the favourites count' do + expect(status.untrusted_favourites_count).to eq favourites + end + end + + context 'with an implicit update' do + before do + status.update!(uri: ActivityPub::TagManager.instance.uri_for(status)) + subject.perform + end + + it_behaves_like 'updates counts' + end + + context 'with an explicit update' do + let(:favourites) { 150 } + let(:reblogs) { 200 } + let(:updated) { Time.now.utc.iso8601 } + + before do + status.update!(uri: ActivityPub::TagManager.instance.uri_for(status)) + subject.perform + end + + it_behaves_like 'updates counts' + end + end end end diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index 90f596843..36b13df81 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -164,6 +164,31 @@ RSpec.describe Status do end end + describe '#untrusted_reblogs_count' do + before do + alice.update(domain: 'example.com') + subject.status_stat.tap do |status_stat| + status_stat.untrusted_reblogs_count = 0 + status_stat.save + end + subject.save + end + + it 'is incremented by the number of reblogs' do + Fabricate(:status, account: bob, reblog: subject) + Fabricate(:status, account: alice, reblog: subject) + + expect(subject.untrusted_reblogs_count).to eq 2 + end + + it 'is decremented when reblog is removed' do + reblog = Fabricate(:status, account: bob, reblog: subject) + expect(subject.untrusted_reblogs_count).to eq 1 + reblog.destroy + expect(subject.untrusted_reblogs_count).to eq 0 + end + end + describe '#replies_count' do it 'is the number of replies' do Fabricate(:status, account: bob, thread: subject) @@ -194,6 +219,31 @@ RSpec.describe Status do end end + describe '#untrusted_favourites_count' do + before do + alice.update(domain: 'example.com') + subject.status_stat.tap do |status_stat| + status_stat.untrusted_favourites_count = 0 + status_stat.save + end + subject.save + end + + it 'is incremented by favorites' do + Fabricate(:favourite, account: bob, status: subject) + Fabricate(:favourite, account: alice, status: subject) + + expect(subject.untrusted_favourites_count).to eq 2 + end + + it 'is decremented when favourite is removed' do + favourite = Fabricate(:favourite, account: bob, status: subject) + expect(subject.untrusted_favourites_count).to eq 1 + favourite.destroy + expect(subject.untrusted_favourites_count).to eq 0 + end + end + describe '#proper' do it 'is itself for original statuses' do expect(subject.proper).to eq subject diff --git a/spec/requests/api/v1/trends/statuses_spec.rb b/spec/requests/api/v1/trends/statuses_spec.rb index fe00c9c64..414d7651b 100644 --- a/spec/requests/api/v1/trends/statuses_spec.rb +++ b/spec/requests/api/v1/trends/statuses_spec.rb @@ -39,6 +39,42 @@ RSpec.describe 'API V1 Trends Statuses' do end Trends::Statuses.new(threshold: 1, decay_threshold: -1).refresh end + + context 'with a comically inflated external interactions count' do + def prepare_fake_trends + fake_remote_account = Fabricate(:account, domain: 'other.com') + fake_status = Fabricate(:status, account: fake_remote_account, text: 'I am a big faker', trendable: true, language: 'en') + fake_status.status_stat.tap do |status_stat| + status_stat.reblogs_count = 0 + status_stat.favourites_count = 0 + status_stat.untrusted_reblogs_count = 1_000_000_000 + status_stat.untrusted_favourites_count = 1_000_000_000 + status_stat.save + end + real_remote_account = Fabricate(:account, domain: 'other.com') + real_status = Fabricate(:status, account: real_remote_account, text: 'I make real friends online', trendable: true, language: 'en') + real_status.status_stat.tap do |status_stat| + status_stat.reblogs_count = 10 + status_stat.favourites_count = 10 + status_stat.untrusted_reblogs_count = 10 + status_stat.untrusted_favourites_count = 10 + status_stat.save + end + Trends.statuses.add(fake_status, 100) + Trends.statuses.add(real_status, 101) + Trends::Statuses.new(threshold: 1, decay_threshold: 1).refresh + end + + it 'ignores the feeble attempts at deception' do + prepare_fake_trends + stub_const('Api::BaseController::DEFAULT_STATUSES_LIMIT', 10) + get '/api/v1/trends/statuses' + + expect(response).to have_http_status(200) + expect(response.parsed_body.length).to eq(1) + expect(response.parsed_body[0]['content']).to eq('I make real friends online') + end + end end end end diff --git a/spec/serializers/rest/status_serializer_spec.rb b/spec/serializers/rest/status_serializer_spec.rb new file mode 100644 index 000000000..e96d1fbe6 --- /dev/null +++ b/spec/serializers/rest/status_serializer_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe REST::StatusSerializer do + subject do + serialized_record_json( + status, + described_class, + options: { + scope: current_user, + scope_name: :current_user, + } + ) + end + + let(:current_user) { Fabricate(:user) } + let(:alice) { Fabricate(:account, username: 'alice') } + let(:bob) { Fabricate(:account, username: 'bob', domain: 'other.com') } + let(:status) { Fabricate(:status, account: alice) } + + context 'with a remote status' do + let(:status) { Fabricate(:status, account: bob) } + + before do + status.status_stat.tap do |status_stat| + status_stat.reblogs_count = 10 + status_stat.favourites_count = 20 + status_stat.save + end + end + + context 'with only trusted counts' do + it 'shows the trusted counts' do + expect(subject['reblogs_count']).to eq(10) + expect(subject['favourites_count']).to eq(20) + end + end + + context 'with untrusted counts' do + before do + status.status_stat.tap do |status_stat| + status_stat.untrusted_reblogs_count = 30 + status_stat.untrusted_favourites_count = 40 + status_stat.save + end + end + + it 'shows the untrusted counts' do + expect(subject['reblogs_count']).to eq(30) + expect(subject['favourites_count']).to eq(40) + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2a2754440..13683e404 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -34,8 +34,8 @@ RSpec.configure do |config| end end -def serialized_record_json(record, serializer, adapter: nil) - options = { serializer: serializer } +def serialized_record_json(record, serializer, adapter: nil, options: {}) + options[:serializer] = serializer options[:adapter] = adapter if adapter.present? JSON.parse( ActiveModelSerializers::SerializableResource.new(