Repository: casbin/casbin
Pull request #1093 (2022-09-01 14:42:24)

feat: add management api for WatcherEx
fix https://github.com/casbin/casbin/issues/1092
part of: https://github.com/casbin/casbin/issues/999

casbin-bot:  @tangyang9464 please review

Patch:
diff --git a/enforcer_interface.go b/enforcer_interface.go
index 7f20f16..dba2464 100644
--- a/enforcer_interface.go
+++ b/enforcer_interface.go
@@ -136,6 +136,15 @@ type IEnforcer interface {
 
 	UpdateGroupingPolicy(oldRule []string, newRule []string) (bool, error)
 	UpdateGroupingPolicies(oldRules [][]string, newRules [][]string) (bool, error)
+
+	/* Management API with autoNotifyWatcher disabled */
+	SelfAddPolicy(sec string, ptype string, rule []string) (bool, error)
+	SelfAddPolicies(sec string, ptype string, rules [][]string) (bool, error)
+	SelfRemovePolicy(sec string, ptype string, rule []string) (bool, error)
+	SelfSelfRemovePolicies(sec string, ptype string, rules [][]string) (bool, error)
+	SelfRemoveFilteredPolicy(sec string, ptype string, fieldIndex int, fieldValues ...string) (bool, error)
+	SelfUpdatePolicy(sec string, ptype string, oldRule, newRule []string) (bool, error)
+	SelfUpdatePolicies(sec string, ptype string, oldRules, newRules [][]string) (bool, error)
 }
 
 var _ IDistributedEnforcer = &DistributedEnforcer{}
diff --git a/internal_api.go b/internal_api.go
index d0ab8f3..68ac9b3 100644
--- a/internal_api.go
+++ b/internal_api.go
@@ -30,8 +30,12 @@ func (e *Enforcer) shouldPersist() bool {
 	return e.adapter != nil && e.autoSave
 }
 
+func (e *Enforcer) shouldNotify() bool {
+	return e.watcher != nil && e.autoNotifyWatcher
+}
+
 // addPolicy adds a rule to the current policy.
-func (e *Enforcer) addPolicy(sec string, ptype string, rule []string) (bool, error) {
+func (e *Enforcer) addPolicyWithoutNotify(sec string, ptype string, rule []string) (bool, error) {
 	if e.dispatcher != nil && e.autoNotifyDispatcher {
 		return true, e.dispatcher.AddPolicies(sec, ptype, [][]string{rule})
 	}
@@ -57,21 +61,11 @@ func (e *Enforcer) addPolicy(sec string, ptype string, rule []string) (bool, err
 		}
 	}
 
-	if e.watcher != nil && e.autoNotifyWatcher {
-		var err error
-		if watcher, ok := e.watcher.(persist.WatcherEx); ok {
-			err = watcher.UpdateForAddPolicy(sec, ptype, rule...)
-		} else {
-			err = e.watcher.Update()
-		}
-		return true, err
-	}
-
 	return true, nil
 }
 
 // addPolicies adds rules to the current policy.
-func (e *Enforcer) addPolicies(sec string, ptype string, rules [][]string) (bool, error) {
+func (e *Enforcer) addPoliciesWithoutNotify(sec string, ptype string, rules [][]string) (bool, error) {
 	if e.dispatcher != nil && e.autoNotifyDispatcher {
 		return true, e.dispatcher.AddPolicies(sec, ptype, rules)
 	}
@@ -97,21 +91,11 @@ func (e *Enforcer) addPolicies(sec string, ptype string, rules [][]string) (bool
 		}
 	}
 
-	if e.watcher != nil && e.autoNotifyWatcher {
-		var err error
-		if watcher, ok := e.watcher.(persist.WatcherEx); ok {
-			err = watcher.UpdateForAddPolicies(sec, ptype, rules...)
-		} else {
-			err = e.watcher.Update()
-		}
-		return true, err
-	}
-
 	return true, nil
 }
 
 // removePolicy removes a rule from the current policy.
