✨ httprule: Add optional "default" handler (#49)

Add an optional "default" handler to `httprule.Handler` that is called if
the request does not match any of the gRPC methods. The default is still to
return a 404 Not Found status if no "default" handler is supplied.

This allows chaining handlers so that other non-gRPC paths can be handled
by other handlers, with precedence given to gRPC paths.

Refactor `httprule.Server` to `httprule.Handler` and add function
options to a new `NewHandler()` constructor. There were already a couple
of optional arguments to `NewServer()` and the "default" handler would
have been another. So switch to function options, leaving the existing
`NewServer()` as is and aliasing `Server` to the new `Handler`.

This merges the following commits:
* httprule: Rename Server to Handler and implement function options
* httprule: Add optional "default" handler

     main.go                                       |  5 +-
     serve/httprule/{server.go => handler.go}      | 97 +++++++++++++++----
     .../{server_test.go => handler_test.go}       | 23 ++++-
     3 files changed, 105 insertions(+), 20 deletions(-)

Co-authored-by: Bob Lail <[email protected]>
Pull-request: #49

main.go

@@ -79,7 +79,10 @@ func (cs *cmdServe) Run(logLevel log.LogLevel) error {
 	}
 
 	if cs.HTTP {
-		h := httprule.NewServer(s.Files, s.UnknownHandler, logger, nil)
+		h, err := httprule.NewHandler(s.Files, s.UnknownHandler, httprule.WithLogger(logger))
+		if err != nil {
+			return err
+		}
 		s.SetHTTPHandler(h)
 	}
 

serve/httprule/server.go → serve/httprule/handler.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"mime"
 	"net/http"
+	"os"
 	"strings"
 
 	"foxygo.at/jig/log"
@@ -24,42 +25,104 @@ type httpMethod struct {
 	rule *annotations.HttpRule
 }
 
-// Server serves protobuf methods, annotated using httprule options, over HTTP.
-type Server struct {
-	httpMethods []*httpMethod
-	grpcHandler grpc.StreamHandler
-	log         log.Logger
+// Handler serves protobuf methods, annotated using httprule options, over HTTP.
+type Handler struct {
+	httpMethods    []*httpMethod
+	grpcHandler    grpc.StreamHandler
+	log            log.Logger
+	ruleTemplates  []*annotations.HttpRule
+	defaultHandler http.Handler
 }
 
-func NewServer(files *registry.Files, handler grpc.StreamHandler, l log.Logger, httpRuleTemplates []*annotations.HttpRule) *Server {
-	return &Server{
-		httpMethods: loadHTTPRules(l, files, httpRuleTemplates),
-		grpcHandler: handler,
-		log:         l,
+// NewHandler returns a new [Handler] that implements [http.Handler] that will
+// dispatch HTTP requests matching the HttpRule annotations in the given
+// registry. Requests that match a method are dispatched to the given gRPC
+// handler.
+func NewHandler(files *registry.Files, handler grpc.StreamHandler, options ...Option) (*Handler, error) {
+	h := &Handler{
+		grpcHandler:    handler,
+		defaultHandler: http.NotFoundHandler(),
 	}
+	for _, opt := range options {
+		if err := opt(h); err != nil {
+			return nil, err
+		}
+	}
+	if h.log == nil {
+		h.log = log.NewLogger(os.Stderr, log.LogLevelError)
+	}
+	h.httpMethods = loadHTTPRules(h.log, files, h.ruleTemplates)
+
+	return h, nil
+}
+
+// Option is a function option for use with [NewHandler].
+type Option func(h *Handler) error
+
+// WithLogger is an [Option] to configure a [Handler] with the given logger.
+func WithLogger(l log.Logger) Option {
+	return func(h *Handler) error {
+		h.log = l
+		return nil
+	}
+}
+
+// WithRuleTemplates is an [Option] to configure a [Handler] to provide a http
+// rule template to be used for gRPC methods that do not have an HttpRule
+// annotation.
+func WithRuleTemplates(httpRuleTemplates []*annotations.HttpRule) Option {
+	return func(h *Handler) error {
+		h.ruleTemplates = httpRuleTemplates
+		return nil
+	}
+}
+
+// WithDefaultHandler is an [Option] to configure a [Handler] with a fallback
+// handler when the request being handled does not match any of the gRPC
+// methods the [Handler] is configured with. By default the [Handler] will
+// return a 404 NotFound response. If a default handler is supplied, it will be
+// called instead of returning that 404 NotFound response.
+func WithDefaultHandler(next http.Handler) Option {
+	return func(h *Handler) error {
+		h.defaultHandler = next
+		return nil
+	}
+}
+
+// Server is a [Handler], and exists for backwards compatibility.
+//
+// Deprecated: Use [Handler] instead.
+type Server = Handler
+
+// NewServer returns a new Handler.
+//
+// Deprecated: Use [NewHandler] instead. [Handler] used to be called [Server].
+func NewServer(files *registry.Files, handler grpc.StreamHandler, l log.Logger, httpRuleTemplates []*annotations.HttpRule) *Handler {
+	h, _ := NewHandler(files, handler, WithLogger(l), WithRuleTemplates(httpRuleTemplates))
+	return h
 }
 
-func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	for _, method := range s.httpMethods {
+func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	for _, method := range h.httpMethods {
 		if vars := MatchRequest(method.rule, r); vars != nil {
-			s.serveHTTPMethod(method, vars, w, r)
+			h.serveHTTPMethod(method, vars, w, r)
 			return
 		}
 	}
-	http.NotFound(w, r)
+	h.defaultHandler.ServeHTTP(w, r)
 }
 
 // Serve a google.api.http annotated method as HTTP
-func (s *Server) serveHTTPMethod(m *httpMethod, vars map[string]string, w http.ResponseWriter, r *http.Request) {
+func (h *Handler) serveHTTPMethod(m *httpMethod, vars map[string]string, w http.ResponseWriter, r *http.Request) {
 	// TODO: Handle streaming calls.
 	ss := &serverStream{
 		req:        r,
 		respWriter: w,
 		rule:       m.rule,
 		vars:       vars,
-		log:        s.log,
+		log:        h.log,
 	}
-	if err := s.grpcHandler(m.desc.FullName(), ss); err != nil {
+	if err := h.grpcHandler(m.desc.FullName(), ss); err != nil {
 		ss.writeError(err)
 		return
 	}

serve/httprule/server_test.go → serve/httprule/handler_test.go

@@ -24,7 +24,8 @@ func TestHTTP(t *testing.T) {
 	ts := serve.NewTestServer(serve.JsonnetEvaluator(), os.DirFS("testdata/greet"), withLogger)
 	defer ts.Stop()
 
-	h := NewServer(ts.Files, ts.UnknownHandler, log.DiscardLogger, nil)
+	h, err := NewHandler(ts.Files, ts.UnknownHandler, WithLogger(log.DiscardLogger))
+	require.NoError(t, err)
 	ts.SetHTTPHandler(h)
 
 	body := `{"first_name": "Stranger"}`
@@ -83,13 +84,30 @@ func TestHTTP(t *testing.T) {
 	})
 
 	t.Run("return 404 for invalid path", func(t *testing.T) {
+		// GET is not handled
 		req, err := http.NewRequest("GET", url, nil)
 		require.NoError(t, err)
 		req.Header.Set("Accept", "application/json; charset=utf-8")
 		resp, err := http.DefaultClient.Do(req)
 		require.NoError(t, err)
 		require.Equal(t, http.StatusNotFound, resp.StatusCode)
 	})
+
+	teapot := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Error(w, "418 I'm a HyperTextTeaPot", http.StatusTeapot)
+	})
+	h, err = NewHandler(ts.Files, ts.UnknownHandler, WithLogger(log.DiscardLogger), WithDefaultHandler(teapot))
+	require.NoError(t, err)
+	ts.SetHTTPHandler(h)
+	t.Run("return 418 for invalid path by next handler", func(t *testing.T) {
+		// GET is not handled
+		req, err := http.NewRequest("GET", url, nil)
+		require.NoError(t, err)
+		req.Header.Set("Accept", "application/json; charset=utf-8")
+		resp, err := http.DefaultClient.Do(req)
+		require.NoError(t, err)
+		require.Equal(t, http.StatusTeapot, resp.StatusCode)
+	})
 }
 
 func TestHTTPRuleInterpolation(t *testing.T) {
@@ -102,7 +120,8 @@ func TestHTTPRuleInterpolation(t *testing.T) {
 		{Pattern: &annotations.HttpRule_Post{Post: "/post/{package}.{service}/{method}"}, Body: "*"},
 		{Pattern: &annotations.HttpRule_Get{Get: "/get/{method}"}},
 	}
-	h := NewServer(ts.Files, ts.UnknownHandler, logger, tmpl)
+	h, err := NewHandler(ts.Files, ts.UnknownHandler, WithLogger(logger), WithRuleTemplates(tmpl))
+	require.NoError(t, err)
 	ts.SetHTTPHandler(h)
 
 	u := "http://" + ts.Addr() + "/get/SimpleHello"