cache by service instead of etag

jalvz · jalvz · commit cd40ddbdc4af · 2019-10-08T09:47:43.000+02:00
diff --git a/agentcfg/cache.go b/agentcfg/cache.go
@@ -28,7 +28,7 @@ import (
 )
 
 const (
-	cleanupInterval time.Duration = 60 * time.Second
+	cleanupInterval = 60 * time.Second
 )
 
 type cache struct {
@@ -45,7 +45,7 @@ func newCache(logger *logp.Logger, exp time.Duration) *cache {
 
 func (c *cache) fetch(q Query, fetch func() (Result, error)) (Result, error) {
 	// return from cache if possible
-	value, found := c.gocache.Get(q.Etag)
+	value, found := c.gocache.Get(q.toString())
 	if found && value != nil {
 		return value.(Result), nil
 	}
@@ -54,7 +54,7 @@ func (c *cache) fetch(q Query, fetch func() (Result, error)) (Result, error) {
 	if err != nil {
 		return result, err
 	}
-	c.gocache.SetDefault(result.Source.Etag, result)
+	c.gocache.SetDefault(q.toString(), result)
 	if !authorized(q.IsRum, result.Source.Agent) {
 		return zeroResult(), nil
 	}
diff --git a/agentcfg/cache_test.go b/agentcfg/cache_test.go
@@ -47,37 +47,36 @@ func newCacheSetup(service string, exp time.Duration, init bool) cacheSetup {
 		result: defaultResult,
 	}
 	if init {
-		setup.c.gocache.SetDefault(setup.result.Source.Etag, setup.result)
+		setup.c.gocache.SetDefault(setup.q.toString(), setup.result)
 	}
 	return setup
 }
 
 func TestCache_fetchAndAdd(t *testing.T) {
 	exp := time.Second
-	for name, tc := range map[string]struct {
-		fn   func() (Result, error)
-		init bool
-
-		doc  Result
-		fail bool
+	for name, testCase := range map[string]struct {
+		fetchFunc  func() (Result, error)
+		init       bool
+		doc        Result
+		shouldFail bool
 	}{
-		"DocFromCache":         {fn: testFn, init: true, doc: defaultResult},
-		"DocFromFunctionFails": {fn: testFnErr, fail: true},
-		"DocFromFunction":      {fn: testFn, doc: externalResult},
-		"EmptyDocFromFunction": {fn: testFnSettingsNil, doc: Result{Source{Settings: Settings{}}}},
-		"NilDocFromFunction":   {fn: testFnNil},
+		"DocFromCache":         {fetchFunc: testFn, init: true, doc: defaultResult},
+		"DocFromFunctionFails": {fetchFunc: testFnErr, shouldFail: true},
+		"DocFromFunction":      {fetchFunc: testFn, doc: externalResult},
+		"EmptyDocFromFunction": {fetchFunc: testFnSettingsNil, doc: Result{Source{Settings: Settings{}}}},
+		"NilDocFromFunction":   {fetchFunc: testFnNil},
 	} {
 		t.Run(name, func(t *testing.T) {
-			setup := newCacheSetup(name, exp, tc.init)
+			setup := newCacheSetup(name, exp, testCase.init)
 
-			doc, err := setup.c.fetch(setup.q, tc.fn)
-			assert.Equal(t, tc.doc, doc)
-			if tc.fail {
+			doc, err := setup.c.fetch(setup.q, testCase.fetchFunc)
+			assert.Equal(t, testCase.doc, doc)
+			if testCase.shouldFail {
 				require.Error(t, err)
 			} else {
 				assert.NoError(t, err)
 				//ensure value is cached afterwards
-				cachedDoc, error := setup.c.fetch(setup.q, tc.fn)
+				cachedDoc, error := setup.c.fetch(setup.q, testCase.fetchFunc)
 				require.NoError(t, error)
 				assert.Equal(t, doc, cachedDoc)
 			}
diff --git a/agentcfg/fetch_test.go b/agentcfg/fetch_test.go
@@ -93,7 +93,7 @@ func TestFetcher_Fetch(t *testing.T) {
 		fetch(fetcher, 0.8, 0.5)
 
 		// after key is expired, fetch from Kibana again
-		fetcher.cache.gocache.Delete("123")
+		fetcher.cache.gocache.Delete(query(t.Name()).toString())
 		fetch(fetcher, 0.7, 0.7)
 
 	})
diff --git a/agentcfg/model.go b/agentcfg/model.go
@@ -57,6 +57,10 @@ type Query struct {
 	IsRum   bool    `json:"-"`
 }
 
+func (q Query) toString() string {
+	return q.Service.Name + q.Service.Environment
+}
+
 // Service holds supported attributes for querying configuration
 type Service struct {
 	Name        string `json:"name"`

Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ import (`
`28`	`28`	`)`
`29`	`29`
`30`	`30`	`const (`
`31`		`- cleanupInterval time.Duration = 60 * time.Second`
	`31`	`+ cleanupInterval = 60 * time.Second`
`32`	`32`	`)`
`33`	`33`
`34`	`34`	`type cache struct {`
`@@ -45,7 +45,7 @@ func newCache(logger logp.Logger, exp time.Duration) cache {`
`45`	`45`
`46`	`46`	`func (c *cache) fetch(q Query, fetch func() (Result, error)) (Result, error) {`
`47`	`47`	`// return from cache if possible`
`48`		`- value, found := c.gocache.Get(q.Etag)`
	`48`	`+ value, found := c.gocache.Get(q.toString())`
`49`	`49`	`if found && value != nil {`
`50`	`50`	`return value.(Result), nil`
`51`	`51`	`}`
`@@ -54,7 +54,7 @@ func (c *cache) fetch(q Query, fetch func() (Result, error)) (Result, error) {`
`54`	`54`	`if err != nil {`
`55`	`55`	`return result, err`
`56`	`56`	`}`
`57`		`- c.gocache.SetDefault(result.Source.Etag, result)`
	`57`	`+ c.gocache.SetDefault(q.toString(), result)`
`58`	`58`	`if !authorized(q.IsRum, result.Source.Agent) {`
`59`	`59`	`return zeroResult(), nil`
`60`	`60`	`}`