From f1bb59d24a2496f34c8d82853e4b61cc07e8d194 Mon Sep 17 00:00:00 2001 From: Kegsay Date: Mon, 20 Feb 2017 15:41:29 +0000 Subject: [PATCH] Use gorilla/mux to route HTTP requests (#11) * Add basic routing based on matched paths * Make /sync and /send use the right API paths --- .../dendrite/clientapi/clientapi.go | 16 +----- .../dendrite/clientapi/readers/sync.go | 10 ++-- .../dendrite/clientapi/routing/routing.go | 49 +++++++++++++++++++ .../dendrite/clientapi/writers/sendmessage.go | 13 ++--- vendor/manifest | 2 +- vendor/src/github.com/matrix-org/util/json.go | 46 ++++++++++++----- .../github.com/matrix-org/util/json_test.go | 38 +++++++++++++- 7 files changed, 131 insertions(+), 43 deletions(-) create mode 100644 src/github.com/matrix-org/dendrite/clientapi/routing/routing.go diff --git a/src/github.com/matrix-org/dendrite/clientapi/clientapi.go b/src/github.com/matrix-org/dendrite/clientapi/clientapi.go index 989dbf3b..b646904d 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/clientapi.go +++ b/src/github.com/matrix-org/dendrite/clientapi/clientapi.go @@ -1,31 +1,19 @@ package main import ( + "github.com/matrix-org/dendrite/clientapi/routing" "net/http" "os" log "github.com/Sirupsen/logrus" - - "github.com/matrix-org/dendrite/clientapi/readers" - "github.com/matrix-org/dendrite/clientapi/writers" - "github.com/matrix-org/util" - "github.com/prometheus/client_golang/prometheus" ) -// setup registers HTTP handlers with the given ServeMux. It also supplies the given http.Client -// to clients which need to make outbound HTTP requests. -func setup(mux *http.ServeMux, httpClient *http.Client) { - mux.Handle("/metrics", prometheus.Handler()) - mux.Handle("/api/send", prometheus.InstrumentHandler("send_message", util.MakeJSONAPI(&writers.SendMessage{}))) - mux.Handle("/api/sync", prometheus.InstrumentHandler("sync", util.MakeJSONAPI(&readers.Sync{}))) -} - func main() { bindAddr := os.Getenv("BIND_ADDRESS") if bindAddr == "" { log.Panic("No BIND_ADDRESS environment variable found.") } log.Info("Starting clientapi") - setup(http.DefaultServeMux, http.DefaultClient) + routing.Setup(http.DefaultServeMux, http.DefaultClient) log.Fatal(http.ListenAndServe(bindAddr, nil)) } diff --git a/src/github.com/matrix-org/dendrite/clientapi/readers/sync.go b/src/github.com/matrix-org/dendrite/clientapi/readers/sync.go index c5de15b0..27a47094 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/readers/sync.go +++ b/src/github.com/matrix-org/dendrite/clientapi/readers/sync.go @@ -3,16 +3,12 @@ package readers import ( "net/http" - log "github.com/Sirupsen/logrus" "github.com/matrix-org/util" ) -// Sync handles HTTP requests to /sync -type Sync struct{} - -// OnIncomingRequest implements util.JSONRequestHandler -func (s *Sync) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) { - logger := req.Context().Value(util.CtxValueLogger).(*log.Entry) +// Sync implements /sync +func Sync(req *http.Request) (interface{}, *util.HTTPError) { + logger := util.GetLogger(req.Context()) logger.Info("Doing stuff...") return nil, &util.HTTPError{ Code: 404, diff --git a/src/github.com/matrix-org/dendrite/clientapi/routing/routing.go b/src/github.com/matrix-org/dendrite/clientapi/routing/routing.go new file mode 100644 index 00000000..13cf4048 --- /dev/null +++ b/src/github.com/matrix-org/dendrite/clientapi/routing/routing.go @@ -0,0 +1,49 @@ +package routing + +import ( + "net/http" + + "github.com/gorilla/mux" + "github.com/matrix-org/dendrite/clientapi/readers" + "github.com/matrix-org/dendrite/clientapi/writers" + "github.com/matrix-org/util" + "github.com/prometheus/client_golang/prometheus" +) + +const pathPrefixR0 = "/_matrix/client/r0" + +// Setup registers HTTP handlers with the given ServeMux. It also supplies the given http.Client +// to clients which need to make outbound HTTP requests. +func Setup(servMux *http.ServeMux, httpClient *http.Client) { + apiMux := mux.NewRouter() + r0mux := apiMux.PathPrefix(pathPrefixR0).Subrouter() + r0mux.Handle("/sync", make("sync", wrap(func(req *http.Request) (interface{}, *util.HTTPError) { + return readers.Sync(req) + }))) + r0mux.Handle("/rooms/{roomID}/send/{eventType}", + make("send_message", wrap(func(req *http.Request) (interface{}, *util.HTTPError) { + vars := mux.Vars(req) + return writers.SendMessage(req, vars["roomID"], vars["eventType"]) + })), + ) + + servMux.Handle("/metrics", prometheus.Handler()) + servMux.Handle("/api/", http.StripPrefix("/api", apiMux)) +} + +// make a util.JSONRequestHandler into an http.Handler +func make(metricsName string, h util.JSONRequestHandler) http.Handler { + return prometheus.InstrumentHandler(metricsName, util.MakeJSONAPI(h)) +} + +// jsonRequestHandlerWrapper is a wrapper to allow in-line functions to conform to util.JSONRequestHandler +type jsonRequestHandlerWrapper struct { + function func(req *http.Request) (interface{}, *util.HTTPError) +} + +func (r *jsonRequestHandlerWrapper) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) { + return r.function(req) +} +func wrap(f func(req *http.Request) (interface{}, *util.HTTPError)) *jsonRequestHandlerWrapper { + return &jsonRequestHandlerWrapper{f} +} diff --git a/src/github.com/matrix-org/dendrite/clientapi/writers/sendmessage.go b/src/github.com/matrix-org/dendrite/clientapi/writers/sendmessage.go index dc4df2f5..11b17740 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/writers/sendmessage.go +++ b/src/github.com/matrix-org/dendrite/clientapi/writers/sendmessage.go @@ -3,18 +3,13 @@ package writers import ( "net/http" - log "github.com/Sirupsen/logrus" "github.com/matrix-org/util" ) -// SendMessage handles HTTP requests to /rooms/$room_id/send/$event_type -type SendMessage struct { -} - -// OnIncomingRequest implements util.JSONRequestHandler -func (s *SendMessage) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) { - logger := req.Context().Value(util.CtxValueLogger).(*log.Entry) - logger.Info("Doing stuff...") +// SendMessage implements /rooms/{roomID}/send/{eventType} +func SendMessage(req *http.Request, roomID, eventType string) (interface{}, *util.HTTPError) { + logger := util.GetLogger(req.Context()) + logger.WithField("roomID", roomID).WithField("eventType", eventType).Info("Doing stuff...") return nil, &util.HTTPError{ Code: 404, Message: "Not implemented yet", diff --git a/vendor/manifest b/vendor/manifest index 1c598ff3..2d503c3e 100644 --- a/vendor/manifest +++ b/vendor/manifest @@ -86,7 +86,7 @@ { "importpath": "github.com/matrix-org/util", "repository": "https://github.com/matrix-org/util", - "revision": "0f4d9cce82badc0741ff1141dcf079312cb4d2f0", + "revision": "0bbc3896e02031e7e7338948b73ce891aa73ab2b", "branch": "master" }, { diff --git a/vendor/src/github.com/matrix-org/util/json.go b/vendor/src/github.com/matrix-org/util/json.go index ac29a138..b310ac92 100644 --- a/vendor/src/github.com/matrix-org/util/json.go +++ b/vendor/src/github.com/matrix-org/util/json.go @@ -12,11 +12,33 @@ import ( log "github.com/Sirupsen/logrus" ) -// ContextKeys is a type alias for string to namespace Context keys per-package. -type ContextKeys string +// contextKeys is a type alias for string to namespace Context keys per-package. +type contextKeys string -// CtxValueLogger is the key to extract the logrus Logger. -const CtxValueLogger = ContextKeys("logger") +// ctxValueRequestID is the key to extract the request ID for an HTTP request +const ctxValueRequestID = contextKeys("requestid") + +// GetRequestID returns the request ID associated with this context, or the empty string +// if one is not associated with this context. +func GetRequestID(ctx context.Context) string { + id := ctx.Value(ctxValueRequestID) + if id == nil { + return "" + } + return id.(string) +} + +// ctxValueLogger is the key to extract the logrus Logger. +const ctxValueLogger = contextKeys("logger") + +// GetLogger retrieves the logrus logger from the supplied context. Returns nil if there is no logger. +func GetLogger(ctx context.Context) *log.Entry { + l := ctx.Value(ctxValueLogger) + if l == nil { + return nil + } + return l.(*log.Entry) +} // JSONRequestHandler represents an interface that must be satisfied in order to respond to incoming // HTTP requests with JSON. The interface returned will be marshalled into JSON to be sent to the client, @@ -34,12 +56,12 @@ type JSONError struct { // Protect panicking HTTP requests from taking down the entire process, and log them using // the correct logger, returning a 500 with a JSON response rather than abruptly closing the -// connection. The http.Request MUST have a CtxValueLogger. +// connection. The http.Request MUST have a ctxValueLogger. func Protect(handler http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { defer func() { if r := recover(); r != nil { - logger := req.Context().Value(CtxValueLogger).(*log.Entry) + logger := req.Context().Value(ctxValueLogger).(*log.Entry) logger.WithFields(log.Fields{ "panic": r, }).Errorf( @@ -56,18 +78,20 @@ func Protect(handler http.HandlerFunc) http.HandlerFunc { // MakeJSONAPI creates an HTTP handler which always responds to incoming requests with JSON responses. // Incoming http.Requests will have a logger (with a request ID/method/path logged) attached to the Context. -// This can be accessed via the const CtxValueLogger. The type of the logger is *log.Entry from github.com/Sirupsen/logrus +// This can be accessed via GetLogger(Context). The type of the logger is *log.Entry from github.com/Sirupsen/logrus func MakeJSONAPI(handler JSONRequestHandler) http.HandlerFunc { return Protect(func(w http.ResponseWriter, req *http.Request) { + reqID := RandomString(12) // Set a Logger on the context - ctx := context.WithValue(req.Context(), CtxValueLogger, log.WithFields(log.Fields{ + ctx := context.WithValue(req.Context(), ctxValueLogger, log.WithFields(log.Fields{ "req.method": req.Method, "req.path": req.URL.Path, - "req.id": RandomString(12), + "req.id": reqID, })) + ctx = context.WithValue(ctx, ctxValueRequestID, reqID) req = req.WithContext(ctx) - logger := req.Context().Value(CtxValueLogger).(*log.Entry) + logger := req.Context().Value(ctxValueLogger).(*log.Entry) logger.Print("Incoming request") res, httpErr := handler.OnIncomingRequest(req) @@ -99,7 +123,7 @@ func MakeJSONAPI(handler JSONRequestHandler) http.HandlerFunc { } func jsonErrorResponse(w http.ResponseWriter, req *http.Request, httpErr *HTTPError) { - logger := req.Context().Value(CtxValueLogger).(*log.Entry) + logger := req.Context().Value(ctxValueLogger).(*log.Entry) if httpErr.Code == 302 { logger.WithField("err", httpErr.Error()).Print("Redirecting") http.Redirect(w, req, httpErr.Message, 302) diff --git a/vendor/src/github.com/matrix-org/util/json_test.go b/vendor/src/github.com/matrix-org/util/json_test.go index 203fa708..0b827245 100644 --- a/vendor/src/github.com/matrix-org/util/json_test.go +++ b/vendor/src/github.com/matrix-org/util/json_test.go @@ -73,12 +73,30 @@ func TestMakeJSONAPIRedirect(t *testing.T) { } } +func TestGetLogger(t *testing.T) { + log.SetLevel(log.PanicLevel) // suppress logs in test output + entry := log.WithField("test", "yep") + mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil) + ctx := context.WithValue(mockReq.Context(), ctxValueLogger, entry) + mockReq = mockReq.WithContext(ctx) + ctxLogger := GetLogger(mockReq.Context()) + if ctxLogger != entry { + t.Errorf("TestGetLogger wanted logger '%v', got '%v'", entry, ctxLogger) + } + + noLoggerInReq, _ := http.NewRequest("GET", "http://example.com/foo", nil) + ctxLogger = GetLogger(noLoggerInReq.Context()) + if ctxLogger != nil { + t.Errorf("TestGetLogger wanted nil logger, got '%v'", ctxLogger) + } +} + func TestProtect(t *testing.T) { log.SetLevel(log.PanicLevel) // suppress logs in test output mockWriter := httptest.NewRecorder() mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil) mockReq = mockReq.WithContext( - context.WithValue(mockReq.Context(), CtxValueLogger, log.WithField("test", "yep")), + context.WithValue(mockReq.Context(), ctxValueLogger, log.WithField("test", "yep")), ) h := Protect(func(w http.ResponseWriter, req *http.Request) { panic("oh noes!") @@ -97,3 +115,21 @@ func TestProtect(t *testing.T) { t.Errorf("TestProtect wanted body %s, got %s", expectBody, actualBody) } } + +func TestGetRequestID(t *testing.T) { + log.SetLevel(log.PanicLevel) // suppress logs in test output + reqID := "alphabetsoup" + mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil) + ctx := context.WithValue(mockReq.Context(), ctxValueRequestID, reqID) + mockReq = mockReq.WithContext(ctx) + ctxReqID := GetRequestID(mockReq.Context()) + if reqID != ctxReqID { + t.Errorf("TestGetRequestID wanted request ID '%s', got '%s'", reqID, ctxReqID) + } + + noReqIDInReq, _ := http.NewRequest("GET", "http://example.com/foo", nil) + ctxReqID = GetRequestID(noReqIDInReq.Context()) + if ctxReqID != "" { + t.Errorf("TestGetRequestID wanted empty request ID, got '%s'", ctxReqID) + } +}