diff options
author | Nick Thomas <nick@gitlab.com> | 2019-12-16 10:55:50 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2019-12-16 10:55:50 +0300 |
commit | 332f72f54c95bf44c8559b65473350516827b1c4 (patch) | |
tree | 06a4fdd9d3c6ac0039c13d8849527205ce41e2ed /internal/source | |
parent | 860072e9807e8ab8ce6b213f4f72f42d91c1ad70 (diff) | |
parent | e3c8677987d96d95bd4e012761b3367c0e173351 (diff) |
Merge branch 'feature/gb/enable-gitlab-source-caching' into 'master'
Add caching on top of gitlab domains source client
Closes #280
See merge request gitlab-org/gitlab-pages!210
Diffstat (limited to 'internal/source')
-rw-r--r-- | internal/source/gitlab/api/resolver.go | 12 | ||||
-rw-r--r-- | internal/source/gitlab/cache/cache.go | 7 | ||||
-rw-r--r-- | internal/source/gitlab/cache/cache_test.go | 4 | ||||
-rw-r--r-- | internal/source/gitlab/cache/retriever.go | 11 | ||||
-rw-r--r-- | internal/source/gitlab/client/client.go | 10 | ||||
-rw-r--r-- | internal/source/gitlab/client/client_stub.go | 7 | ||||
-rw-r--r-- | internal/source/gitlab/gitlab.go | 10 | ||||
-rw-r--r-- | internal/source/gitlab/gitlab_test.go | 7 |
8 files changed, 43 insertions, 25 deletions
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 85f44e11..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 { } } -// Resolve 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. @@ -84,8 +84,3 @@ func (c *Cache) Resolve(ctx context.Context, domain string) *api.Lookup { return entry.Retrieve(ctx, c.client) } - -// New creates a new instance of Cache and sets default expiration -func New() *Cache { - return &Cache{} -} diff --git a/internal/source/gitlab/cache/cache_test.go b/internal/source/gitlab/cache/cache_test.go index ac7c6726..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 diff --git a/internal/source/gitlab/cache/retriever.go b/internal/source/gitlab/cache/retriever.go index 1284b021..cf0541b1 100644 --- a/internal/source/gitlab/cache/retriever.go +++ b/internal/source/gitlab/cache/retriever.go @@ -3,9 +3,10 @@ package cache import ( "context" "errors" - "fmt" "time" + log "github.com/sirupsen/logrus" + "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api" ) @@ -17,18 +18,16 @@ 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) api.Lookup { +func (r *Retriever) Retrieve(domain string) (lookup api.Lookup) { ctx, cancel := context.WithTimeout(context.Background(), retrievalTimeout) defer cancel() - var lookup api.Lookup - select { case <-ctx.Done(): - fmt.Println("retrieval context done") // TODO logme + log.Debug("retrieval context done") lookup = api.Lookup{Error: errors.New("retrieval context done")} case lookup = <-r.resolveWithBackoff(ctx, domain): - fmt.Println("retrieval response sent") // TODO logme + log.Debug("retrieval response sent") } return lookup diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go index 9bf4bf32..f5ecf355 100644 --- a/internal/source/gitlab/client/client.go +++ b/internal/source/gitlab/client/client.go @@ -55,7 +55,15 @@ 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 { params := url.Values{} diff --git a/internal/source/gitlab/client/client_stub.go b/internal/source/gitlab/client/client_stub.go index 00a689d8..604f127b 100644 --- a/internal/source/gitlab/client/client_stub.go +++ b/internal/source/gitlab/client/client_stub.go @@ -13,6 +13,13 @@ 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 { lookup := api.Lookup{Name: host} diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go index 7977e35a..5bb5427e 100644 --- a/internal/source/gitlab/gitlab.go +++ b/internal/source/gitlab/gitlab.go @@ -18,8 +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 - cache *cache.Cache // WIP + client api.Resolver } // New returns a new instance of gitlab domain source. @@ -29,13 +28,13 @@ func New(config client.Config) (*Gitlab, error) { return nil, err } - return &Gitlab{client: client, cache: cache.New()}, nil + return &Gitlab{client: cache.NewCache(client)}, nil } // 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 @@ -60,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 } diff --git a/internal/source/gitlab/gitlab_test.go b/internal/source/gitlab/gitlab_test.go index ad5144f5..d6a81269 100644 --- a/internal/source/gitlab/gitlab_test.go +++ b/internal/source/gitlab/gitlab_test.go @@ -6,14 +6,13 @@ import ( "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/cache" "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/client" ) func TestGetDomain(t *testing.T) { t.Run("when the response if correct", func(t *testing.T) { client := client.StubClient{File: "client/testdata/test.gitlab.io.json"} - source := Gitlab{client: client, cache: cache.New()} + source := Gitlab{client: client} domain, err := source.GetDomain("test.gitlab.io") require.NoError(t, err) @@ -23,7 +22,7 @@ func TestGetDomain(t *testing.T) { t.Run("when the response is not valid", func(t *testing.T) { client := client.StubClient{File: "/dev/null"} - source := Gitlab{client: client, cache: cache.New()} + source := Gitlab{client: client} domain, err := source.GetDomain("test.gitlab.io") @@ -34,7 +33,7 @@ func TestGetDomain(t *testing.T) { func TestResolve(t *testing.T) { client := client.StubClient{File: "client/testdata/test.gitlab.io.json"} - source := Gitlab{client: client, cache: cache.New()} + source := Gitlab{client: client} t.Run("when requesting a nested group project", func(t *testing.T) { target := "https://test.gitlab.io:443/my/pages/project/path/index.html" |