From 2aafa659768158cdf703d6b045427c3eed87e6c5 Mon Sep 17 00:00:00 2001 From: kenshin579 Date: Sat, 15 Jul 2023 13:38:33 +0900 Subject: [PATCH] [#16] not returning response from cache --- cache.go | 58 ++++++++++++++++++++++----------------------------- cache_test.go | 24 ++++++++++++--------- redis_test.go | 48 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 44 deletions(-) diff --git a/cache.go b/cache.go index db8fcce..151e58c 100644 --- a/cache.go +++ b/cache.go @@ -44,10 +44,6 @@ import ( "github.com/labstack/echo/v4/middleware" ) -const ( - refreshKey = "refreshKey" -) - type ( // CacheStore is the interface to be implemented by custom stores. CacheStore interface { @@ -147,36 +143,31 @@ func CacheWithConfig(config CacheConfig) echo.MiddlewareFunc { } if c.Request().Method == http.MethodGet { + // isCached := false sortURLParams(c.Request().URL) key := generateKey(c.Request().Method, c.Request().URL.String()) - params := c.Request().URL.Query() - - if _, ok := params[refreshKey]; ok { - refreshKeyValue := params.Get(refreshKey) - delete(params, refreshKeyValue) - config.Store.Release(key) - } else { - if cachedResponse, ok := config.Store.Get(key); ok { - response := toCacheResponse(cachedResponse) - now := time.Now() - if now.After(response.Expiration) { - response.LastAccess = now - response.Frequency++ - config.Store.Set(key, response.bytes(), response.Expiration) - - for k, v := range response.Header { - c.Response().Header().Set(k, strings.Join(v, ",")) - } - c.Response().WriteHeader(http.StatusOK) - c.Response().Write(response.Value) - return nil - } + if cachedResponse, ok := config.Store.Get(key); ok { + response := toCacheResponse(cachedResponse) + now := time.Now() + + // not expired. return response from the cache + if !isExpired(now, response.Expiration) { + // restore the response in the cache + response.LastAccess = now + response.Frequency++ - config.Store.Release(key) + config.Store.Set(key, response.bytes(), response.Expiration) + for k, v := range response.Header { + c.Response().Header().Set(k, strings.Join(v, ",")) + } + c.Response().WriteHeader(http.StatusOK) + c.Response().Write(response.Value) + return nil } } + // Response resBody := new(bytes.Buffer) mw := io.MultiWriter(c.Response().Writer, resBody) writer := &bodyDumpResponseWriter{Writer: mw, ResponseWriter: c.Response().Writer} @@ -186,9 +177,8 @@ func CacheWithConfig(config CacheConfig) echo.MiddlewareFunc { c.Error(err) } - statusCode := writer.statusCode - value := resBody.Bytes() - if statusCode < http.StatusBadRequest { + if writer.statusCode < http.StatusBadRequest { + value := resBody.Bytes() now := time.Now() response := CacheResponse{ @@ -198,15 +188,13 @@ func CacheWithConfig(config CacheConfig) echo.MiddlewareFunc { LastAccess: now, Frequency: 1, } + if !isAllFieldsEmpty(value) { config.Store.Set(key, response.bytes(), response.Expiration) } } return nil } - if err := next(c); err != nil { - c.Error(err) - } return nil } } @@ -342,3 +330,7 @@ func isMapEmpty(m map[string]any) bool { } return true } + +func isExpired(now, expiration time.Time) bool { + return now.After(expiration) +} diff --git a/cache_test.go b/cache_test.go index 0ee5078..d62351f 100644 --- a/cache_test.go +++ b/cache_test.go @@ -48,8 +48,9 @@ func Test_CacheWithConfig(t *testing.T) { } type wants struct { - code int - isCached bool + code int + responseBody string + isCached bool } tests := []struct { name string @@ -71,15 +72,16 @@ func Test_CacheWithConfig(t *testing.T) { }, }, wants: wants{ - code: http.StatusOK, - isCached: true, + code: http.StatusOK, + responseBody: "test", + isCached: true, }, }, { name: "test ExcludePaths", args: args{ method: http.MethodGet, - url: "http://foo.bar/test-1", + url: "http://foo.bar/test-2", cacheConfig: CacheConfig{ Store: NewCacheMemoryStoreWithConfig(CacheMemoryStoreConfig{ Capacity: 5, @@ -90,8 +92,9 @@ func Test_CacheWithConfig(t *testing.T) { }, }, wants: wants{ - code: http.StatusOK, - isCached: false, + code: http.StatusOK, + responseBody: "test", + isCached: false, }, }, { @@ -109,8 +112,9 @@ func Test_CacheWithConfig(t *testing.T) { }, }, wants: wants{ - code: http.StatusOK, - isCached: false, + code: http.StatusOK, + responseBody: "", + isCached: false, }, }, } @@ -125,7 +129,7 @@ func Test_CacheWithConfig(t *testing.T) { _ = mw(handler)(c) assert.Equal(t, tt.wants.code, rec.Code) - assert.Equal(t, "test", rec.Body.String()) + assert.Equal(t, tt.wants.responseBody, rec.Body.String()) cacheResp, ok := tt.args.cacheConfig.Store.Get(generateKey(tt.args.method, tt.args.url)) assert.Equal(t, tt.wants.isCached, ok) diff --git a/redis_test.go b/redis_test.go index dd455ef..021c35f 100644 --- a/redis_test.go +++ b/redis_test.go @@ -70,7 +70,10 @@ func (suite *cacheRedisStoreTestSuite) Test_Redis_CacheStore() { } func (suite *cacheRedisStoreTestSuite) Test_Echo_CacheWithConfig() { + actualCalledCountForTestAPI := 0 + suite.echo.GET("/test", func(c echo.Context) error { + actualCalledCountForTestAPI++ return c.String(http.StatusOK, "test") }) @@ -82,7 +85,49 @@ func (suite *cacheRedisStoreTestSuite) Test_Echo_CacheWithConfig() { return c.String(http.StatusOK, `{"symbolId":"","type":"","price":0.0}`) }) - suite.Run("GET /test with non empty body", func() { + suite.Run("GET /test - return actual response and store in the cache", func() { + req := httptest.NewRequest(http.MethodGet, "/test", nil) + rec := httptest.NewRecorder() + + suite.echo.ServeHTTP(rec, req) + + suite.Equal(http.StatusOK, rec.Code) + suite.Equal("test", rec.Body.String()) + + key := generateKey(http.MethodGet, "/test") + data, ok := suite.cacheStore.Get(key) + suite.True(ok) + + var cacheResponse CacheResponse + err := json.Unmarshal(data, &cacheResponse) + suite.NoError(err) + suite.Equal("test", string(cacheResponse.Value)) + suite.Equal(1, actualCalledCountForTestAPI) + }) + + suite.Run("GET /test - not expired. return response from the cache", func() { + req := httptest.NewRequest(http.MethodGet, "/test", nil) + rec := httptest.NewRecorder() + + suite.echo.ServeHTTP(rec, req) + + suite.Equal(http.StatusOK, rec.Code) + suite.Equal("test", rec.Body.String()) + + key := generateKey(http.MethodGet, "/test") + data, ok := suite.cacheStore.Get(key) + suite.True(ok) + + var cacheResponse CacheResponse + err := json.Unmarshal(data, &cacheResponse) + suite.NoError(err) + suite.Equal("test", string(cacheResponse.Value)) + suite.Equal(1, actualCalledCountForTestAPI) + }) + + suite.Run("GET /test - expired. return actual response", func() { + time.Sleep(5 * time.Second) + req := httptest.NewRequest(http.MethodGet, "/test", nil) rec := httptest.NewRecorder() @@ -99,6 +144,7 @@ func (suite *cacheRedisStoreTestSuite) Test_Echo_CacheWithConfig() { err := json.Unmarshal(data, &cacheResponse) suite.NoError(err) suite.Equal("test", string(cacheResponse.Value)) + suite.Equal(2, actualCalledCountForTestAPI) }) suite.Run("GET /empty/string", func() {