API feedback (#341)

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip
This commit is contained in:
Tiger Wang (王豫) 2022-07-07 21:54:13 -04:00 committed by GitHub
parent 192e14a2dc
commit 71bb7432cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 128 additions and 31 deletions

View File

@ -13,6 +13,15 @@ type ServerAppListCollection struct {
Version string `json:"version"`
}
// @tiger - 对于用于出参的数据结构,静态信息(例如 title
// 动态信息(例如 state、query_count应该划分到不同的数据结构中
//
// 这样的好处是
// 1 - 多次获取动态信息时可以减少出参复杂度,因为静态信息只获取一次就好
// 2 - 在未来的迭代中,可以降低维护成本(所有字段都展开放在一个层级维护成本略高)
//
// 另外,一些针对性字段,例如 Docker 相关的,可以用 map 来保存。
// 这样在未来增加多态 App例如 Snap不需要维护多个结构或者一个结构保存不必要的字段
type ServerAppList struct {
Id uint `gorm:"column:id;primary_key" json:"id"`
Title string `json:"title"`

View File

@ -18,7 +18,7 @@ type CategoryList struct {
//CreatedAt time.Time `json:"created_at"`
//
//UpdatedAt time.Time `json:"updated_at"`
Font string `json:"font"`
Font string `json:"font"` // @tiger - 如果这个和前端有关,应该不属于后端的出参范围,而是前端去界定
Name string `json:"name"`
Count uint `json:"count"`
Count uint `json:"count"` // @tiger - count 属于动态信息,应该单独放在一个出参结构中(原因见另外一个关于 静态/动态 出参的注释)
}

View File

@ -14,5 +14,5 @@ type DockerStatsModel struct {
Icon string `json:"icon"`
Title string `json:"title"`
Data interface{} `json:"data"`
Pre interface{} `json:"pre"`
Pre interface{} `json:"pre"` // @tiger - pre 不知道什么意思,可以提高一下描述性
}

View File

@ -1,3 +1,5 @@
// 这是一个用来反馈 API 设计的 PR不要 merge
package route
import (
@ -23,6 +25,8 @@ func InitRouter() *gin.Engine {
r.Use(middleware.WriteLog())
r.Use(gzip.Gzip(gzip.DefaultCompression))
gin.SetMode(config.ServerInfo.RunMode)
// @tiger - 为了方便未来的模块化迭代,前端输出需要独立端口,不要和 API 端口公用。
r.StaticFS("/ui", http.FS(web.Static))
r.GET("/", WebUIHome)
// r.StaticFS("/assets", http.Dir("./static/assets"))
@ -33,17 +37,33 @@ func InitRouter() *gin.Engine {
r.POST("/v1/user/register/:key", v1.PostUserRegister)
r.POST("/v1/user/login", v1.PostUserLogin) //
// @tiger - 如果遵循 RESTful 规范name 本身并不是资源,而是属性;资源是 user
// 所以正规的方法是 改成 /v1/users 然后返回所有的 user 对象,具体 name 由前端自行抽取
// 不正规的方式是 改成 /v1/users/names假定 name 也是资源
r.GET("/v1/user/all/name", v1.GetUserAllUserName)
r.GET("/v1/sys/init/check", v1.GetSystemInitCheck)
r.GET("/v1/guide/check", v1.GetGuideCheck)
r.GET("/v1/debug", v1.GetSystemConfigDebug)
// @tiger - 1不要把同一个词汇按单词来分割。2同领域的 API 应该放在同路径下。
r.GET("/v1/sys/init/check", v1.GetSystemInitCheck) // 这里改成 /v1/sys/init_check
r.GET("/v1/guide/check", v1.GetGuideCheck) // 这里改成 /v1/sys/guide_check
r.GET("/v1/debug", v1.GetSystemConfigDebug) // 这里改成 /v1/sys/debug
// @tiger - 如果遵循 RESTful avatar 本身并不是资源,而是属性;资源是 user
// 所以正规的方法是 改成 /v1/user/:id 然后返回 user 对象,具体 avatar 由前端自行抽取
// 不正规的方式是 改成 /v1/user/:id/avatar假定 avatar 也是资源
r.GET("/v1/user/avatar/:id", v1.GetUserAvatar)
// @tiger - 如果遵循 RESTful image 本身并不是资源,而是属性;资源是 user
// 所以正规的方法是 改成 /v1/user/:id 然后返回 user 对象,具体 image 由前端自行抽取
// 不正规的方式是 改成 /v1/user/:id/image假定 image 也是资源
r.GET("/v1/user/image", v1.GetUserImage)
// @tiger - 不要把同一个词汇按单词来分割,改成 /v1/sys/socket_port
r.GET("/v1/sys/socket/port", v1.GetSystemSocketPort)
// @tiger - nice-to-have开源项目应该删除所有注释代码增加代码整洁性。或者增加注释说明
//r.POST("/v1/user/refresh/token", v1.PostUserRefreshToken)
v1Group := r.Group("/v1")
v1Group.Use(jwt2.JWT())
@ -51,49 +71,86 @@ func InitRouter() *gin.Engine {
v1UserGroup := v1Group.Group("/user")
v1UserGroup.Use()
{
// @tiger - info 一词名没有指定性,容易产生困扰。改成 /current
v1UserGroup.GET("/info", v1.GetUserInfo)
// @tiger - RESTful 规范下所有对 user 的写操作,都应该 POST /v1/user/:id
// - 不需要每个更改的属性建一个 API
v1UserGroup.PUT("/username", v1.PutUserName)
v1UserGroup.PUT("/password", v1.PutUserPwd)
v1UserGroup.PUT("/nick", v1.PutUserNick)
v1UserGroup.PUT("/desc", v1.PutUserDesc)
v1UserGroup.PUT("/nick", v1.PutUserNick) // 改成 /nickname
v1UserGroup.PUT("/desc", v1.PutUserDesc) // 改成 /description
// @tiger - RESTful 规范建议是 GET /v1/users/?username=xxxx
// 这是一个查询 API返回一个 users 数组(即使 username 是唯一的)
// 之所以不用 /v1/user/:username 是因为和 /v1/user/:id 路由冲突
//
// 当前这个设计的问题是GET 不应该同时接收 request body。
// GET 方法应该只接收 URL 参数
v1UserGroup.GET("/info", v1.GetUserInfoByUserName)
// @tiger - 改成 /user/current/custom/... 和上面的 current 对应
// 如果未来想获得其它用户的 custom 数据,可以用 /v1/user/:id/custom/... 来保持统一
v1UserGroup.GET("/custom/:key", v1.GetUserCustomConf)
v1UserGroup.POST("/custom/:key", v1.PostUserCustomConf)
v1UserGroup.DELETE("/custom/:key", v1.DeleteUserCustomConf)
// @tiger - 下面这两个 API 从感知上很难区分。
// 如果前者是负责上传,后者负责指定的话,那么
// 前者应该用一个统一的和目的无关的用户文件上传 API而不是针对 image file 的
v1UserGroup.POST("/upload/image/:key", v1.PostUserUploadImage)
v1UserGroup.POST("/file/image/:key", v1.PostUserFileImage)
v1UserGroup.DELETE("/image", v1.DeleteUserImage)
// @tiger - 应该用上面提到的统一的文件上传 API 先上传头像文件,然后
// 用类似上面第二个 API 的方式指定头像文件。这样整体 API 体验更加统一。
v1UserGroup.PUT("/avatar", v1.PutUserAvatar)
v1UserGroup.GET("/avatar", v1.GetUserAvatar)
// @tiger - 删除用户直接用 DELETE /v1/user/:id不需要在路径中用谓语
v1UserGroup.DELETE("/delete/:id", v1.DeleteUser)
}
v1AppGroup := v1Group.Group("/app")
v1AppGroup.Use()
{
// @tiger - 按照 RESTFul 规范,改成 GET /v1/apps?installed=true
//获取我的已安装的列表
v1AppGroup.GET("/my/list", v1.MyAppList)
//
// @tiger - 按照 RESTFul 规范,改成 GET /v1/apps/usage
v1AppGroup.GET("/usage", v1.AppUsageList)
// @tiger - 按照 RESTFul 规范,改成 GET /v1/app/:id
//app详情
v1AppGroup.GET("/appinfo/:id", v1.AppInfo)
// @tiger - 按照 RESTFul 规范,改成 GET /v1/apps?installed=false
//获取未安装的列表
v1AppGroup.GET("/list", v1.AppList)
// @tiger - 这个信息和应用无关,应该挪到 /v1/sys/port/avaiable
//获取端口
v1AppGroup.GET("/port", v1.GetPort)
// @tiger - RESTFul 路径中尽量不要有动词,同时这个信息和应用无关,应该挪到 /v1/sys/port/:port
//检查端口
v1AppGroup.GET("/check/:port", v1.PortCheck)
// @tiger - 应用分类和应用不是一类资源,应该挪到 GET /v1/app_categories
v1AppGroup.GET("/category", v1.CategoryList)
// @tiger - Docker Terminal 和应用不是一类资源,应该挪到 GET /v1/container/:id/terminal
// 另外这个返回的不是一个 HTTP 响应,应该返回一个 wss://... 的 URL给前端由前端另行处理
v1AppGroup.GET("/terminal/:id", v1.DockerTerminal)
// @tiger - 所有跟 Docker 有关的 API应该挪到 /v1/container 下
//app容器详情
v1AppGroup.GET("/info/:id", v1.ContainerInfo)
v1AppGroup.GET("/info/:id", v1.ContainerInfo) // 改成 GET /v1/container/:id
//app容器日志
v1AppGroup.GET("/logs/:id", v1.ContainerLog)
v1AppGroup.GET("/logs/:id", v1.ContainerLog) // 改成 GET /v1/container/:id/log
//暂停或启动容器
v1AppGroup.PUT("/state/:id", v1.ChangAppState)
v1AppGroup.PUT("/state/:id", v1.ChangAppState) // 改成 PUT /v1/container/:id/state
//安装app
v1AppGroup.POST("/install", v1.InstallApp)
//卸载app
@ -104,8 +161,13 @@ func InitRouter() *gin.Engine {
v1AppGroup.PUT("/update/:id/setting", v1.UpdateSetting)
//获取可能新数据
v1AppGroup.GET("/update/:id/info", v1.ContainerUpdateInfo)
// @tiger - rely -> dependency - 依赖是什么意思?
v1AppGroup.GET("/rely/:id/info", v1.ContainerRelyInfo)
// @tiger - 按照 RESTFul 规范,改成 GET /v1/container/:id/config
v1AppGroup.GET("/install/config", v1.GetDockerInstallConfig)
v1AppGroup.PUT("/update/:id", v1.PutAppUpdate)
v1AppGroup.POST("/share", v1.ShareAppFile)
}

View File

@ -83,6 +83,7 @@ func GetPort(c *gin.Context) {
p, _ = port2.GetAvailablePort(t)
ok = !port2.IsPortAvailable(p, t)
}
// @tiger 这里最好封装成 {'port': ...} 的形式,来体现出参的上下文
c.JSON(http.StatusOK, &model.Result{Success: common_err.SUCCESS, Message: common_err.GetMsg(common_err.SUCCESS), Data: p})
}
@ -117,8 +118,8 @@ func MyAppList(c *gin.Context) {
position, _ := strconv.ParseBool(c.DefaultQuery("position", "true"))
list, unTranslation := service.MyService.App().GetMyList(index, size, position)
data := make(map[string]interface{}, 2)
data["list"] = list
data["local"] = unTranslation
data["list"] = list // @tiger - list 不清楚是什么意思,可以提高一下描述性
data["local"] = unTranslation // @tiger - local 不清楚是什么意思,可以提高一下描述性
c.JSON(http.StatusOK, &model.Result{Success: common_err.SUCCESS, Message: common_err.GetMsg(common_err.SUCCESS), Data: data})
}
@ -131,6 +132,7 @@ func MyAppList(c *gin.Context) {
// @Success 200 {string} string "ok"
// @Router /app/usage [get]
func AppUsageList(c *gin.Context) {
// @tiger - 关于出参的问题,见 GetHardwareUsageSteam - 另外 steam 是不是应该为 stream?
list := service.MyService.App().GetHardwareUsage()
c.JSON(http.StatusOK, &model.Result{Success: common_err.SUCCESS, Message: common_err.GetMsg(common_err.SUCCESS), Data: list})
}

View File

@ -708,7 +708,7 @@ func UnInstallApp(c *gin.Context) {
// @Router /app/state/{id} [put]
func ChangAppState(c *gin.Context) {
appId := c.Param("id")
state := c.DefaultPostForm("state", "stop")
state := c.DefaultPostForm("state", "stop") // @tiger - 应该用 JSON 形式
var err error
if state == "stop" {
err = service.MyService.Docker().DockerContainerStop(appId)
@ -727,6 +727,8 @@ func ChangAppState(c *gin.Context) {
c.JSON(http.StatusOK, model.Result{Success: common_err.ERROR, Message: common_err.GetMsg(common_err.ERROR), Data: err.Error()})
return
}
// @tiger - 用 {'state': ...} 来体现出参上下文
c.JSON(http.StatusOK, model.Result{Success: common_err.SUCCESS, Message: common_err.GetMsg(common_err.SUCCESS), Data: info.State})
}
@ -1006,6 +1008,8 @@ func PutAppUpdate(c *gin.Context) {
// @Router /app/info/{id} [get]
func ContainerInfo(c *gin.Context) {
appId := c.Param("id")
// @tiger - 作为最佳实践,不应该直接把数据库的信息返回,来避免未来数据库结构上的迭代带来的新字段
appInfo := service.MyService.App().GetAppDBInfo(appId)
containerInfo, _ := service.MyService.Docker().DockerContainerStats(appId)
var cpuModel = "arm"
@ -1027,13 +1031,13 @@ func ContainerInfo(c *gin.Context) {
Status string `json:"status"`
StartedAt string `json:"started_at"`
CPUShares int64 `json:"cpu_shares"`
Memory int64 `json:"memory"`
Restart string `json:"restart"`
Memory int64 `json:"memory"` // @tiger - 改成 total_memory方便以后增加 free_memory 之类的字段
Restart string `json:"restart"` // @tiger - 改成 restart_policy?
}{Status: info.State.Status, StartedAt: info.State.StartedAt, CPUShares: info.HostConfig.CPUShares, Memory: info.HostConfig.Memory >> 20, Restart: info.HostConfig.RestartPolicy.Name}
data := make(map[string]interface{}, 5)
data["app"] = appInfo
data["cpu"] = cpuModel
data["memory"] = service.MyService.System().GetMemInfo()["total"]
data["app"] = appInfo // @tiget - 最佳实践是,返回 appid然后具体的 app 信息由前端另行获取
data["cpu"] = cpuModel // @tiger - 改成 arch
data["memory"] = service.MyService.System().GetMemInfo()["total"] // @tiger - 改成 total_memory方便以后增加 free_memory 之类的字段
data["container"] = json2.RawMessage(containerInfo)
data["info"] = con
c.JSON(http.StatusOK, model.Result{Success: common_err.SUCCESS, Message: common_err.GetMsg(common_err.SUCCESS), Data: data})

View File

@ -470,7 +470,7 @@ func GetSystemSocketPort(c *gin.Context) {
model.Result{
Success: common_err.SUCCESS,
Message: common_err.GetMsg(common_err.SUCCESS),
Data: config.ServerInfo.SocketPort,
Data: config.ServerInfo.SocketPort, // @tiger 这里最好封装成 {'port': ...} 的形式,来体现出参的上下文
})
}

View File

@ -31,8 +31,11 @@ import (
func PostUserRegister(c *gin.Context) {
json := make(map[string]string)
c.BindJSON(&json)
// @tiger - user_name 改成 username
username := json["user_name"]
pwd := json["password"]
key := c.Param("key")
if _, ok := service.UserRegisterHash[key]; !ok {
c.JSON(http.StatusOK,
@ -86,6 +89,8 @@ func PostUserLogin(c *gin.Context) {
c.BindJSON(&json)
username := json["username"]
// @tiger - 字段命名要一直,在注册的时候如果用 password这里也要用 password
pwd := json["pwd"]
//check params is empty
if len(username) == 0 || len(pwd) == 0 {
@ -114,6 +119,8 @@ func PostUserLogin(c *gin.Context) {
data := make(map[string]interface{}, 2)
user.Password = ""
data["token"] = token
// @tiger - 不建议直接透传数据库对象,而是适配到用于 API 输出的 model 对象
data["user"] = user
c.JSON(http.StatusOK,
@ -175,6 +182,8 @@ func GetUserAvatar(c *gin.Context) {
if user.Id > 0 {
path = user.Avatar
}
// @tiger - RESTful 规范下不应该返回文件本身内容而是返回文件的静态URL由前端去解析
c.File(path)
}
@ -311,13 +320,14 @@ func GetUserInfo(c *gin.Context) {
user := service.MyService.User().GetUserInfoById(id)
//*****
// @tiger - 应该和 PostUserLogin 中的 user 对象一致。而不是重构一系列字段。
var u = make(map[string]string, 5)
u["user_name"] = user.UserName
u["head"] = user.Avatar
u["user_name"] = user.UserName // 改成 username
u["head"] = user.Avatar // 应该和 /v1/user/avatar/:id 一致,改成 avatar
u["email"] = user.Email
u["description"] = user.NickName
u["nick_name"] = user.NickName
u["id"] = strconv.Itoa(user.Id)
u["nick_name"] = user.NickName // 改成 nickname
u["id"] = strconv.Itoa(user.Id) // (nice-to-have) 最佳实践是用随机字符来代表 ID。顺序数字有可预测性
//**
@ -337,6 +347,9 @@ func GetUserInfo(c *gin.Context) {
// @Router /user/info [get]
func GetUserInfoByUserName(c *gin.Context) {
json := make(map[string]string)
// @tiger 当前这个设计的问题是GET 不应该同时接收 request body。
// GET 方法应该只接收 URL 参数
c.BindJSON(&json)
userName := json["user_name"]
if len(userName) == 0 {
@ -359,7 +372,7 @@ func GetUserInfoByUserName(c *gin.Context) {
}
/**
* @description: get all user name
* @description: get all usernames
* @method:GET
* @router:/user/all/name
*/
@ -399,6 +412,7 @@ func GetUserCustomConf(c *gin.Context) {
return
}
filePath := config.AppInfo.UserDataPath + "/" + id + "/" + name + ".json"
data := file.ReadFullFile(filePath)
if !gjson.ValidBytes(data) {
c.JSON(http.StatusOK, model.Result{Success: common_err.SUCCESS, Message: common_err.GetMsg(common_err.SUCCESS), Data: string(data)})
@ -453,7 +467,7 @@ func DeleteUserCustomConf(c *gin.Context) {
return
}
filePath := config.AppInfo.UserDataPath + "/" + strconv.Itoa(user.Id) + "/" + name + ".json"
os.Remove(filePath)
os.Remove(filePath) // @tiger - 这里万一无法实际删除,后面仍然有可能返回成功
c.JSON(http.StatusOK, model.Result{Success: common_err.SUCCESS, Message: common_err.GetMsg(common_err.SUCCESS)})
}
@ -583,6 +597,8 @@ func GetUserImage(c *gin.Context) {
defer fileTmp.Close()
fileName := path.Base(filePath)
// @tiger - RESTful 规范下不应该返回文件本身内容而是返回文件的静态URL由前端去解析
c.Header("Content-Disposition", "attachment; filename*=utf-8''"+url2.PathEscape(fileName))
c.File(filePath)
}

View File

@ -438,6 +438,8 @@ func (a *appStruct) GetHardwareUsageSteam() {
dockerStats.Icon = v.Labels["icon"]
dockerStats.Title = strings.ReplaceAll(v.Names[0], "/", "")
// @tiger - 不建议直接把依赖的数据结构封装返回。
// 如果依赖的数据结构有变化,应该在这里适配或者保存,这样更加对客户端负责
temp.Store(v.ID, dockerStats)
if i == 99 {
stats.Body.Close()

View File

@ -67,13 +67,13 @@ type MyAppList struct {
Index string `json:"index"`
//Order string `json:"order"`
Port string `json:"port"`
UpTime string `json:"up_time"`
UpTime string `json:"up_time"` // @tiger - 如果是安装时间,应该写 installed_at。
Slogan string `json:"slogan"`
Type string `json:"type"`
//Rely model.MapStrings `json:"rely"` //[{"mysql":"id"},{"mysql":"id"}]
Image string `json:"image"`
Volumes string `json:"volumes"`
NewVersion bool `json:"new_version"`
NewVersion bool `json:"new_version"` // @tiger - 无法从词面理解含义,感觉可以更加有描述性
Host string `json:"host"`
Protocol string `json:"protocol"`
}

View File

@ -15,11 +15,11 @@ import "time"
//Soon to be removed
type UserDBModel struct {
Id int `gorm:"column:id;primary_key" json:"id"`
UserName string `json:"user_name"`
UserName string `json:"user_name"` // @tiger - user_name 改 username
Password string `json:"password,omitempty"`
Role string `json:"role"`
Email string `json:"email"`
NickName string `json:"nick_name"`
NickName string `json:"nick_name"` // @tiger - nick_name 改 nickname
Avatar string `json:"avatar"`
Description string `json:"description"`
CreatedAt time.Time `gorm:"<-:create;autoCreateTime" json:"created_at,omitempty"`

View File

@ -35,6 +35,8 @@ func (r *relyService) Create(rely model2.RelyDBModel) {
func (r *relyService) GetInfo(id string) model2.RelyDBModel {
var m model2.RelyDBModel
r.db.Where("custom_id = ?", id).First(&m)
// @tiger - 作为出参不应该直接返回数据库内的格式(见类似问题的注释)
return m
}