fix(fcm): correct the iidEndpoint endpoints used for topic management #335

rueian · 2020-02-20T15:15:00Z

According to the document https://developers.google.com/instance-id/reference/server,
the endpoints should be:

https://iid.googleapis.com/iid/v1:batchAdd
https://iid.googleapis.com/iid/v1:batchRemove

NOT:

https://iid.googleapis.com/iid/v1/:batchAdd
https://iid.googleapis.com/iid/v1/:batchRemove

RELEASE NOTE: Updated the remote endpoint used by the topic management operations.

hiranya911

Thanks for the PR. Both these URL patterns (/v1/:suffix and /v1:suffix) resolve to the same endpoint. So there's no actual bug here. Although I do prefer the /v1:suffix format, since that's what's documented.

And please change the base branch of the PR to dev. We don't merge to master directly.

hiranya911 · 2020-02-20T18:45:56Z

messaging/topic_mgt.go

- iidEndpoint = "https://iid.googleapis.com/iid/v1"
- iidSubscribe = ":batchAdd"
- iidUnsubscribe = ":batchRemove"
+ iidEndpoint = "https://iid.googleapis.com/iid"


Instead of this, let's change how the URL is constructed in the makeTopicManagementRequest() function.

iidSubscribe = "batchAdd" iidUnsubscribe = "batchRemove" fmt.Sprintf("%s:%s", c.iidEndpoint, req.op)

This will cause a test failure, so we need to update the tests too.

According to the document https://developers.google.com/instance-id/reference/server, the endpoints should be: https://iid.googleapis.com/iid/v1:batchAdd https://iid.googleapis.com/iid/v1:batchRemove NOT: https://iid.googleapis.com/iid/v1/:batchAdd https://iid.googleapis.com/iid/v1/:batchRemove

rueian · 2020-02-21T07:50:09Z

Thank you for reviewing my PR.

I have changed the base branch to dev, and fix the tests and the makeTopicManagementRequest.

I am not sure that both /v1/:suffix and /v1:suffix are resolved to the same endpoint:
When using invalid or wrong access token to request /v1/:suffix, it will response a html page:

HTTP/1.1 401 Unauthorized
Content-Type: text/html; charset=UTF-8
Connection: close

<HTML>
<HEAD>
<TITLE>Unauthorized</TITLE>
</HEAD>
<BODY BGCOLOR="#FFFFFF" TEXT="#000000">
<H1>Unauthorized</H1>
<H2>Error 401</H2>
</BODY>
</HTML>

However when requesting to /v1:suffix, it will response a json:

HTTP/1.1 401 Unauthorized
Content-Type: application/json; charset=UTF-8
Connection: close

{"error":"Not authenticated or unauthorized"}

Another question, is there any plan to support the https://iid.googleapis.com/iid/v1:batchImport?

hiranya911

LGTM. Thanks @rueian for the PR.

* chore: Removed Travis CI integration (#326) * chore: Added Actions-based release workflow (#331) * chore: Added Actions-based release workflow * Set GOPATH * Fixed working directory for tests * Decrypting credentials into the testdata directory * Added preflight and post check scripts * chore: Running CI workflow on pull_request (#338) * fix(fcm): correct the iidEndpoint endpoints used for topic management (#335) According to the document https://developers.google.com/instance-id/reference/server, the endpoints should be: https://iid.googleapis.com/iid/v1:batchAdd https://iid.googleapis.com/iid/v1:batchRemove NOT: https://iid.googleapis.com/iid/v1/:batchAdd https://iid.googleapis.com/iid/v1/:batchRemove * fix(fcm): Fix documents of FCM batch request limit (#347) Co-authored-by: Hiranya Jayathilaka <[email protected]> * fix: Deferring credential loading until required (#361) * Bumped version to 3.12.1 (#363) * chore: Specifying correct working directory for staging command (#365) Co-authored-by: Rueian <[email protected]> Co-authored-by: 178inaba <[email protected]>

rueian force-pushed the fix-iid-endpoints branch 2 times, most recently from 74bda9f to 63cc148 Compare February 20, 2020 16:14

rueian requested a review from hiranya911 February 20, 2020 17:36

hiranya911 suggested changes Feb 20, 2020

View reviewed changes

hiranya911 assigned rueian Feb 20, 2020

rueian changed the base branch from master to dev February 20, 2020 19:14

rueian force-pushed the fix-iid-endpoints branch 3 times, most recently from fe513b8 to a5e5cf5 Compare February 21, 2020 07:08

rueian force-pushed the fix-iid-endpoints branch from a5e5cf5 to 3303464 Compare February 21, 2020 07:16

rueian requested a review from hiranya911 February 21, 2020 10:55

Merge branch 'dev' into fix-iid-endpoints

6486d72

hiranya911 approved these changes Feb 25, 2020

View reviewed changes

hiranya911 assigned hiranya911 and unassigned rueian Feb 25, 2020

hiranya911 added the release-note label Feb 25, 2020

hiranya911 merged commit 7c2199d into firebase:dev Feb 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(fcm): correct the iidEndpoint endpoints used for topic management #335

fix(fcm): correct the iidEndpoint endpoints used for topic management #335

rueian commented Feb 20, 2020 •

edited by hiranya911

hiranya911 left a comment •

edited

hiranya911 Feb 20, 2020

rueian commented Feb 21, 2020 •

edited

hiranya911 left a comment

fix(fcm): correct the iidEndpoint endpoints used for topic management #335

fix(fcm): correct the iidEndpoint endpoints used for topic management #335

Conversation

rueian commented Feb 20, 2020 • edited by hiranya911

hiranya911 left a comment • edited

Choose a reason for hiding this comment

hiranya911 Feb 20, 2020

Choose a reason for hiding this comment

rueian commented Feb 21, 2020 • edited

hiranya911 left a comment

Choose a reason for hiding this comment

rueian commented Feb 20, 2020 •

edited by hiranya911

hiranya911 left a comment •

edited

rueian commented Feb 21, 2020 •

edited