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.

Sign up for GitHub

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

Jump to bottom

[autoendpoint] Route FCM notifications #162

Closed
AzureMarker opened this issue Jun 22, 2020 · 4 comments · Fixed by #171 or #231
Closed

[autoendpoint] Route FCM notifications #162

AzureMarker opened this issue Jun 22, 2020 · 4 comments · Fixed by #171 or #231
Assignees
@AzureMarker
Labels
5 Estimate - Medium
Milestone
Autoendpoint Rust...

Comments

@AzureMarker
Copy link
Contributor

AzureMarker commented Jun 22, 2020 •
edited
Loading

WebPush notifications (destined for Android user agents using FCM) should be routed through Firebase Cloud Messaging.

Questions:

  • Do both versions of FCM need to be supported (legacy and v1)?
    Answer: Only support v1
@AzureMarker AzureMarker added this to the Autoendpoint Rust Server milestone Jun 22, 2020
@AzureMarker AzureMarker self-assigned this Jun 22, 2020
@AzureMarker AzureMarker mentioned this issue Jun 22, 2020
Closed
@AzureMarker
Copy link
Contributor Author

#163 was closed because we can route GCM messages with legacy FCM. However, now the question is if can we route legacy FCM over v1 FCM, removing the need to support legacy FCM.

@tublitzed tublitzed added 3 Estimate - Small...ish 5 Estimate - Medium and removed 3 Estimate - Small...ish labels Jun 29, 2020
@AzureMarker
Copy link
Contributor Author

Furthermore, GCM no longer functions:

The GCM server and client APIs were removed on May 29, 2019, and currently any calls to those APIs can be expected to fail.

https://developers.google.com/cloud-messaging

We should remove "gcm" from the list of valid router types.

@AzureMarker
Copy link
Contributor Author

FCM legacy and v1 can both handle FCM messages, so we should at least support v1. @jrconlin Given this, is there any reason we should also support FCM legacy?

@jrconlin
Copy link
Member

No. We can drop GCM and FCM legacy at this point.

@AzureMarker AzureMarker mentioned this issue Jul 9, 2020
Merged
@AzureMarker AzureMarker linked a pull request Jul 9, 2020 that will close this issue
feat: Route notifications to FCM (Android) #171
Merged
AzureMarker added a commit that referenced this issue Jul 20, 2020
@AzureMarker
feat: Route notifications to FCM (Android) (#171)
d9a0d9d 
* Move RouterType to extractors/routers.rs

* Remove GCM as a router type

Google Cloud Messaging has been deprecated in favor of FCM, and as of
May 29, 2019 it no longer functions.

* Add FCM router stub

* Remove some unnecessary FCM settings

* Implement insert_opt on HashMap for more concise code

Also renamed the Content-Encoding header field to "encoding", which is
what the rest of autopush/webpush expects.

* Implement (most of) FcmRouter

* Implement FcmClient

* Differentiate between a connection error and a timeout in FcmClient

* Log and increment metrics in FcmRouter when a FcmClient error occurs

* Add RouterResponse::success to avoid code duplication between routers

* Clarify how long MAX_TTL is

* Move the base FCM url to settings

This will make testing easier, for example with mockito.

* Error if unknown settings are provided

* Add tests to FcmClient

* Add .gitguardian.yml

* Add some logging to FcmRouter

* Fix returning an invalid URL after routing a notification

* Add tests to FcmRouter

* Make router_data optional

This fixes the integration tests, because sometimes users would not have
any router data in DynamoDB. Also, just to be safe, router_data uses
rusoto's AttributeValue instead of serde_json::Value.

* Switch router_data back to serde_json::Value

This allows us to use DynamoDbUser with any version of rusoto.

* Make FCM request timeout a config option

* Use a more obvious UUID when testing

Closes #162
This was referenced Oct 16, 2020
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AzureMarker AzureMarker

Labels
5 Estimate - Medium
Projects
None yet
Milestone
Autoendpoint Rust Server
3 participants
@jrconlin @tublitzed @AzureMarker