From 21e5041b3d7e52959a7434592c08da4306848c98 Mon Sep 17 00:00:00 2001 From: Benny Wong Date: Wed, 10 Dec 2014 23:44:53 -0500 Subject: [PATCH 1/3] Start performing a lot more error checking in Send() --- client.go | 72 ++++++++++++++++++++++++++++++++----------------- notification.go | 8 ++++++ 2 files changed, 55 insertions(+), 25 deletions(-) diff --git a/client.go b/client.go index de7ab1a..b9a31f1 100644 --- a/client.go +++ b/client.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "io" "log" + "sync" "time" ) @@ -27,20 +28,28 @@ func (b *buffer) Add(v interface{}) *list.Element { return e } +type serializedNotif struct { + id uint32 + b []byte +} + type Client struct { Conn *Conn FailedNotifs chan NotificationResult - notifs chan Notification - id uint32 + notifs chan serializedNotif + + id uint32 + idm sync.Mutex } func newClientWithConn(gw string, conn Conn) Client { c := Client{ Conn: &conn, FailedNotifs: make(chan NotificationResult), - id: uint32(1), - notifs: make(chan Notification), + notifs: make(chan serializedNotif), + id: 1, + idm: sync.Mutex{}, } go c.runLoop() @@ -73,10 +82,37 @@ func NewClientWithFiles(gw string, certFile string, keyFile string) (Client, err } func (c *Client) Send(n Notification) error { - c.notifs <- n + // Set identifier if not specified + if n.Identifier == 0 { + n.Identifier = c.nextID() + } else if c.id < n.Identifier { + c.setID(n.Identifier) + } + + b, err := n.ToBinary() + if err != nil { + return err + } + + c.notifs <- serializedNotif{b: b, id: n.Identifier} return nil } +func (c *Client) setID(n uint32) { + c.idm.Lock() + defer c.idm.Unlock() + + c.id = n +} + +func (c *Client) nextID() uint32 { + c.idm.Lock() + defer c.idm.Unlock() + + c.id++ + return c.id +} + func (c *Client) reportFailedPush(v interface{}, err *Error) { failedNotif, ok := v.(Notification) if !ok || v == nil { @@ -93,7 +129,7 @@ func (c *Client) requeue(cursor *list.Element) { // If `cursor` is not nil, this means there are notifications that // need to be delivered (or redelivered) for ; cursor != nil; cursor = cursor.Next() { - if n, ok := cursor.Value.(Notification); ok { + if n, ok := cursor.Value.(serializedNotif); ok { go func() { c.notifs <- n }() } } @@ -103,11 +139,11 @@ func (c *Client) handleError(err *Error, buffer *buffer) *list.Element { cursor := buffer.Back() for cursor != nil { - // Get notification - n, _ := cursor.Value.(Notification) + // Get serialized notification + n, _ := cursor.Value.(serializedNotif) // If the notification, move cursor after the trouble notification - if n.Identifier == err.Identifier { + if n.id == err.Identifier { go c.reportFailedPush(cursor.Value, err) next := cursor.Next() @@ -143,7 +179,7 @@ func (c *Client) runLoop() { // Connection open, listen for notifs and errors for { var err error - var n Notification + var n serializedNotif // Check for notifications or errors. There is a chance we'll send notifications // if we already have an error since `select` will "pseudorandomly" choose a @@ -169,21 +205,7 @@ func (c *Client) runLoop() { // Add to list cursor = sent.Add(n) - // Set identifier if not specified - if n.Identifier == 0 { - n.Identifier = c.id - c.id++ - } else if c.id < n.Identifier { - c.id = n.Identifier + 1 - } - - b, err := n.ToBinary() - if err != nil { - // TODO - continue - } - - _, err = c.Conn.Write(b) + _, err = c.Conn.Write(n.b) if err == io.EOF { log.Println("EOF trying to write notification") diff --git a/notification.go b/notification.go index b7d0b64..d99a1be 100644 --- a/notification.go +++ b/notification.go @@ -15,6 +15,10 @@ const ( PriorityPowerConserve = 5 ) +const ( + validDeviceTokenLength = 64 +) + const ( commandID = 2 @@ -93,6 +97,10 @@ func (p *Payload) MarshalJSON() ([]byte, error) { func (n Notification) ToBinary() ([]byte, error) { b := []byte{} + if len(n.DeviceToken) != validDeviceTokenLength { + return b, errors.New(ErrInvalidToken) + } + binTok, err := hex.DecodeString(n.DeviceToken) if err != nil { return b, fmt.Errorf("convert token to hex error: %s", err) From 507ece93b9b2941d717cf652e5e6616974ba3b98 Mon Sep 17 00:00:00 2001 From: Benny Wong Date: Wed, 10 Dec 2014 23:58:05 -0500 Subject: [PATCH 2/3] Pass the original notification through if we want it with NotificationResult --- client.go | 14 +++++--------- notification.go | 2 +- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/client.go b/client.go index b9a31f1..fdc62ae 100644 --- a/client.go +++ b/client.go @@ -31,6 +31,7 @@ func (b *buffer) Add(v interface{}) *list.Element { type serializedNotif struct { id uint32 b []byte + n *Notification } type Client struct { @@ -94,7 +95,7 @@ func (c *Client) Send(n Notification) error { return err } - c.notifs <- serializedNotif{b: b, id: n.Identifier} + c.notifs <- serializedNotif{b: b, id: n.Identifier, n: &n} return nil } @@ -113,14 +114,9 @@ func (c *Client) nextID() uint32 { return c.id } -func (c *Client) reportFailedPush(v interface{}, err *Error) { - failedNotif, ok := v.(Notification) - if !ok || v == nil { - return - } - +func (c *Client) reportFailedPush(s serializedNotif, err *Error) { select { - case c.FailedNotifs <- NotificationResult{Notif: failedNotif, Err: *err}: + case c.FailedNotifs <- NotificationResult{Notif: s.n, Err: *err}: default: } } @@ -144,7 +140,7 @@ func (c *Client) handleError(err *Error, buffer *buffer) *list.Element { // If the notification, move cursor after the trouble notification if n.id == err.Identifier { - go c.reportFailedPush(cursor.Value, err) + go c.reportFailedPush(n, err) next := cursor.Next() diff --git a/notification.go b/notification.go index d99a1be..5995392 100644 --- a/notification.go +++ b/notification.go @@ -37,7 +37,7 @@ const ( ) type NotificationResult struct { - Notif Notification + Notif *Notification Err Error } From 6c748c9b0b72f719ee6885e9106371e83f1b656d Mon Sep 17 00:00:00 2001 From: Benny Wong Date: Thu, 11 Dec 2014 19:07:49 -0500 Subject: [PATCH 3/3] Undo NotificationResult api change --- client.go | 2 +- notification.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index fdc62ae..2d27b5d 100644 --- a/client.go +++ b/client.go @@ -116,7 +116,7 @@ func (c *Client) nextID() uint32 { func (c *Client) reportFailedPush(s serializedNotif, err *Error) { select { - case c.FailedNotifs <- NotificationResult{Notif: s.n, Err: *err}: + case c.FailedNotifs <- NotificationResult{Notif: *s.n, Err: *err}: default: } } diff --git a/notification.go b/notification.go index 5995392..d99a1be 100644 --- a/notification.go +++ b/notification.go @@ -37,7 +37,7 @@ const ( ) type NotificationResult struct { - Notif *Notification + Notif Notification Err Error }