Code review changes part 1

ConductorOne · Sep 3, 2024 · 790cb5c · 790cb5c
1 parent ab76255
commit 790cb5c
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 107 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
 		}