-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add support for push #311
Add support for push #311
Conversation
681809f
to
8f255d6
Compare
@wangmengyan95 updated the pull request. |
needs rebase |
8f255d6
to
0cb60f7
Compare
@wangmengyan95 updated the pull request. |
+1 love this PR. I got it to work right out of the box (for GCM) |
* Register push senders | ||
* @param {Object} pushConfig The push configuration which is given when parse server is initialized | ||
*/ | ||
ParsePushAdapter.prototype.registerPushSenders = function(pushConfig) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a dissociation per App ID? I'm almost done for the multiple apps support.
👍 |
This is awesome stuff, @wangmengyan95 👍 |
Does this mean following code will work on Parse Server?
|
Yes it should. I tested it last night with curl:
|
awesome work 👍 |
@rogerhu Do you know if it works for targeting spesific users with a query to Installations?
|
Could we get a write up on this where to push the apns certs (does it allow for dev push and then switch to production push for apple) ? Whats the recommended way to que up some push notifications (message que like rabbitmq or service like iron.io?) Sorry just a few questions and thank @wangmengyan95 very much for adding this. |
Looks good to me although I don't really know that much about push. Make sure to write some docs before we push 2.0.8. |
@drew-gross let's waiting for @bnham 's comment. |
0cb60f7
to
76d9871
Compare
@wangmengyan95 updated the pull request. |
Amazing to see a Push service implementation so quickly! 👍 |
Amazing!! Thanks!!! |
@wangmengyan95 updated the pull request. |
8cb51bd
to
e523be2
Compare
@wangmengyan95 updated the pull request. |
// For android, we can only have 1000 recepients per send | ||
let chunkDevices = sliceDevices(pushType, devices, GCM.GCMRegistrationTokensMax); | ||
for (let chunkDevice of chunkDevices) { | ||
sendPromises.push(sender.send(data, chunkDevice)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably fine for a start but it's not really the right sending strategy for APNs.
What this will do for an active app is it will basically spam the dev cert with a bunch of invalid device tokens. Especially in the old APNs binary protocol (which we are still using in this PR, because it looks like there isn't an APNs module that uses the new HTTP/2 protocol), sending a push to an invalid token is extremely expensive. Sending a push to an invalid token will cause Apple to close the socket, and then you have to reopen the socket, which requires the server to validate the client's cert and the client to validate the server's cert (since APNs uses the TLS client auth option). That handshake process takes ~250ms. So in essence, if you sent a push to 100k invalid device tokens, it would take 100k * 250 ms = 25000 seconds to get through those 100k pushes, and Apple would probably block your server's IP from connecting to gateway.sandbox.push.apple.com because it would interpret this as a DoS attack. (This actually happens--we have periodically dealt with this server side when we pushed out buggy deploys in the past.)
What you need to is to examine each installation object and select the right set of certs for that installation on an installation-by-installation basis. The process should look something like this:
- Check to see if the installation has an
appIdentifier
field (which is the bundle id of the app). If it's missing this field, go to step 4. - Find all certs that are associated with this bundle id. Hopefully, there's only one cert associated with that bundle id (this happens if the dev is smart enough to give the dev and prod versions of their app different bundle ids). In that case, only send the push using that cert.
- If there are multiple certs for a given bundle id, then first send the push using a prod cert (as the prod cert is likely to be associated with more devices). If that fails, only then send the push using the dev cert.
- If the installation has no
appIdentifier
field, then you just have to try sending the push with all certs. Try prod certs first, then dev certs.
In particular, we should definitely not be sending pushes using the prod cert and dev cert at the same time. Only send a push to a dev cert if sending the push using the prod cert fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bnham mind 💥 ! Should we implement a queue system around the push then to make it fully asynchronous? Should we write an APN over HTTP/2 module to help with the SSL handshakes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea this gets a bit tricky since a user can register for push notifications , and then later go into settings and disable this. Which then the app/database should make sure to disable that token. I'm not sure how the Parse.com handle the following situations.
- user disables push after enabling it for a while
- user gets a new device and enables push (two tokens now or 2 users in install class in the db?)
- token no longer valid (is this removed or disabled)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
user disables push after enabling it for a while
That falls into the invalid token, the trick is that marking this token invalid would prevent further sends when the user toggle notifications back on from the Settings app. I am not sure how the system handles that, if it issues a new token or the same as the previous one.
In any case, I don't recall any feature in Parse that would explicitly invalidate a token.
If the developer provide a way to disable notifications, then it's in it's Push's Installation query that he should handle such thing.
user gets a new device and enables push (two tokens now or 2 users in install class in the db?)
This is why push token are set on installations, and not on users. A push is delivered to a device, the developer has to manually map the user onto the installation either by a reaction or a Many to one Installation -> User.
token no longer valid (is this removed or disabled)
Maybe that's the developer's responsibility to set the device token to nil when the app fails to register to a remote notification, I'm not sure here.
rebase? |
6465f51
to
2fc345a
Compare
@wangmengyan95 updated the pull request. |
1 similar comment
@wangmengyan95 updated the pull request. |
LGTM for v1 |
@wangmengyan95 updated the pull request. |
2fc345a
to
273a207
Compare
@gfosco Feel free to merge when you decide to make new release. |
@wangmengyan95 updated the pull request. |
Loving the parse community for getting this implemented ❤️ 👍 Thanks guys, well done! |
Something to think about is that this system should hit the APNS feedback server, this will return to you invalid or not in use device tokens that then can be disabled in the the database and not send out pushes out to. Maybe something like this.. note this was taken from a stackoverflow post..
|
Yeah, the feedback service is a nice-to-have, and of course a PR is welcomed. (Parse never used the feedback service or disabled tokens.) |
Forgot that I had to disable the |
are you planning to implement "push_time for scheduled push" functionality in near future? |
Add basic push for parse server.
For
APNS
andGCM
send(data, deviceTokens)
tosend(data, devices)
.device
is an object. Its format isThis allows us to pass some extra data to
GCM
andAPNS
to generate payload. We do not use it right now, but I think it is useful in the future.3. Add
GCM
APNS
response to log.For
PushAdapter
PushAdapter
maintains an instance of push adapter. Developers can register their own adapter in the future.For
ParsePushAdapter
PushAdapter
. It gets the installations frompush.js
, classify the installations and use proper push senders to send the push notifications.