-func (e *Enforcer) removePolicy(sec string, ptype string, rule []string) (bool, error) {
+func (e *Enforcer) removePolicyWithoutNotify(sec string, ptype string, rule []string) (bool, error) {
 	if e.dispatcher != nil && e.autoNotifyDispatcher {
 		return true, e.dispatcher.RemovePolicies(sec, ptype, [][]string{rule})
 	}
@@ -136,21 +120,10 @@ func (e *Enforcer) removePolicy(sec string, ptype string, rule []string) (bool,
 		}
 	}
 
-	if e.watcher != nil && e.autoNotifyWatcher {
-		var err error
-		if watcher, ok := e.watcher.(persist.WatcherEx); ok {
-			err = watcher.UpdateForRemovePolicy(sec, ptype, rule...)
-		} else {
-			err = e.watcher.Update()
-		}
-		return ruleRemoved, err
-
-	}
-
 	return ruleRemoved, nil
 }
 
-func (e *Enforcer) updatePolicy(sec string, ptype string, oldRule []string, newRule []string) (bool, error) {
+func (e *Enforcer) updatePolicyWithoutNotify(sec string, ptype string, oldRule []string, newRule []string) (bool, error) {
 	if e.dispatcher != nil && e.autoNotifyDispatcher {
 		return true, e.dispatcher.UpdatePolicy(sec, ptype, oldRule, newRule)
 	}
@@ -178,20 +151,10 @@ func (e *Enforcer) updatePolicy(sec string, ptype string, oldRule []string, newR
 		}
 	}
 
-	if e.watcher != nil && e.autoNotifyWatcher {
-		var err error
-		if watcher, ok := e.watcher.(persist.WatcherUpdatable); ok {
-			err = watcher.UpdateForUpdatePolicy(oldRule, newRule)
-		} else {
-			err = e.watcher.Update()
-		}
-		return ruleUpdated, err
-	}
-
 	return ruleUpdated, nil
 }
 
-func (e *Enforcer) updatePolicies(sec string, ptype string, oldRules [][]string, newRules [][]string) (bool, error) {
+func (e *Enforcer) updatePoliciesWithoutNotify(sec string, ptype string, oldRules [][]string, newRules [][]string) (bool, error) {
 	if len(newRules) != len(oldRules) {
 		return false, fmt.Errorf("the length of oldRules should be equal to the length of newRules, but got the length of oldRules is %d, the length of newRules is %d", len(oldRules), len(newRules))
 	}
@@ -224,21 +187,11 @@ func (e *Enforcer) updatePolicies(sec string, ptype string, oldRules [][]string,
 		}
 	}
 
-	if e.watcher != nil && e.autoNotifyWatcher {
-		var err error
-		if watcher, ok := e.watcher.(persist.WatcherUpdatable); ok {
-			err = watcher.UpdateForUpdatePolicies(oldRules, newRules)
-		} else {
-			err = e.watcher.Update()
-		}
-		return ruleUpdated, err
-	}
-
 	return ruleUpdated, nil
 }
 
 // removePolicies removes rules from the current policy.
-func (e *Enforcer) removePolicies(sec string, ptype string, rules [][]string) (bool, error) {
+func (e *Enforcer) removePoliciesWithoutNotify(sec string, ptype string, rules [][]string) (bool, error) {
 	if !e.model.HasPolicies(sec, ptype, rules) {
 		return false, nil
 	}
@@ -266,22 +219,11 @@ func (e *Enforcer) removePolicies(sec string, ptype string, rules [][]string) (b
 			return rulesRemoved, err
 		}
 	}
-
-	if e.watcher != nil && e.autoNotifyWatcher {
-		var err error
-		if watcher, ok := e.watcher.(persist.WatcherEx); ok {
-			err = watcher.UpdateForRemovePolicies(sec, ptype, rules...)
-		} else {
-			err = e.watcher.Update()
-		}
-		return true, err
-	}
-
 	return rulesRemoved, nil
 }
 
 // removeFilteredPolicy removes rules based on field filters from the current policy.
