Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FCM support - v1.1 #660

Merged
merged 25 commits into from
Jun 22, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
74b4258
Implement proof of concept authorization
mirkode May 13, 2021
509dae4
Fix logger error
mirkode May 26, 2021
d8559d6
Add some fields to AndroidNotification
mirkode May 26, 2021
8f3cacf
Remove notification node from message payload for FCM notifications
mirkode May 26, 2021
0279be2
Fix notification handling in fcm/notification.rb
mirkode Apr 27, 2022
9e083a9
Add comment to necessary_data_exists? method
mirkode Apr 27, 2022
b3cb24b
Merge branch 'master' into fcm-support-v2
AnilRh Jan 22, 2024
20a3fe4
Fix - missing gcm interface
AnilRh Jan 22, 2024
2f467bd
fix code formatting - no change
AnilRh Jan 22, 2024
8f662c2
Payload Data Size is now calculated based off the message data attribute
AnilRh Jan 23, 2024
3043dd7
Adding tests
AnilRh Jan 23, 2024
0b18e5b
Removing debug line
AnilRh Jan 23, 2024
fb5d567
Adding debug line requested in PR #620
AnilRh Jan 24, 2024
39040cb
Add properties to FCM App to allow operation without env vars
nigimaxx Jan 26, 2024
6a9e199
Merge pull request #1 from nigimaxx/fcm-support-v2
AnilRh Jan 26, 2024
576b336
Adding new attributes to redis model
AnilRh Jan 26, 2024
99b33af
Adding 7.1 migration to active record tests. Updating Gemfile.lock
AnilRh Jan 29, 2024
6498354
FCM returning error when data is sent in Android payload
AnilRh Jan 30, 2024
a5eccd6
Moving notification keys to the correct location - only supporting An…
AnilRh Jan 31, 2024
4429f85
As the FCM code moves around notification keys, it would be good to h…
AnilRh Jan 31, 2024
fd260e5
- Calling hook fcm_invalid_device_token if we get an invalid token re…
AnilRh Feb 1, 2024
3fdf3ca
Adding a google credential cache
AnilRh Feb 12, 2024
06aa50e
Fixing Logging
AnilRh Feb 12, 2024
ccecae3
Adding a google credential cache
AnilRh Feb 13, 2024
4ab0185
Remove content_available from FCM implementation. It doesn't appear t…
AnilRh Apr 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 48 additions & 18 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
PATH
remote: .
specs:
rpush (7.0.1)
rpush (7.1.0)
activesupport (>= 5.2)
googleauth
jwt (>= 1.5.6)
multi_json (~> 1.0)
net-http-persistent
Expand Down Expand Up @@ -38,6 +39,8 @@ GEM
i18n (>= 1.6, < 2)
minitest (>= 5.1)
tzinfo (~> 2.0)
addressable (2.8.6)
public_suffix (>= 2.0.2, < 6.0)
appraisal (2.4.1)
bundler
rake
Expand All @@ -58,17 +61,30 @@ GEM
database_cleaner-core (2.0.1)
diff-lcs (1.5.0)
docile (1.4.0)
erubi (1.10.0)
erubi (1.12.0)
faraday (2.9.0)
faraday-net_http (>= 2.0, < 3.2)
faraday-net_http (3.1.0)
net-http
google-cloud-env (2.1.0)
faraday (>= 1.0, < 3.a)
googleauth (1.9.2)
faraday (>= 1.0, < 3.a)
google-cloud-env (~> 2.1)
jwt (>= 1.4, < 3.0)
multi_json (~> 1.11)
os (>= 0.9, < 2.0)
signet (>= 0.16, < 2.a)
hkdf (0.3.0)
http-2 (0.11.0)
i18n (1.10.0)
concurrent-ruby (~> 1.0)
jwt (2.3.0)
loofah (2.18.0)
jwt (2.7.1)
loofah (2.22.0)
crass (~> 1.0.2)
nokogiri (>= 1.5.9)
nokogiri (>= 1.12.0)
method_source (1.0.0)
mini_portile2 (2.4.0)
mini_portile2 (2.8.5)
minitest (5.15.0)
modis (4.0.1)
activemodel (>= 5.2)
Expand All @@ -79,24 +95,32 @@ GEM
msgpack (1.4.5)
multi_json (1.15.0)
mysql2 (0.5.3)
net-http-persistent (4.0.1)
net-http (0.4.1)
uri
net-http-persistent (4.0.2)
connection_pool (~> 2.2)
net-http2 (0.18.4)
net-http2 (0.18.5)
http-2 (~> 0.11)
nokogiri (1.10.10)
mini_portile2 (~> 2.4.0)
nokogiri (1.16.0)
mini_portile2 (~> 2.8.2)
racc (~> 1.4)
os (1.1.4)
parallel (1.21.0)
parser (3.1.0.0)
ast (~> 2.4.1)
pg (1.2.3)
rack (2.2.3.1)
rack-test (1.1.0)
rack (>= 1.0, < 3)
rails-dom-testing (2.0.3)
activesupport (>= 4.2.0)
public_suffix (5.0.4)
racc (1.7.3)
rack (2.2.8)
rack-test (2.1.0)
rack (>= 1.3)
rails-dom-testing (2.2.0)
activesupport (>= 5.0.0)
minitest
nokogiri (>= 1.6)
rails-html-sanitizer (1.4.3)
loofah (~> 2.3)
rails-html-sanitizer (1.6.0)
loofah (~> 2.21)
nokogiri (~> 1.14)
railties (7.0.2.2)
actionpack (= 7.0.2.2)
activesupport (= 7.0.2.2)
Expand Down Expand Up @@ -139,6 +163,11 @@ GEM
rubocop (>= 1.7.0, < 2.0)
rubocop-ast (>= 0.4.0)
ruby-progressbar (1.11.0)
signet (0.18.0)
addressable (~> 2.8)
faraday (>= 0.17.5, < 3.a)
jwt (>= 1.5, < 3.0)
multi_json (~> 1.10)
simplecov (0.21.2)
docile (~> 1.1)
simplecov-html (~> 0.11)
Expand All @@ -152,10 +181,11 @@ GEM
tzinfo (2.0.4)
concurrent-ruby (~> 1.0)
unicode-display_width (2.1.0)
uri (0.13.0)
webpush (1.1.0)
hkdf (~> 0.2)
jwt (~> 2.0)
zeitwerk (2.5.4)
zeitwerk (2.6.12)

