From 496293afa1e013a10284f3625d9e5b84dc5db677 Mon Sep 17 00:00:00 2001
From: Ben Toogood <bentoogood@gmail.com>
Date: Sat, 23 May 2020 11:34:44 +0100
Subject: [PATCH] Use hash/fnv, add tests, fix request bug

---
 client/cache.go         | 53 ++++++++++++----------------
 client/cache_test.go    | 76 +++++++++++++++++++++++++++++++++++++++++
 util/wrapper/wrapper.go |  4 +--
 3 files changed, 99 insertions(+), 34 deletions(-)
 create mode 100644 client/cache_test.go

diff --git a/client/cache.go b/client/cache.go
index 7a6b8347..e4781dd6 100644
--- a/client/cache.go
+++ b/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{}
-	mutex  sync.Mutex
+	cache *cache.Cache
 }
 
 // Get a response from the cache
-func (c *Cache) Get(ctx context.Context, req *Request) interface{} {
-	md, _ := metadata.FromContext(ctx)
-	ck := cacheKey{req, md}
-
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if val, ok := c.values[ck.Hash()]; ok {
-		return val
-	}
-
-	return nil
+func (c *Cache) Get(ctx context.Context, req *Request) (interface{}, bool) {
+	return c.cache.Get(key(ctx, req))
 }
 
 // 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()
-	c.values[ck.Hash()] = rsp
-	defer c.mutex.Unlock()
-}
+	bytes, _ := json.Marshal(map[string]interface{}{
+		"metadata": md,
+		"request": map[string]interface{}{
+			"service":  (*req).Service(),
+			"endpoint": (*req).Endpoint(),
+			"method":   (*req).Method(),
+			"body":     (*req).Body(),
+		},
+	})
 
-type cacheKey struct {
-	Request  *Request
-	Metadata metadata.Metadata
-}
-
-// Source: https://gobyexample.com/sha1-hashes
-func (k *cacheKey) Hash() string {
-	bytes, _ := json.Marshal(k)
-	h := sha1.New()
+	h := fnv.New64()
 	h.Write(bytes)
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
diff --git a/client/cache_test.go b/client/cache_test.go
new file mode 100644
index 00000000..337312c1
--- /dev/null
+++ b/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")
+		}
+	})
+}
diff --git a/util/wrapper/wrapper.go b/util/wrapper/wrapper.go
index 598df497..d5ff9e83 100644
--- a/util/wrapper/wrapper.go
+++ b/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 {
-		rsp = cRsp
+	if r, ok := cache.Get(ctx, &req); ok {
+		rsp = r
 		return nil
 	}