diff --git a/README.md b/README.md index ae69ea991..d6044f329 100644 --- a/README.md +++ b/README.md @@ -200,20 +200,22 @@ routeWithAuth := NewAuthRoute("/index", "GET" ProcessorFunc, true, Authendicator The caching layer for HTTP routes is specified per Route. ```go -// routeCache is the builder needed to build a cache for the corresponding route -type routeCache struct { +// RouteCache is the builder needed to build a cache for the corresponding route +type RouteCache struct { // cache is the ttl cache implementation to be used cache cache.TTLCache // age specifies the minimum and maximum amount for max-age and min-fresh header values respectively // regarding the client cache-control requests in seconds age age } + +func NewRouteCache(ttlCache cache.TTLCache, age Age) *RouteCache ``` #### server cache - The **cache key** is based on the route path and the url request parameters. - The server caches only **GET requests**. -- The server implementation must specify **Age** parameters upon construction. +- The server implementation must specify an **Age** parameters upon construction. - Age with **Min=0** and **Max=0** effectively disables caching - The route should return always the most fresh object instance. - An **ETag header** must be always in responses that are part of the cache, representing the hash of the response. @@ -229,6 +231,25 @@ That implies that all generic handler functionalities MUST be delegated to a cus i.e. counting number of server client requests etc ... ``` +### Usage + +- provide the cache in the route builder +```go +NewRouteBuilder("/", handler). + WithRouteCache(cache, http.Age{ + Min: 30 * time.Minute, + Max: 1 * time.Hour, + }). + MethodGet() +``` + +- use the cache as a middleware +```go +NewRouteBuilder("/", handler). + WithMiddlewares(NewCachingMiddleware(NewRouteCache(cc, Age{Max: 10 * time.Second}))). + MethodGet() +``` + #### client cache-control The client can control the cache with the appropriate Headers - `max-age=?` @@ -297,6 +318,7 @@ improvement for big response objects. - we could extend the metrics to use the key of the object as a label as well for more fine-grained tuning. But this has been left out for now, due to the potentially huge number of metric objects. We can review according to usage or make this optional in the future. +- improve the serialization performance for the cache response objects ### Asynchronous diff --git a/component/http/cache.go b/component/http/cache.go index a1763c6a8..94e5dff29 100644 --- a/component/http/cache.go +++ b/component/http/cache.go @@ -77,7 +77,7 @@ type executor func(now int64, key string) *CachedResponse // cacheHandler wraps the an execution logic with a cache layer // exec is the processor func that the cache will wrap // rc is the route cache implementation to be used -func cacheHandler(exec executor, rc *routeCache) func(request *cacheHandlerRequest) (response *CacheHandlerResponse, e error) { +func cacheHandler(exec executor, rc *RouteCache) func(request *cacheHandlerRequest) (response *CacheHandlerResponse, e error) { return func(request *cacheHandlerRequest) (response *CacheHandlerResponse, e error) { @@ -114,7 +114,7 @@ func cacheHandler(exec executor, rc *routeCache) func(request *cacheHandlerReque // getResponse will get the appropriate Response either using the cache or the executor, // depending on the -func getResponse(cfg *cacheControl, path, key string, now int64, rc *routeCache, exec executor) *CachedResponse { +func getResponse(cfg *cacheControl, path, key string, now int64, rc *RouteCache, exec executor) *CachedResponse { if cfg.noCache { return exec(now, key) @@ -166,7 +166,7 @@ func isValid(age, maxAge int64, validators ...validator) (bool, validationContex // getFromCache is the implementation that will provide a CachedResponse instance from the cache, // if it exists -func getFromCache(key string, rc *routeCache) *CachedResponse { +func getFromCache(key string, rc *RouteCache) *CachedResponse { if resp, ok, err := rc.cache.Get(key); ok && err == nil { if b, ok := resp.([]byte); ok { r := &CachedResponse{} diff --git a/component/http/cache_model.go b/component/http/cache_model.go index add93c790..3420ba82c 100644 --- a/component/http/cache_model.go +++ b/component/http/cache_model.go @@ -13,6 +13,25 @@ type cacheHandlerRequest struct { query string } +// toCacheHandlerRequest transforms the http Request object to the cache handler request +func toCacheHandlerRequest(req *http.Request) *cacheHandlerRequest { + var header string + if req.Header != nil { + header = req.Header.Get(cacheControlHeader) + } + var path string + var query string + if req.URL != nil { + path = req.URL.Path + query = req.URL.RawQuery + } + return &cacheHandlerRequest{ + header: header, + path: path, + query: query, + } +} + // getKey generates a unique cache key based on the route path and the query parameters func (c *cacheHandlerRequest) getKey() string { return fmt.Sprintf("%s:%s", c.path, c.query) @@ -45,41 +64,3 @@ func (c *CachedResponse) encode() ([]byte, error) { func (c *CachedResponse) decode(data []byte) error { return json.Unmarshal(data, c) } - -// fromRequest transforms the Request object to the cache handler request -func fromRequest(path string, req *Request) *cacheHandlerRequest { - var header string - if req.Headers != nil { - header = req.Headers[cacheControlHeader] - } - var query string - if req.Fields != nil { - if fields, err := json.Marshal(req.Fields); err == nil { - query = string(fields) - } - } - return &cacheHandlerRequest{ - header: header, - path: path, - query: query, - } -} - -// fromHTTPRequest transforms the http Request object to the cache handler request -func fromHTTPRequest(req *http.Request) *cacheHandlerRequest { - var header string - if req.Header != nil { - header = req.Header.Get(cacheControlHeader) - } - var path string - var query string - if req.URL != nil { - path = req.URL.Path - query = req.URL.RawQuery - } - return &cacheHandlerRequest{ - header: header, - path: path, - query: query, - } -} diff --git a/component/http/cache_test.go b/component/http/cache_test.go index 32dea81f5..8ddd0d262 100644 --- a/component/http/cache_test.go +++ b/component/http/cache_test.go @@ -2,8 +2,11 @@ package http import ( "errors" + "fmt" + "net/http" "os" "strconv" + "strings" "testing" "time" @@ -136,11 +139,31 @@ type routeConfig struct { type requestParams struct { path string - header map[string]string - fields map[string]string + header Header + query string timeInstance int64 } +func newRequestAt(timeInstant int64, cacheControlHeaders ...string) requestParams { + params := requestParams{ + query: "VALUE=1", + timeInstance: timeInstant, + header: make(map[string]string), + } + if len(cacheControlHeaders) > 0 { + params.header[cacheControlHeader] = strings.Join(cacheControlHeaders, ",") + } + return params +} + +func maxAgeHeader(value string) string { + return fmt.Sprintf("%s=%s", cacheHeaderMaxAge, value) +} + +func minFreshHeader(value string) string { + return fmt.Sprintf("%s=%s", cacheControlMinFresh, value) +} + type testArgs struct { routeConfig routeConfig cache cache.TTLCache @@ -174,12 +197,9 @@ func TestMinAgeCache_WithoutClientHeader(t *testing.T) { { // initial request, will fill up the cache { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 1, - }, - routeConfig: rc, - response: &Response{Payload: 10, Header: testHeader(10)}, + requestParams: newRequestAt(1), + routeConfig: rc, + response: &Response{Payload: 10, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -192,12 +212,9 @@ func TestMinAgeCache_WithoutClientHeader(t *testing.T) { }, // cache Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 9, - }, - routeConfig: rc, - response: &Response{Payload: 10, Header: testHeader(2)}, + requestParams: newRequestAt(9), + routeConfig: rc, + response: &Response{Payload: 10, Header: testHeader(2)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -211,12 +228,9 @@ func TestMinAgeCache_WithoutClientHeader(t *testing.T) { }, // still cached Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 11, - }, - routeConfig: rc, - response: &Response{Payload: 10, Header: testHeader(0)}, + requestParams: newRequestAt(11), + routeConfig: rc, + response: &Response{Payload: 10, Header: testHeader(0)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -230,12 +244,9 @@ func TestMinAgeCache_WithoutClientHeader(t *testing.T) { }, // new Response , due to expiry validator 10 + 1 - 12 < 0 { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 12, - }, - routeConfig: rc, - response: &Response{Payload: 120, Header: testHeader(10)}, + requestParams: newRequestAt(12), + routeConfig: rc, + response: &Response{Payload: 120, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -268,12 +279,9 @@ func TestNoAgeCache_WithoutClientHeader(t *testing.T) { { // initial request, will fill up the cache { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 1, - }, - routeConfig: rc, - response: &Response{Payload: 10}, + requestParams: newRequestAt(1), + routeConfig: rc, + response: &Response{Payload: 10}, metrics: testMetrics{ map[string]*metricState{ "/": {}, @@ -283,12 +291,9 @@ func TestNoAgeCache_WithoutClientHeader(t *testing.T) { }, // no cached Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 2, - }, - routeConfig: rc, - response: &Response{Payload: 20}, + requestParams: newRequestAt(2), + routeConfig: rc, + response: &Response{Payload: 20}, metrics: testMetrics{ map[string]*metricState{ "/": {}, @@ -298,13 +303,9 @@ func TestNoAgeCache_WithoutClientHeader(t *testing.T) { }, // no cached Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=5"}, - timeInstance: 2, - }, - routeConfig: rc, - response: &Response{Payload: 20}, + requestParams: newRequestAt(2, maxAgeHeader("5")), + routeConfig: rc, + response: &Response{Payload: 20}, metrics: testMetrics{ map[string]*metricState{ "/": {}, @@ -314,13 +315,9 @@ func TestNoAgeCache_WithoutClientHeader(t *testing.T) { }, // no cached Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5"}, - timeInstance: 2, - }, - routeConfig: rc, - response: &Response{Payload: 20}, + requestParams: newRequestAt(2, minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 20}, metrics: testMetrics{ map[string]*metricState{ "/": {}, @@ -345,13 +342,9 @@ func TestCache_WithConstantMaxAgeHeader(t *testing.T) { { // initial request, will fill up the cache { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=5"}, - timeInstance: 1, - }, - routeConfig: rc, - response: &Response{Payload: 10, Header: testHeader(10)}, + requestParams: newRequestAt(1, maxAgeHeader("5")), + routeConfig: rc, + response: &Response{Payload: 10, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -364,13 +357,9 @@ func TestCache_WithConstantMaxAgeHeader(t *testing.T) { }, // cached Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=5"}, - timeInstance: 3, - }, - routeConfig: rc, - response: &Response{Payload: 10, Header: testHeader(8)}, + requestParams: newRequestAt(3, maxAgeHeader("5")), + routeConfig: rc, + response: &Response{Payload: 10, Header: testHeader(8)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -384,13 +373,9 @@ func TestCache_WithConstantMaxAgeHeader(t *testing.T) { }, // new Response, because max-age > 9 - 1 { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=5"}, - timeInstance: 9, - }, - routeConfig: rc, - response: &Response{Payload: 90, Header: testHeader(10)}, + requestParams: newRequestAt(9, maxAgeHeader("5")), + routeConfig: rc, + response: &Response{Payload: 90, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -405,13 +390,9 @@ func TestCache_WithConstantMaxAgeHeader(t *testing.T) { }, // cached Response right before the age threshold max-age == 14 - 9 { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=5"}, - timeInstance: 14, - }, - routeConfig: rc, - response: &Response{Payload: 90, Header: testHeader(5)}, + requestParams: newRequestAt(14, maxAgeHeader("5")), + routeConfig: rc, + response: &Response{Payload: 90, Header: testHeader(5)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -426,13 +407,9 @@ func TestCache_WithConstantMaxAgeHeader(t *testing.T) { }, // new Response, because max-age > 15 - 9 { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=5"}, - timeInstance: 15, - }, - routeConfig: rc, - response: &Response{Payload: 150, Header: testHeader(10)}, + requestParams: newRequestAt(15, maxAgeHeader("5")), + routeConfig: rc, + response: &Response{Payload: 150, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -462,12 +439,9 @@ func TestCache_WithMaxAgeHeaders(t *testing.T) { { // initial request, will fill up the cache { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(30)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(30)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -480,13 +454,9 @@ func TestCache_WithMaxAgeHeaders(t *testing.T) { }, // cached Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=10"}, - timeInstance: 10, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(20)}, + requestParams: newRequestAt(10, maxAgeHeader("10")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(20)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -500,13 +470,9 @@ func TestCache_WithMaxAgeHeaders(t *testing.T) { }, // cached Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=20"}, - timeInstance: 20, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(20, maxAgeHeader("20")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -520,13 +486,9 @@ func TestCache_WithMaxAgeHeaders(t *testing.T) { }, // new Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=5"}, - timeInstance: 20, - }, - routeConfig: rc, - response: &Response{Payload: 200, Header: testHeader(30)}, + requestParams: newRequestAt(20, maxAgeHeader("5")), + routeConfig: rc, + response: &Response{Payload: 200, Header: testHeader(30)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -541,13 +503,9 @@ func TestCache_WithMaxAgeHeaders(t *testing.T) { }, // cache Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=25"}, - timeInstance: 25, - }, - routeConfig: rc, - response: &Response{Payload: 200, Header: testHeader(25)}, + requestParams: newRequestAt(25, maxAgeHeader("25")), + routeConfig: rc, + response: &Response{Payload: 200, Header: testHeader(25)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -577,12 +535,9 @@ func TestMinAgeCache_WithHighMaxAgeHeaders(t *testing.T) { { // initial request, will fill up the cache { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(5)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(5)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -595,13 +550,9 @@ func TestMinAgeCache_WithHighMaxAgeHeaders(t *testing.T) { }, // despite the max-age request, the cache will refresh because of it's ttl { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=100"}, - timeInstance: 6, - }, - routeConfig: rc, - response: &Response{Payload: 60, Header: testHeader(5)}, + requestParams: newRequestAt(6, maxAgeHeader("100")), + routeConfig: rc, + response: &Response{Payload: 60, Header: testHeader(5)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -629,12 +580,9 @@ func TestNoMinAgeCache_WithLowMaxAgeHeaders(t *testing.T) { { // initial request, will fill up the cache { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(30)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(30)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -648,13 +596,9 @@ func TestNoMinAgeCache_WithLowMaxAgeHeaders(t *testing.T) { // a max-age=0 request will always refresh the cache, // if there is not minAge limit set { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=0"}, - timeInstance: 1, - }, - routeConfig: rc, - response: &Response{Payload: 10, Header: testHeader(30)}, + requestParams: newRequestAt(1, maxAgeHeader("0")), + routeConfig: rc, + response: &Response{Payload: 10, Header: testHeader(30)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -683,12 +627,9 @@ func TestMinAgeCache_WithMaxAgeHeaders(t *testing.T) { { // initial request, will fill up the cache { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(30)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(30)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -702,13 +643,9 @@ func TestMinAgeCache_WithMaxAgeHeaders(t *testing.T) { // cached Response still, because of minAge override // note : max-age=2 gets ignored { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=2"}, - timeInstance: 4, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeaderWithWarning(26, "max-age=5")}, + requestParams: newRequestAt(4, maxAgeHeader("2")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeaderWithWarning(26, "max-age=5")}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -722,13 +659,9 @@ func TestMinAgeCache_WithMaxAgeHeaders(t *testing.T) { }, // cached Response because of bigger max-age parameter { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=20"}, - timeInstance: 5, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(25)}, + requestParams: newRequestAt(5, maxAgeHeader("20")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(25)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -742,12 +675,8 @@ func TestMinAgeCache_WithMaxAgeHeaders(t *testing.T) { }, // new Response because of minAge floor { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "max-age=3"}, - timeInstance: 6, - }, - routeConfig: rc, + requestParams: newRequestAt(6, maxAgeHeader("3")), + routeConfig: rc, // note : no Warning because it s a new Response response: &Response{Payload: 60, Header: testHeader(30)}, metrics: testMetrics{ @@ -778,13 +707,9 @@ func TestCache_WithConstantMinFreshHeaders(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0, minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -797,13 +722,9 @@ func TestCache_WithConstantMinFreshHeaders(t *testing.T) { }, // expecting cache Response, as value is still fresh : 5 - 0 == 5 { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5"}, - timeInstance: 5, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(5)}, + requestParams: newRequestAt(5, minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(5)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -817,13 +738,9 @@ func TestCache_WithConstantMinFreshHeaders(t *testing.T) { }, // expecting new Response, as value is not fresh enough { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5"}, - timeInstance: 6, - }, - routeConfig: rc, - response: &Response{Payload: 60, Header: testHeader(10)}, + requestParams: newRequestAt(6, minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 60, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -838,13 +755,9 @@ func TestCache_WithConstantMinFreshHeaders(t *testing.T) { }, // cache Response, as value is expired : 11 - 6 <= 5 { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5"}, - timeInstance: 11, - }, - routeConfig: rc, - response: &Response{Payload: 60, Header: testHeader(5)}, + requestParams: newRequestAt(11, minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 60, Header: testHeader(5)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -859,13 +772,9 @@ func TestCache_WithConstantMinFreshHeaders(t *testing.T) { }, // expecting new Response { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5"}, - timeInstance: 12, - }, - routeConfig: rc, - response: &Response{Payload: 120, Header: testHeader(10)}, + requestParams: newRequestAt(12, minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 120, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -894,13 +803,9 @@ func TestNoMaxFreshCache_WithLargeMinFreshHeaders(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0, minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -912,13 +817,9 @@ func TestNoMaxFreshCache_WithLargeMinFreshHeaders(t *testing.T) { err: nil, }, { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=100"}, - timeInstance: 1, - }, - routeConfig: rc, - response: &Response{Payload: 10, Header: testHeader(10)}, + requestParams: newRequestAt(1, minFreshHeader("100")), + routeConfig: rc, + response: &Response{Payload: 10, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -947,13 +848,9 @@ func TestMaxAgeCache_WithMinFreshHeaders(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0, minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -966,13 +863,9 @@ func TestMaxAgeCache_WithMinFreshHeaders(t *testing.T) { }, // expecting cache Response, as min-fresh is bounded by maxFresh configuration parameter { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=100"}, - timeInstance: 5, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeaderWithWarning(5, "min-fresh=5")}, + requestParams: newRequestAt(5, minFreshHeader("100")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeaderWithWarning(5, "min-fresh=5")}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1000,13 +893,9 @@ func TestCache_WithMixedHeaders(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5,max-age=5"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0, maxAgeHeader("5"), minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1019,13 +908,9 @@ func TestCache_WithMixedHeaders(t *testing.T) { }, // expecting cache Response, as value is still fresh : 5 - 0 == min-fresh and still young : 5 - 0 < max-age { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5,max-age=10"}, - timeInstance: 5, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(5)}, + requestParams: newRequestAt(5, maxAgeHeader("10"), minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(5)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1039,13 +924,9 @@ func TestCache_WithMixedHeaders(t *testing.T) { }, // new Response, as value is not fresh enough : 6 - 0 > min-fresh { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=5,max-age=10"}, - timeInstance: 6, - }, - routeConfig: rc, - response: &Response{Payload: 60, Header: testHeader(10)}, + requestParams: newRequestAt(6, maxAgeHeader("10"), minFreshHeader("5")), + routeConfig: rc, + response: &Response{Payload: 60, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1060,13 +941,9 @@ func TestCache_WithMixedHeaders(t *testing.T) { }, // cached Response, as value is still fresh enough and still young { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=10,max-age=8"}, - timeInstance: 6, - }, - routeConfig: rc, - response: &Response{Payload: 60, Header: testHeaderWithWarning(10, "min-fresh=5")}, + requestParams: newRequestAt(6, maxAgeHeader("8"), minFreshHeader("10")), + routeConfig: rc, + response: &Response{Payload: 60, Header: testHeaderWithWarning(10, "min-fresh=5")}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1081,13 +958,9 @@ func TestCache_WithMixedHeaders(t *testing.T) { }, // new Response, as value is still fresh enough but too old { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "min-fresh=10,max-age=8"}, - timeInstance: 15, - }, - routeConfig: rc, - response: &Response{Payload: 150, Header: testHeader(10)}, + requestParams: newRequestAt(15, maxAgeHeader("8"), minFreshHeader("10")), + routeConfig: rc, + response: &Response{Payload: 150, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1118,12 +991,9 @@ func TestCache_WithHandlerErrorWithoutHeaders(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1134,10 +1004,7 @@ func TestCache_WithHandlerErrorWithoutHeaders(t *testing.T) { }, }, { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 11, - }, + requestParams: newRequestAt(11), routeConfig: routeConfig{ path: rc.path, hnd: func(now int64, key string) *CachedResponse { @@ -1180,11 +1047,8 @@ func TestCache_WithHandlerErr(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, + requestParams: newRequestAt(0), + routeConfig: rc, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1216,13 +1080,10 @@ func TestCache_WithCacheGetErr(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, - cache: cacheImpl, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, + cache: cacheImpl, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1234,13 +1095,10 @@ func TestCache_WithCacheGetErr(t *testing.T) { }, // new Response, because of cache get error { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 1, - }, - routeConfig: rc, - response: &Response{Payload: 10, Header: testHeader(10)}, - cache: cacheImpl, + requestParams: newRequestAt(1), + routeConfig: rc, + response: &Response{Payload: 10, Header: testHeader(10)}, + cache: cacheImpl, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1274,13 +1132,10 @@ func TestCache_WithCacheSetErr(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, - cache: cacheImpl, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, + cache: cacheImpl, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1292,13 +1147,10 @@ func TestCache_WithCacheSetErr(t *testing.T) { }, // new Response, because of cache get error { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 1, - }, - routeConfig: rc, - response: &Response{Payload: 10, Header: testHeader(10)}, - cache: cacheImpl, + requestParams: newRequestAt(1), + routeConfig: rc, + response: &Response{Payload: 10, Header: testHeader(10)}, + cache: cacheImpl, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1328,7 +1180,7 @@ func TestCache_WithMixedPaths(t *testing.T) { // initial request { requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, + query: "VALUE=1", timeInstance: 0, path: "/1", }, @@ -1347,7 +1199,7 @@ func TestCache_WithMixedPaths(t *testing.T) { // cached Response for the same path { requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, + query: "VALUE=1", timeInstance: 1, path: "/1", }, @@ -1367,7 +1219,7 @@ func TestCache_WithMixedPaths(t *testing.T) { // initial request for second path { requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, + query: "VALUE=1", timeInstance: 1, path: "/2", }, @@ -1391,7 +1243,7 @@ func TestCache_WithMixedPaths(t *testing.T) { // cached Response for second path { requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, + query: "VALUE=1", timeInstance: 2, path: "/2", }, @@ -1429,12 +1281,9 @@ func TestCache_WithMixedRequestParameters(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1448,7 +1297,7 @@ func TestCache_WithMixedRequestParameters(t *testing.T) { // cached Response for same request parameter { requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, + query: "VALUE=1", timeInstance: 1, }, routeConfig: rc, @@ -1467,7 +1316,7 @@ func TestCache_WithMixedRequestParameters(t *testing.T) { // new Response for different request parameter { requestParams: requestParams{ - fields: map[string]string{"VALUE": "2"}, + query: "VALUE=2", timeInstance: 1, }, routeConfig: rc, @@ -1486,7 +1335,7 @@ func TestCache_WithMixedRequestParameters(t *testing.T) { // cached Response for second request parameter { requestParams: requestParams{ - fields: map[string]string{"VALUE": "2"}, + query: "VALUE=2", timeInstance: 2, }, routeConfig: rc, @@ -1518,12 +1367,9 @@ func TestZeroAgeCache_WithNoCacheHeaders(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1536,13 +1382,9 @@ func TestZeroAgeCache_WithNoCacheHeaders(t *testing.T) { }, // expecting new Response, as we are using no-cache Header { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "no-cache"}, - timeInstance: 5, - }, - routeConfig: rc, - response: &Response{Payload: 50, Header: testHeader(10)}, + requestParams: newRequestAt(5, "no-cache"), + routeConfig: rc, + response: &Response{Payload: 50, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1570,12 +1412,9 @@ func TestMinAgeCache_WithNoCacheHeaders(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1588,13 +1427,9 @@ func TestMinAgeCache_WithNoCacheHeaders(t *testing.T) { }, // expecting cached Response, as we are using no-cache Header but are within the minAge limit { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "no-cache"}, - timeInstance: 2, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeaderWithWarning(8, "max-age=2")}, + requestParams: newRequestAt(2, "no-cache"), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeaderWithWarning(8, "max-age=2")}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1608,13 +1443,9 @@ func TestMinAgeCache_WithNoCacheHeaders(t *testing.T) { }, // expecting new Response, as we are using no-cache Header { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "no-cache"}, - timeInstance: 5, - }, - routeConfig: rc, - response: &Response{Payload: 50, Header: testHeader(10)}, + requestParams: newRequestAt(5, "no-cache"), + routeConfig: rc, + response: &Response{Payload: 50, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1643,12 +1474,9 @@ func TestZeroAgeCache_WithNoStoreHeaders(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1661,13 +1489,9 @@ func TestZeroAgeCache_WithNoStoreHeaders(t *testing.T) { }, // expecting new Response, as we are using no-store Header { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "no-store"}, - timeInstance: 5, - }, - routeConfig: rc, - response: &Response{Payload: 50, Header: testHeader(10)}, + requestParams: newRequestAt(5, "no-store"), + routeConfig: rc, + response: &Response{Payload: 50, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1695,12 +1519,9 @@ func TestMinAgeCache_WithNoStoreHeaders(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1713,13 +1534,9 @@ func TestMinAgeCache_WithNoStoreHeaders(t *testing.T) { }, // expecting cached Response, as we are using no-store Header but are within the minAge limit { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "no-store"}, - timeInstance: 2, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeaderWithWarning(8, "max-age=2")}, + requestParams: newRequestAt(2, "no-store"), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeaderWithWarning(8, "max-age=2")}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1733,13 +1550,9 @@ func TestMinAgeCache_WithNoStoreHeaders(t *testing.T) { }, // expecting new Response, as we are using no-store Header { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "no-store"}, - timeInstance: 5, - }, - routeConfig: rc, - response: &Response{Payload: 50, Header: testHeader(10)}, + requestParams: newRequestAt(5, "no-store"), + routeConfig: rc, + response: &Response{Payload: 50, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1768,12 +1581,9 @@ func TestCache_WithForceCacheHeaders(t *testing.T) { { // initial request { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - timeInstance: 0, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(10)}, + requestParams: newRequestAt(0), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(10)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1786,13 +1596,9 @@ func TestCache_WithForceCacheHeaders(t *testing.T) { }, // expecting cache Response, as min-fresh is bounded by maxFresh configuration parameter { - requestParams: requestParams{ - fields: map[string]string{"VALUE": "1"}, - header: map[string]string{cacheControlHeader: "only-if-cached"}, - timeInstance: 5, - }, - routeConfig: rc, - response: &Response{Payload: 0, Header: testHeader(5)}, + requestParams: newRequestAt(5, "only-if-cached"), + routeConfig: rc, + response: &Response{Payload: 0, Header: testHeader(5)}, metrics: testMetrics{ map[string]*metricState{ "/": { @@ -1817,7 +1623,7 @@ func assertCache(t *testing.T, args [][]testArgs) { // that returns the current time instant times '10' multiplied by the VALUE parameter in the request exec := func(request requestParams) func(now int64, key string) *CachedResponse { return func(now int64, key string) *CachedResponse { - i, err := strconv.Atoi(request.fields["VALUE"]) + i, err := strconv.Atoi(strings.Split(request.query, "=")[1]) if err != nil { return &CachedResponse{ Err: err, @@ -1846,7 +1652,11 @@ func assertCache(t *testing.T, args [][]testArgs) { path = arg.requestParams.path } - request := fromRequest(path, NewRequest(arg.requestParams.fields, nil, arg.requestParams.header, nil)) + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s?%s", path, arg.requestParams.query), nil) + assert.NoError(t, err) + propagateHeaders(arg.requestParams.header, req.Header) + assert.NoError(t, err) + request := toCacheHandlerRequest(req) var hnd executor if arg.routeConfig.hnd != nil { @@ -1869,10 +1679,7 @@ func assertCache(t *testing.T, args [][]testArgs) { return arg.requestParams.timeInstance } - response, err := cacheHandler(hnd, &routeCache{ - cache: ch, - age: arg.routeConfig.age.toAgeInSeconds(), - })(request) + response, err := cacheHandler(hnd, NewRouteCache(ch, arg.routeConfig.age))(request) if arg.err != nil { assert.Error(t, err) diff --git a/component/http/handler.go b/component/http/handler.go index b6621df5b..c4af5f885 100644 --- a/component/http/handler.go +++ b/component/http/handler.go @@ -2,7 +2,6 @@ package http import ( "errors" - "fmt" "net/http" "strings" @@ -144,8 +143,6 @@ func handleSuccess(w http.ResponseWriter, r *http.Request, rsp *Response, enc en propagateHeaders(rsp.Header, w.Header()) - println(fmt.Sprintf("w.Header() = %v", w.Header())) - _, err = w.Write(p) return err } diff --git a/component/http/middleware.go b/component/http/middleware.go index e9df11c7d..86ec368a0 100644 --- a/component/http/middleware.go +++ b/component/http/middleware.go @@ -123,6 +123,32 @@ func NewLoggingTracingMiddleware(path string) MiddlewareFunc { } } +// NewCachingMiddleware creates a cache layer as a middleware +// when used as part of a middleware chain any middleware later in the chain, +// will not be executed, but the headers it appends will be part of the cache +func NewCachingMiddleware(rc *RouteCache) MiddlewareFunc { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + next.ServeHTTP(w, r) + return + } + req := toCacheHandlerRequest(r) + resp, err := cacheHandler(handlerExecutor(w, r, func(writer http.ResponseWriter, request *http.Request) { + next.ServeHTTP(writer, request) + }), rc)(req) + if err != nil { + log.Errorf("could not handle request with the cache processor: %v", err) + return + } + propagateHeaders(resp.Header, w.Header()) + if i, err := w.Write(resp.Bytes); err != nil { + log.Errorf("could not Write cache processor result into Response %d: %v", i, err) + } + }) + } +} + // MiddlewareChain chains middlewares to a handler func. func MiddlewareChain(f http.Handler, mm ...MiddlewareFunc) http.Handler { for i := len(mm) - 1; i >= 0; i-- { diff --git a/component/http/middleware_test.go b/component/http/middleware_test.go index 00626223b..5c0a68fd1 100644 --- a/component/http/middleware_test.go +++ b/component/http/middleware_test.go @@ -5,6 +5,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -105,6 +106,63 @@ func TestMiddlewares(t *testing.T) { } } +func TestCachingMiddleware(t *testing.T) { + + getRequest, err := http.NewRequest("GET", "/test", nil) + assert.NoError(t, err) + + postRequest, err := http.NewRequest("POST", "/test", nil) + assert.NoError(t, err) + + type args struct { + next http.Handler + mws []MiddlewareFunc + } + + testingCache := newTestingCache() + testingCache.instant = now + + tests := []struct { + name string + args args + r *http.Request + expectedCode int + expectedBody string + cacheState cacheState + }{ + {"caching middleware with POST request", args{next: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(202) + i, err := w.Write([]byte{1, 2, 3, 4}) + assert.NoError(t, err) + assert.Equal(t, 4, i) + }), mws: []MiddlewareFunc{NewCachingMiddleware(NewRouteCache(testingCache, Age{Max: 1 * time.Second}))}}, + postRequest, 202, "\x01\x02\x03\x04", cacheState{}}, + {"caching middleware with GET request", args{next: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + i, err := w.Write([]byte{1, 2, 3, 4}) + assert.NoError(t, err) + assert.Equal(t, 4, i) + }), mws: []MiddlewareFunc{NewCachingMiddleware(NewRouteCache(testingCache, Age{Max: 1 * time.Second}))}}, + getRequest, 200, "\x01\x02\x03\x04", cacheState{ + setOps: 1, + getOps: 1, + size: 1, + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rc := httptest.NewRecorder() + rw := newResponseWriter(rc) + tt.args.next = MiddlewareChain(tt.args.next, tt.args.mws...) + tt.args.next.ServeHTTP(rw, tt.r) + assert.Equal(t, tt.expectedCode, rw.Status()) + assert.Equal(t, tt.expectedBody, rc.Body.String()) + assertCacheState(t, *testingCache, tt.cacheState) + }) + } + +} + func TestResponseWriter(t *testing.T) { rc := httptest.NewRecorder() rw := newResponseWriter(rc) diff --git a/component/http/route.go b/component/http/route.go index 2c34dabad..21888084f 100644 --- a/component/http/route.go +++ b/component/http/route.go @@ -49,7 +49,7 @@ type RouteBuilder struct { middlewares []MiddlewareFunc authenticator auth.Authenticator handler http.HandlerFunc - routeCache *routeCache + routeCache *RouteCache errors []error } @@ -94,10 +94,7 @@ func (rb *RouteBuilder) WithRouteCache(cache cache.TTLCache, ageBounds Age) *Rou log.Warnf("route cache for %s is disabled because of empty Age property %v ") } - rc := &routeCache{ - cache: cache, - age: ageBounds.toAgeInSeconds(), - } + rc := NewRouteCache(cache, ageBounds) rb.routeCache = rc rb.errors = append(rb.errors, cErrors...) @@ -177,26 +174,18 @@ func (rb *RouteBuilder) Build() (Route, error) { if len(rb.middlewares) > 0 { middlewares = append(middlewares, rb.middlewares...) } - - // TODO :refactor ... - var hnd http.HandlerFunc - + // cache middleware is always last, so that it caches only the headers of the handler if rb.routeCache != nil { - if rb.method != http.MethodGet { return Route{}, errors.New("cannot apply cache to a route with any method other than GET ") } - hnd = wrapHandlerFunc(rb.handler, rb.routeCache) - } - - if hnd == nil { - hnd = rb.handler + middlewares = append(middlewares, NewCachingMiddleware(rb.routeCache)) } return Route{ path: rb.path, method: rb.method, - handler: hnd, + handler: rb.handler, middlewares: middlewares, }, nil } diff --git a/component/http/route_cache.go b/component/http/route_cache.go index f475e726e..045b28c61 100644 --- a/component/http/route_cache.go +++ b/component/http/route_cache.go @@ -5,12 +5,10 @@ import ( "time" "github.com/beatlabs/patron/cache" - - "github.com/beatlabs/patron/log" ) -// routeCache is the builder needed to build a cache for the corresponding route -type routeCache struct { +// RouteCache is the builder needed to build a cache for the corresponding route +type RouteCache struct { // cache is the ttl cache implementation to be used cache cache.TTLCache // age specifies the minimum and maximum amount for max-age and min-fresh Header values respectively @@ -18,6 +16,13 @@ type routeCache struct { age age } +func NewRouteCache(ttlCache cache.TTLCache, age Age) *RouteCache { + return &RouteCache{ + cache: ttlCache, + age: age.toAgeInSeconds(), + } +} + // Age defines the route cache life-time boundaries for cached objects type Age struct { // Min adds a minimum age threshold for the client controlled cache responses. @@ -43,21 +48,6 @@ type age struct { max int64 } -// wrapHandlerFunc wraps the handler func with the cache handler interface -func wrapHandlerFunc(handler http.HandlerFunc, rc *routeCache) http.HandlerFunc { - return func(response http.ResponseWriter, request *http.Request) { - req := fromHTTPRequest(request) - if resp, err := cacheHandler(handlerExecutor(response, request, handler), rc)(req); err != nil { - log.Errorf("could not handle request with the cache processor: %v", err) - } else { - propagateHeaders(resp.Header, response.Header()) - if i, err := response.Write(resp.Bytes); err != nil { - log.Errorf("could not Write cache processor result into Response %d: %v", i, err) - } - } - } -} - // handlerExecutor is the function that will create a new CachedResponse based on a HandlerFunc implementation func handlerExecutor(_ http.ResponseWriter, request *http.Request, hnd http.HandlerFunc) executor { return func(now int64, key string) *CachedResponse { diff --git a/component/http/route_cache_test.go b/component/http/route_cache_test.go index b00f865e6..7584a81aa 100644 --- a/component/http/route_cache_test.go +++ b/component/http/route_cache_test.go @@ -92,58 +92,101 @@ func assertRouteBuilder(t *testing.T, arg arg, routeBuilder *RouteBuilder, cache } } -func TestHandlerWrapper(t *testing.T) { +func TestRouteCacheImplementation_WithSingleRequest(t *testing.T) { - type arg struct { - handler http.HandlerFunc - req *http.Request - rsp *responseReadWriter - } + cc := newTestingCache() + cc.instant = now - args := []arg{ + var executions uint32 + + preWrapper := newMiddlewareWrapper(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("pre-middleware-header", "pre") + }) + + postWrapper := newMiddlewareWrapper(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("post-middleware-header", "post") + }) + + routeBuilder := NewRouteBuilder("/path", func(context context.Context, request *Request) (response *Response, e error) { + atomic.AddUint32(&executions, 1) + newResponse := NewResponse("body") + newResponse.Header["Custom-Header"] = "11" + return newResponse, nil + }). + WithRouteCache(cc, Age{Max: 10 * time.Second}). + WithMiddlewares(preWrapper.middleware, postWrapper.middleware). + MethodGet() + + ctx, cln := context.WithTimeout(context.Background(), 5*time.Second) + + port := 50023 + runRoute(ctx, t, routeBuilder, port) + + assertResponse(ctx, t, []http.Response{ { - handler: func(writer http.ResponseWriter, request *http.Request) { - i, err := writer.Write([]byte(request.RequestURI)) - assert.NoError(t, err) - assert.True(t, i > 0) + Header: map[string][]string{ + cacheControlHeader: {"max-age=10"}, + "Content-Type": {"application/json; charset=utf-8"}, + "Content-Length": {"6"}, + "Post-Middleware-Header": {"post"}, + "Pre-Middleware-Header": {"pre"}, + "Custom-Header": {"11"}, }, - rsp: newResponseReadWriter(), - req: &http.Request{RequestURI: "http://www.localhost.com"}, + Body: &bodyReader{body: "\"body\""}, }, - } - - for _, testArg := range args { - c := newTestingCache() - rc := &routeCache{ - cache: c, - age: Age{Max: 10}.toAgeInSeconds(), - } + { + Header: map[string][]string{ + cacheControlHeader: {"max-age=10"}, + "Content-Type": {"application/json; charset=utf-8"}, + "Content-Length": {"6"}, + "Post-Middleware-Header": {"post"}, + "Pre-Middleware-Header": {"pre"}, + "Custom-Header": {"11"}, + }, + Body: &bodyReader{body: "\"body\""}, + }, + }, port) - wrappedHandler := wrapHandlerFunc(testArg.handler, rc) + assertCacheState(t, *cc, cacheState{ + setOps: 1, + getOps: 2, + size: 1, + }) - wrappedHandler(testArg.rsp, testArg.req) + assert.Equal(t, 2, preWrapper.invocations) + assert.Equal(t, 2, postWrapper.invocations) - b, err := testArg.rsp.readAll() - assert.NoError(t, err) - assert.NotNil(t, b) - assert.True(t, len(b) > 0) - } + assert.Equal(t, executions, uint32(1)) + cln() } -func TestRouteCacheImplementation_WithSingleRequest(t *testing.T) { +func TestRouteCacheAsMiddleware_WithSingleRequest(t *testing.T) { cc := newTestingCache() cc.instant = now var executions uint32 + preWrapper := newMiddlewareWrapper(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("pre-middleware-header", "pre") + }) + + postWrapper := newMiddlewareWrapper(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("post-middleware-header", "post") + }) + routeBuilder := NewRouteBuilder("/path", func(context context.Context, request *Request) (response *Response, e error) { atomic.AddUint32(&executions, 1) newResponse := NewResponse("body") - newResponse.Header["Custom-Header"] = "11" + newResponse.Header["internal-handler-header"] = "header" return newResponse, nil - }).WithRouteCache(cc, Age{Max: 10 * time.Second}).MethodGet() + }). + WithMiddlewares( + preWrapper.middleware, + NewCachingMiddleware(NewRouteCache(cc, Age{Max: 10 * time.Second})), + postWrapper.middleware). + MethodGet() ctx, cln := context.WithTimeout(context.Background(), 5*time.Second) @@ -153,19 +196,23 @@ func TestRouteCacheImplementation_WithSingleRequest(t *testing.T) { assertResponse(ctx, t, []http.Response{ { Header: map[string][]string{ - cacheControlHeader: {"max-age=10"}, - "Content-Type": {"application/json; charset=utf-8"}, - "Content-Length": {"6"}, - "Custom-Header": {"11"}, + cacheControlHeader: {"max-age=10"}, + "Content-Type": {"application/json; charset=utf-8"}, + "Content-Length": {"6"}, + "Post-Middleware-Header": {"post"}, + "Pre-Middleware-Header": {"pre"}, + "Internal-Handler-Header": {"header"}, }, Body: &bodyReader{body: "\"body\""}, }, { Header: map[string][]string{ - cacheControlHeader: {"max-age=10"}, - "Content-Type": {"application/json; charset=utf-8"}, - "Content-Length": {"6"}, - "Custom-Header": {"11"}, + cacheControlHeader: {"max-age=10"}, + "Content-Type": {"application/json; charset=utf-8"}, + "Post-Middleware-Header": {"post"}, + "Pre-Middleware-Header": {"pre"}, + "Content-Length": {"6"}, + "Internal-Handler-Header": {"header"}, }, Body: &bodyReader{body: "\"body\""}, }, @@ -177,12 +224,33 @@ func TestRouteCacheImplementation_WithSingleRequest(t *testing.T) { size: 1, }) + assert.Equal(t, 2, preWrapper.invocations) + // NOTE : the post middleware is not executed, as it s hidden behind the cache + assert.Equal(t, 1, postWrapper.invocations) + assert.Equal(t, executions, uint32(1)) cln() } +type middlewareWrapper struct { + middleware MiddlewareFunc + invocations int +} + +func newMiddlewareWrapper(middlewareFunc func(w http.ResponseWriter, r *http.Request)) *middlewareWrapper { + wrapper := &middlewareWrapper{} + wrapper.middleware = func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + wrapper.invocations++ + middlewareFunc(w, r) + next.ServeHTTP(w, r) + }) + } + return wrapper +} + func TestRawRouteCacheImplementation_WithSingleRequest(t *testing.T) { cc := newTestingCache() @@ -190,13 +258,24 @@ func TestRawRouteCacheImplementation_WithSingleRequest(t *testing.T) { var executions uint32 + preWrapper := newMiddlewareWrapper(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("pre-middleware-header", "pre") + }) + + postWrapper := newMiddlewareWrapper(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("post-middleware-header", "post") + }) + routeBuilder := NewRawRouteBuilder("/path", func(writer http.ResponseWriter, request *http.Request) { atomic.AddUint32(&executions, 1) i, err := writer.Write([]byte("\"body\"")) - writer.Header().Set("custom-header", "1") + writer.Header().Set("internal-handler-header", "header") assert.NoError(t, err) assert.True(t, i > 0) - }).WithRouteCache(cc, Age{Max: 10 * time.Second}).MethodGet() + }). + WithRouteCache(cc, Age{Max: 10 * time.Second}). + WithMiddlewares(preWrapper.middleware, postWrapper.middleware). + MethodGet() ctx, cln := context.WithTimeout(context.Background(), 5*time.Second) @@ -206,18 +285,22 @@ func TestRawRouteCacheImplementation_WithSingleRequest(t *testing.T) { assertResponse(ctx, t, []http.Response{ { Header: map[string][]string{ - cacheControlHeader: {"max-age=10"}, - "Content-Type": {"text/plain; charset=utf-8"}, - "Content-Length": {"6"}, - "Custom-Header": {"1"}}, + cacheControlHeader: {"max-age=10"}, + "Content-Type": {"text/plain; charset=utf-8"}, + "Content-Length": {"6"}, + "Post-Middleware-Header": {"post"}, + "Pre-Middleware-Header": {"pre"}, + "Internal-Handler-Header": {"header"}}, Body: &bodyReader{body: "\"body\""}, }, { Header: map[string][]string{ - cacheControlHeader: {"max-age=10"}, - "Content-Type": {"text/plain; charset=utf-8"}, - "Content-Length": {"6"}, - "Custom-Header": {"1"}}, + cacheControlHeader: {"max-age=10"}, + "Content-Type": {"text/plain; charset=utf-8"}, + "Content-Length": {"6"}, + "Post-Middleware-Header": {"post"}, + "Pre-Middleware-Header": {"pre"}, + "Internal-Handler-Header": {"header"}}, Body: &bodyReader{body: "\"body\""}, }, }, port) @@ -228,6 +311,9 @@ func TestRawRouteCacheImplementation_WithSingleRequest(t *testing.T) { size: 1, }) + assert.Equal(t, 2, preWrapper.invocations) + assert.Equal(t, 2, postWrapper.invocations) + assert.Equal(t, executions, uint32(1)) cln() @@ -298,14 +384,9 @@ func assertResponse(ctx context.Context, t *testing.T, expected []http.Response, assert.NoError(t, err) - for k, v := range response.Header { - if k == "Content-Length" || k == "Content-Type" || k == "Custom-Header" { - assert.Equal(t, expectedResponse.Header[k], v) - delete(expectedResponse.Header, k) - } + for k, v := range expectedResponse.Header { + assert.Equal(t, v, response.Header[k]) } - // only 1 expected header should be left to check, we should have deleted the rest above - assert.Equal(t, 1, len(expectedResponse.Header)) assert.Equal(t, expectedResponse.Header.Get(cacheControlHeader), response.Header.Get(cacheControlHeader)) assert.True(t, response.Header.Get(cacheHeaderETagHeader) != "") expectedPayload := make([]byte, 6)