diff --git a/server/server.go b/server/server.go index 7495f232..ddd75562 100644 --- a/server/server.go +++ b/server/server.go @@ -39,7 +39,6 @@ import ( - HIGH Rate limiting: Sensitive endpoints (account/login/change-password/...) - HIGH Stripe payment methods - MEDIUM: Test new token endpoints & never-expiring token -- MEDIUM: Test that anonymous user and user without tier are the same visitor - MEDIUM: Make sure account endpoints make sense for admins - MEDIUM: Reservation (UI): Show "This topic is reserved" error message when trying to reserve a reserved topic (Thorben) - MEDIUM: Reservation (UI): Ask for confirmation when removing reservation (deadcade) diff --git a/server/server_account_test.go b/server/server_account_test.go index fa987726..fe5fe0fc 100644 --- a/server/server_account_test.go +++ b/server/server_account_test.go @@ -622,3 +622,50 @@ func TestAccount_Reservation_Add_Kills_Other_Subscribers(t *testing.T) { t.Fatal("Waiting for user subscription to be killed failed") } } + +func TestAccount_Persist_UserStats_After_Tier_Change(t *testing.T) { + conf := newTestConfigWithAuthFile(t) + conf.AuthDefault = user.PermissionReadWrite + conf.AuthStatsQueueWriterInterval = 100 * time.Millisecond + s := newTestServer(t, conf) + defer s.closeDatabases() + + // Create user with tier + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.CreateTier(&user.Tier{ + Code: "starter", + MessageLimit: 10, + })) + require.Nil(t, s.userManager.CreateTier(&user.Tier{ + Code: "pro", + MessageLimit: 20, + })) + require.Nil(t, s.userManager.ChangeTier("phil", "starter")) + + // Publish a message + rr := request(t, s, "POST", "/mytopic", "hi", map[string]string{ + "Authorization": util.BasicAuth("phil", "phil"), + }) + require.Equal(t, 200, rr.Code) + + // Wait for stats queue writer + time.Sleep(200 * time.Millisecond) + + // Verify that message stats were persisted + u, err := s.userManager.User("phil") + require.Nil(t, err) + require.Equal(t, int64(1), u.Stats.Messages) + + // Change tier, make a request (to reset limiters) + require.Nil(t, s.userManager.ChangeTier("phil", "pro")) + rr = request(t, s, "GET", "/v1/account", "", map[string]string{ + "Authorization": util.BasicAuth("phil", "phil"), + }) + require.Equal(t, 200, rr.Code) + + // Verify that message stats were persisted + time.Sleep(300 * time.Millisecond) + u, err = s.userManager.User("phil") + require.Nil(t, err) + require.Equal(t, int64(0), u.Stats.Messages) // v.EnqueueStats had run! +} diff --git a/server/server_test.go b/server/server_test.go index 3c491144..c3bb9cf9 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1785,10 +1785,45 @@ func TestServer_PublishWhileUpdatingStatsWithLotsOfMessages(t *testing.T) { log.Info("Done: Publishing message; took %s", time.Since(start).Round(time.Millisecond)) // Wait for all goroutines - <-statsChan + select { + case <-statsChan: + case <-time.After(10 * time.Second): + t.Fatal("Timed out waiting for Go routines") + } log.Info("Done: Waiting for all locks") } +func TestServer_AnonymousUser_And_NonTierUser_Are_Same_Visitor(t *testing.T) { + conf := newTestConfigWithAuthFile(t) + s := newTestServer(t, conf) + defer s.closeDatabases() + + // Create user without tier + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + + // Publish a message (anonymous user) + rr := request(t, s, "POST", "/mytopic", "hi", nil) + require.Equal(t, 200, rr.Code) + + // Publish a message (non-tier user) + rr = request(t, s, "POST", "/mytopic", "hi", map[string]string{ + "Authorization": util.BasicAuth("phil", "phil"), + }) + require.Equal(t, 200, rr.Code) + + // User stats (anonymous user) + rr = request(t, s, "GET", "/v1/account", "", nil) + account, _ := util.UnmarshalJSON[apiAccountResponse](io.NopCloser(rr.Body)) + require.Equal(t, int64(2), account.Stats.Messages) + + // User stats (non-tier user) + rr = request(t, s, "GET", "/v1/account", "", map[string]string{ + "Authorization": util.BasicAuth("phil", "phil"), + }) + account, _ = util.UnmarshalJSON[apiAccountResponse](io.NopCloser(rr.Body)) + require.Equal(t, int64(2), account.Stats.Messages) +} + func newTestConfig(t *testing.T) *Config { conf := NewConfig() conf.BaseURL = "http://127.0.0.1:12345" diff --git a/server/visitor.go b/server/visitor.go index 84816fde..e2966dc1 100644 --- a/server/visitor.go +++ b/server/visitor.go @@ -302,13 +302,12 @@ func (v *visitor) resetLimitersNoLock(messages, emails int64, enqueueUpdate bool } else { v.accountLimiter = nil // Users cannot create accounts when logged in } - /* - if enqueueUpdate && v.user != nil { - go v.userManager.EnqueueStats(v.user.ID, &user.Stats{ - Messages: messages, - Emails: emails, - }) - }*/ + if enqueueUpdate && v.user != nil { + go v.userManager.EnqueueStats(v.user.ID, &user.Stats{ + Messages: messages, + Emails: emails, + }) + } } func (v *visitor) Limits() *visitorLimits { diff --git a/user/manager.go b/user/manager.go index 7009d125..a3f79f69 100644 --- a/user/manager.go +++ b/user/manager.go @@ -1113,7 +1113,7 @@ func (a *Manager) CreateTier(tier *Tier) error { if tier.ID == "" { tier.ID = util.RandomStringPrefix(tierIDPrefix, tierIDLength) } - if _, err := a.db.Exec(insertTierQuery, tier.ID, tier.Code, tier.Name, tier.MessageLimit, int64(tier.MessageExpiryDuration.Seconds()), tier.EmailLimit, tier.ReservationLimit, tier.AttachmentFileSizeLimit, tier.AttachmentTotalSizeLimit, int64(tier.AttachmentExpiryDuration.Seconds()), tier.AttachmentBandwidthLimit, tier.StripePriceID); err != nil { + if _, err := a.db.Exec(insertTierQuery, tier.ID, tier.Code, tier.Name, tier.MessageLimit, int64(tier.MessageExpiryDuration.Seconds()), tier.EmailLimit, tier.ReservationLimit, tier.AttachmentFileSizeLimit, tier.AttachmentTotalSizeLimit, int64(tier.AttachmentExpiryDuration.Seconds()), tier.AttachmentBandwidthLimit, nullString(tier.StripePriceID)); err != nil { return err } return nil