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

Remove push_id and push_time from push notifications #9126

Open
mtrezza opened this issue May 15, 2024 · 4 comments
Open

Remove push_id and push_time from push notifications #9126

mtrezza opened this issue May 15, 2024 · 4 comments
Labels
type:feature New feature or improvement of existing feature

Comments

@mtrezza
Copy link
Member

mtrezza commented May 15, 2024

New Feature / Enhancement Checklist

Current Limitation

See parse-community/parse-server-push-adapter#237 (comment); push_id and push_time seem to be legacy keys. Their purpose should be investigated. Maybe they can be removed.

Feature / Enhancement Description

Removal with care, as Parse Android SDK and developers' custom apps may use these keys for something, so they cannot be just removed but must be made optional for phasing out.

Copy link

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

@mman
Copy link
Contributor

mman commented May 17, 2024

Dropping this here as my initial investigation:

When a push is received in an Android app using official Parse Android SDK, the message is handled by the FirebaseMessagingService.onMessageReceived method:

https://github.com/parse-community/Parse-SDK-Android/blob/a08c5d1594b3a083671b493804eb1970c1ce6441/fcm/src/main/java/com/parse/fcm/ParseFirebaseMessagingService.java#L20-L44

This method extracts push_id, data, time, and channel parameters.

channel was/is used for channel based push targeting, where clients can subscribe to given list of channels and automatically receive all pushes targeted to that channel.

data is JSON encoded everything and anything that you actually send via Parse.Push.send from the server side.

push_id, and time are then passed over to PushRouter.handlePush where they are double-checked to not be nil/empty.

https://github.com/parse-community/Parse-SDK-Android/blob/a08c5d1594b3a083671b493804eb1970c1ce6441/parse/src/main/java/com/parse/PushRouter.java#L105-L113

  1. If push_id and time are empty, the push is thrown away.
  2. If they are not empty, the push is stored into history, and then passed to Android BroadcastReceiver.

There is a comment atop of the PushRouter.java file explaining that history is needed essentially because duplicate pushes could be actually received when the push was being sent by the GCM.

  • For PPNS, we provide the last-seen timestamp to the server as part of the handshake. This is
  • used as a cursor into the server-side inbox of recent pushes for this client. - For GCM, we use
  • the history to deduplicate pushes when GCM decides to change the canonical registration id for a
  • client (which can result in duplicate pushes while both the old and new registration id are still
  • valid).

So in order to remove push_id and time parameters from the Parse Android SDK, we need to make sure that:

  1. FCM is never sending duplicate pushes to the same device.
  2. timestamp of last contact is not required by FirebaseMessagingService.

Once we verify these two are handled, we can simply drop them from the Parse Android SDK, along with the history, and start propagating updated SDK to the clients.

Later after some years we can remove them from the Parse Push Adapter, once we are sure that there are no clients expecting them.

The risk is that clients in the wild using old Parse Android SDK will stop receiving pushes - will start dropping pushes because of lack of push_id and time.

@mtrezza
Copy link
Member Author

mtrezza commented May 17, 2024

Thanks for the investigation.

Sending these params can be off by default already in the next Parse Server 8 release. We should just make sure to offer the corrected Android SDK at the point.

We would have to check wether other Parse repos use these params. A quick org-wide search for these params on GitHub shows occurrences in the following repos:

  • Parse Android SDK
  • Parse Flutter SDK
  • Parse Dashboard
  • Parse Server Push Adapter
  • docs

@mman
Copy link
Contributor

mman commented May 17, 2024

From what I could see, I believe the params exist only to workaround problematic behaviour of GCM, and I can imagine a client side workaround in the SDK. But anyway, here we are, so the sooner we investigate/confirm that FCM behaves sanely, and the sooner we release updated Android SDK, the better... for the cleanup of SDK and Push Adapter.

Additionally I can imagine a fix where we can actually lift the push_id and time away from the push adapter and let it be passed as part of request data into Parse.Push.send with parse server 8. That way people like me, who care about old Android installations that will not get updated ever... can still send compatible pushes... by adjusting their payload passed to Parse.Push.send...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

2 participants