Fix being able to import more than allowed number of follows (#15384)

* Fix being able to import more than allowed number of follows

Without this commit, if someone tries importing a second list of accounts to
follow before the first one has been processed, this will queue imports for
the two whole lists, even if they exceed the account's allowed number of
outgoing follows.

This commit changes it so the individual queued imports aren't exempt from
the follow limit check (they remain exempt from the rate-limiting check
though).

* Catch validation errors to not re-queue failed follows

Co-authored-by: Claire <claire.github-309c@sitedethib.com>

diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb
index e2c4b8acf598b2f8f4a58b525cf9de3d3e1a5350..974f57820d08332436492d5b0b3bc2cb65fdfa35 100644 (file)
--- a/app/models/concerns/account_interactions.rb
+++ b/app/models/concerns/account_interactions.rb
@@ -97,8 +97,8 @@ module AccountInteractions
     has_many :announcement_mutes, dependent: :destroy
   end
 
-  def follow!(other_account, reblogs: nil, notify: nil, uri: nil, rate_limit: false)
-    rel = active_relationships.create_with(show_reblogs: reblogs.nil? ? true : reblogs, notify: notify.nil? ? false : notify, uri: uri, rate_limit: rate_limit)
+  def follow!(other_account, reblogs: nil, notify: nil, uri: nil, rate_limit: false, bypass_limit: false)
+    rel = active_relationships.create_with(show_reblogs: reblogs.nil? ? true : reblogs, notify: notify.nil? ? false : notify, uri: uri, rate_limit: rate_limit, bypass_follow_limit: bypass_limit)
                               .find_or_create_by!(target_account: other_account)
 
     rel.show_reblogs = reblogs unless reblogs.nil?
@@ -111,8 +111,8 @@ module AccountInteractions
     rel
   end
 
-  def request_follow!(other_account, reblogs: nil, notify: nil, uri: nil, rate_limit: false)
-    rel = follow_requests.create_with(show_reblogs: reblogs.nil? ? true : reblogs, notify: notify.nil? ? false : notify, uri: uri, rate_limit: rate_limit)
+  def request_follow!(other_account, reblogs: nil, notify: nil, uri: nil, rate_limit: false, bypass_limit: false)
+    rel = follow_requests.create_with(show_reblogs: reblogs.nil? ? true : reblogs, notify: notify.nil? ? false : notify, uri: uri, rate_limit: rate_limit, bypass_follow_limit: bypass_limit)
                          .find_or_create_by!(target_account: other_account)
 
     rel.show_reblogs = reblogs unless reblogs.nil?

diff --git a/app/models/concerns/follow_limitable.rb b/app/models/concerns/follow_limitable.rb
new file mode 100644 (file)
index 0000000..c64060d
--- /dev/null
+++ b/app/models/concerns/follow_limitable.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module FollowLimitable
+  extend ActiveSupport::Concern
+
+  included do
+    validates_with FollowLimitValidator, on: :create, unless: :bypass_follow_limit?
+  end
+
+  def bypass_follow_limit=(value)
+    @bypass_follow_limit = value
+  end
+
+  def bypass_follow_limit?
+    @bypass_follow_limit
+  end
+end

diff --git a/app/models/follow.rb b/app/models/follow.rb
index 69a1722b303ac3f22dabcbc03ca67d45cbcf83da..a5e3fe8099516c717b184316a3d455b4650a8782 100644 (file)
--- a/app/models/follow.rb
+++ b/app/models/follow.rb
@@ -17,6 +17,7 @@ class Follow < ApplicationRecord
   include Paginable
   include RelationshipCacheable
   include RateLimitable
+  include FollowLimitable
 
   rate_limit by: :account, family: :follows
 
@@ -26,7 +27,6 @@ class Follow < ApplicationRecord
   has_one :notification, as: :activity, dependent: :destroy
 
   validates :account_id, uniqueness: { scope: :target_account_id }
-  validates_with FollowLimitValidator, on: :create, if: :rate_limit?
 
   scope :recent, -> { reorder(id: :desc) }
 

diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb
index 2d2a77b59c6efa9d95a89a44491833dc8ccb6d5a..59fefcdf64e402cb87671676421fb56beabd41bd 100644 (file)
--- a/app/models/follow_request.rb
+++ b/app/models/follow_request.rb
@@ -17,6 +17,7 @@ class FollowRequest < ApplicationRecord
   include Paginable
   include RelationshipCacheable
   include RateLimitable
+  include FollowLimitable
 
   rate_limit by: :account, family: :follows
 
@@ -26,7 +27,6 @@ class FollowRequest < ApplicationRecord
   has_one :notification, as: :activity, dependent: :destroy
 
   validates :account_id, uniqueness: { scope: :target_account_id }
-  validates_with FollowLimitValidator, on: :create, if: :rate_limit?
 
   def authorize!
     account.follow!(target_account, reblogs: show_reblogs, notify: notify, uri: uri)

diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb
index 9625728510c910e2bedb12dc84aa43cd18ed2339..b98f7011d1887e55460ef042f2c99127293bed75 100644 (file)
--- a/app/services/follow_service.rb
+++ b/app/services/follow_service.rb
@@ -11,11 +11,12 @@ class FollowService < BaseService
   # @option [Boolean] :reblogs Whether or not to show reblogs, defaults to true
   # @option [Boolean] :notify Whether to create notifications about new posts, defaults to false
   # @option [Boolean] :bypass_locked
+  # @option [Boolean] :bypass_limit Allow following past the total follow number
   # @option [Boolean] :with_rate_limit
   def call(source_account, target_account, options = {})
     @source_account = source_account
     @target_account = ResolveAccountService.new.call(target_account, skip_webfinger: true)
-    @options        = { bypass_locked: false, with_rate_limit: false }.merge(options)
+    @options        = { bypass_locked: false, bypass_limit: false, with_rate_limit: false }.merge(options)
 
     raise ActiveRecord::RecordNotFound if following_not_possible?
     raise Mastodon::NotPermittedError  if following_not_allowed?
@@ -54,7 +55,7 @@ class FollowService < BaseService
   end
 
   def request_follow!
-    follow_request = @source_account.request_follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify], rate_limit: @options[:with_rate_limit])
+    follow_request = @source_account.request_follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify], rate_limit: @options[:with_rate_limit], bypass_limit: @options[:bypass_limit])
 
     if @target_account.local?
       LocalNotificationWorker.perform_async(@target_account.id, follow_request.id, follow_request.class.name, :follow_request)
