Browse Source

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
Eugen Rochko 5 years ago
parent
commit
9f5b55ad4f

+ 9 - 0
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)

+ 6 - 12
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

+ 0 - 9
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

+ 8 - 6
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

+ 5 - 1
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

+ 0 - 9
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

+ 1 - 1
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?
 

+ 23 - 9
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,

+ 9 - 6
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