fix cache logic and test comments

pkg/apiserver/server.go

@@ -51,6 +51,8 @@ func Start(params APIServerParams) {
 	r := mux.NewRouter()
 	r.Use(handlers.CorsMiddleware)
 
+	const DefaultCacheTTL = 1 * time.Minute
+
 	// TODO: make all routes authenticated
 	authRouter := r.NewRoute().Subrouter()
 	authRouter.Use(handlers.RequireValidLicenseIDMiddleware)
@@ -66,9 +68,9 @@ func Start(params APIServerParams) {
 	r.HandleFunc("/api/v1/app/info", handlers.GetCurrentAppInfo).Methods("GET")
 	r.HandleFunc("/api/v1/app/updates", handlers.GetAppUpdates).Methods("GET")
 	r.HandleFunc("/api/v1/app/history", handlers.GetAppHistory).Methods("GET")
-	r.HandleFunc("/api/v1/app/custom-metrics", handlers.CacheMiddleware(handlers.SendCustomAppMetrics, 1*time.Minute)).Methods("POST", "PATCH")
-	r.HandleFunc("/api/v1/app/custom-metrics/{key}", handlers.DeleteCustomAppMetricsKey).Methods("DELETE")
-	r.HandleFunc("/api/v1/app/instance-tags", handlers.SendAppInstanceTags).Methods("POST")
+	r.HandleFunc("/api/v1/app/custom-metrics", handlers.CacheMiddleware(handlers.SendCustomAppMetrics, DefaultCacheTTL)).Methods("POST", "PATCH")
+	r.HandleFunc("/api/v1/app/custom-metrics/{key}", handlers.CacheMiddleware(handlers.DeleteCustomAppMetricsKey, DefaultCacheTTL)).Methods("DELETE")
+	r.HandleFunc("/api/v1/app/instance-tags", handlers.CacheMiddleware(handlers.SendAppInstanceTags, DefaultCacheTTL)).Methods("POST")
 
 	// integration
 	r.HandleFunc("/api/v1/integration/mock-data", handlers.EnforceMockAccess(handlers.PostIntegrationMockData)).Methods("POST")

pkg/handlers/middleware.go

@@ -2,7 +2,9 @@ package handlers
 
 import (
 	"bytes"
+	"crypto/sha256"
 	"encoding/json"
+	"fmt"
 	"io"
 	"net/http"
 	"sync"
@@ -86,6 +88,13 @@ func (c *cache) Set(key string, entry CacheEntry, duration time.Duration) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
+	// Clean up expired entries
+	for k, v := range c.store {
+		if time.Now().After(v.Expiry) {
+			delete(c.store, k)
+		}
+	}
+
 	entry.Expiry = time.Now().Add(duration)
 	c.store[key] = entry
 }
@@ -113,16 +122,18 @@ func CacheMiddleware(next http.HandlerFunc, duration time.Duration) http.Handler
 	return func(w http.ResponseWriter, r *http.Request) {
 		body, err := io.ReadAll(r.Body)
 		if err != nil {
-			logger.Error(errors.Wrap(err, "cache middleware failed ready request body"))
+			logger.Error(errors.Wrap(err, "cache middleware - failed to read request body"))
 			http.Error(w, "cache middleware: unable to read request body", http.StatusInternalServerError)
 			return
 		}
 
 		r.Body = io.NopCloser(bytes.NewBuffer(body))
 
-		key := r.Method + "::" + r.URL.Path + "::" + string(body)
+		hash := sha256.Sum256([]byte(r.Method + "::" + r.URL.Path))
+
+		key := fmt.Sprintf("%x\n", hash)
 
-		if entry, found := cache.Get(key); found {
+		if entry, found := cache.Get(key); found && bytes.Equal(entry.RequestBody, body) {
 			logger.Infof("cache middleware: serving cached payload for method: %s path: %s ttl: %s ", r.Method, r.URL.Path, time.Until(entry.Expiry).Round(time.Second).String())
 			JSONCached(w, entry.StatusCode, json.RawMessage(entry.ResponseBody))
 			return

pkg/handlers/middleware_test.go

@@ -124,7 +124,7 @@ func Test_CacheMiddleware_Expiry(t *testing.T) {
 	cachedHandler.ServeHTTP(recorder, req)
 	require.Equal(t, http.StatusOK, recorder.Code)
 	require.Equal(t, `{"message":"Hello, World!"}`, recorder.Body.String())
-	require.Equal(t, "", recorder.Header().Get("X-Replicated-Served-From-Cache")) // Header should not exist because the response is NOT served from cache
+	require.Equal(t, "", recorder.Header().Get("X-Replicated-Served-From-Cache")) // Header should NOT exist because the response is NOT served from cache
 
 	/* Second request should be served from cache since the payload it the same and under the expiry time */
 	req, recorder = newTestRequest("POST", "/custom-metric", []byte(`{"data": {"numProjects": 2000}}`))
@@ -140,7 +140,7 @@ func Test_CacheMiddleware_Expiry(t *testing.T) {
 	cachedHandler.ServeHTTP(recorder, req)
 	require.Equal(t, http.StatusOK, recorder.Code)
 	require.Equal(t, `{"message":"Hello, World!"}`, recorder.Body.String())
-	require.Equal(t, "", recorder.Header().Get("X-Replicated-Served-From-Cache")) // Header should not exist because the response is NOT served from cache
+	require.Equal(t, "", recorder.Header().Get("X-Replicated-Served-From-Cache")) // Header should NOT exist because the response is NOT served from cache
 
 }
 
@@ -157,13 +157,13 @@ func Test_CacheMiddleware_DoNotCacheErroredPayload(t *testing.T) {
 	cachedHandler.ServeHTTP(recorder, req)
 	require.Equal(t, http.StatusInternalServerError, recorder.Code)
 	require.Equal(t, `{"error":"Something went wrong!"}`, recorder.Body.String())
-	require.Equal(t, "", recorder.Header().Get("X-Replicated-Served-From-Cache")) // Header should not exist because the response is NOT served from cache
+	require.Equal(t, "", recorder.Header().Get("X-Replicated-Served-From-Cache")) // Header should NOT exist because the response is NOT served from cache
 
-	/* Second request should not be served from cache - err'ed payloads not saved to cache */
+	/* Second request should not be served from cache - err'ed payloads are not cached */
 	req, recorder = newTestRequest("POST", "/custom-metric", []byte(`{"data": {"numProjects": 2000}}`))
 	cachedHandler.ServeHTTP(recorder, req)
 	require.Equal(t, http.StatusInternalServerError, recorder.Code)
 	require.Equal(t, `{"error":"Something went wrong!"}`, recorder.Body.String())
-	require.Equal(t, "", recorder.Header().Get("X-Replicated-Served-From-Cache")) // Header should exist because the response is NOT served from cache
+	require.Equal(t, "", recorder.Header().Get("X-Replicated-Served-From-Cache")) // Header should NOT exist because the response is NOT served from cache
 
 }