diff options
author | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2019-12-14 16:28:58 +0300 |
---|---|---|
committer | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2019-12-14 16:31:32 +0300 |
commit | e3c8677987d96d95bd4e012761b3367c0e173351 (patch) | |
tree | 06a4fdd9d3c6ac0039c13d8849527205ce41e2ed /internal/source/gitlab/cache | |
parent | 3ae2f4c7bb3e28f213ee54d33dbf6fd370277996 (diff) |
Introduce API Resolver to make it easier to use caching
Diffstat (limited to 'internal/source/gitlab/cache')
-rw-r--r-- | internal/source/gitlab/cache/cache.go | 4 | ||||
-rw-r--r-- | internal/source/gitlab/cache/cache_test.go | 34 | ||||
-rw-r--r-- | internal/source/gitlab/cache/entry.go | 4 | ||||
-rw-r--r-- | internal/source/gitlab/cache/retriever.go | 10 |
4 files changed, 25 insertions, 27 deletions
diff --git a/internal/source/gitlab/cache/cache.go b/internal/source/gitlab/cache/cache.go index 7bf755a3..492a52a9 100644 --- a/internal/source/gitlab/cache/cache.go +++ b/internal/source/gitlab/cache/cache.go @@ -20,7 +20,7 @@ func NewCache(client api.Client) *Cache { } } -// GetLookup is going to return a lookup based on a domain name. The caching +// Resolve is going to return a lookup based on a domain name. The caching // algorithm works as follows: // - We first check if the cache entry exists, and if it is up-to-date. If it // is fresh we return the lookup entry from cache and it is a cache hit. @@ -69,7 +69,7 @@ func NewCache(client api.Client) *Cache { // - we create a lookup that contains information about an error // - we cache this response // - we pass this lookup upstream to all the clients -func (c *Cache) GetLookup(ctx context.Context, domain string) *api.Lookup { +func (c *Cache) Resolve(ctx context.Context, domain string) *api.Lookup { entry := c.store.LoadOrCreate(domain) if entry.IsUpToDate() { diff --git a/internal/source/gitlab/cache/cache_test.go b/internal/source/gitlab/cache/cache_test.go index 27c11b6d..39f8cb73 100644 --- a/internal/source/gitlab/cache/cache_test.go +++ b/internal/source/gitlab/cache/cache_test.go @@ -21,9 +21,7 @@ type client struct { failure error } -func (c *client) GetLookup(ctx context.Context, _ string) *api.Lookup { - var lookup api.Lookup - +func (c *client) GetLookup(ctx context.Context, _ string) (lookup api.Lookup) { // TODO This might not work on some architectures // // https://golang.org/pkg/sync/atomic/#pkg-note-BUG @@ -42,7 +40,7 @@ func (c *client) GetLookup(ctx context.Context, _ string) *api.Lookup { lookup.Error = c.failure } - return &lookup + return lookup } func withTestCache(config resolverConfig, block func(*Cache, *client)) { @@ -75,7 +73,7 @@ func (cache *Cache) withTestEntry(config entryConfig, block func(*Entry)) { entry := cache.store.LoadOrCreate(domain) if config.retrieved { - entry.setResponse(&api.Lookup{Name: domain}) + entry.setResponse(api.Lookup{Name: domain}) } if config.expired { @@ -96,12 +94,12 @@ type entryConfig struct { retrieved bool } -func TestGetLookup(t *testing.T) { +func TestResolve(t *testing.T) { t.Run("when item is not cached", func(t *testing.T) { withTestCache(resolverConfig{buffered: true}, func(cache *Cache, resolver *client) { resolver.domain <- "my.gitlab.com" - lookup := cache.GetLookup(context.Background(), "my.gitlab.com") + lookup := cache.Resolve(context.Background(), "my.gitlab.com") require.NoError(t, lookup.Error) require.Equal(t, "my.gitlab.com", lookup.Name) @@ -116,7 +114,7 @@ func TestGetLookup(t *testing.T) { receiver := func() { defer wg.Done() - cache.GetLookup(ctx, "my.gitlab.com") + cache.Resolve(ctx, "my.gitlab.com") } wg.Add(3) @@ -136,7 +134,7 @@ func TestGetLookup(t *testing.T) { t.Run("when item is in short cache", func(t *testing.T) { withTestCache(resolverConfig{}, func(cache *Cache, resolver *client) { cache.withTestEntry(entryConfig{expired: false, retrieved: true}, func(*Entry) { - lookup := cache.GetLookup(context.Background(), "my.gitlab.com") + lookup := cache.Resolve(context.Background(), "my.gitlab.com") require.Equal(t, "my.gitlab.com", lookup.Name) require.Equal(t, uint64(0), resolver.lookups) @@ -150,7 +148,7 @@ func TestGetLookup(t *testing.T) { lookup := make(chan *api.Lookup, 1) go func() { - lookup <- cache.GetLookup(context.Background(), "my.gitlab.com") + lookup <- cache.Resolve(context.Background(), "my.gitlab.com") }() <-resolver.bootup @@ -170,7 +168,7 @@ func TestGetLookup(t *testing.T) { t.Run("when item is in long cache only", func(t *testing.T) { withTestCache(resolverConfig{}, func(cache *Cache, resolver *client) { cache.withTestEntry(entryConfig{expired: true, retrieved: true}, func(*Entry) { - lookup := cache.GetLookup(context.Background(), "my.gitlab.com") + lookup := cache.Resolve(context.Background(), "my.gitlab.com") require.Equal(t, "my.gitlab.com", lookup.Name) require.Equal(t, uint64(0), resolver.lookups) @@ -184,9 +182,9 @@ func TestGetLookup(t *testing.T) { t.Run("when item in long cache is requested multiple times", func(t *testing.T) { withTestCache(resolverConfig{}, func(cache *Cache, resolver *client) { cache.withTestEntry(entryConfig{expired: true, retrieved: true}, func(*Entry) { - cache.GetLookup(context.Background(), "my.gitlab.com") - cache.GetLookup(context.Background(), "my.gitlab.com") - cache.GetLookup(context.Background(), "my.gitlab.com") + cache.Resolve(context.Background(), "my.gitlab.com") + cache.Resolve(context.Background(), "my.gitlab.com") + cache.Resolve(context.Background(), "my.gitlab.com") require.Equal(t, uint64(0), resolver.lookups) @@ -200,7 +198,7 @@ func TestGetLookup(t *testing.T) { withTestCache(resolverConfig{failure: errors.New("500 err")}, func(cache *Cache, resolver *client) { maxRetrievalInterval = 0 - lookup := cache.GetLookup(context.Background(), "my.gitlab.com") + lookup := cache.Resolve(context.Background(), "my.gitlab.com") require.Equal(t, uint64(3), resolver.lookups) require.EqualError(t, lookup.Error, "500 err") @@ -212,7 +210,7 @@ func TestGetLookup(t *testing.T) { cancel() withTestCache(resolverConfig{}, func(cache *Cache, resolver *client) { - lookup := cache.GetLookup(ctx, "my.gitlab.com") + lookup := cache.Resolve(ctx, "my.gitlab.com") require.Equal(t, uint64(0), resolver.lookups) require.EqualError(t, lookup.Error, "context done") @@ -224,7 +222,7 @@ func TestGetLookup(t *testing.T) { defer func() { retrievalTimeout = 5 * time.Second }() withTestCache(resolverConfig{}, func(cache *Cache, resolver *client) { - lookup := cache.GetLookup(context.Background(), "my.gitlab.com") + lookup := cache.Resolve(context.Background(), "my.gitlab.com") require.Equal(t, uint64(0), resolver.lookups) require.EqualError(t, lookup.Error, "retrieval context done") @@ -237,7 +235,7 @@ func TestGetLookup(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) response := make(chan *api.Lookup, 1) - go func() { response <- cache.GetLookup(ctx, "my.gitlab.com") }() + go func() { response <- cache.Resolve(ctx, "my.gitlab.com") }() cancel() diff --git a/internal/source/gitlab/cache/entry.go b/internal/source/gitlab/cache/entry.go index 0bf5c3a8..d91bb331 100644 --- a/internal/source/gitlab/cache/entry.go +++ b/internal/source/gitlab/cache/entry.go @@ -92,11 +92,11 @@ func (e *Entry) retrieveWithClient(client api.Client) { e.setResponse(retriever.Retrieve(e.domain)) } -func (e *Entry) setResponse(lookup *api.Lookup) { +func (e *Entry) setResponse(lookup api.Lookup) { e.mux.Lock() defer e.mux.Unlock() - e.response = lookup + e.response = &lookup close(e.retrieved) } diff --git a/internal/source/gitlab/cache/retriever.go b/internal/source/gitlab/cache/retriever.go index ca5421df..cf0541b1 100644 --- a/internal/source/gitlab/cache/retriever.go +++ b/internal/source/gitlab/cache/retriever.go @@ -18,14 +18,14 @@ type Retriever struct { // Retrieve retrieves a lookup response from external source with timeout and // backoff. It has its own context with timeout. -func (r *Retriever) Retrieve(domain string) (lookup *api.Lookup) { +func (r *Retriever) Retrieve(domain string) (lookup api.Lookup) { ctx, cancel := context.WithTimeout(context.Background(), retrievalTimeout) defer cancel() select { case <-ctx.Done(): log.Debug("retrieval context done") - lookup = &api.Lookup{Error: errors.New("retrieval context done")} + lookup = api.Lookup{Error: errors.New("retrieval context done")} case lookup = <-r.resolveWithBackoff(ctx, domain): log.Debug("retrieval response sent") } @@ -33,11 +33,11 @@ func (r *Retriever) Retrieve(domain string) (lookup *api.Lookup) { return lookup } -func (r *Retriever) resolveWithBackoff(ctx context.Context, domain string) <-chan *api.Lookup { - response := make(chan *api.Lookup) +func (r *Retriever) resolveWithBackoff(ctx context.Context, domain string) <-chan api.Lookup { + response := make(chan api.Lookup) go func() { - var lookup *api.Lookup + var lookup api.Lookup for i := 1; i <= maxRetrievalRetries; i++ { lookup = r.client.GetLookup(ctx, domain) |