From 1eb5e3d517c464979036b9081c403193dbd12252 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Thu, 11 Aug 2016 15:06:09 -0400 Subject: [PATCH 1/8] Remove the Gorp dependency Removed the underlying Gorp package as a dependency & replaced with use of the standard database/sql driver. This removes a dependency and makes this package very lightweight and makes database interactions very transparent. Lastly, from the standpoint of unit testing where you want to mock the database layer instead of requiring a real database, you can now easily use a package like [go-SQLMock](https://github.com/DATA-DOG/go-sqlmock) to do just that. The tests may/not pass for Golang 1.5; it appears that the problem is Travis need to install a linter. --- Makefile | 10 ++++-- README.md | 8 ++--- cleanup.go | 2 +- cleanup_test.go | 10 +++--- pgstore.go | 96 ++++++++++++++++++++++++++++++++++--------------- 5 files changed, 84 insertions(+), 42 deletions(-) diff --git a/Makefile b/Makefile index ff2494d..1d37a91 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,5 @@ +DHOST = $(shell echo $$(docker-machine ip)) + all: get-deps build .PHONY: build @@ -18,7 +20,7 @@ check: .PHONY: metalint metalint: which gometalinter > /dev/null || (go get github.com/alecthomas/gometalinter && gometalinter --install --update) - gometalinter --cyclo-over=20 -e "struct field Id should be ID" --enable="gofmt -s" --enable=misspell --fast ./... + gometalinter --cyclo-over=20 --enable="gofmt -s" --enable=misspell --fast ./... .PHONY: fmt fmt: @@ -27,9 +29,11 @@ fmt: .PHONY: docker-test docker-test: docker run -d -p 5432:5432 --name=pgstore_test_1 postgres:9.4 - sleep 5 + @echo "Ugly hack: Sleeping for 75 secs to give the Postgres container time to come up..." + sleep 75 + @echo "Waking up - let's do this!" docker run --rm --link pgstore_test_1:postgres postgres:9.4 psql -c 'create database test;' -U postgres -h postgres - PGSTORE_TEST_CONN="postgres://postgres@127.0.0.1:5432/test?sslmode=disable" make test + PGSTORE_TEST_CONN="postgres://postgres@$(DHOST):5432/test?sslmode=disable" make test docker kill pgstore_test_1 docker rm pgstore_test_1 diff --git a/README.md b/README.md index 097f983..eb79f3a 100644 --- a/README.md +++ b/README.md @@ -74,8 +74,6 @@ I've stolen, borrowed and gotten inspiration from the other backends available: Thank you all for sharing your code! -What makes this backend different is that it's for Postgresql and uses the fine -datamapper [Gorp](https://github.com/coopernurse/gorp). -Make sure you use a somewhat new codebase of Gorp as it now defaults to text for -strings when it used to default to varchar 255. Varchar 255 is unfortunately too -small. +What makes this backend different is that it's for PostgreSQL. + +We've recently refactored this backend to use the standard database/sql driver instead of Gorp. This removes a dependency and makes this package very lightweight and makes database interactions very transparent. Lastly, from the standpoint of unit testing where you want to mock the database layer instead of requiring a real database, you can now easily use a package like [go-SQLMock](https://github.com/DATA-DOG/go-sqlmock) to do just that. diff --git a/cleanup.go b/cleanup.go index 51aee3f..6498365 100644 --- a/cleanup.go +++ b/cleanup.go @@ -53,6 +53,6 @@ func (db *PGStore) cleanup(interval time.Duration, quit <-chan struct{}, done ch // deleteExpired deletes expired sessions from the database. func (db *PGStore) deleteExpired() error { - _, err := db.DbMap.Exec("DELETE FROM http_sessions WHERE expires_on < now()") + _, err := db.DbPool.Exec("DELETE FROM http_sessions WHERE expires_on < now()") return err } diff --git a/cleanup_test.go b/cleanup_test.go index 00ffa58..32f9330 100644 --- a/cleanup_test.go +++ b/cleanup_test.go @@ -43,14 +43,14 @@ func TestCleanup(t *testing.T) { // Give the ticker a moment to run. time.Sleep(time.Millisecond * 1500) - // SELECT expired sessions. We should get a zero-length result slice back. - var results []int64 - _, err = ss.DbMap.Select(&results, "SELECT id FROM http_sessions WHERE expires_on < now()") + // SELECT expired sessions. We should get a count of zero back. + var count int + err = ss.DbPool.QueryRow("SELECT count(*) FROM http_sessions WHERE expires_on < now()").Scan(&count) if err != nil { t.Fatalf("failed to select expired sessions from DB: %v", err) } - if len(results) > 0 { - t.Fatalf("ticker did not delete expired sessions: want 0 got %v", len(results)) + if count > 0 { + t.Fatalf("ticker did not delete expired sessions: want 0 got %v", count) } } diff --git a/pgstore.go b/pgstore.go index 4c3549c..00d0073 100644 --- a/pgstore.go +++ b/pgstore.go @@ -3,11 +3,12 @@ package pgstore import ( "database/sql" "encoding/base32" + "errors" + "fmt" "net/http" "strings" "time" - "github.com/coopernurse/gorp" "github.com/gorilla/securecookie" "github.com/gorilla/sessions" @@ -19,18 +20,18 @@ import ( type PGStore struct { Codecs []securecookie.Codec Options *sessions.Options - path string - DbMap *gorp.DbMap + Path string + DbPool *sql.DB } -// Session type -type Session struct { - Id int64 `db:"id"` - Key string `db:"key"` - Data string `db:"data"` - CreatedOn time.Time `db:"created_on"` - ModifiedOn time.Time `db:"modified_on"` - ExpiresOn time.Time `db:"expires_on"` +// PGSession type +type PGSession struct { + ID int64 + Key string + Data string + CreatedOn time.Time + ModifiedOn time.Time + ExpiresOn time.Time } // NewPGStore creates a new PGStore instance and a new database/sql pool. @@ -38,7 +39,7 @@ type Session struct { func NewPGStore(dbURL string, keyPairs ...[]byte) (*PGStore, error) { db, err := sql.Open("postgres", dbURL) if err != nil { - // Ignore PGStore value and return nil. + // Ignore and return nil. return nil, err } return NewPGStoreFromPool(db, keyPairs...) @@ -46,25 +47,20 @@ func NewPGStore(dbURL string, keyPairs ...[]byte) (*PGStore, error) { // NewPGStoreFromPool creates a new PGStore instance from an existing // database/sql pool. -// This will also create in the database the schema needed by pgstore. +// This will also create the database schema needed by pgstore. func NewPGStoreFromPool(db *sql.DB, keyPairs ...[]byte) (*PGStore, error) { - dbmap := &gorp.DbMap{Db: db, Dialect: gorp.PostgresDialect{}} - dbStore := &PGStore{ Codecs: securecookie.CodecsFromPairs(keyPairs...), Options: &sessions.Options{ Path: "/", MaxAge: 86400 * 30, }, - DbMap: dbmap, + DbPool: db, } // Create table if it doesn't exist - dbmap.AddTableWithName(Session{}, "http_sessions").SetKeys(true, "Id") - err := dbmap.CreateTablesIfNotExists() - + err := dbStore.createSessionsTable() if err != nil { - // Ignore PGStore value and return nil. return nil, err } @@ -73,7 +69,7 @@ func NewPGStoreFromPool(db *sql.DB, keyPairs ...[]byte) (*PGStore, error) { // Close closes the database connection. func (db *PGStore) Close() { - db.DbMap.Db.Close() + db.DbPool.Close() } // Get Fetches a session for a given name after it has been added to the @@ -171,8 +167,9 @@ func (db *PGStore) MaxAge(age int) { // load fetches a session by ID from the database and decodes its content // into session.Values. func (db *PGStore) load(session *sessions.Session) error { - var s Session - err := db.DbMap.SelectOne(&s, "SELECT * FROM http_sessions WHERE key = $1", session.ID) + var s PGSession + + err := db.selectOne(&s, session.ID) if err != nil { return err } @@ -207,7 +204,7 @@ func (db *PGStore) save(session *sessions.Session) error { } } - s := Session{ + s := PGSession{ Key: session.ID, Data: encoded, CreatedOn: createdOn, @@ -216,15 +213,58 @@ func (db *PGStore) save(session *sessions.Session) error { } if session.IsNew { - return db.DbMap.Insert(&s) + return db.insert(&s) } - _, err = db.DbMap.Exec("update http_sessions set data=$1, modified_on=$2, expires_on=$3 where key=$4", s.Data, s.ModifiedOn, s.ExpiresOn, s.Key) - return err + return db.update(&s) } // Delete session func (db *PGStore) destroy(session *sessions.Session) error { - _, err := db.DbMap.Db.Exec("DELETE FROM http_sessions WHERE key = $1", session.ID) + _, err := db.DbPool.Exec("DELETE FROM http_sessions WHERE key = $1", session.ID) + return err +} + +func (db *PGStore) createSessionsTable() error { + stmt := "CREATE TABLE IF NOT EXISTS http_sessions (" + + "id BIGSERIAL PRIMARY KEY," + + "key BYTEA," + + "data BYTEA," + + "created_on TIMESTAMPTZ DEFAULT CURRENT_TIMESTAMP," + + "modified_on TIMESTAMPTZ," + + "expires_on TIMESTAMPTZ);" + + _, err := db.DbPool.Exec(stmt) + if err != nil { + msg := fmt.Sprintf("Unable to create http_sessions table in the database: %s\n", err.Error()) + return errors.New(msg) + } + + return nil +} + +func (db *PGStore) selectOne(s *PGSession, key string) error { + stmt := "SELECT id, key, data, created_on, modified_on, expires_on FROM http_sessions WHERE key = $1" + err := db.DbPool.QueryRow(stmt, key).Scan(&s.ID, &s.Key, &s.Data, &s.CreatedOn, &s.ModifiedOn, &s.ExpiresOn) + if err != nil { + msg := fmt.Sprintf("Unable to find session in the database: %s\n", err.Error()) + return errors.New(msg) + } + + return nil +} + +func (db *PGStore) insert(s *PGSession) error { + stmt := `INSERT INTO http_sessions (key, data, created_on, modified_on, expires_on) + VALUES ($1, $2, $3, $4, $5)` + _, err := db.DbPool.Exec(stmt, s.Key, s.Data, s.CreatedOn, s.ModifiedOn, s.ExpiresOn) + + return err +} + +func (db *PGStore) update(s *PGSession) error { + stmt := `UPDATE http_sessions SET data=$1, modified_on=$2, expires_on=$3 WHERE key=$4` + _, err := db.DbPool.Exec(stmt, s.Data, s.ModifiedOn, s.ExpiresOn, s.Key) + return err } From 2fc037c81e79e4ed42a096681709c33ee85eb218 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Fri, 12 Aug 2016 13:18:15 -0400 Subject: [PATCH 2/8] Enable docker host discovery to work across Mac, Linux --- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1d37a91..b6420e9 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,9 @@ -DHOST = $(shell echo $$(docker-machine ip)) +UNAME := $(shell uname) +ifeq ($(UNAME), Darwin) + DHOST := $(shell echo $$(docker-machine ip)) +else + DHOST := 127.0.0.1 +endif all: get-deps build From f6cbf2c4dfc265433374403c640a28c830d244ab Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Fri, 12 Aug 2016 13:19:46 -0400 Subject: [PATCH 3/8] Clean up the way we specify the CREATE TABLE statement --- pgstore.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pgstore.go b/pgstore.go index 00d0073..aef5747 100644 --- a/pgstore.go +++ b/pgstore.go @@ -226,13 +226,13 @@ func (db *PGStore) destroy(session *sessions.Session) error { } func (db *PGStore) createSessionsTable() error { - stmt := "CREATE TABLE IF NOT EXISTS http_sessions (" + - "id BIGSERIAL PRIMARY KEY," + - "key BYTEA," + - "data BYTEA," + - "created_on TIMESTAMPTZ DEFAULT CURRENT_TIMESTAMP," + - "modified_on TIMESTAMPTZ," + - "expires_on TIMESTAMPTZ);" + stmt := `CREATE TABLE IF NOT EXISTS http_sessions ( + id BIGSERIAL PRIMARY KEY, + key BYTEA, + data BYTEA, + created_on TIMESTAMPTZ DEFAULT CURRENT_TIMESTAMP, + modified_on TIMESTAMPTZ, + expires_on TIMESTAMPTZ);` _, err := db.DbPool.Exec(stmt) if err != nil { From 189f77e155f8728cfd7ef6ef31cebbfb50e2dd79 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Fri, 12 Aug 2016 13:21:25 -0400 Subject: [PATCH 4/8] Add back exclusion for lint error --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b6420e9..f783fd3 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ check: .PHONY: metalint metalint: which gometalinter > /dev/null || (go get github.com/alecthomas/gometalinter && gometalinter --install --update) - gometalinter --cyclo-over=20 --enable="gofmt -s" --enable=misspell --fast ./... + gometalinter --cyclo-over=20 -e "struct field Id should be ID" --enable="gofmt -s" --enable=misspell --fast ./... .PHONY: fmt fmt: From df656e2dc156fd6525c91fce0041afbcc3a4be01 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Fri, 12 Aug 2016 13:39:06 -0400 Subject: [PATCH 5/8] Disable the lll linter due to errors --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f783fd3..b6f4f52 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ check: .PHONY: metalint metalint: which gometalinter > /dev/null || (go get github.com/alecthomas/gometalinter && gometalinter --install --update) - gometalinter --cyclo-over=20 -e "struct field Id should be ID" --enable="gofmt -s" --enable=misspell --fast ./... + gometalinter --cyclo-over=20 -e "struct field Id should be ID" --enable="gofmt -s" --enable=misspell --disable=lll --fast ./... .PHONY: fmt fmt: From e34d9b56286e409e7059bbe108164c7f8a4e7916 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Mon, 15 Aug 2016 18:56:58 -0400 Subject: [PATCH 6/8] Enable GO15VENDOREXPERIMENT environment variable Try to fix the Travis failure of the lll linter --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index fbe673b..16c8ebb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,6 +15,7 @@ script: - make check env: + - GO15VENDOREXPERIMENT=1 - PGSTORE_TEST_CONN="postgres://postgres@127.0.0.1/test?sslmode=disable" before_script: From 1cfe32f2bfae1cf4df1a78b0c8abdf7fd025650f Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Mon, 15 Aug 2016 19:00:25 -0400 Subject: [PATCH 7/8] Fix env line --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 16c8ebb..828727f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,8 +15,7 @@ script: - make check env: - - GO15VENDOREXPERIMENT=1 - - PGSTORE_TEST_CONN="postgres://postgres@127.0.0.1/test?sslmode=disable" + - GO15VENDOREXPERIMENT=1 PGSTORE_TEST_CONN="postgres://postgres@127.0.0.1/test?sslmode=disable" before_script: - psql -c 'create database test;' -U postgres From 5a829a35de460d09fc5a40fa1e21e021cc298ef6 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Mon, 15 Aug 2016 19:03:51 -0400 Subject: [PATCH 8/8] Dont need to disable lll --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b6f4f52..f783fd3 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ check: .PHONY: metalint metalint: which gometalinter > /dev/null || (go get github.com/alecthomas/gometalinter && gometalinter --install --update) - gometalinter --cyclo-over=20 -e "struct field Id should be ID" --enable="gofmt -s" --enable=misspell --disable=lll --fast ./... + gometalinter --cyclo-over=20 -e "struct field Id should be ID" --enable="gofmt -s" --enable=misspell --fast ./... .PHONY: fmt fmt: