From b8bae9664711f99bfb438687ff3b4b396db69924 Mon Sep 17 00:00:00 2001 From: Lex Alexander Date: Thu, 12 Oct 2017 14:52:09 -1000 Subject: Retoot count increases without reason (#5363) * Retoot count increases without reason -The store_uri method for Statuses was being called on after_create and causing reblogs to be incremented twice. -This calls it when the transaction is finished by using after_create_commit. -Fixes #4916. * Added test case for after_create_commit callback for checking reblog count. * Rewrote test to keep original, but added one for only the after_create_commit callback. --- app/models/status.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models/status.rb') diff --git a/app/models/status.rb b/app/models/status.rb index 0d249244f..6db1f2a4c 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -134,7 +134,7 @@ class Status < ApplicationRecord CustomEmoji.from_text([spoiler_text, text].join(' '), account.domain) end - after_create :store_uri, if: :local? + after_create_commit :store_uri, if: :local? around_create Mastodon::Snowflake::Callbacks -- cgit From b8db386e056fe236ed3f41e563a824e30733c4ce Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 13 Oct 2017 16:44:29 +0200 Subject: Fix UserTrackingConcern firing on every request, optimize some queries (#5368) - For some reason, :if option on before_action did not work. It got executed every time, returned false, and the action run anyway, which led to the current_sign_in_at and sign_in_count being updated on every request - Return "do not filter" early in FeedManager#filter_from_home? if the status is authored by receiver. Usually this method is not called for own statuses at all, but it is called when Feed#get uses the database - Return early if #reload_stale_associations! has nothing to load to save a database query with WHERE 1=0 --- app/controllers/concerns/user_tracking_concern.rb | 6 ++++-- app/lib/feed_manager.rb | 3 ++- app/models/notification.rb | 5 ++++- app/models/status.rb | 6 +++++- 4 files changed, 15 insertions(+), 5 deletions(-) (limited to 'app/models/status.rb') diff --git a/app/controllers/concerns/user_tracking_concern.rb b/app/controllers/concerns/user_tracking_concern.rb index 8a63af95d..8663c3086 100644 --- a/app/controllers/concerns/user_tracking_concern.rb +++ b/app/controllers/concerns/user_tracking_concern.rb @@ -7,12 +7,14 @@ module UserTrackingConcern UPDATE_SIGN_IN_HOURS = 24 included do - before_action :set_user_activity, if: %i(user_signed_in? user_needs_sign_in_update?) + before_action :set_user_activity end private def set_user_activity + return unless user_needs_sign_in_update? + # Mark as signed-in today current_user.update_tracked_fields!(request) @@ -21,7 +23,7 @@ module UserTrackingConcern end def user_needs_sign_in_update? - current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < UPDATE_SIGN_IN_HOURS.hours.ago + user_signed_in? && (current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < UPDATE_SIGN_IN_HOURS.hours.ago) end def user_needs_feed_update? diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 100f6c8f8..89aeaadcd 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -115,7 +115,8 @@ class FeedManager end def filter_from_home?(status, receiver_id) - return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?) + return false if receiver_id == status.account_id + return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?) check_for_mutes = [status.account_id] check_for_mutes.concat([status.reblog.account_id]) if status.reblog? diff --git a/app/models/notification.rb b/app/models/notification.rb index 1e64d1ae9..0a5d987cf 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -68,7 +68,10 @@ class Notification < ApplicationRecord class << self def reload_stale_associations!(cached_items) account_ids = cached_items.map(&:from_account_id).uniq - accounts = Account.where(id: account_ids).map { |a| [a.id, a] }.to_h + + return if account_ids.empty? + + accounts = Account.where(id: account_ids).map { |a| [a.id, a] }.to_h cached_items.each do |item| item.from_account = accounts[item.from_account_id] diff --git a/app/models/status.rb b/app/models/status.rb index 6db1f2a4c..624e33395 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -194,7 +194,11 @@ class Status < ApplicationRecord account_ids << item.reblog.account_id if item.reblog? end - accounts = Account.where(id: account_ids.uniq).map { |a| [a.id, a] }.to_h + account_ids.uniq! + + return if account_ids.empty? + + accounts = Account.where(id: account_ids).map { |a| [a.id, a] }.to_h cached_items.each do |item| item.account = accounts[item.account_id] -- cgit From a1c54220e8e79c4bb0cc20da193888388efe2ba4 Mon Sep 17 00:00:00 2001 From: unarist Date: Fri, 13 Oct 2017 23:53:44 +0900 Subject: Optimize Status#permitted_for 500x (account timeline) (#5373) The main change of this PR is removing `order by visibility` hack. This was introduced to force using of `index_statuses_on_account_id` instead of PK index, but it seems no longer needed probably due to `index_statuses_on_account_id_id`. Removing this avoids reading all rows, so really improves first fetching of the user who has lot of statuses. I have also changed JOIN to IN + subquery, which slightly faster in most cases. --- app/models/status.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'app/models/status.rb') diff --git a/app/models/status.rb b/app/models/status.rb index 624e33395..5a7245613 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -220,9 +220,7 @@ class Status < ApplicationRecord # non-followers can see everything that isn't private/direct, but can see stuff they are mentioned in. visibility.push(:private) if account.following?(target_account) - joins("LEFT OUTER JOIN mentions ON statuses.id = mentions.status_id AND mentions.account_id = #{account.id}") - .where(arel_table[:visibility].in(visibility).or(Mention.arel_table[:id].not_eq(nil))) - .order(visibility: :desc) + where(visibility: visibility).or(where(id: account.mentions.select(:status_id))) end end -- cgit