Add mobile push notifications to Forem (#12419)

* First commit with iOS PN working

* RPush cleanup worker + unique jobs config

* Remove rpush tables from schema.rb

* PR feedback

* Feature flag and test for route

* Tests and feature flag PushNotification ::Send

* Update app/controllers/devices_controller.rb

Co-authored-by: Michael Kohl <citizen428@dev.to>

* Update spec/routing/devices_routes_spec.rb

Co-authored-by: Michael Kohl <citizen428@dev.to>

* Update spec/services/push_notifications/send_spec.rb

Co-authored-by: Michael Kohl <citizen428@dev.to>

* PR feedback

* Set Rpush driver and url

* More PR feedback

* Apply suggestions from code review

Co-authored-by: Jamie Gaskins <jgaskins@gmail.com>

* PR feedback from Rhymes

* Don’t double render

* Sure

Co-authored-by: Josh Puetz <hi@joshpuetz.com>
Co-authored-by: Josh Puetz <josh@dev.to>
Co-authored-by: Michael Kohl <citizen428@dev.to>
Co-authored-by: Jamie Gaskins <jgaskins@gmail.com>
This commit is contained in:
Fernando Valverde 2021-03-12 14:08:18 -06:00 committed by GitHub
parent f1d85e666b
commit cf3bde4259
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
30 changed files with 538 additions and 5 deletions

View file

@ -31,6 +31,9 @@ SESSION_EXPIRY_SECONDS=1209600 # two weeks in seconds
# Redis Sidekiq storage
REDIS_SIDEKIQ_URL="redis://localhost:6379"
# Redis Devices/Rpush storage
REDIS_RPUSH_URL="redis://localhost:6379"
# Elasticsearch
ELASTICSEARCH_URL="http://localhost:9200"
@ -42,7 +45,7 @@ FORCE_SSL_IN_RAILS="not applicable in dev"
# Postgres DB
DATABASE_NAME="Forem_development" # Used when creating a db instance `rails db:create`
DATABASE_URL="postgresql://localhost:5432/$DATABASE_NAME" # This takes precedence over `DATABASE_NAME` if they don't match
DATABASE_URL="postgresql://localhost:5432/$DATABASE_NAME" # This takes precedence over `DATABASE_NAME` if they don't match
RAILS_MAX_THREADS=5 # Specifies the number of Puma threads
WEB_CONCURRENCY=2 # Specifies the number of `workers` to boot in clustered mode.
DATABASE_POOL_SIZE=5 # The number of concurrent connections to Postgres. If unset the value of RAILS_MAX_THREADS will be used in it's place.

3
.gitignore vendored
View file

@ -54,6 +54,9 @@ package-lock.json
#Jetbrains Tools
.idea/
#Nova from Panic
.nova/
#sitemap
/public/sitemap.xml.gz

View file

@ -81,6 +81,8 @@ gem "ransack", "~> 2.4" # Searching and sorting
gem "recaptcha", "~> 5.7", require: "recaptcha/rails" # Helpers for the reCAPTCHA API
gem "redcarpet", "~> 3.5" # A fast, safe and extensible Markdown to (X)HTML parser
gem "redis", "~> 4.2.5" # Redis ruby client
gem "rpush", "~> 5.3" # Push Notification library for Rails
gem "rpush-redis", "~> 1.1" # Redis module capability for rpush library
# NOTE: [@rhymes]: sign in doesn't work with the redis-rack 2.1.0+ or with redis-actionpack 5.2.0+
# We need to investigate why exactly, in the meantime we are hardcoding these two transitive dependencies

View file

@ -371,6 +371,8 @@ GEM
hashie (4.1.0)
heapy (0.2.0)
thor
hiredis (0.6.3)
hkdf (0.3.0)
honeybadger (4.7.3)
honeycomb-beeline (2.4.0)
libhoney (~> 1.14, >= 1.14.2)
@ -378,6 +380,7 @@ GEM
html_truncator (0.4.2)
nokogiri (~> 1.5)
htmlentities (4.3.4)
http-2 (0.11.0)
http (4.4.1)
addressable (~> 2.3)
http-cookie (~> 1.0)
@ -468,6 +471,13 @@ GEM
mini_mime (1.0.2)
mini_portile2 (2.5.0)
minitest (5.14.4)
modis (3.3.0)
activemodel (>= 4.2)
activesupport (>= 4.2)
connection_pool (>= 2)
hiredis (>= 0.5)
msgpack (>= 0.5)
redis (>= 3.0)
msgpack (1.4.2)
multi_json (1.15.0)
multi_xml (0.6.0)
@ -475,6 +485,10 @@ GEM
nakayoshi_fork (0.0.4)
naught (1.1.0)
nenv (0.3.0)
net-http-persistent (4.0.1)
connection_pool (~> 2.2)
net-http2 (0.18.4)
http-2 (~> 0.11)
netrc (0.11.0)
nio4r (2.5.5)
nokogiri (1.11.1)
@ -644,6 +658,18 @@ GEM
rexml (3.2.4)
rolify (5.3.0)
rouge (3.26.0)
rpush (5.3.0)
activesupport (>= 5.0)
jwt (>= 1.5.6)
multi_json (~> 1.0)
net-http-persistent
net-http2 (~> 0.18, >= 0.18.3)
railties
rainbow
thor (>= 0.18.1, < 2.0)
webpush (~> 1.0)
rpush-redis (1.1.0)
modis (~> 3.0)
rspec (3.10.0)
rspec-core (~> 3.10.0)
rspec-expectations (~> 3.10.0)
@ -839,6 +865,9 @@ GEM
rack-proxy (>= 0.6.1)
railties (>= 5.2)
semantic_range (>= 2.3.0)
webpush (1.1.0)
hkdf (~> 0.2)
jwt (~> 2.0)
websocket-driver (0.7.3)
websocket-extensions (>= 0.1.0)
websocket-extensions (0.1.5)
@ -968,6 +997,8 @@ DEPENDENCIES
reverse_markdown (~> 2.0)
rolify (~> 5.3)
rouge (~> 3.26)
rpush (~> 5.3)
rpush-redis (~> 1.1)
rspec-rails (~> 4.1)
rspec-retry (~> 0.6)
rubocop (~> 1.11)
@ -1018,4 +1049,4 @@ RUBY VERSION
ruby 2.7.2p137
BUNDLED WITH
2.2.10
2.2.11

View file

@ -44,7 +44,12 @@ module Api
end
def all_cache_instances_connected?
[ENV["REDIS_URL"], ENV["REDIS_SESSIONS_URL"], ENV["REDIS_SIDEKIQ_URL"]].compact.all? do |url|
[
ENV["REDIS_URL"],
ENV["REDIS_SESSIONS_URL"],
ENV["REDIS_SIDEKIQ_URL"],
ENV["REDIS_RPUSH_URL"],
].compact.all? do |url|
Redis.new(url: url).ping == "PONG"
end
end

View file

@ -0,0 +1,45 @@
class DevicesController < ApplicationController
# Devices can only be created for authenticated users.
# This replaces the Authenticated Users Pusher Beams solution.
# See: https://github.com/forem/forem/pull/12419/files#r563906038
before_action :authenticate_user!, only: [:create]
rescue_from ActiveRecord::ActiveRecordError do |exc|
render json: { error: exc.message, status: 422 }, status: :unprocessable_entity
end
def create
device = Device.find_or_create_by(device_params)
if device.persisted?
head :ok
else
render json: { error: device.errors_as_sentence }, status: :bad_request
end
end
def destroy
device = Device.find_by(device_params)
unless device
render json: { error: "Not Found", status: 404 }, status: :not_found
return
end
device&.destroy
if device&.destroyed?
head :no_content
else
render json: { error: device.errors_as_sentence }, status: :bad_request
end
end
private
def device_params
{
user: current_user,
token: params[:token],
platform: params[:platform],
app_bundle: params[:app_bundle]
}
end
end

View file

@ -14,6 +14,7 @@ module ForemStatsDrivers
c.use :redis, service_name: "redis", describes: { url: ENV["REDIS_URL"] }
c.use :redis, service_name: "redis-sessions", describes: { url: ENV["REDIS_SESSIONS_URL"] }
c.use :redis, service_name: "redis-sidekiq", describes: { url: ENV["REDIS_SIDEKIQ_URL"] }
c.use :redis, service_name: "redis-rpush", describes: { url: ENV["REDIS_RPUSH_URL"] }
c.use :rails
c.use :http, split_by_domain: false
c.use :faraday, split_by_domain: true

80
app/models/device.rb Normal file
View file

@ -0,0 +1,80 @@
class Device < ApplicationRecord
belongs_to :user
IOS = "iOS".freeze
ANDROID = "Android".freeze
validates :token, uniqueness: { scope: %i[user_id platform app_bundle] }
validates :platform, inclusion: { in: [IOS, ANDROID] }
def create_notification(title, body, payload)
case platform
when IOS
ios_notification(title, body, payload)
when ANDROID
android_notification(title, body, payload)
end
end
private
def ios_notification(title, body, payload)
n = Rpush::Apns2::Notification.new
# [@forem/backend] `.where().first` is necessary because we use Redis data storage
# https://github.com/rpush/rpush/wiki/Using-Redis#find_by_name-cannot-be-used-in-rpush-redis
# rubocop:disable Rails/FindBy
n.app = Rpush::Apns2::App.where(name: app_bundle).first || recreate_ios_app!
# rubocop:enable Rails/FindBy
n.device_token = token
n.data = {
aps: {
alert: {
title: ApplicationConfig["COMMUNITY_NAME"],
subtitle: title,
body: body
},
'thread-id': ApplicationConfig["COMMUNITY_NAME"]
},
data: payload
}
n.save!
end
def android_notification(title, body, payload)
n = Rpush::Gcm::Notification.new
# [@forem/backend] `.where().first` is necessary because we use Redis data storage
# https://github.com/rpush/rpush/wiki/Using-Redis#find_by_name-cannot-be-used-in-rpush-redis
# rubocop:disable Rails/FindBy
n.app = Rpush::Gcm::App.where(name: app_bundle).first || recreate_android_app!
# rubocop:enable Rails/FindBy
n.registration_ids = [token]
n.priority = "high"
n.content_available = true
n.notification = { title: title, body: body }
n.data = { data: payload }
n.save!
end
def recreate_ios_app!
app = Rpush::Apns2::App.new
app.name = app_bundle
app.certificate = Base64.decode64(ApplicationConfig["RPUSH_IOS_PEM"])
app.environment = Rails.env.production? ? "production" : "development"
app.password = ""
app.bundle_id = app_bundle
app.connections = 1
app.save!
app
end
def recreate_android_app!
app = Rpush::Gcm::App.new
app.name = app_bundle
app.auth_key = "..."
app.connections = 1
app.save!
app
end
end

View file

@ -232,6 +232,9 @@ class SiteConfig < RailsSettings::Base
xlarge: 250
}
# Push Notifications
field :push_notifications_ios_pem, type: :string
# Returns true if we are operating on a local installation, false otherwise
def self.local?
app_domain.include?("localhost")

