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

Use OAuth2 token instead of server key for v1 #123

Merged
merged 19 commits into from
Sep 18, 2024

Conversation

erimicel
Copy link
Contributor

@erimicel erimicel commented Sep 2, 2024

Added this PR thanks to @aap17 's change
#122 (comment)

Apps using the deprecated FCM legacy APIs for HTTP and XMPP should migrate to the HTTP v1 API at the earliest opportunity. Sending messages (including upstream messages) with those APIs was deprecated on June 20, 2023, and shutdown begins on July 22, 2024.

https://firebase.google.com/docs/cloud-messaging/migrate-v1
Before
Authorization: key=AIzaSyZ-1u...0GBYzPu7Udno5aA

After
Authorization: Bearer ya29.ElqKBGN2Ri_Uz...HnS_uNreA

This is already breaking change for sending messages via legacy api and should be removed;

Breaking Changes

  • Remove deprecated API_KEY
  • Remove deprecated send method
  • Remove deprecated send_with_notification_key method
  • Remove subscribe_instance_id_to_topic method
  • Remove unsubscribe_instance_id_from_topic method
  • Remove batch_subscribe_instance_ids_to_topic method
  • Remove batch_unsubscribe_instance_ids_from_topic method

Supported Features

  • Add HTTP v1 API support for send_to_topic_condition method
  • Add HTTP v1 API support for send_to_topic method

TODO

  • Adding tests for failing responses

lib/fcm.rb Show resolved Hide resolved
lib/fcm.rb Show resolved Hide resolved
lib/fcm.rb Outdated Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
@sabman sabman mentioned this pull request Sep 8, 2024
@sabman
Copy link
Member

sabman commented Sep 8, 2024

@erimicel thanks for doing this. Could you also look at the README.md and review if the docs are up-to-date with this PR. We can then cut a new version. Since its not backwards compatible we will cut a new version: 2.0.0

spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
lib/fcm.rb Outdated Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
@erimicel
Copy link
Contributor Author

erimicel commented Sep 9, 2024

@erimicel thanks for doing this. Could you also look at the README.md and review if the docs are up-to-date with this PR. We can then cut a new version. Since its not backwards compatible we will cut a new version: 2.0.0

@sabman I've added a more detailed README. The core changes are listed below—could you review and let me know if they look sensible to you?

The PR also has several styling issues flagged. Let me know if you think those need to be addressed.

I performed a real production environment test with the given changes for each method, and all looks fine at the moment.

Breaking Changes

  • Remove deprecated API_KEY
  • Remove deprecated send method
  • Remove deprecated send_with_notification_key method
  • Remove subscribe_instance_id_to_topic method as dup of topic_subscription method
  • Remove unsubscribe_instance_id_from_topic method as dup of topic_unsubscription method
  • Remove batch_subscribe_instance_ids_to_topic method as dup of topic_bath_subscription method
  • Remove batch_unsubscribe_instance_ids_from_topic method as dup of topic_bath_unsubscription method

Supported Features

  • Add HTTP v1 API support for send_to_topic_condition method
  • Add HTTP v1 API support for send_to_topic method

We could leave the duplicated topic subscription methods in place and soft delete the api_key. However, if we release a new version, it might be a good opportunity for cleanup. But removing API_KEY will definitely break client initializers for everybody, so I am not sure.

spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
lib/fcm.rb Outdated Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
spec/fcm_spec.rb Show resolved Hide resolved
@tiendo1011
Copy link

tiendo1011 commented Sep 17, 2024

@AllanQ can we merge this?

@tiendo1011
Copy link

@sabman can we merge this?

@AllanQ
Copy link

AllanQ commented Sep 18, 2024

@AllanQ can we merge this?

@tiendo1011,
I am not authorized to make the decision.

@sabman
Copy link
Member

sabman commented Sep 18, 2024

Just running tests will merge shortly.

@sabman sabman merged commit f0f400a into decision-labs:master Sep 18, 2024
5 checks passed
@sabman
Copy link
Member

sabman commented Sep 18, 2024

Thanks to all the awesome contributors!

@erimicel erimicel deleted the v1-OAuth2-migration-update branch September 19, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants