From 0f053b07a55c6dad2ec4b6da75995ccfa26bcb4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= Date: Fri, 29 Dec 2017 13:53:02 -0800 Subject: [PATCH] Improve user API responses --- model/user.go | 12 ++--- server/middleware/basic_auth.go | 9 +++- storage/user.go | 93 +++++++++++++-------------------- 3 files changed, 51 insertions(+), 63 deletions(-) diff --git a/model/user.go b/model/user.go index 8026a368..aa1156d3 100644 --- a/model/user.go +++ b/model/user.go @@ -14,13 +14,13 @@ type User struct { ID int64 `json:"id"` Username string `json:"username"` Password string `json:"password,omitempty"` - IsAdmin bool `json:"is_admin,omitempty"` - Theme string `json:"theme,omitempty"` - Language string `json:"language,omitempty"` - Timezone string `json:"timezone,omitempty"` - EntryDirection string `json:"entry_sorting_direction,omitempty"` + IsAdmin bool `json:"is_admin"` + Theme string `json:"theme"` + Language string `json:"language"` + Timezone string `json:"timezone"` + EntryDirection string `json:"entry_sorting_direction"` LastLoginAt *time.Time `json:"last_login_at,omitempty"` - Extra map[string]string `json:"-"` + Extra map[string]string `json:"extra"` } // NewUser returns a new User. diff --git a/server/middleware/basic_auth.go b/server/middleware/basic_auth.go index c4a27360..35a9f816 100644 --- a/server/middleware/basic_auth.go +++ b/server/middleware/basic_auth.go @@ -39,7 +39,14 @@ func (b *BasicAuthMiddleware) Handler(next http.Handler) http.Handler { } user, err := b.store.UserByUsername(username) - if err != nil || user == nil { + if err != nil { + logger.Error("[Middleware:BasicAuth] %v", err) + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(errorResponse)) + return + } + + if user == nil { logger.Info("[Middleware:BasicAuth] User not found: %s", username) w.WriteHeader(http.StatusUnauthorized) w.Write([]byte(errorResponse)) diff --git a/storage/user.go b/storage/user.go index 427b3914..912ee04f 100644 --- a/storage/user.go +++ b/storage/user.go @@ -177,78 +177,40 @@ func (s *Storage) UpdateUser(user *model.User) error { func (s *Storage) UserByID(userID int64) (*model.User, error) { defer helper.ExecutionTime(time.Now(), fmt.Sprintf("[Storage:UserByID] userID=%d", userID)) query := `SELECT - id, username, is_admin, theme, language, timezone, entry_direction, extra + id, username, is_admin, theme, language, timezone, entry_direction, last_login_at, extra FROM users WHERE id = $1` - var user model.User - var extra hstore.Hstore - row := s.db.QueryRow(query, userID) - err := row.Scan( - &user.ID, - &user.Username, - &user.IsAdmin, - &user.Theme, - &user.Language, - &user.Timezone, - &user.EntryDirection, - &extra, - ) - if err == sql.ErrNoRows { - return nil, nil - } else if err != nil { - return nil, fmt.Errorf("unable to fetch user: %v", err) - } - - user.Extra = make(map[string]string) - for key, value := range extra.Map { - if value.Valid { - user.Extra[key] = value.String - } - } - - return &user, nil + return s.fetchUser(query, userID) } // UserByUsername finds a user by the username. func (s *Storage) UserByUsername(username string) (*model.User, error) { defer helper.ExecutionTime(time.Now(), fmt.Sprintf("[Storage:UserByUsername] username=%s", username)) query := `SELECT - id, username, is_admin, theme, language, timezone, entry_direction + id, username, is_admin, theme, language, timezone, entry_direction, last_login_at, extra FROM users WHERE username=LOWER($1)` - var user model.User - row := s.db.QueryRow(query, username) - err := row.Scan( - &user.ID, - &user.Username, - &user.IsAdmin, - &user.Theme, - &user.Language, - &user.Timezone, - &user.EntryDirection, - ) - if err == sql.ErrNoRows { - return nil, nil - } else if err != nil { - return nil, fmt.Errorf("unable to fetch user: %v", err) - } - - return &user, nil + return s.fetchUser(query, username) } // UserByExtraField finds a user by an extra field value. func (s *Storage) UserByExtraField(field, value string) (*model.User, error) { defer helper.ExecutionTime(time.Now(), fmt.Sprintf("[Storage:UserByExtraField] field=%s", field)) query := `SELECT - id, username, is_admin, theme, language, timezone, entry_direction + id, username, is_admin, theme, language, timezone, entry_direction, last_login_at, extra FROM users WHERE extra->$1=$2` - var user model.User - row := s.db.QueryRow(query, field, value) - err := row.Scan( + return s.fetchUser(query, field, value) +} + +func (s *Storage) fetchUser(query string, args ...interface{}) (*model.User, error) { + var extra hstore.Hstore + + user := model.NewUser() + err := s.db.QueryRow(query, args...).Scan( &user.ID, &user.Username, &user.IsAdmin, @@ -256,14 +218,23 @@ func (s *Storage) UserByExtraField(field, value string) (*model.User, error) { &user.Language, &user.Timezone, &user.EntryDirection, + &user.LastLoginAt, + &extra, ) + if err == sql.ErrNoRows { return nil, nil } else if err != nil { return nil, fmt.Errorf("unable to fetch user: %v", err) } - return &user, nil + for key, value := range extra.Map { + if value.Valid { + user.Extra[key] = value.String + } + } + + return user, nil } // RemoveUser deletes a user. @@ -290,8 +261,9 @@ func (s *Storage) RemoveUser(userID int64) error { // Users returns all users. func (s *Storage) Users() (model.Users, error) { defer helper.ExecutionTime(time.Now(), "[Storage:Users]") - query := `SELECT - id, username, is_admin, theme, language, timezone, last_login_at + query := ` + SELECT + id, username, is_admin, theme, language, timezone, entry_direction, last_login_at, extra FROM users ORDER BY username ASC` @@ -303,7 +275,8 @@ func (s *Storage) Users() (model.Users, error) { var users model.Users for rows.Next() { - var user model.User + var extra hstore.Hstore + user := model.NewUser() err := rows.Scan( &user.ID, &user.Username, @@ -311,14 +284,22 @@ func (s *Storage) Users() (model.Users, error) { &user.Theme, &user.Language, &user.Timezone, + &user.EntryDirection, &user.LastLoginAt, + &extra, ) if err != nil { return nil, fmt.Errorf("unable to fetch users row: %v", err) } - users = append(users, &user) + for key, value := range extra.Map { + if value.Valid { + user.Extra[key] = value.String + } + } + + users = append(users, user) } return users, nil