View file

@ -178,6 +178,7 @@ class User < ApplicationRecord
has_many :subscribers, through: :source_authored_user_subscriptions, dependent: :destroy
has_many :tweets, dependent: :nullify
has_many :webhook_endpoints, class_name: "Webhook::Endpoint", inverse_of: :user, dependent: :delete_all
has_many :devices, dependent: :delete_all
mount_uploader :profile_image, ProfileImageUploader

View file

@ -0,0 +1,29 @@
module PushNotifications
class Send
def self.call(user:, title:, body:, payload:)
new(user: user, title: title, body: body, payload: payload).call
end
def initialize(user:, title:, body:, payload:)
@user = user
@title = title
@body = body
@payload = payload
end
def call
return unless FeatureFlag.enabled?(:mobile_notifications)
@user.devices.find_each do |device|
device.create_notification(@title, @body, @payload)
end
# perform_in(30.seconds) is key: The background worker will execute Rpush.push which will send out all
# pending notifications. This means we don't need to run it for every PN. PushNotifications::DeliverWorker
# has a constraint that will execute the job only once (30 seconds from now). So if we need to send 1 PN
# every 5 seconds we only execute this once every 30s (no duplicate/unnecessary processing).
# If no PNs are scheduled to be sent for a 6h span then 0 jobs are executed.
PushNotifications::DeliverWorker.perform_in(30.seconds)
end
end
end

