commit 7107fe7693d75c47525b0abe6ebe941f9c01490e
parent 514d418125ee1797512874dc70fffebd71d085df
Author: Jacob R. Edwards <jacob@jacobedwards.org>
Date: Mon, 5 Aug 2024 14:59:45 -0700
Update settings functions
- Settings are now validated
- Remove the settings/$setting endpoints to reduce complexity
- Replace the PUT settings endpoint with a PATCH endpoint
- Add settings.go for non-user-specific settings functions and data
- Replace backend Setting type (an interface{}) with a raw interface{}
to remove the need for casting in some other code. (Maybe generics
should have been used instead.)
Diffstat:
4 files changed, 121 insertions(+), 60 deletions(-)
diff --git a/cmd/api/main.go b/cmd/api/main.go
@@ -71,6 +71,7 @@ func initEnv(database string) (*Env, error) {
func setRoutes(env *Env, r *gin.RouterGroup, auth *jwt.GinJWTMiddleware) {
r.POST("/tokens", auth.LoginHandler)
r.GET("/tokens", auth.RefreshHandler)
+ r.GET("/settings", env.GetSettings)
users := r.Group("/users")
users.GET("/:user", env.GetUser)
@@ -83,10 +84,8 @@ func setAuthenticatedRoutes(env *Env, r *gin.RouterGroup) {
r.DELETE("/users/:user", env.DeleteUser)
user := r.Group("/users/:user/")
- user.PUT("/settings", env.SetUserSettings)
- user.PUT("/settings/:setting", env.SetUserSetting)
+ user.PATCH("/settings", env.UpdateUserSettings)
user.GET("/settings", env.GetUserSettings)
- user.GET("/settings/:setting", env.GetUserSetting)
fp := r.Group("/floorplans/:user")
fp.GET("", env.GetFloorplans)
diff --git a/cmd/api/settings.go b/cmd/api/settings.go
@@ -0,0 +1,70 @@
+package main
+
+import (
+ "net/mail"
+ "errors"
+
+ "github.com/gin-gonic/gin"
+)
+
+type settingDef struct {
+ primitive string
+ valid func(interface{}) (bool, error)
+ defaultValue interface{}
+ // min *int, max *int // for integer types, min and max value, for string, length
+ memo string
+}
+
+var userSettings = map[string]settingDef {
+ "email": { "string", validateEmail, nil, "email address", },
+}
+
+func (e *Env) GetSettings(c *gin.Context) {
+ settings := make(gin.H)
+ for name, def := range userSettings {
+ settings[name] = gin.H{ "type": def.primitive, "description": def.memo }
+ }
+ Respond(c, 200, settings)
+}
+
+func splitInvalidSettings(settings map[string]interface{}) (purged map[string]interface{}, err error) {
+ purged = make(map[string]interface{})
+ for k, v := range settings {
+ valid, err := validateSetting(k, v)
+ if err != nil {
+ return nil, err
+ }
+ if !valid {
+ purged[k] = v
+ }
+ }
+
+ // Doing this afterwords incase validateSetting returns an
+ // error so settings won't be altered on error
+ for k := range purged {
+ settings[k] = nil
+ }
+ return purged, nil
+}
+
+func validateSetting(name string, value interface{}) (bool, error) {
+ def, exists := userSettings[name]
+ if !exists {
+ return false, errors.New("Setting does not exist")
+ }
+
+ return def.valid(value)
+}
+
+func validateEmail(value interface{}) (bool, error) {
+ s, ok := value.(string)
+ if !ok {
+ return false, errors.New("Invalid type")
+ }
+
+ _, err := mail.ParseAddress(s)
+ if err != nil {
+ return false, err
+ }
+ return true, nil
+}
diff --git a/cmd/api/users.go b/cmd/api/users.go
@@ -1,6 +1,7 @@
package main
import (
+ "log"
"net/http"
"github.com/gin-gonic/gin"
@@ -62,47 +63,64 @@ func (e *Env) GetUserSettings(c *gin.Context) {
RespondError(c, 400, "Unable to get settings: %s", err.Error())
return
}
-
- Respond(c, http.StatusOK, settings)
+ invalids, err := splitInvalidSettings(settings)
+ if err != nil {
+ RespondError(c, 500, "Unable to validate settings")
+ } else {
+ for name, value := range invalids {
+ log.Printf("WARNING: Setting has invalid value (%q = %v)", name, value)
+ }
+ Respond(c, http.StatusOK, settings)
+ }
}
-func (e *Env) GetUserSetting(c *gin.Context) {
+func (e *Env) UpdateUserSettings(c *gin.Context) {
user := c.Param("user")
- name := c.Param("setting")
- if user == "" || name == "" {
- RespondError(c, http.StatusNotFound, "Username or setting not given")
+ if user == "" {
+ RespondError(c, http.StatusNotFound, "No username given")
return
}
- setting, err := e.backend.GetUserSettings(nil, user, name)
- if err != nil {
- RespondError(c, 400, "Unable to get settings: %s", err.Error())
+ patches := make([]Patch, 16)
+ if err := c.ShouldBind(&patches); err != nil {
+ RespondError(c, 400, "Unable to read patchset")
return
}
- Respond(c, http.StatusOK, setting)
-}
+ tx, err := e.backend.DB.Begin()
+ if err != nil {
+ RespondError(c, 500, "Unable to begin transaction")
+ return
+ }
+ defer tx.Rollback()
-func (e *Env) SetUserSettings(c *gin.Context) {
- user := c.Param("user")
- if user == "" {
- RespondError(c, http.StatusNotFound, "No username given")
+ settings, err := e.backend.GetUserSettings(tx, user)
+ if err != nil {
+ RespondError(c, 400, "Unable to get settings")
return
}
- settings := make(map[string]backend.Setting)
- if err := c.ShouldBind(&settings); err != nil {
- RespondError(c, 400, "Unable to read settings: %s", err.Error())
+ if err = applyPatchset(settings, patches); err != nil {
+ RespondError(c, 400, "Unable to apply patches")
return
}
- tx, err := e.backend.DB.Begin()
+ invalids, err := splitInvalidSettings(settings)
if err != nil {
- RespondError(c, 400, "Unable to begin transaction: %s", err.Error())
+ RespondError(c, 400, "Unable to validate setting: %s", err.Error())
+ return
+ }
+ if len(invalids) > 0 {
+ var one string
+ // Is there a better way?
+ for one = range(invalids) {
+ break
+ }
+ RespondError(c, 400, "%d invalid settings, including %q", len(invalids), one)
return
}
+
if _, err := e.backend.DeleteUserSettings(tx, user); err != nil {
- tx.Rollback()
RespondError(c, 400, "Unable to truncate settings: %s", err.Error())
return
}
@@ -117,27 +135,3 @@ func (e *Env) SetUserSettings(c *gin.Context) {
Respond(c, http.StatusOK, settings)
}
-
-func (e *Env) SetUserSetting(c *gin.Context) {
- user := c.Param("user")
- name := c.Param("setting")
- if user == "" || name == "" {
- RespondError(c, http.StatusNotFound, "Username or setting name not given")
- return
- }
-
- var value interface{}
- if err := c.ShouldBind(&value); err != nil {
- RespondError(c, 400, "Unable to get value: %s", err.Error())
- return
- }
-
- setting := make(map[string]backend.Setting)
- setting[name] = value
- if err := e.backend.UpdateUserSettings(nil, user, setting);
- err != nil {
- RespondError(c, 400, "Unable to update %q: %s", setting, err.Error())
- }
-
- Respond(c, 200, setting)
-}
diff --git a/internal/backend/user.go b/internal/backend/user.go
@@ -13,8 +13,6 @@ type User struct {
hash string
}
-type Setting interface{}
-
type rawSetting struct {
username *string
name *string
@@ -81,7 +79,7 @@ func (e *Env) LoginUser(username string, password string) (User, error) {
}
-func (e *Env) UpdateUserSettings(tx *sql.Tx, username string, settings map[string]Setting) error {
+func (e *Env) UpdateUserSettings(tx *sql.Tx, username string, settings map[string]interface{}) error {
stmt, err := e.CacheStmt("set_user_settings", `INSERT INTO user_settings VALUES ($1, $2, $3, $4, $5)
ON CONFLICT (username, name) DO UPDATE
SET (strval, numval, boolval) = (EXCLUDED.strval, EXCLUDED.numval, EXCLUDED.boolval)`)
@@ -104,7 +102,7 @@ func (e *Env) UpdateUserSettings(tx *sql.Tx, username string, settings map[strin
return nil
}
-func (e *Env) GetUserSettings(tx *sql.Tx, username string, names ...string) (map[string]Setting, error) {
+func (e *Env) GetUserSettings(tx *sql.Tx, username string, names ...string) (map[string]interface{}, error) {
stmt, err := e.CacheStmt("get_user_settings", `SELECT * FROM user_settings WHERE
user_settings.username = $1 AND
(ARRAY_LENGTH($2::varchar[], 1) IS NULL OR user_settings.name = ANY ($2))`)
@@ -124,10 +122,10 @@ func (e *Env) GetUserSettings(tx *sql.Tx, username string, names ...string) (map
return collectSettings(rows)
}
-func (e *Env) DeleteUserSettings(tx *sql.Tx, username string, names ...string) (map[string]Setting, error) {
- var deleted map[string]Setting
+func (e *Env) DeleteUserSettings(tx *sql.Tx, username string, names ...string) (map[string]interface{}, error) {
+ var deleted map[string]interface{}
stmt, err := e.CacheStmt("del_user_settings", `DELETE FROM user_settings WHERE user_settings.username = $1 AND
- (ARRAY_LENGTH($2::string[], 1) IS NULL OR user_settings.name = ANY ($2)) RETURNING`)
+ (ARRAY_LENGTH($2::varchar[], 1) IS NULL OR user_settings.name = ANY ($2)) RETURNING *`)
if err != nil {
return deleted, err
}
@@ -136,14 +134,14 @@ func (e *Env) DeleteUserSettings(tx *sql.Tx, username string, names ...string) (
tx.Stmt(stmt)
}
- rows, err := stmt.Query(username, names)
+ rows, err := stmt.Query(username, pq.Array(names))
if err != nil {
return deleted, err
}
return collectSettings(rows)
}
-func toRawSetting(username string, name string, setting Setting) *rawSetting {
+func toRawSetting(username string, name string, setting interface{}) *rawSetting {
var b *bool
var i *int
var s *string
@@ -168,10 +166,10 @@ func toRawSetting(username string, name string, setting Setting) *rawSetting {
}
}
-func collectSettings(rows *sql.Rows) (map[string]Setting, error) {
- settings := make(map[string]Setting)
+func collectSettings(rows *sql.Rows) (map[string]interface{}, error) {
+ settings := make(map[string]interface{})
for rows.Next() {
- var setting Setting
+ var setting interface{}
var raw rawSetting
err := rows.Scan(&raw.username, &raw.name, &raw.strval, &raw.numval, &raw.boolval)
if err != nil {
