Repository: casbin/casbin
Pull request #1217 (2023-03-25 15:40:52)

feat: add sync cache and synced cached enforcer
Fix: https://github.com/casbin/casbin/issues/1214

I tried to add a sync cache and then add SyncedCachedEnforcer. 

CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/casbin/casbin?pullRequest=1217) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/casbin/casbin?pullRequest=1217) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/casbin/casbin?pullRequest=1217) it.</sub>
casbin-bot:  @tangyang9464 @JalinWang please review
hsluoyz: @SnowWarri0r fix all CI errors:

![image](https://user-images.githubusercontent.com/3787410/227728660-1ef48dde-5ed3-42c6-8d30-d7a8bbe5bd2f.png)

SnowWarri0r: in my local,i can pass all test,i just changed examples/rbac_model.conf from CRLF to LF.
my go version is 1.18.4
![image](https://user-images.githubusercontent.com/62747881/227730141-5c6b760b-b50b-4a6c-96f2-f3ec660bfe44.png)
 
hsluoyz: @PokIsemaine 
PokIsemaine: @SnowWarri0r 
![image](https://user-images.githubusercontent.com/46661603/227751548-9337d2be-aed3-4865-bc3f-1f88a6e21dcd.png)

You can try to refactor your code.
![image](https://user-images.githubusercontent.com/46661603/227751539-1faa585b-c2fe-427e-805a-9faaa6aaf2dd.png)

PokIsemaine: > in my local,i can pass all test,i just changed examples/rbac_model.conf from CRLF to LF. my go version is 1.18.4 ![image](https://user-images.githubusercontent.com/62747881/227730141-5c6b760b-b50b-4a6c-96f2-f3ec660bfe44.png)
CRLF and LF can be solved by setting up your git or editor.

When testing concurrently, you can consider repeating the test multiple times (or even looping infinitely until an error is reported when testing locally) to reduce the possibility of accidental success. You can refer to SyncedEnforcer's test.

SnowWarri0r: > > in my local,i can pass all test,i just changed examples/rbac_model.conf from CRLF to LF. my go version is 1.18.4 ![image](https://user-images.githubusercontent.com/62747881/227730141-5c6b760b-b50b-4a6c-96f2-f3ec660bfe44.png)
> 
> 1. CRLF and LF can be solved by setting up your git or editor.
> 2. When testing concurrently, you can consider repeating the test multiple times (or even looping infinitely until an error is reported when testing locally) to reduce the possibility of accidental success. You can refer to SyncedEnforcer's test.

i changed SyncedCachedEnforcer internal Enforcer to SyncedEnforcer.
then I tried to run testing, had been starting 1000000 goroutines and then had passed.
about refactoring, i extract getKey method to function.
SnowWarri0r: force-pushed to fix commit message
SnowWarri0r: @hsluoyz
hsluoyz: @yugant007 @PokIsemaine plz review
PokIsemaine: @SnowWarri0r 
Thanks for your work, the basic functionality and tests are well done.

@hsluoyz 
Regarding "code climate", I think the relevant threshold may be a little stricter, you may consider manually skipping "code climate" to merge this PR (of course, this needs to be considered carefully, because it may cause a bad start)
Please let @SnowWarri0r  know if you think refactoring is still needed
hsluoyz: "code climate" is optional

Patch:
diff --git a/enforcer_cached.go b/enforcer_cached.go
index b5ab43f..daf225f 100644
--- a/enforcer_cached.go
+++ b/enforcer_cached.go
@@ -147,6 +147,17 @@ func (e *CachedEnforcer) setCachedResult(key string, res bool, extra ...interfac
 }
 
 func (e *CachedEnforcer) getKey(params ...interface{}) (string, bool) {
+	return GetCacheKey(params...)
+}
+
+// InvalidateCache deletes all the existing cached decisions.
+func (e *CachedEnforcer) InvalidateCache() error {
+	e.locker.Lock()
+	defer e.locker.Unlock()
+	return e.cache.Clear()
+}
+
+func GetCacheKey(params ...interface{}) (string, bool) {
 	key := strings.Builder{}
 	for _, param := range params {
 		switch typedParam := param.(type) {
@@ -160,11 +171,4 @@ func (e *CachedEnforcer) getKey(params ...interface{}) (string, bool) {
 		key.WriteString("$$")
 	}
 	return key.String(), true
-}
-
-// InvalidateCache deletes all the existing cached decisions.
-func (e *CachedEnforcer) InvalidateCache() error {
-	e.locker.Lock()
-	defer e.locker.Unlock()
-	return e.cache.Clear()
-}
+}
\ No newline at end of file
diff --git a/enforcer_cached_synced.go b/enforcer_cached_synced.go
new file mode 100644
index 0000000..d1a324e
--- /dev/null
+++ b/enforcer_cached_synced.go
@@ -0,0 +1,180 @@
+// Copyright 2018 The casbin Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package casbin
+
+import (
+	"sync"
+	"sync/atomic"
+	"time"
+
+	"github.com/casbin/casbin/v2/persist/cache"
+)
+
+// SyncedCachedEnforcer wraps Enforcer and provides decision sync cache
+type SyncedCachedEnforcer struct {
+	*SyncedEnforcer
+	expireTime  time.Duration
+	cache       cache.Cache
+	enableCache int32
+	locker      *sync.RWMutex
+}
+
+// NewSyncedCachedEnforcer creates a sync cached enforcer via file or DB.
+func NewSyncedCachedEnforcer(params ...interface{}) (*SyncedCachedEnforcer, error) {
+	e := &SyncedCachedEnforcer{}
+	var err error
+	e.SyncedEnforcer, err = NewSyncedEnforcer(params...)
+	if err != nil {
+		return nil, err
+	}
+
+	e.enableCache = 1
+	e.cache, _ = cache.NewSyncCache()
+	e.locker = new(sync.RWMutex)
+	return e, nil
+}
+
+// EnableCache determines whether to enable cache on Enforce(). When enableCache is enabled, cached result (true | false) will be returned for previous decisions.
+func (e *SyncedCachedEnforcer) EnableCache(enableCache bool) {
+	var enabled int32
+	if enableCache {
+		enabled = 1
+	}
+	atomic.StoreInt32(&e.enableCache, enabled)
+}
+
+// Enforce decides whether a "subject" can access a "object" with the operation "action", input parameters are usually: (sub, obj, act).
+// if rvals is not string , ingore the cache
+func (e *SyncedCachedEnforcer) Enforce(rvals ...interface{}) (bool, error) {
+	if atomic.LoadInt32(&e.enableCache) == 0 {
+		return e.SyncedEnforcer.Enforce(rvals...)
+	}
+
+	key, ok := e.getKey(rvals...)
+	if !ok {
+		return e.SyncedEnforcer.Enforce(rvals...)
+	}
+
+	if res, err := e.getCachedResult(key); err == nil {
+		return res, nil
+	} else if err != cache.ErrNoSuchKey {
+		return res, err
+	}
+
+	res, err := e.SyncedEnforcer.Enforce(rvals...)
+	if err != nil {
+		return false, err
+	}
+
+	err = e.setCachedResult(key, res, e.expireTime)
+	return res, err
+}
+
+func (e *SyncedCachedEnforcer) LoadPolicy() error {
+	if atomic.LoadInt32(&e.enableCache) != 0 {
+		if err := e.cache.Clear(); err != nil {
+			return err
+		}
+	}
+	return e.SyncedEnforcer.LoadPolicy()
+}
+
+func (e *SyncedCachedEnforcer) AddPolicy(params ...interface{}) (bool, error) {
+	if ok, err := e.checkOneAndRemoveCache(params...); !ok {
+		return ok, err
+	}
+	return e.SyncedEnforcer.AddPolicy(params...)
+}
+
+func (e *SyncedCachedEnforcer) AddPolicies(rules [][]string) (bool, error) {
+	if ok, err := e.checkManyAndRemoveCache(rules); !ok {
+		return ok, err
+	}
+	return e.SyncedEnforcer.RemovePolicies(rules)
+}
+
+func (e *SyncedCachedEnforcer) RemovePolicy(params ...interface{}) (bool, error) {
+	if ok, err := e.checkOneAndRemoveCache(params...); !ok {
+		return ok, err
+	}
+	return e.SyncedEnforcer.RemovePolicy(params...)
+}
+
+func (e *SyncedCachedEnforcer) RemovePolicies(rules [][]string) (bool, error) {
+	if ok, err := e.checkManyAndRemoveCache(rules); !ok {
+		return ok, err
+	}
+	return e.SyncedEnforcer.RemovePolicies(rules)
+}
+
+func (e *SyncedCachedEnforcer) getCachedResult(key string) (res bool, err error) {
+	return e.cache.Get(key)
+}
+
+func (e *SyncedCachedEnforcer) SetExpireTime(expireTime time.Duration) {
+	e.locker.Lock()
+	defer e.locker.Unlock()
+	e.expireTime = expireTime
+}
+
+// SetCache need to be sync cache
+func (e *SyncedCachedEnforcer) SetCache(c cache.Cache) {
+	e.locker.Lock()
+	defer e.locker.Unlock()
+	e.cache = c
+}
+
+func (e *SyncedCachedEnforcer) setCachedResult(key string, res bool, extra ...interface{}) error {
+	return e.cache.Set(key, res, extra...)
+}
+
+func (e *SyncedCachedEnforcer) getKey(params ...interface{}) (string, bool) {
+	return GetCacheKey(params...)
+}
+
+// InvalidateCache deletes all the existing cached decisions.
+func (e *SyncedCachedEnforcer) InvalidateCache() error {
+	return e.cache.Clear()
+}
+
+func (e *SyncedCachedEnforcer) checkOneAndRemoveCache(params ...interface{}) (bool, error) {
+	if atomic.LoadInt32(&e.enableCache) != 0 {
+		key, ok := e.getKey(params...)
+		if ok {
+			if err := e.cache.Delete(key); err != nil && err != cache.ErrNoSuchKey {
+				return false, err
+			}
+		}
+	}
+	return true, nil
+}
+
+func (e *SyncedCachedEnforcer) checkManyAndRemoveCache(rules [][]string) (bool, error) {
+	if len(rules) != 0 {
+		if atomic.LoadInt32(&e.enableCache) != 0 {
+			irule := make([]interface{}, len(rules[0]))
+			for _, rule := range rules {
+				for i, param := range rule {
+					irule[i] = param
+				}
+				key, _ := e.getKey(irule...)
+				if err := e.cache.Delete(key); err != nil && err != cache.ErrNoSuchKey {
+					return false, err
+				}
+			}
+		}
+	}
+	return true, nil
+}
diff --git a/persist/cache/cache_sync.go b/persist/cache/cache_sync.go
new file mode 100644
index 0000000..d083a03
--- /dev/null
+++ b/persist/cache/cache_sync.go
@@ -0,0 +1,87 @@
+// Copyright 2021 The casbin Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cache
+
+import (
+	"sync"
+	"time"
+)
+
+type SyncCache struct {
+	cache DefaultCache
+	sync.RWMutex
+}
+
+func (c *SyncCache) Set(key string, value bool, extra ...interface{}) error {
+	ttl := time.Duration(-1)
+	if len(extra) > 0 {
+		ttl = extra[0].(time.Duration)
+	}
+	(*c).Lock()
+	defer (*c).Unlock()
+	(*c).cache[key] = cacheItem{
+		value:     value,
+		expiresAt: time.Now().Add(ttl),
+		ttl:       ttl,
+	}
+	return nil
+}
+
+func (c *SyncCache) Get(key string) (bool, error) {
+	(*c).RLock()
+	res, ok := (*c).cache[key]
+	(*c).RUnlock()
+	if !ok {
+		return false, ErrNoSuchKey
+	} else {
+		if res.ttl > 0 && time.Now().After(res.expiresAt) {
+			(*c).Lock()
+			defer (*c).Unlock()
+			delete((*c).cache, key)
+			return false, ErrNoSuchKey
+		}
+		return res.value, nil
+	}
+}
+
+func (c *SyncCache) Delete(key string) error {
+	(*c).RLock()
+	_, ok := (*c).cache[key]
+	(*c).RUnlock()
+	if !ok {
+		return ErrNoSuchKey
+	} else {
+		(*c).Lock()
+		defer (*c).Unlock()
+		delete((*c).cache, key)
+		return nil
+	}
+}
+
+func (c *SyncCache) Clear() error {
+	*c = SyncCache{
+		make(DefaultCache),
+		sync.RWMutex{},
+	}
+	return nil
+}
+
+func NewSyncCache() (Cache, error) {
+	cache := SyncCache{
+		make(DefaultCache),
+		sync.RWMutex{},
+	}
+	return &cache, nil
+}


Test patch:
diff --git a/enforcer_cached_synced_test.go b/enforcer_cached_synced_test.go
new file mode 100644
index 0000000..9738d8c
--- /dev/null
+++ b/enforcer_cached_synced_test.go
@@ -0,0 +1,74 @@
+// Copyright 2018 The casbin Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package casbin
+
+import (
+	"sync"
+	"testing"
+)
+
+func testSyncEnforceCache(t *testing.T, e *SyncedCachedEnforcer, sub string, obj interface{}, act string, res bool) {
+	t.Helper()
+	if myRes, _ := e.Enforce(sub, obj, act); myRes != res {
+		t.Errorf("%s, %v, %s: %t, supposed to be %t", sub, obj, act, myRes, res)
+	}
+}
+
+func TestSyncCache(t *testing.T) {
+	e, _ := NewSyncedCachedEnforcer("examples/basic_model.conf", "examples/basic_policy.csv")
+	// The cache is enabled by default for NewCachedEnforcer.
+	g := sync.WaitGroup{}
+	goThread := 1000
+	g.Add(goThread)
+	for i := 0; i < goThread; i++ {
+		go func() {
+			_, _ = e.AddPolicy("alice", "data2", "read")
+			testSyncEnforceCache(t, e, "alice", "data2", "read", true)
+			g.Done()
+		}()
+	}
+	g.Wait()
+	_, _ = e.RemovePolicy("alice", "data2", "read")
+
+	testSyncEnforceCache(t, e, "alice", "data1", "read", true)
+	testSyncEnforceCache(t, e, "alice", "data1", "write", false)
+	testSyncEnforceCache(t, e, "alice", "data2", "read", false)
+	testSyncEnforceCache(t, e, "alice", "data2", "write", false)
+	// The cache is enabled, calling RemovePolicy, LoadPolicy or RemovePolicies will
+	// also operate cached items.
+	_, _ = e.RemovePolicy("alice", "data1", "read")
+
+	testSyncEnforceCache(t, e, "alice", "data1", "read", false)
+	testSyncEnforceCache(t, e, "alice", "data1", "write", false)
+	testSyncEnforceCache(t, e, "alice", "data2", "read", false)
+	testSyncEnforceCache(t, e, "alice", "data2", "write", false)
+
+	e, _ = NewSyncedCachedEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
+
+	testSyncEnforceCache(t, e, "alice", "data1", "read", true)
+	testSyncEnforceCache(t, e, "bob", "data2", "write", true)
+	testSyncEnforceCache(t, e, "alice", "data2", "read", true)
+	testSyncEnforceCache(t, e, "alice", "data2", "write", true)
+
+	_, _ = e.RemovePolicies([][]string{
+		{"alice", "data1", "read"},
+		{"bob", "data2", "write"},
+	})
+
+	testSyncEnforceCache(t, e, "alice", "data1", "read", false)
+	testSyncEnforceCache(t, e, "bob", "data2", "write", false)
+	testSyncEnforceCache(t, e, "alice", "data2", "read", true)
+	testSyncEnforceCache(t, e, "alice", "data2", "write", true)
+}