moving GetCacheKey to Cache package, comments, Do trace log

README.md

@@ -67,8 +67,9 @@ func main() {
 ## Todo
 
 - Maybe the context usage throughout the project could be improved
-- Maybe add a Logger to the Cache and RateLimit interfaces
+- Maybe add and move logging to the Cache and RateLimit interfaces, instead of passing them around
 - Add checks for duration of tests that include any WaitN/any blocking
+- Add more integration tests
 - RateLimit
   - Add Redis store, using a lua script
   - Maybe add more options (presets?) to customize the rate limiter

cache/cache.go

@@ -3,7 +3,9 @@ package cache
 
 import (
 	"context"
+	"crypto/sha256"
 	"errors"
+	"fmt"
 	"time"
 
 	"github.com/allegro/bigcache/v3"
@@ -116,3 +118,34 @@ func (c *Cache) Clear(ctx context.Context) error {
 	}
 	return c.store.Clear(ctx)
 }
+
+// Returns the Cache key for the provided URL and a bool indicating if the key has an accessToken hash. Most of the time this will just return the URL.
+func GetCacheKey(url string, authHeader string) (string, bool) {
+	// I plan to use xxhash instead of sha256 in the future since it is already imported by `go-redis`.
+	//
+	// Issues with this:
+	// 	- I want to keep the Cache accessible to other clients without having to add hashing to them just to access the Cache.
+	//	- I don't know about xxhash support in other languages.
+	//	- Some operations(in go) use unsafe, can be avoided but it would be slower.
+	//
+	// Cache keys should be URLs with the exception of the hashed portion included in methods requiring `accessToken`, meaning,
+	// you would need to hash something to get the Cache key ONLY for those methods.
+	//
+	// Example:
+	//
+	// "https://asia.api.riotgames.com/riot/account/v1/accounts/me-ec2cc2a7cbc79c8d8def89cb9b9a1bccf4c2efc56a9c8063f9f4ae806f08c4d7"
+	//
+	//	- URL = "https://asia.api.riotgames.com/riot/account/v1/accounts/me"
+	//	- Separator = "-"
+	//	- Hash of an accessToken = "ec2cc2a7cbc79c8d8def89cb9b9a1bccf4c2efc56a9c8063f9f4ae806f08c4d7"
+
+	if authHeader == "" {
+		return url, false
+	}
+
+	hash := sha256.New()
+	_, _ = hash.Write([]byte(authHeader))
+	hashedAuth := hash.Sum(nil)
+
+	return fmt.Sprintf("%s-%x", url, hashedAuth), true
+}

cache/cache_test.go

@@ -2,8 +2,11 @@ package cache_test
 
 import (
 	"context"
+	"net/http"
+	"net/url"
 	"testing"
 
+	"github.com/Kyagara/equinox/v2/api"
 	"github.com/Kyagara/equinox/v2/cache"
 	"github.com/Kyagara/equinox/v2/test/util"
 	"github.com/rs/zerolog"
@@ -39,3 +42,28 @@ func TestCacheMethods(t *testing.T) {
 	logger := util.NewTestLogger()
 	logger.Debug().Object("cache", cacheStore).Msg("Testing cache marshal")
 }
+
+func TestGetCacheKey(t *testing.T) {
+	t.Parallel()
+
+	req := &http.Request{
+		URL: &url.URL{
+			Scheme: "http",
+			Host:   "example.com",
+			Path:   "/path",
+		},
+		Header: http.Header{},
+	}
+
+	equinoxReq := api.EquinoxRequest{Request: req}
+	equinoxReq.URL = req.URL.String()
+
+	hash, isRSO := cache.GetCacheKey(equinoxReq.URL, equinoxReq.Request.Header.Get("Authorization"))
+	require.Equal(t, "http://example.com/path", hash)
+	require.False(t, isRSO)
+
+	req.Header.Set("Authorization", "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghij")
+	hash, isRSO = cache.GetCacheKey(equinoxReq.URL, equinoxReq.Request.Header.Get("Authorization"))
+	require.Equal(t, "http://example.com/path-ec2cc2a7cbc79c8d8def89cb9b9a1bccf4c2efc56a9c8063f9f4ae806f08c4d7", hash)
+	require.True(t, isRSO)
+}

internal/client.go

@@ -3,7 +3,6 @@ package internal
 import (
 	"bytes"
 	"context"
-	"crypto/sha256"
 	"errors"
 	"fmt"
 	"io"
@@ -124,29 +123,33 @@ func (c *Client) Request(ctx context.Context, logger zerolog.Logger, httpMethod
 }
 
 // Executes a 'EquinoxRequest', checks cache and unmarshals the response into 'target'.
+//
+// ctx accepts 'api.ExecuteOptions', 'api.Revalidate' for example can be used to revalidate the cache, forcing an update to it.
 func (c *Client) Execute(ctx context.Context, equinoxReq api.EquinoxRequest, target any) error {
 	equinoxReq.Logger.Trace().Msg("Execute")
 
 	if ctx == nil {
 		return ErrContextIsNil
 	}
 
-	revalidate := ctx.Value(api.Revalidate)
-	key, isRSO := GetCacheKey(equinoxReq)
+	key, isRSO := cache.GetCacheKey(equinoxReq.URL, equinoxReq.Request.Header.Get("Authorization"))
 
-	if c.IsCacheEnabled && equinoxReq.Request.Method == http.MethodGet && revalidate == nil {
-		item, err := c.cache.Get(ctx, key)
-		if err != nil {
-			equinoxReq.Logger.Error().Err(err).Msg("Error retrieving cached response")
-			return err
-		}
+	if c.IsCacheEnabled && equinoxReq.Request.Method == http.MethodGet {
+		revalidate := ctx.Value(api.Revalidate)
+		if revalidate == nil {
+			item, err := c.cache.Get(ctx, key)
+			if err != nil {
+				equinoxReq.Logger.Error().Err(err).Msg("Error retrieving cached response")
+				return err
+			}
 
-		if item != nil {
-			equinoxReq.Logger.Debug().Str("route", equinoxReq.Route).Msg("Cache hit")
+			if item != nil {
+				equinoxReq.Logger.Debug().Str("route", equinoxReq.Route).Msg("Cache hit")
 
-			// Only valid json is cached, so unmarshal shouldn't fail
-			_ = jsonv2.Unmarshal(item, target)
-			return nil
+				// Only valid json is cached, so unmarshal shouldn't fail
+				_ = jsonv2.Unmarshal(item, target)
+				return nil
+			}
 		}
 	}
 
@@ -201,6 +204,8 @@ func (c *Client) Execute(ctx context.Context, equinoxReq api.EquinoxRequest, tar
 		return nil
 	}
 
+	// UnmarshalRead any other http methods
+
 	err = jsonv2.UnmarshalRead(response.Body, target)
 	if err != nil {
 		equinoxReq.Logger.Error().Err(err).Msg("Error unmarshalling response")
@@ -242,7 +247,7 @@ func (c *Client) ExecuteBytes(ctx context.Context, equinoxReq api.EquinoxRequest
 	return body, nil
 }
 
-// Sends the request, retries if enabled.
+// Sends the request using the internal http.Client, retries if enabled.
 func (c *Client) Do(ctx context.Context, equinoxReq api.EquinoxRequest) (*http.Response, error) {
 	equinoxReq.Logger.Trace().Msg("Do")
 
@@ -258,6 +263,7 @@ func (c *Client) Do(ctx context.Context, equinoxReq api.EquinoxRequest) (*http.R
 
 		delay, retryable, err := c.checkResponse(ctx, equinoxReq, response)
 		if err == nil && delay == 0 {
+			equinoxReq.Logger.Trace().Str("route", equinoxReq.Route).Msg("Success")
 			return response, nil
 		}
 
@@ -281,7 +287,7 @@ func (c *Client) Do(ctx context.Context, equinoxReq api.EquinoxRequest) (*http.R
 	return nil, fmt.Errorf("%w: %w", ErrMaxRetries, httpErr)
 }
 
-// Checks the response, check if it contains a 'Retry-After' header and whether it should be retried (StatusCode within range 429-599).
+// Checks if the response contains a 'Retry-After' header and if it should be retried (StatusCode within range 429-599).
 func (c *Client) checkResponse(ctx context.Context, equinoxReq api.EquinoxRequest, response *http.Response) (time.Duration, bool, error) {
 	// Delay in milliseconds
 	var retryAfter time.Duration
@@ -314,35 +320,3 @@ func (c *Client) checkResponse(ctx context.Context, equinoxReq api.EquinoxReques
 
 	return 0, false, fmt.Errorf("unexpected status code: %d", response.StatusCode)
 }
-
-// Returns the Cache key with a hash of the access token if it exists.
-func GetCacheKey(req api.EquinoxRequest) (string, bool) {
-	// I plan to use xxhash instead of sha256 in the future since it is already imported by `go-redis`.
-	//
-	// Issues with this:
-	// 	- I want to keep the Cache accessible to other clients without having to add hashing to them just to reuse the Cache.
-	//	- I don't know about xxhash support in other languages.
-	//	- Some operations(in go) use unsafe, can be avoided but it would be slower.
-	//
-	// Cache keys should be URLs with the exception of the hashed portion included in methods requiring `accessToken`, meaning,
-	// you would need to hash something to get the Cache key ONLY for those methods.
-	//
-	// Example:
-	//
-	// "https://asia.api.riotgames.com/riot/account/v1/accounts/me-ec2cc2a7cbc79c8d8def89cb9b9a1bccf4c2efc56a9c8063f9f4ae806f08c4d7"
-	//
-	//	- URL = "https://asia.api.riotgames.com/riot/account/v1/accounts/me"
-	//	- Separator = "-"
-	//	- Hash of the access token = "ec2cc2a7cbc79c8d8def89cb9b9a1bccf4c2efc56a9c8063f9f4ae806f08c4d7"
-
-	auth := req.Request.Header.Get("Authorization")
-	if auth == "" {
-		return req.URL, false
-	}
-
-	hash := sha256.New()
-	_, _ = hash.Write([]byte(auth))
-	hashedAuth := hash.Sum(nil)
-
-	return fmt.Sprintf("%s-%x", req.URL, hashedAuth), true
-}

internal/client_test.go

@@ -54,31 +54,6 @@ func TestNewInternalClient(t *testing.T) {
 	require.True(t, internalClient.IsRetryEnabled)
 }
 
-func TestGetCacheKey(t *testing.T) {
-	t.Parallel()
-
-	req := &http.Request{
-		URL: &url.URL{
-			Scheme: "http",
-			Host:   "example.com",
-			Path:   "/path",
-		},
-		Header: http.Header{},
-	}
-
-	equinoxReq := api.EquinoxRequest{Request: req}
-	equinoxReq.URL = req.URL.String()
-
-	hash, isRSO := internal.GetCacheKey(equinoxReq)
-	require.Equal(t, "http://example.com/path", hash)
-	require.False(t, isRSO)
-
-	req.Header.Set("Authorization", "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghij")
-	hash, isRSO = internal.GetCacheKey(equinoxReq)
-	require.Equal(t, "http://example.com/path-ec2cc2a7cbc79c8d8def89cb9b9a1bccf4c2efc56a9c8063f9f4ae806f08c4d7", hash)
-	require.True(t, isRSO)
-}
-
 func TestStatusCodeToError(t *testing.T) {
 	httpmock.Activate()
 	defer httpmock.DeactivateAndReset()

test/benchmark/cache_test.go

@@ -2,10 +2,13 @@ package benchmark_test
 
 import (
 	"context"
+	"net/http"
+	"net/url"
 	"testing"
 	"time"
 
 	"github.com/Kyagara/equinox/v2"
+	"github.com/Kyagara/equinox/v2/api"
 	"github.com/Kyagara/equinox/v2/cache"
 	"github.com/Kyagara/equinox/v2/clients/lol"
 	"github.com/Kyagara/equinox/v2/test/util"
@@ -99,3 +102,38 @@ func BenchmarkCacheRedisSummonerByPUUID(b *testing.B) {
 		}
 	}
 }
+
+func BenchmarkCacheGetKey(b *testing.B) {
+	b.ReportAllocs()
+	httpmock.Activate()
+	defer httpmock.DeactivateAndReset()
+
+	httpmock.RegisterResponder("GET", "https://kr.api.riotgames.com/lol/summoner/v4/summoners/by-puuid/puuid",
+		httpmock.NewStringResponder(200, `"response"`))
+
+	req := &http.Request{
+		URL: &url.URL{
+			Scheme: "http",
+			Host:   "example.com",
+			Path:   "/path",
+		},
+		Header: http.Header{},
+	}
+
+	equinoxReq := api.EquinoxRequest{Request: req}
+	equinoxReq.URL = req.URL.String()
+
+	// Random JWT I asked ChatGPT, its invalid, also around 300 characters longer than the access token I used for testing.
+	req.Header.Set("Authorization", "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghij")
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		key, isRSO := cache.GetCacheKey(equinoxReq.URL, equinoxReq.Request.Header.Get("Authorization"))
+		if key != "http://example.com/path-ec2cc2a7cbc79c8d8def89cb9b9a1bccf4c2efc56a9c8063f9f4ae806f08c4d7" {
+			b.Fatalf("key != http://example.com/path-ec2cc2a7cbc79c8d8def89cb9b9a1bccf4c2efc56a9c8063f9f4ae806f08c4d7, got: %s", key)
+		}
+		if !isRSO {
+			b.Fatal("isRSO != true")
+		}
+	}
+}

test/benchmark/internal_test.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"net/http"
-	"net/url"
 	"testing"
 
 	"github.com/Kyagara/equinox/v2"
@@ -145,38 +144,3 @@ func BenchmarkInternalExecuteBytes(b *testing.B) {
 		}
 	}
 }
-
-func BenchmarkInternalGetCacheKey(b *testing.B) {
-	b.ReportAllocs()
-	httpmock.Activate()
-	defer httpmock.DeactivateAndReset()
-
-	httpmock.RegisterResponder("GET", "https://kr.api.riotgames.com/lol/summoner/v4/summoners/by-puuid/puuid",
-		httpmock.NewStringResponder(200, `"response"`))
-
-	req := &http.Request{
-		URL: &url.URL{
-			Scheme: "http",
-			Host:   "example.com",
-			Path:   "/path",
-		},
-		Header: http.Header{},
-	}
-
-	equinoxReq := api.EquinoxRequest{Request: req}
-	equinoxReq.URL = req.URL.String()
-
-	// Random JWT I asked ChatGPT, its invalid, also around 300 characters longer than the access token I used for testing.
-	req.Header.Set("Authorization", "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghij")
-
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
-		url, isRSO := internal.GetCacheKey(equinoxReq)
-		if url != "http://example.com/path-ec2cc2a7cbc79c8d8def89cb9b9a1bccf4c2efc56a9c8063f9f4ae806f08c4d7" {
-			b.Fatalf("URL != http://example.com/path-ec2cc2a7cbc79c8d8def89cb9b9a1bccf4c2efc56a9c8063f9f4ae806f08c4d7, got: %s", url)
-		}
-		if !isRSO {
-			b.Fatal("isRSO != true")
-		}
-	}
-}