Skip to content

Commit

Permalink
Refactor HTTP response (#1150)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lucifercr07
lucifercr07 authored Oct 19, 2024
1 parent c3400e8 commit 91f3d2b
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 52 deletions.
40 changes: 39 additions & 1 deletion integration_tests/commands/http/json_arrpop_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func TestJSONARRPOP(t *testing.T) {
arrayAtRoot := []interface{}{0, 1, 2, 3}
nestedArray := map[string]interface{}{"a": 2, "b": []interface{}{0, 1, 2, 3}}
arrayWithinArray := map[string]interface{}{"a": 2, "b": []interface{}{0, 1, 2, []interface{}{3, 4, 5}}}

simpleJson := map[string]interface{}{"a": 2}
// Deleting the used keys
exec.FireCommand(HTTPCommand{
Command: "DEL",
Expand Down Expand Up @@ -122,6 +122,44 @@ func TestJSONARRPOP(t *testing.T) {
},
expected: []interface{}{"OK", "ERR invalid JSONPath"},
},
{
name: "key doesn't exist error",
commands: []HTTPCommand{
{
Command: "JSON.ARRPOP",
Body: map[string]interface{}{"key": "doc_new"},
},
},
expected: []interface{}{"ERR could not perform this operation on a key that doesn't exist"},
},
{
name: "arr pop on wrong key type",
commands: []HTTPCommand{
{
Command: "SET",
Body: map[string]interface{}{"key": "doc_new", "value": "v1"},
},
{
Command: "JSON.ARRPOP",
Body: map[string]interface{}{"key": "doc_new"},
},
},
expected: []interface{}{"OK", "ERR Existing key has wrong Dice type"},
},
{
name: "nil response for arr pop",
commands: []HTTPCommand{
{
Command: "JSON.SET",
Body: map[string]interface{}{"key": "doc", "path": "$", "json": simpleJson},
},
{
Command: "JSON.ARRPOP",
Body: map[string]interface{}{"key": "doc", "path": "$.a", "index": "1"},
},
},
expected: []interface{}{"OK", []interface{}{nil}},
},
}

for _, tc := range testCases {
Expand Down
150 changes: 99 additions & 51 deletions internal/server/httpServer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"sync"
"time"

"github.com/dicedb/dice/internal/eval"

"github.com/dicedb/dice/config"
"github.com/dicedb/dice/internal/clientio"
"github.com/dicedb/dice/internal/cmd"
Expand All @@ -22,14 +24,15 @@ import (
"github.com/dicedb/dice/internal/shard"
)

const Abort = "ABORT"
const (
Abort = "ABORT"
stringNil = "(nil)"
)

var unimplementedCommands = map[string]bool{
"Q.UNWATCH": true,
}

const stringNil = "(nil)"

type HTTPServer struct {
shardManager *shard.ShardManager
ioChan chan *ops.StoreResponse
Expand Down Expand Up @@ -326,33 +329,23 @@ func (s *HTTPServer) writeQWatchResponse(writer http.ResponseWriter, response in
}

func (s *HTTPServer) writeResponse(writer http.ResponseWriter, result *ops.StoreResponse, diceDBCmd *cmd.DiceDBCmd) {
var (
responseValue interface{}
err error
httpResponse utils.HTTPResponse
isDiceErr bool
)

// Check if the command is migrated, if it is we use EvalResponse values
// else we use RESPParser to decode the response
_, ok := WorkerCmdsMeta[diceDBCmd.Cmd]
var rp *clientio.RESPParser

var responseValue interface{}
var isDiceErr bool = false
var httpResponse utils.HTTPResponse
// TODO: Remove this conditional check and if (true) condition when all commands are migrated
if !ok {
var err error
if result.EvalResponse.Error != nil {
rp = clientio.NewRESPParser(bytes.NewBuffer([]byte(result.EvalResponse.Error.Error())))
} else {
rp = clientio.NewRESPParser(bytes.NewBuffer(result.EvalResponse.Result.([]byte)))
}

res, err := rp.DecodeOne()
responseValue = replaceNilInInterface(res)
responseValue, err = decodeEvalResponse(result.EvalResponse)
if err != nil {
s.logger.Error("Error decoding response", "error", err)
httpResponse := utils.HTTPResponse{Status: utils.HTTPStatusError, Data: "Internal Server Error"}
responseJSON, _ := json.Marshal(httpResponse)
writer.Header().Set("Content-Type", "application/json")
writer.WriteHeader(http.StatusInternalServerError) // Set HTTP status code to 500
_, err = writer.Write(responseJSON)
if err != nil {
s.logger.Error("Error writing response", "error", err)
}
httpResponse = utils.HTTPResponse{Status: utils.HTTPStatusError, Data: "Internal Server Error"}
writeJSONResponse(writer, httpResponse, http.StatusInternalServerError)
return
}
} else {
Expand All @@ -364,7 +357,55 @@ func (s *HTTPServer) writeResponse(writer http.ResponseWriter, result *ops.Store
}
}

// func HandlePredefinedResponse(response interface{}) []byte {
// Create the HTTP response
httpResponse = createHTTPResponse(responseValue)
if isDiceErr {
httpResponse.Status = utils.HTTPStatusError
} else {
httpResponse.Status = utils.HTTPStatusSuccess
}

// Write the response back to the client
writeJSONResponse(writer, httpResponse, http.StatusOK)
}

// Helper function to decode EvalResponse based on the error or result
func decodeEvalResponse(evalResp *eval.EvalResponse) (interface{}, error) {
var rp *clientio.RESPParser

if evalResp.Error != nil {
rp = clientio.NewRESPParser(bytes.NewBuffer([]byte(evalResp.Error.Error())))
} else {
rp = clientio.NewRESPParser(bytes.NewBuffer(evalResp.Result.([]byte)))
}

res, err := rp.DecodeOne()
if err != nil {
return nil, err
}

return replaceNilInInterface(res), nil
}

// Helper function to write the JSON response
func writeJSONResponse(writer http.ResponseWriter, response utils.HTTPResponse, statusCode int) {
writer.Header().Set("Content-Type", "application/json")
writer.WriteHeader(statusCode)

responseJSON, err := json.Marshal(response)
if err != nil {
slog.Error("Error marshaling response", "error", err)
http.Error(writer, "Internal Server Error", http.StatusInternalServerError)
return
}

_, err = writer.Write(responseJSON)
if err != nil {
slog.Error("Error writing response", "error", err)
}
}

func createHTTPResponse(responseValue interface{}) utils.HTTPResponse {
respArr := []string{
"(nil)", // Represents a RESP Nil Bulk String, which indicates a null value.
"OK", // Represents a RESP Simple String with value "OK".
Expand All @@ -376,36 +417,43 @@ func (s *HTTPServer) writeResponse(writer http.ResponseWriter, result *ops.Store
"*0", // Represents an empty RESP Array.
}

if val, ok := responseValue.(clientio.RespType); ok {
responseValue = respArr[val]
}
switch v := responseValue.(type) {
case []interface{}:
// Parses []interface{} as part of EvalResponse e.g. JSON.ARRPOP
// and adds to httpResponse. Also replaces "(nil)" with null JSON value
// in response array.
r := make([]interface{}, 0, len(v))
for _, resp := range v {
if val, ok := resp.(clientio.RespType); ok {
if stringNil == respArr[val] {
r = append(r, nil)
} else {
r = append(r, respArr[val])
}
} else {
r = append(r, resp)
}
}
return utils.HTTPResponse{Data: r}

if responseValue == stringNil {
responseValue = nil // in order to convert it in json null
}
case []byte:
return utils.HTTPResponse{Data: string(v)}

if bt, ok := responseValue.([]byte); ok {
responseValue = string(bt)
}
if isDiceErr {
httpResponse = utils.HTTPResponse{Status: utils.HTTPStatusError, Data: responseValue}
} else {
httpResponse = utils.HTTPResponse{Status: utils.HTTPStatusSuccess, Data: responseValue}
}
case clientio.RespType:
responseValue = respArr[v]
if responseValue == stringNil {
responseValue = nil // in order to convert it in json null
}

responseJSON, err := json.Marshal(httpResponse)
if err != nil {
s.logger.Error("Error marshaling response", "error", err)
http.Error(writer, "Internal Server Error", http.StatusInternalServerError)
return
}
return utils.HTTPResponse{Data: responseValue}

writer.Header().Set("Content-Type", "application/json")
writer.WriteHeader(http.StatusOK)
_, err = writer.Write(responseJSON)
if err != nil {
s.logger.Error("Error writing response", "error", err)
case interface{}:
if val, ok := v.(clientio.RespType); ok {
return utils.HTTPResponse{Data: respArr[val]}
}
}

return utils.HTTPResponse{Data: responseValue}
}

func generateUniqueInt32(r *http.Request) uint32 {
Expand Down

0 comments on commit 91f3d2b

Please sign in to comment.