refactor: Change handler functions implementation and response format…

…ting (#498) RELEVANT ISSUE(S) Resolves #384 Resolves #458 Resolves #494 DESCRIPTION After the HTTP API refactor, we now focus on the refactoring of the handler functions themselves ensuring common response formats for both successful and error responses across the API. This PR also changes the API version number to v0 from v1 to show "unstable" status and responds with the appropriate content-type with JSON payloads.
sourcenetwork · Jun 8, 2022 · b8beabb · b8beabb
1 parent d200cb2
commit b8beabb
Show file tree

Hide file tree

Showing 11 changed files with 920 additions and 207 deletions.
diff --git a/README.md b/README.md
@@ -136,7 +136,7 @@ defradb client ping
 ```
 which should respond with `Success!`
 
-Once you've confirmed your node is running correctly, if you're using the GraphiQL client to interact with the database, then make sure you set the `GraphQL Endpoint` to `http://localhost:9181/api/v1/graphql`.
+Once you've confirmed your node is running correctly, if you're using the GraphiQL client to interact with the database, then make sure you set the `GraphQL Endpoint` to `http://localhost:9181/api/v0/graphql`.
 
 ### Add a Schema type
 

diff --git a/api/http/errors.go b/api/http/errors.go
@@ -21,9 +21,18 @@ import (
 var env = os.Getenv("DEFRA_ENV")
 
 type errorResponse struct {
-	Status  int    `json:"status"`
-	Message string `json:"message"`
-	Stack   string `json:"stack,omitempty"`
+	Errors []errorItem `json:"errors"`
+}
+
+type errorItem struct {
+	Message    string      `json:"message"`
+	Extensions *extensions `json:"extensions,omitempty"`
+}
+
+type extensions struct {
+	Status    int    `json:"status"`
+	HTTPError string `json:"httpError"`
+	Stack     string `json:"stack,omitempty"`
 }
 
 func handleErr(ctx context.Context, rw http.ResponseWriter, err error, status int) {
@@ -35,9 +44,16 @@ func handleErr(ctx context.Context, rw http.ResponseWriter, err error, status in
 		ctx,
 		rw,
 		errorResponse{
-			Status:  status,
-			Message: http.StatusText(status),
-			Stack:   formatError(err),
+			Errors: []errorItem{
+				{
+					Message: err.Error(),
+					Extensions: &extensions{
+						Status:    status,
+						HTTPError: http.StatusText(status),
+						Stack:     formatError(err),
+					},
+				},
+			},
 		},
 		status,
 	)

diff --git a/api/http/errors_test.go b/api/http/errors_test.go
@@ -54,11 +54,14 @@ func TestHandleErrOnBadRequest(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	assert.Equal(t, http.StatusBadRequest, errResponse.Status)
-	assert.Equal(t, http.StatusText(http.StatusBadRequest), errResponse.Message)
+	if len(errResponse.Errors) != 1 {
+		t.Fatal("expecting exactly one error")
+	}
 
-	lines := strings.Split(errResponse.Stack, "\n")
-	assert.Equal(t, "[DEV] test error", lines[0])
+	assert.Equal(t, http.StatusBadRequest, errResponse.Errors[0].Extensions.Status)
+	assert.Equal(t, http.StatusText(http.StatusBadRequest), errResponse.Errors[0].Extensions.HTTPError)
+	assert.Equal(t, "test error", errResponse.Errors[0].Message)
+	assert.Contains(t, errResponse.Errors[0].Extensions.Stack, "[DEV] test error")
 }
 
 func TestHandleErrOnInternalServerError(t *testing.T) {
@@ -83,11 +86,13 @@ func TestHandleErrOnInternalServerError(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	assert.Equal(t, http.StatusInternalServerError, errResponse.Status)
-	assert.Equal(t, http.StatusText(http.StatusInternalServerError), errResponse.Message)
-
-	lines := strings.Split(errResponse.Stack, "\n")
-	assert.Equal(t, "[DEV] test error", lines[0])
+	if len(errResponse.Errors) != 1 {
+		t.Fatal("expecting exactly one error")
+	}
+	assert.Equal(t, http.StatusInternalServerError, errResponse.Errors[0].Extensions.Status)
+	assert.Equal(t, http.StatusText(http.StatusInternalServerError), errResponse.Errors[0].Extensions.HTTPError)
+	assert.Equal(t, "test error", errResponse.Errors[0].Message)
+	assert.Contains(t, errResponse.Errors[0].Extensions.Stack, "[DEV] test error")
 }
 
 func TestHandleErrOnNotFound(t *testing.T) {
@@ -112,11 +117,14 @@ func TestHandleErrOnNotFound(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	assert.Equal(t, http.StatusNotFound, errResponse.Status)
-	assert.Equal(t, http.StatusText(http.StatusNotFound), errResponse.Message)
+	if len(errResponse.Errors) != 1 {
+		t.Fatal("expecting exactly one error")
+	}
 
-	lines := strings.Split(errResponse.Stack, "\n")
-	assert.Equal(t, "[DEV] test error", lines[0])
+	assert.Equal(t, http.StatusNotFound, errResponse.Errors[0].Extensions.Status)
+	assert.Equal(t, http.StatusText(http.StatusNotFound), errResponse.Errors[0].Extensions.HTTPError)
+	assert.Equal(t, "test error", errResponse.Errors[0].Message)
+	assert.Contains(t, errResponse.Errors[0].Extensions.Stack, "[DEV] test error")
 }
 
 func TestHandleErrOnDefault(t *testing.T) {
@@ -141,9 +149,12 @@ func TestHandleErrOnDefault(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	assert.Equal(t, http.StatusUnauthorized, errResponse.Status)
-	assert.Equal(t, "Unauthorized", errResponse.Message)
+	if len(errResponse.Errors) != 1 {
+		t.Fatal("expecting exactly one error")
+	}
 
-	lines := strings.Split(errResponse.Stack, "\n")
-	assert.Equal(t, "[DEV] Unauthorized", lines[0])
+	assert.Equal(t, http.StatusUnauthorized, errResponse.Errors[0].Extensions.Status)
+	assert.Equal(t, http.StatusText(http.StatusUnauthorized), errResponse.Errors[0].Extensions.HTTPError)
+	assert.Equal(t, "Unauthorized", errResponse.Errors[0].Message)
+	assert.Contains(t, errResponse.Errors[0].Extensions.Stack, "[DEV] Unauthorized")
 }
diff --git a/api/http/handler.go b/api/http/handler.go
@@ -32,6 +32,35 @@ type handler struct {
 
 type ctxDB struct{}
 
+type dataResponse struct {
+	Data interface{} `json:"data"`
+}
+
+// simpleDataResponse is a helper function that returns a dataResponse struct.
+// Odd arguments are the keys and must be strings otherwise they are ignored.
+// Even arguments are the values associated with the previous key.
+// Odd arguments are also ignored if there are no following arguments.
+func simpleDataResponse(args ...interface{}) dataResponse {
+	data := make(map[string]interface{})
+
+	for i := 0; i < len(args); i += 2 {
+		if len(args) >= i+2 {
+			switch a := args[i].(type) {
+			case string:
+				data[a] = args[i+1]
+
+			default:
+				continue
+
+			}
+		}
+	}
+
+	return dataResponse{
+		Data: data,
+	}
+}
+
 // newHandler returns a handler with the router instantiated.
 func newHandler(db client.DB, opts serverOptions) *handler {
 	return setRoutes(&handler{

diff --git a/api/http/handler_test.go b/api/http/handler_test.go
@@ -28,6 +28,35 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestSimpleDataResponse(t *testing.T) {
+	resp := simpleDataResponse("key", "value", "key2", "value2")
+	switch v := resp.Data.(type) {
+	case map[string]interface{}:
+		assert.Equal(t, "value", v["key"])
+		assert.Equal(t, "value2", v["key2"])
+	default:
+		t.Fatalf("data should be of type map[string]interface{} but got %T", resp.Data)
+	}
+
+	resp2 := simpleDataResponse("key", "value", "key2")
+	switch v := resp2.Data.(type) {
+	case map[string]interface{}:
+		assert.Equal(t, "value", v["key"])
+		assert.Equal(t, nil, v["key2"])
+	default:
+		t.Fatalf("data should be of type map[string]interface{} but got %T", resp.Data)
+	}
+
+	resp3 := simpleDataResponse("key", "value", 2, "value2")
+	switch v := resp3.Data.(type) {
+	case map[string]interface{}:
+		assert.Equal(t, "value", v["key"])
+		assert.Equal(t, nil, v["2"])
+	default:
+		t.Fatalf("data should be of type map[string]interface{} but got %T", resp.Data)
+	}
+}
+
 func TestNewHandlerWithLogger(t *testing.T) {
 	h := newHandler(nil, serverOptions{})
 
@@ -40,14 +69,14 @@ func TestNewHandlerWithLogger(t *testing.T) {
 		OutputPaths:   []string{logFile},
 	})
 
-	req, err := http.NewRequest("GET", "/ping", nil)
+	req, err := http.NewRequest("GET", PingPath, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	rec := httptest.NewRecorder()
-
-	loggerMiddleware(h.handle(pingHandler)).ServeHTTP(rec, req)
+	lrw := newLoggingResponseWriter(rec)
+	h.ServeHTTP(lrw, req)
 	assert.Equal(t, 200, rec.Result().StatusCode)
 
 	// inspect the log file
@@ -65,13 +94,12 @@ func TestGetJSON(t *testing.T) {
 		Name string
 	}
 
-	jsonStr := []byte(`
-		{
-			"Name": "John Doe"
-		}
-	`)
+	jsonStr := `
+{
+	"Name": "John Doe"
+}`
 
-	req, err := http.NewRequest("POST", "/ping", bytes.NewBuffer(jsonStr))
+	req, err := http.NewRequest("POST", "/ping", bytes.NewBuffer([]byte(jsonStr)))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -90,13 +118,12 @@ func TestGetJSONWithError(t *testing.T) {
 		Name string
 	}
 
-	jsonStr := []byte(`
-		{
-			"Name": 10
-		}
-	`)
+	jsonStr := `
+{
+	"Name": 10
+}`
 
-	req, err := http.NewRequest("POST", "/ping", bytes.NewBuffer(jsonStr))
+	req, err := http.NewRequest("POST", "/ping", bytes.NewBuffer([]byte(jsonStr)))
 	if err != nil {
 		t.Fatal(err)
 	}