PLATFORMS
ruby
Expand Down
1 change: 1 addition & 0 deletions lib/generators/rpush_migration_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def copy_migration
add_rpush_migration('rpush_4_1_0_updates')
add_rpush_migration('rpush_4_1_1_updates')
add_rpush_migration('rpush_4_2_0_updates')
add_rpush_migration('rpush_7_1_0_updates')
end

protected
Expand Down
17 changes: 17 additions & 0 deletions lib/generators/templates/rpush.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,23 @@
# 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.fcm_delivered_to_recipient do |notification|
# 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.fcm_failed_to_recipient do |notification, error|
# end

# Called when the FCM returns a failure that indicates an invalid device token.
# You will need to delete the device token from your records.
# on.fcm_invalid_device_token do |app, error, device_token|
# end
Comment on lines +83 to +98

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the new fcm_canonical_id reflection is missing from the updated template?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/lufia/til/blob/master/fcm.rst#canonical-registration-id
I understand that Canonical ID is no longer used
(Someone please point out if I am wrong).


# Called for each recipient which successfully receives a notification. This
# can occur more than once for the same notification when there are multiple
# recipients.
Expand Down
12 changes: 12 additions & 0 deletions lib/generators/templates/rpush_7_1_0_updates.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class Rpush710Updates < ActiveRecord::Migration["#{ActiveRecord::VERSION::MAJOR}.#{ActiveRecord::VERSION::MINOR}"]
def self.up
add_column :rpush_apps, :firebase_project_id, :string
add_column :rpush_apps, :json_key, :text
end

def self.down
remove_column :rpush_apps, :firebase_project_id
remove_column :rpush_apps, :json_key
end
end

1 change: 1 addition & 0 deletions lib/rpush.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'active_support/all'
require 'net-http2'
require 'jwt'
require 'googleauth'

