From 3a92885a860df12b12d8356faf179a3fc63be6f2 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 11 Mar 2019 00:49:31 +0100 Subject: [PATCH 1/3] Support pushing and receiving updates to poll tallies (#10209) * Process incoming poll tallies update * Send Update on poll vote * Do not send Updates for a poll more often than once every 3 minutes * Include voters in people to notify of results update * Schedule closing poll worker on poll creation * Add new notification type for ending polls * Add front-end support for ended poll notifications * Fix UpdatePollSerializer * Fix Updates not being triggered by local votes * Fix tests failure * Fix web push notifications for closing polls * Minor cleanup * Notify voters of both remote and local polls when those close * Fix delivery of poll updates to mentioned accounts and voters --- .../mastodon/actions/notifications.js | 2 +- .../notifications/components/notification.js | 34 ++++++++++ app/javascript/mastodon/locales/en.json | 1 + app/javascript/mastodon/reducers/settings.js | 3 + .../service_worker/web_push_locales.js | 1 + app/lib/activitypub/activity/create.rb | 2 + app/lib/activitypub/activity/update.rb | 11 ++++ app/models/notification.rb | 8 ++- .../activitypub/update_poll_serializer.rb | 27 ++++++++ .../rest/notification_serializer.rb | 2 +- .../activitypub/fetch_remote_poll_service.rb | 51 +-------------- .../activitypub/process_poll_service.rb | 64 +++++++++++++++++++ app/services/notify_service.rb | 6 +- app/services/post_status_service.rb | 1 + app/services/vote_service.rb | 19 +++--- .../distribute_poll_update_worker.rb | 62 ++++++++++++++++++ app/workers/poll_expiration_notify_worker.rb | 24 +++++++ 17 files changed, 256 insertions(+), 62 deletions(-) create mode 100644 app/serializers/activitypub/update_poll_serializer.rb create mode 100644 app/services/activitypub/process_poll_service.rb create mode 100644 app/workers/activitypub/distribute_poll_update_worker.rb create mode 100644 app/workers/poll_expiration_notify_worker.rb diff --git a/app/javascript/mastodon/actions/notifications.js b/app/javascript/mastodon/actions/notifications.js index 4c145febc..61fef19e9 100644 --- a/app/javascript/mastodon/actions/notifications.js +++ b/app/javascript/mastodon/actions/notifications.js @@ -92,7 +92,7 @@ export function updateNotifications(notification, intlMessages, intlLocale) { const excludeTypesFromSettings = state => state.getIn(['settings', 'notifications', 'shows']).filter(enabled => !enabled).keySeq().toJS(); const excludeTypesFromFilter = filter => { - const allTypes = ImmutableList(['follow', 'favourite', 'reblog', 'mention']); + const allTypes = ImmutableList(['follow', 'favourite', 'reblog', 'mention', 'poll']); return allTypes.filterNot(item => item === filter).toJS(); }; diff --git a/app/javascript/mastodon/features/notifications/components/notification.js b/app/javascript/mastodon/features/notifications/components/notification.js index 9669b6e7d..61023bed6 100644 --- a/app/javascript/mastodon/features/notifications/components/notification.js +++ b/app/javascript/mastodon/features/notifications/components/notification.js @@ -205,6 +205,38 @@ class Notification extends ImmutablePureComponent { ); } + renderPoll (notification) { + const { intl } = this.props; + + return ( + +
+
+
+ +
+ + + + +
+ +
+ + ); + } + render () { const { notification } = this.props; const account = notification.get('account'); @@ -220,6 +252,8 @@ class Notification extends ImmutablePureComponent { return this.renderFavourite(notification, link); case 'reblog': return this.renderReblog(notification, link); + case 'poll': + return this.renderPoll(notification); } return null; diff --git a/app/javascript/mastodon/locales/en.json b/app/javascript/mastodon/locales/en.json index b0d7488ca..d20bc47bc 100644 --- a/app/javascript/mastodon/locales/en.json +++ b/app/javascript/mastodon/locales/en.json @@ -240,6 +240,7 @@ "notification.favourite": "{name} favourited your status", "notification.follow": "{name} followed you", "notification.mention": "{name} mentioned you", + "notification.poll": "Your poll has ended", "notification.reblog": "{name} boosted your status", "notifications.clear": "Clear notifications", "notifications.clear_confirmation": "Are you sure you want to permanently clear all your notifications?", diff --git a/app/javascript/mastodon/reducers/settings.js b/app/javascript/mastodon/reducers/settings.js index 2e1878cf7..a0eea137f 100644 --- a/app/javascript/mastodon/reducers/settings.js +++ b/app/javascript/mastodon/reducers/settings.js @@ -31,6 +31,7 @@ const initialState = ImmutableMap({ favourite: true, reblog: true, mention: true, + poll: true, }), quickFilter: ImmutableMap({ @@ -44,6 +45,7 @@ const initialState = ImmutableMap({ favourite: true, reblog: true, mention: true, + poll: true, }), sounds: ImmutableMap({ @@ -51,6 +53,7 @@ const initialState = ImmutableMap({ favourite: true, reblog: true, mention: true, + poll: true, }), }), diff --git a/app/javascript/mastodon/service_worker/web_push_locales.js b/app/javascript/mastodon/service_worker/web_push_locales.js index ce96ae297..5ce8c7b50 100644 --- a/app/javascript/mastodon/service_worker/web_push_locales.js +++ b/app/javascript/mastodon/service_worker/web_push_locales.js @@ -18,6 +18,7 @@ filenames.forEach(filename => { 'notification.follow': full['notification.follow'] || '', 'notification.mention': full['notification.mention'] || '', 'notification.reblog': full['notification.reblog'] || '', + 'notification.poll': full['notification.poll'] || '', 'status.show_more': full['status.show_more'] || '', 'status.reblog': full['status.reblog'] || '', diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 7e4e57ead..b49806ecd 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -243,6 +243,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity 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 + true end def resolve_thread(status) diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index 67dae3f81..5f15f5274 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -5,6 +5,7 @@ class ActivityPub::Activity::Update < ActivityPub::Activity def perform update_account if equals_or_includes_any?(@object['type'], SUPPORTED_TYPES) + update_poll if equals_or_includes_any?(@object['type'], %w(Question)) end private @@ -14,4 +15,14 @@ class ActivityPub::Activity::Update < ActivityPub::Activity ActivityPub::ProcessAccountService.new.call(@account.username, @account.domain, @object, signed_with_known_key: true) end + + 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? + + ActivityPub::ProcessPollService.new.call(poll, @object) + end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 2f0a9b78c..982136c05 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -22,6 +22,7 @@ class Notification < ApplicationRecord follow: 'Follow', follow_request: 'FollowRequest', favourite: 'Favourite', + poll: 'Poll', }.freeze STATUS_INCLUDES = [:account, :application, :media_attachments, :tags, active_mentions: :account, reblog: [:account, :application, :media_attachments, :tags, active_mentions: :account]].freeze @@ -35,6 +36,7 @@ class Notification < ApplicationRecord belongs_to :follow, foreign_type: 'Follow', foreign_key: 'activity_id', optional: true belongs_to :follow_request, foreign_type: 'FollowRequest', foreign_key: 'activity_id', optional: true belongs_to :favourite, foreign_type: 'Favourite', foreign_key: 'activity_id', optional: true + belongs_to :poll, foreign_type: 'Poll', foreign_key: 'activity_id', optional: true validates :account_id, uniqueness: { scope: [:activity_type, :activity_id] } validates :activity_type, inclusion: { in: TYPE_CLASS_MAP.values } @@ -44,7 +46,7 @@ class Notification < ApplicationRecord where(activity_type: types) } - cache_associated :from_account, status: STATUS_INCLUDES, mention: [status: STATUS_INCLUDES], favourite: [:account, status: STATUS_INCLUDES], follow: :account + cache_associated :from_account, status: STATUS_INCLUDES, mention: [status: STATUS_INCLUDES], favourite: [:account, status: STATUS_INCLUDES], follow: :account, poll: [status: STATUS_INCLUDES] def type @type ||= TYPE_CLASS_MAP.invert[activity_type].to_sym @@ -58,6 +60,8 @@ class Notification < ApplicationRecord favourite&.status when :mention mention&.status + when :poll + poll&.status end end @@ -97,7 +101,7 @@ class Notification < ApplicationRecord return unless new_record? case activity_type - when 'Status', 'Follow', 'Favourite', 'FollowRequest' + when 'Status', 'Follow', 'Favourite', 'FollowRequest', 'Poll' self.from_account_id = activity&.account_id when 'Mention' self.from_account_id = activity&.status&.account_id diff --git a/app/serializers/activitypub/update_poll_serializer.rb b/app/serializers/activitypub/update_poll_serializer.rb new file mode 100644 index 000000000..f7933346f --- /dev/null +++ b/app/serializers/activitypub/update_poll_serializer.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class ActivityPub::UpdatePollSerializer < ActiveModel::Serializer + attributes :id, :type, :actor, :to + + has_one :object, serializer: ActivityPub::NoteSerializer + + def id + [ActivityPub::TagManager.instance.uri_for(object), '#updates/', object.poll.updated_at.to_i].join + end + + def type + 'Update' + end + + def actor + ActivityPub::TagManager.instance.uri_for(object) + end + + def to + ActivityPub::TagManager.instance.to(object) + end + + def cc + ActivityPub::TagManager.instance.cc(object) + end +end diff --git a/app/serializers/rest/notification_serializer.rb b/app/serializers/rest/notification_serializer.rb index 541a6b8b5..80812ad0d 100644 --- a/app/serializers/rest/notification_serializer.rb +++ b/app/serializers/rest/notification_serializer.rb @@ -11,6 +11,6 @@ class REST::NotificationSerializer < ActiveModel::Serializer end def status_type? - [:favourite, :reblog, :mention].include?(object.type) + [:favourite, :reblog, :mention, :poll].include?(object.type) end end diff --git a/app/services/activitypub/fetch_remote_poll_service.rb b/app/services/activitypub/fetch_remote_poll_service.rb index 4f9814fcd..44a23712c 100644 --- a/app/services/activitypub/fetch_remote_poll_service.rb +++ b/app/services/activitypub/fetch_remote_poll_service.rb @@ -4,54 +4,7 @@ class ActivityPub::FetchRemotePollService < BaseService include JsonLdHelper def call(poll, on_behalf_of = nil) - @json = fetch_resource(poll.status.uri, true, on_behalf_of) - - return unless supported_context? && expected_type? - - expires_at = begin - if @json['closed'].is_a?(String) - @json['closed'] - elsif !@json['closed'].nil? && !@json['closed'].is_a?(FalseClass) - Time.now.utc - else - @json['endTime'] - end - end - - items = begin - if @json['anyOf'].is_a?(Array) - @json['anyOf'] - else - @json['oneOf'] - end - end - - latest_options = items.map { |item| item['name'].presence || item['content'] } - - # If for some reasons the options were changed, it invalidates all previous - # votes, so we need to remove them - poll.votes.delete_all if latest_options != poll.options - - begin - poll.update!( - last_fetched_at: Time.now.utc, - expires_at: expires_at, - options: latest_options, - cached_tallies: items.map { |item| item.dig('replies', 'totalItems') || 0 } - ) - rescue ActiveRecord::StaleObjectError - poll.reload - retry - end - end - - private - - def supported_context? - super(@json) - end - - def expected_type? - equals_or_includes_any?(@json['type'], %w(Question)) + json = fetch_resource(poll.status.uri, true, on_behalf_of) + ActivityPub::ProcessPollService.new.call(poll, json) end end diff --git a/app/services/activitypub/process_poll_service.rb b/app/services/activitypub/process_poll_service.rb new file mode 100644 index 000000000..ee248169d --- /dev/null +++ b/app/services/activitypub/process_poll_service.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +class ActivityPub::ProcessPollService < BaseService + include JsonLdHelper + + def call(poll, json) + @json = json + return unless supported_context? && expected_type? + + previous_expires_at = poll.expires_at + + expires_at = begin + if @json['closed'].is_a?(String) + @json['closed'] + elsif !@json['closed'].nil? && !@json['closed'].is_a?(FalseClass) + Time.now.utc + else + @json['endTime'] + end + end + + items = begin + if @json['anyOf'].is_a?(Array) + @json['anyOf'] + else + @json['oneOf'] + end + end + + latest_options = items.map { |item| item['name'].presence || item['content'] } + + # If for some reasons the options were changed, it invalidates all previous + # votes, so we need to remove them + poll.votes.delete_all if latest_options != poll.options + + begin + poll.update!( + last_fetched_at: Time.now.utc, + expires_at: expires_at, + options: latest_options, + cached_tallies: items.map { |item| item.dig('replies', 'totalItems') || 0 } + ) + rescue ActiveRecord::StaleObjectError + poll.reload + retry + end + + # If the poll had no expiration date set but now has, and people have voted, + # schedule a notification. + if previous_expires_at.nil? && poll.expires_at.present? && poll.votes.exists? + PollExpirationNotifyWorker.perform_at(poll.expires_at + 5.minutes, poll.id) + end + end + + private + + def supported_context? + super(@json) + end + + def expected_type? + equals_or_includes_any?(@json['type'], %w(Question)) + end +end diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index b80ceef03..7a86879f0 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -38,6 +38,10 @@ class NotifyService < BaseService false end + def blocked_poll? + false + end + def following_sender? return @following_sender if defined?(@following_sender) @following_sender = @recipient.following?(@notification.from_account) || @recipient.requested?(@notification.from_account) @@ -88,7 +92,7 @@ class NotifyService < BaseService def blocked? blocked = @recipient.suspended? # Skip if the recipient account is suspended anyway - blocked ||= from_self? # Skip for interactions with self + blocked ||= from_self? unless @notification.type == :poll # Skip for interactions with self return blocked if message? && from_staff? diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index c045a553e..a1705a6ad 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -90,6 +90,7 @@ class PostStatusService < BaseService DistributionWorker.perform_async(@status.id) Pubsubhubbub::DistributionWorker.perform_async(@status.stream_entry.id) ActivityPub::DistributionWorker.perform_async(@status.id) + PollExpirationNotifyWorker.perform_at(@status.poll.expires_at, @status.poll.id) if @status.poll end def validate_media! diff --git a/app/services/vote_service.rb b/app/services/vote_service.rb index 5b80da03a..34a1fe2aa 100644 --- a/app/services/vote_service.rb +++ b/app/services/vote_service.rb @@ -19,14 +19,17 @@ class VoteService < BaseService end end - return if @poll.account.local? - - @votes.each do |vote| - ActivityPub::DeliveryWorker.perform_async( - build_json(vote), - @account.id, - @poll.account.inbox_url - ) + if @poll.account.local? + ActivityPub::DistributePollUpdateWorker.perform_in(3.minutes, @poll.status.id) unless @poll.hide_totals + 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? end end diff --git a/app/workers/activitypub/distribute_poll_update_worker.rb b/app/workers/activitypub/distribute_poll_update_worker.rb new file mode 100644 index 000000000..718279c1b --- /dev/null +++ b/app/workers/activitypub/distribute_poll_update_worker.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +class ActivityPub::DistributePollUpdateWorker + include Sidekiq::Worker + + sidekiq_options queue: 'push', unique: :until_executed, retry: 0 + + def perform(status_id) + @status = Status.find(status_id) + @account = @status.account + + return unless @status.poll + + ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| + [payload, @account.id, inbox_url] + end + + relay! if relayable? + rescue ActiveRecord::RecordNotFound + true + end + + private + + def relayable? + @status.public_visibility? + end + + 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.uniq! + @inboxes + end + + def signed_payload + Oj.dump(ActivityPub::LinkedDataSignature.new(unsigned_payload).sign!(@account)) + end + + def unsigned_payload + ActiveModelSerializers::SerializableResource.new( + @status, + serializer: ActivityPub::UpdatePollSerializer, + adapter: ActivityPub::Adapter + ).as_json + end + + def payload + @payload ||= @status.distributable? ? signed_payload : Oj.dump(unsigned_payload) + end + + def relay! + ActivityPub::DeliveryWorker.push_bulk(Relay.enabled.pluck(:inbox_url)) do |inbox_url| + [payload, @account.id, inbox_url] + end + end +end diff --git a/app/workers/poll_expiration_notify_worker.rb b/app/workers/poll_expiration_notify_worker.rb new file mode 100644 index 000000000..ae72298b8 --- /dev/null +++ b/app/workers/poll_expiration_notify_worker.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class PollExpirationNotifyWorker + include Sidekiq::Worker + + sidekiq_options unique: :until_executed + + def perform(poll_id) + poll = Poll.find(poll_id) + + # Notify poll owner and remote voters + if poll.local? + ActivityPub::DistributePollUpdateWorker.perform_async(poll.status.id) + NotifyService.new.call(poll.account, poll) + end + + # Notify local voters + poll.votes.includes(:account).map(&:account).filter(&:local?).each do |account| + NotifyService.new.call(account, poll) + end + rescue ActiveRecord::RecordNotFound + true + end +end From 5506b9406db7847ffd0892a0cda1c042b3157a6a Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 11 Mar 2019 00:50:31 +0100 Subject: [PATCH 2/3] Avoid race conditions when creating backups (#10234) Under load, multiple backups for a single user could be planned, which is very expensive. --- app/controllers/settings/exports_controller.rb | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/app/controllers/settings/exports_controller.rb b/app/controllers/settings/exports_controller.rb index 0135f2189..3012fbf77 100644 --- a/app/controllers/settings/exports_controller.rb +++ b/app/controllers/settings/exports_controller.rb @@ -13,11 +13,25 @@ class Settings::ExportsController < Settings::BaseController end def create - authorize :backup, :create? + raise Mastodon::NotPermittedError unless user_signed_in? + + backup = nil + + RedisLock.acquire(lock_options) do |lock| + if lock.acquired? + authorize :backup, :create? + backup = current_user.backups.create! + else + raise Mastodon::RaceConditionError + end + end - backup = current_user.backups.create! BackupWorker.perform_async(backup.id) redirect_to settings_export_path end + + def lock_options + { redis: Redis.current, key: "backup:#{current_user.id}" } + end end From 13a7f05030cdcbab24aeb25944a9a430238dbff1 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 11 Mar 2019 00:51:23 +0100 Subject: [PATCH 3/3] Fix streaming API always attempting to use SSL with Postgres (#10231) Fix #10223 --- streaming/index.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/streaming/index.js b/streaming/index.js index 86a91d12b..2a51a1a0d 100644 --- a/streaming/index.js +++ b/streaming/index.js @@ -89,7 +89,6 @@ const startWorker = (workerId) => { host: process.env.DB_HOST || pg.defaults.host, port: process.env.DB_PORT || pg.defaults.port, max: 10, - ssl: !!process.env.DB_SSLMODE && process.env.DB_SSLMODE !== 'disable' ? true : undefined, }, production: { @@ -99,11 +98,15 @@ const startWorker = (workerId) => { host: process.env.DB_HOST || 'localhost', port: process.env.DB_PORT || 5432, max: 10, - ssl: !!process.env.DB_SSLMODE && process.env.DB_SSLMODE !== 'disable' ? true : undefined, }, }; - const app = express(); + if (!!process.env.DB_SSLMODE && process.env.DB_SSLMODE !== 'disable') { + pgConfigs.development.ssl = true; + pgConfigs.production.ssl = true; + } + + const app = express(); app.set('trusted proxy', process.env.TRUSTED_PROXY_IP || 'loopback,uniquelocal'); const pgPool = new pg.Pool(Object.assign(pgConfigs[env], dbUrlToConfig(process.env.DATABASE_URL)));