From 78f767a0afb42ef153da8751ce8ff9601dfd0b3a Mon Sep 17 00:00:00 2001
From: Will McCutchen <will@mccutch.org>
Date: Mon, 14 Jan 2019 15:49:31 -0800
Subject: [PATCH] Adopt functional options pattern, rename MaxMemory ->
 MaxBodySize

---
 cmd/go-httpbin/main.go   | 17 ++++++------
 httpbin/handlers.go      | 12 ++++-----
 httpbin/handlers_test.go | 15 +++++------
 httpbin/httpbin.go       | 57 ++++++++++++++++++++++------------------
 httpbin/httpbin_test.go  | 34 +++++++++++++-----------
 5 files changed, 71 insertions(+), 64 deletions(-)

diff --git a/cmd/go-httpbin/main.go b/cmd/go-httpbin/main.go
index 5f0a873..d4fbf2e 100644
--- a/cmd/go-httpbin/main.go
+++ b/cmd/go-httpbin/main.go
@@ -17,13 +17,13 @@ const defaultPort = 8080
 
 var (
 	port        int
-	maxMemory   int64
+	maxBodySize int64
 	maxDuration time.Duration
 )
 
 func main() {
 	flag.IntVar(&port, "port", defaultPort, "Port to listen on")
-	flag.Int64Var(&maxMemory, "max-memory", httpbin.DefaultMaxMemory, "Maximum size of request or response, in bytes")
+	flag.Int64Var(&maxBodySize, "max-body-size", httpbin.DefaultMaxBodySize, "Maximum size of request or response, in bytes")
 	flag.DurationVar(&maxDuration, "max-duration", httpbin.DefaultMaxDuration, "Maximum duration a response may take")
 	flag.Parse()
 
@@ -33,10 +33,10 @@ func main() {
 	// check for environment vars if we have default values for our command
 	// line flags.
 	var err error
-	if maxMemory == httpbin.DefaultMaxMemory && os.Getenv("MAX_MEMORY") != "" {
-		maxMemory, err = strconv.ParseInt(os.Getenv("MAX_MEMORY"), 10, 64)
+	if maxBodySize == httpbin.DefaultMaxBodySize && os.Getenv("MAX_BODY_SIZE") != "" {
+		maxBodySize, err = strconv.ParseInt(os.Getenv("MAX_BODY_SIZE"), 10, 64)
 		if err != nil {
-			fmt.Printf("invalid value %#v for env var MAX_MEMORY: %s\n", os.Getenv("MAX_MEMORY"), err)
+			fmt.Printf("invalid value %#v for env var MAX_BODY_SIZE: %s\n", os.Getenv("MAX_BODY_SIZE"), err)
 			flag.Usage()
 			os.Exit(1)
 		}
@@ -58,10 +58,9 @@ func main() {
 		}
 	}
 
-	h := httpbin.NewHTTPBinWithOptions(&httpbin.Options{
-		MaxMemory:   maxMemory,
-		MaxDuration: maxDuration,
-	})
+	h := httpbin.New(
+		httpbin.WithMaxBodySize(maxBodySize),
+		httpbin.WithMaxDuration(maxDuration))
 
 	listenAddr := net.JoinHostPort("0.0.0.0", strconv.Itoa(port))
 	log.Printf("addr=%s", listenAddr)
diff --git a/httpbin/handlers.go b/httpbin/handlers.go
index 273acc4..5df341f 100644
--- a/httpbin/handlers.go
+++ b/httpbin/handlers.go
@@ -462,7 +462,7 @@ func (h *HTTPBin) Delay(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	delay, err := parseBoundedDuration(parts[2], 0, h.options.MaxDuration)
+	delay, err := parseBoundedDuration(parts[2], 0, h.MaxDuration)
 	if err != nil {
 		http.Error(w, "Invalid duration", http.StatusBadRequest)
 		return
@@ -490,7 +490,7 @@ func (h *HTTPBin) Drip(w http.ResponseWriter, r *http.Request) {
 
 	userDuration := q.Get("duration")
 	if userDuration != "" {
-		duration, err = parseBoundedDuration(userDuration, 0, h.options.MaxDuration)
+		duration, err = parseBoundedDuration(userDuration, 0, h.MaxDuration)
 		if err != nil {
 			http.Error(w, "Invalid duration", http.StatusBadRequest)
 			return
@@ -499,7 +499,7 @@ func (h *HTTPBin) Drip(w http.ResponseWriter, r *http.Request) {
 
 	userDelay := q.Get("delay")
 	if userDelay != "" {
-		delay, err = parseBoundedDuration(userDelay, 0, h.options.MaxDuration)
+		delay, err = parseBoundedDuration(userDelay, 0, h.MaxDuration)
 		if err != nil {
 			http.Error(w, "Invalid delay", http.StatusBadRequest)
 			return
@@ -509,7 +509,7 @@ func (h *HTTPBin) Drip(w http.ResponseWriter, r *http.Request) {
 	userNumBytes := q.Get("numbytes")
 	if userNumBytes != "" {
 		numbytes, err = strconv.ParseInt(userNumBytes, 10, 64)
-		if err != nil || numbytes <= 0 || numbytes > h.options.MaxMemory {
+		if err != nil || numbytes <= 0 || numbytes > h.MaxBodySize {
 			http.Error(w, "Invalid numbytes", http.StatusBadRequest)
 			return
 		}
@@ -524,7 +524,7 @@ func (h *HTTPBin) Drip(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	if duration+delay > h.options.MaxDuration {
+	if duration+delay > h.MaxDuration {
 		http.Error(w, "Too much time", http.StatusBadRequest)
 		return
 	}
@@ -573,7 +573,7 @@ func (h *HTTPBin) Range(w http.ResponseWriter, r *http.Request) {
 	w.Header().Add("ETag", fmt.Sprintf("range%d", numBytes))
 	w.Header().Add("Accept-Ranges", "bytes")
 
-	if numBytes <= 0 || numBytes > h.options.MaxMemory {
+	if numBytes <= 0 || numBytes > h.MaxBodySize {
 		http.Error(w, "Invalid number of bytes", http.StatusBadRequest)
 		return
 	}
diff --git a/httpbin/handlers_test.go b/httpbin/handlers_test.go
index ab70f5c..00c094c 100644
--- a/httpbin/handlers_test.go
+++ b/httpbin/handlers_test.go
@@ -19,13 +19,12 @@ import (
 	"time"
 )
 
-const maxMemory int64 = 1024 * 1024
+const maxBodySize int64 = 1024 * 1024
 const maxDuration time.Duration = 1 * time.Second
 
-var app = NewHTTPBinWithOptions(&Options{
-	MaxMemory:   maxMemory,
-	MaxDuration: maxDuration,
-})
+var app = New(
+	WithMaxBodySize(maxBodySize),
+	WithMaxDuration(maxDuration))
 
 var handler = app.Handler()
 
@@ -589,7 +588,7 @@ func TestPost__InvalidJSON(t *testing.T) {
 }
 
 func TestPost__BodyTooBig(t *testing.T) {
-	body := make([]byte, maxMemory+1)
+	body := make([]byte, maxBodySize+1)
 
 	r, _ := http.NewRequest("POST", "/post", bytes.NewReader(body))
 	w := httptest.NewRecorder()
@@ -1479,7 +1478,7 @@ func TestDrip(t *testing.T) {
 
 		{&url.Values{"numbytes": {"1"}}, 0, 1, http.StatusOK},
 		{&url.Values{"numbytes": {"101"}}, 0, 101, http.StatusOK},
-		{&url.Values{"numbytes": {fmt.Sprintf("%d", maxMemory)}}, 0, int(maxMemory), http.StatusOK},
+		{&url.Values{"numbytes": {fmt.Sprintf("%d", maxBodySize)}}, 0, int(maxBodySize), http.StatusOK},
 
 		{&url.Values{"code": {"100"}}, 0, 10, 100},
 		{&url.Values{"code": {"404"}}, 0, 10, 404},
@@ -1569,7 +1568,7 @@ func TestDrip(t *testing.T) {
 		{&url.Values{"numbytes": {"0"}}, http.StatusBadRequest},
 		{&url.Values{"numbytes": {"-1"}}, http.StatusBadRequest},
 		{&url.Values{"numbytes": {"0xff"}}, http.StatusBadRequest},
-		{&url.Values{"numbytes": {fmt.Sprintf("%d", maxMemory+1)}}, http.StatusBadRequest},
+		{&url.Values{"numbytes": {fmt.Sprintf("%d", maxBodySize+1)}}, http.StatusBadRequest},
 
 		{&url.Values{"code": {"foo"}}, http.StatusBadRequest},
 		{&url.Values{"code": {"-1"}}, http.StatusBadRequest},
diff --git a/httpbin/httpbin.go b/httpbin/httpbin.go
index ad12c29..37f0601 100644
--- a/httpbin/httpbin.go
+++ b/httpbin/httpbin.go
@@ -8,7 +8,7 @@ import (
 
 // Default configuration values
 const (
-	DefaultMaxMemory   int64 = 1024 * 1024
+	DefaultMaxBodySize int64 = 1024 * 1024
 	DefaultMaxDuration       = 10 * time.Second
 )
 
@@ -76,20 +76,14 @@ type streamResponse struct {
 	URL     string      `json:"url"`
 }
 
-// Options are used to configure HTTPBin
-type Options struct {
-	// How much memory a request is allowed to consume in bytes, as a limit on
-	// the size of incoming request bodies and on responses generated
-	MaxMemory int64
-
-	// Maximum duration of a request, for those requests that allow user
-	// control over timing (e.g. /delay)
-	MaxDuration time.Duration
-}
-
 // HTTPBin contains the business logic
 type HTTPBin struct {
-	options *Options
+	// Max size of an incoming request generated response body, in bytes
+	MaxBodySize int64
+
+	// Max duration of a request, for those requests that allow user control
+	// over timing (e.g. /delay)
+	MaxDuration time.Duration
 }
 
 // Handler returns an http.Handler that exposes all HTTPBin endpoints
@@ -173,25 +167,38 @@ func (h *HTTPBin) Handler() http.Handler {
 	// Apply global middleware
 	var handler http.Handler
 	handler = mux
-	handler = limitRequestSize(h.options.MaxMemory, handler)
+	handler = limitRequestSize(h.MaxBodySize, handler)
 	handler = metaRequests(handler)
 	handler = logger(handler)
 	return handler
 }
 
-// NewHTTPBin creates a new HTTPBin instance with default options
-func NewHTTPBin() *HTTPBin {
-	return &HTTPBin{
-		options: &Options{
-			MaxMemory:   DefaultMaxMemory,
-			MaxDuration: DefaultMaxDuration,
-		},
+// New creates a new HTTPBin instance
+func New(opts ...OptionFunc) *HTTPBin {
+	h := &HTTPBin{
+		MaxBodySize: DefaultMaxBodySize,
+		MaxDuration: DefaultMaxDuration,
+	}
+	for _, opt := range opts {
+		opt(h)
+	}
+	return h
+}
+
+// OptionFunc uses the "functional options" pattern to customize an HTTPBin
+// instance
+type OptionFunc func(*HTTPBin)
+
+// WithMaxBodySize sets the maximum amount of memory
+func WithMaxBodySize(m int64) OptionFunc {
+	return func(h *HTTPBin) {
+		h.MaxBodySize = m
 	}
 }
 
-// NewHTTPBinWithOptions creates a new HTTPBin instance with the given options
-func NewHTTPBinWithOptions(options *Options) *HTTPBin {
-	return &HTTPBin{
-		options: options,
+// WithMaxDuration sets the maximum amount of time httpbin may take to respond
+func WithMaxDuration(d time.Duration) OptionFunc {
+	return func(h *HTTPBin) {
+		h.MaxDuration = d
 	}
 }
diff --git a/httpbin/httpbin_test.go b/httpbin/httpbin_test.go
index 8e2e991..1d0d4fc 100644
--- a/httpbin/httpbin_test.go
+++ b/httpbin/httpbin_test.go
@@ -5,26 +5,28 @@ import (
 	"time"
 )
 
-func TestNewHTTPBin__Defaults(t *testing.T) {
-	h := NewHTTPBin()
-	if h.options.MaxMemory != DefaultMaxMemory {
-		t.Fatalf("expected default MaxMemory == %d, got %#v", DefaultMaxMemory, h.options.MaxMemory)
+func TestNew(t *testing.T) {
+	h := New()
+	if h.MaxBodySize != DefaultMaxBodySize {
+		t.Fatalf("expected default MaxBodySize == %d, got %#v", DefaultMaxBodySize, h.MaxBodySize)
 	}
-	if h.options.MaxDuration != DefaultMaxDuration {
-		t.Fatalf("expected default MaxDuration == %s, got %#v", DefaultMaxDuration, h.options.MaxDuration)
+	if h.MaxDuration != DefaultMaxDuration {
+		t.Fatalf("expected default MaxDuration == %s, got %#v", DefaultMaxDuration, h.MaxDuration)
 	}
 }
 
-func TestNewHTTPBinWithOptions__Defaults(t *testing.T) {
-	o := &Options{
-		MaxDuration: 1 * time.Second,
-		MaxMemory:   1024,
-	}
-	h := NewHTTPBinWithOptions(o)
-	if h.options.MaxMemory != o.MaxMemory {
-		t.Fatalf("expected MaxMemory == %d, got %#v", o.MaxMemory, h.options.MaxMemory)
+func TestNewOptions(t *testing.T) {
+	maxDuration := 1 * time.Second
+	maxBodySize := int64(1024)
+
+	h := New(
+		WithMaxBodySize(maxBodySize),
+		WithMaxDuration(maxDuration))
+
+	if h.MaxBodySize != maxBodySize {
+		t.Fatalf("expected MaxBodySize == %d, got %#v", maxBodySize, h.MaxBodySize)
 	}
-	if h.options.MaxDuration != o.MaxDuration {
-		t.Fatalf("expected MaxDuration == %s, got %#v", o.MaxDuration, h.options.MaxDuration)
+	if h.MaxDuration != maxDuration {
+		t.Fatalf("expected MaxDuration == %s, got %#v", maxDuration, h.MaxDuration)
 	}
 }
-- 
GitLab