From 9ce05ecf73b88c80e5390c9c375c9d2604e220d3 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Wed, 12 Oct 2016 18:48:09 -0700 Subject: [PATCH] storage/sql: add garbage collection method --- storage/sql/config.go | 5 ++-- storage/sql/config_test.go | 4 +-- storage/sql/crud.go | 20 ++++++++++++++ storage/sql/gc.go | 53 -------------------------------------- storage/sql/gc_test.go | 53 -------------------------------------- 5 files changed, 24 insertions(+), 111 deletions(-) delete mode 100644 storage/sql/gc.go delete mode 100644 storage/sql/gc_test.go diff --git a/storage/sql/config.go b/storage/sql/config.go index 4bff016b..f8cdf248 100644 --- a/storage/sql/config.go +++ b/storage/sql/config.go @@ -5,7 +5,6 @@ import ( "fmt" "net/url" "strconv" - "time" "github.com/coreos/dex/storage" ) @@ -22,7 +21,7 @@ func (s *SQLite3) Open() (storage.Storage, error) { if err != nil { return nil, err } - return withGC(conn, time.Now), nil + return conn, nil } func (s *SQLite3) open() (*conn, error) { @@ -76,7 +75,7 @@ func (p *Postgres) Open() (storage.Storage, error) { if err != nil { return nil, err } - return withGC(conn, time.Now), nil + return conn, nil } func (p *Postgres) open() (*conn, error) { diff --git a/storage/sql/config_test.go b/storage/sql/config_test.go index 55a9d69c..e877421e 100644 --- a/storage/sql/config_test.go +++ b/storage/sql/config_test.go @@ -54,7 +54,7 @@ func TestSQLite3(t *testing.T) { } withTimeout(time.Second*10, func() { - conformance.RunTestSuite(t, newStorage) + conformance.RunTests(t, newStorage) }) } @@ -85,6 +85,6 @@ func TestPostgres(t *testing.T) { return conn } withTimeout(time.Minute*1, func() { - conformance.RunTestSuite(t, newStorage) + conformance.RunTests(t, newStorage) }) } diff --git a/storage/sql/crud.go b/storage/sql/crud.go index ca941f7c..0f8858de 100644 --- a/storage/sql/crud.go +++ b/storage/sql/crud.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "strings" + "time" "github.com/coreos/dex/storage" ) @@ -83,6 +84,25 @@ type scanner interface { Scan(dest ...interface{}) error } +func (c *conn) GarbageCollect(now time.Time) (result storage.GCResult, err error) { + r, err := c.Exec(`delete from auth_request where expiry < $1`, now) + if err != nil { + return result, fmt.Errorf("gc auth_request: %v", err) + } + if n, err := r.RowsAffected(); err == nil { + result.AuthRequests = n + } + + r, err = c.Exec(`delete from auth_code where expiry < $1`, now) + if err != nil { + return result, fmt.Errorf("gc auth_code: %v", err) + } + if n, err := r.RowsAffected(); err == nil { + result.AuthCodes = n + } + return +} + func (c *conn) CreateAuthRequest(a storage.AuthRequest) error { _, err := c.Exec(` insert into auth_request ( diff --git a/storage/sql/gc.go b/storage/sql/gc.go deleted file mode 100644 index 1636c087..00000000 --- a/storage/sql/gc.go +++ /dev/null @@ -1,53 +0,0 @@ -package sql - -import ( - "context" - "fmt" - "log" - "time" - - "github.com/coreos/dex/storage" -) - -type gc struct { - now func() time.Time - conn *conn -} - -func (gc gc) run() error { - for _, table := range []string{"auth_request", "auth_code"} { - _, err := gc.conn.Exec(`delete from `+table+` where expiry < $1`, gc.now()) - if err != nil { - return fmt.Errorf("gc %s: %v", table, err) - } - // TODO(ericchiang): when we have levelled logging print how many rows were gc'd - } - return nil -} - -type withCancel struct { - storage.Storage - cancel context.CancelFunc -} - -func (w withCancel) Close() error { - w.cancel() - return w.Storage.Close() -} - -func withGC(conn *conn, now func() time.Time) storage.Storage { - ctx, cancel := context.WithCancel(context.Background()) - run := (gc{now, conn}).run - go func() { - for { - select { - case <-time.After(time.Second * 30): - if err := run(); err != nil { - log.Printf("gc failed: %v", err) - } - case <-ctx.Done(): - } - } - }() - return withCancel{conn, cancel} -} diff --git a/storage/sql/gc_test.go b/storage/sql/gc_test.go deleted file mode 100644 index ad6097e0..00000000 --- a/storage/sql/gc_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package sql - -import ( - "testing" - "time" - - "github.com/coreos/dex/storage" -) - -func TestGC(t *testing.T) { - // TODO(ericchiang): Add a GarbageCollect method to the storage interface so - // we can write conformance tests instead of directly testing each implementation. - s := &SQLite3{":memory:"} - conn, err := s.open() - if err != nil { - t.Fatal(err) - } - defer conn.Close() - - clock := time.Now() - now := func() time.Time { return clock } - - runGC := (gc{now, conn}).run - - a := storage.AuthRequest{ - ID: storage.NewID(), - Expiry: now().Add(time.Second), - } - - if err := conn.CreateAuthRequest(a); err != nil { - t.Fatal(err) - } - - if err := runGC(); err != nil { - t.Errorf("gc failed: %v", err) - } - - if _, err := conn.GetAuthRequest(a.ID); err != nil { - t.Errorf("failed to get auth request after gc: %v", err) - } - - clock = clock.Add(time.Minute) - - if err := runGC(); err != nil { - t.Errorf("gc failed: %v", err) - } - - if _, err := conn.GetAuthRequest(a.ID); err == nil { - t.Errorf("expected error after gc'ing auth request: %v", err) - } else if err != storage.ErrNotFound { - t.Errorf("expected error storage.NotFound got: %v", err) - } -}