Adding Firebase Cloud Messaging #62

chemidy · 2018-01-02T19:15:29Z

I just add Firebase Cloud Messaging V1 API support

But it's not very easy to test everything with integration test (all kind : APNS, Android, WebPush with different configurations : data, notification).

hiranya911 · 2018-01-02T19:52:52Z

@chemidy This is fantastic! Your timing couldn't have been better. You may have noticed that I've started to work on FCM support in Node.js and Java Admin SDKs. Your PR will help us greatly speed up the development efforts.

Since this is a new API addition, we need to get this through some internal design reviews. It will take some time, so please hang tight. Feel free to ask any questions in the meantime, and please do keep those code contributions coming 👍

hiranya911 · 2018-01-02T19:57:29Z

messaging/messaging.go

+//
+// Send a message to specified target (a registration token, topic or condition).
+// https://firebase.google.com/docs/cloud-messaging/send-message
+func (c *Client) SendMessage(ctx context.Context, payload *RequestMessage) (msg *ResponseMessage, err error) {


Just spitballing some ideas here. How about the following API signatures?

func (c *Client) Send(ctx context.Context, message *Message) (string, error) func (c *Client) SendDryRun(ctx context.Context, message *Message) (string, error)

This is what I was going for in Node.js and Java environments. We accept a Message instead of RequestMessage, and do the necessary wrapping in the SDK. Also we return the string ID returned by the FCM server, instead of the parsed JSON struct.

OK it sounds good to me, i'm working on it

… / sendDryRun

hiranya911 · 2018-01-17T23:54:03Z

Looks like I closed this accidentally. Sorry about that. We are still very much interested in this. Work is underway to get the necessary internal approvals for the API.

hiranya911 · 2018-01-17T23:55:45Z

messaging/messaging.go

+const messagingEndpoint = "https://fcm.googleapis.com/v1"
+
+var errorCodes = map[int]string{
+	400: "malformed argument",


Use http constants as keys?

hiranya911 · 2018-01-17T23:56:31Z

messaging/messaging.go

+
+// ResponseMessage is the identifier of the message sent.
+// See https://firebase.google.com/docs/reference/fcm/rest/v1/projects.messages
+type ResponseMessage struct {


Rename to responseMessage and remove from the public API.

hiranya911 · 2018-01-18T00:13:13Z

messaging/messaging.go

+
+// validators
+
+// TODO add validator : Data messages can have a 4KB maximum payload.


In other SDKs I've implemented following validations:

Topic name doesn't have the /topics/ prefix.

Topic name contains the valid

Exactly one of topic, token or condition is specified

AndroidConfig.Ttl has the correct format

AndroidConfig.Priority value is valid

AndroidNotification.Color has the correct format

TitleLocKey is specified when TitleLocArgs are specified

BodyLocKey is specified when BodyLocArgs are specified

See firebase/firebase-admin-java#127 for reference.

hiranya911 · 2018-01-18T00:13:29Z

messaging/messaging.go

+// ApnsConfig is Apple Push Notification Service specific options.
+// See https://firebase.google.com/docs/reference/fcm/rest/v1/projects.messages#ApnsConfig
+type ApnsConfig struct {
+	Headers map[string]interface{} `json:"headers,omitempty"`


map[string]string

hiranya911 · 2018-01-18T00:13:48Z

messaging/messaging.go

+// WebpushConfig is Webpush protocol options.
+// See https://firebase.google.com/docs/reference/fcm/rest/v1/projects.messages#WebpushConfig
+type WebpushConfig struct {
+	Headers      map[string]interface{} `json:"headers,omitempty"`


map[string]string for both Headers and Data

hiranya911 · 2018-01-18T00:13:58Z

messaging/messaging.go

+	Priority              string                 `json:"priority,omitempty"`
+	TTL                   string                 `json:"ttl,omitempty"`
+	RestrictedPackageName string                 `json:"restricted_package_name,omitempty"`
+	Data                  map[string]interface{} `json:"data,omitempty"`


map[string]string

hiranya911 · 2018-01-18T00:17:39Z

messaging/messaging_test.go

+		t.Errorf("response Name = %q; want = %q", name, msgName)
+	}
+
+	if !strings.HasPrefix(name, "projects/test-project/messages/") {


I believe this is redundant since we already test for name != msgName above.

hiranya911 · 2018-01-18T00:17:58Z

messaging/messaging_test.go

+		t.Errorf("response Name = %q; want = %q", name, msgName)
+	}
+
+	if !strings.HasPrefix(name, "projects/test-project/messages/") {


hiranya911 · 2018-01-18T00:18:45Z

messaging/messaging_test.go

+		t.Errorf("response Name = %q; want prefix = %q", name, "projects/test-project/messages/")
+	}
+
+	if tr.Body == nil {


Can we do a stronger check here to make sure both the message and the validate_only params were passed to the server?

hiranya911 · 2018-01-18T00:21:40Z

integration/messaging/messaging_test.go

+		return
+	}
+
+	token, err := ioutil.ReadFile(internal.Resource("integration_token.txt"))


All these files require a lot of setting up. Lets keep this simple by doing something like: https://github.com/firebase/firebase-admin-java/blob/a4661a0f44d1dc35d1531eac78f9e3459d6bccb7/src/test/java/com/google/firebase/messaging/FirebaseMessagingIT.java

…ing]interface{} as map[string]string

hiranya911

Looks mostly good. Thanks @chemidy for making the changes.

Update from my end is that we are currently finalizing the API review process. We have decided to add a couple of additional types to support better integration with APNS, but we can do that as a follow up PR once this PR has been merged.

hiranya911 · 2018-01-22T23:05:11Z

messaging/messaging.go

@@ -128,8 +128,8 @@ type WebpushNotification struct {
 // ApnsConfig is Apple Push Notification Service specific options.
 // See https://firebase.google.com/docs/reference/fcm/rest/v1/projects.messages#ApnsConfig
 type ApnsConfig struct {


Should be APNSConfig as per Go conventions.

hiranya911 · 2018-01-26T01:03:32Z

This looks great @chemidy. Thanks again for patiently working through all the feedback. This API is now approved, and I will start the process for getting it merged soon.

I would like to do a bit of work on top of what you've already done before I merge this to dev. So what I'd do is accept the PR and merge it into a separate fcm branch, and do my changes there. Finally I can merge the whole thing to dev. Does that sound reasonable to you? Do you plan to do further changes before I merge this (I see a number of TODOs in the code)?

chemidy · 2018-01-26T08:47:35Z

I was very busy lately, as you said there are TODOs for validators.
I can work on it tomorrow, but if you need to work quickly on fcm you can merge it on your branch and work on it :)

It's ok for me thks for the reviews.

hiranya911 · 2018-01-26T18:06:34Z

No problem @chemidy. I'll wait for you to finish your changes.

hiranya911 · 2018-01-30T20:16:34Z

Hi @chemidy. Do you have an ETA on when the remaining work can be finished?

chemidy · 2018-01-30T21:33:26Z

sorry I was sick this weekend :(

I just added the validators, I let you take over, you'll probably have more availability than me to finish the feature.

hiranya911

LGTM 👍

Thanks @chemidy for seeing this through. I'll take it from here.

* Adding Firebase Cloud Messaging (#62) * initial commit for adding Firebase Cloud Messaging * add validator * use http const in messaging test * add client version header for stats * init integration test * add integration test (validated on IOS today) * add comment with URL to enable Firebase Cloud Messaging API * fix broken test * add integration tests * accept a Message instead of RequestMessage + and rename method + send / sendDryRun * update fcm url * rollback url endpoint * fix http constants, change responseMessage visibility, change map[string]interface{} as map[string]string * fix http constants * fix integration tests * fix APNS naming * add validators * Added APNS types; Updated tests * Added more tests; Fixed APNS serialization * Updated documentation * Improved error handling inFCM * Added utils file * Updated integration tests * Implemented topic management operations * Added integration tests * Updated CHANGELOG * Addressing code review comments * Supporting 0 valued Aps.Badge * Addressing some review comments * Removed some unused vars * Accepting prefixed topic names (#84) * Accepting prefixed topic named * Added a comment * Using new FCM error codes (#89)

* Renamed some tests and test parameters for clarity, and adhere to Go conventions (#74) * clean unused types (#76) * Create CHANGELOG.md (#75) (#79) * Create CHANGELOG.md Initial changelog based on https://firebase.google.com/support/release-notes/admin/go * change instance ID format (#82) Changing the format of the "non-existing" instance ID in the integration tests to comply with the expected iid format. * Import context from golang.org/x/net/ for 1.6 compatibility (#87) * import golang.org/x/net/context instead of context for 1.6 compatibility * Document non existing name in integration tests for iid (#85) * Revoke Tokens (#77) Adding TokensValidAfterMillis property, RevokeRefreshTokens(), and VerifyIDTokenAndCheckRevoked(). * Firebase Cloud Messaging API (#81) * Adding Firebase Cloud Messaging (#62) * initial commit for adding Firebase Cloud Messaging * add validator * use http const in messaging test * add client version header for stats * init integration test * add integration test (validated on IOS today) * add comment with URL to enable Firebase Cloud Messaging API * fix broken test * add integration tests * accept a Message instead of RequestMessage + and rename method + send / sendDryRun * update fcm url * rollback url endpoint * fix http constants, change responseMessage visibility, change map[string]interface{} as map[string]string * fix http constants * fix integration tests * fix APNS naming * add validators * Added APNS types; Updated tests * Added more tests; Fixed APNS serialization * Updated documentation * Improved error handling inFCM * Added utils file * Updated integration tests * Implemented topic management operations * Added integration tests * Updated CHANGELOG * Addressing code review comments * Supporting 0 valued Aps.Badge * Addressing some review comments * Removed some unused vars * Accepting prefixed topic names (#84) * Accepting prefixed topic named * Added a comment * Using new FCM error codes (#89) * Bumped version to 2.5.0 (#90)

* Renamed some tests and test parameters for clarity, and adhere to Go conventions (#74) * clean unused types (#76) * Create CHANGELOG.md (#75) (#79) * Create CHANGELOG.md Initial changelog based on https://firebase.google.com/support/release-notes/admin/go * change instance ID format (#82) Changing the format of the "non-existing" instance ID in the integration tests to comply with the expected iid format. * Import context from golang.org/x/net/ for 1.6 compatibility (#87) * import golang.org/x/net/context instead of context for 1.6 compatibility * Document non existing name in integration tests for iid (#85) * Revoke Tokens (#77) Adding TokensValidAfterMillis property, RevokeRefreshTokens(), and VerifyIDTokenAndCheckRevoked(). * Firebase Cloud Messaging API (#81) * Adding Firebase Cloud Messaging (#62) * initial commit for adding Firebase Cloud Messaging * add validator * use http const in messaging test * add client version header for stats * init integration test * add integration test (validated on IOS today) * add comment with URL to enable Firebase Cloud Messaging API * fix broken test * add integration tests * accept a Message instead of RequestMessage + and rename method + send / sendDryRun * update fcm url * rollback url endpoint * fix http constants, change responseMessage visibility, change map[string]interface{} as map[string]string * fix http constants * fix integration tests * fix APNS naming * add validators * Added APNS types; Updated tests * Added more tests; Fixed APNS serialization * Updated documentation * Improved error handling inFCM * Added utils file * Updated integration tests * Implemented topic management operations * Added integration tests * Updated CHANGELOG * Addressing code review comments * Supporting 0 valued Aps.Badge * Addressing some review comments * Removed some unused vars * Accepting prefixed topic names (#84) * Accepting prefixed topic named * Added a comment * Using new FCM error codes (#89) * Bumped version to 2.5.0 (#90) * Lint (#96) * fix misspelling * add check error * missing copyright * Doc (#97) * update readme with Authentication Guide & Release Notes * fix a misspelling : separately * fix missing newline before package * add Go Report Card + update doc * add travis build for go versions 1.7.x -> 1.10.x (#98) * add build for go version 1.6.x -> 1.10.x * fix 1.10 version * fix context to golang.org/x/net/context for go 1.6 compatibility * add race detector + go vet on build + build without failure on go unstable * add go16 et go17 file due to req.withcontext which is only go 1.7 * fix context package * update go16.go to remove WithContext * update bad import * remove unused func * finally use ctxhttp.Do with multiple build version * ignore integration package for install * fix go get command * put go 1.6.X in allow_failures dur to test failure * fix inversion of code * remove go 1.6 support * revert initial version with req.WithContext * fix travis to support go 1.10.x * nits * Import context from standard package (#101) * Import context from standard package. * Firebase Database API (#92) * Experimental RTDB code * Added ref.Set() * Added Push(), Update(), Remove() and tests * Adding Transaction() support * Fixed Transaction() API * Code cleanup * Implemented Query() API * Added GetIfChanged() and integration tests * More integration tests * Updated unit test * More integration tests * Integration tests for queries * Auth override support and more tests * More test cases; AuthOverride support in App * Implemented AuthOverride support; Added tests * Implementing the new API * More code cleanup * Code clean up * Refactored the http client code * More tests * Boosted test coverage to 97% * Better error messages in tests; Added license headers * Added documentatioon and cleaned up tests * Fixing a build break * Finishing up documentation * More test cases * Implemented a reusable HTTP client API * Added test cases * Comment clean up * Using the shared http client API * Simplified the usage by adding HTTPClient * using the new client API * Using the old ctx import * Using the old context import * Refactored db code * More refactoring * Support for arbitrary entity types in the request * Renamed fields; Added documentation * Removing a redundant else case * Code readability improvements * Cleaned up the RTDB HTTP client code * Added shallow reads support; Added the new txn API * Implementing GetOrdered() for queries * Adding more sorting tests * Added Query ordering tests * Fixing some lint errors and compilation errors * Removing unused function * Cleaned up unit tests for db * Updated query impl and tests * Added integration tests for ordered queries * Removed With*() from query functions * Updated change log; Added more tests * Support for database url in auto init * Support for loading auth overrides from env * Removed db.AuthOverride type * Renamed ao to authOverride everywhere; Other code review nits * Introducing the QueryNode interface to handle ordered query results (#100) * Database Sample Snippets (#102) * Adding database snippets * Adding query snippets * Added complex query samples * Updated variable name * Fixing a typo * Fixing query example * Updated DB snippets to use GetOrdered() * Removing unnecessary placeholders in Fatalln() calls * Removing unnecessary placeholders in Fatalln() calls * Handling FCM canonical error codes (#103) * Formatting test file with gofmt (#104) * Bumped version to 2.6.0 (#105) * Formatting (simplification) changes (#107) * Checking for unformatted files in CI (#108) * Checking for unformatted files in CI * Adding newline at eof * Document Minimum Go Version (#111) * Fix invalid endpoint URL for topic unsubscribe (#114) * Fix error message for missing user (#113) * Update CHANGELOG.md (#117) * Removing unused member from auth.Client (#118) * Support Go 1.6 (#120) * all: use golang.org/x/net/context * internal: use ctxhttp to use /x/ context The 1.6 Request type doesn't have WithContext. * all: don't use subtests to keep 1.6 compatibility * integration: use float64 for fields with exp value Values like -7e+07 cannot be parsed into ints in Go 1.6. So, use floats instead. * integration/messaging: use t.Fatal not log.Fatal * travis: add 1.6.x * changelog: mention addition of 1.6 support * readme: mention go version support * Bumped version to 2.6.1 (#121) * Changlog updates (#123)

* Adding Firebase Cloud Messaging (#62) * initial commit for adding Firebase Cloud Messaging * add validator * use http const in messaging test * add client version header for stats * init integration test * add integration test (validated on IOS today) * add comment with URL to enable Firebase Cloud Messaging API * fix broken test * add integration tests * accept a Message instead of RequestMessage + and rename method + send / sendDryRun * update fcm url * rollback url endpoint * fix http constants, change responseMessage visibility, change map[string]interface{} as map[string]string * fix http constants * fix integration tests * fix APNS naming * add validators * Added APNS types; Updated tests * Added more tests; Fixed APNS serialization * Updated documentation * Improved error handling inFCM * Added utils file * Updated integration tests * Implemented topic management operations * Added integration tests * Updated CHANGELOG * Addressing code review comments * Supporting 0 valued Aps.Badge * Addressing some review comments * Removed some unused vars * Accepting prefixed topic names (#84) * Accepting prefixed topic named * Added a comment * Using new FCM error codes (#89)

chemidy and others added 11 commits December 26, 2017 14:18

initial commit for adding Firebase Cloud Messaging

f6208a4

add validator

0759106

Merge remote-tracking branch 'upstream/dev' into fcm

049c425

use http const in messaging test

a0ba55c

add client version header for stats

c40516b

init integration test

c91fb40

add integration test (validated on IOS today)

d312b7c

add comment with URL to enable Firebase Cloud Messaging API

0e83458

fix broken test

94cd35e

add integration tests

6d76105

Merge branch 'dev' into fcm

4e24472

hiranya911 self-requested a review January 2, 2018 19:46

hiranya911 self-assigned this Jan 2, 2018

hiranya911 reviewed Jan 2, 2018

View reviewed changes

chemidy added 6 commits January 2, 2018 21:35

accept a Message instead of RequestMessage + and rename method + send…

aee449c

… / sendDryRun

Merge branch 'dev' into fcm

4b37eee

Merge branch 'dev' into fcm

642de9f

Merge remote-tracking branch 'upstream/dev' into fcm

3c6dbcf

update fcm url

746bfb7

rollback url endpoint

8a58a72

hiranya911 closed this Jan 11, 2018

hiranya911 reopened this Jan 17, 2018

hiranya911 reviewed Jan 18, 2018

View reviewed changes

Cyrille Hemidy added 4 commits January 18, 2018 13:11

Merge remote-tracking branch 'upstream/dev' into fcm

78303e6

fix http constants, change responseMessage visibility, change map[str…

c72d90f

…ing]interface{} as map[string]string

fix http constants

7091e4b

fix integration tests

06195be

hiranya911 reviewed Jan 22, 2018

View reviewed changes

chemidy added 2 commits January 25, 2018 23:16

fix APNS naming

8b2f6bf

Merge branch 'dev' into fcm

cb9e342

chemidy added 2 commits January 30, 2018 22:29

Merge branch 'dev' into fcm

0802b84

add validators

24bdea0

hiranya911 changed the base branch from dev to fcm-api January 31, 2018 01:09

hiranya911 approved these changes Jan 31, 2018

View reviewed changes

hiranya911 merged commit 91dbe61 into firebase:fcm-api Jan 31, 2018

hiranya911 mentioned this pull request Feb 1, 2018

Firebase Cloud Messaging API #81

Merged

chemidy deleted the fcm branch February 17, 2018 23:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Firebase Cloud Messaging #62

Adding Firebase Cloud Messaging #62

chemidy commented Jan 2, 2018

hiranya911 commented Jan 2, 2018

hiranya911 Jan 2, 2018

chemidy Jan 2, 2018

hiranya911 commented Jan 17, 2018

hiranya911 Jan 17, 2018

hiranya911 Jan 17, 2018

hiranya911 Jan 18, 2018

hiranya911 Jan 18, 2018

hiranya911 Jan 18, 2018

hiranya911 Jan 18, 2018

hiranya911 Jan 18, 2018

hiranya911 Jan 18, 2018

hiranya911 Jan 18, 2018

hiranya911 Jan 18, 2018

hiranya911 left a comment

hiranya911 Jan 22, 2018

hiranya911 commented Jan 26, 2018

chemidy commented Jan 26, 2018

hiranya911 commented Jan 26, 2018

hiranya911 commented Jan 30, 2018

chemidy commented Jan 30, 2018

hiranya911 left a comment


		// validators

		// TODO add validator : Data messages can have a 4KB maximum payload.

Adding Firebase Cloud Messaging #62

Adding Firebase Cloud Messaging #62

Conversation

chemidy commented Jan 2, 2018

hiranya911 commented Jan 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hiranya911 commented Jan 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hiranya911 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hiranya911 commented Jan 26, 2018

chemidy commented Jan 26, 2018

hiranya911 commented Jan 26, 2018

hiranya911 commented Jan 30, 2018

chemidy commented Jan 30, 2018

hiranya911 left a comment

Choose a reason for hiding this comment