From c0b54d821e8df4b9c39d93094815239270ff4d49 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Wed, 1 Oct 2014 13:09:36 -0800 Subject: [PATCH] User creation, DB transactions, createdb flag changes. --- api/handler.go | 1 + api/users.go | 19 ++++++++++++++++ cmd/bactdb/bactdb.go | 10 ++++++--- datastore/datastore.go | 6 +++++ datastore/db.go | 32 ++++++++++++++++++++++++++ datastore/users.go | 50 ++++++++++++++++++++++++++++++++++++++++- datastore/users_test.go | 25 ++++++++++++++++++++- models/users.go | 37 ++++++++++++++++++++++++++++-- models/users_test.go | 36 +++++++++++++++++++++++++++++ router/api.go | 1 + router/routes.go | 5 +++-- 11 files changed, 213 insertions(+), 9 deletions(-) diff --git a/api/handler.go b/api/handler.go index 96f68b7..93dd042 100644 --- a/api/handler.go +++ b/api/handler.go @@ -19,6 +19,7 @@ var ( func Handler() *mux.Router { m := router.API() m.Get(router.User).Handler(handler(serveUser)) + m.Get(router.CreateUser).Handler(handler(serveCreateUser)) m.Get(router.Users).Handler(handler(serveUsers)) return m } diff --git a/api/users.go b/api/users.go index 1ce892c..6b29c31 100644 --- a/api/users.go +++ b/api/users.go @@ -1,6 +1,7 @@ package api import ( + "encoding/json" "strconv" "github.com/gorilla/mux" @@ -24,6 +25,24 @@ func serveUser(w http.ResponseWriter, r *http.Request) error { return writeJSON(w, user) } +func serveCreateUser(w http.ResponseWriter, r *http.Request) error { + var user models.User + err := json.NewDecoder(r.Body).Decode(&user) + if err != nil { + return err + } + + created, err := store.Users.Create(&user) + if err != nil { + return err + } + if created { + w.WriteHeader(http.StatusCreated) + } + + return writeJSON(w, user) +} + func serveUsers(w http.ResponseWriter, r *http.Request) error { var opt models.UserListOptions if err := schemaDecoder.Decode(&opt, r.URL.Query()); err != nil { diff --git a/cmd/bactdb/bactdb.go b/cmd/bactdb/bactdb.go index d28fd5e..2c563eb 100644 --- a/cmd/bactdb/bactdb.go +++ b/cmd/bactdb/bactdb.go @@ -63,7 +63,7 @@ type subcmd struct { var subcmds = []subcmd{ {"serve", "start web server", serveCmd}, - {"create-db", "create the database schema", createDBCmd}, + {"createdb", "create the database schema", createDBCmd}, } func serveCmd(args []string) { @@ -98,9 +98,10 @@ The options are: } func createDBCmd(args []string) { - fs := flag.NewFlagSet("create-db", flag.ExitOnError) + fs := flag.NewFlagSet("createdb", flag.ExitOnError) + drop := fs.Bool("drop", false, "drop DB before creating") fs.Usage = func() { - fmt.Fprintln(os.Stderr, `usage: bactdb create-db [options] + fmt.Fprintln(os.Stderr, `usage: bactdb createdb [options] Creates the necessary DB schema. @@ -116,5 +117,8 @@ The options are: } datastore.Connect() + if *drop { + datastore.Drop() + } datastore.Create() } diff --git a/datastore/datastore.go b/datastore/datastore.go index 3a8e927..b18c012 100644 --- a/datastore/datastore.go +++ b/datastore/datastore.go @@ -22,3 +22,9 @@ func NewDatastore(dbh modl.SqlExecutor) *Datastore { d.Users = &usersStore{d} return d } + +func NewMockDatastore() *Datastore { + return &Datastore{ + Users: &models.MockUsersService{}, + } +} diff --git a/datastore/db.go b/datastore/db.go index e387ca9..c97ef65 100644 --- a/datastore/db.go +++ b/datastore/db.go @@ -51,3 +51,35 @@ func Drop() { // TODO(mrd): raise errors. DB.DropTables() } + +// transact calls fn in a DB transaction. If dbh is a transaction, then it just calls +// the function. Otherwise, it begins a transaction, rolling back on failure and +// committing on success. +func transact(dbh modl.SqlExecutor, fn func(fbh modl.SqlExecutor) error) error { + var sharedTx bool + tx, sharedTx := dbh.(*modl.Transaction) + if !sharedTx { + var err error + tx, err = dbh.(*modl.DbMap).Begin() + if err != nil { + return err + } + defer func() { + if err != nil { + tx.Rollback() + } + }() + } + + if err := fn(tx); err != nil { + return err + } + + if !sharedTx { + if err := tx.Commit(); err != nil { + return err + } + } + + return nil +} diff --git a/datastore/users.go b/datastore/users.go index 69370bf..ee233a4 100644 --- a/datastore/users.go +++ b/datastore/users.go @@ -1,6 +1,14 @@ package datastore -import "github.com/thermokarst/bactdb/models" +import ( + "fmt" + "math/rand" + "strings" + "time" + + "github.com/jmoiron/modl" + "github.com/thermokarst/bactdb/models" +) func init() { DB.AddTableWithName(models.User{}, "users").SetKeys(true, "Id") @@ -24,6 +32,46 @@ func (s *usersStore) Get(id int64) (*models.User, error) { return users[0], nil } +func (s *usersStore) Create(user *models.User) (bool, error) { + retries := 3 + var wantRetry bool + +retry: + retries-- + wantRetry = false + if retries == 0 { + return false, fmt.Errorf("failed to create user with username %q after retrying", user.UserName) + } + + var created bool + err := transact(s.dbh, func(tx modl.SqlExecutor) error { + var existing []*models.User + if err := tx.Select(&existing, `SELECT * FROM users WHERE username=$1 LIMIT 1;`, user.UserName); err != nil { + return err + } + if len(existing) > 0 { + *user = *existing[0] + return nil + } + + if err := tx.Insert(user); err != nil { + if strings.Contains(err.Error(), `violates unique constraint "username_idx"`) { + time.Sleep(time.Duration(rand.Intn(75)) * time.Millisecond) + wantRetry = true + return err + } + return err + } + + created = true + return nil + }) + if wantRetry { + goto retry + } + return created, err +} + func (s *usersStore) List(opt *models.UserListOptions) ([]*models.User, error) { if opt == nil { opt = &models.UserListOptions{} diff --git a/datastore/users_test.go b/datastore/users_test.go index eaff1e0..a3efdd6 100644 --- a/datastore/users_test.go +++ b/datastore/users_test.go @@ -12,6 +12,7 @@ func TestUsersStore_Get_db(t *testing.T) { tx, _ := DB.Begin() defer tx.Rollback() + // Test on a clean database tx.Exec(`DELETE FROM users;`) if err := tx.Insert(want); err != nil { @@ -30,10 +31,32 @@ func TestUsersStore_Get_db(t *testing.T) { } } +func TestUsersStore_Create_db(t *testing.T) { + user := &models.User{Id: 1, UserName: "Test User"} + + tx, _ := DB.Begin() + defer tx.Rollback() + + // Test on a clean database + tx.Exec(`DELETE FROM users;`) + + d := NewDatastore(tx) + created, err := d.Users.Create(user) + if err != nil { + t.Fatal(err) + } + + if !created { + t.Error("!created") + } + if user.Id == 0 { + t.Error("want nonzero user.Id after submitting") + } +} + func TestUsersStore_List_db(t *testing.T) { want := []*models.User{{Id: 1, UserName: "Test User"}} - // tx := DBH tx, _ := DB.Begin() defer tx.Rollback() diff --git a/models/users.go b/models/users.go index 0022897..76b8a04 100644 --- a/models/users.go +++ b/models/users.go @@ -2,6 +2,7 @@ package models import ( "errors" + "net/http" "strconv" "time" @@ -24,6 +25,9 @@ type UsersService interface { // List all users. List(opt *UserListOptions) ([]*User, error) + + // Create a new user. The newly created user's ID is written to user.Id + Create(user *User) (created bool, err error) } var ( @@ -58,6 +62,25 @@ func (s *usersService) Get(id int64) (*User, error) { return user, nil } +func (s *usersService) Create(user *User) (bool, error) { + url, err := s.client.url(router.CreateUser, nil, nil) + if err != nil { + return false, err + } + + req, err := s.client.NewRequest("POST", url.String(), user) + if err != nil { + return false, err + } + + resp, err := s.client.Do(req, &user) + if err != nil { + return false, err + } + + return resp.StatusCode == http.StatusCreated, nil +} + type UserListOptions struct { ListOptions } @@ -83,10 +106,13 @@ func (s *usersService) List(opt *UserListOptions) ([]*User, error) { } type MockUsersService struct { - Get_ func(id int64) (*User, error) - List_ func(opt *UserListOptions) ([]*User, error) + Get_ func(id int64) (*User, error) + List_ func(opt *UserListOptions) ([]*User, error) + Create_ func(post *User) (bool, error) } +var _ UsersService = &MockUsersService{} + func (s *MockUsersService) Get(id int64) (*User, error) { if s.Get_ == nil { return nil, nil @@ -94,6 +120,13 @@ func (s *MockUsersService) Get(id int64) (*User, error) { return s.Get_(id) } +func (s *MockUsersService) Create(user *User) (bool, error) { + if s.Create_ == nil { + return false, nil + } + return s.Create_(user) +} + func (s *MockUsersService) List(opt *UserListOptions) ([]*User, error) { if s.List_ == nil { return nil, nil diff --git a/models/users_test.go b/models/users_test.go index 42c67de..274aeb4 100644 --- a/models/users_test.go +++ b/models/users_test.go @@ -38,6 +38,42 @@ func TestUsersService_Get(t *testing.T) { } } +func TestUsersService_Create(t *testing.T) { + setup() + defer teardown() + + want := &User{Id: 1, UserName: "Test User"} + + var called bool + mux.HandleFunc(urlPath(t, router.CreateUser, nil), func(w http.ResponseWriter, r *http.Request) { + called = true + testMethod(t, r, "POST") + testBody(t, r, `{"id":1,"user_name":"Test User","created_at":"0001-01-01T00:00:00Z","updated_at":"0001-01-01T00:00:00Z","deleted_at":"0001-01-01T00:00:00Z"}`+"\n") + + w.WriteHeader(http.StatusCreated) + writeJSON(w, want) + }) + + user := &User{Id: 1, UserName: "Test User"} + created, err := client.Users.Create(user) + if err != nil { + t.Errorf("Users.Create returned error: %v", err) + } + + if !created { + t.Error("!created") + } + + if !called { + t.Fatal("!called") + } + + normalizeTime(&want.CreatedAt, &want.UpdatedAt, &want.DeletedAt) + if !reflect.DeepEqual(user, want) { + t.Errorf("Users.Create returned %+v, want %+v", user, want) + } +} + func TestUsersService_List(t *testing.T) { setup() defer teardown() diff --git a/router/api.go b/router/api.go index 8560644..df2effb 100644 --- a/router/api.go +++ b/router/api.go @@ -5,6 +5,7 @@ import "github.com/gorilla/mux" func API() *mux.Router { m := mux.NewRouter() m.Path("/users").Methods("GET").Name(Users) + m.Path("/users").Methods("POST").Name(CreateUser) m.Path("/users/{Id:.+}").Methods("GET").Name(User) return m } diff --git a/router/routes.go b/router/routes.go index 930f9ea..4e7dd46 100644 --- a/router/routes.go +++ b/router/routes.go @@ -1,6 +1,7 @@ package router const ( - User = "user" - Users = "users" + User = "user" + CreateUser = "user:create" + Users = "users" )