Introduce API Resolver to make it easier to use caching

9 files changed, 65 insertions, 39 deletions

diff --git a/internal/source/gitlab/api/client.go b/internal/source/gitlab/api/client.go
index 5d575cc6..7206e25a 100644
--- a/internal/source/gitlab/api/client.go
+++ b/internal/source/gitlab/api/client.go
@@ -7,5 +7,5 @@ import (
 // Client represents an interface we use to retrieve information from GitLab
 type Client interface {
 	// GetLookup retrives an VirtualDomain from GitLab API and wraps it into Lookup
-	GetLookup(ctx context.Context, domain string) *Lookup
+	GetLookup(ctx context.Context, domain string) Lookup
 }
diff --git a/internal/source/gitlab/api/resolver.go b/internal/source/gitlab/api/resolver.go
new file mode 100644
index 00000000..061a1ddd
--- /dev/null
+++ b/internal/source/gitlab/api/resolver.go
@@ -0,0 +1,12 @@
+package api
+
+import (
+	"context"
+)
+
+// Resolver represents an interface we use to retrieve information from GitLab
+// in a more generic way. It can be a concrete API client or cached client.
+type Resolver interface {
+	// Resolve retrives an VirtualDomain from GitLab API and wraps it into Lookup
+	Resolve(ctx context.Context, domain string) *Lookup
+}
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)
diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go
index e66ef51c..f5ecf355 100644
--- a/internal/source/gitlab/client/client.go
+++ b/internal/source/gitlab/client/client.go
@@ -55,25 +55,33 @@ func NewFromConfig(config Config) (*Client, error) {
 	return NewClient(config.GitlabServerURL(), config.GitlabAPISecret())
 }
 
-// GetLookup returns a VirtualDomain configuration wrap into a Lookup for a
+// Resolve returns a VirtualDomain configuration wrapped into a Lookup for a
+// given host. It implements api.Resolve type.
+func (gc *Client) Resolve(ctx context.Context, host string) *api.Lookup {
+	lookup := gc.GetLookup(ctx, host)
+
+	return &lookup
+}
+
+// GetLookup returns a VirtualDomain configuration wrapped into a Lookup for a
 // given host
-func (gc *Client) GetLookup(ctx context.Context, host string) *api.Lookup {
+func (gc *Client) GetLookup(ctx context.Context, host string) api.Lookup {
 	params := url.Values{}
 	params.Set("host", host)
 
 	resp, err := gc.get(ctx, "/api/v4/internal/pages", params)
 	if err != nil {
-		return &api.Lookup{Name: host, Error: err}
+		return api.Lookup{Name: host, Error: err}
 	}
 
 	if resp == nil {
-		return &api.Lookup{Name: host}
+		return api.Lookup{Name: host}
 	}
 
 	lookup := api.Lookup{Name: host}
 	lookup.Error = json.NewDecoder(resp.Body).Decode(&lookup.Domain)
 
-	return &lookup
+	return lookup
 }
 
 func (gc *Client) get(ctx context.Context, path string, params url.Values) (*http.Response, error) {
diff --git a/internal/source/gitlab/client/client_stub.go b/internal/source/gitlab/client/client_stub.go
index 9c8cf19a..604f127b 100644
--- a/internal/source/gitlab/client/client_stub.go
+++ b/internal/source/gitlab/client/client_stub.go
@@ -13,18 +13,25 @@ type StubClient struct {
 	File string
 }
 
+// Resolve implements api.Resolver
+func (c StubClient) Resolve(ctx context.Context, host string) *api.Lookup {
+	lookup := c.GetLookup(ctx, host)
+
+	return &lookup
+}
+
 // GetLookup reads a test fixture and unmarshalls it
-func (c StubClient) GetLookup(ctx context.Context, host string) *api.Lookup {
+func (c StubClient) GetLookup(ctx context.Context, host string) api.Lookup {
 	lookup := api.Lookup{Name: host}
 
 	f, err := os.Open(c.File)
 	if err != nil {
 		lookup.Error = err
-		return &lookup
+		return lookup
 	}
 	defer f.Close()
 
 	lookup.Error = json.NewDecoder(f).Decode(&lookup.Domain)
 
-	return &lookup
+	return lookup
 }
diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go
index 18798b4f..5bb5427e 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -18,7 +18,7 @@ import (
 // Gitlab source represent a new domains configuration source. We fetch all the
 // information about domains from GitLab instance.
 type Gitlab struct {
-	client api.Client
+	client api.Resolver
 }
 
 // New returns a new instance of gitlab domain source.
@@ -34,7 +34,7 @@ func New(config client.Config) (*Gitlab, error) {
 // GetDomain return a representation of a domain that we have fetched from
 // GitLab
 func (g *Gitlab) GetDomain(name string) (*domain.Domain, error) {
-	lookup := g.client.GetLookup(context.Background(), name)
+	lookup := g.client.Resolve(context.Background(), name)
 
 	if lookup.Error != nil {
 		return nil, lookup.Error
@@ -59,7 +59,8 @@ func (g *Gitlab) GetDomain(name string) (*domain.Domain, error) {
 // the GitLab source
 func (g *Gitlab) Resolve(r *http.Request) (*serving.LookupPath, string, error) {
 	host := request.GetHostWithoutPort(r)
-	response := g.client.GetLookup(r.Context(), host)
+
+	response := g.client.Resolve(r.Context(), host)
 	if response.Error != nil {
 		return nil, "", response.Error
 	}