Optimize the process of following someone (#9220)

* Eliminate extra accounts select query from FollowService

* Optimistically update follow state in web UI and hide loading bar

Fix #6205

* Asynchronize NotifyService in FollowService

And fix failing test

* Skip Webfinger resolve routine when called from FollowService if possible

If an account is ActivityPub, then webfinger re-resolving is not necessary
when called from FollowService. Improve options of ResolveAccountService

diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb
index 1d5372a8cdd4f2d7d0793119d99545efd1deba00..f711c467675a9d9f5eb46804ce2cc285c4e46d49 100644 (file)
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -17,7 +17,7 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def follow
-    FollowService.new.call(current_user.account, @account.acct, reblogs: truthy_param?(:reblogs))
+    FollowService.new.call(current_user.account, @account, reblogs: truthy_param?(:reblogs))
 
     options = @account.locked? ? {} : { following_map: { @account.id => { reblogs: truthy_param?(:reblogs) } }, requested_map: { @account.id => false } }
 

diff --git a/app/javascript/mastodon/actions/accounts.js b/app/javascript/mastodon/actions/accounts.js
index cbae62a0f4cf1bb3fb7524053d5e71e3a92b7a2d..d4a824e2c9d646c4030b9c09715408e3df05a710 100644 (file)
--- a/app/javascript/mastodon/actions/accounts.js
+++ b/app/javascript/mastodon/actions/accounts.js
@@ -145,12 +145,14 @@ export function fetchAccountFail(id, error) {
 export function followAccount(id, reblogs = true) {
   return (dispatch, getState) => {
     const alreadyFollowing = getState().getIn(['relationships', id, 'following']);
-    dispatch(followAccountRequest(id));
+    const locked = getState().getIn(['accounts', id, 'locked'], false);
+
+    dispatch(followAccountRequest(id, locked));
 
     api(getState).post(`/api/v1/accounts/${id}/follow`, { reblogs }).then(response => {
       dispatch(followAccountSuccess(response.data, alreadyFollowing));
     }).catch(error => {
-      dispatch(followAccountFail(error));
+      dispatch(followAccountFail(error, locked));
     });
   };
 };
@@ -167,10 +169,12 @@ export function unfollowAccount(id) {
   };
 };
 
-export function followAccountRequest(id) {
+export function followAccountRequest(id, locked) {
   return {
     type: ACCOUNT_FOLLOW_REQUEST,
     id,
+    locked,
+    skipLoading: true,
   };
 };
 
@@ -179,13 +183,16 @@ export function followAccountSuccess(relationship, alreadyFollowing) {
     type: ACCOUNT_FOLLOW_SUCCESS,
     relationship,
     alreadyFollowing,
+    skipLoading: true,
   };
 };
 
-export function followAccountFail(error) {
+export function followAccountFail(error, locked) {
   return {
     type: ACCOUNT_FOLLOW_FAIL,
     error,
+    locked,
+    skipLoading: true,
   };
 };
 
@@ -193,6 +200,7 @@ export function unfollowAccountRequest(id) {
   return {
     type: ACCOUNT_UNFOLLOW_REQUEST,
     id,
+    skipLoading: true,
   };
 };
 
@@ -201,6 +209,7 @@ export function unfollowAccountSuccess(relationship, statuses) {
     type: ACCOUNT_UNFOLLOW_SUCCESS,
     relationship,
     statuses,
+    skipLoading: true,
   };
 };
 
@@ -208,6 +217,7 @@ export function unfollowAccountFail(error) {
   return {
     type: ACCOUNT_UNFOLLOW_FAIL,
     error,
+    skipLoading: true,
   };
 };
 

diff --git a/app/javascript/mastodon/reducers/relationships.js b/app/javascript/mastodon/reducers/relationships.js
index f460492972361254c49e647b97448318707026cd..8322780de569d68c26f88dcac12b90d716d4d61e 100644 (file)
--- a/app/javascript/mastodon/reducers/relationships.js
+++ b/app/javascript/mastodon/reducers/relationships.js
@@ -1,6 +1,10 @@
 import {
   ACCOUNT_FOLLOW_SUCCESS,
+  ACCOUNT_FOLLOW_REQUEST,
+  ACCOUNT_FOLLOW_FAIL,
   ACCOUNT_UNFOLLOW_SUCCESS,
+  ACCOUNT_UNFOLLOW_REQUEST,
+  ACCOUNT_UNFOLLOW_FAIL,
   ACCOUNT_BLOCK_SUCCESS,
   ACCOUNT_UNBLOCK_SUCCESS,
   ACCOUNT_MUTE_SUCCESS,
@@ -37,6 +41,14 @@ const initialState = ImmutableMap();
 
 export default function relationships(state = initialState, action) {
   switch(action.type) {
+  case ACCOUNT_FOLLOW_REQUEST:
+    return state.setIn([action.id, action.locked ? 'requested' : 'following'], true);
+  case ACCOUNT_FOLLOW_FAIL:
+    return state.setIn([action.id, action.locked ? 'requested' : 'following'], false);
+  case ACCOUNT_UNFOLLOW_REQUEST:
+    return state.setIn([action.id, 'following'], false);
+  case ACCOUNT_UNFOLLOW_FAIL:
+    return state.setIn([action.id, 'following'], true);
   case ACCOUNT_FOLLOW_SUCCESS:
   case ACCOUNT_UNFOLLOW_SUCCESS:
   case ACCOUNT_BLOCK_SUCCESS:

diff --git a/app/services/concerns/author_extractor.rb b/app/services/concerns/author_extractor.rb
index 1e00eb803b73a00c517e96b48c44d9f8d22ceeb2..c2419e9ecb23d6520897848a6bcd4396d87246de 100644 (file)
--- a/app/services/concerns/author_extractor.rb
+++ b/app/services/concerns/author_extractor.rb
@@ -18,6 +18,6 @@ module AuthorExtractor
       acct   = "#{username}@#{domain}"
     end
 
-    ResolveAccountService.new.call(acct, update_profile)
+    ResolveAccountService.new.call(acct, update_profile: update_profile)
   end
 end

diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb
index f6888a68d4f7a1e2dd58d0e5916e9ef1c47d8f07..0020bc9fec7e93d9f8c98bdb080ed4a17f773fbd 100644 (file)
--- a/app/services/follow_service.rb
+++ b/app/services/follow_service.rb
@@ -7,9 +7,9 @@ class FollowService < BaseService
   # @param [Account] source_account From which to follow
   # @param [String, Account] uri User URI to follow in the form of username@domain (or account record)
   # @param [true, false, nil] reblogs Whether or not to show reblogs, defaults to true
-  def call(source_account, uri, reblogs: nil)
+  def call(source_account, target_account, reblogs: nil)
     reblogs = true if reblogs.nil?
-    target_account = uri.is_a?(Account) ? uri : ResolveAccountService.new.call(uri)
+    target_account = ResolveAccountService.new.call(target_account, skip_webfinger: true)
 
     raise ActiveRecord::RecordNotFound if target_account.nil? || target_account.id == source_account.id || target_account.suspended?
     raise Mastodon::NotPermittedError  if target_account.blocking?(source_account) || source_account.blocking?(target_account)
@@ -42,7 +42,7 @@ class FollowService < BaseService
     follow_request = FollowRequest.create!(account: source_account, target_account: target_account, show_reblogs: reblogs)
 
     if target_account.local?
-      NotifyService.new.call(target_account, follow_request)
+      LocalNotificationWorker.perform_async(target_account.id, follow_request.id, follow_request.class.name)
     elsif target_account.ostatus?
       NotificationWorker.perform_async(build_follow_request_xml(follow_request), source_account.id, target_account.id)
       AfterRemoteFollowRequestWorker.perform_async(follow_request.id)
@@ -57,7 +57,7 @@ class FollowService < BaseService
     follow = source_account.follow!(target_account, reblogs: reblogs)
 
     if target_account.local?
-      NotifyService.new.call(target_account, follow)
+      LocalNotificationWorker.perform_async(target_account.id, follow.id, follow.class.name)
     else
       Pubsubhubbub::SubscribeWorker.perform_async(target_account.id) unless target_account.subscribed?
       NotificationWorker.perform_async(build_follow_xml(follow), source_account.id, target_account.id)

diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb
index b4641c4b4aba3d204583044434b83f8494d96644..ec7d33b1d82755279b1f976e1bd45a60c1700b1e 100644 (file)
--- a/app/services/process_mentions_service.rb
+++ b/app/services/process_mentions_service.rb
@@ -47,7 +47,7 @@ class ProcessMentionsService < BaseService
     mentioned_account = mention.account
 
     if mentioned_account.local?
-      LocalNotificationWorker.perform_async(mention.id)
+      LocalNotificationWorker.perform_async(mentioned_account.id, mention.id, mention.class.name)
     elsif mentioned_account.ostatus? && !@status.stream_entry.hidden?
       NotificationWorker.perform_async(ostatus_xml, @status.account_id, mentioned_account.id)
     elsif mentioned_account.activitypub?

diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb
index 4323e7f06d57f77c8fe37a618ba463a7faddf3c8..c3064211dcf605ad0c735704ddfebf6e58e80d95 100644 (file)
--- a/app/services/resolve_account_service.rb
+++ b/app/services/resolve_account_service.rb
@@ -9,17 +9,27 @@ class ResolveAccountService < BaseService
   # Find or create a local account for a remote user.
   # When creating, look up the user's webfinger and fetch all
   # important information from their feed
-  # @param [String] uri User URI in the form of username@domain
+  # @param [String, Account] uri User URI in the form of username@domain
+  # @param [Hash] options
   # @return [Account]
-  def call(uri, update_profile = true, redirected = nil)
-    @username, @domain = uri.split('@')
-    @update_profile    = update_profile
+  def call(uri, options = {})
+    @options = options
 
-    return Account.find_local(@username) if TagManager.instance.local_domain?(@domain)
+    if uri.is_a?(Account)
+      @account  = uri
+      @username = @account.username
+      @domain   = @account.domain
+
+      return @account if @account.local? || !webfinger_update_due?
+    else
+      @username, @domain = uri.split('@')
 
-    @account = Account.find_remote(@username, @domain)
+      return Account.find_local(@username) if TagManager.instance.local_domain?(@domain)
 
-    return @account unless webfinger_update_due?
+      @account = Account.find_remote(@username, @domain)
+
+      return @account unless webfinger_update_due?
+    end
 
     Rails.logger.debug "Looking up webfinger for #{uri}"
 
@@ -30,8 +40,8 @@ class ResolveAccountService < BaseService
     if confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero?
       @username = confirmed_username
       @domain   = confirmed_domain
-    elsif redirected.nil?
-      return call("#{confirmed_username}@#{confirmed_domain}", update_profile, true)
+    elsif options[:redirected].nil?
+      return call("#{confirmed_username}@#{confirmed_domain}", options.merge(redirected: true))
     else
       Rails.logger.debug 'Requested and returned acct URIs do not match'
       return
@@ -76,7 +86,7 @@ class ResolveAccountService < BaseService
   end
 
   def webfinger_update_due?
-    @account.nil? || @account.possibly_stale?
+    @account.nil? || ((!@options[:skip_webfinger] || @account.ostatus?) && @account.possibly_stale?)
   end
 
   def activitypub_ready?
@@ -93,7 +103,7 @@ class ResolveAccountService < BaseService
   end
 
   def update_profile?
-    @update_profile
+    @options[:update_profile]
   end
 
   def handle_activitypub

diff --git a/app/workers/local_notification_worker.rb b/app/workers/local_notification_worker.rb
index 748270563c532a73f1ebf6bf3d7dff253d1e2ebb..48635e498fffd7e09893fd25ec92d509da241f0e 100644 (file)
--- a/app/workers/local_notification_worker.rb
+++ b/app/workers/local_notification_worker.rb
@@ -3,9 +3,16 @@
 class LocalNotificationWorker
   include Sidekiq::Worker
 
-  def perform(mention_id)
-    mention = Mention.find(mention_id)
-    NotifyService.new.call(mention.account, mention)
+  def perform(receiver_account_id, activity_id = nil, activity_class_name = nil)
+    if activity_id.nil? && activity_class_name.nil?
+      activity = Mention.find(receiver_account_id)
+      receiver = activity.account
+    else
+      receiver = Account.find(receiver_account_id)
+      activity = activity_class_name.constantize.find(activity_id)
+    end
+
+    NotifyService.new.call(receiver, activity)
   rescue ActiveRecord::RecordNotFound
     true
   end

diff --git a/spec/controllers/authorize_interactions_controller_spec.rb b/spec/controllers/authorize_interactions_controller_spec.rb
index 81fd9ceb766df46ce4aa1415b60a33d619e6746d..ce4257b68dcda85e45156d0e57c99f186976a58c 100644 (file)
--- a/spec/controllers/authorize_interactions_controller_spec.rb
+++ b/spec/controllers/authorize_interactions_controller_spec.rb
@@ -99,10 +99,12 @@ describe AuthorizeInteractionsController do
 
         allow(ResolveAccountService).to receive(:new).and_return(service)
         allow(service).to receive(:call).with('user@hostname').and_return(target_account)
+        allow(service).to receive(:call).with(target_account, skip_webfinger: true).and_return(target_account)
+
 
         post :create, params: { acct: 'acct:user@hostname' }
 
-        expect(service).to have_received(:call).with('user@hostname')
+        expect(service).to have_received(:call).with(target_account, skip_webfinger: true)
         expect(account.following?(target_account)).to be true
         expect(response).to render_template(:success)
       end