require 'rails'

Expand Down
5 changes: 5 additions & 0 deletions lib/rpush/client/active_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
require 'rpush/client/active_model/adm/app'
require 'rpush/client/active_model/adm/notification'

require 'rpush/client/active_model/fcm/expiry_collapse_key_mutual_inclusion_validator'
require 'rpush/client/active_model/fcm/notification_keys_in_allowed_list_validator'
require 'rpush/client/active_model/fcm/app'
require 'rpush/client/active_model/fcm/notification'

require 'rpush/client/active_model/gcm/expiry_collapse_key_mutual_inclusion_validator'
require 'rpush/client/active_model/gcm/app'
require 'rpush/client/active_model/gcm/notification'
Expand Down
20 changes: 20 additions & 0 deletions lib/rpush/client/active_model/fcm/app.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module Rpush
module Client
module ActiveModel
module Fcm
module App
def self.included(base)
base.instance_eval do
# TODO: Add whatever validation is needed here
# validates :auth_key, presence: true
end
end

def service_name
'fcm'
end
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
module Rpush
module Client
module ActiveModel
module Fcm
class ExpiryCollapseKeyMutualInclusionValidator < ::ActiveModel::Validator
def validate(record)
return unless record.collapse_key && !record.expiry
record.errors.add :expiry, 'must be set when using a collapse_key'
end
end
end
end
end
end
118 changes: 118 additions & 0 deletions lib/rpush/client/active_model/fcm/notification.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
module Rpush
module Client
module ActiveModel
module Fcm
module Notification
FCM_PRIORITY_HIGH = Rpush::Client::ActiveModel::Apns::Notification::APNS_PRIORITY_IMMEDIATE
FCM_PRIORITY_NORMAL = Rpush::Client::ActiveModel::Apns::Notification::APNS_PRIORITY_CONSERVE_POWER
FCM_PRIORITIES = [FCM_PRIORITY_HIGH, FCM_PRIORITY_NORMAL]

ROOT_NOTIFICATION_KEYS = %w[title body image].freeze
ANDROID_NOTIFICATION_KEYS = %w[icon tag color click_action body_loc_key body_loc_args title_loc_key
title_loc_args channel_id ticker sticky event_time local_only
default_vibrate_timings default_light_settings vibrate_timings
visibility notification_count light_settings].freeze

def self.included(base)
base.instance_eval do
validates :device_token, presence: true
validates :priority, inclusion: { in: FCM_PRIORITIES }, allow_nil: true

validates_with Rpush::Client::ActiveModel::PayloadDataSizeValidator, limit: 4096
validates_with Rpush::Client::ActiveModel::RegistrationIdsCountValidator, limit: 1000

validates_with Rpush::Client::ActiveModel::Fcm::ExpiryCollapseKeyMutualInclusionValidator
validates_with Rpush::Client::ActiveModel::Fcm::NotificationKeysInAllowedListValidator
end
end

def payload_data_size
multi_json_dump(as_json['message']['data']).bytesize
end

# This is a hack. The schema defines `priority` to be an integer, but FCM expects a string.
# But for users of rpush to have an API they might expect (setting priority to `high`, not 10)
# we do a little conversion here.
def priority=(priority)
case priority
when 'high', FCM_PRIORITY_HIGH
super(FCM_PRIORITY_HIGH)
when 'normal', FCM_PRIORITY_NORMAL
super(FCM_PRIORITY_NORMAL)
else
errors.add(:priority, 'must be one of either "normal" or "high"')
end
end

def dry_run=(value)
fail ArgumentError, 'FCM does not support dry run' if value
end

def mutable_content=(value)
fail ArgumentError, 'RPush does not currently support mutable_content for FCM' if value
end

def as_json(options = nil) # rubocop:disable Metrics/PerceivedComplexity
json = {
'data' => data,
'android' => android_config,
'token' => device_token
}
json['content_available'] = content_available if content_available
json['notification'] = root_notification if notification
{ 'message' => json }
end

