Copilot commented on code in PR #153:
URL:
https://github.com/apache/dubbo-go-pixiu-samples/pull/153#discussion_r3252600671
##########
dubbohttpproxy/server/http/server.go:
##########
@@ -36,41 +35,37 @@ func main() {
}
func user(w http.ResponseWriter, r *http.Request) {
- switch r.Method {
- case constant.Post:
- byts, err := io.ReadAll(r.Body)
- if err != nil {
- w.Write([]byte(err.Error()))
- }
- var name string
- var user User
- err = json.Unmarshal(byts, &name)
- if err != nil {
- w.Write([]byte(err.Error()))
- }
- _, ok := cache.Get(name)
- if ok {
- w.Header().Set(constant.HeaderKeyContextType,
constant.HeaderValueJsonUtf8)
- w.Write([]byte("{\"message\":\"data is exist\"}"))
- return
- }
- user.ID = randSeq(5)
- if cache.Add(&user) {
- b, _ := json.Marshal(&user)
- w.Header().Set(constant.HeaderKeyContextType,
constant.HeaderValueJsonUtf8)
- w.Write(b)
- return
- }
- w.Write(nil)
+ if r.Method != constant.Post {
+ w.WriteHeader(http.StatusMethodNotAllowed)
+ return
}
-}
-
-var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
-
-func randSeq(n int) string {
- b := make([]rune, n)
- for i := range b {
- b[i] = letters[rand.Intn(len(letters))]
+ byts, err := io.ReadAll(r.Body)
+ if err != nil {
+ w.WriteHeader(http.StatusBadRequest)
+ w.Write([]byte(err.Error()))
+ return
+ }
+ // Pixiu 的 dgp.filter.dubbo.http 把 generic invocation 的实参数组整体
+ // json.Marshal 后发出，body 形如 ["0001"]，所以反序列化目标是 []string。
+ var args []string
+ if err := json.Unmarshal(byts, &args); err != nil {
+ w.WriteHeader(http.StatusBadRequest)
+ w.Write([]byte(err.Error()))
+ return
+ }
+ if len(args) == 0 {
+ w.WriteHeader(http.StatusBadRequest)
+ w.Write([]byte("missing id argument"))
+ return
+ }
+ u, ok := cache.Get(args[0])
+ if !ok {
+ w.Header().Set(constant.HeaderKeyContextType,
constant.HeaderValueJsonUtf8)
+ w.WriteHeader(http.StatusNotFound)
+ w.Write([]byte(`{"message":"user not found"}`))
+ return
}
- return string(b)
+ b, _ := json.Marshal(u)
Review Comment:
`json.Marshal(u)` ignores the returned error. Even if it’s unlikely to fail
for `User`, handling the error and returning `500` (or at least not writing a
partial/empty response) would make the handler behavior more robust and
consistent with the other explicit error paths.
##########
dubbohttpproxy/server/http/user.go:
##########
@@ -50,7 +50,7 @@ func (db *UserDB) Add(u *User) bool {
db.lock.Lock()
defer db.lock.Unlock()
- db.cacheMap[u.Name] = u
+ db.cacheMap[u.ID] = u
return true
Review Comment:
`UserDB` now keys `cacheMap` by `u.ID`, but the struct comment and `Get(n
string)` parameter naming still imply the key is `Name`. Please update the
comments and parameter name to reflect that the cache key is the user ID, to
avoid future confusion/misuse.
##########
dubbohttpproxy/server/http/server.go:
##########
@@ -21,7 +21,6 @@ import (
"encoding/json"
"io"
"log"
- "math/rand"
"net/http"
)
Review Comment:
This file uses two separate `import` blocks (one for stdlib, one for
`constant`). `gofmt`/`goimports` typically merges them into a single block;
consider combining them to keep formatting consistent and tooling predictable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
