diff --git a/.travis.yml b/.travis.yml index 64ab288..32e28a8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,14 @@ language: go go: - - 1.2 + - 1.5 + - 1.6 - tip -script: make test +script: + - make get-deps + - make metalint + - make check env: - PGSTORE_TEST_CONN="postgres://postgres@127.0.0.1/test?sslmode=disable" @@ -13,6 +17,6 @@ before_script: - psql -c 'create database test;' -U postgres addons: - postgresql: "9.3" + postgresql: "9.4" sudo: false diff --git a/Makefile b/Makefile index 735beee..25325b0 100644 --- a/Makefile +++ b/Makefile @@ -1,13 +1,39 @@ all: get-deps build +.PHONY: build build: - @go build pgstore.go + go build ./... +.PHONY: get-deps get-deps: - @go get -d -v ./... + go get -v ./... -test: get-deps - @go test -v ./... +.PHONY: test +test: get-deps metalint check -format: - @go fmt ./... +.PHONY: check +check: + go test -v -race -cover ./... + +.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 ./... + +.PHONY: fmt +fmt: + @go fmt ./... | awk '{ print "Please run go fmt"; exit 1 }' + +.PHONY: docker-test +docker-test: + docker run -d -p 5432:5432 --name=pgstore_test_1 postgres:9.4 + sleep 5 + 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 check + docker kill pgstore_test_1 + docker rm pgstore_test_1 + +.PHONY: docker-clean +docker-clean: + -docker kill pgstore_test_1 + -docker rm pgstore_test_1 diff --git a/cleanup.go b/cleanup.go index 5a5caed..51aee3f 100644 --- a/cleanup.go +++ b/cleanup.go @@ -38,11 +38,11 @@ func (db *PGStore) cleanup(interval time.Duration, quit <-chan struct{}, done ch for { select { case <-quit: - // Handle the quit signal + // Handle the quit signal. done <- struct{}{} return case <-ticker.C: - // Delete expired sessions on each tick + // Delete expired sessions on each tick. err := db.deleteExpired() if err != nil { log.Printf("pgstore: unable to delete expired sessions: %v", err) diff --git a/cleanup_test.go b/cleanup_test.go index f63ccbe..00ffa58 100644 --- a/cleanup_test.go +++ b/cleanup_test.go @@ -8,14 +8,14 @@ import ( ) func TestCleanup(t *testing.T) { - ss, err := NewPGStore(os.Getenv("PGSTORE_TEST_CONN"), []byte(secret)) - - if err != nil { - t.Fatal("Failed to get store", err) + dsn := os.Getenv("PGSTORE_TEST_CONN") + if dsn == "" { + t.Skip("This test requires a real database.") } - if ss == nil { - t.Skip("This test requires a real database") + ss, err := NewPGStore(dsn, []byte(secret)) + if err != nil { + t.Fatal("Failed to get store", err) } defer ss.Close() @@ -32,7 +32,7 @@ func TestCleanup(t *testing.T) { t.Fatal("Failed to create session", err) } - // Expire the session + // Expire the session. session.Options.MaxAge = 1 m := make(http.Header) @@ -40,8 +40,8 @@ func TestCleanup(t *testing.T) { t.Fatal("failed to save session:", err.Error()) } - // Give the ticker a moment to run - time.Sleep(time.Second * 1) + // 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 diff --git a/pgstore.go b/pgstore.go index 3fe7165..4c3549c 100644 --- a/pgstore.go +++ b/pgstore.go @@ -10,10 +10,12 @@ import ( "github.com/coopernurse/gorp" "github.com/gorilla/securecookie" "github.com/gorilla/sessions" + + // Include the pq postgres driver. _ "github.com/lib/pq" ) -// PGStore represents the currently configured session store +// PGStore represents the currently configured session store. type PGStore struct { Codecs []securecookie.Codec Options *sessions.Options @@ -36,7 +38,7 @@ type Session struct { func NewPGStore(dbURL string, keyPairs ...[]byte) (*PGStore, error) { db, err := sql.Open("postgres", dbURL) if err != nil { - // Ignore and return nil + // Ignore PGStore value and return nil. return nil, err } return NewPGStoreFromPool(db, keyPairs...) @@ -62,14 +64,14 @@ func NewPGStoreFromPool(db *sql.DB, keyPairs ...[]byte) (*PGStore, error) { err := dbmap.CreateTablesIfNotExists() if err != nil { - // Ignore and return nil + // Ignore PGStore value and return nil. return nil, err } return dbStore, nil } -// Close closes the database connection +// Close closes the database connection. func (db *PGStore) Close() { db.DbMap.Db.Close() } @@ -80,12 +82,13 @@ func (db *PGStore) Get(r *http.Request, name string) (*sessions.Session, error) return sessions.GetRegistry(r).Get(db, name) } -// New returns a new session for the given name w/o adding it to the registry. +// New returns a new session for the given name without adding it to the registry. func (db *PGStore) New(r *http.Request, name string) (*sessions.Session, error) { session := sessions.NewSession(db, name) if session == nil { - return session, nil + return nil, nil } + opts := *db.Options session.Options = &(opts) session.IsNew = true @@ -121,7 +124,8 @@ func (db *PGStore) Save(r *http.Request, w http.ResponseWriter, session *session // Generate a random session ID key suitable for storage in the DB session.ID = strings.TrimRight( base32.StdEncoding.EncodeToString( - securecookie.GenerateRandomKey(32)), "=") + securecookie.GenerateRandomKey(32), + ), "=") } if err := db.save(session); err != nil { @@ -165,39 +169,33 @@ func (db *PGStore) MaxAge(age int) { } // load fetches a session by ID from the database and decodes its content -// into session.Values +// 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) - - if err := securecookie.DecodeMulti(session.Name(), string(s.Data), - &session.Values, db.Codecs...); err != nil { + if err != nil { return err } - return err + return securecookie.DecodeMulti(session.Name(), string(s.Data), &session.Values, db.Codecs...) } // save writes encoded session.Values to a database record. // writes to http_sessions table by default. func (db *PGStore) save(session *sessions.Session) error { - encoded, err := securecookie.EncodeMulti(session.Name(), session.Values, - db.Codecs...) - + encoded, err := securecookie.EncodeMulti(session.Name(), session.Values, db.Codecs...) if err != nil { return err } - var createdOn time.Time - var expiresOn time.Time - crOn := session.Values["created_on"] exOn := session.Values["expires_on"] - if crOn == nil { + var expiresOn time.Time + + createdOn, ok := crOn.(time.Time) + if !ok { createdOn = time.Now() - } else { - createdOn = crOn.(time.Time) } if exOn == nil { @@ -218,11 +216,10 @@ func (db *PGStore) save(session *sessions.Session) error { } if session.IsNew { - err = db.DbMap.Insert(&s) - } else { - _, 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 db.DbMap.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 } diff --git a/pgstore_test.go b/pgstore_test.go index 33144be..cfc0a92 100644 --- a/pgstore_test.go +++ b/pgstore_test.go @@ -27,14 +27,14 @@ func (ho headerOnlyResponseWriter) WriteHeader(int) { var secret = "EyaC2BPcJtNqU3tjEHy+c+Wmqc1yihYIbUWEl/jk0Ga73kWBclmuSFd9HuJKwJw/Wdsh1XnjY2Bw1HBVph6WOw==" func TestPGStore(t *testing.T) { - ss, err := NewPGStore(os.Getenv("PGSTORE_TEST_CONN"), []byte(secret)) - - if err != nil { - t.Fatal("failed to get store", err.Error()) + dsn := os.Getenv("PGSTORE_TEST_CONN") + if dsn == "" { + t.Skip("This test requires a real database.") } - if ss == nil { - t.Skip("This test requires a real database") + ss, err := NewPGStore(dsn, []byte(secret)) + if err != nil { + t.Fatal("Failed to get store", err) } defer ss.Close() @@ -125,14 +125,14 @@ func TestPGStore(t *testing.T) { } func TestSessionOptionsAreUniquePerSession(t *testing.T) { - ss, err := NewPGStore(os.Getenv("PGSTORE_TEST_CONN"), []byte(secret)) - - if err != nil { - t.Fatal("Failed to get store", err) + dsn := os.Getenv("PGSTORE_TEST_CONN") + if dsn == "" { + t.Skip("This test requires a real database.") } - if ss == nil { - t.Skip("This test requires a real database") + ss, err := NewPGStore(dsn, []byte(secret)) + if err != nil { + t.Fatal("Failed to get store", err) } defer ss.Close()