From 9f5b55ad4f6788f2a2e70a0d11bf12bcc121653d Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 12 Mar 2019 22:58:59 +0100 Subject: [PATCH] Fix poll update handler calling method was that was not available (#10246) * Fix poll update handler calling method was that was not available Fix regression from #10209 * Refactor VoteService * Refactor ActivityPub::DistributePollUpdateWorker and optimize it * Fix typo * Fix typo --- app/helpers/jsonld_helper.rb | 9 ++++++ app/lib/activitypub/activity/create.rb | 18 ++++------- app/lib/activitypub/activity/delete.rb | 9 ------ app/lib/activitypub/activity/update.rb | 14 ++++---- app/models/concerns/expireable.rb | 6 +++- .../activitypub/fetch_replies_service.rb | 9 ------ app/services/notify_service.rb | 2 +- app/services/vote_service.rb | 32 +++++++++++++------ .../distribute_poll_update_worker.rb | 15 +++++---- 9 files changed, 61 insertions(+), 53 deletions(-) diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb index f0a19e332..5b4011275 100644 --- a/app/helpers/jsonld_helper.rb +++ b/app/helpers/jsonld_helper.rb @@ -47,6 +47,15 @@ module JsonLdHelper !uri.start_with?('http://', 'https://') end + def invalid_origin?(url) + return true if unsupported_uri_scheme?(url) + + needle = Addressable::URI.parse(url).host + haystack = Addressable::URI.parse(@account.uri).host + + !haystack.casecmp(needle).zero? + end + def canonicalize(json) graph = RDF::Graph.new << JSON::LD::API.toRdf(json, documentLoader: method(:load_jsonld_context)) graph.dump(:normalize) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index b49806ecd..8fe7b9138 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -241,9 +241,12 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def poll_vote? return false if replied_to_status.nil? || replied_to_status.poll.nil? || !replied_to_status.local? || !replied_to_status.poll.options.include?(@object['name']) - return true if replied_to_status.poll.expired? - replied_to_status.poll.votes.create!(account: @account, choice: replied_to_status.poll.options.index(@object['name']), uri: @object['id']) - ActivityPub::DistributePollUpdateWorker.perform_in(3.minutes, replied_to_status.id) unless replied_to_status.poll.hide_totals + + unless replied_to_status.poll.expired? + replied_to_status.poll.votes.create!(account: @account, choice: replied_to_status.poll.options.index(@object['name']), uri: @object['id']) + ActivityPub::DistributePollUpdateWorker.perform_in(3.minutes, replied_to_status.id) unless replied_to_status.poll.hide_totals? + end + true end @@ -371,15 +374,6 @@ class ActivityPub::Activity::Create < ActivityPub::Activity @skip_download ||= DomainBlock.find_by(domain: @account.domain)&.reject_media? end - def invalid_origin?(url) - return true if unsupported_uri_scheme?(url) - - needle = Addressable::URI.parse(url).host - haystack = Addressable::URI.parse(@account.uri).host - - !haystack.casecmp(needle).zero? - end - def reply_to_local? !replied_to_status.nil? && replied_to_status.account.local? end diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index dc76dd3e2..4236af071 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -75,13 +75,4 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity def lock_options { redis: Redis.current, key: "create:#{object_uri}" } end - - def invalid_origin?(url) - return true if unsupported_uri_scheme?(url) - - needle = Addressable::URI.parse(url).host - haystack = Addressable::URI.parse(@account.uri).host - - !haystack.casecmp(needle).zero? - end end diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index 5f15f5274..bc9a63f98 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -4,8 +4,11 @@ class ActivityPub::Activity::Update < ActivityPub::Activity SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze def perform - update_account if equals_or_includes_any?(@object['type'], SUPPORTED_TYPES) - update_poll if equals_or_includes_any?(@object['type'], %w(Question)) + if equals_or_includes_any?(@object['type'], SUPPORTED_TYPES) + update_account + elsif equals_or_includes_any?(@object['type'], %w(Question)) + update_poll + end end private @@ -18,11 +21,10 @@ class ActivityPub::Activity::Update < ActivityPub::Activity def update_poll return reject_payload! if invalid_origin?(@object['id']) + status = Status.find_by(uri: object_uri, account_id: @account.id) - return if status.nil? || status.poll_id.nil? - poll = Poll.find(status.poll_id) - return if poll.nil? + return if status.nil? || status.poll.nil? - ActivityPub::ProcessPollService.new.call(poll, @object) + ActivityPub::ProcessPollService.new.call(status.poll, @object) end end diff --git a/app/models/concerns/expireable.rb b/app/models/concerns/expireable.rb index 2c0631476..f7d2bab49 100644 --- a/app/models/concerns/expireable.rb +++ b/app/models/concerns/expireable.rb @@ -18,7 +18,11 @@ module Expireable end def expired? - !expires_at.nil? && expires_at < Time.now.utc + expires? && expires_at < Time.now.utc + end + + def expires? + !expires_at.nil? end end end diff --git a/app/services/activitypub/fetch_replies_service.rb b/app/services/activitypub/fetch_replies_service.rb index 569d0d7c1..8cb309e52 100644 --- a/app/services/activitypub/fetch_replies_service.rb +++ b/app/services/activitypub/fetch_replies_service.rb @@ -46,13 +46,4 @@ class ActivityPub::FetchRepliesService < BaseService # Also limit to 5 fetched replies to limit potential for DoS. @items.map { |item| value_or_id(item) }.reject { |uri| invalid_origin?(uri) }.take(5) end - - def invalid_origin?(url) - return true if unsupported_uri_scheme?(url) - - needle = Addressable::URI.parse(url).host - haystack = Addressable::URI.parse(@account.uri).host - - !haystack.casecmp(needle).zero? - end end diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index 7a86879f0..b5c721589 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -92,7 +92,7 @@ class NotifyService < BaseService def blocked? blocked = @recipient.suspended? # Skip if the recipient account is suspended anyway - blocked ||= from_self? unless @notification.type == :poll # Skip for interactions with self + blocked ||= from_self? && @notification.type != :poll # Skip for interactions with self return blocked if message? && from_staff? diff --git a/app/services/vote_service.rb b/app/services/vote_service.rb index 34a1fe2aa..0cace6c00 100644 --- a/app/services/vote_service.rb +++ b/app/services/vote_service.rb @@ -20,21 +20,35 @@ class VoteService < BaseService end if @poll.account.local? - ActivityPub::DistributePollUpdateWorker.perform_in(3.minutes, @poll.status.id) unless @poll.hide_totals + distribute_poll! else - @votes.each do |vote| - ActivityPub::DeliveryWorker.perform_async( - build_json(vote), - @account.id, - @poll.account.inbox_url - ) - end - PollExpirationNotifyWorker.perform_at(@poll.expires_at + 5.minutes, @poll.id) unless @poll.expires_at.nil? + deliver_votes! + queue_final_poll_check! end end private + def distribute_poll! + return if @poll.hide_totals? + ActivityPub::DistributePollUpdateWorker.perform_in(3.minutes, @poll.status.id) + end + + def queue_final_poll_check! + return unless @poll.expires? + PollExpirationNotifyWorker.perform_at(@poll.expires_at + 5.minutes, @poll.id) + end + + def deliver_votes! + @votes.each do |vote| + ActivityPub::DeliveryWorker.perform_async( + build_json(vote), + @account.id, + @poll.account.inbox_url + ) + end + end + def build_json(vote) ActiveModelSerializers::SerializableResource.new( vote, diff --git a/app/workers/activitypub/distribute_poll_update_worker.rb b/app/workers/activitypub/distribute_poll_update_worker.rb index 718279c1b..d60fde557 100644 --- a/app/workers/activitypub/distribute_poll_update_worker.rb +++ b/app/workers/activitypub/distribute_poll_update_worker.rb @@ -28,13 +28,16 @@ class ActivityPub::DistributePollUpdateWorker def inboxes return @inboxes if defined?(@inboxes) - target_accounts = @status.mentions.map(&:account).reject(&:local?) - target_accounts += @status.reblogs.map(&:account).reject(&:local?) - target_accounts += @status.poll.votes.map(&:account).reject(&:local?) - target_accounts.uniq!(&:id) - @inboxes = target_accounts.select(&:activitypub?).pluck(&:inbox_url) - @inboxes += @account.followers.inboxes unless @status.direct_visibility? + + @inboxes = [@status.mentions, @status.reblogs, @status.poll.votes].flat_map do |relation| + relation.includes(:account).map do |record| + record.account.preferred_inbox_url if !record.account.local? && record.account.activitypub? + end + end + + @inboxes.concat(@account.followers.inboxes) unless @status.direct_visibility? @inboxes.uniq! + @inboxes.compact! @inboxes end