From 8a6cba44aada8ad165f2573127de5d88bb98440e Mon Sep 17 00:00:00 2001 From: ThibG Date: Fri, 4 May 2018 20:17:01 +0200 Subject: [PATCH 1/2] Do not render first page of following and followers collections unless explicitly asked to (#7357) --- .../follower_accounts_controller.rb | 30 ++++++++++--------- .../following_accounts_controller.rb | 30 ++++++++++--------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/app/controllers/follower_accounts_controller.rb b/app/controllers/follower_accounts_controller.rb index 2d2315034c..ac0d4c54ef 100644 --- a/app/controllers/follower_accounts_controller.rb +++ b/app/controllers/follower_accounts_controller.rb @@ -4,11 +4,10 @@ class FollowerAccountsController < ApplicationController include AccountControllerConcern def index - @follows = Follow.where(target_account: @account).recent.page(params[:page]).per(FOLLOW_PER_PAGE).preload(:account) - respond_to do |format| format.html do - @relationships = AccountRelationshipsPresenter.new(@follows.map(&:account_id), current_user.account_id) if user_signed_in? + follows + @relationships = AccountRelationshipsPresenter.new(follows.map(&:account_id), current_user.account_id) if user_signed_in? end format.json do @@ -22,28 +21,31 @@ class FollowerAccountsController < ApplicationController private + def follows + @follows ||= Follow.where(target_account: @account).recent.page(params[:page]).per(FOLLOW_PER_PAGE).preload(:account) + end + def page_url(page) account_followers_url(@account, page: page) unless page.nil? end def collection_presenter - page = ActivityPub::CollectionPresenter.new( - id: account_followers_url(@account, page: params.fetch(:page, 1)), - type: :ordered, - size: @account.followers_count, - items: @follows.map { |f| ActivityPub::TagManager.instance.uri_for(f.account) }, - part_of: account_followers_url(@account), - next: page_url(@follows.next_page), - prev: page_url(@follows.prev_page) - ) if params[:page].present? - page + ActivityPub::CollectionPresenter.new( + id: account_followers_url(@account, page: params.fetch(:page, 1)), + type: :ordered, + size: @account.followers_count, + items: follows.map { |f| ActivityPub::TagManager.instance.uri_for(f.account) }, + part_of: account_followers_url(@account), + next: page_url(follows.next_page), + prev: page_url(follows.prev_page) + ) else ActivityPub::CollectionPresenter.new( id: account_followers_url(@account), type: :ordered, size: @account.followers_count, - first: page + first: page_url(1) ) end end diff --git a/app/controllers/following_accounts_controller.rb b/app/controllers/following_accounts_controller.rb index 169f9057d8..974d95c8e4 100644 --- a/app/controllers/following_accounts_controller.rb +++ b/app/controllers/following_accounts_controller.rb @@ -4,11 +4,10 @@ class FollowingAccountsController < ApplicationController include AccountControllerConcern def index - @follows = Follow.where(account: @account).recent.page(params[:page]).per(FOLLOW_PER_PAGE).preload(:target_account) - respond_to do |format| format.html do - @relationships = AccountRelationshipsPresenter.new(@follows.map(&:target_account_id), current_user.account_id) if user_signed_in? + follows + @relationships = AccountRelationshipsPresenter.new(follows.map(&:target_account_id), current_user.account_id) if user_signed_in? end format.json do @@ -22,28 +21,31 @@ class FollowingAccountsController < ApplicationController private + def follows + @follows ||= Follow.where(account: @account).recent.page(params[:page]).per(FOLLOW_PER_PAGE).preload(:target_account) + end + def page_url(page) account_following_index_url(@account, page: page) unless page.nil? end def collection_presenter - page = ActivityPub::CollectionPresenter.new( - id: account_following_index_url(@account, page: params.fetch(:page, 1)), - type: :ordered, - size: @account.following_count, - items: @follows.map { |f| ActivityPub::TagManager.instance.uri_for(f.target_account) }, - part_of: account_following_index_url(@account), - next: page_url(@follows.next_page), - prev: page_url(@follows.prev_page) - ) if params[:page].present? - page + ActivityPub::CollectionPresenter.new( + id: account_following_index_url(@account, page: params.fetch(:page, 1)), + type: :ordered, + size: @account.following_count, + items: follows.map { |f| ActivityPub::TagManager.instance.uri_for(f.target_account) }, + part_of: account_following_index_url(@account), + next: page_url(follows.next_page), + prev: page_url(follows.prev_page) + ) else ActivityPub::CollectionPresenter.new( id: account_following_index_url(@account), type: :ordered, size: @account.following_count, - first: page + first: page_url(1) ) end end From 154076e8e71c69fb0dbad464b0cb103c1c0c2d9e Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 4 May 2018 21:14:34 +0200 Subject: [PATCH 2/2] Store URIs of follows, follow requests and blocks for ActivityPub (#7160) Same URI passed between follow request and follow, since they are the same thing in ActivityPub. Local URIs are generated during creation using UUIDs and are passed to serializers. --- app/lib/activitypub/activity/block.rb | 2 +- app/lib/activitypub/activity/follow.rb | 2 +- app/lib/activitypub/tag_manager.rb | 4 ++++ app/models/block.rb | 10 ++++++++++ app/models/concerns/account_interactions.rb | 13 ++++++++----- app/models/follow.rb | 13 +++++++++++++ app/models/follow_request.rb | 16 ++++++++++++++-- app/serializers/activitypub/follow_serializer.rb | 2 +- .../20180416210259_add_uri_to_relationships.rb | 7 +++++++ db/schema.rb | 6 ++++-- spec/models/follow_request_spec.rb | 2 +- 11 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 db/migrate/20180416210259_add_uri_to_relationships.rb diff --git a/app/lib/activitypub/activity/block.rb b/app/lib/activitypub/activity/block.rb index f630d5db2a..26da8bdf5c 100644 --- a/app/lib/activitypub/activity/block.rb +++ b/app/lib/activitypub/activity/block.rb @@ -7,6 +7,6 @@ class ActivityPub::Activity::Block < ActivityPub::Activity return if target_account.nil? || !target_account.local? || delete_arrived_first?(@json['id']) || @account.blocking?(target_account) UnfollowService.new.call(target_account, @account) if target_account.following?(@account) - @account.block!(target_account) + @account.block!(target_account, uri: @json['id']) end end diff --git a/app/lib/activitypub/activity/follow.rb b/app/lib/activitypub/activity/follow.rb index 8adbbb9c33..fbbf358a87 100644 --- a/app/lib/activitypub/activity/follow.rb +++ b/app/lib/activitypub/activity/follow.rb @@ -12,7 +12,7 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity return end - follow_request = FollowRequest.create!(account: @account, target_account: target_account) + follow_request = FollowRequest.create!(account: @account, target_account: target_account, uri: @json['id']) if target_account.locked? NotifyService.new.call(target_account, follow_request) diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index fa2a8f7d31..908ea96391 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -38,6 +38,10 @@ class ActivityPub::TagManager end end + def generate_uri_for(_target) + URI.join(root_url, 'payloads', SecureRandom.uuid) + end + def activity_uri_for(target) raise ArgumentError, 'target must be a local activity' unless %i(note comment activity).include?(target.object_type) && target.local? diff --git a/app/models/block.rb b/app/models/block.rb index df4a6bbacb..bf3e076003 100644 --- a/app/models/block.rb +++ b/app/models/block.rb @@ -8,6 +8,7 @@ # updated_at :datetime not null # account_id :bigint(8) not null # target_account_id :bigint(8) not null +# uri :string # class Block < ApplicationRecord @@ -19,7 +20,12 @@ class Block < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } + def local? + false # Force uri_for to use uri attribute + end + after_commit :remove_blocking_cache + before_validation :set_uri, only: :create private @@ -27,4 +33,8 @@ class Block < ApplicationRecord Rails.cache.delete("exclude_account_ids_for:#{account_id}") Rails.cache.delete("exclude_account_ids_for:#{target_account_id}") end + + def set_uri + self.uri = ActivityPub::TagManager.instance.generate_uri_for(self) if uri.nil? + end end diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 4a01eed654..ae43711bea 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -82,16 +82,19 @@ module AccountInteractions has_many :domain_blocks, class_name: 'AccountDomainBlock', dependent: :destroy end - def follow!(other_account, reblogs: nil) + def follow!(other_account, reblogs: nil, uri: nil) reblogs = true if reblogs.nil? - rel = active_relationships.create_with(show_reblogs: reblogs).find_or_create_by!(target_account: other_account) - rel.update!(show_reblogs: reblogs) + rel = active_relationships.create_with(show_reblogs: reblogs, uri: uri) + .find_or_create_by!(target_account: other_account) + + rel.update!(show_reblogs: reblogs) rel end - def block!(other_account) - block_relationships.find_or_create_by!(target_account: other_account) + def block!(other_account, uri: nil) + block_relationships.create_with(uri: uri) + .find_or_create_by!(target_account: other_account) end def mute!(other_account, notifications: nil) diff --git a/app/models/follow.rb b/app/models/follow.rb index 2ca42ff70b..eaf8445f3b 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -9,6 +9,7 @@ # account_id :bigint(8) not null # target_account_id :bigint(8) not null # show_reblogs :boolean default(TRUE), not null +# uri :string # class Follow < ApplicationRecord @@ -26,4 +27,16 @@ class Follow < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } scope :recent, -> { reorder(id: :desc) } + + def local? + false # Force uri_for to use uri attribute + end + + before_validation :set_uri, only: :create + + private + + def set_uri + self.uri = ActivityPub::TagManager.instance.generate_uri_for(self) if uri.nil? + end end diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index d559a8f62f..9c4875564b 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -9,6 +9,7 @@ # account_id :bigint(8) not null # target_account_id :bigint(8) not null # show_reblogs :boolean default(TRUE), not null +# uri :string # class FollowRequest < ApplicationRecord @@ -23,11 +24,22 @@ class FollowRequest < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } def authorize! - account.follow!(target_account, reblogs: show_reblogs) + account.follow!(target_account, reblogs: show_reblogs, uri: uri) MergeWorker.perform_async(target_account.id, account.id) - destroy! end alias reject! destroy! + + def local? + false # Force uri_for to use uri attribute + end + + before_validation :set_uri, only: :create + + private + + def set_uri + self.uri = ActivityPub::TagManager.instance.generate_uri_for(self) if uri.nil? + end end diff --git a/app/serializers/activitypub/follow_serializer.rb b/app/serializers/activitypub/follow_serializer.rb index 86c9992fe3..24dfe96f89 100644 --- a/app/serializers/activitypub/follow_serializer.rb +++ b/app/serializers/activitypub/follow_serializer.rb @@ -5,7 +5,7 @@ class ActivityPub::FollowSerializer < ActiveModel::Serializer attribute :virtual_object, key: :object def id - [ActivityPub::TagManager.instance.uri_for(object.account), '#follows/', object.id].join + ActivityPub::TagManager.instance.uri_for(object) end def type diff --git a/db/migrate/20180416210259_add_uri_to_relationships.rb b/db/migrate/20180416210259_add_uri_to_relationships.rb new file mode 100644 index 0000000000..d8eaca450b --- /dev/null +++ b/db/migrate/20180416210259_add_uri_to_relationships.rb @@ -0,0 +1,7 @@ +class AddUriToRelationships < ActiveRecord::Migration[5.2] + def change + add_column :follows, :uri, :string + add_column :follow_requests, :uri, :string + add_column :blocks, :uri, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 10a8f2edc5..566a320d87 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,10 +10,9 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_04_10_204633) do +ActiveRecord::Schema.define(version: 2018_04_16_210259) do # These are extensions that must be enabled in order to support this database - enable_extension "pg_stat_statements" enable_extension "plpgsql" create_table "account_domain_blocks", force: :cascade do |t| @@ -112,6 +111,7 @@ ActiveRecord::Schema.define(version: 2018_04_10_204633) do t.datetime "updated_at", null: false t.bigint "account_id", null: false t.bigint "target_account_id", null: false + t.string "uri" t.index ["account_id", "target_account_id"], name: "index_blocks_on_account_id_and_target_account_id", unique: true end @@ -176,6 +176,7 @@ ActiveRecord::Schema.define(version: 2018_04_10_204633) do t.bigint "account_id", null: false t.bigint "target_account_id", null: false t.boolean "show_reblogs", default: true, null: false + t.string "uri" t.index ["account_id", "target_account_id"], name: "index_follow_requests_on_account_id_and_target_account_id", unique: true end @@ -185,6 +186,7 @@ ActiveRecord::Schema.define(version: 2018_04_10_204633) do t.bigint "account_id", null: false t.bigint "target_account_id", null: false t.boolean "show_reblogs", default: true, null: false + t.string "uri" t.index ["account_id", "target_account_id"], name: "index_follows_on_account_id_and_target_account_id", unique: true end diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb index 59893a14fa..2cf28b263c 100644 --- a/spec/models/follow_request_spec.rb +++ b/spec/models/follow_request_spec.rb @@ -7,7 +7,7 @@ RSpec.describe FollowRequest, type: :model do let(:target_account) { Fabricate(:account) } it 'calls Account#follow!, MergeWorker.perform_async, and #destroy!' do - expect(account).to receive(:follow!).with(target_account, reblogs: true) + expect(account).to receive(:follow!).with(target_account, reblogs: true, uri: follow_request.uri) expect(MergeWorker).to receive(:perform_async).with(target_account.id, account.id) expect(follow_request).to receive(:destroy!) follow_request.authorize!