Minor refactor + code comments

Author: mmmarcos

cache.go

@@ -8,17 +8,21 @@ import (
 	"time"
 )
 
-// Basic in-memory cache for response data (currently only a named type for map)
+// Simple in-memory cache middleware.
+//
+// Response are indexed by the request URL path and are stored
+// only if they contain the "Expires" header.
 type CacheMiddleware struct {
 	cache map[string]*cachedResponse
 }
 
+// A cached response with its given Expires
 type cachedResponse struct {
 	responseData []byte
 	expires      int64
 }
 
-// Creates an empty in-memory cache middleware
+// Creates an empty cache middleware
 func NewCacheMiddleware() *CacheMiddleware {
 	var c CacheMiddleware
 	c.cache = make(map[string]*cachedResponse)
@@ -31,23 +35,22 @@ func (c *CacheMiddleware) ProcessRequest(rw http.ResponseWriter, req *http.Reque
 
 	entry, ok := c.cache[req.URL.Path]
 
-	// Not on cache or expired response
+	// Response is not on cache or already expired
 	if !ok || entry.expires <= time.Now().Unix() {
-		log.Printf("Cache MISS")
-		rw.Header().Add("X-Cache-Status", "MISS")
-
+		setCacheStatus(rw, "MISS")
 		delete(c.cache, req.URL.Path) // not necessary?
 		return
 	}
 
-	// Read cached (and not expired) response
+	// Read cached response
 	res, err := http.ReadResponse(bufio.NewReader(bytes.NewReader(entry.responseData)), req)
 	if err != nil {
-		log.Fatal(err)
+		log.Printf("Error reading response stored in cache: %v", err)
+		setCacheStatus(rw, "MISS")
+		return
 	}
 
-	log.Printf("Cache HIT")
-	rw.Header().Add("X-Cache-Status", "HIT")
+	setCacheStatus(rw, "HIT")
 	WriteResponse(rw, res)
 }
 
@@ -71,3 +74,8 @@ func (c *CacheMiddleware) ProcessResponse(res *http.Response, req *http.Request)
 	b := DumpResponse(res)
 	c.cache[req.URL.Path] = &cachedResponse{responseData: b.Bytes(), expires: t.Unix()}
 }
+
+func setCacheStatus(rw http.ResponseWriter, status string) {
+	log.Printf("Cache %v", status)
+	rw.Header().Add("X-Cache-Status", status)
+}

cache_test.go

@@ -2,14 +2,14 @@ package main
 
 import (
 	"bytes"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 	"time"
 )
 
-// Tests processing requests and responses resulting in HIT/MISS
+// Tests requests resulting in HIT/MISS
 func TestCacheMiddleware(t *testing.T) {
 	// Setup test
 	c := NewCacheMiddleware()
@@ -63,6 +63,6 @@ func makeDummyResponse() *http.Response {
 		ProtoMajor: 1,
 		ProtoMinor: 0,
 		Header:     make(http.Header),
-		Body:       ioutil.NopCloser(bytes.NewBufferString("Hello World")),
+		Body:       io.NopCloser(bytes.NewBufferString("Hello World")),
 	}
 }

reverso.go

@@ -1,68 +1,52 @@
 package main
 
 import (
-	"fmt"
 	"log"
 	"net/http"
 	"net/url"
 )
 
 // Reverso is an HTTP handler behaving as a reverse proxy.
 //
-// Reverso forwards incoming requests to a target server and
-// sends the response back to the client.
+// Incoming requests are forwarded to the host specified in originURL.
+//
+// Responses containing the "Expires" header are stored in an in-memory cache
+// and served from there on further requests, as long as they do not expire
 type Reverso struct {
 	// Origin server URL to forward requests.
 	originURL url.URL
 
-	// In-memory cache middleware
+	// In-memory cache middleware to store response data.
 	cache CacheMiddleware
 }
 
 // Handler function to responds to an HTTP request.
 func (r *Reverso) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 	log.Println(req.Method, req.URL.Path)
 
-	r.cache.ProcessRequest(rw, req)
+	// Fetch from cache if available
+	r.cache.ProcessRequest(rw, req) // writes X-Cache-Status header
 
 	if rw.Header().Get("X-Cache-Status") == "MISS" {
-
-		// Fetch request from origin server
-		res, err := r.fetchFromOrigin(req)
+		// Modify request host and scheme to point to origin server
+		req.URL.Host = r.originURL.Host
+		req.URL.Scheme = r.originURL.Scheme
+		req.RequestURI = "" // Must be empty for client requests (see field docs in https://pkg.go.dev/net/http#Request)
+
+		// Fetch from origin server
+		log.Printf("Forwarding request to: '%v'", req.URL.String())
+		res, err := (&http.Client{}).Do(req)
 		if err != nil {
-			log.Println(err)
+			log.Printf("Error: %v", err)
 			rw.WriteHeader(http.StatusInternalServerError)
 			return
 		}
 		b := DumpResponse(res)
 
+		// Process response, stores in cache if contains Expires header
 		r.cache.ProcessResponse(ReadResponse(b.Bytes(), req), req)
 
+		// Write response back
 		WriteResponse(rw, ReadResponse(b.Bytes(), req))
 	}
 }
