commit: 0518492158af247f3b99a8f27f4498d1bcc91117 parent: 94d21827174c52a6b70ba2e45f098223f5d904fa Author: Eugen Rochko <eugen@zeonfederated.com> Date: Sun, 12 Feb 2017 00:48:53 +0100 Stop trying to shoehorn all Salmon updates into the poor database-connected StreamEntry model. Simply render Salmon slaps as they are neededDiffstat:
22 files changed, 304 insertions(+), 266 deletions(-)
diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb @@ -16,7 +16,7 @@ class AccountsController < ApplicationController end format.atom do - @entries = @account.stream_entries.order('id desc').where(hidden: false).with_includes.paginate_by_max_id(20, params[:max_id], params[:since_id]) + @entries = @account.stream_entries.order('id desc').where(activity_type: 'Status').where(hidden: false).with_includes.paginate_by_max_id(20, params[:max_id], params[:since_id]) end format.activitystreams2 diff --git a/app/models/block.rb b/app/models/block.rb @@ -2,27 +2,10 @@ class Block < ApplicationRecord include Paginable - include Streamable belongs_to :account belongs_to :target_account, class_name: 'Account' validates :account, :target_account, presence: true validates :account_id, uniqueness: { scope: :target_account_id } - - def verb - destroyed? ? :unblock : :block - end - - def target - target_account - end - - def hidden? - true - end - - def title - destroyed? ? "#{account.acct} is no longer blocking #{target_account.acct}" : "#{account.acct} blocked #{target_account.acct}" - end end diff --git a/app/models/favourite.rb b/app/models/favourite.rb @@ -2,7 +2,6 @@ class Favourite < ApplicationRecord include Paginable - include Streamable belongs_to :account, inverse_of: :favourites belongs_to :status, inverse_of: :favourites @@ -11,26 +10,6 @@ class Favourite < ApplicationRecord validates :status_id, uniqueness: { scope: :account_id } - def verb - destroyed? ? :unfavorite : :favorite - end - - def title - destroyed? ? "#{account.acct} no longer favourites a status by #{status.account.acct}" : "#{account.acct} favourited a status by #{status.account.acct}" - end - - def thread - status - end - - def target - thread - end - - def hidden? - status.private_visibility? - end - before_validation do self.status = status.reblog if status.reblog? end diff --git a/app/models/follow.rb b/app/models/follow.rb @@ -2,7 +2,6 @@ class Follow < ApplicationRecord include Paginable - include Streamable belongs_to :account belongs_to :target_account, class_name: 'Account' @@ -11,16 +10,4 @@ class Follow < ApplicationRecord validates :account, :target_account, presence: true validates :account_id, uniqueness: { scope: :target_account_id } - - def verb - destroyed? ? :unfollow : :follow - end - - def target - target_account - end - - def title - destroyed? ? "#{account.acct} is no longer following #{target_account.acct}" : "#{account.acct} started following #{target_account.acct}" - end end diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb @@ -2,7 +2,6 @@ class FollowRequest < ApplicationRecord include Paginable - include Streamable belongs_to :account belongs_to :target_account, class_name: 'Account' @@ -13,9 +12,6 @@ class FollowRequest < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } def authorize! - @verb = :authorize - @target = clone.freeze - account.follow!(target_account) MergeWorker.perform_async(target_account.id, account.id) @@ -23,44 +19,6 @@ class FollowRequest < ApplicationRecord end def reject! - @verb = :reject - @target = clone.freeze - destroy! end - - def verb - destroyed? ? (@verb || :delete) : :request_friend - end - - def target - if destroyed? && @verb - @target - else - target_account - end - end - - def hidden? - true - end - - def needs_stream_entry? - true - end - - def title - if destroyed? - case @verb - when :authorize - "#{target_account.acct} authorized #{account.acct}'s request to follow" - when :reject - "#{target_account.acct} rejected #{account.acct}'s request to follow" - else - "#{account.acct} withdrew the request to follow #{target_account.acct}" - end - else - "#{account.acct} requested to follow #{target_account.acct}" - end - end end diff --git a/app/models/stream_entry.rb b/app/models/stream_entry.rb @@ -6,17 +6,13 @@ class StreamEntry < ApplicationRecord belongs_to :account, inverse_of: :stream_entries belongs_to :activity, polymorphic: true - belongs_to :status, foreign_type: 'Status', foreign_key: 'activity_id' - belongs_to :follow, foreign_type: 'Follow', foreign_key: 'activity_id' - belongs_to :favourite, foreign_type: 'Favourite', foreign_key: 'activity_id' - belongs_to :block, foreign_type: 'Block', foreign_key: 'activity_id' - belongs_to :follow_request, foreign_type: 'FollowRequest', foreign_key: 'activity_id' + belongs_to :status, foreign_type: 'Status', foreign_key: 'activity_id', inverse_of: :stream_entry validates :account, :activity, presence: true STATUS_INCLUDES = [:account, :stream_entry, :media_attachments, :tags, mentions: :account, reblog: [:stream_entry, :account, mentions: :account], thread: [:stream_entry, :account]].freeze - scope :with_includes, -> { includes(:account, status: STATUS_INCLUDES, favourite: [:account, :stream_entry, status: STATUS_INCLUDES], follow: [:target_account, :stream_entry]) } + scope :with_includes, -> { includes(:account, status: STATUS_INCLUDES) } def object_type if orphaned? diff --git a/app/services/authorize_follow_service.rb b/app/services/authorize_follow_service.rb @@ -1,12 +1,37 @@ # frozen_string_literal: true class AuthorizeFollowService < BaseService - include StreamEntryRenderer - def call(source_account, target_account) follow_request = FollowRequest.find_by!(account: source_account, target_account: target_account) follow_request.authorize! - NotificationWorker.perform_async(stream_entry_to_xml(follow_request.stream_entry), target_account.id, source_account.id) unless source_account.local? - follow_request.stream_entry.destroy + NotificationWorker.perform_async(build_xml(follow_request), target_account.id, source_account.id) unless source_account.local? + end + + private + + def build_xml(follow_request) + Nokogiri::XML::Builder.new do |xml| + entry(xml, true) do + author(xml) do + include_author xml, follow_request.target_account + end + + object_type xml, :activity + verb xml, :authorize + + target(xml) do + author(xml) do + include_author xml, follow_request.account + end + + object_type xml, :activity + verb xml, :request_friend + + target(xml) do + include_author xml, follow_request.target_account + end + end + end + end.to_xml end end diff --git a/app/services/block_service.rb b/app/services/block_service.rb @@ -12,6 +12,27 @@ class BlockService < BaseService block = account.block!(target_account) BlockWorker.perform_async(account.id, target_account.id) - NotificationWorker.perform_async(stream_entry_to_xml(block.stream_entry), account.id, target_account.id) unless target_account.local? + NotificationWorker.perform_async(build_xml(block), account.id, target_account.id) unless target_account.local? + end + + private + + def build_xml(block) + Nokogiri::XML::Builder.new do |xml| + entry(xml, true) do + title xml, "#{block.account.acct} no longer wishes to interact with #{block.target_account.acct}" + + author(xml) do + include_author xml, block.account + end + + object_type xml, :activity + verb xml, :block + + target(xml) do + include_author xml, block.target_account + end + end + end.to_xml end end diff --git a/app/services/favourite_service.rb b/app/services/favourite_service.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class FavouriteService < BaseService - include StreamEntryRenderer - # Favourite a status and notify remote user # @param [Account] account # @param [Status] status @@ -12,14 +10,37 @@ class FavouriteService < BaseService favourite = Favourite.create!(account: account, status: status) - Pubsubhubbub::DistributionWorker.perform_async(favourite.stream_entry.id) - if status.local? NotifyService.new.call(favourite.status.account, favourite) else - NotificationWorker.perform_async(stream_entry_to_xml(favourite.stream_entry), account.id, status.account_id) + NotificationWorker.perform_async(build_xml(favourite), account.id, status.account_id) end favourite end + + private + + def build_xml(favourite) + Nokogiri::XML::Builder.new do |xml| + entry(xml, true) do + title xml, "#{favourite.account.acct} favourited a status by #{favourite.status.account.acct}" + + author(xml) do + include_author xml, favourite.account + end + + object_type xml, :activity + verb xml, :favourite + + target(xml) do + author(xml) do + include_author xml, favourite.status.account + end + + include_entry xml, favourite.status.stream_entry + end + end + end.to_xml + end end diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb @@ -7,7 +7,7 @@ class FollowService < BaseService # @param [Account] source_account From which to follow # @param [String] uri User URI to follow in the form of username@domain def call(source_account, uri) - target_account = follow_remote_account_service.call(uri) + target_account = FollowRemoteAccountService.new.call(uri) raise ActiveRecord::RecordNotFound if target_account.nil? || target_account.id == source_account.id || target_account.suspended? raise Mastodon::NotPermitted if target_account.blocking?(source_account) || source_account.blocking?(target_account) @@ -27,7 +27,7 @@ class FollowService < BaseService if target_account.local? NotifyService.new.call(target_account, follow_request) else - NotificationWorker.perform_async(stream_entry_to_xml(follow_request.stream_entry), source_account.id, target_account.id) + NotificationWorker.perform_async(build_follow_request_xml(follow_request), source_account.id, target_account.id) AfterRemoteFollowRequestWorker.perform_async(follow_request.id) end @@ -40,13 +40,12 @@ class FollowService < BaseService if target_account.local? NotifyService.new.call(target_account, follow) else - subscribe_service.call(target_account) unless target_account.subscribed? - NotificationWorker.perform_async(stream_entry_to_xml(follow.stream_entry), source_account.id, target_account.id) + SubscribeService.new.call(target_account) unless target_account.subscribed? + NotificationWorker.perform_async(build_follow_xml(follow), source_account.id, target_account.id) AfterRemoteFollowWorker.perform_async(follow.id) end MergeWorker.perform_async(target_account.id, source_account.id) - Pubsubhubbub::DistributionWorker.perform_async(follow.stream_entry.id) follow end @@ -55,11 +54,41 @@ class FollowService < BaseService Redis.current end - def follow_remote_account_service - @follow_remote_account_service ||= FollowRemoteAccountService.new + def build_follow_request_xml(follow_request) + Nokogiri::XML::Builder.new do |xml| + entry(xml, true) do + title xml, "#{follow_request.account.acct} requested to follow #{follow_request.target_account.acct}" + + author(xml) do + include_author xml, follow_request.account + end + + object_type xml, :activity + verb xml, :request_friend + + target(xml) do + include_author xml, follow_request.target_account + end + end + end.to_xml end - def subscribe_service - @subscribe_service ||= SubscribeService.new + def build_follow_xml(follow) + Nokogiri::XML::Builder.new do |xml| + entry(xml, true) do + title xml, "#{follow.account.acct} started following #{follow.target_account.acct}" + + author(xml) do + include_author xml, follow.account + end + + object_type xml, :activity + verb xml, :follow + + target(xml) do + include_author xml, follow.target_account + end + end + end.to_xml end end diff --git a/app/services/reject_follow_service.rb b/app/services/reject_follow_service.rb @@ -1,12 +1,37 @@ # frozen_string_literal: true class RejectFollowService < BaseService - include StreamEntryRenderer - def call(source_account, target_account) follow_request = FollowRequest.find_by!(account: source_account, target_account: target_account) follow_request.reject! - NotificationWorker.perform_async(stream_entry_to_xml(follow_request.stream_entry), target_account.id, source_account.id) unless source_account.local? - follow_request.stream_entry.destroy + NotificationWorker.perform_async(build_xml(follow_request), target_account.id, source_account.id) unless source_account.local? + end + + private + + def build_xml(follow_request) + Nokogiri::XML::Builder.new do |xml| + entry(xml, true) do + author(xml) do + include_author xml, follow_request.target_account + end + + object_type xml, :activity + verb xml, :reject + + target(xml) do + author(xml) do + include_author xml, follow_request.account + end + + object_type xml, :activity + verb xml, :request_friend + + target(xml) do + include_author xml, follow_request.target_account + end + end + end + end.to_xml end end diff --git a/app/services/unblock_service.rb b/app/services/unblock_service.rb @@ -1,12 +1,31 @@ # frozen_string_literal: true class UnblockService < BaseService - include StreamEntryRenderer - def call(account, target_account) return unless account.blocking?(target_account) unblock = account.unblock!(target_account) - NotificationWorker.perform_async(stream_entry_to_xml(unblock.stream_entry), account.id, target_account.id) unless target_account.local? + NotificationWorker.perform_async(build_xml(unblock), account.id, target_account.id) unless target_account.local? + end + + private + + def build_xml(block) + Nokogiri::XML::Builder.new do |xml| + entry(xml, true) do + title xml, "#{block.account.acct} no longer blocks #{block.target_account.acct}" + + author(xml) do + include_author xml, block.account + end + + object_type xml, :activity + verb xml, :unblock + + target(xml) do + include_author xml, block.target_account + end + end + end.to_xml end end diff --git a/app/services/unfavourite_service.rb b/app/services/unfavourite_service.rb @@ -1,16 +1,37 @@ # frozen_string_literal: true class UnfavouriteService < BaseService - include StreamEntryRenderer - def call(account, status) favourite = Favourite.find_by!(account: account, status: status) favourite.destroy! - unless status.local? - NotificationWorker.perform_async(stream_entry_to_xml(favourite.stream_entry), account.id, status.account_id) - end + NotificationWorker.perform_async(build_xml(favourite), account.id, status.account_id) unless status.local? favourite end + + private + + def build_xml(favourite) + Nokogiri::XML::Builder.new do |xml| + entry(xml, true) do + title xml, "#{favourite.account.acct} no longer favourites a status by #{favourite.status.account.acct}" + + author(xml) do + include_author xml, favourite.account + end + + object_type xml, :activity + verb xml, :unfavourite + + target(xml) do + author(xml) do + include_author xml, favourite.status.account + end + + include_entry xml, favourite.status.stream_entry + end + end + end.to_xml + end end diff --git a/app/services/unfollow_service.rb b/app/services/unfollow_service.rb @@ -1,14 +1,33 @@ # frozen_string_literal: true class UnfollowService < BaseService - include StreamEntryRenderer - # Unfollow and notify the remote user # @param [Account] source_account Where to unfollow from # @param [Account] target_account Which to unfollow def call(source_account, target_account) follow = source_account.unfollow!(target_account) - NotificationWorker.perform_async(stream_entry_to_xml(follow.stream_entry), source_account.id, target_account.id) unless target_account.local? + NotificationWorker.perform_async(build_xml(follow), source_account.id, target_account.id) unless target_account.local? UnmergeWorker.perform_async(target_account.id, source_account.id) end + + private + + def build_xml(follow) + Nokogiri::XML::Builder.new do |xml| + entry(xml, true) do + title xml, "#{follow.account.acct} is no longer following #{follow.target_account.acct}" + + author(xml) do + include_author xml, follow.account + end + + object_type xml, :activity + verb xml, :unfollow + + target(xml) do + include_author xml, follow.target_account + end + end + end.to_xml + end end diff --git a/app/views/stream_entries/_favourite.html.haml b/app/views/stream_entries/_favourite.html.haml @@ -1,5 +0,0 @@ -.entry.entry-favourite - .content.emojify - %strong= favourite.account.acct - = t('stream_entries.favourited') - %strong= favourite.status.account.acct diff --git a/app/views/stream_entries/_follow.html.haml b/app/views/stream_entries/_follow.html.haml @@ -1,5 +0,0 @@ -.entry.entry-follow - .content.emojify - %strong= link_to follow.account.acct, account_path(follow.account) - = t('stream_entries.is_now_following') - %strong= link_to follow.target_account.acct, TagManager.instance.url_for(follow.target_account) diff --git a/config/routes.rb b/config/routes.rb @@ -4,7 +4,6 @@ require 'sidekiq/web' Rails.application.routes.draw do mount LetterOpenerWeb::Engine, at: 'letter_opener' if Rails.env.development? - mount ActionCable.server, at: 'cable' authenticate :user, lambda { |u| u.admin? } do mount Sidekiq::Web, at: 'sidekiq', as: :sidekiq diff --git a/spec/lib/tag_manager_spec.rb b/spec/lib/tag_manager_spec.rb @@ -47,22 +47,6 @@ RSpec.describe TagManager do expect(subject).to be_a String end end - - context 'Follow' do - let(:target) { Fabricate(:follow, account: alice, target_account: bob) } - - it 'returns a string' do - expect(subject).to be_a String - end - end - - context 'Favourite' do - let(:target) { Fabricate(:favourite, account: bob, status: status) } - - it 'returns a string' do - expect(subject).to be_a String - end - end end describe '#url_for' do @@ -87,21 +71,5 @@ RSpec.describe TagManager do expect(subject).to be_a String end end - - context 'Follow' do - let(:target) { Fabricate(:follow, account: alice, target_account: bob) } - - it 'returns a URL' do - expect(subject).to be_a String - end - end - - context 'Favourite' do - let(:target) { Fabricate(:favourite, account: bob, status: status) } - - it 'returns a URL' do - expect(subject).to be_a String - end - end end end diff --git a/spec/models/favourite_spec.rb b/spec/models/favourite_spec.rb @@ -6,40 +6,4 @@ RSpec.describe Favourite, type: :model do let(:status) { Fabricate(:status, account: bob) } subject { Favourite.new(account: alice, status: status) } - - describe '#verb' do - it 'is always favorite' do - expect(subject.verb).to be :favorite - end - end - - describe '#title' do - it 'describes the favourite' do - expect(subject.title).to eql 'alice favourited a status by bob' - end - end - - describe '#content' do - it 'equals the title' do - expect(subject.content).to eq subject.title - end - end - - describe '#object_type' do - it 'is an activity' do - expect(subject.object_type).to be :activity - end - end - - describe '#target' do - it 'is the status that was favourited' do - expect(subject.target).to eq status - end - end - - describe '#thread' do - it 'equals the target' do - expect(subject.thread).to eq subject.target - end - end end diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb @@ -5,34 +5,4 @@ RSpec.describe Follow, type: :model do let(:bob) { Fabricate(:account, username: 'bob') } subject { Follow.new(account: alice, target_account: bob) } - - describe '#verb' do - it 'is follow' do - expect(subject.verb).to be :follow - end - end - - describe '#title' do - it 'describes the follow' do - expect(subject.title).to eql 'alice started following bob' - end - end - - describe '#content' do - it 'is the same as the title' do - expect(subject.content).to eql subject.title - end - end - - describe '#object_type' do - it 'is an activity' do - expect(subject.object_type).to be :activity - end - end - - describe '#target' do - it 'is the person being followed' do - expect(subject.target).to eq bob - end - end end diff --git a/spec/models/stream_entry_spec.rb b/spec/models/stream_entry_spec.rb @@ -3,21 +3,11 @@ require 'rails_helper' RSpec.describe StreamEntry, type: :model do let(:alice) { Fabricate(:account, username: 'alice') } let(:bob) { Fabricate(:account, username: 'bob') } - let(:follow) { Fabricate(:follow, account: alice, target_account: bob) } let(:status) { Fabricate(:status, account: alice) } let(:reblog) { Fabricate(:status, account: bob, reblog: status) } let(:reply) { Fabricate(:status, account: bob, thread: status) } - let(:favourite) { Fabricate(:favourite, account: alice, status: status) } describe '#targeted?' do - it 'returns true for a follow' do - expect(follow.stream_entry.targeted?).to be true - end - - it 'returns true for a favourite' do - expect(favourite.stream_entry.targeted?).to be true - end - it 'returns true for a reblog' do expect(reblog.stream_entry.targeted?).to be true end @@ -28,10 +18,6 @@ RSpec.describe StreamEntry, type: :model do end describe '#threaded?' do - it 'returns true for a favourite' do - expect(favourite.stream_entry.threaded?).to be true - end - it 'returns true for a reply' do expect(reply.stream_entry.threaded?).to be true end diff --git a/spec/services/process_interaction_service_spec.rb b/spec/services/process_interaction_service_spec.rb @@ -1,15 +1,93 @@ require 'rails_helper' RSpec.describe ProcessInteractionService do + let(:receiver) { Fabricate(:user, email: 'alice@example.com', account: Fabricate(:account, username: 'alice')).account } + let(:sender) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } + subject { ProcessInteractionService.new } - it 'creates account for new remote user' - it 'updates account for existing remote user' - it 'ignores envelopes that do not address the local user' - it 'accepts a status that mentions the local user' - it 'accepts a status that is a reply to the local user\'s' - it 'accepts a favourite to a status by the local user' - it 'accepts a reblog of a status of the local user' - it 'accepts a follow of the local user' - it 'accepts an unfollow of the local user' + describe 'follow request slap' do + before do + receiver.update(locked: true) + + payload = <<XML +<entry xmlns="http://www.w3.org/2005/Atom" xmlns:activity="http://activitystrea.ms/spec/1.0/"> + <author> + <name>bob</name> + <uri>https://cb6e6126.ngrok.io/users/bob</uri> + </author> + + <id>someIdHere</id> + <activity:verb>http://activitystrea.ms/schema/1.0/request-friend</activity:verb> +</entry> +XML + + envelope = OStatus2::Salmon.new.pack(payload, sender.keypair) + subject.call(envelope, receiver) + end + + it 'creates a record' do + expect(FollowRequest.find_by(account: sender, target_account: receiver)).to_not be_nil + end + end + + describe 'follow request authorization slap' do + before do + receiver.update(locked: true) + FollowRequest.create(account: sender, target_account: receiver) + + payload = <<XML +<entry xmlns="http://www.w3.org/2005/Atom" xmlns:activity="http://activitystrea.ms/spec/1.0/"> + <author> + <name>alice</name> + <uri>https://cb6e6126.ngrok.io/users/alice</uri> + </author> + + <id>someIdHere</id> + <activity:verb>http://activitystrea.ms/schema/1.0/authorize</activity:verb> +</entry> +XML + + envelope = OStatus2::Salmon.new.pack(payload, receiver.keypair) + subject.call(envelope, sender) + end + + it 'creates a follow relationship' do + expect(Follow.find_by(account: sender, target_account: receiver)).to_not be_nil + end + + it 'removes the follow request' do + expect(FollowRequest.find_by(account: sender, target_account: receiver)).to be_nil + end + end + + describe 'follow request rejection slap' do + before do + receiver.update(locked: true) + FollowRequest.create(account: sender, target_account: receiver) + + payload = <<XML +<entry xmlns="http://www.w3.org/2005/Atom" xmlns:activity="http://activitystrea.ms/spec/1.0/"> + <author> + <name>alice</name> + <uri>https://cb6e6126.ngrok.io/users/alice</uri> + </author> + + <id>someIdHere</id> + <activity:verb>http://activitystrea.ms/schema/1.0/reject</activity:verb> +</entry> +XML + + envelope = OStatus2::Salmon.new.pack(payload, receiver.keypair) + subject.call(envelope, sender) + end + + it 'does not create a follow relationship' do + expect(Follow.find_by(account: sender, target_account: receiver)).to be_nil + end + + it 'removes the follow request' do + expect(FollowRequest.find_by(account: sender, target_account: receiver)).to be_nil + end + end end