From 790cb5c678c595eb647f93e94941a1451dc4315d Mon Sep 17 00:00:00 2001 From: mchavez Date: Tue, 3 Sep 2024 11:37:54 -0600 Subject: [PATCH] Code review changes part 1 --- pkg/uhttp/client.go | 50 +++++++++++++++++++++++++++++++++ pkg/uhttp/dbcache.go | 59 ++++----------------------------------- pkg/uhttp/dbcache_test.go | 6 ++-- pkg/uhttp/gocache.go | 49 -------------------------------- pkg/uhttp/wrapper.go | 2 +- 5 files changed, 59 insertions(+), 107 deletions(-) diff --git a/pkg/uhttp/client.go b/pkg/uhttp/client.go index 6b800c58..1768b6af 100644 --- a/pkg/uhttp/client.go +++ b/pkg/uhttp/client.go @@ -2,8 +2,12 @@ package uhttp import ( "context" + "crypto/sha256" "crypto/tls" + "fmt" "net/http" + "sort" + "strings" "time" "go.uber.org/zap" @@ -72,3 +76,49 @@ func NewClient(ctx context.Context, options ...Option) (*http.Client, error) { httpClient.Transport = t return httpClient, nil } + +type ICache interface { + Get(ctx context.Context, key string) (*http.Response, error) + Set(ctx context.Context, key string, value *http.Response) error +} + +// CreateCacheKey generates a cache key based on the request URL, query parameters, and headers. +func CreateCacheKey(req *http.Request) (string, error) { + var sortedParams []string + // Normalize the URL path + path := strings.ToLower(req.URL.Path) + // Combine the path with sorted query parameters + queryParams := req.URL.Query() + for k, v := range queryParams { + for _, value := range v { + sortedParams = append(sortedParams, fmt.Sprintf("%s=%s", k, value)) + } + } + + sort.Strings(sortedParams) + queryString := strings.Join(sortedParams, "&") + // Include relevant headers in the cache key + var headerParts []string + for key, values := range req.Header { + for _, value := range values { + if key == "Accept" || key == "Content-Type" || key == "Cookie" || key == "Range" { + headerParts = append(headerParts, fmt.Sprintf("%s=%s", key, value)) + } + } + } + + sort.Strings(headerParts) + headersString := strings.Join(headerParts, "&") + // Create a unique string for the cache key + cacheString := fmt.Sprintf("%s?%s&headers=%s", path, queryString, headersString) + + // Hash the cache string to create a key + hash := sha256.New() + _, err := hash.Write([]byte(cacheString)) + if err != nil { + return "", err + } + + cacheKey := fmt.Sprintf("%x", hash.Sum(nil)) + return cacheKey, nil +} diff --git a/pkg/uhttp/dbcache.go b/pkg/uhttp/dbcache.go index 4d076b16..0637201d 100644 --- a/pkg/uhttp/dbcache.go +++ b/pkg/uhttp/dbcache.go @@ -4,7 +4,6 @@ import ( "bufio" "bytes" "context" - "crypto/sha256" "database/sql" "encoding/json" "errors" @@ -13,8 +12,6 @@ import ( "net/http/httputil" "os" "path/filepath" - "sort" - "strings" "time" _ "github.com/glebarez/go-sqlite" @@ -23,11 +20,6 @@ import ( "go.uber.org/zap" ) -type ICache interface { - Get(ctx context.Context, key string) (*http.Response, error) - Set(ctx context.Context, key string, value *http.Response) error - CreateCacheKey(req *http.Request) (string, error) -} type DBCache struct { db *sql.DB waitDuration int64 @@ -125,47 +117,6 @@ func NewDBCache(ctx context.Context, cfg CacheConfig) (*DBCache, error) { return dc, nil } -// CreateCacheKey generates a cache key based on the request URL, query parameters, and headers. -func (d *DBCache) CreateCacheKey(req *http.Request) (string, error) { - var sortedParams []string - // Normalize the URL path - path := strings.ToLower(req.URL.Path) - // Combine the path with sorted query parameters - queryParams := req.URL.Query() - for k, v := range queryParams { - for _, value := range v { - sortedParams = append(sortedParams, fmt.Sprintf("%s=%s", k, value)) - } - } - - sort.Strings(sortedParams) - queryString := strings.Join(sortedParams, "&") - // Include relevant headers in the cache key - var headerParts []string - for key, values := range req.Header { - for _, value := range values { - if key == "Accept" || key == "Content-Type" || key == "Cookie" || key == "Range" { - headerParts = append(headerParts, fmt.Sprintf("%s=%s", key, value)) - } - } - } - - sort.Strings(headerParts) - headersString := strings.Join(headerParts, "&") - // Create a unique string for the cache key - cacheString := fmt.Sprintf("%s?%s&headers=%s", path, queryString, headersString) - - // Hash the cache string to create a key - hash := sha256.New() - _, err := hash.Write([]byte(cacheString)) - if err != nil { - return "", err - } - - cacheKey := fmt.Sprintf("%x", hash.Sum(nil)) - return cacheKey, nil -} - func (d *DBCache) Load(ctx context.Context) (*DBCache, error) { l := ctxzap.Extract(ctx) cacheDir, err := os.UserCacheDir() @@ -212,7 +163,7 @@ func (d *DBCache) Get(ctx context.Context, key string) (*http.Response, error) { return nil, fmt.Errorf("%s", nilConnection) } - entry, err := d.Select(ctx, key) + entry, err := d.pick(ctx, key) if err == nil && len(entry) > 0 { r := bufio.NewReader(bytes.NewReader(entry)) resp, err := http.ReadResponse(r, nil) @@ -252,7 +203,7 @@ func (d *DBCache) Set(ctx context.Context, key string, value *http.Response) err url = getFullUrl(value.Request) } - err = d.Insert(ctx, + err = d.insert(ctx, key, cacheableResponse, url, @@ -307,7 +258,7 @@ func (d *DBCache) cleanup(ctx context.Context) error { } // Insert data into the cache table. -func (d *DBCache) Insert(ctx context.Context, key string, value any, url string) error { +func (d *DBCache) insert(ctx context.Context, key string, value any, url string) error { var ( bytes []byte err error @@ -388,8 +339,8 @@ func (d *DBCache) IsNilConnection() bool { return d.db == nil } -// Select query for cached response. -func (d *DBCache) Select(ctx context.Context, key string) ([]byte, error) { +// select query for cached response. +func (d *DBCache) pick(ctx context.Context, key string) ([]byte, error) { var data []byte if d.IsNilConnection() { return nil, fmt.Errorf("%s", nilConnection) diff --git a/pkg/uhttp/dbcache_test.go b/pkg/uhttp/dbcache_test.go index e4426373..67adbd1c 100644 --- a/pkg/uhttp/dbcache_test.go +++ b/pkg/uhttp/dbcache_test.go @@ -29,7 +29,7 @@ func TestDBCacheGettersAndSetters(t *testing.T) { var ic ICache = &DBCache{ db: fc.db, } - cKey, err := ic.CreateCacheKey(resp.Request) + cKey, err := CreateCacheKey(resp.Request) require.Nil(t, err) err = ic.Set(ctx, cKey, resp) @@ -45,10 +45,10 @@ func TestDBCache(t *testing.T) { fc, err := getDBCacheForTesting() require.Nil(t, err) - err = fc.Insert(ctx, "urlTest", urlTest, "http://example.com") + err = fc.insert(ctx, "urlTest", urlTest, "http://example.com") require.Nil(t, err) - res, err := fc.Select(ctx, "urlTest") + res, err := fc.pick(ctx, "urlTest") require.Nil(t, err) require.NotNil(t, res) diff --git a/pkg/uhttp/gocache.go b/pkg/uhttp/gocache.go index 9efc5519..39f5bab8 100644 --- a/pkg/uhttp/gocache.go +++ b/pkg/uhttp/gocache.go @@ -4,12 +4,8 @@ import ( "bufio" "bytes" "context" - "crypto/sha256" - "fmt" "net/http" "net/http/httputil" - "sort" - "strings" "time" bigCache "github.com/allegro/bigcache/v3" @@ -53,51 +49,6 @@ func (g *GoCache) Statistics() bigCache.Stats { return g.rootLibrary.Stats() } -// CreateCacheKey generates a cache key based on the request URL, query parameters, and headers. -// The key is a SHA-256 hash of the normalized URL path, sorted query parameters, and relevant headers. -func (g *GoCache) CreateCacheKey(req *http.Request) (string, error) { - // Normalize the URL path - path := strings.ToLower(req.URL.Path) - - // Combine the path with sorted query parameters - queryParams := req.URL.Query() - var sortedParams []string - for k, v := range queryParams { - for _, value := range v { - sortedParams = append(sortedParams, fmt.Sprintf("%s=%s", k, value)) - } - } - - sort.Strings(sortedParams) - queryString := strings.Join(sortedParams, "&") - - // Include relevant headers in the cache key - var headerParts []string - for key, values := range req.Header { - for _, value := range values { - if key == "Accept" || key == "Authorization" || key == "Cookie" || key == "Range" { - headerParts = append(headerParts, fmt.Sprintf("%s=%s", key, value)) - } - } - } - - sort.Strings(headerParts) - headersString := strings.Join(headerParts, "&") - - // Create a unique string for the cache key - cacheString := fmt.Sprintf("%s?%s&headers=%s", path, queryString, headersString) - - // Hash the cache string to create a key - hash := sha256.New() - _, err := hash.Write([]byte(cacheString)) - if err != nil { - return "", err - } - - cacheKey := fmt.Sprintf("%x", hash.Sum(nil)) - return cacheKey, nil -} - func (g *GoCache) Get(ctx context.Context, key string) (*http.Response, error) { if g.rootLibrary == nil { return nil, nil diff --git a/pkg/uhttp/wrapper.go b/pkg/uhttp/wrapper.go index 549f6b9b..74271632 100644 --- a/pkg/uhttp/wrapper.go +++ b/pkg/uhttp/wrapper.go @@ -246,7 +246,7 @@ func (c *BaseHttpClient) Do(req *http.Request, options ...DoOption) (*http.Respo ) l := ctxzap.Extract(req.Context()) if req.Method == http.MethodGet { - cacheKey, err = c.baseHttpCache.CreateCacheKey(req) + cacheKey, err = CreateCacheKey(req) if err != nil { return nil, err }