Use hash/fnv, add tests, fix request bug

client/cache.go

@@ -2,63 +2,52 @@ package client
 
 import (
 	"context"
-	"crypto/sha1"
 	"encoding/json"
 	"fmt"
-	"sync"
+	"hash/fnv"
 	"time"
 
 	"github.com/micro/go-micro/v2/metadata"
+	cache "github.com/patrickmn/go-cache"
 )
 
 // NewCache returns an initialised cache.
-// TODO: Setup a go routine to expire records in the cache.
 func NewCache() *Cache {
 	return &Cache{
-		values: make(map[string]interface{}),
+		cache: cache.New(cache.NoExpiration, 30*time.Second),
 	}
 }
 
 // Cache for responses
 type Cache struct {
-	values map[string]interface{}
+	cache *cache.Cache
-	mutex  sync.Mutex
 }
 
 // Get a response from the cache
-func (c *Cache) Get(ctx context.Context, req *Request) interface{} {
+func (c *Cache) Get(ctx context.Context, req *Request) (interface{}, bool) {
-	md, _ := metadata.FromContext(ctx)
+	return c.cache.Get(key(ctx, req))
-	ck := cacheKey{req, md}
-
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if val, ok := c.values[ck.Hash()]; ok {
-		return val
-	}
-
-	return nil
 }
 
 // Set a response in the cache
 func (c *Cache) Set(ctx context.Context, req *Request, rsp interface{}, expiry time.Duration) {
+	c.cache.Set(key(ctx, req), rsp, expiry)
+}
+
+// key returns a hash for the context and request
+func key(ctx context.Context, req *Request) string {
 	md, _ := metadata.FromContext(ctx)
-	ck := cacheKey{req, md}
 
-	c.mutex.Lock()
+	bytes, _ := json.Marshal(map[string]interface{}{
-	c.values[ck.Hash()] = rsp
+		"metadata": md,
-	defer c.mutex.Unlock()
+		"request": map[string]interface{}{
-}
+			"service":  (*req).Service(),
+			"endpoint": (*req).Endpoint(),
+			"method":   (*req).Method(),
+			"body":     (*req).Body(),
+		},
+	})
 
-type cacheKey struct {
+	h := fnv.New64()
-	Request  *Request
-	Metadata metadata.Metadata
-}
-
-// Source: https://gobyexample.com/sha1-hashes
-func (k *cacheKey) Hash() string {
-	bytes, _ := json.Marshal(k)
-	h := sha1.New()
 	h.Write(bytes)
 	return fmt.Sprintf("%x", h.Sum(nil))
 }

client/cache_test.go

@@ -0,0 +1,76 @@
+package client
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/micro/go-micro/v2/metadata"
+)
+
+func TestCache(t *testing.T) {
+	ctx := context.TODO()
+	req := NewRequest("go.micro.service.foo", "Foo.Bar", nil)
+
+	t.Run("CacheMiss", func(t *testing.T) {
+		if _, ok := NewCache().Get(ctx, &req); ok {
+			t.Errorf("Expected to get no result from Get")
+		}
+	})
+
+	t.Run("CacheHit", func(t *testing.T) {
+		c := NewCache()
+
+		rsp := "theresponse"
+		c.Set(ctx, &req, rsp, time.Minute)
+
+		if res, ok := c.Get(ctx, &req); !ok {
+			t.Errorf("Expected a result, got nothing")
+		} else if res != rsp {
+			t.Errorf("Expected '%v' result, got '%v'", rsp, res)
+		}
+	})
+}
+
+func TestCacheKey(t *testing.T) {
+	ctx := context.TODO()
+	req1 := NewRequest("go.micro.service.foo", "Foo.Bar", nil)
+	req2 := NewRequest("go.micro.service.foo", "Foo.Baz", nil)
+	req3 := NewRequest("go.micro.service.foo", "Foo.Baz", "customquery")
+
+	t.Run("IdenticalRequests", func(t *testing.T) {
+		key1 := key(ctx, &req1)
+		key2 := key(ctx, &req1)
+		if key1 != key2 {
+			t.Errorf("Expected the keys to match for identical requests and context")
+		}
+	})
+
+	t.Run("DifferentRequestEndpoints", func(t *testing.T) {
+		key1 := key(ctx, &req1)
+		key2 := key(ctx, &req2)
+
+		if key1 == key2 {
+			t.Errorf("Expected the keys to differ for different request endpoints")
+		}
+	})
+
+	t.Run("DifferentRequestBody", func(t *testing.T) {
+		key1 := key(ctx, &req2)
+		key2 := key(ctx, &req3)
+
+		if key1 == key2 {
+			t.Errorf("Expected the keys to differ for different request bodies")
+		}
+	})
+
+	t.Run("DifferentMetadata", func(t *testing.T) {
+		mdCtx := metadata.Set(context.TODO(), "foo", "bar")
+		key1 := key(mdCtx, &req1)
+		key2 := key(ctx, &req1)
+
+		if key1 == key2 {
+			t.Errorf("Expected the keys to differ for different metadata")
+		}
+	})
+}

util/wrapper/wrapper.go

@@ -253,8 +253,8 @@ func (c *cacheWrapper) Call(ctx context.Context, req client.Request, rsp interfa
 	}
 
 	// check to see if there is a response
-	if cRsp := cache.Get(ctx, &req); cRsp != nil {
+	if r, ok := cache.Get(ctx, &req); ok {
-		rsp = cRsp
+		rsp = r
 		return nil
 	}