From 6a6da75814263c686ebd35250e8e3fe7fdc956e9 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Wed, 27 Apr 2022 09:37:14 -0400 Subject: [PATCH 01/19] http api refactor draft --- api/api.go | 29 ++++ api/api_test.go | 38 +++++ api/http/api.go | 289 ------------------------------------- api/http/handler.go | 298 +++++++++++++++++++++++++++++++++++++++ api/http/handler_test.go | 79 +++++++++++ api/http/logger.go | 97 +++++++++++++ api/http/logger_test.go | 97 +++++++++++++ api/http/router.go | 30 ++++ api/http/server.go | 30 ++++ api/http/server_test.go | 27 ++++ cli/defradb/cmd/start.go | 4 +- 11 files changed, 727 insertions(+), 291 deletions(-) create mode 100644 api/api.go create mode 100644 api/api_test.go delete mode 100644 api/http/api.go create mode 100644 api/http/handler.go create mode 100644 api/http/handler_test.go create mode 100644 api/http/logger.go create mode 100644 api/http/logger_test.go create mode 100644 api/http/router.go create mode 100644 api/http/server.go create mode 100644 api/http/server_test.go diff --git a/api/api.go b/api/api.go new file mode 100644 index 0000000000..fe42caa317 --- /dev/null +++ b/api/api.go @@ -0,0 +1,29 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package api + +import ( + "github.com/sourcenetwork/defradb/api/http" + "github.com/sourcenetwork/defradb/client" +) + +// NewHTTPServer returns a Server loaded with the HTTP API Handler. +func NewHTTPServer(db client.DB, c ...*http.HandlerConfig) *http.Server { + s := http.NewServer() + + if len(c) > 0 { + s.Handler = http.NewHandler(db, c[0]) + } + + s.Handler = http.NewHandler(db, nil) + + return s +} diff --git a/api/api_test.go b/api/api_test.go new file mode 100644 index 0000000000..6480f6f3c4 --- /dev/null +++ b/api/api_test.go @@ -0,0 +1,38 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package api + +import ( + "testing" + + "github.com/sourcenetwork/defradb/api/http" + "github.com/sourcenetwork/defradb/logging" + "github.com/stretchr/testify/assert" +) + +func TestServer(t *testing.T) { + s := NewHTTPServer(nil, nil) + + if ok := assert.NotNil(t, s); ok { + assert.Error(t, s.Listen(":303000")) + } + +} + +func TestServerWithConfig(t *testing.T) { + s := NewHTTPServer(nil, &http.HandlerConfig{ + Logger: logging.MustNewLogger("defra.http.test"), + }) + + if ok := assert.NotNil(t, s); ok { + assert.Error(t, s.Listen(":303000")) + } +} diff --git a/api/http/api.go b/api/http/api.go deleted file mode 100644 index e2f4db8f0c..0000000000 --- a/api/http/api.go +++ /dev/null @@ -1,289 +0,0 @@ -// Copyright 2022 Democratized Data Foundation -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -package http - -import ( - "context" - "encoding/json" - "io" - "net/http" - - "github.com/multiformats/go-multihash" - "github.com/sourcenetwork/defradb/client" - corecrdt "github.com/sourcenetwork/defradb/core/crdt" - - "github.com/go-chi/chi" - "github.com/go-chi/chi/middleware" - "github.com/ipfs/go-cid" - ds "github.com/ipfs/go-datastore" - dshelp "github.com/ipfs/go-ipfs-ds-help" - dag "github.com/ipfs/go-merkledag" - "github.com/sourcenetwork/defradb/logging" -) - -var ( - log = logging.MustNewLogger("defra.http") -) - -type Server struct { - db client.DB - router *chi.Mux -} - -func NewServer(db client.DB) *Server { - s := &Server{ - db: db, - } - r := chi.NewRouter() - // todo - we should log via our own log, not middleware.logger - r.Use(middleware.Logger) - r.Get("/", func(w http.ResponseWriter, r *http.Request) { - _, err := w.Write( - []byte("Welcome to the DefraDB HTTP API. Use /graphql to send queries to the database"), - ) - if err != nil { - log.ErrorE(context.Background(), "DefraDB HTTP API Welcome message writing failed", err) - } - }) - - r.Get("/ping", s.ping) - r.Get("/dump", s.dump) - r.Get("/blocks/get/{cid}", s.getBlock) - r.Get("/graphql", s.execGQL) - r.Post("/schema/load", s.loadSchema) - s.router = r - return s -} - -func (s *Server) Listen(addr string) error { - ctx := context.Background() - if err := http.ListenAndServe(addr, s.router); err != nil { - log.FatalE(ctx, "Error: HTTP Listening and Serving Failed", err) - return err - } - return nil -} - -func (s *Server) ping(w http.ResponseWriter, r *http.Request) { - ctx := context.Background() - _, err := w.Write([]byte("pong")) - if err != nil { - log.ErrorE(ctx, "Writing pong with HTTP failed", err) - } -} - -func (s *Server) dump(w http.ResponseWriter, r *http.Request) { - ctx := context.Background() - s.db.PrintDump(ctx) - - _, err := w.Write([]byte("ok")) - if err != nil { - log.ErrorE(ctx, "Writing ok with HTTP failed", err) - } -} - -func (s *Server) execGQL(w http.ResponseWriter, r *http.Request) { - ctx := context.Background() - query := r.URL.Query().Get("query") - result := s.db.ExecQuery(ctx, query) - - err := json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } -} - -func (s *Server) loadSchema(w http.ResponseWriter, r *http.Request) { - ctx := context.Background() - var result client.QueryResult - sdl, err := io.ReadAll(r.Body) - - defer func() { - err = r.Body.Close() - if err != nil { - log.ErrorE(ctx, "Error on body close", err) - } - }() - - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } - - w.WriteHeader(http.StatusBadRequest) - return - } - - err = s.db.AddSchema(ctx, string(sdl)) - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } - - w.WriteHeader(http.StatusBadRequest) - return - } - - result.Data = map[string]string{ - "result": "success", - } - - err = json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } -} - -func (s *Server) getBlock(w http.ResponseWriter, r *http.Request) { - ctx := context.Background() - var result client.QueryResult - cidStr := chi.URLParam(r, "cid") - - // try to parse CID - c, err := cid.Decode(cidStr) - if err != nil { - // If we can't try to parse DSKeyToCID - // return error if we still can't - key := ds.NewKey(cidStr) - var hash multihash.Multihash - hash, err = dshelp.DsKeyToMultihash(key) - if err != nil { - result.Errors = []interface{}{err.Error()} - result.Data = err.Error() - - err = json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } - - w.WriteHeader(http.StatusBadRequest) - return - } - c = cid.NewCidV1(cid.Raw, hash) - } - - block, err := s.db.Blockstore().Get(ctx, c) - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } - - w.WriteHeader(http.StatusBadRequest) - return - } - - nd, err := dag.DecodeProtobuf(block.RawData()) - if err != nil { - result.Errors = []interface{}{err.Error()} - result.Data = err.Error() - - err = json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } - - w.WriteHeader(http.StatusBadRequest) - return - } - buf, err := nd.MarshalJSON() - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } - - w.WriteHeader(http.StatusBadRequest) - return - } - - reg := corecrdt.LWWRegister{} - delta, err := reg.DeltaDecode(nd) - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } - - w.WriteHeader(http.StatusBadRequest) - return - } - - data, err := delta.Marshal() - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } - - w.WriteHeader(http.StatusBadRequest) - return - } - - // var val interface{} - // err = cbor.Unmarshal(delta.Value().([]byte), &val) - // if err != nil { - // result.Errors = []interface{}{err.Error()} - // err = json.NewEncoder(w).Encode(result) - // if err != nil { - // http.Error(w, err.Error(), 500) - // return - // } - // w.WriteHeader(http.StatusBadRequest) - // return - // } - result.Data = map[string]interface{}{ - "block": string(buf), - "delta": string(data), - "val": delta.Value(), - } - - enc := json.NewEncoder(w) - enc.SetIndent("", "\t") - err = enc.Encode(result) - if err != nil { - result.Errors = []interface{}{err.Error()} - result.Data = nil - - err := json.NewEncoder(w).Encode(result) - if err != nil { - http.Error(w, err.Error(), 500) - return - } - - w.WriteHeader(http.StatusBadRequest) - return - } -} diff --git a/api/http/handler.go b/api/http/handler.go new file mode 100644 index 0000000000..201673b41d --- /dev/null +++ b/api/http/handler.go @@ -0,0 +1,298 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "encoding/json" + "io/ioutil" + "net/http" + + "github.com/go-chi/chi" + "github.com/ipfs/go-cid" + ds "github.com/ipfs/go-datastore" + dshelp "github.com/ipfs/go-ipfs-ds-help" + dag "github.com/ipfs/go-merkledag" + "github.com/multiformats/go-multihash" + "github.com/sourcenetwork/defradb/client" + corecrdt "github.com/sourcenetwork/defradb/core/crdt" + "github.com/sourcenetwork/defradb/logging" +) + +type handler struct { + db client.DB + + *chi.Mux + *logger +} + +// HandlerConfig holds the handler configurable parameters +type HandlerConfig struct { + Logger logging.Logger +} + +// NewHandler returns a handler with the router instantiated and configuration applied. +func NewHandler(db client.DB, c *HandlerConfig) *handler { + h := &handler{ + db: db, + } + + h.setRoutes() + + if c != nil { + if c.Logger != nil { + h.logger = withLogger(c.Logger) + } + + return h + } + + h.logger = defaultLogger() + + return h +} + +type context struct { + res http.ResponseWriter + req *http.Request + db client.DB + log *logger +} + +func (h *handler) handle(f func(*context)) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + f(&context{ + res: w, + req: r, + db: h.db, + log: h.logger, + }) + } +} + +func root(c *context) { + _, err := c.res.Write( + []byte("Welcome to the DefraDB HTTP API. Use /graphql to send queries to the database"), + ) + if err != nil { + c.log.ErrorE(c.req.Context(), "DefraDB HTTP API Welcome message writing failed", err) + } +} + +func ping(c *context) { + _, err := c.res.Write([]byte("pong")) + if err != nil { + c.log.ErrorE(c.req.Context(), "Writing pong with HTTP failed", err) + } +} + +func dump(c *context) { + c.db.PrintDump(c.req.Context()) + + _, err := c.res.Write([]byte("ok")) + if err != nil { + c.log.ErrorE(c.req.Context(), "Writing ok with HTTP failed", err) + } +} + +func execGQL(c *context) { + query := c.req.URL.Query().Get("query") + result := c.db.ExecQuery(c.req.Context(), query) + + err := json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } +} + +func loadSchema(c *context) { + var result client.QueryResult + sdl, err := ioutil.ReadAll(c.req.Body) + + defer func() { + err = c.req.Body.Close() + if err != nil { + c.log.ErrorE(c.req.Context(), "Error on body close", err) + } + }() + + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + err = c.db.AddSchema(c.req.Context(), string(sdl)) + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + result.Data = map[string]string{ + "result": "success", + } + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } +} + +func getBlock(c *context) { + var result client.QueryResult + cidStr := chi.URLParam(c.req, "cid") + + // try to parse CID + cID, err := cid.Decode(cidStr) + if err != nil { + // If we can't try to parse DSKeyToCID + // return error if we still can't + key := ds.NewKey(cidStr) + var hash multihash.Multihash + hash, err = dshelp.DsKeyToMultihash(key) + if err != nil { + result.Errors = []interface{}{err.Error()} + result.Data = err.Error() + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + cID = cid.NewCidV1(cid.Raw, hash) + } + + block, err := c.db.Blockstore().Get(c.req.Context(), cID) + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + nd, err := dag.DecodeProtobuf(block.RawData()) + if err != nil { + result.Errors = []interface{}{err.Error()} + result.Data = err.Error() + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + buf, err := nd.MarshalJSON() + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + reg := corecrdt.LWWRegister{} + delta, err := reg.DeltaDecode(nd) + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + data, err := delta.Marshal() + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + // var val interface{} + // err = cbor.Unmarshal(delta.Value().([]byte), &val) + // if err != nil { + // result.Errors = []interface{}{err.Error()} + // err = json.NewEncoder(c.res).Encode(result) + // if err != nil { + // http.Error(c.res, err.Error(), 500) + // return + // } + // c.c.res.WriteHeader(http.StatusBadRequest) + // return + // } + result.Data = map[string]interface{}{ + "block": string(buf), + "delta": string(data), + "val": delta.Value(), + } + + enc := json.NewEncoder(c.res) + enc.SetIndent("", "\t") + err = enc.Encode(result) + if err != nil { + result.Errors = []interface{}{err.Error()} + result.Data = nil + + err := json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), 500) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } +} diff --git a/api/http/handler_test.go b/api/http/handler_test.go new file mode 100644 index 0000000000..30864064d9 --- /dev/null +++ b/api/http/handler_test.go @@ -0,0 +1,79 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "net/http" + "net/http/httptest" + "path" + "testing" + + "github.com/sourcenetwork/defradb/logging" + "github.com/stretchr/testify/assert" +) + +func TestHandler(t *testing.T) { + h := NewHandler(nil, nil) + + dir := t.TempDir() + + // send logs to temp file so we can inspect it + logFile := path.Join(dir, "http_test.log") + h.ApplyConfig(logging.Config{ + EncoderFormat: logging.NewEncoderFormatOption(logging.JSON), + OutputPaths: []string{logFile}, + }) + + req, err := http.NewRequest("GET", "/ping", nil) + assert.NoError(t, err) + + rec2 := httptest.NewRecorder() + + h.loggerMiddleware(h.handle(ping)).ServeHTTP(rec2, req) + assert.Equal(t, 200, rec2.Result().StatusCode) + + // inspect the log file + kv, err := readLog(logFile) + assert.NoError(t, err) + + assert.Equal(t, "defra.http", kv["logger"]) + +} + +func TestHandlerWithConfig(t *testing.T) { + h := NewHandler(nil, &HandlerConfig{ + Logger: withLogger(logging.MustNewLogger("defra.http.test")), + }) + + dir := t.TempDir() + + // send logs to temp file so we can inspect it + logFile := path.Join(dir, "http_test.log") + h.ApplyConfig(logging.Config{ + EncoderFormat: logging.NewEncoderFormatOption(logging.JSON), + OutputPaths: []string{logFile}, + }) + + req, err := http.NewRequest("GET", "/ping", nil) + assert.NoError(t, err) + + rec2 := httptest.NewRecorder() + + h.loggerMiddleware(h.handle(ping)).ServeHTTP(rec2, req) + assert.Equal(t, 200, rec2.Result().StatusCode) + + // inspect the log file + kv, err := readLog(logFile) + assert.NoError(t, err) + + assert.Equal(t, "defra.http.test", kv["logger"]) + +} diff --git a/api/http/logger.go b/api/http/logger.go new file mode 100644 index 0000000000..bbabb6ab31 --- /dev/null +++ b/api/http/logger.go @@ -0,0 +1,97 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "net/http" + "strconv" + "time" + + "github.com/sourcenetwork/defradb/logging" +) + +type logger struct { + logging.Logger +} + +func defaultLogger() *logger { + return &logger{ + Logger: logging.MustNewLogger("defra.http"), + } +} + +func withLogger(l logging.Logger) *logger { + return &logger{ + Logger: l, + } +} + +type loggingResponseWriter struct { + statusCode int + contentLength int + + http.ResponseWriter +} + +func newLoggingResponseWriter(w http.ResponseWriter) *loggingResponseWriter { + return &loggingResponseWriter{ + statusCode: http.StatusOK, + contentLength: 0, + ResponseWriter: w, + } +} + +func (lrw *loggingResponseWriter) WriteHeader(code int) { + lrw.statusCode = code + lrw.ResponseWriter.WriteHeader(code) +} + +func (lrw *loggingResponseWriter) Write(b []byte) (int, error) { + if lrw.ResponseWriter.Header().Get("Content-Length") != "" { + return lrw.ResponseWriter.Write(b) + } + lrw.contentLength = len(b) + lrw.ResponseWriter.Header().Set("Content-Length", strconv.Itoa(lrw.contentLength)) + return lrw.ResponseWriter.Write(b) +} + +func (l *logger) loggerMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + start := time.Now() + lrw := newLoggingResponseWriter(w) + next.ServeHTTP(lrw, r) + elapsed := time.Since(start) + l.Info( + r.Context(), + "Request", + logging.NewKV( + "Method", + r.Method, + ), + logging.NewKV( + "Path", + r.URL.Path, + ), + logging.NewKV( + "Status", + lrw.statusCode, + ), + logging.NewKV( + "Length", + lrw.contentLength, + ), + logging.NewKV( + "Elapsed", + elapsed, + ), + ) + }) +} diff --git a/api/http/logger_test.go b/api/http/logger_test.go new file mode 100644 index 0000000000..3674749ea6 --- /dev/null +++ b/api/http/logger_test.go @@ -0,0 +1,97 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "bufio" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path" + "testing" + + "github.com/pkg/errors" + "github.com/sourcenetwork/defradb/logging" + "github.com/stretchr/testify/assert" +) + +func TestLogger(t *testing.T) { + rec := httptest.NewRecorder() + lrw := newLoggingResponseWriter(rec) + + lrw.WriteHeader(400) + assert.Equal(t, 400, lrw.statusCode) + + content := "Hello world!" + + length, err := lrw.Write([]byte(content)) + assert.NoError(t, err) + assert.Equal(t, length, lrw.contentLength) + assert.Equal(t, rec.Body.String(), content) +} + +func TestLoggerLogs(t *testing.T) { + dir := t.TempDir() + + // send logs to temp file so we can inspect it + logFile := path.Join(dir, "http_test.log") + l := withLogger(logging.MustNewLogger("defra.http.test")) + l.ApplyConfig(logging.Config{ + EncoderFormat: logging.NewEncoderFormatOption(logging.JSON), + OutputPaths: []string{logFile}, + }) + + req, err := http.NewRequest("GET", "/ping", nil) + assert.NoError(t, err) + + rec2 := httptest.NewRecorder() + + h := handler{ + logger: l, + } + h.loggerMiddleware(h.handle(ping)).ServeHTTP(rec2, req) + assert.Equal(t, 200, rec2.Result().StatusCode) + + // inspect the log file + kv, err := readLog(logFile) + assert.NoError(t, err) + + // check that everything is as expected + assert.Equal(t, "pong", rec2.Body.String()) + assert.Equal(t, "INFO", kv["level"]) + assert.Equal(t, "defra.http.test", kv["logger"]) + assert.Equal(t, "Request", kv["msg"]) + assert.Equal(t, "GET", kv["Method"]) + assert.Equal(t, "/ping", kv["Path"]) + assert.Equal(t, float64(200), kv["Status"]) + assert.Equal(t, float64(4), kv["Length"]) +} + +func readLog(path string) (map[string]interface{}, error) { + // inspect the log file + f, err := os.Open(path) + if err != nil { + return nil, errors.WithStack(err) + } + + scanner := bufio.NewScanner(f) + scanner.Scan() + logLine := scanner.Text() + + kv := make(map[string]interface{}) + err = json.Unmarshal([]byte(logLine), &kv) + if err != nil { + return nil, errors.WithStack(err) + } + + return kv, nil +} diff --git a/api/http/router.go b/api/http/router.go new file mode 100644 index 0000000000..fed86bb4d9 --- /dev/null +++ b/api/http/router.go @@ -0,0 +1,30 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "github.com/go-chi/chi" +) + +func (h *handler) setRoutes() { + h.Mux = chi.NewRouter() + + // setup logger middleware + h.Use(h.loggerMiddleware) + + // define routes + h.Get("/", h.handle(root)) + h.Get("/ping", h.handle(ping)) + h.Get("/dump", h.handle(dump)) + h.Get("/blocks/get/{cid}", h.handle(getBlock)) + h.Get("/graphql", h.handle(execGQL)) + h.Post("/schema/load", h.handle(loadSchema)) +} diff --git a/api/http/server.go b/api/http/server.go new file mode 100644 index 0000000000..d16450915e --- /dev/null +++ b/api/http/server.go @@ -0,0 +1,30 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "net/http" +) + +// The Server struct holds the Handler for the HTTP API +type Server struct { + Handler http.Handler +} + +// NewServer instantiated a new server. +func NewServer() *Server { + return &Server{} +} + +// Listen calls ListenAndServe with our router. +func (s *Server) Listen(addr string) error { + return http.ListenAndServe(addr, s.Handler) +} diff --git a/api/http/server_test.go b/api/http/server_test.go new file mode 100644 index 0000000000..bd119d3513 --- /dev/null +++ b/api/http/server_test.go @@ -0,0 +1,27 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestServer(t *testing.T) { + // @TODO: maybe it would be worth doing something a bit more thorough + + // test with no config + s := NewServer() + if ok := assert.NotNil(t, s); ok { + assert.Error(t, s.Listen(":303000")) + } +} diff --git a/cli/defradb/cmd/start.go b/cli/defradb/cmd/start.go index c161d06f64..3c665a3a63 100644 --- a/cli/defradb/cmd/start.go +++ b/cli/defradb/cmd/start.go @@ -32,7 +32,7 @@ import ( badger "github.com/dgraph-io/badger/v3" ds "github.com/ipfs/go-datastore" - api "github.com/sourcenetwork/defradb/api/http" + "github.com/sourcenetwork/defradb/api" "github.com/sourcenetwork/defradb/logging" "github.com/spf13/cobra" "github.com/textileio/go-threads/broadcast" @@ -178,7 +178,7 @@ var startCmd = &cobra.Command{ config.Database.Address, ), ) - s := api.NewServer(db) + s := api.NewHTTPServer(db) if err := s.Listen(config.Database.Address); err != nil { log.ErrorE(ctx, "Failed to start HTTP API listener", err) if n != nil { From f7ad42fe73633561020c3e80fadcc9a36abf3a97 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Wed, 27 Apr 2022 00:16:38 -0400 Subject: [PATCH 02/19] graphql post and setRoute fix --- api/http/handler.go | 46 ++++++++++++++++++++++++++++++--------------- api/http/router.go | 1 + 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/api/http/handler.go b/api/http/handler.go index 201673b41d..e408e99016 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -44,8 +44,6 @@ func NewHandler(db client.DB, c *HandlerConfig) *handler { db: db, } - h.setRoutes() - if c != nil { if c.Logger != nil { h.logger = withLogger(c.Logger) @@ -56,6 +54,8 @@ func NewHandler(db client.DB, c *HandlerConfig) *handler { h.logger = defaultLogger() + h.setRoutes() + return h } @@ -103,12 +103,28 @@ func dump(c *context) { } func execGQL(c *context) { - query := c.req.URL.Query().Get("query") + var query string + if c.req.Method == "GET" { + query = c.req.URL.Query().Get("query") + } else { + body, err := io.ReadAll(c.req.Body) + if err != nil { + http.Error(c.res, err.Error(), http.StatusBadRequest) + return + } + query = string(body) + } + + if query == "" { + http.Error(c.res, "missing GraphQL query", http.StatusBadRequest) + return + } + result := c.db.ExecQuery(c.req.Context(), query) err := json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } } @@ -129,7 +145,7 @@ func loadSchema(c *context) { err = json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } @@ -143,7 +159,7 @@ func loadSchema(c *context) { err = json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } @@ -157,7 +173,7 @@ func loadSchema(c *context) { err = json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } } @@ -180,7 +196,7 @@ func getBlock(c *context) { err = json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } @@ -196,7 +212,7 @@ func getBlock(c *context) { err = json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } @@ -211,7 +227,7 @@ func getBlock(c *context) { err = json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } @@ -224,7 +240,7 @@ func getBlock(c *context) { err = json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } @@ -239,7 +255,7 @@ func getBlock(c *context) { err = json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } @@ -253,7 +269,7 @@ func getBlock(c *context) { err = json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } @@ -267,7 +283,7 @@ func getBlock(c *context) { // result.Errors = []interface{}{err.Error()} // err = json.NewEncoder(c.res).Encode(result) // if err != nil { - // http.Error(c.res, err.Error(), 500) + // http.Error(c.res, err.Error(), http.StatusInternalServerError) // return // } // c.c.res.WriteHeader(http.StatusBadRequest) @@ -288,7 +304,7 @@ func getBlock(c *context) { err := json.NewEncoder(c.res).Encode(result) if err != nil { - http.Error(c.res, err.Error(), 500) + http.Error(c.res, err.Error(), http.StatusInternalServerError) return } diff --git a/api/http/router.go b/api/http/router.go index fed86bb4d9..6dd7a4c479 100644 --- a/api/http/router.go +++ b/api/http/router.go @@ -26,5 +26,6 @@ func (h *handler) setRoutes() { h.Get("/dump", h.handle(dump)) h.Get("/blocks/get/{cid}", h.handle(getBlock)) h.Get("/graphql", h.handle(execGQL)) + h.Post("/graphql", h.handle(execGQL)) h.Post("/schema/load", h.handle(loadSchema)) } From 60edcc3fbe9f3f34bb168e3690cd08b455f8bf59 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Wed, 27 Apr 2022 14:10:58 -0400 Subject: [PATCH 03/19] update README and fix ioutil import --- README.md | 3 ++- api/http/handler.go | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 971704c6e7..c8954b9d70 100644 --- 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/graphql` and the `Method` to `GET`. +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/graphql`. ### Add a Schema type @@ -391,6 +391,7 @@ When contributing to a DefraDB feature, you can find the relevant license in the - Andrew Sisley ([@AndrewSisley](https://github.com/AndrewSisley)) - Shahzad Lone ([@shahzadlone](https://github.com/shahzadlone)) - Orpheus Lummis ([@orpheuslummis](https://github.com/orpheuslummis)) +- Fred Carle ([@fredcarle](https://github.com/fredcarle))
diff --git a/api/http/handler.go b/api/http/handler.go index e408e99016..ca5fff70e8 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -12,7 +12,7 @@ package http import ( "encoding/json" - "io/ioutil" + "io" "net/http" "github.com/go-chi/chi" @@ -131,7 +131,7 @@ func execGQL(c *context) { func loadSchema(c *context) { var result client.QueryResult - sdl, err := ioutil.ReadAll(c.req.Body) + sdl, err := io.ReadAll(c.req.Body) defer func() { err = c.req.Body.Close() From fa6769754cdc3cf88931e61d30fe6de0aa996c0f Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Wed, 27 Apr 2022 14:13:48 -0400 Subject: [PATCH 04/19] remove commented code Co-authored-by: Shahzad Lone --- api/http/handler.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/api/http/handler.go b/api/http/handler.go index ca5fff70e8..eff5c51229 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -277,18 +277,6 @@ func getBlock(c *context) { return } - // var val interface{} - // err = cbor.Unmarshal(delta.Value().([]byte), &val) - // if err != nil { - // result.Errors = []interface{}{err.Error()} - // err = json.NewEncoder(c.res).Encode(result) - // if err != nil { - // http.Error(c.res, err.Error(), http.StatusInternalServerError) - // return - // } - // c.c.res.WriteHeader(http.StatusBadRequest) - // return - // } result.Data = map[string]interface{}{ "block": string(buf), "delta": string(data), From 722ae1254940304863e2929b772b2390683b1335 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Wed, 27 Apr 2022 14:44:47 -0400 Subject: [PATCH 05/19] fix missing route bug --- api/http/handler.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/http/handler.go b/api/http/handler.go index eff5c51229..ec42873f46 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -49,11 +49,10 @@ func NewHandler(db client.DB, c *HandlerConfig) *handler { h.logger = withLogger(c.Logger) } - return h + } else { + h.logger = defaultLogger() } - h.logger = defaultLogger() - h.setRoutes() return h From 4e6600b9e6d802091807db514b08ed4ed708e8b6 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Wed, 27 Apr 2022 18:05:37 -0400 Subject: [PATCH 06/19] applying feedback Rename test functions to be more specific. Remove api package and use http.NewServer. Put handlerFuncs in their own file. Change the way options are set on the handler. --- api/api.go | 29 ----- api/api_test.go | 38 ------ api/http/handler.go | 269 +++------------------------------------ api/http/handler_test.go | 14 +- api/http/handlerfuncs.go | 250 ++++++++++++++++++++++++++++++++++++ api/http/logger.go | 8 +- api/http/logger_test.go | 17 +-- api/http/router.go | 4 +- api/http/server.go | 10 +- api/http/server_test.go | 4 +- cli/defradb/cmd/start.go | 4 +- 11 files changed, 297 insertions(+), 350 deletions(-) delete mode 100644 api/api.go delete mode 100644 api/api_test.go create mode 100644 api/http/handlerfuncs.go diff --git a/api/api.go b/api/api.go deleted file mode 100644 index fe42caa317..0000000000 --- a/api/api.go +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2022 Democratized Data Foundation -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -package api - -import ( - "github.com/sourcenetwork/defradb/api/http" - "github.com/sourcenetwork/defradb/client" -) - -// NewHTTPServer returns a Server loaded with the HTTP API Handler. -func NewHTTPServer(db client.DB, c ...*http.HandlerConfig) *http.Server { - s := http.NewServer() - - if len(c) > 0 { - s.Handler = http.NewHandler(db, c[0]) - } - - s.Handler = http.NewHandler(db, nil) - - return s -} diff --git a/api/api_test.go b/api/api_test.go deleted file mode 100644 index 6480f6f3c4..0000000000 --- a/api/api_test.go +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2022 Democratized Data Foundation -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -package api - -import ( - "testing" - - "github.com/sourcenetwork/defradb/api/http" - "github.com/sourcenetwork/defradb/logging" - "github.com/stretchr/testify/assert" -) - -func TestServer(t *testing.T) { - s := NewHTTPServer(nil, nil) - - if ok := assert.NotNil(t, s); ok { - assert.Error(t, s.Listen(":303000")) - } - -} - -func TestServerWithConfig(t *testing.T) { - s := NewHTTPServer(nil, &http.HandlerConfig{ - Logger: logging.MustNewLogger("defra.http.test"), - }) - - if ok := assert.NotNil(t, s); ok { - assert.Error(t, s.Listen(":303000")) - } -} diff --git a/api/http/handler.go b/api/http/handler.go index ec42873f46..04fc6a7100 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -11,45 +11,33 @@ package http import ( - "encoding/json" - "io" "net/http" "github.com/go-chi/chi" - "github.com/ipfs/go-cid" - ds "github.com/ipfs/go-datastore" - dshelp "github.com/ipfs/go-ipfs-ds-help" - dag "github.com/ipfs/go-merkledag" - "github.com/multiformats/go-multihash" "github.com/sourcenetwork/defradb/client" - corecrdt "github.com/sourcenetwork/defradb/core/crdt" "github.com/sourcenetwork/defradb/logging" ) -type handler struct { +type Handler struct { db client.DB *chi.Mux *logger } -// HandlerConfig holds the handler configurable parameters -type HandlerConfig struct { - Logger logging.Logger -} - -// NewHandler returns a handler with the router instantiated and configuration applied. -func NewHandler(db client.DB, c *HandlerConfig) *handler { - h := &handler{ +// newHandler returns a handler with the router instantiated and configuration applied. +func newHandler(db client.DB, options ...func(*Handler)) *Handler { + h := &Handler{ db: db, } - if c != nil { - if c.Logger != nil { - h.logger = withLogger(c.Logger) - } + // apply options + for _, o := range options { + o(h) + } - } else { + // ensure we have a logger defined + if h.logger == nil { h.logger = defaultLogger() } @@ -58,16 +46,23 @@ func NewHandler(db client.DB, c *HandlerConfig) *handler { return h } -type context struct { +// WithLogger returns an option loading function for logger. +func WithLogger(l logging.Logger) func(*Handler) { + return func(h *Handler) { + h.logger = &logger{l} + } +} + +type requestContext struct { res http.ResponseWriter req *http.Request db client.DB log *logger } -func (h *handler) handle(f func(*context)) http.HandlerFunc { +func (h *Handler) handle(f func(*requestContext)) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - f(&context{ + f(&requestContext{ res: w, req: r, db: h.db, @@ -75,227 +70,3 @@ func (h *handler) handle(f func(*context)) http.HandlerFunc { }) } } - -func root(c *context) { - _, err := c.res.Write( - []byte("Welcome to the DefraDB HTTP API. Use /graphql to send queries to the database"), - ) - if err != nil { - c.log.ErrorE(c.req.Context(), "DefraDB HTTP API Welcome message writing failed", err) - } -} - -func ping(c *context) { - _, err := c.res.Write([]byte("pong")) - if err != nil { - c.log.ErrorE(c.req.Context(), "Writing pong with HTTP failed", err) - } -} - -func dump(c *context) { - c.db.PrintDump(c.req.Context()) - - _, err := c.res.Write([]byte("ok")) - if err != nil { - c.log.ErrorE(c.req.Context(), "Writing ok with HTTP failed", err) - } -} - -func execGQL(c *context) { - var query string - if c.req.Method == "GET" { - query = c.req.URL.Query().Get("query") - } else { - body, err := io.ReadAll(c.req.Body) - if err != nil { - http.Error(c.res, err.Error(), http.StatusBadRequest) - return - } - query = string(body) - } - - if query == "" { - http.Error(c.res, "missing GraphQL query", http.StatusBadRequest) - return - } - - result := c.db.ExecQuery(c.req.Context(), query) - - err := json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } -} - -func loadSchema(c *context) { - var result client.QueryResult - sdl, err := io.ReadAll(c.req.Body) - - defer func() { - err = c.req.Body.Close() - if err != nil { - c.log.ErrorE(c.req.Context(), "Error on body close", err) - } - }() - - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } - - c.res.WriteHeader(http.StatusBadRequest) - return - } - - err = c.db.AddSchema(c.req.Context(), string(sdl)) - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } - - c.res.WriteHeader(http.StatusBadRequest) - return - } - - result.Data = map[string]string{ - "result": "success", - } - - err = json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } -} - -func getBlock(c *context) { - var result client.QueryResult - cidStr := chi.URLParam(c.req, "cid") - - // try to parse CID - cID, err := cid.Decode(cidStr) - if err != nil { - // If we can't try to parse DSKeyToCID - // return error if we still can't - key := ds.NewKey(cidStr) - var hash multihash.Multihash - hash, err = dshelp.DsKeyToMultihash(key) - if err != nil { - result.Errors = []interface{}{err.Error()} - result.Data = err.Error() - - err = json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } - - c.res.WriteHeader(http.StatusBadRequest) - return - } - cID = cid.NewCidV1(cid.Raw, hash) - } - - block, err := c.db.Blockstore().Get(c.req.Context(), cID) - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } - - c.res.WriteHeader(http.StatusBadRequest) - return - } - - nd, err := dag.DecodeProtobuf(block.RawData()) - if err != nil { - result.Errors = []interface{}{err.Error()} - result.Data = err.Error() - - err = json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } - - c.res.WriteHeader(http.StatusBadRequest) - return - } - buf, err := nd.MarshalJSON() - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } - - c.res.WriteHeader(http.StatusBadRequest) - return - } - - reg := corecrdt.LWWRegister{} - delta, err := reg.DeltaDecode(nd) - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } - - c.res.WriteHeader(http.StatusBadRequest) - return - } - - data, err := delta.Marshal() - if err != nil { - result.Errors = []interface{}{err.Error()} - - err = json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } - - c.res.WriteHeader(http.StatusBadRequest) - return - } - - result.Data = map[string]interface{}{ - "block": string(buf), - "delta": string(data), - "val": delta.Value(), - } - - enc := json.NewEncoder(c.res) - enc.SetIndent("", "\t") - err = enc.Encode(result) - if err != nil { - result.Errors = []interface{}{err.Error()} - result.Data = nil - - err := json.NewEncoder(c.res).Encode(result) - if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) - return - } - - c.res.WriteHeader(http.StatusBadRequest) - return - } -} diff --git a/api/http/handler_test.go b/api/http/handler_test.go index 30864064d9..668e851b2a 100644 --- a/api/http/handler_test.go +++ b/api/http/handler_test.go @@ -20,8 +20,8 @@ import ( "github.com/stretchr/testify/assert" ) -func TestHandler(t *testing.T) { - h := NewHandler(nil, nil) +func TestNewHandlerWithLogger(t *testing.T) { + h := newHandler(nil) dir := t.TempDir() @@ -37,7 +37,7 @@ func TestHandler(t *testing.T) { rec2 := httptest.NewRecorder() - h.loggerMiddleware(h.handle(ping)).ServeHTTP(rec2, req) + h.logger.middleware(h.handle(ping)).ServeHTTP(rec2, req) assert.Equal(t, 200, rec2.Result().StatusCode) // inspect the log file @@ -48,10 +48,8 @@ func TestHandler(t *testing.T) { } -func TestHandlerWithConfig(t *testing.T) { - h := NewHandler(nil, &HandlerConfig{ - Logger: withLogger(logging.MustNewLogger("defra.http.test")), - }) +func TestNewHandlerWithConfigAndLogger(t *testing.T) { + h := newHandler(nil, WithLogger(logging.MustNewLogger("defra.http.test"))) dir := t.TempDir() @@ -67,7 +65,7 @@ func TestHandlerWithConfig(t *testing.T) { rec2 := httptest.NewRecorder() - h.loggerMiddleware(h.handle(ping)).ServeHTTP(rec2, req) + h.logger.middleware(h.handle(ping)).ServeHTTP(rec2, req) assert.Equal(t, 200, rec2.Result().StatusCode) // inspect the log file diff --git a/api/http/handlerfuncs.go b/api/http/handlerfuncs.go new file mode 100644 index 0000000000..ab81eb4496 --- /dev/null +++ b/api/http/handlerfuncs.go @@ -0,0 +1,250 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "encoding/json" + "io" + "net/http" + + "github.com/go-chi/chi" + "github.com/ipfs/go-cid" + ds "github.com/ipfs/go-datastore" + dshelp "github.com/ipfs/go-ipfs-ds-help" + dag "github.com/ipfs/go-merkledag" + "github.com/multiformats/go-multihash" + "github.com/sourcenetwork/defradb/client" + corecrdt "github.com/sourcenetwork/defradb/core/crdt" +) + +func root(c *requestContext) { + _, err := c.res.Write( + []byte("Welcome to the DefraDB HTTP API. Use /graphql to send queries to the database"), + ) + if err != nil { + c.log.ErrorE(c.req.Context(), "DefraDB HTTP API Welcome message writing failed", err) + } +} + +func ping(c *requestContext) { + _, err := c.res.Write([]byte("pong")) + if err != nil { + c.log.ErrorE(c.req.Context(), "Writing pong with HTTP failed", err) + } +} + +func dump(c *requestContext) { + c.db.PrintDump(c.req.Context()) + + _, err := c.res.Write([]byte("ok")) + if err != nil { + c.log.ErrorE(c.req.Context(), "Writing ok with HTTP failed", err) + } +} + +func execGQL(c *requestContext) { + var query string + if c.req.Method == "GET" { + query = c.req.URL.Query().Get("query") + } else { + body, err := io.ReadAll(c.req.Body) + if err != nil { + http.Error(c.res, err.Error(), http.StatusBadRequest) + return + } + query = string(body) + } + + if query == "" { + http.Error(c.res, "missing GraphQL query", http.StatusBadRequest) + return + } + + result := c.db.ExecQuery(c.req.Context(), query) + + err := json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } +} + +func loadSchema(c *requestContext) { + var result client.QueryResult + sdl, err := io.ReadAll(c.req.Body) + + defer func() { + err = c.req.Body.Close() + if err != nil { + c.log.ErrorE(c.req.Context(), "Error on body close", err) + } + }() + + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + err = c.db.AddSchema(c.req.Context(), string(sdl)) + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + result.Data = map[string]string{ + "result": "success", + } + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } +} + +func getBlock(c *requestContext) { + var result client.QueryResult + cidStr := chi.URLParam(c.req, "cid") + + // try to parse CID + cID, err := cid.Decode(cidStr) + if err != nil { + // If we can't try to parse DSKeyToCID + // return error if we still can't + key := ds.NewKey(cidStr) + var hash multihash.Multihash + hash, err = dshelp.DsKeyToMultihash(key) + if err != nil { + result.Errors = []interface{}{err.Error()} + result.Data = err.Error() + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + cID = cid.NewCidV1(cid.Raw, hash) + } + + block, err := c.db.Blockstore().Get(c.req.Context(), cID) + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + nd, err := dag.DecodeProtobuf(block.RawData()) + if err != nil { + result.Errors = []interface{}{err.Error()} + result.Data = err.Error() + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + buf, err := nd.MarshalJSON() + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + reg := corecrdt.LWWRegister{} + delta, err := reg.DeltaDecode(nd) + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + data, err := delta.Marshal() + if err != nil { + result.Errors = []interface{}{err.Error()} + + err = json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } + + result.Data = map[string]interface{}{ + "block": string(buf), + "delta": string(data), + "val": delta.Value(), + } + + enc := json.NewEncoder(c.res) + enc.SetIndent("", "\t") + err = enc.Encode(result) + if err != nil { + result.Errors = []interface{}{err.Error()} + result.Data = nil + + err := json.NewEncoder(c.res).Encode(result) + if err != nil { + http.Error(c.res, err.Error(), http.StatusInternalServerError) + return + } + + c.res.WriteHeader(http.StatusBadRequest) + return + } +} diff --git a/api/http/logger.go b/api/http/logger.go index bbabb6ab31..0dfd73ab6a 100644 --- a/api/http/logger.go +++ b/api/http/logger.go @@ -28,12 +28,6 @@ func defaultLogger() *logger { } } -func withLogger(l logging.Logger) *logger { - return &logger{ - Logger: l, - } -} - type loggingResponseWriter struct { statusCode int contentLength int @@ -63,7 +57,7 @@ func (lrw *loggingResponseWriter) Write(b []byte) (int, error) { return lrw.ResponseWriter.Write(b) } -func (l *logger) loggerMiddleware(next http.Handler) http.Handler { +func (l *logger) middleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { start := time.Now() lrw := newLoggingResponseWriter(w) diff --git a/api/http/logger_test.go b/api/http/logger_test.go index 3674749ea6..4ffa8dda66 100644 --- a/api/http/logger_test.go +++ b/api/http/logger_test.go @@ -24,7 +24,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestLogger(t *testing.T) { +func TestNewLoggingResponseWriterLogger(t *testing.T) { rec := httptest.NewRecorder() lrw := newLoggingResponseWriter(rec) @@ -44,21 +44,18 @@ func TestLoggerLogs(t *testing.T) { // send logs to temp file so we can inspect it logFile := path.Join(dir, "http_test.log") - l := withLogger(logging.MustNewLogger("defra.http.test")) - l.ApplyConfig(logging.Config{ - EncoderFormat: logging.NewEncoderFormatOption(logging.JSON), - OutputPaths: []string{logFile}, - }) req, err := http.NewRequest("GET", "/ping", nil) assert.NoError(t, err) rec2 := httptest.NewRecorder() - h := handler{ - logger: l, - } - h.loggerMiddleware(h.handle(ping)).ServeHTTP(rec2, req) + h := newHandler(nil, WithLogger(logging.MustNewLogger("defra.http.test"))) + h.logger.ApplyConfig(logging.Config{ + EncoderFormat: logging.NewEncoderFormatOption(logging.JSON), + OutputPaths: []string{logFile}, + }) + h.logger.middleware(h.handle(ping)).ServeHTTP(rec2, req) assert.Equal(t, 200, rec2.Result().StatusCode) // inspect the log file diff --git a/api/http/router.go b/api/http/router.go index 6dd7a4c479..5c34aa7b23 100644 --- a/api/http/router.go +++ b/api/http/router.go @@ -14,11 +14,11 @@ import ( "github.com/go-chi/chi" ) -func (h *handler) setRoutes() { +func (h *Handler) setRoutes() { h.Mux = chi.NewRouter() // setup logger middleware - h.Use(h.loggerMiddleware) + h.Use(h.logger.middleware) // define routes h.Get("/", h.handle(root)) diff --git a/api/http/server.go b/api/http/server.go index d16450915e..441d087564 100644 --- a/api/http/server.go +++ b/api/http/server.go @@ -12,6 +12,8 @@ package http import ( "net/http" + + "github.com/sourcenetwork/defradb/client" ) // The Server struct holds the Handler for the HTTP API @@ -19,9 +21,11 @@ type Server struct { Handler http.Handler } -// NewServer instantiated a new server. -func NewServer() *Server { - return &Server{} +// NewServer instantiated a new server with the given http.Handler. +func NewServer(db client.DB, handlerOptions ...func(*Handler)) *Server { + return &Server{ + Handler: newHandler(db, handlerOptions...), + } } // Listen calls ListenAndServe with our router. diff --git a/api/http/server_test.go b/api/http/server_test.go index bd119d3513..d4a4c48b7e 100644 --- a/api/http/server_test.go +++ b/api/http/server_test.go @@ -16,11 +16,11 @@ import ( "github.com/stretchr/testify/assert" ) -func TestServer(t *testing.T) { +func TestNewServerAndListen(t *testing.T) { // @TODO: maybe it would be worth doing something a bit more thorough // test with no config - s := NewServer() + s := NewServer(nil) if ok := assert.NotNil(t, s); ok { assert.Error(t, s.Listen(":303000")) } diff --git a/cli/defradb/cmd/start.go b/cli/defradb/cmd/start.go index 3c665a3a63..af14c3b259 100644 --- a/cli/defradb/cmd/start.go +++ b/cli/defradb/cmd/start.go @@ -32,7 +32,7 @@ import ( badger "github.com/dgraph-io/badger/v3" ds "github.com/ipfs/go-datastore" - "github.com/sourcenetwork/defradb/api" + "github.com/sourcenetwork/defradb/api/http" "github.com/sourcenetwork/defradb/logging" "github.com/spf13/cobra" "github.com/textileio/go-threads/broadcast" @@ -178,7 +178,7 @@ var startCmd = &cobra.Command{ config.Database.Address, ), ) - s := api.NewHTTPServer(db) + s := http.NewServer(db) if err := s.Listen(config.Database.Address); err != nil { log.ErrorE(ctx, "Failed to start HTTP API listener", err) if n != nil { From 38ca252895c7952d5f50547d98fe5b3cce6e060e Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Thu, 28 Apr 2022 00:02:23 -0400 Subject: [PATCH 07/19] logger as package level logger and more Add better error handling. Remove the context struct. Use context.Context to pass db client. Remove handler options. --- api/http/errors.go | 68 +++++++++++++++++ api/http/errors_test.go | 58 +++++++++++++++ api/http/handler.go | 67 ++++++++--------- api/http/handler_test.go | 36 +-------- api/http/handlerfuncs.go | 156 +++++++++++++++++++++++++-------------- api/http/logger.go | 26 +++---- api/http/logger_test.go | 8 +- api/http/router.go | 6 +- api/http/server.go | 4 +- 9 files changed, 278 insertions(+), 151 deletions(-) create mode 100644 api/http/errors.go create mode 100644 api/http/errors_test.go diff --git a/api/http/errors.go b/api/http/errors.go new file mode 100644 index 0000000000..1eb42f451a --- /dev/null +++ b/api/http/errors.go @@ -0,0 +1,68 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "context" + "fmt" + "net/http" + "os" + "strings" +) + +const ( + errBadRequest = "Bad Request" + errInternalServerError = "Internal Server Error" + errNotFound = "Not Found" +) + +var env = os.Getenv("ENV") + +type errorResponse struct { + Status int `json:"status"` + Message string `json:"message"` + Stack string `json:"stack,omitempty"` +} + +func handleErr(rw http.ResponseWriter, err error, status int) { + var message string + + switch status { + case http.StatusBadRequest: + message = errBadRequest + + case http.StatusInternalServerError: + message = errInternalServerError + // @TODO: The internal server error log should be sent to a different location + // ideally not in the http logs. + log.ErrorE(context.Background(), errInternalServerError, err) + + case http.StatusNotFound: + message = errNotFound + } + + sendJSON( + rw, + errorResponse{ + Status: status, + Message: message, + Stack: formatError(err), + }, + status, + ) +} + +func formatError(err error) string { + if strings.ToLower(env) == "dev" || strings.ToLower(env) == "development" { + return fmt.Sprintf("[DEV] %+v\n", err) + } + return "" +} diff --git a/api/http/errors_test.go b/api/http/errors_test.go new file mode 100644 index 0000000000..9d949d584e --- /dev/null +++ b/api/http/errors_test.go @@ -0,0 +1,58 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" +) + +func TestFormatError(t *testing.T) { + env = "prod" + s := formatError(errors.New("test error")) + assert.Equal(t, "", s) + + env = "dev" + s = formatError(errors.New("test error")) + lines := strings.Split(s, "\n") + assert.Equal(t, "[DEV] test error", lines[0]) +} + +func TestHandleErr(t *testing.T) { + env = "dev" + f := func(rw http.ResponseWriter, req *http.Request) { + handleErr(rw, errors.New("test error"), http.StatusBadRequest) + } + req, err := http.NewRequest("GET", "/test", nil) + assert.NoError(t, err) + + rec := httptest.NewRecorder() + + f(rec, req) + + resp := rec.Result() + + errResponse := errorResponse{} + err = json.NewDecoder(resp.Body).Decode(&errResponse) + assert.NoError(t, err) + + assert.Equal(t, http.StatusBadRequest, errResponse.Status) + assert.Equal(t, "Bad Request", errResponse.Message) + + lines := strings.Split(errResponse.Stack, "\n") + assert.Equal(t, "[DEV] test error", lines[0]) +} diff --git a/api/http/handler.go b/api/http/handler.go index 04fc6a7100..e026923f5c 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -11,62 +11,55 @@ package http import ( + "context" + "encoding/json" + "fmt" + "io" "net/http" "github.com/go-chi/chi" + "github.com/pkg/errors" "github.com/sourcenetwork/defradb/client" - "github.com/sourcenetwork/defradb/logging" ) type Handler struct { db client.DB - *chi.Mux - *logger } -// newHandler returns a handler with the router instantiated and configuration applied. -func newHandler(db client.DB, options ...func(*Handler)) *Handler { - h := &Handler{ - db: db, - } +type ctxKey string - // apply options - for _, o := range options { - o(h) - } +// newHandler returns a handler with the router instantiated. +func newHandler(db client.DB) *Handler { + return setRoutes(&Handler{db: db}) +} - // ensure we have a logger defined - if h.logger == nil { - h.logger = defaultLogger() +func (h *Handler) handle(f http.HandlerFunc) http.HandlerFunc { + return func(rw http.ResponseWriter, req *http.Request) { + ctx := context.WithValue(req.Context(), ctxKey("DB"), h.db) + f(rw, req.WithContext(ctx)) } - - h.setRoutes() - - return h } -// WithLogger returns an option loading function for logger. -func WithLogger(l logging.Logger) func(*Handler) { - return func(h *Handler) { - h.logger = &logger{l} +func getJSON(r *http.Request, v interface{}) error { + err := json.NewDecoder(r.Body).Decode(v) + if err != nil { + return errors.Wrap(err, "unmarshall error") } + return nil } -type requestContext struct { - res http.ResponseWriter - req *http.Request - db client.DB - log *logger -} +func sendJSON(rw http.ResponseWriter, v interface{}, code int) { + rw.Header().Set("Content-Type", "application/json") -func (h *Handler) handle(f func(*requestContext)) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - f(&requestContext{ - res: w, - req: r, - db: h.db, - log: h.logger, - }) + b, err := json.Marshal(v) + if err != nil { + log.Error(context.Background(), fmt.Sprintf("Error while encoding JSON: %v", err)) + rw.WriteHeader(http.StatusInternalServerError) + io.WriteString(rw, `{"error": "Internal server error"}`) + return } + + rw.WriteHeader(code) + rw.Write(b) } diff --git a/api/http/handler_test.go b/api/http/handler_test.go index 668e851b2a..86956d8b25 100644 --- a/api/http/handler_test.go +++ b/api/http/handler_test.go @@ -27,7 +27,7 @@ func TestNewHandlerWithLogger(t *testing.T) { // send logs to temp file so we can inspect it logFile := path.Join(dir, "http_test.log") - h.ApplyConfig(logging.Config{ + log.ApplyConfig(logging.Config{ EncoderFormat: logging.NewEncoderFormatOption(logging.JSON), OutputPaths: []string{logFile}, }) @@ -35,10 +35,10 @@ func TestNewHandlerWithLogger(t *testing.T) { req, err := http.NewRequest("GET", "/ping", nil) assert.NoError(t, err) - rec2 := httptest.NewRecorder() + rec := httptest.NewRecorder() - h.logger.middleware(h.handle(ping)).ServeHTTP(rec2, req) - assert.Equal(t, 200, rec2.Result().StatusCode) + loggerMiddleware(h.handle(ping)).ServeHTTP(rec, req) + assert.Equal(t, 200, rec.Result().StatusCode) // inspect the log file kv, err := readLog(logFile) @@ -47,31 +47,3 @@ func TestNewHandlerWithLogger(t *testing.T) { assert.Equal(t, "defra.http", kv["logger"]) } - -func TestNewHandlerWithConfigAndLogger(t *testing.T) { - h := newHandler(nil, WithLogger(logging.MustNewLogger("defra.http.test"))) - - dir := t.TempDir() - - // send logs to temp file so we can inspect it - logFile := path.Join(dir, "http_test.log") - h.ApplyConfig(logging.Config{ - EncoderFormat: logging.NewEncoderFormatOption(logging.JSON), - OutputPaths: []string{logFile}, - }) - - req, err := http.NewRequest("GET", "/ping", nil) - assert.NoError(t, err) - - rec2 := httptest.NewRecorder() - - h.logger.middleware(h.handle(ping)).ServeHTTP(rec2, req) - assert.Equal(t, 200, rec2.Result().StatusCode) - - // inspect the log file - kv, err := readLog(logFile) - assert.NoError(t, err) - - assert.Equal(t, "defra.http.test", kv["logger"]) - -} diff --git a/api/http/handlerfuncs.go b/api/http/handlerfuncs.go index ab81eb4496..274fc5defd 100644 --- a/api/http/handlerfuncs.go +++ b/api/http/handlerfuncs.go @@ -21,97 +21,134 @@ import ( dshelp "github.com/ipfs/go-ipfs-ds-help" dag "github.com/ipfs/go-merkledag" "github.com/multiformats/go-multihash" + "github.com/pkg/errors" "github.com/sourcenetwork/defradb/client" corecrdt "github.com/sourcenetwork/defradb/core/crdt" ) -func root(c *requestContext) { - _, err := c.res.Write( +func root(rw http.ResponseWriter, req *http.Request) { + _, err := rw.Write( []byte("Welcome to the DefraDB HTTP API. Use /graphql to send queries to the database"), ) if err != nil { - c.log.ErrorE(c.req.Context(), "DefraDB HTTP API Welcome message writing failed", err) + handleErr( + rw, + errors.WithMessage( + err, + "DefraDB HTTP API Welcome message writing failed", + ), + http.StatusInternalServerError, + ) } } -func ping(c *requestContext) { - _, err := c.res.Write([]byte("pong")) +func ping(rw http.ResponseWriter, req *http.Request) { + _, err := rw.Write([]byte("pong")) if err != nil { - c.log.ErrorE(c.req.Context(), "Writing pong with HTTP failed", err) + handleErr( + rw, + errors.WithMessage( + err, + "Writing pong with HTTP failed", + ), + http.StatusInternalServerError, + ) } } -func dump(c *requestContext) { - c.db.PrintDump(c.req.Context()) +func dump(rw http.ResponseWriter, req *http.Request) { + db, ok := req.Context().Value(ctxKey("DB")).(client.DB) + if !ok { + handleErr(rw, errors.New("no database available"), http.StatusInternalServerError) + return + } + db.PrintDump(req.Context()) - _, err := c.res.Write([]byte("ok")) + _, err := rw.Write([]byte("ok")) if err != nil { - c.log.ErrorE(c.req.Context(), "Writing ok with HTTP failed", err) + handleErr( + rw, + errors.WithMessage( + err, + "Writing ok with HTTP failed", + ), + http.StatusInternalServerError, + ) } } -func execGQL(c *requestContext) { +func execGQL(rw http.ResponseWriter, req *http.Request) { var query string - if c.req.Method == "GET" { - query = c.req.URL.Query().Get("query") + if req.Method == "GET" { + query = req.URL.Query().Get("query") } else { - body, err := io.ReadAll(c.req.Body) + body, err := io.ReadAll(req.Body) if err != nil { - http.Error(c.res, err.Error(), http.StatusBadRequest) + handleErr(rw, errors.WithStack(err), http.StatusBadRequest) return } query = string(body) } if query == "" { - http.Error(c.res, "missing GraphQL query", http.StatusBadRequest) + handleErr(rw, errors.New("missing GraphQL query"), http.StatusBadRequest) return } - result := c.db.ExecQuery(c.req.Context(), query) + db, ok := req.Context().Value(ctxKey("DB")).(client.DB) + if !ok { + handleErr(rw, errors.New("no database available"), http.StatusInternalServerError) + return + } + result := db.ExecQuery(req.Context(), query) - err := json.NewEncoder(c.res).Encode(result) + err := json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusBadRequest) return } } -func loadSchema(c *requestContext) { +func loadSchema(rw http.ResponseWriter, req *http.Request) { var result client.QueryResult - sdl, err := io.ReadAll(c.req.Body) + sdl, err := io.ReadAll(req.Body) defer func() { - err = c.req.Body.Close() + err = req.Body.Close() if err != nil { - c.log.ErrorE(c.req.Context(), "Error on body close", err) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) } }() if err != nil { result.Errors = []interface{}{err.Error()} - err = json.NewEncoder(c.res).Encode(result) + err = json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) return } - c.res.WriteHeader(http.StatusBadRequest) + rw.WriteHeader(http.StatusBadRequest) return } - err = c.db.AddSchema(c.req.Context(), string(sdl)) + db, ok := req.Context().Value(ctxKey("DB")).(client.DB) + if !ok { + handleErr(rw, errors.New("no database available"), http.StatusInternalServerError) + return + } + err = db.AddSchema(req.Context(), string(sdl)) if err != nil { result.Errors = []interface{}{err.Error()} - err = json.NewEncoder(c.res).Encode(result) + err = json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) return } - c.res.WriteHeader(http.StatusBadRequest) + rw.WriteHeader(http.StatusBadRequest) return } @@ -119,16 +156,16 @@ func loadSchema(c *requestContext) { "result": "success", } - err = json.NewEncoder(c.res).Encode(result) + err = json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) return } } -func getBlock(c *requestContext) { +func getBlock(rw http.ResponseWriter, req *http.Request) { var result client.QueryResult - cidStr := chi.URLParam(c.req, "cid") + cidStr := chi.URLParam(req, "cid") // try to parse CID cID, err := cid.Decode(cidStr) @@ -142,29 +179,34 @@ func getBlock(c *requestContext) { result.Errors = []interface{}{err.Error()} result.Data = err.Error() - err = json.NewEncoder(c.res).Encode(result) + err = json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) return } - c.res.WriteHeader(http.StatusBadRequest) + rw.WriteHeader(http.StatusBadRequest) return } cID = cid.NewCidV1(cid.Raw, hash) } - block, err := c.db.Blockstore().Get(c.req.Context(), cID) + db, ok := req.Context().Value(ctxKey("DB")).(client.DB) + if !ok { + handleErr(rw, errors.New("no database available"), http.StatusInternalServerError) + return + } + block, err := db.Blockstore().Get(req.Context(), cID) if err != nil { result.Errors = []interface{}{err.Error()} - err = json.NewEncoder(c.res).Encode(result) + err = json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) return } - c.res.WriteHeader(http.StatusBadRequest) + rw.WriteHeader(http.StatusBadRequest) return } @@ -173,26 +215,26 @@ func getBlock(c *requestContext) { result.Errors = []interface{}{err.Error()} result.Data = err.Error() - err = json.NewEncoder(c.res).Encode(result) + err = json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) return } - c.res.WriteHeader(http.StatusBadRequest) + rw.WriteHeader(http.StatusBadRequest) return } buf, err := nd.MarshalJSON() if err != nil { result.Errors = []interface{}{err.Error()} - err = json.NewEncoder(c.res).Encode(result) + err = json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) return } - c.res.WriteHeader(http.StatusBadRequest) + rw.WriteHeader(http.StatusBadRequest) return } @@ -201,13 +243,13 @@ func getBlock(c *requestContext) { if err != nil { result.Errors = []interface{}{err.Error()} - err = json.NewEncoder(c.res).Encode(result) + err = json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) return } - c.res.WriteHeader(http.StatusBadRequest) + rw.WriteHeader(http.StatusBadRequest) return } @@ -215,13 +257,13 @@ func getBlock(c *requestContext) { if err != nil { result.Errors = []interface{}{err.Error()} - err = json.NewEncoder(c.res).Encode(result) + err = json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) return } - c.res.WriteHeader(http.StatusBadRequest) + rw.WriteHeader(http.StatusBadRequest) return } @@ -231,20 +273,20 @@ func getBlock(c *requestContext) { "val": delta.Value(), } - enc := json.NewEncoder(c.res) + enc := json.NewEncoder(rw) enc.SetIndent("", "\t") err = enc.Encode(result) if err != nil { result.Errors = []interface{}{err.Error()} result.Data = nil - err := json.NewEncoder(c.res).Encode(result) + err := json.NewEncoder(rw).Encode(result) if err != nil { - http.Error(c.res, err.Error(), http.StatusInternalServerError) + handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) return } - c.res.WriteHeader(http.StatusBadRequest) + rw.WriteHeader(http.StatusBadRequest) return } } diff --git a/api/http/logger.go b/api/http/logger.go index 0dfd73ab6a..15abf46395 100644 --- a/api/http/logger.go +++ b/api/http/logger.go @@ -18,15 +18,7 @@ import ( "github.com/sourcenetwork/defradb/logging" ) -type logger struct { - logging.Logger -} - -func defaultLogger() *logger { - return &logger{ - Logger: logging.MustNewLogger("defra.http"), - } -} +var log = logging.MustNewLogger("defra.http") type loggingResponseWriter struct { statusCode int @@ -57,22 +49,22 @@ func (lrw *loggingResponseWriter) Write(b []byte) (int, error) { return lrw.ResponseWriter.Write(b) } -func (l *logger) middleware(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +func loggerMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { start := time.Now() - lrw := newLoggingResponseWriter(w) - next.ServeHTTP(lrw, r) + lrw := newLoggingResponseWriter(rw) + next.ServeHTTP(lrw, req) elapsed := time.Since(start) - l.Info( - r.Context(), + log.Info( + req.Context(), "Request", logging.NewKV( "Method", - r.Method, + req.Method, ), logging.NewKV( "Path", - r.URL.Path, + req.URL.Path, ), logging.NewKV( "Status", diff --git a/api/http/logger_test.go b/api/http/logger_test.go index 4ffa8dda66..ec6ce0e362 100644 --- a/api/http/logger_test.go +++ b/api/http/logger_test.go @@ -50,12 +50,12 @@ func TestLoggerLogs(t *testing.T) { rec2 := httptest.NewRecorder() - h := newHandler(nil, WithLogger(logging.MustNewLogger("defra.http.test"))) - h.logger.ApplyConfig(logging.Config{ + h := newHandler(nil) + log.ApplyConfig(logging.Config{ EncoderFormat: logging.NewEncoderFormatOption(logging.JSON), OutputPaths: []string{logFile}, }) - h.logger.middleware(h.handle(ping)).ServeHTTP(rec2, req) + loggerMiddleware(h.handle(ping)).ServeHTTP(rec2, req) assert.Equal(t, 200, rec2.Result().StatusCode) // inspect the log file @@ -65,7 +65,7 @@ func TestLoggerLogs(t *testing.T) { // check that everything is as expected assert.Equal(t, "pong", rec2.Body.String()) assert.Equal(t, "INFO", kv["level"]) - assert.Equal(t, "defra.http.test", kv["logger"]) + assert.Equal(t, "defra.http", kv["logger"]) assert.Equal(t, "Request", kv["msg"]) assert.Equal(t, "GET", kv["Method"]) assert.Equal(t, "/ping", kv["Path"]) diff --git a/api/http/router.go b/api/http/router.go index 5c34aa7b23..dde1fe11c7 100644 --- a/api/http/router.go +++ b/api/http/router.go @@ -14,11 +14,11 @@ import ( "github.com/go-chi/chi" ) -func (h *Handler) setRoutes() { +func setRoutes(h *Handler) *Handler { h.Mux = chi.NewRouter() // setup logger middleware - h.Use(h.logger.middleware) + h.Use(loggerMiddleware) // define routes h.Get("/", h.handle(root)) @@ -28,4 +28,6 @@ func (h *Handler) setRoutes() { h.Get("/graphql", h.handle(execGQL)) h.Post("/graphql", h.handle(execGQL)) h.Post("/schema/load", h.handle(loadSchema)) + + return h } diff --git a/api/http/server.go b/api/http/server.go index 441d087564..bf838ef62e 100644 --- a/api/http/server.go +++ b/api/http/server.go @@ -22,9 +22,9 @@ type Server struct { } // NewServer instantiated a new server with the given http.Handler. -func NewServer(db client.DB, handlerOptions ...func(*Handler)) *Server { +func NewServer(db client.DB) *Server { return &Server{ - Handler: newHandler(db, handlerOptions...), + Handler: newHandler(db), } } From f2ffd8bace4e708eb298c43dae53b6ca56fc9323 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Thu, 28 Apr 2022 00:46:11 -0400 Subject: [PATCH 08/19] fix unhandled errors, dead code and gql query --- api/http/errors.go | 4 ++++ api/http/handler.go | 17 ++++++---------- api/http/handlerfuncs.go | 44 ++++++++++++++++++++++++++++++++-------- 3 files changed, 46 insertions(+), 19 deletions(-) diff --git a/api/http/errors.go b/api/http/errors.go index 1eb42f451a..d06980f170 100644 --- a/api/http/errors.go +++ b/api/http/errors.go @@ -49,6 +49,10 @@ func handleErr(rw http.ResponseWriter, err error, status int) { message = errNotFound } + if message == "" { + message = err.Error() + } + sendJSON( rw, errorResponse{ diff --git a/api/http/handler.go b/api/http/handler.go index e026923f5c..7a847a1cbd 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -18,7 +18,6 @@ import ( "net/http" "github.com/go-chi/chi" - "github.com/pkg/errors" "github.com/sourcenetwork/defradb/client" ) @@ -41,14 +40,6 @@ func (h *Handler) handle(f http.HandlerFunc) http.HandlerFunc { } } -func getJSON(r *http.Request, v interface{}) error { - err := json.NewDecoder(r.Body).Decode(v) - if err != nil { - return errors.Wrap(err, "unmarshall error") - } - return nil -} - func sendJSON(rw http.ResponseWriter, v interface{}, code int) { rw.Header().Set("Content-Type", "application/json") @@ -56,10 +47,14 @@ func sendJSON(rw http.ResponseWriter, v interface{}, code int) { if err != nil { log.Error(context.Background(), fmt.Sprintf("Error while encoding JSON: %v", err)) rw.WriteHeader(http.StatusInternalServerError) - io.WriteString(rw, `{"error": "Internal server error"}`) + if _, err := io.WriteString(rw, `{"error": "Internal server error"}`); err != nil { + log.Error(context.Background(), err.Error()) + } return } rw.WriteHeader(code) - rw.Write(b) + if _, err = rw.Write(b); err != nil { + log.Error(context.Background(), err.Error()) + } } diff --git a/api/http/handlerfuncs.go b/api/http/handlerfuncs.go index 274fc5defd..19ed8ba258 100644 --- a/api/http/handlerfuncs.go +++ b/api/http/handlerfuncs.go @@ -26,6 +26,12 @@ import ( corecrdt "github.com/sourcenetwork/defradb/core/crdt" ) +const ( + ContentTypeJSON = "application/json" + ContentTypeGraphQL = "application/graphql" + ContentTypeFormURLEncoded = "application/x-www-form-urlencoded" +) + func root(rw http.ResponseWriter, req *http.Request) { _, err := rw.Write( []byte("Welcome to the DefraDB HTTP API. Use /graphql to send queries to the database"), @@ -78,18 +84,40 @@ func dump(rw http.ResponseWriter, req *http.Request) { } func execGQL(rw http.ResponseWriter, req *http.Request) { - var query string - if req.Method == "GET" { - query = req.URL.Query().Get("query") - } else { - body, err := io.ReadAll(req.Body) - if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusBadRequest) + query := req.URL.Query().Get("query") + + if query == "" { + switch req.Header.Get("Content-Type") { + case ContentTypeJSON: + handleErr( + rw, + errors.New("content type application/json not yet supported"), + http.StatusBadRequest, + ) + return + + case ContentTypeFormURLEncoded: + handleErr( + rw, + errors.New("content type application/x-www-form-urlencoded not yet supported"), + http.StatusBadRequest, + ) return + + case ContentTypeGraphQL: + fallthrough + + default: + body, err := io.ReadAll(req.Body) + if err != nil { + handleErr(rw, errors.WithStack(err), http.StatusBadRequest) + return + } + query = string(body) } - query = string(body) } + // if at this point query is still empty, return an error if query == "" { handleErr(rw, errors.New("missing GraphQL query"), http.StatusBadRequest) return From 9bb07d5bc882237c3ec583971795635871fee76f Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Thu, 28 Apr 2022 01:10:25 -0400 Subject: [PATCH 09/19] api version in path --- README.md | 2 +- api/http/router.go | 22 +++++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index c8954b9d70..172287e786 100644 --- 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/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/v1/graphql`. ### Add a Schema type diff --git a/api/http/router.go b/api/http/router.go index dde1fe11c7..4ea627161e 100644 --- a/api/http/router.go +++ b/api/http/router.go @@ -11,9 +11,13 @@ package http import ( + "path" + "github.com/go-chi/chi" ) +const version = "/api/v1" + func setRoutes(h *Handler) *Handler { h.Mux = chi.NewRouter() @@ -21,13 +25,17 @@ func setRoutes(h *Handler) *Handler { h.Use(loggerMiddleware) // define routes - h.Get("/", h.handle(root)) - h.Get("/ping", h.handle(ping)) - h.Get("/dump", h.handle(dump)) - h.Get("/blocks/get/{cid}", h.handle(getBlock)) - h.Get("/graphql", h.handle(execGQL)) - h.Post("/graphql", h.handle(execGQL)) - h.Post("/schema/load", h.handle(loadSchema)) + h.Get(apiPath("/"), h.handle(root)) + h.Get(apiPath("/ping"), h.handle(ping)) + h.Get(apiPath("/dump"), h.handle(dump)) + h.Get(apiPath("/blocks/get/{cid}"), h.handle(getBlock)) + h.Get(apiPath("/graphql"), h.handle(execGQL)) + h.Post(apiPath("/graphql"), h.handle(execGQL)) + h.Post(apiPath("/schema/load"), h.handle(loadSchema)) return h } + +func apiPath(pattern string) string { + return path.Join(version, pattern) +} From 64aad492c3240c2c62fe8eb7614cd76ea4904740 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Thu, 28 Apr 2022 19:40:41 -0400 Subject: [PATCH 10/19] Add http api version to endpoint path --- api/http/router.go | 44 +++++++++++++++++++++++++++-------- cli/defradb/cmd/blocks_get.go | 4 ++-- cli/defradb/cmd/dump.go | 3 ++- cli/defradb/cmd/ping.go | 3 ++- cli/defradb/cmd/query.go | 4 ++-- cli/defradb/cmd/schema_add.go | 4 ++-- 6 files changed, 44 insertions(+), 18 deletions(-) diff --git a/api/http/router.go b/api/http/router.go index 4ea627161e..9a65f10281 100644 --- a/api/http/router.go +++ b/api/http/router.go @@ -11,12 +11,23 @@ package http import ( + "context" + "net/url" "path" "github.com/go-chi/chi" ) -const version = "/api/v1" +const ( + version string = "/api/v1" + + HomePath string = version + "/" + PingPath string = version + "/ping" + DumpPath string = version + "/dump" + BlocksPath string = version + "/blocks/get" + GraphQLPath string = version + "/graphql" + SchemaLoadPath string = version + "/schema/load" +) func setRoutes(h *Handler) *Handler { h.Mux = chi.NewRouter() @@ -25,17 +36,30 @@ func setRoutes(h *Handler) *Handler { h.Use(loggerMiddleware) // define routes - h.Get(apiPath("/"), h.handle(root)) - h.Get(apiPath("/ping"), h.handle(ping)) - h.Get(apiPath("/dump"), h.handle(dump)) - h.Get(apiPath("/blocks/get/{cid}"), h.handle(getBlock)) - h.Get(apiPath("/graphql"), h.handle(execGQL)) - h.Post(apiPath("/graphql"), h.handle(execGQL)) - h.Post(apiPath("/schema/load"), h.handle(loadSchema)) + h.Get(HomePath, h.handle(root)) + h.Get(PingPath, h.handle(ping)) + h.Get(DumpPath, h.handle(dump)) + h.Get(BlocksPath+"/{cid}", h.handle(getBlock)) + h.Get(GraphQLPath, h.handle(execGQL)) + h.Post(GraphQLPath, h.handle(execGQL)) + h.Post(SchemaLoadPath, h.handle(loadSchema)) return h } -func apiPath(pattern string) string { - return path.Join(version, pattern) +// JoinPaths takes a base path and any number of additionnal paths +// and combines them safely to form a full URL path or a simple path if +// the base parameter is not a valid URL starting with `http://` or `https://`. +func JoinPaths(base string, paths ...string) string { + u, err := url.Parse(base) + if err != nil { + log.Error(context.Background(), err.Error()) + paths = append(([]string{base}), paths...) + return path.Join(paths...) + } + + paths = append(([]string{u.Path}), paths...) + u.Path = path.Join(paths...) + + return u.String() } diff --git a/cli/defradb/cmd/blocks_get.go b/cli/defradb/cmd/blocks_get.go index a76e5e437f..3ca064093d 100644 --- a/cli/defradb/cmd/blocks_get.go +++ b/cli/defradb/cmd/blocks_get.go @@ -12,11 +12,11 @@ package cmd import ( "context" - "fmt" "io" "net/http" "strings" + httpapi "github.com/sourcenetwork/defradb/api/http" "github.com/sourcenetwork/defradb/logging" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -43,7 +43,7 @@ var getCmd = &cobra.Command{ } cid := args[0] - res, err := http.Get(fmt.Sprintf("%s/blocks/get/%s", dbaddr, cid)) + res, err := http.Get(httpapi.JoinPaths(dbaddr, httpapi.BlocksPath, cid)) if err != nil { log.ErrorE(ctx, "request failed", err) return diff --git a/cli/defradb/cmd/dump.go b/cli/defradb/cmd/dump.go index 9b7098b638..f4609bfab1 100644 --- a/cli/defradb/cmd/dump.go +++ b/cli/defradb/cmd/dump.go @@ -17,6 +17,7 @@ import ( "net/http" "strings" + httpapi "github.com/sourcenetwork/defradb/api/http" "github.com/sourcenetwork/defradb/logging" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -38,7 +39,7 @@ var dumpCmd = &cobra.Command{ dbaddr = "http://" + dbaddr } - res, err := http.Get(fmt.Sprintf("%s/dump", dbaddr)) + res, err := http.Get(httpapi.JoinPaths(dbaddr, httpapi.DumpPath)) if err != nil { log.ErrorE(ctx, "request failed", err) return diff --git a/cli/defradb/cmd/ping.go b/cli/defradb/cmd/ping.go index 7e62cde35d..9b508e7ce7 100644 --- a/cli/defradb/cmd/ping.go +++ b/cli/defradb/cmd/ping.go @@ -17,6 +17,7 @@ import ( "net/http" "strings" + httpapi "github.com/sourcenetwork/defradb/api/http" "github.com/sourcenetwork/defradb/logging" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -39,7 +40,7 @@ var pingCmd = &cobra.Command{ } log.Info(ctx, "Sending ping...") - res, err := http.Get(fmt.Sprintf("%s/ping", dbaddr)) + res, err := http.Get(httpapi.JoinPaths(dbaddr, httpapi.PingPath)) if err != nil { log.ErrorE(ctx, "request failed", err) return diff --git a/cli/defradb/cmd/query.go b/cli/defradb/cmd/query.go index 265b62a067..07d4e573b6 100644 --- a/cli/defradb/cmd/query.go +++ b/cli/defradb/cmd/query.go @@ -12,12 +12,12 @@ package cmd import ( "context" - "fmt" "io" "net/http" "net/url" "strings" + httpapi "github.com/sourcenetwork/defradb/api/http" "github.com/sourcenetwork/defradb/logging" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -60,7 +60,7 @@ the additional documentation found at: https://hackmd.io/@source/BksQY6Qfw. log.Error(ctx, "missing query") return } - endpointStr := fmt.Sprintf("%s/graphql", dbaddr) + endpointStr := httpapi.JoinPaths(dbaddr, httpapi.GraphQLPath) endpoint, err := url.Parse(endpointStr) if err != nil { log.FatalE(ctx, "", err) diff --git a/cli/defradb/cmd/schema_add.go b/cli/defradb/cmd/schema_add.go index 10f94ed5ca..e843799cfb 100644 --- a/cli/defradb/cmd/schema_add.go +++ b/cli/defradb/cmd/schema_add.go @@ -13,13 +13,13 @@ package cmd import ( "bytes" "context" - "fmt" "io" "net/http" "net/url" "os" "strings" + httpapi "github.com/sourcenetwork/defradb/api/http" "github.com/sourcenetwork/defradb/logging" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -57,7 +57,7 @@ var addCmd = &cobra.Command{ if !strings.HasPrefix(dbaddr, "http") { dbaddr = "http://" + dbaddr } - endpointStr := fmt.Sprintf("%s/schema/load", dbaddr) + endpointStr := httpapi.JoinPaths(dbaddr, httpapi.SchemaLoadPath) endpoint, err := url.Parse(endpointStr) cobra.CheckErr(err) From 871eebfb79506ec9a23a9220bb3ba25163110a58 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Thu, 28 Apr 2022 19:52:09 -0400 Subject: [PATCH 11/19] Update chi version --- api/http/handler.go | 2 +- api/http/handlerfuncs.go | 2 +- api/http/router.go | 4 ++-- go.mod | 2 +- go.sum | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/api/http/handler.go b/api/http/handler.go index 7a847a1cbd..b802e2aec1 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -17,7 +17,7 @@ import ( "io" "net/http" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/sourcenetwork/defradb/client" ) diff --git a/api/http/handlerfuncs.go b/api/http/handlerfuncs.go index 19ed8ba258..4e64f3f2c4 100644 --- a/api/http/handlerfuncs.go +++ b/api/http/handlerfuncs.go @@ -15,7 +15,7 @@ import ( "io" "net/http" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/ipfs/go-cid" ds "github.com/ipfs/go-datastore" dshelp "github.com/ipfs/go-ipfs-ds-help" diff --git a/api/http/router.go b/api/http/router.go index 9a65f10281..4ff2519897 100644 --- a/api/http/router.go +++ b/api/http/router.go @@ -15,7 +15,7 @@ import ( "net/url" "path" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" ) const ( @@ -23,7 +23,7 @@ const ( HomePath string = version + "/" PingPath string = version + "/ping" - DumpPath string = version + "/dump" + DumpPath string = version + "/debug/dump" BlocksPath string = version + "/blocks/get" GraphQLPath string = version + "/graphql" SchemaLoadPath string = version + "/schema/load" diff --git a/go.mod b/go.mod index 9b829a11eb..1b7a3ea779 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/davecgh/go-spew v1.1.1 github.com/dgraph-io/badger/v3 v3.2103.2 github.com/fxamacker/cbor/v2 v2.2.0 - github.com/go-chi/chi v1.5.2 github.com/gogo/protobuf v1.3.2 github.com/google/uuid v1.3.0 // indirect github.com/graphql-go/graphql v0.7.9 @@ -50,6 +49,7 @@ require ( require ( github.com/fatih/color v1.13.0 + github.com/go-chi/chi/v5 v5.0.7 github.com/pkg/errors v0.9.1 ) diff --git a/go.sum b/go.sum index 8e3448c27a..36d9bcf143 100644 --- a/go.sum +++ b/go.sum @@ -248,8 +248,8 @@ github.com/gin-contrib/sse v0.1.0/go.mod h1:RHrZQHXnP2xjPF+u1gW/2HnVO7nvIa9PG3Gm github.com/gin-gonic/gin v1.6.3/go.mod h1:75u5sXoLsGZoRN5Sgbi1eraJ4GU3++wFwWzhwvtwp4M= github.com/gliderlabs/ssh v0.1.1/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aevW3Awn0= github.com/go-check/check v0.0.0-20180628173108-788fd7840127/go.mod h1:9ES+weclKsC9YodN5RgxqK/VD9HM9JsCSh7rNhMZE98= -github.com/go-chi/chi v1.5.2 h1:YcLIBANL4OTaAOcTdp//sskGa0yGACQMCtbnr7YEn0Q= -github.com/go-chi/chi v1.5.2/go.mod h1:REp24E+25iKvxgeTfHmdUoL5x15kBiDBlnIl5bCwe2k= +github.com/go-chi/chi/v5 v5.0.7 h1:rDTPXLDHGATaeHvVlLcR4Qe0zftYethFucbjVQ1PxU8= +github.com/go-chi/chi/v5 v5.0.7/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8= github.com/go-delve/delve v1.5.0/go.mod h1:c6b3a1Gry6x8a4LGCe/CWzrocrfaHvkUxCj3k4bvSUQ= github.com/go-errors/errors v1.0.1/go.mod h1:f4zRHt4oKfwPJE5k8C9vpYG+aDHdBFUsgrm6/TyX73Q= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= From 6dcbf1d19288a8fef347883b0daf7652e9828542 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Thu, 28 Apr 2022 19:56:41 -0400 Subject: [PATCH 12/19] Add TestJoinPaths --- api/http/router_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 api/http/router_test.go diff --git a/api/http/router_test.go b/api/http/router_test.go new file mode 100644 index 0000000000..03e4459b3f --- /dev/null +++ b/api/http/router_test.go @@ -0,0 +1,22 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestJoinPaths(t *testing.T) { + path := JoinPaths("http://localhost:9181", BlocksPath, "cid_of_some_sort") + assert.Equal(t, "http://localhost:9181"+BlocksPath+"/cid_of_some_sort", path) +} From 5209812e87c34ebff97373ec0763091fafc29e4e Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Tue, 3 May 2022 00:50:44 -0400 Subject: [PATCH 13/19] Resolve review comments Move log to api.go Add more specific unit test Add ctx to handleErr Add suffix to handlerfuncs name Make Handler local Fix JoinPaths --- api/http/api.go | 15 +++++++ api/http/errors.go | 8 ++-- api/http/errors_test.go | 81 +++++++++++++++++++++++++++++++-- api/http/handler.go | 26 +++++++---- api/http/handler_test.go | 2 +- api/http/handlerfuncs.go | 85 ++++++++++++++++++----------------- api/http/logger.go | 2 - api/http/logger_test.go | 2 +- api/http/router.go | 42 +++++++++-------- api/http/router_test.go | 35 +++++++++++++-- cli/defradb/cmd/blocks_get.go | 8 +++- cli/defradb/cmd/dump.go | 8 +++- cli/defradb/cmd/ping.go | 9 +++- cli/defradb/cmd/query.go | 7 +-- cli/defradb/cmd/schema_add.go | 9 ++-- cli/defradb/cmd/start.go | 5 ++- 16 files changed, 252 insertions(+), 92 deletions(-) create mode 100644 api/http/api.go diff --git a/api/http/api.go b/api/http/api.go new file mode 100644 index 0000000000..cea5da641a --- /dev/null +++ b/api/http/api.go @@ -0,0 +1,15 @@ +// Copyright 2022 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package http + +import "github.com/sourcenetwork/defradb/logging" + +var log = logging.MustNewLogger("defra.http") diff --git a/api/http/errors.go b/api/http/errors.go index d06980f170..cf405d9745 100644 --- a/api/http/errors.go +++ b/api/http/errors.go @@ -24,7 +24,7 @@ const ( errNotFound = "Not Found" ) -var env = os.Getenv("ENV") +var env = os.Getenv("DEFRA_ENV") type errorResponse struct { Status int `json:"status"` @@ -32,7 +32,7 @@ type errorResponse struct { Stack string `json:"stack,omitempty"` } -func handleErr(rw http.ResponseWriter, err error, status int) { +func handleErr(ctx context.Context, rw http.ResponseWriter, err error, status int) { var message string switch status { @@ -47,13 +47,13 @@ func handleErr(rw http.ResponseWriter, err error, status int) { case http.StatusNotFound: message = errNotFound - } - if message == "" { + default: message = err.Error() } sendJSON( + ctx, rw, errorResponse{ Status: status, diff --git a/api/http/errors_test.go b/api/http/errors_test.go index 9d949d584e..5df271d964 100644 --- a/api/http/errors_test.go +++ b/api/http/errors_test.go @@ -32,10 +32,10 @@ func TestFormatError(t *testing.T) { assert.Equal(t, "[DEV] test error", lines[0]) } -func TestHandleErr(t *testing.T) { +func TestHandleErrOnBadRequest(t *testing.T) { env = "dev" f := func(rw http.ResponseWriter, req *http.Request) { - handleErr(rw, errors.New("test error"), http.StatusBadRequest) + handleErr(req.Context(), rw, errors.New("test error"), http.StatusBadRequest) } req, err := http.NewRequest("GET", "/test", nil) assert.NoError(t, err) @@ -51,8 +51,83 @@ func TestHandleErr(t *testing.T) { assert.NoError(t, err) assert.Equal(t, http.StatusBadRequest, errResponse.Status) - assert.Equal(t, "Bad Request", errResponse.Message) + assert.Equal(t, errBadRequest, errResponse.Message) lines := strings.Split(errResponse.Stack, "\n") assert.Equal(t, "[DEV] test error", lines[0]) } + +func TestHandleErrOnInternalServerError(t *testing.T) { + env = "dev" + f := func(rw http.ResponseWriter, req *http.Request) { + handleErr(req.Context(), rw, errors.New("test error"), http.StatusInternalServerError) + } + req, err := http.NewRequest("GET", "/test", nil) + assert.NoError(t, err) + + rec := httptest.NewRecorder() + + f(rec, req) + + resp := rec.Result() + + errResponse := errorResponse{} + err = json.NewDecoder(resp.Body).Decode(&errResponse) + assert.NoError(t, err) + + assert.Equal(t, http.StatusInternalServerError, errResponse.Status) + assert.Equal(t, errInternalServerError, errResponse.Message) + + lines := strings.Split(errResponse.Stack, "\n") + assert.Equal(t, "[DEV] test error", lines[0]) +} + +func TestHandleErrOnNotFound(t *testing.T) { + env = "dev" + f := func(rw http.ResponseWriter, req *http.Request) { + handleErr(req.Context(), rw, errors.New("test error"), http.StatusNotFound) + } + req, err := http.NewRequest("GET", "/test", nil) + assert.NoError(t, err) + + rec := httptest.NewRecorder() + + f(rec, req) + + resp := rec.Result() + + errResponse := errorResponse{} + err = json.NewDecoder(resp.Body).Decode(&errResponse) + assert.NoError(t, err) + + assert.Equal(t, http.StatusNotFound, errResponse.Status) + assert.Equal(t, errNotFound, errResponse.Message) + + lines := strings.Split(errResponse.Stack, "\n") + assert.Equal(t, "[DEV] test error", lines[0]) +} + +func TestHandleErrOnDefault(t *testing.T) { + env = "dev" + f := func(rw http.ResponseWriter, req *http.Request) { + handleErr(req.Context(), rw, errors.New("Unauthorized"), http.StatusUnauthorized) + } + req, err := http.NewRequest("GET", "/test", nil) + assert.NoError(t, err) + + rec := httptest.NewRecorder() + + f(rec, req) + + resp := rec.Result() + + errResponse := errorResponse{} + err = json.NewDecoder(resp.Body).Decode(&errResponse) + assert.NoError(t, err) + + assert.Equal(t, http.StatusUnauthorized, errResponse.Status) + assert.Equal(t, "Unauthorized", errResponse.Message) + + lines := strings.Split(errResponse.Stack, "\n") + assert.Equal(t, "[DEV] Unauthorized", lines[0]) +} diff --git a/api/http/handler.go b/api/http/handler.go index b802e2aec1..91f5d959fd 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -18,10 +18,11 @@ import ( "net/http" "github.com/go-chi/chi/v5" + "github.com/pkg/errors" "github.com/sourcenetwork/defradb/client" ) -type Handler struct { +type handler struct { db client.DB *chi.Mux } @@ -29,32 +30,41 @@ type Handler struct { type ctxKey string // newHandler returns a handler with the router instantiated. -func newHandler(db client.DB) *Handler { - return setRoutes(&Handler{db: db}) +func newHandler(db client.DB) *handler { + return setRoutes(&handler{db: db}) } -func (h *Handler) handle(f http.HandlerFunc) http.HandlerFunc { +func (h *handler) handle(f http.HandlerFunc) http.HandlerFunc { return func(rw http.ResponseWriter, req *http.Request) { ctx := context.WithValue(req.Context(), ctxKey("DB"), h.db) f(rw, req.WithContext(ctx)) } } -func sendJSON(rw http.ResponseWriter, v interface{}, code int) { +func sendJSON(ctx context.Context, rw http.ResponseWriter, v interface{}, code int) { rw.Header().Set("Content-Type", "application/json") b, err := json.Marshal(v) if err != nil { - log.Error(context.Background(), fmt.Sprintf("Error while encoding JSON: %v", err)) + log.Error(ctx, fmt.Sprintf("Error while encoding JSON: %v", err)) rw.WriteHeader(http.StatusInternalServerError) if _, err := io.WriteString(rw, `{"error": "Internal server error"}`); err != nil { - log.Error(context.Background(), err.Error()) + log.Error(ctx, err.Error()) } return } rw.WriteHeader(code) if _, err = rw.Write(b); err != nil { - log.Error(context.Background(), err.Error()) + log.Error(ctx, err.Error()) } } + +func dbFromContext(ctx context.Context) (client.DB, error) { + db, ok := ctx.Value(ctxKey("DB")).(client.DB) + if !ok { + return nil, errors.New("no database available") + } + + return db, nil +} diff --git a/api/http/handler_test.go b/api/http/handler_test.go index 86956d8b25..8f912a0ffe 100644 --- a/api/http/handler_test.go +++ b/api/http/handler_test.go @@ -37,7 +37,7 @@ func TestNewHandlerWithLogger(t *testing.T) { rec := httptest.NewRecorder() - loggerMiddleware(h.handle(ping)).ServeHTTP(rec, req) + loggerMiddleware(h.handle(pingHandler)).ServeHTTP(rec, req) assert.Equal(t, 200, rec.Result().StatusCode) // inspect the log file diff --git a/api/http/handlerfuncs.go b/api/http/handlerfuncs.go index 4e64f3f2c4..6f87790355 100644 --- a/api/http/handlerfuncs.go +++ b/api/http/handlerfuncs.go @@ -27,17 +27,18 @@ import ( ) const ( - ContentTypeJSON = "application/json" - ContentTypeGraphQL = "application/graphql" - ContentTypeFormURLEncoded = "application/x-www-form-urlencoded" + contentTypeJSON = "application/json" + contentTypeGraphQL = "application/graphql" + contentTypeFormURLEncoded = "application/x-www-form-urlencoded" ) -func root(rw http.ResponseWriter, req *http.Request) { +func rootHandler(rw http.ResponseWriter, req *http.Request) { _, err := rw.Write( []byte("Welcome to the DefraDB HTTP API. Use /graphql to send queries to the database"), ) if err != nil { handleErr( + req.Context(), rw, errors.WithMessage( err, @@ -48,10 +49,11 @@ func root(rw http.ResponseWriter, req *http.Request) { } } -func ping(rw http.ResponseWriter, req *http.Request) { +func pingHandler(rw http.ResponseWriter, req *http.Request) { _, err := rw.Write([]byte("pong")) if err != nil { handleErr( + req.Context(), rw, errors.WithMessage( err, @@ -62,17 +64,18 @@ func ping(rw http.ResponseWriter, req *http.Request) { } } -func dump(rw http.ResponseWriter, req *http.Request) { - db, ok := req.Context().Value(ctxKey("DB")).(client.DB) - if !ok { - handleErr(rw, errors.New("no database available"), http.StatusInternalServerError) +func dumpHandler(rw http.ResponseWriter, req *http.Request) { + db, err := dbFromContext(req.Context()) + if err != nil { + handleErr(req.Context(), rw, err, http.StatusInternalServerError) return } db.PrintDump(req.Context()) - _, err := rw.Write([]byte("ok")) + _, err = rw.Write([]byte("ok")) if err != nil { handleErr( + req.Context(), rw, errors.WithMessage( err, @@ -83,34 +86,36 @@ func dump(rw http.ResponseWriter, req *http.Request) { } } -func execGQL(rw http.ResponseWriter, req *http.Request) { +func execGQLHandler(rw http.ResponseWriter, req *http.Request) { query := req.URL.Query().Get("query") if query == "" { switch req.Header.Get("Content-Type") { - case ContentTypeJSON: + case contentTypeJSON: handleErr( + req.Context(), rw, errors.New("content type application/json not yet supported"), http.StatusBadRequest, ) return - case ContentTypeFormURLEncoded: + case contentTypeFormURLEncoded: handleErr( + req.Context(), rw, errors.New("content type application/x-www-form-urlencoded not yet supported"), http.StatusBadRequest, ) return - case ContentTypeGraphQL: + case contentTypeGraphQL: fallthrough default: body, err := io.ReadAll(req.Body) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusBadRequest) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusBadRequest) return } query = string(body) @@ -119,32 +124,32 @@ func execGQL(rw http.ResponseWriter, req *http.Request) { // if at this point query is still empty, return an error if query == "" { - handleErr(rw, errors.New("missing GraphQL query"), http.StatusBadRequest) + handleErr(req.Context(), rw, errors.New("missing GraphQL query"), http.StatusBadRequest) return } - db, ok := req.Context().Value(ctxKey("DB")).(client.DB) - if !ok { - handleErr(rw, errors.New("no database available"), http.StatusInternalServerError) + db, err := dbFromContext(req.Context()) + if err != nil { + handleErr(req.Context(), rw, err, http.StatusInternalServerError) return } result := db.ExecQuery(req.Context(), query) - err := json.NewEncoder(rw).Encode(result) + err = json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusBadRequest) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusBadRequest) return } } -func loadSchema(rw http.ResponseWriter, req *http.Request) { +func loadSchemaHandler(rw http.ResponseWriter, req *http.Request) { var result client.QueryResult sdl, err := io.ReadAll(req.Body) defer func() { err = req.Body.Close() if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) } }() @@ -153,7 +158,7 @@ func loadSchema(rw http.ResponseWriter, req *http.Request) { err = json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) return } @@ -161,9 +166,9 @@ func loadSchema(rw http.ResponseWriter, req *http.Request) { return } - db, ok := req.Context().Value(ctxKey("DB")).(client.DB) - if !ok { - handleErr(rw, errors.New("no database available"), http.StatusInternalServerError) + db, err := dbFromContext(req.Context()) + if err != nil { + handleErr(req.Context(), rw, err, http.StatusInternalServerError) return } err = db.AddSchema(req.Context(), string(sdl)) @@ -172,7 +177,7 @@ func loadSchema(rw http.ResponseWriter, req *http.Request) { err = json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) return } @@ -186,12 +191,12 @@ func loadSchema(rw http.ResponseWriter, req *http.Request) { err = json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) return } } -func getBlock(rw http.ResponseWriter, req *http.Request) { +func getBlockHandler(rw http.ResponseWriter, req *http.Request) { var result client.QueryResult cidStr := chi.URLParam(req, "cid") @@ -209,7 +214,7 @@ func getBlock(rw http.ResponseWriter, req *http.Request) { err = json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) return } @@ -219,9 +224,9 @@ func getBlock(rw http.ResponseWriter, req *http.Request) { cID = cid.NewCidV1(cid.Raw, hash) } - db, ok := req.Context().Value(ctxKey("DB")).(client.DB) - if !ok { - handleErr(rw, errors.New("no database available"), http.StatusInternalServerError) + db, err := dbFromContext(req.Context()) + if err != nil { + handleErr(req.Context(), rw, err, http.StatusInternalServerError) return } block, err := db.Blockstore().Get(req.Context(), cID) @@ -230,7 +235,7 @@ func getBlock(rw http.ResponseWriter, req *http.Request) { err = json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) return } @@ -245,7 +250,7 @@ func getBlock(rw http.ResponseWriter, req *http.Request) { err = json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) return } @@ -258,7 +263,7 @@ func getBlock(rw http.ResponseWriter, req *http.Request) { err = json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) return } @@ -273,7 +278,7 @@ func getBlock(rw http.ResponseWriter, req *http.Request) { err = json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) return } @@ -287,7 +292,7 @@ func getBlock(rw http.ResponseWriter, req *http.Request) { err = json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) return } @@ -310,7 +315,7 @@ func getBlock(rw http.ResponseWriter, req *http.Request) { err := json.NewEncoder(rw).Encode(result) if err != nil { - handleErr(rw, errors.WithStack(err), http.StatusInternalServerError) + handleErr(req.Context(), rw, errors.WithStack(err), http.StatusInternalServerError) return } diff --git a/api/http/logger.go b/api/http/logger.go index 15abf46395..56785a0231 100644 --- a/api/http/logger.go +++ b/api/http/logger.go @@ -18,8 +18,6 @@ import ( "github.com/sourcenetwork/defradb/logging" ) -var log = logging.MustNewLogger("defra.http") - type loggingResponseWriter struct { statusCode int contentLength int diff --git a/api/http/logger_test.go b/api/http/logger_test.go index ec6ce0e362..cbb6069f11 100644 --- a/api/http/logger_test.go +++ b/api/http/logger_test.go @@ -55,7 +55,7 @@ func TestLoggerLogs(t *testing.T) { EncoderFormat: logging.NewEncoderFormatOption(logging.JSON), OutputPaths: []string{logFile}, }) - loggerMiddleware(h.handle(ping)).ServeHTTP(rec2, req) + loggerMiddleware(h.handle(pingHandler)).ServeHTTP(rec2, req) assert.Equal(t, 200, rec2.Result().StatusCode) // inspect the log file diff --git a/api/http/router.go b/api/http/router.go index 4ff2519897..48cea3dab2 100644 --- a/api/http/router.go +++ b/api/http/router.go @@ -11,17 +11,18 @@ package http import ( - "context" "net/url" "path" + "strings" "github.com/go-chi/chi/v5" + "github.com/pkg/errors" ) const ( version string = "/api/v1" - HomePath string = version + "/" + RootPath string = version + "" PingPath string = version + "/ping" DumpPath string = version + "/debug/dump" BlocksPath string = version + "/blocks/get" @@ -29,37 +30,40 @@ const ( SchemaLoadPath string = version + "/schema/load" ) -func setRoutes(h *Handler) *Handler { +var schemeError = errors.New("base must start with the http or https scheme") + +func setRoutes(h *handler) *handler { h.Mux = chi.NewRouter() // setup logger middleware h.Use(loggerMiddleware) // define routes - h.Get(HomePath, h.handle(root)) - h.Get(PingPath, h.handle(ping)) - h.Get(DumpPath, h.handle(dump)) - h.Get(BlocksPath+"/{cid}", h.handle(getBlock)) - h.Get(GraphQLPath, h.handle(execGQL)) - h.Post(GraphQLPath, h.handle(execGQL)) - h.Post(SchemaLoadPath, h.handle(loadSchema)) + h.Get(RootPath, h.handle(rootHandler)) + h.Get(PingPath, h.handle(pingHandler)) + h.Get(DumpPath, h.handle(dumpHandler)) + h.Get(BlocksPath+"/{cid}", h.handle(getBlockHandler)) + h.Get(GraphQLPath, h.handle(execGQLHandler)) + h.Post(GraphQLPath, h.handle(execGQLHandler)) + h.Post(SchemaLoadPath, h.handle(loadSchemaHandler)) return h } // JoinPaths takes a base path and any number of additionnal paths -// and combines them safely to form a full URL path or a simple path if -// the base parameter is not a valid URL starting with `http://` or `https://`. -func JoinPaths(base string, paths ...string) string { +// and combines them safely to form a full URL path. +// The base must start with a http or https. +func JoinPaths(base string, paths ...string) (*url.URL, error) { + if !strings.HasPrefix(base, "http") { + return nil, schemeError + } + u, err := url.Parse(base) if err != nil { - log.Error(context.Background(), err.Error()) - paths = append(([]string{base}), paths...) - return path.Join(paths...) + return nil, errors.WithStack(err) } - paths = append(([]string{u.Path}), paths...) - u.Path = path.Join(paths...) + u.Path = path.Join(u.Path, strings.Join(paths, "/")) - return u.String() + return u, nil } diff --git a/api/http/router_test.go b/api/http/router_test.go index 03e4459b3f..cb4bee3610 100644 --- a/api/http/router_test.go +++ b/api/http/router_test.go @@ -16,7 +16,36 @@ import ( "github.com/stretchr/testify/assert" ) -func TestJoinPaths(t *testing.T) { - path := JoinPaths("http://localhost:9181", BlocksPath, "cid_of_some_sort") - assert.Equal(t, "http://localhost:9181"+BlocksPath+"/cid_of_some_sort", path) +func TestJoinPathsWithBase(t *testing.T) { + path, err := JoinPaths("http://localhost:9181", BlocksPath, "cid_of_some_sort") + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, "http://localhost:9181"+BlocksPath+"/cid_of_some_sort", path.String()) +} + +func TestJoinPathsWithNoBase(t *testing.T) { + _, err := JoinPaths("", BlocksPath, "cid_of_some_sort") + assert.ErrorIs(t, schemeError, err) +} + +func TestJoinPathsWithBaseWithoutHttpPrefix(t *testing.T) { + _, err := JoinPaths("localhost:9181", BlocksPath, "cid_of_some_sort") + assert.ErrorIs(t, schemeError, err) + +} + +func TestJoinPathsWithNoPaths(t *testing.T) { + path, err := JoinPaths("http://localhost:9181") + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, "http://localhost:9181", path.String()) +} + +func TestJoinPathsWithInvalidCharacter(t *testing.T) { + _, err := JoinPaths("https://%gh&%ij") + assert.Error(t, err) } diff --git a/cli/defradb/cmd/blocks_get.go b/cli/defradb/cmd/blocks_get.go index 3ca064093d..2815c403fb 100644 --- a/cli/defradb/cmd/blocks_get.go +++ b/cli/defradb/cmd/blocks_get.go @@ -43,7 +43,13 @@ var getCmd = &cobra.Command{ } cid := args[0] - res, err := http.Get(httpapi.JoinPaths(dbaddr, httpapi.BlocksPath, cid)) + endpoint, err := httpapi.JoinPaths(dbaddr, httpapi.BlocksPath, cid) + if err != nil { + log.ErrorE(ctx, "join paths failed", err) + return + } + + res, err := http.Get(endpoint.String()) if err != nil { log.ErrorE(ctx, "request failed", err) return diff --git a/cli/defradb/cmd/dump.go b/cli/defradb/cmd/dump.go index f4609bfab1..8d27420e1d 100644 --- a/cli/defradb/cmd/dump.go +++ b/cli/defradb/cmd/dump.go @@ -39,7 +39,13 @@ var dumpCmd = &cobra.Command{ dbaddr = "http://" + dbaddr } - res, err := http.Get(httpapi.JoinPaths(dbaddr, httpapi.DumpPath)) + endpoint, err := httpapi.JoinPaths(dbaddr, httpapi.DumpPath) + if err != nil { + log.ErrorE(ctx, "join paths failed", err) + return + } + + res, err := http.Get(endpoint.String()) if err != nil { log.ErrorE(ctx, "request failed", err) return diff --git a/cli/defradb/cmd/ping.go b/cli/defradb/cmd/ping.go index 9b508e7ce7..4a6fa1c659 100644 --- a/cli/defradb/cmd/ping.go +++ b/cli/defradb/cmd/ping.go @@ -40,7 +40,14 @@ var pingCmd = &cobra.Command{ } log.Info(ctx, "Sending ping...") - res, err := http.Get(httpapi.JoinPaths(dbaddr, httpapi.PingPath)) + + endpoint, err := httpapi.JoinPaths(dbaddr, httpapi.PingPath) + if err != nil { + log.ErrorE(ctx, "join paths failed", err) + return + } + + res, err := http.Get(endpoint.String()) if err != nil { log.ErrorE(ctx, "request failed", err) return diff --git a/cli/defradb/cmd/query.go b/cli/defradb/cmd/query.go index 07d4e573b6..e30d14db41 100644 --- a/cli/defradb/cmd/query.go +++ b/cli/defradb/cmd/query.go @@ -60,10 +60,11 @@ the additional documentation found at: https://hackmd.io/@source/BksQY6Qfw. log.Error(ctx, "missing query") return } - endpointStr := httpapi.JoinPaths(dbaddr, httpapi.GraphQLPath) - endpoint, err := url.Parse(endpointStr) + + endpoint, err := httpapi.JoinPaths(dbaddr, httpapi.GraphQLPath) if err != nil { - log.FatalE(ctx, "", err) + log.ErrorE(ctx, "join paths failed", err) + return } p := url.Values{} diff --git a/cli/defradb/cmd/schema_add.go b/cli/defradb/cmd/schema_add.go index e843799cfb..ef1a348478 100644 --- a/cli/defradb/cmd/schema_add.go +++ b/cli/defradb/cmd/schema_add.go @@ -15,7 +15,6 @@ import ( "context" "io" "net/http" - "net/url" "os" "strings" @@ -57,9 +56,11 @@ var addCmd = &cobra.Command{ if !strings.HasPrefix(dbaddr, "http") { dbaddr = "http://" + dbaddr } - endpointStr := httpapi.JoinPaths(dbaddr, httpapi.SchemaLoadPath) - endpoint, err := url.Parse(endpointStr) - cobra.CheckErr(err) + endpoint, err := httpapi.JoinPaths(dbaddr, httpapi.SchemaLoadPath) + if err != nil { + log.ErrorE(ctx, "join paths failed", err) + return + } res, err := http.Post(endpoint.String(), "text", bytes.NewBuffer(schema)) cobra.CheckErr(err) diff --git a/cli/defradb/cmd/start.go b/cli/defradb/cmd/start.go index af14c3b259..6feb20a5f0 100644 --- a/cli/defradb/cmd/start.go +++ b/cli/defradb/cmd/start.go @@ -21,6 +21,7 @@ import ( "time" ma "github.com/multiformats/go-multiaddr" + httpapi "github.com/sourcenetwork/defradb/api/http" badgerds "github.com/sourcenetwork/defradb/datastore/badger/v3" "github.com/sourcenetwork/defradb/db" netapi "github.com/sourcenetwork/defradb/net/api" @@ -173,9 +174,11 @@ var startCmd = &cobra.Command{ log.Info( ctx, fmt.Sprintf( - "Providing HTTP API at http://%s. Use the GraphQL query endpoint at http://%s/graphql ", + "Providing HTTP API at http://%s%s. Use the GraphQL query endpoint at http://%s%s/graphql ", config.Database.Address, + httpapi.RootPath, config.Database.Address, + httpapi.RootPath, ), ) s := http.NewServer(db) From 45a453300cb2bd625b61c4637feef547768b4ef0 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Tue, 3 May 2022 09:55:09 -0400 Subject: [PATCH 14/19] Add graphQL JSON body parsing --- api/http/handler.go | 8 ++++++++ api/http/handlerfuncs.go | 23 ++++++++++++++++------- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/api/http/handler.go b/api/http/handler.go index 91f5d959fd..57e316945d 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -41,6 +41,14 @@ func (h *handler) handle(f http.HandlerFunc) http.HandlerFunc { } } +func getJSON(req *http.Request, v interface{}) error { + err := json.NewDecoder(req.Body).Decode(v) + if err != nil { + return errors.Wrap(err, "unmarshall error") + } + return nil +} + func sendJSON(ctx context.Context, rw http.ResponseWriter, v interface{}, code int) { rw.Header().Set("Content-Type", "application/json") diff --git a/api/http/handlerfuncs.go b/api/http/handlerfuncs.go index 6f87790355..5b5c9109c8 100644 --- a/api/http/handlerfuncs.go +++ b/api/http/handlerfuncs.go @@ -86,19 +86,28 @@ func dumpHandler(rw http.ResponseWriter, req *http.Request) { } } +type gqlRequest struct { + Query string `json:"query"` + Variables map[string]interface{} `json:"variables"` + OperationName string `json:"operationName"` +} + func execGQLHandler(rw http.ResponseWriter, req *http.Request) { + query := req.URL.Query().Get("query") if query == "" { switch req.Header.Get("Content-Type") { case contentTypeJSON: - handleErr( - req.Context(), - rw, - errors.New("content type application/json not yet supported"), - http.StatusBadRequest, - ) - return + gqlReq := gqlRequest{} + + err := getJSON(req, &gqlReq) + if err != nil { + handleErr(req.Context(), rw, err, http.StatusBadRequest) + return + } + + query = gqlReq.Query case contentTypeFormURLEncoded: handleErr( From 695a0f08ec44d64cc245f4166f0b4f69c9c2ea8b Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Tue, 3 May 2022 12:00:12 -0400 Subject: [PATCH 15/19] Change to http package error strings --- api/http/errors.go | 27 +++------------------------ api/http/errors_test.go | 6 +++--- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/api/http/errors.go b/api/http/errors.go index cf405d9745..1f24141868 100644 --- a/api/http/errors.go +++ b/api/http/errors.go @@ -18,12 +18,6 @@ import ( "strings" ) -const ( - errBadRequest = "Bad Request" - errInternalServerError = "Internal Server Error" - errNotFound = "Not Found" -) - var env = os.Getenv("DEFRA_ENV") type errorResponse struct { @@ -33,23 +27,8 @@ type errorResponse struct { } func handleErr(ctx context.Context, rw http.ResponseWriter, err error, status int) { - var message string - - switch status { - case http.StatusBadRequest: - message = errBadRequest - - case http.StatusInternalServerError: - message = errInternalServerError - // @TODO: The internal server error log should be sent to a different location - // ideally not in the http logs. - log.ErrorE(context.Background(), errInternalServerError, err) - - case http.StatusNotFound: - message = errNotFound - - default: - message = err.Error() + if status == http.StatusInternalServerError { + log.ErrorE(context.Background(), http.StatusText(status), err) } sendJSON( @@ -57,7 +36,7 @@ func handleErr(ctx context.Context, rw http.ResponseWriter, err error, status in rw, errorResponse{ Status: status, - Message: message, + Message: http.StatusText(status), Stack: formatError(err), }, status, diff --git a/api/http/errors_test.go b/api/http/errors_test.go index 5df271d964..0db40bcfb1 100644 --- a/api/http/errors_test.go +++ b/api/http/errors_test.go @@ -51,7 +51,7 @@ func TestHandleErrOnBadRequest(t *testing.T) { assert.NoError(t, err) assert.Equal(t, http.StatusBadRequest, errResponse.Status) - assert.Equal(t, errBadRequest, errResponse.Message) + assert.Equal(t, http.StatusText(http.StatusBadRequest), errResponse.Message) lines := strings.Split(errResponse.Stack, "\n") assert.Equal(t, "[DEV] test error", lines[0]) @@ -76,7 +76,7 @@ func TestHandleErrOnInternalServerError(t *testing.T) { assert.NoError(t, err) assert.Equal(t, http.StatusInternalServerError, errResponse.Status) - assert.Equal(t, errInternalServerError, errResponse.Message) + assert.Equal(t, http.StatusText(http.StatusInternalServerError), errResponse.Message) lines := strings.Split(errResponse.Stack, "\n") assert.Equal(t, "[DEV] test error", lines[0]) @@ -101,7 +101,7 @@ func TestHandleErrOnNotFound(t *testing.T) { assert.NoError(t, err) assert.Equal(t, http.StatusNotFound, errResponse.Status) - assert.Equal(t, errNotFound, errResponse.Message) + assert.Equal(t, http.StatusText(http.StatusNotFound), errResponse.Message) lines := strings.Split(errResponse.Stack, "\n") assert.Equal(t, "[DEV] test error", lines[0]) From fa44712dba0a66b9a5915ab2ac6747412b78c3ce Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Thu, 5 May 2022 12:20:20 -0400 Subject: [PATCH 16/19] Add unit tests to get 100% coverage except handlerfuncs --- api/http/errors_test.go | 32 +++++++--- api/http/handler_test.go | 129 ++++++++++++++++++++++++++++++++++++++- api/http/logger_test.go | 38 ++++++++++-- 3 files changed, 185 insertions(+), 14 deletions(-) diff --git a/api/http/errors_test.go b/api/http/errors_test.go index 0db40bcfb1..f5e6f78bef 100644 --- a/api/http/errors_test.go +++ b/api/http/errors_test.go @@ -38,7 +38,9 @@ func TestHandleErrOnBadRequest(t *testing.T) { handleErr(req.Context(), rw, errors.New("test error"), http.StatusBadRequest) } req, err := http.NewRequest("GET", "/test", nil) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } rec := httptest.NewRecorder() @@ -48,7 +50,9 @@ func TestHandleErrOnBadRequest(t *testing.T) { errResponse := errorResponse{} err = json.NewDecoder(resp.Body).Decode(&errResponse) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } assert.Equal(t, http.StatusBadRequest, errResponse.Status) assert.Equal(t, http.StatusText(http.StatusBadRequest), errResponse.Message) @@ -63,7 +67,9 @@ func TestHandleErrOnInternalServerError(t *testing.T) { handleErr(req.Context(), rw, errors.New("test error"), http.StatusInternalServerError) } req, err := http.NewRequest("GET", "/test", nil) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } rec := httptest.NewRecorder() @@ -73,7 +79,9 @@ func TestHandleErrOnInternalServerError(t *testing.T) { errResponse := errorResponse{} err = json.NewDecoder(resp.Body).Decode(&errResponse) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } assert.Equal(t, http.StatusInternalServerError, errResponse.Status) assert.Equal(t, http.StatusText(http.StatusInternalServerError), errResponse.Message) @@ -88,7 +96,9 @@ func TestHandleErrOnNotFound(t *testing.T) { handleErr(req.Context(), rw, errors.New("test error"), http.StatusNotFound) } req, err := http.NewRequest("GET", "/test", nil) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } rec := httptest.NewRecorder() @@ -98,7 +108,9 @@ func TestHandleErrOnNotFound(t *testing.T) { errResponse := errorResponse{} err = json.NewDecoder(resp.Body).Decode(&errResponse) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } assert.Equal(t, http.StatusNotFound, errResponse.Status) assert.Equal(t, http.StatusText(http.StatusNotFound), errResponse.Message) @@ -113,7 +125,9 @@ func TestHandleErrOnDefault(t *testing.T) { handleErr(req.Context(), rw, errors.New("Unauthorized"), http.StatusUnauthorized) } req, err := http.NewRequest("GET", "/test", nil) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } rec := httptest.NewRecorder() @@ -123,7 +137,9 @@ func TestHandleErrOnDefault(t *testing.T) { errResponse := errorResponse{} err = json.NewDecoder(resp.Body).Decode(&errResponse) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } assert.Equal(t, http.StatusUnauthorized, errResponse.Status) assert.Equal(t, "Unauthorized", errResponse.Message) diff --git a/api/http/handler_test.go b/api/http/handler_test.go index 8f912a0ffe..5b81d449c9 100644 --- a/api/http/handler_test.go +++ b/api/http/handler_test.go @@ -11,11 +11,19 @@ package http import ( + "bytes" + "context" + "io/ioutil" + "math" "net/http" "net/http/httptest" "path" "testing" + badger "github.com/dgraph-io/badger/v3" + "github.com/pkg/errors" + badgerds "github.com/sourcenetwork/defradb/datastore/badger/v3" + "github.com/sourcenetwork/defradb/db" "github.com/sourcenetwork/defradb/logging" "github.com/stretchr/testify/assert" ) @@ -33,7 +41,9 @@ func TestNewHandlerWithLogger(t *testing.T) { }) req, err := http.NewRequest("GET", "/ping", nil) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } rec := httptest.NewRecorder() @@ -42,8 +52,123 @@ func TestNewHandlerWithLogger(t *testing.T) { // inspect the log file kv, err := readLog(logFile) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } assert.Equal(t, "defra.http", kv["logger"]) } + +func TestGetJSON(t *testing.T) { + var obj struct { + Name string + } + + jsonStr := []byte(` + { + "Name": "John Doe" + } + `) + + req, err := http.NewRequest("POST", "/ping", bytes.NewBuffer(jsonStr)) + if err != nil { + t.Fatal(err) + } + + err = getJSON(req, &obj) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, "John Doe", obj.Name) + +} + +func TestGetJSONWithError(t *testing.T) { + var obj struct { + Name string + } + + jsonStr := []byte(` + { + "Name": 10 + } + `) + + req, err := http.NewRequest("POST", "/ping", bytes.NewBuffer(jsonStr)) + if err != nil { + t.Fatal(err) + } + + err = getJSON(req, &obj) + assert.Error(t, err) +} + +func TestSendJSONWithNoErrors(t *testing.T) { + obj := struct { + Name string + }{ + Name: "John Doe", + } + + rec := httptest.NewRecorder() + + sendJSON(context.Background(), rec, obj, 200) + + body, err := ioutil.ReadAll(rec.Result().Body) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, []byte("{\"Name\":\"John Doe\"}"), body) +} + +func TestSendJSONWithMarshallFailure(t *testing.T) { + rec := httptest.NewRecorder() + + sendJSON(context.Background(), rec, math.Inf(1), 200) + + assert.Equal(t, http.StatusInternalServerError, rec.Result().StatusCode) +} + +type loggerTest struct { + loggingResponseWriter +} + +func (lt *loggerTest) Write(b []byte) (int, error) { + return 0, errors.New("this write will fail") +} + +func TestSendJSONWithWriteFailure(t *testing.T) { + rec := httptest.NewRecorder() + lrw := loggerTest{} + lrw.ResponseWriter = rec + + sendJSON(context.Background(), &lrw, math.Inf(1), 200) + + assert.Equal(t, http.StatusInternalServerError, lrw.statusCode) +} + +func TestDbFromContext(t *testing.T) { + _, err := dbFromContext(context.Background()) + assert.Error(t, err) + + opts := badgerds.Options{Options: badger.DefaultOptions("").WithInMemory(true)} + rootstore, err := badgerds.NewDatastore("", &opts) + if err != nil { + t.Fatal(err) + } + + var options []db.Option + ctx := context.Background() + + defra, err := db.NewDB(ctx, rootstore, options...) + if err != nil { + t.Fatal(err) + } + + reqCtx := context.WithValue(ctx, ctxKey("DB"), defra) + + _, err = dbFromContext(reqCtx) + assert.NoError(t, err) +} diff --git a/api/http/logger_test.go b/api/http/logger_test.go index cbb6069f11..32e92eb349 100644 --- a/api/http/logger_test.go +++ b/api/http/logger_test.go @@ -17,6 +17,7 @@ import ( "net/http/httptest" "os" "path" + "strconv" "testing" "github.com/pkg/errors" @@ -34,19 +35,46 @@ func TestNewLoggingResponseWriterLogger(t *testing.T) { content := "Hello world!" length, err := lrw.Write([]byte(content)) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } assert.Equal(t, length, lrw.contentLength) assert.Equal(t, rec.Body.String(), content) } -func TestLoggerLogs(t *testing.T) { +func TestLogginResponseWriterWriteWithChunks(t *testing.T) { + rec := httptest.NewRecorder() + lrw := newLoggingResponseWriter(rec) + + content := "Hello world!" + contentLength := len(content) + + lrw.Header().Set("Content-Length", strconv.Itoa(contentLength)) + + length1, err := lrw.Write([]byte(content[:contentLength/2])) + if err != nil { + t.Fatal(err) + } + + length2, err := lrw.Write([]byte(content[contentLength/2:])) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, contentLength, length1+length2) + assert.Equal(t, rec.Body.String(), content) +} + +func TestLoggerKeyValueOutput(t *testing.T) { dir := t.TempDir() // send logs to temp file so we can inspect it logFile := path.Join(dir, "http_test.log") req, err := http.NewRequest("GET", "/ping", nil) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } rec2 := httptest.NewRecorder() @@ -60,7 +88,9 @@ func TestLoggerLogs(t *testing.T) { // inspect the log file kv, err := readLog(logFile) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } // check that everything is as expected assert.Equal(t, "pong", rec2.Body.String()) From 55c9bcf8c2f5d5e699cf3ce0f71185dac18076b4 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Thu, 5 May 2022 21:52:07 -0400 Subject: [PATCH 17/19] fix SendJSONWithWriteFailure --- api/http/handler.go | 1 + api/http/handler_test.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/api/http/handler.go b/api/http/handler.go index 57e316945d..147f0b9924 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -64,6 +64,7 @@ func sendJSON(ctx context.Context, rw http.ResponseWriter, v interface{}, code i rw.WriteHeader(code) if _, err = rw.Write(b); err != nil { + rw.WriteHeader(http.StatusInternalServerError) log.Error(ctx, err.Error()) } } diff --git a/api/http/handler_test.go b/api/http/handler_test.go index 5b81d449c9..011ce59558 100644 --- a/api/http/handler_test.go +++ b/api/http/handler_test.go @@ -140,11 +140,17 @@ func (lt *loggerTest) Write(b []byte) (int, error) { } func TestSendJSONWithWriteFailure(t *testing.T) { + obj := struct { + Name string + }{ + Name: "John Doe", + } + rec := httptest.NewRecorder() lrw := loggerTest{} lrw.ResponseWriter = rec - sendJSON(context.Background(), &lrw, math.Inf(1), 200) + sendJSON(context.Background(), &lrw, obj, 200) assert.Equal(t, http.StatusInternalServerError, lrw.statusCode) } From f021c4683dfb4292d768479382740b7b9963d685 Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Fri, 6 May 2022 17:16:42 -0400 Subject: [PATCH 18/19] add test for marshall error with following write error --- api/http/handler_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/api/http/handler_test.go b/api/http/handler_test.go index 011ce59558..663a267492 100644 --- a/api/http/handler_test.go +++ b/api/http/handler_test.go @@ -139,6 +139,16 @@ func (lt *loggerTest) Write(b []byte) (int, error) { return 0, errors.New("this write will fail") } +func TestSendJSONWithMarshallFailureAndWriteFailer(t *testing.T) { + rec := httptest.NewRecorder() + lrw := loggerTest{} + lrw.ResponseWriter = rec + + sendJSON(context.Background(), &lrw, math.Inf(1), 200) + + assert.Equal(t, http.StatusInternalServerError, rec.Result().StatusCode) +} + func TestSendJSONWithWriteFailure(t *testing.T) { obj := struct { Name string From 56862b5638da936914e84e97459e047fbef65a1d Mon Sep 17 00:00:00 2001 From: Fred Carle Date: Sun, 8 May 2022 23:39:31 -0400 Subject: [PATCH 19/19] fix final suggestions --- api/http/errors.go | 2 +- api/http/handler.go | 6 +++--- api/http/handler_test.go | 2 +- api/http/{api.go => http.go} | 0 api/http/logger.go | 3 +++ api/http/server.go | 9 ++++++--- api/http/server_test.go | 20 +++++++++++++++++--- 7 files changed, 31 insertions(+), 11 deletions(-) rename api/http/{api.go => http.go} (100%) diff --git a/api/http/errors.go b/api/http/errors.go index 1f24141868..2d440890a8 100644 --- a/api/http/errors.go +++ b/api/http/errors.go @@ -28,7 +28,7 @@ type errorResponse struct { func handleErr(ctx context.Context, rw http.ResponseWriter, err error, status int) { if status == http.StatusInternalServerError { - log.ErrorE(context.Background(), http.StatusText(status), err) + log.ErrorE(ctx, http.StatusText(status), err) } sendJSON( diff --git a/api/http/handler.go b/api/http/handler.go index 147f0b9924..23cdda6d52 100644 --- a/api/http/handler.go +++ b/api/http/handler.go @@ -27,7 +27,7 @@ type handler struct { *chi.Mux } -type ctxKey string +type ctxDB struct{} // newHandler returns a handler with the router instantiated. func newHandler(db client.DB) *handler { @@ -36,7 +36,7 @@ func newHandler(db client.DB) *handler { func (h *handler) handle(f http.HandlerFunc) http.HandlerFunc { return func(rw http.ResponseWriter, req *http.Request) { - ctx := context.WithValue(req.Context(), ctxKey("DB"), h.db) + ctx := context.WithValue(req.Context(), ctxDB{}, h.db) f(rw, req.WithContext(ctx)) } } @@ -70,7 +70,7 @@ func sendJSON(ctx context.Context, rw http.ResponseWriter, v interface{}, code i } func dbFromContext(ctx context.Context) (client.DB, error) { - db, ok := ctx.Value(ctxKey("DB")).(client.DB) + db, ok := ctx.Value(ctxDB{}).(client.DB) if !ok { return nil, errors.New("no database available") } diff --git a/api/http/handler_test.go b/api/http/handler_test.go index 663a267492..04d3a681e2 100644 --- a/api/http/handler_test.go +++ b/api/http/handler_test.go @@ -183,7 +183,7 @@ func TestDbFromContext(t *testing.T) { t.Fatal(err) } - reqCtx := context.WithValue(ctx, ctxKey("DB"), defra) + reqCtx := context.WithValue(ctx, ctxDB{}, defra) _, err = dbFromContext(reqCtx) assert.NoError(t, err) diff --git a/api/http/api.go b/api/http/http.go similarity index 100% rename from api/http/api.go rename to api/http/http.go diff --git a/api/http/logger.go b/api/http/logger.go index 56785a0231..29fdb6ddd3 100644 --- a/api/http/logger.go +++ b/api/http/logger.go @@ -39,9 +39,12 @@ func (lrw *loggingResponseWriter) WriteHeader(code int) { } func (lrw *loggingResponseWriter) Write(b []byte) (int, error) { + // used for chucked payloads. Content-Length should not be set + // for each chunk. if lrw.ResponseWriter.Header().Get("Content-Length") != "" { return lrw.ResponseWriter.Write(b) } + lrw.contentLength = len(b) lrw.ResponseWriter.Header().Set("Content-Length", strconv.Itoa(lrw.contentLength)) return lrw.ResponseWriter.Write(b) diff --git a/api/http/server.go b/api/http/server.go index bf838ef62e..f75fd28114 100644 --- a/api/http/server.go +++ b/api/http/server.go @@ -18,17 +18,20 @@ import ( // The Server struct holds the Handler for the HTTP API type Server struct { - Handler http.Handler + http.Server } // NewServer instantiated a new server with the given http.Handler. func NewServer(db client.DB) *Server { return &Server{ - Handler: newHandler(db), + http.Server{ + Handler: newHandler(db), + }, } } // Listen calls ListenAndServe with our router. func (s *Server) Listen(addr string) error { - return http.ListenAndServe(addr, s.Handler) + s.Addr = addr + return s.ListenAndServe() } diff --git a/api/http/server_test.go b/api/http/server_test.go index d4a4c48b7e..d8c1c5e4fc 100644 --- a/api/http/server_test.go +++ b/api/http/server_test.go @@ -11,17 +11,31 @@ package http import ( + "context" + "net/http" "testing" "github.com/stretchr/testify/assert" ) func TestNewServerAndListen(t *testing.T) { - // @TODO: maybe it would be worth doing something a bit more thorough - - // test with no config s := NewServer(nil) if ok := assert.NotNil(t, s); ok { assert.Error(t, s.Listen(":303000")) } + + serverRunning := make(chan struct{}) + serverDone := make(chan struct{}) + go func() { + close(serverRunning) + err := s.Listen(":3131") + assert.ErrorIs(t, http.ErrServerClosed, err) + defer close(serverDone) + }() + + <-serverRunning + + s.Shutdown(context.Background()) + + <-serverDone }