@@ -66,7 +67,7 @@ class FollowService < BaseService
   end
 
   def direct_follow!
-    follow = @source_account.follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify], rate_limit: @options[:with_rate_limit])
+    follow = @source_account.follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify], rate_limit: @options[:with_rate_limit], bypass_limit: @options[:bypass_limit])
 
     LocalNotificationWorker.perform_async(@target_account.id, follow.id, follow.class.name, :follow)
     MergeWorker.perform_async(@target_account.id, @source_account.id)

diff --git a/app/workers/authorize_follow_worker.rb b/app/workers/authorize_follow_worker.rb
index 0d50146246301af88653513a6e4200e3eadd7476..f57900fa59a31b902d9f2e6d36c56a95ad5537a5 100644 (file)
--- a/app/workers/authorize_follow_worker.rb
+++ b/app/workers/authorize_follow_worker.rb
@@ -7,7 +7,7 @@ class AuthorizeFollowWorker
     source_account = Account.find(source_account_id)
     target_account = Account.find(target_account_id)
 
-    AuthorizeFollowService.new.call(source_account, target_account)
+    AuthorizeFollowService.new.call(source_account, target_account, bypass_limit: true)
   rescue ActiveRecord::RecordNotFound
     true
   end

diff --git a/app/workers/import/relationship_worker.rb b/app/workers/import/relationship_worker.rb
index 4a455f3aebd3003904f2578f0151d288ef83ee75..4a7100435f7dc42a89fd133dc8e7ae6186a3928a 100644 (file)
--- a/app/workers/import/relationship_worker.rb
+++ b/app/workers/import/relationship_worker.rb
@@ -15,7 +15,11 @@ class Import::RelationshipWorker
 
     case relationship
     when 'follow'
