From e49c813f7d4aa7b18e69e654015413a5b26fe75f Mon Sep 17 00:00:00 2001 From: Ravi Atluri Date: Thu, 31 Aug 2023 18:25:53 +0530 Subject: [PATCH 1/6] Added a cached loader --- xload/providers/cached/cache.go | 73 +++++++++++++++++++++++++++ xload/providers/cached/cache_test.go | 43 ++++++++++++++++ xload/providers/cached/go.mod | 20 ++++++++ xload/providers/cached/go.sum | 27 ++++++++++ xload/providers/cached/loader.go | 34 +++++++++++++ xload/providers/cached/loader_test.go | 9 ++++ xload/providers/cached/options.go | 46 +++++++++++++++++ 7 files changed, 252 insertions(+) create mode 100644 xload/providers/cached/cache.go create mode 100644 xload/providers/cached/cache_test.go create mode 100644 xload/providers/cached/go.mod create mode 100644 xload/providers/cached/go.sum create mode 100644 xload/providers/cached/loader.go create mode 100644 xload/providers/cached/loader_test.go create mode 100644 xload/providers/cached/options.go diff --git a/xload/providers/cached/cache.go b/xload/providers/cached/cache.go new file mode 100644 index 0000000..12c0551 --- /dev/null +++ b/xload/providers/cached/cache.go @@ -0,0 +1,73 @@ +package cached + +import ( + "sync" + "time" +) + +// Cacher is the interface for custom cache implementations. +type Cacher interface { + Get(key string) (string, error) + Set(key, value string, ttl time.Duration) error +} + +// MapCache is a simple cache implementation using a map. +type MapCache struct { + m map[string]string + ttl map[string]time.Time + + now func() time.Time + mu sync.Mutex +} + +// NewMapCache returns a new MapCache. +func NewMapCache() *MapCache { + return &MapCache{ + m: make(map[string]string), + ttl: make(map[string]time.Time), + now: time.Now, + } +} + +// Get returns the value for the given key, if cached. +func (c *MapCache) Get(key string) (string, error) { + c.mu.Lock() + defer c.mu.Unlock() + + if c.isExpired(key) { + c.delete(key) + + return "", nil + } + + if v, ok := c.m[key]; ok { + return v, nil + } + + return "", nil +} + +// Set sets the value for the given key. +func (c *MapCache) Set(key, value string, ttl time.Duration) error { + c.mu.Lock() + defer c.mu.Unlock() + + c.m[key] = value + c.ttl[key] = c.now().Add(ttl) + + return nil +} + +func (c *MapCache) isExpired(key string) bool { + t, ok := c.ttl[key] + if !ok { + return false + } + + return c.now().After(t) +} + +func (c *MapCache) delete(key string) { + delete(c.m, key) + delete(c.ttl, key) +} diff --git a/xload/providers/cached/cache_test.go b/xload/providers/cached/cache_test.go new file mode 100644 index 0000000..42f20ed --- /dev/null +++ b/xload/providers/cached/cache_test.go @@ -0,0 +1,43 @@ +package cached + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestMapCache(t *testing.T) { + cache := NewMapCache() + + err := cache.Set("foo", "bar", 1*time.Minute) + assert.NoError(t, err) + + v, err := cache.Get("foo") + assert.NoError(t, err) + assert.Equal(t, "bar", v) + + v, err = cache.Get("not-found") + assert.NoError(t, err) + assert.Equal(t, "", v) +} + +func TestMapCacheExpired(t *testing.T) { + cache := NewMapCache() + now := time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC) + + cache.now = func() time.Time { + return now + } + + err := cache.Set("foo", "bar", 1*time.Minute) + assert.NoError(t, err) + + cache.now = func() time.Time { + return now.Add(2 * time.Minute) + } + + v, err := cache.Get("foo") + assert.NoError(t, err) + assert.Equal(t, "", v) +} diff --git a/xload/providers/cached/go.mod b/xload/providers/cached/go.mod new file mode 100644 index 0000000..ab68a10 --- /dev/null +++ b/xload/providers/cached/go.mod @@ -0,0 +1,20 @@ +module github.com/gojekfarm/xtools/xload/providers/cached + +go 1.20 + +replace github.com/gojekfarm/xtools/xload => ../.. + +require ( + github.com/gojekfarm/xtools/xload v0.0.0-00010101000000-000000000000 + github.com/stretchr/testify v1.8.4 +) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/sourcegraph/conc v0.3.0 // indirect + github.com/spf13/cast v1.5.1 // indirect + go.uber.org/atomic v1.7.0 // indirect + go.uber.org/multierr v1.9.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/xload/providers/cached/go.sum b/xload/providers/cached/go.sum new file mode 100644 index 0000000..8ada964 --- /dev/null +++ b/xload/providers/cached/go.sum @@ -0,0 +1,27 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/frankban/quicktest v1.14.4 h1:g2rn0vABPOOXmZUj+vbmUp0lPoXEMuhTpIluN0XL9UY= +github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= +github.com/gotidy/ptr v1.4.0 h1:7++suUs+HNHMnyz6/AW3SE+4EnBhupPSQTSI7QNijVc= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= +github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo= +github.com/sourcegraph/conc v0.3.0/go.mod h1:Sdozi7LEKbFPqYX2/J+iBAM6HpqSLTASQIKqDmF7Mt0= +github.com/spf13/cast v1.5.1 h1:R+kOtfhWQE6TVQzY+4D7wJLBgkdVasCEFxSUBYBYIlA= +github.com/spf13/cast v1.5.1/go.mod h1:b9PdjNptOpzXr7Rq1q9gJML/2cdGQAo69NKzQ10KN48= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw= +go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= +go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI= +go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/xload/providers/cached/loader.go b/xload/providers/cached/loader.go new file mode 100644 index 0000000..55f0aef --- /dev/null +++ b/xload/providers/cached/loader.go @@ -0,0 +1,34 @@ +package cached + +import ( + "context" + + "github.com/gojekfarm/xtools/xload" +) + +// NewLoader returns a new cached loader. +func NewLoader(l xload.Loader, opts ...Option) xload.LoaderFunc { + o := defaultOptions() + + o.apply(opts...) + + return xload.LoaderFunc(func(ctx context.Context, key string) (string, error) { + v, err := o.cache.Get(key) + if err != nil { + return "", err + } + + if v != "" { + return v, nil + } + + loaded, err := l.Load(ctx, key) + if err != nil { + return "", err + } + + err = o.cache.Set(key, loaded, o.ttl) + + return loaded, err + }) +} diff --git a/xload/providers/cached/loader_test.go b/xload/providers/cached/loader_test.go new file mode 100644 index 0000000..0ff3914 --- /dev/null +++ b/xload/providers/cached/loader_test.go @@ -0,0 +1,9 @@ +package cached + +import ( + "testing" +) + +func TestNewLoader(t *testing.T) { + +} diff --git a/xload/providers/cached/options.go b/xload/providers/cached/options.go new file mode 100644 index 0000000..c2638a4 --- /dev/null +++ b/xload/providers/cached/options.go @@ -0,0 +1,46 @@ +package cached + +import "time" + +// Option configures a cached loader. +type Option interface { + apply(*options) +} + +type optionFunc func(*options) + +func (f optionFunc) apply(opts *options) { f(opts) } + +// TTL sets the TTL for the cached keys. +type TTL time.Duration + +func (t TTL) apply(o *options) { o.ttl = time.Duration(t) } + +// Cache sets the cache implementation for the loader. +func Cache(c Cacher) Option { + return optionFunc(func(o *options) { o.cache = c }) +} + +type options struct { + ttl time.Duration + cache Cacher +} + +func defaultOptions() *options { + return &options{ + ttl: 5 * time.Minute, + } +} + +func (o *options) apply(opts ...Option) { + for _, opt := range opts { + opt.apply(o) + } + + // DESIGN: If no cache is provided, use a simple map cache. + // This is after applying the options to avoid unnecessary + // allocations for the default cache. + if o.cache == nil { + o.cache = NewMapCache() + } +} From 78e3365b7e5f2a8cf40b342827b0b89ab714750e Mon Sep 17 00:00:00 2001 From: Ravi Atluri Date: Fri, 1 Sep 2023 09:44:53 +0530 Subject: [PATCH 2/6] Added examples & tests --- xload/providers/cached/cache.go | 2 + xload/providers/cached/doc.go | 6 ++ xload/providers/cached/example_test.go | 82 ++++++++++++++++++ xload/providers/cached/go.mod | 1 + xload/providers/cached/go.sum | 6 ++ xload/providers/cached/loader.go | 3 + xload/providers/cached/loader_test.go | 110 +++++++++++++++++++++++++ xload/providers/cached/mock_test.go | 67 +++++++++++++++ xproto/pb_test.go | 2 +- 9 files changed, 278 insertions(+), 1 deletion(-) create mode 100644 xload/providers/cached/doc.go create mode 100644 xload/providers/cached/example_test.go create mode 100644 xload/providers/cached/mock_test.go diff --git a/xload/providers/cached/cache.go b/xload/providers/cached/cache.go index 12c0551..d6702f6 100644 --- a/xload/providers/cached/cache.go +++ b/xload/providers/cached/cache.go @@ -6,6 +6,8 @@ import ( ) // Cacher is the interface for custom cache implementations. +// +//go:generate mockery --name Cacher --structname MockCache --filename mock_test.go --outpkg cached --output . type Cacher interface { Get(key string) (string, error) Set(key, value string, ttl time.Duration) error diff --git a/xload/providers/cached/doc.go b/xload/providers/cached/doc.go new file mode 100644 index 0000000..9ced968 --- /dev/null +++ b/xload/providers/cached/doc.go @@ -0,0 +1,6 @@ +// Package cached provides a cached provider for xload. +// This loader can be used to cache the results of any loader. +// +// Useful for loaders that have a high latency, or loaders that are +// called frequently. +package cached diff --git a/xload/providers/cached/example_test.go b/xload/providers/cached/example_test.go new file mode 100644 index 0000000..c5e320d --- /dev/null +++ b/xload/providers/cached/example_test.go @@ -0,0 +1,82 @@ +package cached_test + +import ( + "context" + "time" + + "github.com/gojekfarm/xtools/xload" + "github.com/gojekfarm/xtools/xload/providers/cached" +) + +func Example() { + // This example shows how to use the cached loader + // with a remote loader. + + ctx := context.Background() + cfg := struct { + Title string `env:"TITLE"` + Link string `env:"LINK"` + ButtonLabel string `env:"BUTTON_LABEL"` + }{} + + remoteLoader := xload.LoaderFunc(func(ctx context.Context, key string) (string, error) { + // Load the value from a remote source. + + return "", nil + }) + + err := xload.Load( + ctx, &cfg, + cached.NewLoader( + remoteLoader, + cached.TTL(5*60*time.Minute), + ), + ) + if err != nil { + panic(err) + } +} + +type CustomCache struct{} + +func NewCustomCache() *CustomCache { + return &CustomCache{} +} + +func (c *CustomCache) Get(key string) (string, error) { + return "", nil +} + +func (c *CustomCache) Set(key, value string, ttl time.Duration) error { + return nil +} + +func Example_customCache() { + // This example shows how to use a custom cache + // with the cached loader. + + ctx := context.Background() + cfg := struct { + Title string `env:"TITLE"` + Link string `env:"LINK"` + ButtonLabel string `env:"BUTTON_LABEL"` + }{} + + remoteLoader := xload.LoaderFunc(func(ctx context.Context, key string) (string, error) { + // Load the value from a remote source. + + return "", nil + }) + + err := xload.Load( + ctx, &cfg, + cached.NewLoader( + remoteLoader, + cached.TTL(5*60*time.Minute), + cached.Cache(NewCustomCache()), + ), + ) + if err != nil { + panic(err) + } +} diff --git a/xload/providers/cached/go.mod b/xload/providers/cached/go.mod index ab68a10..c6f5fd5 100644 --- a/xload/providers/cached/go.mod +++ b/xload/providers/cached/go.mod @@ -14,6 +14,7 @@ require ( github.com/pmezard/go-difflib v1.0.0 // indirect github.com/sourcegraph/conc v0.3.0 // indirect github.com/spf13/cast v1.5.1 // indirect + github.com/stretchr/objx v0.5.0 // indirect go.uber.org/atomic v1.7.0 // indirect go.uber.org/multierr v1.9.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/xload/providers/cached/go.sum b/xload/providers/cached/go.sum index 8ada964..d7bed1f 100644 --- a/xload/providers/cached/go.sum +++ b/xload/providers/cached/go.sum @@ -14,7 +14,12 @@ github.com/sourcegraph/conc v0.3.0/go.mod h1:Sdozi7LEKbFPqYX2/J+iBAM6HpqSLTASQIK github.com/spf13/cast v1.5.1 h1:R+kOtfhWQE6TVQzY+4D7wJLBgkdVasCEFxSUBYBYIlA= github.com/spf13/cast v1.5.1/go.mod h1:b9PdjNptOpzXr7Rq1q9gJML/2cdGQAo69NKzQ10KN48= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw= @@ -23,5 +28,6 @@ go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI= go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/xload/providers/cached/loader.go b/xload/providers/cached/loader.go index 55f0aef..1522a19 100644 --- a/xload/providers/cached/loader.go +++ b/xload/providers/cached/loader.go @@ -27,6 +27,9 @@ func NewLoader(l xload.Loader, opts ...Option) xload.LoaderFunc { return "", err } + // DESIGN: If the loader returns an empty value, we + // consider it a cache HIT and cache the empty value. + err = o.cache.Set(key, loaded, o.ttl) return loaded, err diff --git a/xload/providers/cached/loader_test.go b/xload/providers/cached/loader_test.go index 0ff3914..ad02000 100644 --- a/xload/providers/cached/loader_test.go +++ b/xload/providers/cached/loader_test.go @@ -1,9 +1,119 @@ package cached import ( + "context" "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "github.com/gojekfarm/xtools/xload" ) +type config struct { + Key1 string `env:"KEY_1"` + Key2 string `env:"KEY_2"` + Key3 string `env:"KEY_3"` +} + func TestNewLoader(t *testing.T) { + loader := xload.MapLoader(map[string]string{ + "KEY_1": "value-1", + "KEY_2": "value-2", + }) + + cachedLoader := NewLoader(loader) + + t.Run("Cache MISS", func(t *testing.T) { + cfg := config{} + + err := xload.Load(context.TODO(), &cfg, cachedLoader) + assert.NoError(t, err) + assert.Equal(t, "value-1", cfg.Key1) + assert.Equal(t, "value-2", cfg.Key2) + }) + + t.Run("Cache HIT", func(t *testing.T) { + cfg := config{} + + err := xload.Load(context.TODO(), &cfg, cachedLoader) + assert.NoError(t, err) + assert.Equal(t, "value-1", cfg.Key1) + assert.Equal(t, "value-2", cfg.Key2) + }) +} + +func TestNewLoader_WithTTL(t *testing.T) { + loader := xload.MapLoader(map[string]string{ + "KEY_1": "value-1", + "KEY_2": "value-2", + }) + + ttl := 123 * time.Second + + mc := NewMockCache(t) + mc.On("Get", mock.Anything).Return("", nil).Times(3) + mc.On("Set", mock.Anything, mock.Anything, ttl).Return(nil).Times(3) + + cachedLoader := NewLoader(loader, TTL(ttl), Cache(mc)) + + cfg := config{} + + err := xload.Load(context.Background(), &cfg, cachedLoader) + assert.NoError(t, err) + + mc.AssertExpectations(t) +} + +func TestNewLoader_ForwardError(t *testing.T) { + failingLoader := xload.LoaderFunc(func(ctx context.Context, key string) (string, error) { + return "", assert.AnError + }) + + cachedLoader := NewLoader(failingLoader) + + cfg := config{} + + err := xload.Load(context.Background(), &cfg, cachedLoader) + assert.Error(t, err) + assert.Equal(t, assert.AnError, err) +} + +func TestNewLoader_CacheError(t *testing.T) { + loader := xload.MapLoader(map[string]string{ + "KEY_1": "value-1", + "KEY_2": "value-2", + }) + + cfg := config{} + + t.Run("Cache SET error", func(t *testing.T) { + mc := NewMockCache(t) + + mc.On("Get", "KEY_1").Return("", nil) + mc.On("Set", "KEY_1", "value-1", mock.Anything).Return(assert.AnError) + + cachedLoader := NewLoader(loader, Cache(mc)) + + err := xload.Load(context.Background(), &cfg, cachedLoader) + assert.Error(t, err) + assert.Equal(t, assert.AnError, err) + + mc.AssertExpectations(t) + }) + + t.Run("Cache GET error", func(t *testing.T) { + mc := NewMockCache(t) + + mc.On("Get", "KEY_1").Return("", assert.AnError) + + cachedLoader := NewLoader(loader, Cache(mc)) + + err := xload.Load(context.Background(), &cfg, cachedLoader) + assert.Error(t, err) + assert.Equal(t, assert.AnError, err) + mc.AssertExpectations(t) + }) } diff --git a/xload/providers/cached/mock_test.go b/xload/providers/cached/mock_test.go new file mode 100644 index 0000000..259be46 --- /dev/null +++ b/xload/providers/cached/mock_test.go @@ -0,0 +1,67 @@ +// Code generated by mockery v2.20.0. DO NOT EDIT. + +package cached + +import ( + time "time" + + mock "github.com/stretchr/testify/mock" +) + +// MockCache is an autogenerated mock type for the Cacher type +type MockCache struct { + mock.Mock +} + +// Get provides a mock function with given fields: key +func (_m *MockCache) Get(key string) (string, error) { + ret := _m.Called(key) + + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func(string) (string, error)); ok { + return rf(key) + } + if rf, ok := ret.Get(0).(func(string) string); ok { + r0 = rf(key) + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(key) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Set provides a mock function with given fields: key, value, ttl +func (_m *MockCache) Set(key string, value string, ttl time.Duration) error { + ret := _m.Called(key, value, ttl) + + var r0 error + if rf, ok := ret.Get(0).(func(string, string, time.Duration) error); ok { + r0 = rf(key, value, ttl) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +type mockConstructorTestingTNewMockCache interface { + mock.TestingT + Cleanup(func()) +} + +// NewMockCache creates a new instance of MockCache. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewMockCache(t mockConstructorTestingTNewMockCache) *MockCache { + mock := &MockCache{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/xproto/pb_test.go b/xproto/pb_test.go index 73555e6..3f326fc 100644 --- a/xproto/pb_test.go +++ b/xproto/pb_test.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: // protoc-gen-go v1.28.1 -// protoc v3.21.12 +// protoc v4.23.4 // source: test.proto package xproto From 6b4aedd39493d89dc954e96154cbc87d7e5ad27c Mon Sep 17 00:00:00 2001 From: Ravi Atluri Date: Mon, 4 Sep 2023 10:38:03 +0530 Subject: [PATCH 3/6] Added DisableEmptyValueHit option --- xload/providers/cached/example_test.go | 31 ++++++++++++++++++++++++++ xload/providers/cached/loader.go | 10 +++++++-- xload/providers/cached/loader_test.go | 26 +++++++++++++++++++++ xload/providers/cached/options.go | 13 ++++++++--- 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/xload/providers/cached/example_test.go b/xload/providers/cached/example_test.go index c5e320d..9bb84b0 100644 --- a/xload/providers/cached/example_test.go +++ b/xload/providers/cached/example_test.go @@ -80,3 +80,34 @@ func Example_customCache() { panic(err) } } + +func Example_disableEmptyValueHit() { + // By default, the cached loader caches empty values. + // This example shows how to disable caching of empty values + // with the cached loader. + + ctx := context.Background() + cfg := struct { + Title string `env:"TITLE"` + Link string `env:"LINK"` + ButtonLabel string `env:"BUTTON_LABEL"` + }{} + + remoteLoader := xload.LoaderFunc(func(ctx context.Context, key string) (string, error) { + // Load the value from a remote source. + + return "", nil + }) + + err := xload.Load( + ctx, &cfg, + cached.NewLoader( + remoteLoader, + cached.TTL(5*60*time.Minute), + cached.DisableEmptyValueHit(), + ), + ) + if err != nil { + panic(err) + } +} diff --git a/xload/providers/cached/loader.go b/xload/providers/cached/loader.go index 1522a19..a72a373 100644 --- a/xload/providers/cached/loader.go +++ b/xload/providers/cached/loader.go @@ -7,6 +7,11 @@ import ( ) // NewLoader returns a new cached loader. +// +// The cached loader uses these defaults: +// - TTL: 5 minutes. Configurable via `TTL` option. +// - Cache: A simple unbounded map cache. Configurable via `Cache` option. +// - Empty value hit: Enabled. Configurable via `DisableEmptyValueHit` option. func NewLoader(l xload.Loader, opts ...Option) xload.LoaderFunc { o := defaultOptions() @@ -27,8 +32,9 @@ func NewLoader(l xload.Loader, opts ...Option) xload.LoaderFunc { return "", err } - // DESIGN: If the loader returns an empty value, we - // consider it a cache HIT and cache the empty value. + if loaded == "" && !o.emptyValueHit { + return "", nil + } err = o.cache.Set(key, loaded, o.ttl) diff --git a/xload/providers/cached/loader_test.go b/xload/providers/cached/loader_test.go index ad02000..bbda240 100644 --- a/xload/providers/cached/loader_test.go +++ b/xload/providers/cached/loader_test.go @@ -66,6 +66,32 @@ func TestNewLoader_WithTTL(t *testing.T) { mc.AssertExpectations(t) } +func TestNewLoader_WithDisableEmptyValueHit(t *testing.T) { + loader := xload.MapLoader(map[string]string{ + "KEY_1": "value-1", + "KEY_2": "value-2", + }) + + ttl := 123 * time.Second + + mc := NewMockCache(t) + mc.On("Get", mock.Anything).Return("", nil).Times(6) + mc.On("Set", mock.Anything, mock.Anything, ttl).Return(nil).Times(4) + + cachedLoader := NewLoader(loader, TTL(ttl), Cache(mc), DisableEmptyValueHit()) + + cfg := config{} + + err := xload.Load(context.Background(), &cfg, cachedLoader) + assert.NoError(t, err) + + // load again to ensure that the empty value is not cached + err = xload.Load(context.Background(), &cfg, cachedLoader) + assert.NoError(t, err) + + mc.AssertExpectations(t) +} + func TestNewLoader_ForwardError(t *testing.T) { failingLoader := xload.LoaderFunc(func(ctx context.Context, key string) (string, error) { return "", assert.AnError diff --git a/xload/providers/cached/options.go b/xload/providers/cached/options.go index c2638a4..a041cf5 100644 --- a/xload/providers/cached/options.go +++ b/xload/providers/cached/options.go @@ -21,14 +21,21 @@ func Cache(c Cacher) Option { return optionFunc(func(o *options) { o.cache = c }) } +// DisableEmptyValueHit disables caching of empty values. +func DisableEmptyValueHit() Option { + return optionFunc(func(o *options) { o.emptyValueHit = false }) +} + type options struct { - ttl time.Duration - cache Cacher + ttl time.Duration + cache Cacher + emptyValueHit bool } func defaultOptions() *options { return &options{ - ttl: 5 * time.Minute, + ttl: 5 * time.Minute, + emptyValueHit: true, } } From 914739207c99be7680f87ecaeb1e58dc6f96e844 Mon Sep 17 00:00:00 2001 From: Ravi Atluri Date: Mon, 4 Sep 2023 10:49:28 +0530 Subject: [PATCH 4/6] Used sync.Map for cache --- xload/providers/cached/cache.go | 49 ++++++++++++++------------------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/xload/providers/cached/cache.go b/xload/providers/cached/cache.go index d6702f6..e5a0434 100644 --- a/xload/providers/cached/cache.go +++ b/xload/providers/cached/cache.go @@ -13,63 +13,56 @@ type Cacher interface { Set(key, value string, ttl time.Duration) error } +type mv struct { + val string + ttl time.Time +} + // MapCache is a simple cache implementation using a map. type MapCache struct { - m map[string]string - ttl map[string]time.Time + m sync.Map now func() time.Time - mu sync.Mutex } // NewMapCache returns a new MapCache. func NewMapCache() *MapCache { return &MapCache{ - m: make(map[string]string), - ttl: make(map[string]time.Time), + m: sync.Map{}, now: time.Now, } } // Get returns the value for the given key, if cached. func (c *MapCache) Get(key string) (string, error) { - c.mu.Lock() - defer c.mu.Unlock() + v, ok := c.m.Load(key) + if !ok { + return "", nil + } + + mv, ok := v.(*mv) - if c.isExpired(key) { + if !ok || c.now().After(mv.ttl) { c.delete(key) return "", nil } - if v, ok := c.m[key]; ok { - return v, nil - } - - return "", nil + return mv.val, nil } // Set sets the value for the given key. func (c *MapCache) Set(key, value string, ttl time.Duration) error { - c.mu.Lock() - defer c.mu.Unlock() + v := &mv{ + val: value, + ttl: c.now().Add(ttl), + } - c.m[key] = value - c.ttl[key] = c.now().Add(ttl) + c.m.Store(key, v) return nil } -func (c *MapCache) isExpired(key string) bool { - t, ok := c.ttl[key] - if !ok { - return false - } - - return c.now().After(t) -} - func (c *MapCache) delete(key string) { - delete(c.m, key) - delete(c.ttl, key) + c.m.Delete(key) } From 44c231f472cc395059ad72ab1a8e11ee1dcf3f9a Mon Sep 17 00:00:00 2001 From: Ravi Atluri Date: Mon, 4 Sep 2023 11:42:21 +0530 Subject: [PATCH 5/6] Refactored DisableEmptyValueHit --- xload/providers/cached/example_test.go | 2 +- xload/providers/cached/loader.go | 2 +- xload/providers/cached/loader_test.go | 2 +- xload/providers/cached/options.go | 18 ++++++++++-------- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/xload/providers/cached/example_test.go b/xload/providers/cached/example_test.go index 9bb84b0..169e594 100644 --- a/xload/providers/cached/example_test.go +++ b/xload/providers/cached/example_test.go @@ -104,7 +104,7 @@ func Example_disableEmptyValueHit() { cached.NewLoader( remoteLoader, cached.TTL(5*60*time.Minute), - cached.DisableEmptyValueHit(), + cached.DisableEmptyValueHit, ), ) if err != nil { diff --git a/xload/providers/cached/loader.go b/xload/providers/cached/loader.go index a72a373..369aab6 100644 --- a/xload/providers/cached/loader.go +++ b/xload/providers/cached/loader.go @@ -32,7 +32,7 @@ func NewLoader(l xload.Loader, opts ...Option) xload.LoaderFunc { return "", err } - if loaded == "" && !o.emptyValueHit { + if loaded == "" && !o.emptyHit { return "", nil } diff --git a/xload/providers/cached/loader_test.go b/xload/providers/cached/loader_test.go index bbda240..9783872 100644 --- a/xload/providers/cached/loader_test.go +++ b/xload/providers/cached/loader_test.go @@ -78,7 +78,7 @@ func TestNewLoader_WithDisableEmptyValueHit(t *testing.T) { mc.On("Get", mock.Anything).Return("", nil).Times(6) mc.On("Set", mock.Anything, mock.Anything, ttl).Return(nil).Times(4) - cachedLoader := NewLoader(loader, TTL(ttl), Cache(mc), DisableEmptyValueHit()) + cachedLoader := NewLoader(loader, TTL(ttl), Cache(mc), DisableEmptyValueHit) cfg := config{} diff --git a/xload/providers/cached/options.go b/xload/providers/cached/options.go index a041cf5..dd62054 100644 --- a/xload/providers/cached/options.go +++ b/xload/providers/cached/options.go @@ -22,20 +22,22 @@ func Cache(c Cacher) Option { } // DisableEmptyValueHit disables caching of empty values. -func DisableEmptyValueHit() Option { - return optionFunc(func(o *options) { o.emptyValueHit = false }) -} +var DisableEmptyValueHit = emptyValHit(false) + +type emptyValHit bool + +func (e emptyValHit) apply(o *options) { o.emptyHit = bool(e) } type options struct { - ttl time.Duration - cache Cacher - emptyValueHit bool + ttl time.Duration + cache Cacher + emptyHit bool } func defaultOptions() *options { return &options{ - ttl: 5 * time.Minute, - emptyValueHit: true, + ttl: 5 * time.Minute, + emptyHit: true, } } From 4f2570f9bc2886969c774c503fd8c4c9bde48861 Mon Sep 17 00:00:00 2001 From: ajatprabha Date: Mon, 4 Sep 2023 12:13:48 +0530 Subject: [PATCH 6/6] change type of DisableEmptyValueHit --- xload/providers/cached/options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xload/providers/cached/options.go b/xload/providers/cached/options.go index dd62054..d09c4e8 100644 --- a/xload/providers/cached/options.go +++ b/xload/providers/cached/options.go @@ -22,7 +22,7 @@ func Cache(c Cacher) Option { } // DisableEmptyValueHit disables caching of empty values. -var DisableEmptyValueHit = emptyValHit(false) +var DisableEmptyValueHit Option = emptyValHit(false) type emptyValHit bool