From bee6e4e5596551e9ca7ddfb5875fc1f6c1d3b4c9 Mon Sep 17 00:00:00 2001
From: Armon Dadgar <armon.dadgar@gmail.com>
Date: Fri, 8 Aug 2014 17:37:13 -0700
Subject: [PATCH] acl: Adding additional tier of caching

---
 acl/cache.go      | 45 +++++++++++++++++++++++++++++----------------
 acl/cache_test.go | 11 ++++++++++-
 2 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/acl/cache.go b/acl/cache.go
index a4aa61e71..f5ef96fb8 100644
--- a/acl/cache.go
+++ b/acl/cache.go
@@ -19,10 +19,11 @@ type aclEntry struct {
 
 // Cache is used to implement policy and ACL caching
 type Cache struct {
-	aclCache    *lru.Cache
+	aclCache    *lru.Cache // Cache id -> acl
 	faultfn     FaultFunc
 	parent      ACL
-	policyCache *lru.Cache
+	policyCache *lru.Cache // Cache policy -> acl
+	ruleCache   *lru.Cache // Cache rules -> policy
 }
 
 // NewCache contructs a new policy and ACL cache of a given size
@@ -30,6 +31,7 @@ func NewCache(size int, parent ACL, faultfn FaultFunc) (*Cache, error) {
 	if size <= 0 {
 		return nil, fmt.Errorf("Must provide positive cache size")
 	}
+	rc, _ := lru.New(size)
 	pc, _ := lru.New(size)
 	ac, _ := lru.New(size)
 	c := &Cache{
@@ -37,6 +39,7 @@ func NewCache(size int, parent ACL, faultfn FaultFunc) (*Cache, error) {
 		faultfn:     faultfn,
 		parent:      parent,
 		policyCache: pc,
+		ruleCache:   rc,
 	}
 	return c, nil
 }
@@ -50,7 +53,7 @@ func (c *Cache) GetPolicy(rules string) (*Policy, error) {
 // getPolicy is an internal method to get a cached policy,
 // but it assumes a pre-computed ID
 func (c *Cache) getPolicy(id, rules string) (*Policy, error) {
-	raw, ok := c.policyCache.Get(id)
+	raw, ok := c.ruleCache.Get(id)
 	if ok {
 		return raw.(*Policy), nil
 	}
@@ -59,7 +62,7 @@ func (c *Cache) getPolicy(id, rules string) (*Policy, error) {
 		return nil, err
 	}
 	policy.ID = id
-	c.policyCache.Add(id, policy)
+	c.ruleCache.Add(id, policy)
 	return policy, nil
 
 }
@@ -75,7 +78,7 @@ func (c *Cache) GetACLPolicy(id string) (*Policy, error) {
 	// Check for a cached acl
 	if raw, ok := c.aclCache.Get(id); ok {
 		cached := raw.(aclEntry)
-		if raw, ok := c.policyCache.Get(cached.PolicyID); ok {
+		if raw, ok := c.ruleCache.Get(cached.PolicyID); ok {
 			return raw.(*Policy), nil
 		}
 	}
@@ -106,21 +109,31 @@ func (c *Cache) GetACL(id string) (ACL, error) {
 	}
 	ruleID := c.ruleID(rules)
 
-	// Get the policy
-	policy, err := c.getPolicy(ruleID, rules)
-	if err != nil {
-		return nil, err
-	}
+	// Check for a compiled ACL
+	var compiled ACL
+	if raw, ok := c.policyCache.Get(ruleID); ok {
+		compiled = raw.(ACL)
+	} else {
+		// Get the policy
+		policy, err := c.getPolicy(ruleID, rules)
+		if err != nil {
+			return nil, err
+		}
 
-	// Get the ACL
-	acl, err := New(c.parent, policy)
-	if err != nil {
-		return nil, err
+		// Compile the ACL
+		acl, err := New(c.parent, policy)
+		if err != nil {
+			return nil, err
+		}
+
+		// Cache the compiled ACL
+		c.policyCache.Add(ruleID, acl)
+		compiled = acl
 	}
 
 	// Cache and return the ACL
-	c.aclCache.Add(id, aclEntry{acl, ruleID})
-	return acl, nil
+	c.aclCache.Add(id, aclEntry{compiled, ruleID})
+	return compiled, nil
 }
 
 // ClearACL is used to clear the ACL cache if any
diff --git a/acl/cache_test.go b/acl/cache_test.go
index 96b4bf0c8..341ad9c16 100644
--- a/acl/cache_test.go
+++ b/acl/cache_test.go
@@ -43,7 +43,7 @@ func TestCache_GetPolicy(t *testing.T) {
 func TestCache_GetACL(t *testing.T) {
 	policies := map[string]string{
 		"foo": testSimplePolicy,
-		"bar": testSimplePolicy,
+		"bar": testSimplePolicy2,
 	}
 	faultfn := func(id string) (string, error) {
 		return policies[id], nil
@@ -113,6 +113,9 @@ func TestCache_ClearACL(t *testing.T) {
 	// Nuke the cache
 	c.ClearACL("foo")
 
+	// Clear the policy cache
+	c.policyCache.Remove(c.ruleID(testSimplePolicy))
+
 	acl2, err := c.GetACL("foo")
 	if err != nil {
 		t.Fatalf("err: %v", err)
@@ -170,3 +173,9 @@ key "foo/" {
 	policy = "read"
 }
 `
+
+var testSimplePolicy2 = `
+key "bar/" {
+	policy = "read"
+}
+`