def android_config
json = {
'notification' => android_notification,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think we need to set explicitly notification here since original library don't have it and we can set it dynamically when someone creating the object, for some use case devs used to configure all stuff under data key, on those cases empty notification object is needed which helps custom rendering of the pushes

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make it more dynamic and remove setting custom sound, default sound under notification key

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting the notification here can cause issues as data messages would not get handled when the application is running in the background.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@muzzamilpulsate @KrakenWagen This was done to try and make migration as easy as possible. My understanding of the api changes google provided (documentation) was that previously Google's backend manipulated things to make them Android or Apple friendly, while the new api does not.
So either when upgrading to Rpush FCM users will need to change their backend payload or we put something like this in place. I don't mind either way, I thought easy upgrade was preferable.

That said, if this will cause issues with data messages that obviously would need to be fixed. What exactly is the issue with data messages? Is it that default_sound would be set to true in this section? I think everything else would be empty for a data message.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here could be that 'notification' is a reserved keyword, so even if this is not setting the root 'notification' key, setting 'android'->'notification' is enough to cause data messages to not be handled in the background.

https://firebase.google.com/docs/cloud-messaging/concept-options#data_messages

Make sure that you do not use any reserved words in your custom key-value pairs. Reserved words include "from", "notification," "message_type", or any word starting with "google" or "gcm."

Copy link
Contributor

@Henridv Henridv May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting notification here is allowed. It's part of AndroidConfig of the message: https://firebase.google.com/docs/reference/fcm/rest/v1/projects.messages#AndroidConfig

The link you shared is for the data key.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had to comment out this line in order to make it work for our project. We are using data-only messages and including the notification key caused an empty notification to be shown and our custom FirebaseMessagingService was not called.

}
json['collapse_key'] = collapse_key if collapse_key
json['priority'] = priority_str if priority
json['ttl'] = "#{expiry}s" if expiry
json
end

def notification=(value)
super(value.with_indifferent_access)
end

def root_notification
return {} unless notification

notification.slice(*ROOT_NOTIFICATION_KEYS)
end

def android_notification
json = notification&.slice(*ANDROID_NOTIFICATION_KEYS) || {}
json['notification_priority'] = priority_for_notification if priority
json['sound'] = sound if sound
json['default_sound'] = !sound || sound == 'default' ? true : false
json
end

def priority_str
case
when priority <= 5 then 'normal'
else
'high'
end
end

def priority_for_notification
case priority
when 0 then 'PRIORITY_UNSPECIFIED'
when 1 then 'PRIORITY_MIN'
when 2 then 'PRIORITY_LOW'
when 5 then 'PRIORITY_DEFAULT'
when 6 then 'PRIORITY_HIGH'
when 10 then 'PRIORITY_MAX'
else
'PRIORITY_DEFAULT'
end
end
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module Rpush
module Client
module ActiveModel
module Fcm
class NotificationKeysInAllowedListValidator < ::ActiveModel::Validator
def validate(record)
return unless record.notification

allowed_keys = Notification::ROOT_NOTIFICATION_KEYS + Notification::ANDROID_NOTIFICATION_KEYS
invalid_keys = record.notification.keys - allowed_keys

return if invalid_keys.empty?

record.errors.add(:notification, "contains invalid keys: #{invalid_keys.join(', ')}")
end
end
end
end
end
end
3 changes: 3 additions & 0 deletions lib/rpush/client/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
require 'rpush/client/active_record/apnsp8/notification'
require 'rpush/client/active_record/apnsp8/app'

require 'rpush/client/active_record/fcm/notification'
require 'rpush/client/active_record/fcm/app'

require 'rpush/client/active_record/gcm/notification'
require 'rpush/client/active_record/gcm/app'

Expand Down
11 changes: 11 additions & 0 deletions lib/rpush/client/active_record/fcm/app.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Rpush
module Client
module ActiveRecord
module Fcm
class App < Rpush::Client::ActiveRecord::App
include Rpush::Client::ActiveModel::Fcm::App
end
end
end
end
end
11 changes: 11 additions & 0 deletions lib/rpush/client/active_record/fcm/notification.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Rpush
module Client
module ActiveRecord
module Fcm
class Notification < Rpush::Client::ActiveRecord::Notification
include Rpush::Client::ActiveModel::Fcm::Notification
end
end
end
end
end
Loading