View file

@ -0,0 +1,21 @@
module PushNotifications
class CleanupWorker
include Sidekiq::Worker
sidekiq_options queue: :low_priority, retry: 10
def perform
redis = Redis.new(url: ENV["REDIS_RPUSH_URL"] || ENV["REDIS_URL"])
cursor = 0
until (cursor, keys = redis.scan(cursor, match: "rpush:notifications:*")).first.to_i.zero?
keys.each do |key|
next unless redis.ttl(key) == -1
next unless redis.type(key) == "hash"
next if redis.hget(key, "delivered").nil?
redis.expire(key, 8.hours.to_i)
end
end
end
end
end

View file

@ -0,0 +1,18 @@
module PushNotifications
class DeliverWorker
include Sidekiq::Worker
sidekiq_options queue: :medium_priority,
retry: 10,
lock: :until_expired,
lock_ttl: 30,
on_conflict: :reject
def perform
# Deliver all pending Push Notifications
Rpush.push
# Callback for feedback (see `config/initializers/rpush.rb`)
Rpush.apns_feedback
end
end
end

View file

@ -0,0 +1,149 @@
Rpush.configure do |config|
# Supported clients are :active_record and :redis
config.client = :redis
# Options passed to Redis.new
config.redis_options = { url: ENV["REDIS_RPUSH_URL"] || ENV["REDIS_URL"], driver: :ruby }
# Frequency in seconds to check for new notifications.
config.push_poll = 2
# The maximum number of notifications to load from the store every `push_poll` seconds.
# If some notifications are still enqueued internally, Rpush will load the batch_size less
# the number enqueued. An exception to this is if the service is able to receive multiple
# notification payloads over the connection with a single write, such as APNs.
config.batch_size = 100
# Path to write PID file. Relative to current directory unless absolute.
# config.pid_file = "tmp/rpush.pid"
# Path to log file. Relative to current directory unless absolute.
# config.log_file = "log/rpush.log"
config.log_level = (defined?(Rails) && Rails.logger ? Rails.logger.level : ::Logger::Severity::INFO)
# Define a custom logger.
config.logger = Rails.logger
# By default in foreground mode logs are directed both to the logger and to stdout.
# If the logger goes to stdout, you can disable foreground logging to avoid duplication.
config.foreground_logging = false
# config.apns.feedback_receiver.enabled = true
# config.apns.feedback_receiver.frequency = 60
end
Rpush.reflect do |on|
# Called with a Rpush::Apns::Feedback instance when feedback is received
# from the APNs that a notification has failed to be delivered.
# Further notifications should not be sent to the device.
# on.apns_feedback do |feedback|
# end
# Called when a notification is queued internally for delivery.
# The internal queue for each app runner can be inspected:
#
# Rpush::Daemon::AppRunner.runners.each do |app_id, runner|
# runner.app
# runner.queue_size
# end
#
# on.notification_enqueued do |notification|
# end
# Called when a notification is successfully delivered.
# on.notification_delivered do |notification|
# end
# Called when notification delivery failed.
# Call 'error_code' and 'error_description' on the notification for the cause.
on.notification_failed do |notification|
if notification.error_description == "BadDeviceToken"
# When adding new platforms we'll need to check to only clear out devices
# scoped to that specific platform. i.e. notification.class.to_s =~ /Apns.+::Notification/
Device.where(token: notification.device_token, platform: "iOS").destroy_all
end
ForemStatsClient.increment(
"push_notifications.errors",
tags: [
"error:#{e.class}",
"message:#{e.error_description}",
],
)
end
# Called when the notification delivery failed and only the notification ID
# is present in memory.
# on.notification_id_failed do |app, notification_id, error_code, error_description|
# end
# Called when a notification will be retried at a later date.
# Call 'deliver_after' on the notification for the next delivery date
# and 'retries' for the number of times this notification has been retried.
# on.notification_will_retry do |notification|
# end
# Called when a notification will be retried and only the notification ID
# is present in memory.
# on.notification_id_will_retry do |app, notification_id, retry_after|
# end
# Called when a TCP connection is lost and will be reconnected.
# on.tcp_connection_lost do |app, error|
# end
# Called for each recipient which successfully receives a notification. This
# can occur more than once for the same notification when there are multiple
# recipients.
# on.gcm_delivered_to_recipient do |notification, registration_id|
# end
# Called for each recipient which fails to receive a notification. This
# can occur more than once for the same notification when there are multiple
# recipients. (do not handle invalid registration IDs here)
# on.gcm_failed_to_recipient do |notification, error, registration_id|
# end
# Called when the GCM returns a canonical registration ID.
# You will need to replace old_id with canonical_id in your records.
# on.gcm_canonical_id do |old_id, canonical_id|
# end
# Called when the GCM returns a failure that indicates an invalid registration id.
# You will need to delete the registration_id from your records.
# on.gcm_invalid_registration_id do |app, error, registration_id|
# end
# Called when an SSL certificate will expire within 1 month.
# Implement on.error to catch errors raised when the certificate expires.
# on.ssl_certificate_will_expire do |app, expiration_time|
# end
# Called when an SSL certificate has been revoked.
# on.ssl_certificate_revoked do |app, error|
# end
# Called when the ADM returns a canonical registration ID.
# You will need to replace old_id with canonical_id in your records.
# on.adm_canonical_id do |old_id, canonical_id|
# end
# Called when Failed to deliver to ADM. Check the 'reason' string for further
# explanations.
#
# If the reason is the string 'Unregistered', you should remove
# this registration id from your records.
# on.adm_failed_to_recipient do |notification, registration_id, reason|
# end
# Called when Failed to deliver to WNS. Check the 'reason' string for further
# explanations.
# You should remove this uri from your records
# on.wns_invalid_channel do |notification, uri, reason|
# end
# Called when an exception is raised.
# on.error do |error|
# end
end

