From 38aee8d9eabfd208dd7c5c88807ad3fcb5b23bb2 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Fri, 3 Feb 2023 18:55:22 +0530 Subject: [PATCH] chore: Switch to web-push gem (#6390) - The previous gem, `webpush` was last updated a while ago. Also, with the recent ruby upgrade, we needed a fix for zaru/webpush#106. Hence switching to the `web-push` gem where the issues are fixed. --- Gemfile | 7 +------ Gemfile.lock | 18 +++++++----------- .../notification/push_notification_service.rb | 6 +++--- lib/vapid_service.rb | 2 +- .../push_notification_service_spec.rb | 6 +++--- 5 files changed, 15 insertions(+), 24 deletions(-) diff --git a/Gemfile b/Gemfile index 3f520988b..a0164da96 100644 --- a/Gemfile +++ b/Gemfile @@ -107,12 +107,7 @@ gem 'sidekiq-cron', '~> 1.3' ##-- Push notification service --## gem 'fcm' - -# Ref: https://github.com/mastodon/mastodon/pull/18449 -# ref: https://github.com/zaru/webpush/pull/106 -# lets switch to web-push gem once the above PR is merged -# https://github.com/zaru/webpush/pull/106#issuecomment-1342925261 -gem 'webpush', git: 'https://github.com/ClearlyClaire/webpush.git', ref: 'f14a4d52e201128b1b00245d11b6de80d6cfdcd9' +gem 'web-push' ##-- geocoding / parse location from ip --## # http://www.rubygeocoder.com/ diff --git a/Gemfile.lock b/Gemfile.lock index 78edcd6ef..e263681b0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,12 +1,3 @@ -GIT - remote: https://github.com/ClearlyClaire/webpush.git - revision: f14a4d52e201128b1b00245d11b6de80d6cfdcd9 - ref: f14a4d52e201128b1b00245d11b6de80d6cfdcd9 - specs: - webpush (0.3.8) - hkdf (~> 0.2) - jwt (~> 2.0) - GIT remote: https://github.com/chatwoot/devise-secure_password revision: d777b04f12652d576b1272b8f39857e3e0b3fc26 @@ -332,7 +323,7 @@ GEM hana (1.3.7) hashdiff (1.0.1) hashie (5.0.0) - hkdf (0.3.0) + hkdf (1.0.0) html2text (0.2.1) nokogiri (~> 1.6) http (5.1.0) @@ -470,6 +461,7 @@ GEM omniauth-oauth2 (1.8.0) oauth2 (>= 1.4, < 3) omniauth (~> 2.0) + openssl (3.1.0) orm_adapter (0.5.0) os (1.1.4) parallel (1.22.1) @@ -711,6 +703,10 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) + web-push (3.0.0) + hkdf (~> 1.0) + jwt (~> 2.0) + openssl (~> 3.0) webmock (3.18.1) addressable (>= 2.8.0) crack (>= 0.3.2) @@ -842,9 +838,9 @@ DEPENDENCIES uglifier valid_email2 web-console + web-push webmock webpacker (~> 5.4, >= 5.4.3) - webpush! wisper (= 2.0.0) working_hours diff --git a/app/services/notification/push_notification_service.rb b/app/services/notification/push_notification_service.rb index 2f2bb11e3..765483f5e 100644 --- a/app/services/notification/push_notification_service.rb +++ b/app/services/notification/push_notification_service.rb @@ -49,7 +49,7 @@ class Notification::PushNotificationService def send_browser_push(subscription) return unless send_browser_push?(subscription) - Webpush.payload_send( + WebPush.payload_send( message: JSON.generate(push_message), endpoint: subscription.subscription_attributes['endpoint'], p256dh: subscription.subscription_attributes['p256dh'], @@ -63,10 +63,10 @@ class Notification::PushNotificationService open_timeout: 5, read_timeout: 5 ) - rescue Webpush::ExpiredSubscription + rescue WebPush::ExpiredSubscription subscription.destroy! rescue Errno::ECONNRESET, Net::OpenTimeout, Net::ReadTimeout => e - Rails.logger.error "Webpush operation error: #{e.message}" + Rails.logger.error "WebPush operation error: #{e.message}" end def send_fcm_push(subscription) diff --git a/lib/vapid_service.rb b/lib/vapid_service.rb index 2e8b581ea..ea3498658 100644 --- a/lib/vapid_service.rb +++ b/lib/vapid_service.rb @@ -12,7 +12,7 @@ class VapidService return config['VAPID_KEYS'] if config['VAPID_KEYS'].present? # keys don't exist in the database. so let's generate and save them - keys = Webpush.generate_key + keys = WebPush.generate_key # TODO: remove the logic on environment variables when we completely deprecate public_key = ENV.fetch('VAPID_PUBLIC_KEY') { keys.public_key } private_key = ENV.fetch('VAPID_PRIVATE_KEY') { keys.private_key } diff --git a/spec/services/notification/push_notification_service_spec.rb b/spec/services/notification/push_notification_service_spec.rb index d5217f635..17fc5f6f3 100644 --- a/spec/services/notification/push_notification_service_spec.rb +++ b/spec/services/notification/push_notification_service_spec.rb @@ -7,7 +7,7 @@ describe Notification::PushNotificationService do let(:fcm_double) { double } before do - allow(Webpush).to receive(:payload_send).and_return(true) + allow(WebPush).to receive(:payload_send).and_return(true) allow(FCM).to receive(:new).and_return(fcm_double) allow(fcm_double).to receive(:send).and_return({ body: { 'results': [] }.to_json }) end @@ -18,7 +18,7 @@ describe Notification::PushNotificationService do create(:notification_subscription, user: notification.user) described_class.new(notification: notification).perform - expect(Webpush).to have_received(:payload_send) + expect(WebPush).to have_received(:payload_send) expect(FCM).not_to have_received(:new) end end @@ -29,7 +29,7 @@ describe Notification::PushNotificationService do described_class.new(notification: notification).perform expect(FCM).to have_received(:new) - expect(Webpush).not_to have_received(:payload_send) + expect(WebPush).not_to have_received(:payload_send) end end end