Push handling cleanup

[natario1]

Following #729 , this does not solve the issue but makes it easier to think of a fix, and also introduce FCM in the future.

• General clean up, renaming some classes
• PPNS is ripped out
• PushHandler interface for handling payloads of different providers (GCM, FCM)
• PushService acts as a router and streamlines payloads to handlers in a single thread

To fix 729 I would probably act on ServiceUtils to launch PushService either as a Service or as a job depending on API level

[codecov]

Codecov Report

Merging #732 into master will increase coverage by 0.43%.
The diff coverage is 33.62%.

@@             Coverage Diff              @@
##             master     #732      +/-   ##
============================================
+ Coverage     52.98%   53.42%   +0.43%     
- Complexity     1730     1745      +15     
============================================
  Files           132      131       -1     
  Lines         10261    10258       -3     
  Branches       1433     1435       +2     
============================================
+ Hits           5437     5480      +43     
+ Misses         4375     4325      -50     
- Partials        449      453       +4

Impacted Files	Coverage Δ	Complexity Δ
Parse/src/main/java/com/parse/ServiceUtils.java	8.57% <ø> (+8.57%)	2 <0> (+2)	⬆️
Parse/src/main/java/com/parse/GcmRegistrar.java	0% <ø> (-6.1%)	0 <0> (-3)
...in/java/com/parse/ParsePushChannelsController.java	6.45% <0%> (ø)	2 <0> (ø)	⬇️
Parse/src/main/java/com/parse/GcmPushHandler.java	1.58% <1.58%> (ø)	1 <1> (?)
Parse/src/main/java/com/parse/Parse.java	48.37% <100%> (-1.4%)	21 <0> (-2)
...ain/java/com/parse/ParsePushBroadcastReceiver.java	4.83% <54.54%> (+4.83%)	3 <3> (+3)	⬆️
Parse/src/main/java/com/parse/PushType.java	76.92% <66.66%> (+3.58%)	5 <0> (ø)	⬇️
Parse/src/main/java/com/parse/PushHandler.java	71.42% <71.42%> (ø)	0 <0> (?)
Parse/src/main/java/com/parse/PushService.java	85.93% <90%> (+85.93%)	18 <6> (+18)	⬆️
... and 7 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a69d994...d6f4e08. Read the comment docs.

[rogerhu]

why delete this constant?

[natario1]

I had my brain in cleanup mode :) and this looked already clear from the method implementation to me. I can add again.

I can also add some tests to the patch but first wanted to know if there's consensus over this being merged

[rogerhu]

PR works for me. It looks like you removed PPNS while also refactoring to a PushHandler interface with an eye towards allowing the FCM handler? The only thing is that FCM/GCM shouldn't co-exist so we may need to move hasAnyGcmSpecificDeclaration() out too as part of the interface? (canHandlePush?)

[natario1]

That sounds good! I will do it. In the future we might even make the handler public so people can provide their adapters like parse server.

By the way, is parse-server ready to send FCM-like messages? AFAIK currently the handlers here need to update the ParseInstallation pushType to tell parse server adapter how to send the push. What happens when we start setting pushType = 'fcm' ?

[rogerhu]

Yeah parse server may need to be upgraded. But the push tokens should be interoperable so hopefully the changes are minimal.

[rogerhu]

FCM is already handled:

https://github.com/parse-server-modules/parse-server-push-adapter/blob/dfa75b51eb3fb909572ad7ecaa11f29a9ecc917f/src/ParsePushAdapter.js#L15

[rogerhu]

I do think that the deviceType (

Parse-SDK-Android/Parse/src/main/java/com/parse/ParseInstallation.java

Line 288 in 6e8804c

String deviceType = "android";

) is what gets used over GCM/FCM right now..

See https://github.com/parse-server-modules/parse-server-push-adapter/blob/ef59a8c8eec303ad9547e02223765a3699cc5032/src/PushAdapterUtils.js#L20

[natario1]

The actual send implementation is still GCM based though? https://github.com/parse-server-modules/parse-server-push-adapter/blob/master/src/GCM.js I don't know if that's a problem and how we should manage

[natario1]

This got bigger than I expected but seems solid to me. Makes it easy to fix #729 (I hope), and to add FCM (I hope).

I was very tempted to rip out PushRouter and PushHistory but resisted, though I'm not 100% sure they are still useful. I hope this can be done in the future.

I also think we could add PushService to our own manifest to ease the developer set-up. The developer would still be able to choose not to support push, by not registering any ParsePushBroadcastReceiver. But I don't know what would happen when merging manifests that both have a push service declared, so I gave up.

I also think we could add GcmBroadcastReceiver (or FCM in the future) to our manifest and remove a lot of boilerplate from everyone (would have to take care of #638 use case)

[rogerhu]

Thanks for doing this!