View file

@ -325,7 +325,13 @@ Rails.application.routes.draw do
end
end
resources :comment_mutes, only: %i[update]
resources :users, only: %i[index], defaults: { format: :json } # internal API
resources :users, only: %i[index], defaults: { format: :json } do # internal API
constraints(-> { FeatureFlag.enabled?(:mobile_notifications) }) do
collection do
resources :devices, only: %i[create destroy]
end
end
end
resources :users, only: %i[update]
resources :reactions, only: %i[index create]
resources :response_templates, only: %i[index create edit update destroy]

View file

@ -136,4 +136,7 @@ update_latest_github_repos:
description: "Fetches the latest info about stored GitHub repositories (runs hourly, 45 min after the hour)"
cron: "45 * * * *"
class: "GithubRepos::UpdateLatestWorker"
push_notifications_cleanup:
description: "Cleans up Push Notifications from Redis (runs every 8 hours on the 20th minute after the hour)"
cron: "20 */8 * * *"
class: "PushNotifications::CleanupWorker"

View file

@ -0,0 +1,14 @@
class CreateDevices < ActiveRecord::Migration[6.0]
def change
create_table :devices do |t|
t.references :user, null: false, foreign_key: true, index: false
t.string :token, null: false
t.string :platform, null: false
t.string :app_bundle, null: false
t.timestamps
end
add_index :devices, [:user_id, :token, :platform, :app_bundle], unique: true
end
end

