From 388d093beb2950553f3806a72e72afe77ecc23ac Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 13 Oct 2017 16:44:02 +0200 Subject: When unfollowing, remove from home in web UI immediately (#5369) Do NOT send "delete" through streaming API when unmerging from home timeline. "delete" implies that the original status was deleted, which is not true! --- app/lib/feed_manager.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/lib') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 6398aa6d6..100f6c8f8 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -85,7 +85,7 @@ class FeedManager oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true)&.first&.last&.to_i || 0 from_account.statuses.select('id, reblog_of_id').where('id > ?', oldest_home_score).reorder(nil).find_each do |status| - unpush(:home, into_account, status) + remove_from_feed(:home, into_account, status) end end -- 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/lib') 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 df7dbc41ae08647a02b2171fd1bfce143acec610 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 13 Oct 2017 16:44:43 +0200 Subject: Fix NameError: uninitialized constant OStatus::AtomSerializer::TagManager (#5371) This error occurred at least in development environment --- app/lib/ostatus/atom_serializer.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'app/lib') diff --git a/app/lib/ostatus/atom_serializer.rb b/app/lib/ostatus/atom_serializer.rb index a1ac11a51..3ca6c5943 100644 --- a/app/lib/ostatus/atom_serializer.rb +++ b/app/lib/ostatus/atom_serializer.rb @@ -23,7 +23,7 @@ class OStatus::AtomSerializer append_element(author, 'name', account.username) append_element(author, 'email', account.local? ? account.local_username_and_domain : account.acct) append_element(author, 'summary', Formatter.instance.simplified_format(account).to_str, type: :html) if account.note? - append_element(author, 'link', nil, rel: :alternate, type: 'text/html', href: TagManager.instance.url_for(account)) + append_element(author, 'link', nil, rel: :alternate, type: 'text/html', href: ::TagManager.instance.url_for(account)) append_element(author, 'link', nil, rel: :avatar, type: account.avatar_content_type, 'media:width': 120, 'media:height': 120, href: full_asset_url(account.avatar.url(:original))) if account.avatar? append_element(author, 'link', nil, rel: :header, type: account.header_content_type, 'media:width': 700, 'media:height': 335, href: full_asset_url(account.header.url(:original))) if account.header? append_element(author, 'poco:preferredUsername', account.username) @@ -47,7 +47,7 @@ class OStatus::AtomSerializer feed << author(account) - append_element(feed, 'link', nil, rel: :alternate, type: 'text/html', href: TagManager.instance.url_for(account)) + append_element(feed, 'link', nil, rel: :alternate, type: 'text/html', href: ::TagManager.instance.url_for(account)) append_element(feed, 'link', nil, rel: :self, type: 'application/atom+xml', href: account_url(account, format: 'atom')) append_element(feed, 'link', nil, rel: :next, type: 'application/atom+xml', href: account_url(account, format: 'atom', max_id: stream_entries.last.id)) if stream_entries.size == 20 append_element(feed, 'link', nil, rel: :hub, href: api_push_url) @@ -86,9 +86,9 @@ class OStatus::AtomSerializer serialize_status_attributes(entry, stream_entry.status) end - append_element(entry, 'link', nil, rel: :alternate, type: 'text/html', href: TagManager.instance.url_for(stream_entry.status)) + append_element(entry, 'link', nil, rel: :alternate, type: 'text/html', href: ::TagManager.instance.url_for(stream_entry.status)) append_element(entry, 'link', nil, rel: :self, type: 'application/atom+xml', href: account_stream_entry_url(stream_entry.account, stream_entry, format: 'atom')) - append_element(entry, 'thr:in-reply-to', nil, ref: OStatus::TagManager.instance.uri_for(stream_entry.thread), href: TagManager.instance.url_for(stream_entry.thread)) if stream_entry.threaded? + append_element(entry, 'thr:in-reply-to', nil, ref: OStatus::TagManager.instance.uri_for(stream_entry.thread), href: ::TagManager.instance.url_for(stream_entry.thread)) if stream_entry.threaded? append_element(entry, 'ostatus:conversation', nil, ref: conversation_uri(stream_entry.status.conversation)) unless stream_entry&.status&.conversation_id.nil? entry @@ -109,8 +109,8 @@ class OStatus::AtomSerializer serialize_status_attributes(object, status) - append_element(object, 'link', nil, rel: :alternate, type: 'text/html', href: TagManager.instance.url_for(status)) - append_element(object, 'thr:in-reply-to', nil, ref: OStatus::TagManager.instance.uri_for(status.thread), href: TagManager.instance.url_for(status.thread)) unless status.thread.nil? + append_element(object, 'link', nil, rel: :alternate, type: 'text/html', href: ::TagManager.instance.url_for(status)) + append_element(object, 'thr:in-reply-to', nil, ref: OStatus::TagManager.instance.uri_for(status.thread), href: ::TagManager.instance.url_for(status.thread)) unless status.thread.nil? append_element(object, 'ostatus:conversation', nil, ref: conversation_uri(status.conversation)) unless status.conversation_id.nil? object @@ -290,7 +290,7 @@ class OStatus::AtomSerializer entry << object(favourite.status) - append_element(entry, 'thr:in-reply-to', nil, ref: OStatus::TagManager.instance.uri_for(favourite.status), href: TagManager.instance.url_for(favourite.status)) + append_element(entry, 'thr:in-reply-to', nil, ref: OStatus::TagManager.instance.uri_for(favourite.status), href: ::TagManager.instance.url_for(favourite.status)) entry end @@ -312,7 +312,7 @@ class OStatus::AtomSerializer entry << object(favourite.status) - append_element(entry, 'thr:in-reply-to', nil, ref: OStatus::TagManager.instance.uri_for(favourite.status), href: TagManager.instance.url_for(favourite.status)) + append_element(entry, 'thr:in-reply-to', nil, ref: OStatus::TagManager.instance.uri_for(favourite.status), href: ::TagManager.instance.url_for(favourite.status)) entry end -- cgit From 8125fdc19f310a22fdd1395365c2f02d6887b389 Mon Sep 17 00:00:00 2001 From: unarist Date: Sat, 14 Oct 2017 21:42:09 +0900 Subject: Use atomUri in Undo activity of Announce (#5376) This allows deletion of reblogs which delivered before with OStatus URI. --- app/lib/activitypub/activity/undo.rb | 3 ++- app/serializers/activitypub/activity_serializer.rb | 5 +++++ spec/lib/activitypub/activity/undo_spec.rb | 24 +++++++++++++++++----- 3 files changed, 26 insertions(+), 6 deletions(-) (limited to 'app/lib') diff --git a/app/lib/activitypub/activity/undo.rb b/app/lib/activitypub/activity/undo.rb index 4b0905de2..cbed417c4 100644 --- a/app/lib/activitypub/activity/undo.rb +++ b/app/lib/activitypub/activity/undo.rb @@ -17,7 +17,8 @@ class ActivityPub::Activity::Undo < ActivityPub::Activity private def undo_announce - status = Status.find_by(uri: object_uri, account: @account) + status = Status.find_by(uri: object_uri, account: @account) + status ||= Status.find_by(uri: @object['atomUri'], account: @account) if @object.is_a?(Hash) && @object['atomUri'].present? if status.nil? delete_later!(object_uri) diff --git a/app/serializers/activitypub/activity_serializer.rb b/app/serializers/activitypub/activity_serializer.rb index df399211c..50c4f6a04 100644 --- a/app/serializers/activitypub/activity_serializer.rb +++ b/app/serializers/activitypub/activity_serializer.rb @@ -5,6 +5,7 @@ class ActivityPub::ActivitySerializer < ActiveModel::Serializer has_one :proper, key: :object, serializer: ActivityPub::NoteSerializer, unless: :announce? attribute :proper_uri, key: :object, if: :announce? + attribute :atom_uri, if: :announce? def id ActivityPub::TagManager.instance.activity_uri_for(object) @@ -34,6 +35,10 @@ class ActivityPub::ActivitySerializer < ActiveModel::Serializer ActivityPub::TagManager.instance.uri_for(object.proper) end + def atom_uri + OStatus::TagManager.instance.uri_for(object) + end + def announce? object.reblog? end diff --git a/spec/lib/activitypub/activity/undo_spec.rb b/spec/lib/activitypub/activity/undo_spec.rb index 14c68efe5..e01c5e03e 100644 --- a/spec/lib/activitypub/activity/undo_spec.rb +++ b/spec/lib/activitypub/activity/undo_spec.rb @@ -25,16 +25,30 @@ RSpec.describe ActivityPub::Activity::Undo do type: 'Announce', actor: ActivityPub::TagManager.instance.uri_for(sender), object: ActivityPub::TagManager.instance.uri_for(status), + atomUri: 'barbar', } end - before do - Fabricate(:status, reblog: status, account: sender, uri: 'bar') + context do + before do + Fabricate(:status, reblog: status, account: sender, uri: 'bar') + end + + it 'deletes the reblog' do + subject.perform + expect(sender.reblogged?(status)).to be false + end end - it 'deletes the reblog' do - subject.perform - expect(sender.reblogged?(status)).to be false + context 'with atomUri' do + before do + Fabricate(:status, reblog: status, account: sender, uri: 'barbar') + end + + it 'deletes the reblog by atomUri' do + subject.perform + expect(sender.reblogged?(status)).to be false + end end end -- cgit