Skip to content
This repository was archived by the owner on Jul 12, 2023. It is now read-only.

Commit f30eb3a

Browse files
authored
User report cleanup (#2367)
* ensure user report cleanup time isn't too low * add log * push settings read to SQL * comments
1 parent 61a0dcf commit f30eb3a

File tree

4 files changed

+78
-2
lines changed

4 files changed

+78
-2
lines changed

pkg/config/cleanup_server_config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ type CleanupConfig struct {
7777
VerificationTokenMaxAge time.Duration `env:"VERIFICATION_TOKEN_MAX_AGE, default=24h"`
7878

7979
// UserReportUnclaimedMaxAge is how long a user report phone hash will be kept if the record goes unclaimed.
80-
UserReportUnclaimedMaxAge time.Duration `env:"USER_REPORT_UNCLAIMED_MAX_AGE, default=30m"`
80+
UserReportUnclaimedMaxAge time.Duration `env:"USER_REPORT_UNCLAIMED_MAX_AGE, default=60m"`
8181
// UserReportMaxAge is how long a claimed user report phone hash will be kept.
8282
UserReportMaxAge time.Duration `env:"USER_REPORT_MAX_AGE, default=720h"` // 720h = 30 days
8383
}

pkg/controller/cleanup/handle_cleanup.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,20 @@ func (c *Controller) HandleCleanup() http.Handler {
266266
func() {
267267
defer enobs.RecordLatency(ctx, time.Now(), mLatencyMs, &result, &item)
268268
item = tag.Upsert(itemTagKey, "UNCLAIMED_USER_REPORTS")
269-
if count, err := c.db.PurgeUnclaimedUserReports(c.config.UserReportUnclaimedMaxAge); err != nil {
269+
270+
realmMaxAge, err := c.db.MaximumUserReportTimeout()
271+
if err != nil {
272+
merr = multierror.Append(merr, fmt.Errorf("failed to determine max user report timeout: %w", err))
273+
result = enobs.ResultError("FAILED")
274+
return
275+
}
276+
maxAge := c.config.UserReportMaxAge
277+
if realmMaxAge > maxAge {
278+
logger.Warnw("overriding user report max age cleanup due to realm settings", "config", c.config.UserPurgeMaxAge, "using", realmMaxAge)
279+
maxAge = realmMaxAge
280+
}
281+
282+
if count, err := c.db.PurgeUnclaimedUserReports(maxAge); err != nil {
270283
merr = multierror.Append(merr, fmt.Errorf("failed to purge unclaimed user reports: %w", err))
271284
result = enobs.ResultError("FAILED")
272285
} else {

pkg/database/realm.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ const (
5656
TestTypeLikely
5757
TestTypeNegative
5858
TestTypeUserReport
59+
// if a 5 test type is added, update MaximumUserReportTimeout
5960
)
6061

6162
func (t TestType) Display() string {
@@ -1294,6 +1295,27 @@ func (r *Realm) ValidTestType(typ string) bool {
12941295
}
12951296
}
12961297

1298+
func (db *Database) MaximumUserReportTimeout() (time.Duration, error) {
1299+
// TestTypeUserReport == 16
1300+
// If we ever add ANOTHER test type, this statement would need
1301+
// to be updated.
1302+
sql := `
1303+
SELECT
1304+
MAX(code_duration) AS duration
1305+
FROM realms
1306+
WHERE
1307+
allowed_test_types >= 16;`
1308+
1309+
var result struct {
1310+
Duration int64 `gorm:"column:duration;"`
1311+
}
1312+
if err := db.db.Raw(sql).Scan(&result).Error; err != nil {
1313+
return 0, err
1314+
}
1315+
timeout := time.Duration(result.Duration) * time.Second
1316+
return timeout, nil
1317+
}
1318+
12971319
func (db *Database) FindRealmByRegion(region string) (*Realm, error) {
12981320
var realm Realm
12991321
if err := db.db.

pkg/database/realm_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ func TestTestType(t *testing.T) {
4242
{TestTypeConfirmed, 2},
4343
{TestTypeLikely, 4},
4444
{TestTypeNegative, 8},
45+
{TestTypeUserReport, 16},
4546
}
4647

4748
for _, tc := range cases {
@@ -692,6 +693,46 @@ func TestRealm_FindVerificationCodeByUUID(t *testing.T) {
692693
}
693694
}
694695

696+
func TestRealm_Maximum(t *testing.T) {
697+
t.Parallel()
698+
699+
db, _ := testDatabaseInstance.NewDatabase(t, nil)
700+
701+
realm1 := NewRealmWithDefaults("realm1")
702+
realm1.RegionCode = "US-MOO"
703+
realm1.AllowedTestTypes = TestTypeConfirmed | TestTypeUserReport
704+
realm1.CodeDuration = FromDuration(15 * time.Minute)
705+
if err := db.SaveRealm(realm1, SystemTest); err != nil {
706+
t.Fatal(err)
707+
}
708+
709+
realm2 := NewRealmWithDefaults("realm2")
710+
realm2.RegionCode = "US-FOO"
711+
realm2.AllowedTestTypes = TestTypeConfirmed | TestTypeUserReport
712+
realm2.CodeDuration = FromDuration(42 * time.Minute)
713+
if err := db.SaveRealm(realm2, SystemTest); err != nil {
714+
t.Fatal(err)
715+
}
716+
717+
realm3 := NewRealmWithDefaults("realm3")
718+
realm3.RegionCode = "US-BAR"
719+
realm3.AllowedTestTypes = TestTypeConfirmed
720+
realm3.CodeDuration = FromDuration(59 * time.Minute)
721+
if err := db.SaveRealm(realm3, SystemTest); err != nil {
722+
t.Fatal(err)
723+
}
724+
725+
maxDuration, err := db.MaximumUserReportTimeout()
726+
if err != nil {
727+
t.Fatalf("Unable to read max user report timeout: %v", err)
728+
}
729+
730+
want := 42 * time.Minute
731+
if want != maxDuration {
732+
t.Fatalf("Incorrect max timeout, want: %v ,got: %v", want, maxDuration)
733+
}
734+
}
735+
695736
func TestRealm_FindRealm(t *testing.T) {
696737
t.Parallel()
697738

0 commit comments

Comments
 (0)