Fixed issue caching between methods

evg4b · Nov 21, 2023 · d3a5c02 · d3a5c02
1 parent f751aa8
commit d3a5c02
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 3 deletions.
diff --git a/internal/handler/cache/middleware.go b/internal/handler/cache/middleware.go
@@ -46,7 +46,7 @@ func (m *Middleware) Wrap(next contracts.Handler) contracts.Handler {
 }
 
 func (m *Middleware) cacheRequest(writer contracts.ResponseWriter, request *contracts.Request, next contracts.Handler) {
-	cacheKey := m.extractCacheKey(request.URL)
+	cacheKey := m.extractCacheKey(request.Method, request.URL)
 	m.logger.Debugf("extracted %s from request", cacheKey)
 	if cachedResponse := m.getCachedResponse(cacheKey); cachedResponse != nil {
 		m.logger.Debugf("extracted %s from request", cacheKey)
@@ -94,7 +94,7 @@ func (m *Middleware) isCacheableRequest(request *contracts.Request) bool {
 	})
 }
 
-func (m *Middleware) extractCacheKey(url *url.URL) string {
+func (m *Middleware) extractCacheKey(method string, url *url.URL) string {
 	values := url.Query()
 	items := make([]string, 0, len(values))
 	for key, value := range values {
@@ -105,7 +105,7 @@ func (m *Middleware) extractCacheKey(url *url.URL) string {
 
 	sort.Strings(items)
 
-	return helpers.Sprintf("[%s]%s?%s", url.Hostname(), url.Path, strings.Join(items, ";"))
+	return helpers.Sprintf("[%s]%s%s?%s", method, url.Hostname(), url.Path, strings.Join(items, ";"))
 }
 
 func (m *Middleware) getCachedResponse(cacheKey string) *CachedResponse {

diff --git a/internal/handler/cache/middleware_test.go b/internal/handler/cache/middleware_test.go
@@ -189,6 +189,12 @@ func TestCacheMiddleware(t *testing.T) {
 
 		wrappedHandler := middleware.Wrap(handler)
 
+		handler := testutils.NewCounter(func(writer contracts.ResponseWriter, request *contracts.Request) {
+			writer.WriteHeader(http.StatusOK)
+			testutils.CopyHeaders(expectedHeader, writer.Header())
+			helpers.Fprintf(writer, request.Method)
+		})
+
 		testutils.Times(count, func(index int) {
 			recorder := httptest.NewRecorder()
 			url := fmt.Sprintf("https://test-host-%d.com:4200/api/test", index)
@@ -203,4 +209,37 @@ func TestCacheMiddleware(t *testing.T) {
 
 		assert.Equal(t, count, handler.Count())
 	})
+
+	t.Run("should not cache response between different methods matched by one rule", func(t *testing.T) {
+		methods := []string{http.MethodGet, http.MethodPost, http.MethodDelete, http.MethodPut}
+		handler.Reset()
+
+		middleware := cache.NewMiddleware(
+			cache.WithCacheStorage(goCache.New(time.Minute, time.Minute)),
+			cache.WithLogger(mocks.NewNoopLogger(t)),
+			cache.WithMethods(methods),
+			cache.WithGlobs(config.CacheGlobs{"/api/**"}),
+		)
+
+		handler := testutils.NewCounter(func(writer contracts.ResponseWriter, request *contracts.Request) {
+			writer.WriteHeader(http.StatusOK)
+			testutils.CopyHeaders(expectedHeader, writer.Header())
+			helpers.Fprintf(writer, request.Method)
+		})
+
+		wrappedHandler := middleware.Wrap(handler)
+
+		for _, method := range methods {
+			recorder := httptest.NewRecorder()
+			request := httptest.NewRequest(method, "https://test-host.com:4200/api/test", nil)
+			wrappedHandler.ServeHTTP(
+				contracts.WrapResponseWriter(recorder),
+				request,
+			)
+			assert.Equal(t, expectedHeader, recorder.Header())
+			assert.Equal(t, method, testutils.ReadBody(t, recorder))
+		}
+
+		assert.Equal(t, len(methods), handler.Count())
+	})
 }