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

Split recipients array into chunks to respect FCM limits #50

Merged
merged 3 commits into from
Sep 18, 2019
Merged

Split recipients array into chunks to respect FCM limits #50

merged 3 commits into from
Sep 18, 2019

Conversation

chimit
Copy link
Contributor

@chimit chimit commented Sep 5, 2019

PR to address this issue #48

@chimit
Copy link
Contributor Author

chimit commented Sep 6, 2019

There is one problem with this approach. In case of one request the send() method returns FCM response body. For example,

{"multicast_id":7512246268530429272,"success":0,"failure":2,"canonical_ids":0,"results":[{"error":"InvalidRegistration"},{"error":"InvalidRegistration"}]}

But in case of multiple requests this PR returns only the latest response.

The above request can be split into two separate and the responses would be:

{"multicast_id":7306578091349811732,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]}
{"multicast_id":4783702789326450040,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]}

We could merge multiple response objects into one by adding success, failure, canonical_ids numbers and merging results arrays, but there is also a multicast_id which is unique and can't be just transformed into array. At least if we care about backward compatibility.

What do you guys think?

@benwilkins
Copy link
Owner

Seems like the best option would be to release a major version (due to breaking changes) that returned an array of responses:

[
    {"multicast_id":7306578091349811732,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]},
    {"multicast_id":4783702789326450040,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]}
]

You could then loop over the responses, even if there's only one, and get whatever you need from them.

@chimit
Copy link
Contributor Author

chimit commented Sep 18, 2019

@benwilkins, please, check.

@benwilkins benwilkins merged commit d003d6d into benwilkins:master Sep 18, 2019
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.

2 participants