View file

@ -445,6 +445,16 @@ ActiveRecord::Schema.define(version: 2021_03_10_154630) do
t.index ["file_name"], name: "index_data_update_scripts_on_file_name", unique: true
end
create_table "devices", force: :cascade do |t|
t.bigint "user_id", null: false
t.string "token", null: false
t.string "platform", null: false
t.string "app_bundle", null: false
t.datetime "created_at", precision: 6, null: false
t.datetime "updated_at", precision: 6, null: false
t.index ["user_id", "token", "platform", "app_bundle"], name: "index_devices_on_user_id_and_token_and_platform_and_app_bundle", unique: true
end
create_table "display_ad_events", force: :cascade do |t|
t.string "category"
t.string "context_type"
@ -1423,6 +1433,7 @@ ActiveRecord::Schema.define(version: 2021_03_10_154630) do
add_foreign_key "credits", "organizations", on_delete: :restrict
add_foreign_key "credits", "users", on_delete: :cascade
add_foreign_key "custom_profile_fields", "profiles", on_delete: :cascade
add_foreign_key "devices", "users"
add_foreign_key "display_ad_events", "display_ads", on_delete: :cascade
add_foreign_key "display_ad_events", "users", on_delete: :cascade
add_foreign_key "display_ads", "organizations", on_delete: :cascade

