Merge pull request #289 from linode/fix/cache-include-opts

fix: Include ListOptions as part of cached endpoint keys
linode · Jan 5, 2023 · 40af10b · 40af10b
2 parents 983213f + 966c2a7
commit 40af10b
Show file tree

Hide file tree

Showing 9 changed files with 283 additions and 266 deletions.
diff --git a/client.go b/client.go
@@ -207,14 +207,9 @@ func (c *Client) addRetryConditional(retryConditional RetryConditional) *Client
 	return c
 }
 
-func (c *Client) addCachedResponse(endpoint string, response any, expiry *time.Duration) error {
+func (c *Client) addCachedResponse(endpoint string, response any, expiry *time.Duration) {
 	if !c.shouldCache {
-		return nil
-	}
-
-	u, err := url.Parse(endpoint)
-	if err != nil {
-		return fmt.Errorf("failed to parse URL for caching: %s", err)
+		return
 	}
 
 	responseValue := reflect.ValueOf(response)
@@ -236,19 +231,12 @@ func (c *Client) addCachedResponse(endpoint string, response any, expiry *time.D
 	c.cachedEntryLock.Lock()
 	defer c.cachedEntryLock.Unlock()
 
-	c.cachedEntries[u.Path] = entry
-
-	return nil
+	c.cachedEntries[endpoint] = entry
 }
 
-func (c *Client) getCachedResponse(endpoint string) (any, error) {
+func (c *Client) getCachedResponse(endpoint string) any {
 	if !c.shouldCache {
-		return nil, nil
-	}
-
-	u, err := url.Parse(endpoint)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse URL for caching: %s", err)
+		return nil
 	}
 
 	c.cachedEntryLock.RLock()
@@ -265,9 +253,9 @@ func (c *Client) getCachedResponse(endpoint string) (any, error) {
 		}
 	}()
 
-	entry, ok := c.cachedEntries[u.Path]
+	entry, ok := c.cachedEntries[endpoint]
 	if !ok {
-		return nil, nil
+		return nil
 	}
 
 	// Handle expired entries
@@ -286,11 +274,11 @@ func (c *Client) getCachedResponse(endpoint string) (any, error) {
 		c.cachedEntryLock.Lock()
 		defer c.cachedEntryLock.Unlock()
 
-		delete(c.cachedEntries, u.Path)
-		return nil, nil
+		delete(c.cachedEntries, endpoint)
+		return nil
 	}
 
-	return c.cachedEntries[u.Path].Data, nil
+	return c.cachedEntries[endpoint].Data
 }
 
 // InvalidateCache clears all cached responses for all endpoints.
@@ -523,3 +511,16 @@ func copyTime(tPtr *time.Time) *time.Time {
 
 	return &t
 }
+
+func generateListCacheURL(endpoint string, opts *ListOptions) (string, error) {
+	if opts == nil {
+		return "", nil
+	}
+
+	hashedOpts, err := opts.Hash()
+	if err != nil {
+		return "", err
+	}
+
+	return fmt.Sprintf("%s:%s", endpoint, hashedOpts), nil
+}
diff --git a/databases.go b/databases.go
@@ -216,20 +216,21 @@ func (c *Client) ListDatabases(ctx context.Context, opts *ListOptions) ([]Databa
 func (c *Client) ListDatabaseEngines(ctx context.Context, opts *ListOptions) ([]DatabaseEngine, error) {
 	response := DatabaseEnginesPagedResponse{}
 
-	if result, err := c.getCachedResponse(response.endpoint()); err != nil {
+	endpoint, err := generateListCacheURL(response.endpoint(), opts)
+	if err != nil {
 		return nil, err
-	} else if result != nil {
+	}
+
+	if result := c.getCachedResponse(endpoint); result != nil {
 		return result.([]DatabaseEngine), nil
 	}
 
-	err := c.listHelper(ctx, &response, opts)
+	err = c.listHelper(ctx, &response, opts)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := c.addCachedResponse(response.endpoint(), response.Data, &cacheExpiryTime); err != nil {
-		return nil, err
-	}
+	c.addCachedResponse(endpoint, response.Data, &cacheExpiryTime)
 
 	return response.Data, nil
 }
@@ -238,9 +239,7 @@ func (c *Client) ListDatabaseEngines(ctx context.Context, opts *ListOptions) ([]
 func (c *Client) GetDatabaseEngine(ctx context.Context, opts *ListOptions, engineID string) (*DatabaseEngine, error) {
 	e := fmt.Sprintf("databases/engines/%s", engineID)
 
-	if result, err := c.getCachedResponse(e); err != nil {
-		return nil, err
-	} else if result != nil {
+	if result := c.getCachedResponse(e); result != nil {
 		result := result.(DatabaseEngine)
 		return &result, nil
 	}
@@ -251,9 +250,7 @@ func (c *Client) GetDatabaseEngine(ctx context.Context, opts *ListOptions, engin
 		return nil, err
 	}
 
-	if err := c.addCachedResponse(e, r.Result(), &cacheExpiryTime); err != nil {
-		return nil, err
-	}
+	c.addCachedResponse(e, r.Result(), &cacheExpiryTime)
 
 	return r.Result().(*DatabaseEngine), nil
 }
@@ -262,20 +259,21 @@ func (c *Client) GetDatabaseEngine(ctx context.Context, opts *ListOptions, engin
 func (c *Client) ListDatabaseTypes(ctx context.Context, opts *ListOptions) ([]DatabaseType, error) {
 	response := DatabaseTypesPagedResponse{}
 
-	if result, err := c.getCachedResponse(response.endpoint()); err != nil {
+	endpoint, err := generateListCacheURL(response.endpoint(), opts)
+	if err != nil {
 		return nil, err
-	} else if result != nil {
+	}
+
+	if result := c.getCachedResponse(endpoint); result != nil {
 		return result.([]DatabaseType), nil
 	}
 
-	err := c.listHelper(ctx, &response, opts)
+	err = c.listHelper(ctx, &response, opts)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := c.addCachedResponse(response.endpoint(), response.Data, &cacheExpiryTime); err != nil {
-		return nil, err
-	}
+	c.addCachedResponse(endpoint, response.Data, &cacheExpiryTime)
 
 	return response.Data, nil
 }
@@ -284,9 +282,7 @@ func (c *Client) ListDatabaseTypes(ctx context.Context, opts *ListOptions) ([]Da
 func (c *Client) GetDatabaseType(ctx context.Context, opts *ListOptions, typeID string) (*DatabaseType, error) {
 	e := fmt.Sprintf("databases/types/%s", typeID)
 
-	if result, err := c.getCachedResponse(e); err != nil {
-		return nil, err
-	} else if result != nil {
+	if result := c.getCachedResponse(e); result != nil {
 		result := result.(DatabaseType)
 		return &result, nil
 	}
@@ -297,9 +293,7 @@ func (c *Client) GetDatabaseType(ctx context.Context, opts *ListOptions, typeID
 		return nil, err
 	}
 
-	if err := c.addCachedResponse(e, r.Result(), &cacheExpiryTime); err != nil {
-		return nil, err
-	}
+	c.addCachedResponse(e, r.Result(), &cacheExpiryTime)
 
 	return r.Result().(*DatabaseType), nil
 }
diff --git a/kernels.go b/kernels.go
@@ -43,20 +43,21 @@ func (resp *LinodeKernelsPagedResponse) castResult(r *resty.Request, e string) (
 func (c *Client) ListKernels(ctx context.Context, opts *ListOptions) ([]LinodeKernel, error) {
 	response := LinodeKernelsPagedResponse{}
 
-	if result, err := c.getCachedResponse(response.endpoint()); err != nil {
+	endpoint, err := generateListCacheURL(response.endpoint(), opts)
+	if err != nil {
 		return nil, err
-	} else if result != nil {
+	}
+
+	if result := c.getCachedResponse(endpoint); result != nil {
 		return result.([]LinodeKernel), nil
 	}
 
-	err := c.listHelper(ctx, &response, opts)
+	err = c.listHelper(ctx, &response, opts)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := c.addCachedResponse(response.endpoint(), response.Data, nil); err != nil {
-		return nil, err
-	}
+	c.addCachedResponse(endpoint, response.Data, nil)
 
 	return response.Data, nil
 }
@@ -65,9 +66,7 @@ func (c *Client) ListKernels(ctx context.Context, opts *ListOptions) ([]LinodeKe
 func (c *Client) GetKernel(ctx context.Context, kernelID string) (*LinodeKernel, error) {
 	e := fmt.Sprintf("linode/kernels/%s", kernelID)
 
-	if result, err := c.getCachedResponse(e); err != nil {
-		return nil, err
-	} else if result != nil {
+	if result := c.getCachedResponse(e); result != nil {
 		result := result.(LinodeKernel)
 		return &result, nil
 	}
@@ -78,9 +77,7 @@ func (c *Client) GetKernel(ctx context.Context, kernelID string) (*LinodeKernel,
 		return nil, err
 	}
 
-	if err := c.addCachedResponse(e, r.Result(), nil); err != nil {
-		return nil, err
-	}
+	c.addCachedResponse(e, r.Result(), nil)
 
 	return r.Result().(*LinodeKernel), nil
 }
diff --git a/lke_clusters.go b/lke_clusters.go
@@ -142,20 +142,21 @@ func (resp *LKEVersionsPagedResponse) castResult(r *resty.Request, e string) (in
 func (c *Client) ListLKEVersions(ctx context.Context, opts *ListOptions) ([]LKEVersion, error) {
 	response := LKEVersionsPagedResponse{}
 
-	if result, err := c.getCachedResponse(response.endpoint()); err != nil {
+	endpoint, err := generateListCacheURL(response.endpoint(), opts)
+	if err != nil {
 		return nil, err
-	} else if result != nil {
+	}
+
+	if result := c.getCachedResponse(endpoint); result != nil {
 		return result.([]LKEVersion), nil
 	}
 
-	err := c.listHelper(ctx, &response, opts)
+	err = c.listHelper(ctx, &response, opts)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := c.addCachedResponse(response.endpoint(), response.Data, &cacheExpiryTime); err != nil {
-		return nil, err
-	}
+	c.addCachedResponse(endpoint, response.Data, &cacheExpiryTime)
 
 	return response.Data, nil
 }
@@ -164,9 +165,7 @@ func (c *Client) ListLKEVersions(ctx context.Context, opts *ListOptions) ([]LKEV
 func (c *Client) GetLKEVersion(ctx context.Context, version string) (*LKEVersion, error) {
 	e := fmt.Sprintf("lke/versions/%s", version)
 
-	if result, err := c.getCachedResponse(e); err != nil {
-		return nil, err
-	} else if result != nil {
+	if result := c.getCachedResponse(e); result != nil {
 		result := result.(LKEVersion)
 		return &result, nil
 	}
@@ -177,9 +176,7 @@ func (c *Client) GetLKEVersion(ctx context.Context, version string) (*LKEVersion
 		return nil, err
 	}
 
-	if err := c.addCachedResponse(e, r.Result(), &cacheExpiryTime); err != nil {
-		return nil, err
-	}
+	c.addCachedResponse(e, r.Result(), &cacheExpiryTime)
 
 	return r.Result().(*LKEVersion), nil
 }

diff --git a/pagination.go b/pagination.go
@@ -6,6 +6,9 @@ package linodego
 
 import (
 	"context"
+	"crypto/sha256"
+	"encoding/json"
+	"fmt"
 	"strconv"
 
 	"github.com/go-resty/resty/v2"
@@ -31,6 +34,19 @@ func NewListOptions(page int, filter string) *ListOptions {
 	return &ListOptions{PageOptions: &PageOptions{Page: page}, Filter: filter}
 }
 
+// Hash returns the sha256 hash of the provided ListOptions.
+// This is necessary for caching purposes.
+func (l ListOptions) Hash() (string, error) {
+	data, err := json.Marshal(l)
+	if err != nil {
+		return "", fmt.Errorf("failed to cache ListOptions: %s", err)
+	}
+
+	h := sha256.New()
+
+	return string(h.Sum(data)), nil
+}
+
 func applyListOptionsToRequest(opts *ListOptions, req *resty.Request) {
 	if opts != nil {
 		if opts.PageOptions != nil && opts.Page > 0 {

diff --git a/regions.go b/regions.go
@@ -52,20 +52,21 @@ func (resp *RegionsPagedResponse) castResult(r *resty.Request, e string) (int, i
 func (c *Client) ListRegions(ctx context.Context, opts *ListOptions) ([]Region, error) {
 	response := RegionsPagedResponse{}
 
-	if result, err := c.getCachedResponse(response.endpoint()); err != nil {
+	endpoint, err := generateListCacheURL(response.endpoint(), opts)
+	if err != nil {
 		return nil, err
-	} else if result != nil {
+	}
+
+	if result := c.getCachedResponse(endpoint); result != nil {
 		return result.([]Region), nil
 	}
 
-	err := c.listHelper(ctx, &response, opts)
+	err = c.listHelper(ctx, &response, opts)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := c.addCachedResponse(response.endpoint(), response.Data, &cacheExpiryTime); err != nil {
-		return nil, err
-	}
+	c.addCachedResponse(endpoint, response.Data, &cacheExpiryTime)
 
 	return response.Data, nil
 }
@@ -74,9 +75,7 @@ func (c *Client) ListRegions(ctx context.Context, opts *ListOptions) ([]Region,
 func (c *Client) GetRegion(ctx context.Context, regionID string) (*Region, error) {
 	e := fmt.Sprintf("regions/%s", regionID)
 
-	if result, err := c.getCachedResponse(e); err != nil {
-		return nil, err
-	} else if result != nil {
+	if result := c.getCachedResponse(e); result != nil {
 		result := result.(Region)
 		return &result, nil
 	}
@@ -87,9 +86,7 @@ func (c *Client) GetRegion(ctx context.Context, regionID string) (*Region, error
 		return nil, err
 	}
 
-	if err := c.addCachedResponse(e, r.Result(), &cacheExpiryTime); err != nil {
-		return nil, err
-	}
+	c.addCachedResponse(e, r.Result(), &cacheExpiryTime)
 
 	return r.Result().(*Region), nil
 }