-      FollowService.new.call(from_account, target_account, options)
+      begin
+        FollowService.new.call(from_account, target_account, options)
+      rescue ActiveRecord::RecordInvalid
+        raise if FollowLimitValidator.limit_for_account(from_account) < from_account.following_count
+      end
     when 'unfollow'
       UnfollowService.new.call(from_account, target_account)
     when 'block'

diff --git a/app/workers/refollow_worker.rb b/app/workers/refollow_worker.rb
index 98940680d07156a36bd2753689d27e6601ddfe53..319b001097b4374a1416865122256d39fd117bb3 100644 (file)
--- a/app/workers/refollow_worker.rb
+++ b/app/workers/refollow_worker.rb
@@ -19,7 +19,7 @@ class RefollowWorker
 
       # Schedule re-follow
       begin
-        FollowService.new.call(follower, target_account, reblogs: reblogs, notify: notify)
+        FollowService.new.call(follower, target_account, reblogs: reblogs, notify: notify, bypass_limit: true)
       rescue Mastodon::NotPermittedError, ActiveRecord::RecordNotFound, Mastodon::UnexpectedResponseError, HTTP::Error, OpenSSL::SSL::SSLError
         next
       end

diff --git a/app/workers/unfollow_follow_worker.rb b/app/workers/unfollow_follow_worker.rb
index 71b5a0e3fa580b4891060cbc9106ca82d8fea5f2..0bd5ff472e845d03eb5d5a03fe9b3632bc5e8424 100644 (file)
--- a/app/workers/unfollow_follow_worker.rb
+++ b/app/workers/unfollow_follow_worker.rb
@@ -14,7 +14,7 @@ class UnfollowFollowWorker
     reblogs = follow&.show_reblogs?
     notify  = follow&.notify?
 
-    FollowService.new.call(follower_account, new_target_account, reblogs: reblogs, notify: notify, bypass_locked: bypass_locked)
+    FollowService.new.call(follower_account, new_target_account, reblogs: reblogs, notify: notify, bypass_locked: bypass_locked, bypass_limit: true)
     UnfollowService.new.call(follower_account, old_target_account, skip_unmerge: true)
   rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError
     true

diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb
index cdd1db9954e5eb24b1af1aa3d0781d0a4ed31730..653bfca3010a07710d2cd59a68e3c17c9aab55fd 100644 (file)
--- a/lib/mastodon/accounts_cli.rb
+++ b/lib/mastodon/accounts_cli.rb
@@ -384,7 +384,7 @@ module Mastodon
       end
 
       processed, = parallelize_with_progress(Account.local.without_suspended) do |account|
-        FollowService.new.call(account, target_account)
+        FollowService.new.call(account, target_account, bypass_limit: true)
       end
 
       say("OK, followed target from #{processed} accounts", :green)

diff --git a/spec/workers/refollow_worker_spec.rb b/spec/workers/refollow_worker_spec.rb
index 6b4c042912bd72b66b08d153896b6b3e716ad221..df6731b6400041db44247083945c7618ff166528 100644 (file)
--- a/spec/workers/refollow_worker_spec.rb
+++ b/spec/workers/refollow_worker_spec.rb
@@ -23,8 +23,8 @@ describe RefollowWorker do
       result = subject.perform(account.id)
 
       expect(result).to be_nil
-      expect(service).to have_received(:call).with(alice, account, reblogs: true, notify: false)
-      expect(service).to have_received(:call).with(bob, account, reblogs: false, notify: false)
+      expect(service).to have_received(:call).with(alice, account, reblogs: true, notify: false, bypass_limit: true)
+      expect(service).to have_received(:call).with(bob, account, reblogs: false, notify: false, bypass_limit: true)
     end
   end
 end