From e96b5103a1ff808abc946d4a276bbb8fbe29bed2 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 20 Feb 2017 11:58:56 +0000 Subject: [PATCH] Put path var parsing along with path pattern --- .../dendrite/clientapi/readers/sync.go | 3 +- .../dendrite/clientapi/routing/routing.go | 31 +++++++++++-- .../dendrite/clientapi/writers/sendmessage.go | 11 ++--- vendor/manifest | 2 +- vendor/src/github.com/matrix-org/util/json.go | 46 ++++++++++++++----- .../github.com/matrix-org/util/json_test.go | 38 ++++++++++++++- 6 files changed, 103 insertions(+), 28 deletions(-) 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 c5de15b06..f83c34954 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/readers/sync.go +++ b/src/github.com/matrix-org/dendrite/clientapi/readers/sync.go @@ -3,7 +3,6 @@ package readers import ( "net/http" - log "github.com/Sirupsen/logrus" "github.com/matrix-org/util" ) @@ -12,7 +11,7 @@ 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) + 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 index e952d57f4..e993276e8 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/routing/routing.go +++ b/src/github.com/matrix-org/dendrite/clientapi/routing/routing.go @@ -12,18 +12,39 @@ import ( const pathPrefixR0 = "/_matrix/client/r0" -func make(metricsName string, h util.JSONRequestHandler) http.Handler { - return prometheus.InstrumentHandler(metricsName, util.MakeJSONAPI(h)) -} - // 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", &readers.Sync{})) - r0mux.Handle("/rooms/{roomID}/send/{eventType}", make("send_message", &writers.SendMessage{})) + r0mux.Handle("/rooms/{roomID}/send/{eventType}", + make("send_message", wrap(func(req *http.Request) (interface{}, *util.HTTPError) { + vars := mux.Vars(req) + roomID := vars["roomID"] + eventType := vars["eventType"] + h := &writers.SendMessage{} + return h.OnIncomingRequest(req, roomID, 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 1c3ab30d8..c3dec7855 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/writers/sendmessage.go +++ b/src/github.com/matrix-org/dendrite/clientapi/writers/sendmessage.go @@ -3,8 +3,6 @@ package writers import ( "net/http" - log "github.com/Sirupsen/logrus" - "github.com/gorilla/mux" "github.com/matrix-org/util" ) @@ -12,12 +10,9 @@ import ( 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) - vars := mux.Vars(req) - roomID := vars["roomID"] - eventType := vars["eventType"] +// OnIncomingRequest implements /rooms/{roomID}/send/{eventType} +func (s *SendMessage) OnIncomingRequest(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, diff --git a/vendor/manifest b/vendor/manifest index 1c598ff33..2d503c3ef 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 ac29a138e..b310ac928 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 203fa708c..0b8272457 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) + } +}