View file

@ -0,0 +1,8 @@
FactoryBot.define do
factory :device do
association :user, strategy: :create
token { "12345" }
platform { Device::IOS }
app_bundle { Faker::Internet.domain_name(subdomain: true) }
end
end

View file

@ -0,0 +1,42 @@
require "rails_helper"
RSpec.describe "User devices routes", type: :routing do
it "renders the user devices routes if the mobile_notifications feature flag is enabled", :aggregate_failures do
allow(FeatureFlag).to receive(:enabled?).twice.with(:mobile_notifications).and_return(true)
expect(post: devices_path).to route_to(
controller: "devices",
action: "create",
format: :json,
locale: nil,
)
expect(delete: "#{devices_path}/1").to route_to(
controller: "devices",
action: "destroy",
id: "1",
format: :json,
locale: nil,
)
end
it "does not render the user devices routes if the mobile_notifications feature flag is disabled",
:aggregate_failures do
allow(FeatureFlag).to receive(:enabled?).at_least(:twice).with(:mobile_notifications).and_return(false)
expect(post: devices_path).not_to route_to(
controller: "devices",
action: "create",
format: :json,
locale: nil,
)
expect(delete: "#{devices_path}/1").not_to route_to(
controller: "devices",
action: "destroy",
id: "1",
format: :json,
locale: nil,
)
end
end

View file

@ -0,0 +1,58 @@
require "rails_helper"
RSpec.describe PushNotifications::Send, type: :service do
let(:user) { create(:user) }
let(:params) do
{
user: user,
title: "Alert",
body: "some alert here",
payload: ""
}
end
context "with feature flag set to false/off" do
before { allow(FeatureFlag).to receive(:enabled?).with(:mobile_notifications).and_return(false) }
it "does nothing if the feature flag is disabled" do
expect { described_class.call(params) }
.not_to change { Rpush::Client::Redis::Notification.all.count }
end
end
context "with no devices for user" do
before do
allow(FeatureFlag).to receive(:enabled?).with(:mobile_notifications).and_return(true)
user.devices.delete
end
it "does nothing", :aggregate_failures do
expect(user.devices.count).to eq(0)
expect { described_class.call(params) }
.not_to change { Rpush::Client::Redis::Notification.all.count }
end
end
context "with devices for user" do
before do
allow(FeatureFlag).to receive(:enabled?).with(:mobile_notifications).and_return(true)
allow(ApplicationConfig).to receive(:[]).with("RPUSH_IOS_PEM").and_return("dGVzdGluZw==")
allow(ApplicationConfig).to receive(:[]).with("COMMUNITY_NAME").and_return("Forem")
create(:device, user: user)
end
it "creates a notification and enqueues it" do
expect { described_class.call(params) }
.to change { Rpush::Client::Redis::Notification.all.count }.by(1)
.and change(PushNotifications::DeliverWorker.jobs, :size).by(1)
end
it "creates a single notification for each of the user's devices when they have multiple" do
create(:device, user: user)
expect { described_class.call(params) }
.to change { Rpush::Client::Redis::Notification.all.count }.by(2)
.and change(PushNotifications::DeliverWorker.jobs, :size).by(1)
end
end
end

BIN
vendor/cache/hiredis-0.6.3.gem vendored Normal file

Binary file not shown.

BIN
vendor/cache/hkdf-0.3.0.gem vendored Normal file

Binary file not shown.

BIN
vendor/cache/http-2-0.11.0.gem vendored Normal file

Binary file not shown.

BIN
vendor/cache/modis-3.3.0.gem vendored Normal file

Binary file not shown.

Binary file not shown.

BIN
vendor/cache/net-http2-0.18.4.gem vendored Normal file

Binary file not shown.

BIN
vendor/cache/rpush-5.3.0.gem vendored Normal file

Binary file not shown.

BIN
vendor/cache/rpush-redis-1.1.0.gem vendored Normal file

Binary file not shown.

BIN
vendor/cache/webpush-1.1.0.gem vendored Normal file

Binary file not shown.