-func (e *Enforcer) removeFilteredPolicy(sec string, ptype string, fieldIndex int, fieldValues ...string) (bool, error) {
+func (e *Enforcer) removeFilteredPolicyWithoutNotify(sec string, ptype string, fieldIndex int, fieldValues []string) (bool, error) {
 	if len(fieldValues) == 0 {
 		return false, Err.INVALID_FIELDVAULES_PARAMETER
 	}
@@ -309,20 +251,11 @@ func (e *Enforcer) removeFilteredPolicy(sec string, ptype string, fieldIndex int
 			return ruleRemoved, err
 		}
 	}
-	if e.watcher != nil && e.autoNotifyWatcher {
-		var err error
-		if watcher, ok := e.watcher.(persist.WatcherEx); ok {
-			err = watcher.UpdateForRemoveFilteredPolicy(sec, ptype, fieldIndex, fieldValues...)
-		} else {
-			err = e.watcher.Update()
-		}
-		return ruleRemoved, err
-	}
 
 	return ruleRemoved, nil
 }
 
-func (e *Enforcer) updateFilteredPolicies(sec string, ptype string, newRules [][]string, fieldIndex int, fieldValues ...string) (bool, error) {
+func (e *Enforcer) updateFilteredPoliciesWithoutNotify(sec string, ptype string, newRules [][]string, fieldIndex int, fieldValues ...string) ([][]string, error) {
 	var (
 		oldRules [][]string
 		err      error
@@ -331,7 +264,7 @@ func (e *Enforcer) updateFilteredPolicies(sec string, ptype string, newRules [][
 	if e.shouldPersist() {
 		if oldRules, err = e.adapter.(persist.UpdatableAdapter).UpdateFilteredPolicies(sec, ptype, newRules, fieldIndex, fieldValues...); err != nil {
 			if err.Error() != notImplemented {
-				return false, err
+				return nil, err
 			}
 		}
 		// For compatibility, because some adapters return oldRules containing ptype, see https://github.com/casbin/xorm-adapter/issues/49
@@ -343,38 +276,187 @@ func (e *Enforcer) updateFilteredPolicies(sec string, ptype string, newRules [][
 	}
 
 	if e.dispatcher != nil && e.autoNotifyDispatcher {
-		return true, e.dispatcher.UpdateFilteredPolicies(sec, ptype, oldRules, newRules)
+		return oldRules, e.dispatcher.UpdateFilteredPolicies(sec, ptype, oldRules, newRules)
 	}
 
 	ruleChanged := e.model.RemovePolicies(sec, ptype, oldRules)
 	e.model.AddPolicies(sec, ptype, newRules)
 	ruleChanged = ruleChanged && len(newRules) != 0
 	if !ruleChanged {
-		return ruleChanged, nil
+		return make([][]string, 0), nil
 	}
 
 	if sec == "g" {
 		err := e.BuildIncrementalRoleLinks(model.PolicyRemove, ptype, oldRules) // remove the old rules
 		if err != nil {
-			return ruleChanged, err
+			return oldRules, err
 		}
 		err = e.BuildIncrementalRoleLinks(model.PolicyAdd, ptype, newRules) // add the new rules
 		if err != nil {
-			return ruleChanged, err
+			return oldRules, err
 		}
 	}
 
-	if e.watcher != nil && e.autoNotifyWatcher {
+	return oldRules, nil
+}
+
+// addPolicy adds a rule to the current policy.
+func (e *Enforcer) addPolicy(sec string, ptype string, rule []string) (bool, error) {
+	ok, err := e.addPolicyWithoutNotify(sec, ptype, rule)
+	if !ok || err != nil {
+		return ok, err
+	}
+
+	if e.shouldNotify() {
+		var err error
+		if watcher, ok := e.watcher.(persist.WatcherEx); ok {
+			err = watcher.UpdateForAddPolicy(sec, ptype, rule...)
+		} else {
+			err = e.watcher.Update()
+		}
+		return true, err
+	}
+
+	return true, nil
+}
+
+// addPolicies adds rules to the current policy.
+func (e *Enforcer) addPolicies(sec string, ptype string, rules [][]string) (bool, error) {
+	ok, err := e.addPoliciesWithoutNotify(sec, ptype, rules)
+	if !ok || err != nil {
+		return ok, err
+	}
+
+	if e.shouldNotify() {
+		var err error
+		if watcher, ok := e.watcher.(persist.WatcherEx); ok {
+			err = watcher.UpdateForAddPolicies(sec, ptype, rules...)
+		} else {
+			err = e.watcher.Update()
+		}
+		return true, err
+	}
+
+	return true, nil
+}
+
+// removePolicy removes a rule from the current policy.
+func (e *Enforcer) removePolicy(sec string, ptype string, rule []string) (bool, error) {
+	ok, err := e.removePolicyWithoutNotify(sec, ptype, rule)
+	if !ok || err != nil {
+		return ok, err
+	}
+
+	if e.shouldNotify() {
+		var err error
+		if watcher, ok := e.watcher.(persist.WatcherEx); ok {
+			err = watcher.UpdateForRemovePolicy(sec, ptype, rule...)
+		} else {
+			err = e.watcher.Update()
+		}
+		return true, err
+
+	}
+
+	return true, nil
+}
+
+func (e *Enforcer) updatePolicy(sec string, ptype string, oldRule []string, newRule []string) (bool, error) {
+	ok, err := e.updatePolicyWithoutNotify(sec, ptype, oldRule, newRule)
+	if !ok || err != nil {
+		return ok, err
+	}
+
+	if e.shouldNotify() {
+		var err error
+		if watcher, ok := e.watcher.(persist.WatcherUpdatable); ok {
+			err = watcher.UpdateForUpdatePolicy(oldRule, newRule)
+		} else {
+			err = e.watcher.Update()
+		}
+		return true, err
+	}
+
+	return true, nil
+}
+
+func (e *Enforcer) updatePolicies(sec string, ptype string, oldRules [][]string, newRules [][]string) (bool, error) {
+	ok, err := e.updatePoliciesWithoutNotify(sec, ptype, oldRules, newRules)
+	if !ok || err != nil {
+		return ok, err
+	}
+
+	if e.shouldNotify() {
 		var err error
 		if watcher, ok := e.watcher.(persist.WatcherUpdatable); ok {
 			err = watcher.UpdateForUpdatePolicies(oldRules, newRules)
 		} else {
 			err = e.watcher.Update()
 		}
-		return ruleChanged, err
+		return true, err
 	}
 
-	return ruleChanged, nil
+	return true, nil
+}
+
+// removePolicies removes rules from the current policy.
+func (e *Enforcer) removePolicies(sec string, ptype string, rules [][]string) (bool, error) {
+	ok, err := e.removePoliciesWithoutNotify(sec, ptype, rules)
+	if !ok || err != nil {
+		return ok, err
+	}
+
+	if e.shouldNotify() {
+		var err error
+		if watcher, ok := e.watcher.(persist.WatcherEx); ok {
+			err = watcher.UpdateForRemovePolicies(sec, ptype, rules...)
+		} else {
+			err = e.watcher.Update()
+		}
+		return true, err
+	}
+
+	return true, nil
+}
+
+// removeFilteredPolicy removes rules based on field filters from the current policy.
+func (e *Enforcer) removeFilteredPolicy(sec string, ptype string, fieldIndex int, fieldValues []string) (bool, error) {
+	ok, err := e.removeFilteredPolicyWithoutNotify(sec, ptype, fieldIndex, fieldValues)
+	if !ok || err != nil {
+		return ok, err
+	}
+
+	if e.shouldNotify() {
+		var err error
+		if watcher, ok := e.watcher.(persist.WatcherEx); ok {
+			err = watcher.UpdateForRemoveFilteredPolicy(sec, ptype, fieldIndex, fieldValues...)
+		} else {
+			err = e.watcher.Update()
+		}
+		return true, err
+	}
+
+	return true, nil
+}
+
+func (e *Enforcer) updateFilteredPolicies(sec string, ptype string, newRules [][]string, fieldIndex int, fieldValues ...string) (bool, error) {
+	oldRules, err := e.updateFilteredPoliciesWithoutNotify(sec, ptype, newRules, fieldIndex, fieldValues...)
+	ok := len(oldRules) != 0
+	if !ok || err != nil {
+		return ok, err
+	}
+
+	if e.shouldNotify() {
+		var err error
+		if watcher, ok := e.watcher.(persist.WatcherUpdatable); ok {
+			err = watcher.UpdateForUpdatePolicies(oldRules, newRules)
+		} else {
+			err = e.watcher.Update()
+		}
+		return true, err
+	}
+
+	return true, nil
 }
 
 func (e *Enforcer) GetFieldIndex(ptype string, field string) (int, error) {
diff --git a/management_api.go b/management_api.go
index a8456b7..c38d4c3 100644
--- a/management_api.go
+++ b/management_api.go
@@ -291,7 +291,7 @@ func (e *Enforcer) RemoveNamedPolicies(ptype string, rules [][]string) (bool, er
 
 // RemoveFilteredNamedPolicy removes an authorization rule from the current named policy, field filters can be specified.
 func (e *Enforcer) RemoveFilteredNamedPolicy(ptype string, fieldIndex int, fieldValues ...string) (bool, error) {
-	return e.removeFilteredPolicy("p", ptype, fieldIndex, fieldValues...)
+	return e.removeFilteredPolicy("p", ptype, fieldIndex, fieldValues)
 }
 
 // HasGroupingPolicy determines whether a role inheritance rule exists.
@@ -411,10 +411,38 @@ func (e *Enforcer) UpdateNamedGroupingPolicies(ptype string, oldRules [][]string
 
 // RemoveFilteredNamedGroupingPolicy removes a role inheritance rule from the current named policy, field filters can be specified.
 func (e *Enforcer) RemoveFilteredNamedGroupingPolicy(ptype string, fieldIndex int, fieldValues ...string) (bool, error) {
-	return e.removeFilteredPolicy("g", ptype, fieldIndex, fieldValues...)
+	return e.removeFilteredPolicy("g", ptype, fieldIndex, fieldValues)
 }
 
 // AddFunction adds a customized function.
 func (e *Enforcer) AddFunction(name string, function govaluate.ExpressionFunction) {
 	e.fm.AddFunction(name, function)
 }
+
+func (e *Enforcer) SelfAddPolicy(sec string, ptype string, rule []string) (bool, error) {
+	return e.addPolicyWithoutNotify(sec, ptype, rule)
+}
+
+func (e *Enforcer) SelfAddPolicies(sec string, ptype string, rules [][]string) (bool, error) {
+	return e.addPoliciesWithoutNotify(sec, ptype, rules)
+}
+
+func (e *Enforcer) SelfRemovePolicy(sec string, ptype string, rule []string) (bool, error) {
+	return e.removePolicyWithoutNotify(sec, ptype, rule)
+}
+
+func (e *Enforcer) SelfSelfRemovePolicies(sec string, ptype string, rules [][]string) (bool, error) {
+	return e.removePoliciesWithoutNotify(sec, ptype, rules)
+}
+
+func (e *Enforcer) SelfRemoveFilteredPolicy(sec string, ptype string, fieldIndex int, fieldValues ...string) (bool, error) {
+	return e.removeFilteredPolicyWithoutNotify(sec, ptype, fieldIndex, fieldValues)
+}
+
+func (e *Enforcer) SelfUpdatePolicy(sec string, ptype string, oldRule, newRule []string) (bool, error) {
+	return e.updatePolicyWithoutNotify(sec, ptype, oldRule, newRule)
+}
+
+func (e *Enforcer) SelfUpdatePolicies(sec string, ptype string, oldRules, newRules [][]string) (bool, error) {
+	return e.updatePoliciesWithoutNotify(sec, ptype, oldRules, newRules)
+}
diff --git a/persist/watcher_ex.go b/persist/watcher_ex.go
index c928224..1c6f429 100644
--- a/persist/watcher_ex.go
+++ b/persist/watcher_ex.go
@@ -16,7 +16,7 @@ package persist
 
 import "github.com/casbin/casbin/v2/model"
 
-// WatcherEx is the strengthen for Casbin watchers.
+// WatcherEx is the strengthened Casbin watchers.
 type WatcherEx interface {
 	Watcher
 	// UpdateForAddPolicy calls the update callback of other instances to synchronize their policy.


Test patch:
diff --git a/watcher_ex_test.go b/watcher_ex_test.go
index 66596d0..b33df0f 100644
--- a/watcher_ex_test.go
+++ b/watcher_ex_test.go
@@ -50,7 +50,7 @@ func (w SampleWatcherEx) UpdateForRemovePolicies(sec string, ptype string, rules
 func TestSetWatcherEx(t *testing.T) {
 	e, _ := NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
 
-	sampleWatcherEx := SampleWatcherEx{}
+	sampleWatcherEx := &SampleWatcherEx{}
 	err := e.SetWatcher(sampleWatcherEx)
 	if err != nil {
 		t.Fatal(err)
diff --git a/watcher_test.go b/watcher_test.go
index 783cc59..9edb792 100644
--- a/watcher_test.go
+++ b/watcher_test.go
@@ -17,16 +17,21 @@ package casbin
 import "testing"
 
 type SampleWatcher struct {
+	callback func(string)
 }
 
-func (w SampleWatcher) Close() {
+func (w *SampleWatcher) Close() {
 }
 
-func (w SampleWatcher) SetUpdateCallback(func(string)) error {
+func (w *SampleWatcher) SetUpdateCallback(callback func(string)) error {
+	w.callback = callback
 	return nil
 }
 
-func (w SampleWatcher) Update() error {
+func (w *SampleWatcher) Update() error {
+	if w.callback != nil {
+		w.callback("")
+	}
 	return nil
 }
 
@@ -35,7 +40,7 @@ func TestSetWatcher(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	sampleWatcher := SampleWatcher{}
+	sampleWatcher := &SampleWatcher{}
 	err = e.SetWatcher(sampleWatcher)
 	if err != nil {
 		t.Fatal(err)
@@ -45,3 +50,42 @@ func TestSetWatcher(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestSelfModify(t *testing.T) {
+	e, err := NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	sampleWatcher := &SampleWatcher{}
+	err = e.SetWatcher(sampleWatcher)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var called int
+
+	called = -1
+	_ = e.watcher.SetUpdateCallback(func(s string) {
+		called = 1
+	})
+	_, err = e.AddPolicy("eva", "data", "read") //calls watcher.Update()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if called != 1 {
+		t.Fatal("callback should be called")
+	}
+
+	called = -1
+	_ = e.watcher.SetUpdateCallback(func(s string) {
+		called = 1
+	})
+	_, err = e.SelfAddPolicy("p", "p", []string{"eva", "data", "write"}) //calls watcher.Update()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if called != -1 {
+		t.Fatal("callback should not be called")
+	}
+}
diff --git a/watcher_update_test.go b/watcher_update_test.go
index 3909260..953a2fb 100644
--- a/watcher_update_test.go
+++ b/watcher_update_test.go
@@ -29,7 +29,7 @@ func (w SampleWatcherUpdatable) UpdateForUpdatePolicy(params ...string) error {
 func TestSetWatcherUpdatable(t *testing.T) {
 	e, _ := NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
 
-	sampleWatcherEx := SampleWatcherUpdatable{}
+	sampleWatcherEx := &SampleWatcherUpdatable{}
 	err := e.SetWatcher(sampleWatcherEx)
 	if err != nil {
 		t.Fatal(err)