From 8367902e606cf079363f059d92c79f4533d17553 Mon Sep 17 00:00:00 2001 From: Eric Wollesen Date: Mon, 6 May 2024 09:53:25 -0600 Subject: [PATCH 1/2] adds List and Get methods to alerts client The Get endpoint already exists on the service, so only the List endpoint needed to be added there. BACK-2554 --- alerts/client.go | 39 +++++++++++++++++++++++++----- alerts/config.go | 1 + data/service/api/v1/alerts.go | 37 ++++++++++++++++++++++++++++ data/service/api/v1/alerts_test.go | 21 +++++++++++++--- data/store/mongo/mongo_alerts.go | 21 ++++++++++++++++ 5 files changed, 110 insertions(+), 9 deletions(-) diff --git a/alerts/client.go b/alerts/client.go index bc6db1f888..9abaaae9f3 100644 --- a/alerts/client.go +++ b/alerts/client.go @@ -9,6 +9,7 @@ import ( "github.com/tidepool-org/platform/auth" "github.com/tidepool-org/platform/client" + "github.com/tidepool-org/platform/errors" platformlog "github.com/tidepool-org/platform/log" "github.com/tidepool-org/platform/log/null" "github.com/tidepool-org/platform/platform" @@ -43,6 +44,8 @@ type PlatformClient interface { requestBody interface{}, responseBody interface{}, inspectors ...request.ResponseInspector) error } +// TokenProvider retrieves session tokens for calling the alerts API. +// // client.External is one implementation type TokenProvider interface { // ServerSessionToken provides a server-to-server API authentication token. @@ -51,12 +54,12 @@ type TokenProvider interface { // request performs common operations before passing a request off to the // underlying platform.Client. -func (c *Client) request(ctx context.Context, method, url string, body any) error { +func (c *Client) request(ctx context.Context, method, url string, reqBody, resBody any) error { // Platform's client.Client expects a logger to exist in the request's // context. If it doesn't exist, request processing will panic. loggingCtx := platformlog.NewContextWithLogger(ctx, c.logger) // Make sure the auth token is injected into the request's headers. - return c.requestWithAuth(loggingCtx, method, url, body) + return c.requestWithAuth(loggingCtx, method, url, reqBody, resBody) } // requestWithAuth injects an auth token before calling platform.Client.RequestData. @@ -65,24 +68,48 @@ func (c *Client) request(ctx context.Context, method, url string, body any) erro // platform.Client. It might be nice to be able to use a mutator, but the auth // is specifically handled by the platform.Client via the context field, and // if left blank, platform.Client errors. -func (c *Client) requestWithAuth(ctx context.Context, method, url string, body any) error { +func (c *Client) requestWithAuth(ctx context.Context, method, url string, reqBody, resBody any) error { authCtx, err := c.ctxWithAuth(ctx) if err != nil { return err } - return c.client.RequestData(authCtx, method, url, nil, body, nil) + return c.client.RequestData(authCtx, method, url, nil, reqBody, resBody) } // Upsert updates cfg if it exists or creates it if it doesn't. func (c *Client) Upsert(ctx context.Context, cfg *Config) error { url := c.client.ConstructURL("v1", "users", cfg.FollowedUserID, "followers", cfg.UserID, "alerts") - return c.request(ctx, http.MethodPost, url, cfg) + return c.request(ctx, http.MethodPost, url, cfg, nil) } // Delete the alerts config. func (c *Client) Delete(ctx context.Context, cfg *Config) error { url := c.client.ConstructURL("v1", "users", cfg.FollowedUserID, "followers", cfg.UserID, "alerts") - return c.request(ctx, http.MethodDelete, url, nil) + return c.request(ctx, http.MethodDelete, url, nil, nil) +} + +// Get a user's alerts configuration for the followed user. +func (c *Client) Get(ctx context.Context, followedUserID, userID string) (*Config, error) { + url := c.client.ConstructURL("v1", "users", followedUserID, "followers", userID, "alerts") + cfg := &Config{} + err := c.request(ctx, http.MethodGet, url, nil, cfg) + if err != nil { + return nil, errors.Wrap(err, "Unable to request alerts config") + } + return cfg, nil +} + +// List the alerts configurations that follow the given user. +// +// This method should only be called via an authenticated service session. +func (c *Client) List(ctx context.Context, followedUserID string) ([]*Config, error) { + url := c.client.ConstructURL("v1", "users", followedUserID, "followers", "alerts") + configs := []*Config{} + err := c.request(ctx, http.MethodGet, url, nil, &configs) + if err != nil { + return nil, errors.Wrap(err, "Unable to request alerts configs list") + } + return configs, nil } // ctxWithAuth injects a server session token into the context. diff --git a/alerts/config.go b/alerts/config.go index 67f2b1d72c..d9931b0f9a 100644 --- a/alerts/config.go +++ b/alerts/config.go @@ -239,6 +239,7 @@ type Repository interface { Get(ctx context.Context, conf *Config) (*Config, error) Upsert(ctx context.Context, conf *Config) error Delete(ctx context.Context, conf *Config) error + List(ctx context.Context, userID string) ([]*Config, error) EnsureIndexes() error } diff --git a/data/service/api/v1/alerts.go b/data/service/api/v1/alerts.go index d07891247e..70941b9e20 100644 --- a/data/service/api/v1/alerts.go +++ b/data/service/api/v1/alerts.go @@ -24,6 +24,7 @@ func AlertsRoutes() []service.Route { service.Get("/v1/users/:userId/followers/:followerUserId/alerts", GetAlert, api.RequireAuth), service.Post("/v1/users/:userId/followers/:followerUserId/alerts", UpsertAlert, api.RequireAuth), service.Delete("/v1/users/:userId/followers/:followerUserId/alerts", DeleteAlert, api.RequireAuth), + service.Get("/v1/users/:userId/followers/alerts", ListAlerts, api.RequireServer), } } @@ -134,6 +135,42 @@ func UpsertAlert(dCtx service.Context) { } } +func ListAlerts(dCtx service.Context) { + r := dCtx.Request() + ctx := r.Context() + authDetails := request.GetAuthDetails(ctx) + repo := dCtx.AlertsRepository() + lgr := log.LoggerFromContext(ctx) + + if err := checkAuthentication(authDetails); err != nil { + lgr.Debug("authentication failed") + dCtx.RespondWithError(platform.ErrorUnauthorized()) + return + } + + pathsUserID := r.PathParam("userId") + if err := checkUserIDConsistency(authDetails, pathsUserID); err != nil { + lgr.WithFields(log.Fields{"path": pathsUserID, "auth": authDetails.UserID()}). + Debug("user id consistency failed") + dCtx.RespondWithError(platform.ErrorUnauthorized()) + return + } + + alerts, err := repo.List(ctx, pathsUserID) + if err != nil { + dCtx.RespondWithInternalServerFailure("listing alerts configs", err) + lgr.WithError(err).Error("listing alerts config") + return + } + if len(alerts) == 0 { + dCtx.RespondWithError(ErrorUserIDNotFound(pathsUserID)) + lgr.Debug("no alerts configs found") + } + + responder := request.MustNewResponder(dCtx.Response(), r) + responder.Data(http.StatusOK, alerts) +} + // checkUserIDConsistency verifies the userIDs in a request. // // For safety reasons, if these values don't agree, return an error. diff --git a/data/service/api/v1/alerts_test.go b/data/service/api/v1/alerts_test.go index c3b4b2f2a5..d48be38a6f 100644 --- a/data/service/api/v1/alerts_test.go +++ b/data/service/api/v1/alerts_test.go @@ -160,12 +160,15 @@ var _ = Describe("Alerts endpoints", func() { }) type mockRepo struct { - UserID string - Error error + UserID string + Error error + AlertsForUserID map[string][]*alerts.Config } func newMockRepo() *mockRepo { - return &mockRepo{} + return &mockRepo{ + AlertsForUserID: make(map[string][]*alerts.Config), + } } func (r *mockRepo) ReturnsError(err error) { @@ -202,6 +205,18 @@ func (r *mockRepo) Delete(ctx context.Context, conf *alerts.Config) error { return nil } +func (r *mockRepo) List(ctx context.Context, userID string) ([]*alerts.Config, error) { + if r.Error != nil { + return nil, r.Error + } + r.UserID = userID + alerts, ok := r.AlertsForUserID[userID] + if !ok { + return nil, nil + } + return alerts, nil +} + func (r *mockRepo) EnsureIndexes() error { return nil } diff --git a/data/store/mongo/mongo_alerts.go b/data/store/mongo/mongo_alerts.go index ee313f3ffb..cbb49a1c5c 100644 --- a/data/store/mongo/mongo_alerts.go +++ b/data/store/mongo/mongo_alerts.go @@ -9,6 +9,7 @@ import ( "go.mongodb.org/mongo-driver/mongo/options" "github.com/tidepool-org/platform/alerts" + "github.com/tidepool-org/platform/errors" structuredmongo "github.com/tidepool-org/platform/store/structured/mongo" ) @@ -34,6 +35,26 @@ func (r *alertsRepo) Delete(ctx context.Context, cfg *alerts.Config) error { return nil } +// List will retrieve any Configs that are defined by followers of the given user. +func (r *alertsRepo) List(ctx context.Context, userID string) ([]*alerts.Config, error) { + filter := bson.D{ + {Key: "followedUserId", Value: userID}, + } + cursor, err := r.Find(ctx, filter, nil) + if err != nil { + return nil, errors.Wrapf(err, "Unable to list alerts.Config(s) for user %s", userID) + } + defer cursor.Close(ctx) + out := []*alerts.Config{} + if err := cursor.All(ctx, &out); err != nil { + return nil, errors.Wrapf(err, "Unable to decode alerts.Config(s) for user %s", userID) + } + if err := cursor.Err(); err != nil { + return nil, errors.Wrapf(err, "Unexpected error for user %s", userID) + } + return out, nil +} + // Get will retrieve the given Config. func (r *alertsRepo) Get(ctx context.Context, cfg *alerts.Config) (*alerts.Config, error) { res := r.FindOne(ctx, r.filter(cfg), nil) From 13aec209448f5dff5c181afc4c426152867c6db3 Mon Sep 17 00:00:00 2001 From: Eric Wollesen Date: Mon, 6 May 2024 10:22:02 -0600 Subject: [PATCH 2/2] lift Repeat out of the base alert config Through discussions it was confirmed that Repeat is not universal to all alerts. So it's lifted out of the Base alert and re-inserted into those alerts where it should be present (namely Low and High alerts only). BACK-2554 --- alerts/config.go | 28 ++++++----- alerts/config_test.go | 106 ++++++++++++++++++++++-------------------- 2 files changed, 72 insertions(+), 62 deletions(-) diff --git a/alerts/config.go b/alerts/config.go index d9931b0f9a..b83cf2b25f 100644 --- a/alerts/config.go +++ b/alerts/config.go @@ -72,16 +72,10 @@ func (a Alerts) Validate(validator structure.Validator) { type Base struct { // Enabled controls whether notifications should be sent for this alert. Enabled bool `json:"enabled" bson:"enabled"` - // Repeat is measured in minutes. - // - // A value of 0 (the default) disables repeat notifications. - Repeat DurationMinutes `json:"repeat,omitempty" bson:"repeat"` } func (b Base) Validate(validator structure.Validator) { validator.Bool("enabled", &b.Enabled) - dur := b.Repeat.Duration() - validator.Duration("repeat", &dur).Using(validateRepeat) } const ( @@ -110,7 +104,7 @@ type UrgentLowAlert struct { Base `bson:",inline"` // Threshold is compared the current value to determine if an alert should // be triggered. - Threshold `json:"threshold"` + Threshold `json:"threshold" bson:"threshold"` } func (a UrgentLowAlert) Validate(validator structure.Validator) { @@ -149,13 +143,19 @@ type LowAlert struct { // be triggered. Threshold `json:"threshold"` Delay DurationMinutes `json:"delay,omitempty"` + // Repeat is measured in minutes. + // + // A value of 0 (the default) disables repeat notifications. + Repeat DurationMinutes `json:"repeat,omitempty" bson:"repeat"` } func (a LowAlert) Validate(validator structure.Validator) { a.Base.Validate(validator) - dur := a.Delay.Duration() - validator.Duration("delay", &dur).InRange(0, 2*time.Hour) + delayDur := a.Delay.Duration() + validator.Duration("delay", &delayDur).InRange(0, 2*time.Hour) a.Threshold.Validate(validator) + repeatDur := a.Repeat.Duration() + validator.Duration("repeat", &repeatDur).Using(validateRepeat) } // HighAlert extends Base with a threshold and a delay. @@ -165,13 +165,19 @@ type HighAlert struct { // be triggered. Threshold `json:"threshold"` Delay DurationMinutes `json:"delay,omitempty"` + // Repeat is measured in minutes. + // + // A value of 0 (the default) disables repeat notifications. + Repeat DurationMinutes `json:"repeat,omitempty" bson:"repeat"` } func (a HighAlert) Validate(validator structure.Validator) { a.Base.Validate(validator) a.Threshold.Validate(validator) - dur := a.Delay.Duration() - validator.Duration("delay", &dur).InRange(0, 6*time.Hour) + delayDur := a.Delay.Duration() + validator.Duration("delay", &delayDur).InRange(0, 6*time.Hour) + repeatDur := a.Repeat.Duration() + validator.Duration("repeat", &repeatDur).Using(validateRepeat) } // DurationMinutes reads a JSON integer and converts it to a time.Duration. diff --git a/alerts/config_test.go b/alerts/config_test.go index 5cfa983643..8fc6e0240a 100644 --- a/alerts/config_test.go +++ b/alerts/config_test.go @@ -43,7 +43,6 @@ var _ = Describe("Config", func() { }, "urgentLow": { "enabled": false, - "repeat": 30, "threshold": { "units": "mg/dL", "value": 47.5 @@ -60,12 +59,10 @@ var _ = Describe("Config", func() { }, "notLooping": { "enabled": true, - "repeat": 32, "delay": 4 }, "noCommunication": { "enabled": true, - "repeat": 33, "delay": 6 } }`, mockUserID1, mockUserID2, mockUploadID) @@ -86,14 +83,11 @@ var _ = Describe("Config", func() { Expect(conf.Low.Threshold.Value).To(Equal(80.0)) Expect(conf.Low.Threshold.Units).To(Equal(glucose.MgdL)) Expect(conf.UrgentLow.Enabled).To(Equal(false)) - Expect(conf.UrgentLow.Repeat).To(Equal(DurationMinutes(30 * time.Minute))) Expect(conf.UrgentLow.Threshold.Value).To(Equal(47.5)) Expect(conf.UrgentLow.Threshold.Units).To(Equal(glucose.MgdL)) Expect(conf.NotLooping.Enabled).To(Equal(true)) - Expect(conf.NotLooping.Repeat).To(Equal(DurationMinutes(32 * time.Minute))) Expect(conf.NotLooping.Delay).To(Equal(DurationMinutes(4 * time.Minute))) Expect(conf.NoCommunication.Enabled).To(Equal(true)) - Expect(conf.NoCommunication.Repeat).To(Equal(DurationMinutes(33 * time.Minute))) Expect(conf.NoCommunication.Delay).To(Equal(DurationMinutes(6 * time.Minute))) }) @@ -322,32 +316,41 @@ var _ = Describe("Config", func() { }) Context("repeat", func() { + var defaultAlert = LowAlert{ + Threshold: Threshold{Value: 11, Units: glucose.MmolL}, + } + It("accepts values of 0 (indicating disabled)", func() { val := validator.New() - b := Base{Repeat: 0} - b.Validate(val) + l := defaultAlert + l.Repeat = 0 + l.Validate(val) Expect(val.Error()).To(Succeed()) }) It("accepts values of 15 minutes to 4 hours (inclusive)", func() { val := validator.New() - b := Base{Repeat: DurationMinutes(15 * time.Minute)} - b.Validate(val) + l := defaultAlert + l.Repeat = DurationMinutes(15 * time.Minute) + l.Validate(val) Expect(val.Error()).To(Succeed()) val = validator.New() - b = Base{Repeat: DurationMinutes(4 * time.Hour)} - b.Validate(val) + l = defaultAlert + l.Repeat = DurationMinutes(4 * time.Hour) + l.Validate(val) Expect(val.Error()).To(Succeed()) val = validator.New() - b = Base{Repeat: DurationMinutes(4*time.Hour + 1)} - b.Validate(val) + l = defaultAlert + l.Repeat = DurationMinutes(4*time.Hour + 1) + l.Validate(val) Expect(val.Error()).NotTo(Succeed()) val = validator.New() - b = Base{Repeat: DurationMinutes(15*time.Minute - 1)} - b.Validate(val) + l = defaultAlert + l.Repeat = DurationMinutes(15*time.Minute - 1) + l.Validate(val) Expect(val.Error()).NotTo(Succeed()) }) }) @@ -359,67 +362,68 @@ var _ = Describe("Config", func() { err := request.DecodeObject(nil, buf, threshold) Expect(err).To(MatchError("json is malformed")) }) - It("validates repeat minutes (negative)", func() { + }) + + Context("low", func() { + It("accepts a blank repeat", func() { buf := buff(`{ "userId": "%s", "followedUserId": "%s", "uploadId": "%s", - "urgentLow": { - "enabled": false, - "repeat": -11, + "low": { + "enabled": true, + "delay": 10, "threshold": { - "units": "%s", - "value": 47.5 + "units": "mg/dL", + "value": 80 } } -}`, mockUserID1, mockUserID2, mockUploadID, glucose.MgdL) - cfg := &Config{} - err := request.DecodeObject(nil, buf, cfg) - Expect(err).To(MatchError("value -11m0s is not greater than or equal to 15m0s")) +}`, mockUserID1, mockUserID2, mockUploadID) + conf := &Config{} + err := request.DecodeObject(nil, buf, conf) + Expect(err).To(Succeed()) + Expect(conf.Low.Repeat).To(Equal(DurationMinutes(0))) }) - It("validates repeat minutes (string)", func() { - buf := buff(`{ + }) + It("validates repeat minutes (negative)", func() { + buf := buff(`{ "userId": "%s", "followedUserId": "%s", - "urgentLow": { + "uploadId": "%s", + "low": { "enabled": false, - "repeat": "a", + "repeat": -11, "threshold": { "units": "%s", - "value": 1 + "value": 47.5 } } -}`, mockUserID1, mockUserID2, glucose.MgdL) - cfg := &Config{} - err := request.DecodeObject(nil, buf, cfg) - Expect(err).To(MatchError("json is malformed")) - }) +}`, mockUserID1, mockUserID2, mockUploadID, glucose.MgdL) + cfg := &Config{} + err := request.DecodeObject(nil, buf, cfg) + Expect(err).To(MatchError("value -11m0s is not greater than or equal to 15m0s")) }) - - Context("low", func() { - It("accepts a blank repeat", func() { - buf := buff(`{ + It("validates repeat minutes (string)", func() { + buf := buff(`{ "userId": "%s", "followedUserId": "%s", "uploadId": "%s", "low": { - "enabled": true, - "delay": 10, + "enabled": false, + "repeat": "a", "threshold": { - "units": "mg/dL", - "value": 80 + "units": "%s", + "value": 1 } } -}`, mockUserID1, mockUserID2, mockUploadID) - conf := &Config{} - err := request.DecodeObject(nil, buf, conf) - Expect(err).To(Succeed()) - Expect(conf.Low.Repeat).To(Equal(DurationMinutes(0))) - }) +}`, mockUserID1, mockUserID2, mockUploadID, glucose.MgdL) + cfg := &Config{} + err := request.DecodeObject(nil, buf, cfg) + Expect(err).To(MatchError("json is malformed")) }) }) -var _ = Describe("Duration", func() { +var _ = Describe("DurationMinutes", func() { It("parses 42", func() { d := DurationMinutes(0) err := d.UnmarshalJSON([]byte(`42`))