From 65f671c96c5c1d1e02600763f6ac02cd2494af6e Mon Sep 17 00:00:00 2001 From: Andre Medeiros Date: Sat, 10 Jul 2021 14:51:42 -0400 Subject: [PATCH 1/5] Fix notification payloads --- cmd/apollo-worker-notifications/main.go | 49 ++++++++++++++++++++++++- internal/reddit/client.go | 8 ++++ internal/reddit/types.go | 4 ++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/cmd/apollo-worker-notifications/main.go b/cmd/apollo-worker-notifications/main.go index aab4550..9093632 100644 --- a/cmd/apollo-worker-notifications/main.go +++ b/cmd/apollo-worker-notifications/main.go @@ -370,7 +370,7 @@ func (c *Consumer) Consume(delivery rmq.Delivery) { for _, msg := range msgs.MessageListing.Messages { notification := &apns2.Notification{} notification.Topic = "com.christianselig.Apollo" - notification.Payload = payload.NewPayload().AlertTitle(msg.Subject).AlertBody(msg.Body) + notification.Payload = payloadFromMessage(&msg) for _, device := range devices { notification.DeviceToken = device.APNSToken @@ -404,6 +404,53 @@ func (c *Consumer) Consume(delivery rmq.Delivery) { }).Debug("finishing job") } +func payloadFromMessage(msg *reddit.MessageData) *payload.Payload { + postBody := msg.Body + if len(postBody) > 2000 { + postBody = msg.Body[:2000] + } + + postTitle := msg.LinkTitle + if postTitle == "" { + postTitle = msg.Subject + } + if len(postTitle) > 75 { + postTitle = fmt.Sprintf("%s…", postTitle[0:75]) + } + + payload := payload.NewPayload().Sound("traloop.wav").AlertBody(postBody).Custom("author", msg.Author).Custom("parent_id", msg.ParentID).AlertSummaryArg(msg.Author).MutableContent() + + switch { + case (msg.Kind == "t1" && msg.Type == "username_mention"): + title := fmt.Sprintf(`Mention in “%s”`, postTitle) + payload = payload.AlertTitle(title).Custom("type", "username") + + pType, _ := reddit.SplitID(msg.ParentID) + if pType == "t1" { + payload = payload.Category("inbox-username-mention-context") + } else { + payload = payload.Category("inbox-username-mention-no-context") + } + + payload = payload.Custom("subject", "comment").ThreadID("comment") + break + case (msg.Kind == "t1" && msg.Type == "post_reply"): + title := fmt.Sprintf(`%s to “%s”`, msg.Author, postTitle) + payload = payload.AlertTitle(title).Custom("type", "post").Category("inbox-post-reply").Custom("subject", "comment").ThreadID("comment") + break + case (msg.Kind == "t1" && msg.Type == "comment_reply"): + title := fmt.Sprintf(`%s in “%s”`, msg.Author, postTitle) + payload = payload.AlertTitle(title).Custom("type", "comment").Category("inbox-comment-reply").Custom("subject", "comment").ThreadID("comment") + break + case (msg.Kind == "t4"): + title := fmt.Sprintf(`Message from %s`, msg.Author) + payload = payload.AlertTitle(title).AlertSubtitle(postTitle).Custom("type", "private-message").Category("inbox-private-message") + break + } + + return payload +} + func logErrors(errChan <-chan error) { for err := range errChan { log.Print("error: ", err) diff --git a/internal/reddit/client.go b/internal/reddit/client.go index 34a7025..8ed2c07 100644 --- a/internal/reddit/client.go +++ b/internal/reddit/client.go @@ -25,6 +25,14 @@ type Client struct { statsd *statsd.Client } +func SplitID(id string) (string, string) { + if parts := strings.Split(id, "_"); len(parts) == 2 { + return parts[0], parts[1] + } + + return "", "" +} + func NewClient(id, secret string, statsd *statsd.Client) *Client { tracer := &httptrace.ClientTrace{ GotConn: func(info httptrace.GotConnInfo) { diff --git a/internal/reddit/types.go b/internal/reddit/types.go index 7165ff7..2dda199 100644 --- a/internal/reddit/types.go +++ b/internal/reddit/types.go @@ -5,10 +5,14 @@ import "fmt" type Message struct { ID string `json:"id"` Kind string `json:"kind"` + Type string `json:"type"` Author string `json:"author"` Subject string `json:"subject"` Body string `json:"body"` CreatedAt float64 `json:"created_utc"` + Context string `json:"context"` + ParentID string `json:"parent_id"` + LinkTitle string `json:"link_title"` } type MessageData struct { From fad7191035a05480b2f7ce4c95667cf8b3542474 Mon Sep 17 00:00:00 2001 From: Andre Medeiros Date: Mon, 12 Jul 2021 14:15:13 -0400 Subject: [PATCH 2/5] add most of the missing fields --- cmd/apollo-worker-notifications/main.go | 11 ++++++----- internal/reddit/types.go | 22 ++++++++++++---------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/cmd/apollo-worker-notifications/main.go b/cmd/apollo-worker-notifications/main.go index 9093632..54f01ae 100644 --- a/cmd/apollo-worker-notifications/main.go +++ b/cmd/apollo-worker-notifications/main.go @@ -370,7 +370,7 @@ func (c *Consumer) Consume(delivery rmq.Delivery) { for _, msg := range msgs.MessageListing.Messages { notification := &apns2.Notification{} notification.Topic = "com.christianselig.Apollo" - notification.Payload = payloadFromMessage(&msg) + notification.Payload = payloadFromMessage(account, &msg, len(msgs.MessageListing.Messages)) for _, device := range devices { notification.DeviceToken = device.APNSToken @@ -404,7 +404,7 @@ func (c *Consumer) Consume(delivery rmq.Delivery) { }).Debug("finishing job") } -func payloadFromMessage(msg *reddit.MessageData) *payload.Payload { +func payloadFromMessage(acct *data.Account, msg *reddit.MessageData, badgeCount int) *payload.Payload { postBody := msg.Body if len(postBody) > 2000 { postBody = msg.Body[:2000] @@ -418,7 +418,7 @@ func payloadFromMessage(msg *reddit.MessageData) *payload.Payload { postTitle = fmt.Sprintf("%s…", postTitle[0:75]) } - payload := payload.NewPayload().Sound("traloop.wav").AlertBody(postBody).Custom("author", msg.Author).Custom("parent_id", msg.ParentID).AlertSummaryArg(msg.Author).MutableContent() + payload := payload.NewPayload().Sound("traloop.wav").AlertBody(postBody).Custom("author", msg.Author).Custom("parent_id", msg.ParentID).AlertSummaryArg(msg.Author).MutableContent().Badge(badgeCount).Custom("post_title", msg.LinkTitle).Custom("destination_author", msg.Destination).Custom("subreddit", msg.Subreddit) switch { case (msg.Kind == "t1" && msg.Type == "username_mention"): @@ -436,11 +436,12 @@ func payloadFromMessage(msg *reddit.MessageData) *payload.Payload { break case (msg.Kind == "t1" && msg.Type == "post_reply"): title := fmt.Sprintf(`%s to “%s”`, msg.Author, postTitle) - payload = payload.AlertTitle(title).Custom("type", "post").Category("inbox-post-reply").Custom("subject", "comment").ThreadID("comment") + payload = payload.AlertTitle(title).Custom("type", "post").Category("inbox-post-reply").Custom("subject", "comment").ThreadID("comment").Custom("post_id", msg.ID) break case (msg.Kind == "t1" && msg.Type == "comment_reply"): title := fmt.Sprintf(`%s in “%s”`, msg.Author, postTitle) - payload = payload.AlertTitle(title).Custom("type", "comment").Category("inbox-comment-reply").Custom("subject", "comment").ThreadID("comment") + _, postID := reddit.SplitID(msg.ParentID) + payload = payload.AlertTitle(title).Custom("type", "comment").Category("inbox-comment-reply").Custom("subject", "comment").ThreadID("comment").Custom("post_id", postID).Custom("comment_id", msg.ID) break case (msg.Kind == "t4"): title := fmt.Sprintf(`Message from %s`, msg.Author) diff --git a/internal/reddit/types.go b/internal/reddit/types.go index 2dda199..eba8247 100644 --- a/internal/reddit/types.go +++ b/internal/reddit/types.go @@ -3,16 +3,18 @@ package reddit import "fmt" type Message struct { - ID string `json:"id"` - Kind string `json:"kind"` - Type string `json:"type"` - Author string `json:"author"` - Subject string `json:"subject"` - Body string `json:"body"` - CreatedAt float64 `json:"created_utc"` - Context string `json:"context"` - ParentID string `json:"parent_id"` - LinkTitle string `json:"link_title"` + ID string `json:"id"` + Kind string `json:"kind"` + Type string `json:"type"` + Author string `json:"author"` + Subject string `json:"subject"` + Body string `json:"body"` + CreatedAt float64 `json:"created_utc"` + Context string `json:"context"` + ParentID string `json:"parent_id"` + LinkTitle string `json:"link_title"` + Destination string `json:"dest"` + Subreddit string `json:"subreddit"` } type MessageData struct { From a431c4ff5b563bac58db2eb27534c3f16e8e956a Mon Sep 17 00:00:00 2001 From: Andre Medeiros Date: Mon, 12 Jul 2021 14:36:08 -0400 Subject: [PATCH 3/5] better error handling --- cmd/apollo-worker-notifications/main.go | 39 ++++++++++++++++++++++--- internal/data/accounts.go | 1 + internal/reddit/client.go | 19 +++++++++++- internal/reddit/types.go | 9 ++++++ 4 files changed, 63 insertions(+), 5 deletions(-) diff --git a/cmd/apollo-worker-notifications/main.go b/cmd/apollo-worker-notifications/main.go index 54f01ae..f987881 100644 --- a/cmd/apollo-worker-notifications/main.go +++ b/cmd/apollo-worker-notifications/main.go @@ -222,6 +222,7 @@ func (c *Consumer) Consume(delivery rmq.Delivery) { stmt := `SELECT id, username, + account_id, access_token, refresh_token, expires_at, @@ -233,6 +234,7 @@ func (c *Consumer) Consume(delivery rmq.Delivery) { if err := c.pool.QueryRow(ctx, stmt, id).Scan( &account.ID, &account.Username, + &account.AccountID, &account.AccessToken, &account.RefreshToken, &account.ExpiresAt, @@ -418,7 +420,19 @@ func payloadFromMessage(acct *data.Account, msg *reddit.MessageData, badgeCount postTitle = fmt.Sprintf("%s…", postTitle[0:75]) } - payload := payload.NewPayload().Sound("traloop.wav").AlertBody(postBody).Custom("author", msg.Author).Custom("parent_id", msg.ParentID).AlertSummaryArg(msg.Author).MutableContent().Badge(badgeCount).Custom("post_title", msg.LinkTitle).Custom("destination_author", msg.Destination).Custom("subreddit", msg.Subreddit) + payload := payload. + NewPayload(). + AlertBody(postBody). + AlertSummaryArg(msg.Author). + Badge(badgeCount). + Custom("account_id", acct.AccountID). + Custom("author", msg.Author). + Custom("destination_author", msg.Destination). + Custom("parent_id", msg.ParentID). + Custom("post_title", msg.LinkTitle). + Custom("subreddit", msg.Subreddit). + MutableContent(). + Sound("traloop.wav") switch { case (msg.Kind == "t1" && msg.Type == "username_mention"): @@ -436,16 +450,33 @@ func payloadFromMessage(acct *data.Account, msg *reddit.MessageData, badgeCount break case (msg.Kind == "t1" && msg.Type == "post_reply"): title := fmt.Sprintf(`%s to “%s”`, msg.Author, postTitle) - payload = payload.AlertTitle(title).Custom("type", "post").Category("inbox-post-reply").Custom("subject", "comment").ThreadID("comment").Custom("post_id", msg.ID) + payload = payload. + AlertTitle(title). + Category("inbox-post-reply"). + Custom("post_id", msg.ID). + Custom("subject", "comment"). + Custom("type", "post"). + ThreadID("comment") break case (msg.Kind == "t1" && msg.Type == "comment_reply"): title := fmt.Sprintf(`%s in “%s”`, msg.Author, postTitle) _, postID := reddit.SplitID(msg.ParentID) - payload = payload.AlertTitle(title).Custom("type", "comment").Category("inbox-comment-reply").Custom("subject", "comment").ThreadID("comment").Custom("post_id", postID).Custom("comment_id", msg.ID) + payload = payload. + AlertTitle(title). + Category("inbox-comment-reply"). + Custom("comment_id", msg.ID). + Custom("post_id", postID). + Custom("subject", "comment"). + Custom("type", "comment"). + ThreadID("comment") break case (msg.Kind == "t4"): title := fmt.Sprintf(`Message from %s`, msg.Author) - payload = payload.AlertTitle(title).AlertSubtitle(postTitle).Custom("type", "private-message").Category("inbox-private-message") + payload = payload. + AlertTitle(title). + AlertSubtitle(postTitle). + Category("inbox-private-message"). + Custom("type", "private-message") break } diff --git a/internal/data/accounts.go b/internal/data/accounts.go index f110254..ea7ba04 100644 --- a/internal/data/accounts.go +++ b/internal/data/accounts.go @@ -8,6 +8,7 @@ import ( type Account struct { ID int64 Username string + AccountID string AccessToken string RefreshToken string ExpiresAt int64 diff --git a/internal/reddit/client.go b/internal/reddit/client.go index 8ed2c07..02cc3b4 100644 --- a/internal/reddit/client.go +++ b/internal/reddit/client.go @@ -2,6 +2,7 @@ package reddit import ( "encoding/json" + "fmt" "io/ioutil" "net/http" "net/http/httptrace" @@ -93,7 +94,23 @@ func (rac *AuthenticatedClient) request(r *Request) ([]byte, error) { } defer resp.Body.Close() - return ioutil.ReadAll(resp.Body) + bb, err := ioutil.ReadAll(resp.Body) + if err != nil { + rac.statsd.Incr("reddit.api.errors", r.tags, 0.1) + return nil, err + } + + if resp.StatusCode != 200 { + rac.statsd.Incr("reddit.api.errors", r.tags, 0.1) + + // Try to parse a json error. Otherwise we generate a generic one + rerr := &Error{} + if jerr := json.Unmarshal(bb, rerr); jerr != nil { + return nil, fmt.Errorf("error from reddit: %d", resp.StatusCode) + } + return nil, rerr + } + return bb, nil } func (rac *AuthenticatedClient) RefreshTokens() (*RefreshTokenResponse, error) { diff --git a/internal/reddit/types.go b/internal/reddit/types.go index eba8247..d87eaa2 100644 --- a/internal/reddit/types.go +++ b/internal/reddit/types.go @@ -2,6 +2,15 @@ package reddit import "fmt" +type Error struct { + Message string `json:"message"` + Code int `json:"error"` +} + +func (err *Error) Error() string { + return fmt.Sprintf("%s (%d)", err.Message, err.Code) +} + type Message struct { ID string `json:"id"` Kind string `json:"kind"` From 7a89d5503cdce561d0a401e07308c563dccef30c Mon Sep 17 00:00:00 2001 From: Andre Medeiros Date: Mon, 12 Jul 2021 14:46:28 -0400 Subject: [PATCH 4/5] fix up API to include account ID and refresh token --- cmd/apollo-api/accounts.go | 48 +++++++++++++++++++++++++++++--------- cmd/apollo-api/main.go | 16 +++++++++++-- internal/data/accounts.go | 4 ++-- internal/reddit/client.go | 8 ------- internal/reddit/types.go | 14 ++++++++++- 5 files changed, 66 insertions(+), 24 deletions(-) diff --git a/cmd/apollo-api/accounts.go b/cmd/apollo-api/accounts.go index 29fa20c..f727cdc 100644 --- a/cmd/apollo-api/accounts.go +++ b/cmd/apollo-api/accounts.go @@ -2,11 +2,11 @@ package main import ( "encoding/json" - "fmt" "net/http" "time" "github.com/julienschmidt/httprouter" + "github.com/sirupsen/logrus" "github.com/christianselig/apollo-backend/internal/data" ) @@ -14,32 +14,54 @@ import ( func (app *application) upsertAccountHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { a := &data.Account{} if err := json.NewDecoder(r.Body).Decode(a); err != nil { - fmt.Println("failing on decoding json") + app.logger.WithFields(logrus.Fields{ + "err": err, + }).Info("failed to parse request json") + app.errorResponse(w, r, 422, err.Error()) + return + } + + // Here we check whether the account is supplied with a valid token. + ac := app.client.NewAuthenticatedClient(a.RefreshToken, a.AccessToken) + tokens, err := ac.RefreshTokens() + if err != nil { + app.logger.WithFields(logrus.Fields{ + "err": err, + }).Info("failed to refresh token") app.errorResponse(w, r, 500, err.Error()) return } - a.ExpiresAt = time.Now().Unix() + 3300 + // Reset expiration timer + a.ExpiresAt = time.Now().Unix() + 3540 - // Here we check whether the account is supplied with a valid token. - ac := app.client.NewAuthenticatedClient(a.RefreshToken, a.AccessToken) + ac = app.client.NewAuthenticatedClient(tokens.RefreshToken, tokens.AccessToken) me, err := ac.Me() if err != nil { - fmt.Println("failing on fetching remote user") + app.logger.WithFields(logrus.Fields{ + "err": err, + }).Info("failed to grab user details") app.errorResponse(w, r, 500, err.Error()) return } if me.NormalizedUsername() != a.NormalizedUsername() { - fmt.Println("failing on account username comparison") - app.errorResponse(w, r, 500, "nice try") + app.logger.WithFields(logrus.Fields{ + "err": err, + }).Info("user is not who they say they are") + app.errorResponse(w, r, 422, "nice try") return } + // Set account ID from Reddit + a.AccountID = me.ID + // Upsert account if err := app.models.Accounts.Upsert(a); err != nil { - fmt.Println("failing on account upsert") + app.logger.WithFields(logrus.Fields{ + "err": err, + }).Info("failed updating account in database") app.errorResponse(w, r, 500, err.Error()) return } @@ -47,13 +69,17 @@ func (app *application) upsertAccountHandler(w http.ResponseWriter, r *http.Requ // Associate d, err := app.models.Devices.GetByAPNSToken(ps.ByName("apns")) if err != nil { - fmt.Println("failing on apns") + app.logger.WithFields(logrus.Fields{ + "err": err, + }).Info("failed fetching account devices") app.errorResponse(w, r, 500, err.Error()) return } if err := app.models.DevicesAccounts.Associate(a.ID, d.ID); err != nil { - fmt.Println("failing on associate") + app.logger.WithFields(logrus.Fields{ + "err": err, + }).Info("failed associating account with device") app.errorResponse(w, r, 500, err.Error()) return } diff --git a/cmd/apollo-api/main.go b/cmd/apollo-api/main.go index 72b1909..af343b7 100644 --- a/cmd/apollo-api/main.go +++ b/cmd/apollo-api/main.go @@ -10,6 +10,7 @@ import ( "github.com/DataDog/datadog-go/statsd" "github.com/joho/godotenv" _ "github.com/lib/pq" + "github.com/sirupsen/logrus" "github.com/christianselig/apollo-backend/internal/data" "github.com/christianselig/apollo-backend/internal/reddit" @@ -21,14 +22,25 @@ type config struct { type application struct { cfg config - logger *log.Logger + logger *logrus.Logger db *sql.DB models *data.Models client *reddit.Client } func main() { - logger := log.New(os.Stdout, "", log.Ldate|log.Ltime) + var logger *logrus.Logger + { + logger = logrus.New() + if os.Getenv("ENV") == "" { + logger.SetLevel(logrus.DebugLevel) + } else { + logger.SetFormatter(&logrus.TextFormatter{ + DisableColors: true, + FullTimestamp: true, + }) + } + } if err := godotenv.Load(); err != nil { logger.Printf("Couldn't find .env so I will read from existing ENV.") diff --git a/internal/data/accounts.go b/internal/data/accounts.go index ea7ba04..ea15b51 100644 --- a/internal/data/accounts.go +++ b/internal/data/accounts.go @@ -26,14 +26,14 @@ type AccountModel struct { func (am *AccountModel) Upsert(a *Account) error { query := ` - INSERT INTO accounts (username, access_token, refresh_token, expires_at, last_message_id, device_count, last_checked_at) + INSERT INTO accounts (username, account_id, access_token, refresh_token, expires_at, last_message_id, device_count, last_checked_at) VALUES ($1, $2, $3, $4, '', 0, 0) ON CONFLICT(username) DO UPDATE SET access_token = $2, refresh_token = $3, expires_at = $4, last_message_id = $5, last_checked_at = $6 RETURNING id` - args := []interface{}{a.NormalizedUsername(), a.AccessToken, a.RefreshToken, a.ExpiresAt, a.LastMessageID, a.LastCheckedAt} + args := []interface{}{a.NormalizedUsername(), a.AccountID, a.AccessToken, a.RefreshToken, a.ExpiresAt, a.LastMessageID, a.LastCheckedAt} return am.DB.QueryRow(query, args...).Scan(&a.ID) } diff --git a/internal/reddit/client.go b/internal/reddit/client.go index 02cc3b4..4b7c1c8 100644 --- a/internal/reddit/client.go +++ b/internal/reddit/client.go @@ -154,14 +154,6 @@ func (rac *AuthenticatedClient) MessageInbox(from string) (*MessageListingRespon return mlr, nil } -type MeResponse struct { - Name string -} - -func (mr *MeResponse) NormalizedUsername() string { - return strings.ToLower(mr.Name) -} - func (rac *AuthenticatedClient) Me() (*MeResponse, error) { req := NewRequest( WithTags([]string{"url:/api/v1/me"}), diff --git a/internal/reddit/types.go b/internal/reddit/types.go index d87eaa2..e2754a7 100644 --- a/internal/reddit/types.go +++ b/internal/reddit/types.go @@ -1,6 +1,9 @@ package reddit -import "fmt" +import ( + "fmt" + "strings" +) type Error struct { Message string `json:"message"` @@ -47,3 +50,12 @@ type RefreshTokenResponse struct { AccessToken string `json:"access_token"` RefreshToken string `json:"refresh_token"` } + +type MeResponse struct { + ID string `json:"id"` + Name string +} + +func (mr *MeResponse) NormalizedUsername() string { + return strings.ToLower(mr.Name) +} From bc5acffcbefe4415a7e4230bfe7f45e52ae58598 Mon Sep 17 00:00:00 2001 From: Andre Medeiros Date: Mon, 12 Jul 2021 14:47:22 -0400 Subject: [PATCH 5/5] token refresh failure is unprocessable and error is not ours --- cmd/apollo-api/accounts.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/apollo-api/accounts.go b/cmd/apollo-api/accounts.go index f727cdc..e7bbec2 100644 --- a/cmd/apollo-api/accounts.go +++ b/cmd/apollo-api/accounts.go @@ -28,7 +28,7 @@ func (app *application) upsertAccountHandler(w http.ResponseWriter, r *http.Requ app.logger.WithFields(logrus.Fields{ "err": err, }).Info("failed to refresh token") - app.errorResponse(w, r, 500, err.Error()) + app.errorResponse(w, r, 422, err.Error()) return }