-
-func (r *Reverso) fetchFromOrigin(req *http.Request) (*http.Response, error) {
-	// Modify request to forward to origin server
-	req.URL.Scheme = r.originURL.Scheme
-	req.URL.Host = r.originURL.Host
-	req.RequestURI = "" // Should be empty for client requests (see src/net/http/client.go:217)
-
-	log.Printf("Forwarding request to: '%v'", req.URL.String())
-
-	// Send request to the origin server
-	res, err := (&http.Client{}).Do(req)
-	if err != nil {
-		return nil, &internalError{err.Error()}
-	}
-
-	return res, nil
-}
-
-type internalError struct {
-	msg string
-}
-
-func (e *internalError) Error() string {
-	return fmt.Sprintf("Error: %v", e.msg)
-}

reverso_test.go

@@ -10,49 +10,52 @@ import (
 	"testing"
 )
 
-// Checks if client receives body and headers sent from origin
+// Checks if client receives response sent by origin
 func TestHandleSimpleRequest(t *testing.T) {
 	const expectedBodyStr string = "Hello from the other side"
 	const customHeaderKey string = "X-Test-Header"
 	const customHeaderVal string = "Custom header from origin"
 
-	// Mock objects
+	// Setup
 	req := httptest.NewRequest(http.MethodGet, "/", nil)
 	rec := httptest.NewRecorder()
 	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set(customHeaderKey, customHeaderVal)
 		fmt.Fprint(w, expectedBodyStr)
 	}))
 	defer svr.Close()
-
-	// Test reverse proxy
 	r := &Reverso{originURL: parseServerURL(svr.URL), cache: *NewCacheMiddleware()}
+
+	// Test reverse proxy handler
 	r.ServeHTTP(rec, req)
 
 	res := rec.Result()
 	defer res.Body.Close()
+	body, err := io.ReadAll(res.Body)
+	if err != nil {
+		log.Fatal(err)
+	}
 
-	bodyStr := readAll(res.Body)
-	if bodyStr != expectedBodyStr {
-		t.Errorf("Expected '%v', got '%v'", expectedBodyStr, bodyStr)
+	if string(body) != expectedBodyStr {
+		t.Errorf("Expected '%v', got '%v'", expectedBodyStr, string(body))
 	}
 
 	if v := res.Header.Get(customHeaderKey); v != customHeaderVal {
-		t.Errorf("Expected header value '%v', got '%v'", customHeaderVal, v)
+		t.Errorf("Expected header '%v:%v', got '%v'", customHeaderKey, customHeaderVal, v)
 	}
 }
 
 // Checks if client receives HTTP status code sent by origin
 func TestHandleTeapotRequest(t *testing.T) {
-	// Mock objects
+	// Setup
 	req := httptest.NewRequest(http.MethodGet, "/teapot", nil)
 	rec := httptest.NewRecorder()
 	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusTeapot)
 	}))
 	defer svr.Close()
 
-	// Test reverse proxy
+	// Test reverse proxy handler
 	r := &Reverso{originURL: parseServerURL(svr.URL), cache: *NewCacheMiddleware()}
 	r.ServeHTTP(rec, req)
 
@@ -63,11 +66,13 @@ func TestHandleTeapotRequest(t *testing.T) {
 	}
 }
 
-// Checks for internal server errors when using an invalid server URL
 func TestInvalidOriginURL(t *testing.T) {
+	// Setup
 	req := httptest.NewRequest(http.MethodGet, "/", nil)
 	rec := httptest.NewRecorder()
 	r := &Reverso{} // empty origin URL
+
+	// Test reverse proxy handler
 	r.ServeHTTP(rec, req)
 	res := rec.Result()
 
@@ -84,12 +89,3 @@ func parseServerURL(rawURL string) url.URL {
 	}
 	return *serverURL
 }
-
-// Read all from r and return content as string
-func readAll(r io.Reader) string {
-	body, err := io.ReadAll(r)
-	if err != nil {
-		log.Fatal(err)
-	}
-	return string(body)
-}

util.go

@@ -13,7 +13,6 @@ func WriteResponse(rw http.ResponseWriter, res *http.Response) {
 	// Write header
 	for key, values := range res.Header {
 		for _, value := range values {
-			log.Println(key, value)
 			rw.Header().Add(key, value)
 		}
 	}