From 8387b3928ec7658192907da79df65e65aaa8a7fc Mon Sep 17 00:00:00 2001 From: Sorin Davidoi Date: Tue, 18 Jul 2017 16:25:40 +0200 Subject: [PATCH] fix(push-subscriptions): Refactor how Sidekiq jobs are handled (#4226) --- app/models/web/push_subscription.rb | 8 +++++--- app/services/notify_service.rb | 6 +++++- app/workers/web_push_notification_worker.rb | 22 +++++++++------------ 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb index 4440706a6..baf6a1ece 100644 --- a/app/models/web/push_subscription.rb +++ b/app/models/web/push_subscription.rb @@ -12,6 +12,9 @@ # updated_at :datetime not null # +require 'webpush' +require_relative '../../models/setting' + class Web::PushSubscription < ApplicationRecord include RoutingHelper include StreamEntriesHelper @@ -37,7 +40,6 @@ class Web::PushSubscription < ApplicationRecord nsfw = notification.target_status.nil? || notification.target_status.spoiler_text.empty? ? nil : notification.target_status.spoiler_text # TODO: Make sure that the payload does not exceed 4KB - Webpush::PayloadTooLarge - # TODO: Queue the requests - Webpush::TooManyRequests Webpush.payload_send( message: JSON.generate( title: title, @@ -59,7 +61,7 @@ class Web::PushSubscription < ApplicationRecord p256dh: key_p256dh, auth: key_auth, vapid: { - # subject: "mailto:#{Setting.site_contact_email}", + subject: "mailto:#{Setting.site_contact_email}", private_key: Rails.configuration.x.vapid_private_key, public_key: Rails.configuration.x.vapid_public_key, }, @@ -166,7 +168,7 @@ class Web::PushSubscription < ApplicationRecord p256dh: key_p256dh, auth: key_auth, vapid: { - # subject: "mailto:#{Setting.site_contact_email}", + subject: "mailto:#{Setting.site_contact_email}", private_key: Rails.configuration.x.vapid_private_key, public_key: Rails.configuration.x.vapid_public_key, }, diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index 0ab61b634..c7d8ad50a 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -65,7 +65,11 @@ class NotifyService < BaseService end def send_push_notifications - WebPushNotificationWorker.perform_async(@recipient.id, @notification.id) + sessions_with_subscriptions_ids = @recipient.user.session_activations.where.not(web_push_subscription: nil).pluck(:id) + + WebPushNotificationWorker.push_bulk(sessions_with_subscriptions_ids) do |session_activation_id| + [session_activation_id, @notification.id] + end end def send_email diff --git a/app/workers/web_push_notification_worker.rb b/app/workers/web_push_notification_worker.rb index e8f1d72bd..da4043ddb 100644 --- a/app/workers/web_push_notification_worker.rb +++ b/app/workers/web_push_notification_worker.rb @@ -5,22 +5,18 @@ class WebPushNotificationWorker sidekiq_options backtrace: true - def perform(recipient_id, notification_id) - recipient = Account.find(recipient_id) + def perform(session_activation_id, notification_id) + session_activation = SessionActivation.find(session_activation_id) notification = Notification.find(notification_id) - sessions_with_subscriptions = recipient.user.session_activations.where.not(web_push_subscription: nil) + begin + session_activation.web_push_subscription.push(notification) + rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription => e + # Subscription expiration is not currently implemented in any browser + session_activation.web_push_subscription.destroy! + session_activation.update!(web_push_subscription: nil) - sessions_with_subscriptions.each do |session| - begin - session.web_push_subscription.push(notification) - rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription - # Subscription expiration is not currently implemented in any browser - session.web_push_subscription.destroy! - session.update!(web_push_subscription: nil) - rescue Webpush::PayloadTooLarge => e - Rails.logger.error(e) - end + raise e end end end