Repository: casbin/casbin
Pull request #592 (2020-09-08 20:25:09)

feat: New Feature UpdatePolicy
Fix: https://github.com/casbin/casbin/issues/584

I am not sure if there is anything missing in my code, please review carefully, thanks

hsluoyz: @00LT00 please remove all the "Merge xxx" commits:

![image](https://user-images.githubusercontent.com/3787410/92677862-4dba9800-f357-11ea-94b3-e8876cd81ab0.png)

Then fix the semantic PR:

![image](https://user-images.githubusercontent.com/3787410/92677883-59a65a00-f357-11ea-8c78-75c2e467a611.png)


00LT00: @hsluoyz Maybe this pr should be merge in master when the major version is updated?
hsluoyz: @nodece @GopherJ please review.
nodece: @hsluoyz The RP is breaking changes.
00LT00: ping @hsluoyz 
hsluoyz: @00LT00 commented.
hsluoyz: @00LT00 reviewed.

Patch:
diff --git a/enforcer.go b/enforcer.go
index a79cef3..12a9d15 100644
--- a/enforcer.go
+++ b/enforcer.go
@@ -248,7 +248,6 @@ func (e *Enforcer) LoadPolicy() error {
 		return err
 	}
 
-	e.model.PrintPolicy()
 	if e.autoBuildRoleLinks {
 		err := e.BuildRoleLinks()
 		if err != nil {
diff --git a/internal_api.go b/internal_api.go
index 3b9638e..cc453c5 100644
--- a/internal_api.go
+++ b/internal_api.go
@@ -133,6 +133,43 @@ func (e *Enforcer) removePolicy(sec string, ptype string, rule []string) (bool,
 	return ruleRemoved, nil
 }
 
+func (e *Enforcer) updatePolicy(sec string, ptype string, oldRule []string, newRule []string) (bool, error) {
+	if e.shouldPersist() {
+		if err := e.adapter.(persist.UpdatableAdapter).UpdatePolicy(sec, ptype, oldRule, newRule); err != nil {
+			if err.Error() != notImplemented {
+				return false, err
+			}
+		}
+	}
+	ruleUpdated := e.model.UpdatePolicy(sec, ptype, oldRule, newRule)
+	if !ruleUpdated {
+		return ruleUpdated, nil
+	}
+
+	if sec == "g" {
+		err := e.BuildIncrementalRoleLinks(model.PolicyRemove, ptype, [][]string{oldRule}) // remove the old rule
+		if err != nil {
+			return ruleUpdated, err
+		}
+		err = e.BuildIncrementalRoleLinks(model.PolicyAdd, ptype, [][]string{newRule}) // add the new rule
+		if err != nil {
+			return ruleUpdated, err
+		}
+	}
+
+	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
+}
+
 // removePolicies removes rules from the current policy.
 func (e *Enforcer) removePolicies(sec string, ptype string, rules [][]string) (bool, error) {
 	if !e.model.HasPolicies(sec, ptype, rules) {
diff --git a/management_api.go b/management_api.go
index 48c9810..85591d2 100644
--- a/management_api.go
+++ b/management_api.go
@@ -156,6 +156,15 @@ func (e *Enforcer) RemovePolicy(params ...interface{}) (bool, error) {
 	return e.RemoveNamedPolicy("p", params...)
 }
 
+// UpdatePolicy updates an authorization rule from the current policy.
+func (e *Enforcer) UpdatePolicy(rule1 []string, rule2 []string) (bool, error) {
+	return e.UpdateNamedPolicy("p", rule1, rule2)
+}
+
+func (e *Enforcer) UpdateNamedPolicy(ptype string, p1 []string, p2 []string) (bool, error) {
+	return e.updatePolicy("p", ptype, p1, p2)
+}
+
 // RemovePolicies removes authorization rules from the current policy.
 func (e *Enforcer) RemovePolicies(rules [][]string) (bool, error) {
 	return e.RemoveNamedPolicies("p", rules)
diff --git a/model/assertion.go b/model/assertion.go
index 2917318..49387d8 100644
--- a/model/assertion.go
+++ b/model/assertion.go
@@ -84,5 +84,6 @@ func (ast *Assertion) buildRoleLinks(rm rbac.RoleManager) error {
 	}
 
 	log.LogPrint("Role links for: " + ast.Key)
+
 	return ast.RM.PrintRoles()
 }
diff --git a/model/policy.go b/model/policy.go
index 53208e8..0cfc99d 100644
--- a/model/policy.go
+++ b/model/policy.go
@@ -43,6 +43,7 @@ func (model Model) BuildIncrementalRoleLinks(rm rbac.RoleManager, op PolicyOp, s
 
 // BuildRoleLinks initializes the roles in RBAC.
 func (model Model) BuildRoleLinks(rm rbac.RoleManager) error {
+	model.PrintPolicy()
 	for _, ast := range model["g"] {
 		err := ast.buildRoleLinks(rm)
 		if err != nil {
@@ -55,6 +56,9 @@ func (model Model) BuildRoleLinks(rm rbac.RoleManager) error {
 
 // PrintPolicy prints the policy to log.
 func (model Model) PrintPolicy() {
+	if !log.GetLogger().IsEnabled() {
+		return
+	}
 	log.LogPrint("Policy:")
 	for key, ast := range model["p"] {
 		log.LogPrint(key, ": ", ast.Value, ": ", ast.Policy)
@@ -156,6 +160,21 @@ func (model Model) RemovePolicy(sec string, ptype string, rule []string) bool {
 	return true
 }
 
+// UpdatePolicy updates a policy rule from the model.
+func (model Model) UpdatePolicy(sec string, ptype string, oldRule []string, newRule []string) bool {
+	oldPolicy := strings.Join(oldRule, DefaultSep)
+	index, ok := model[sec][ptype].PolicyMap[oldPolicy]
+	if !ok {
+		return false
+	}
+
+	model[sec][ptype].Policy[index] = newRule
+	delete(model[sec][ptype].PolicyMap, oldPolicy)
+	model[sec][ptype].PolicyMap[strings.Join(newRule, DefaultSep)] = index
+
+	return true
+}
+
 // RemovePolicies removes policy rules from the model.
 func (model Model) RemovePolicies(sec string, ptype string, rules [][]string) bool {
 	for _, rule := range rules {
diff --git a/persist/file-adapter/adapter.go b/persist/file-adapter/adapter.go
index 5b08d7a..8f243da 100644
--- a/persist/file-adapter/adapter.go
+++ b/persist/file-adapter/adapter.go
@@ -32,6 +32,10 @@ type Adapter struct {
 	filePath string
 }
 
+func (a *Adapter) UpdatePolicy(sec string, ptype string, oldRule, newPolicy []string) error {
+	return errors.New("not implemented")
+}
+
 // NewAdapter is the constructor for Adapter.
 func NewAdapter(filePath string) *Adapter {
 	return &Adapter{filePath: filePath}
diff --git a/persist/file-adapter/adapter_mock.go b/persist/file-adapter/adapter_mock.go
index b9c71d8..5512e35 100644
--- a/persist/file-adapter/adapter_mock.go
+++ b/persist/file-adapter/adapter_mock.go
@@ -104,6 +104,11 @@ func (a *AdapterMock) RemovePolicies(sec string, ptype string, rules [][]string)
 	return a.GetMockErr()
 }
 
+// UpdatePolicy removes a policy rule from the storage.
+func (a *AdapterMock) UpdatePolicy(sec string, ptype string, oldRule, newPolicy []string) error {
+	return a.GetMockErr()
+}
+
 // RemoveFilteredPolicy removes policy rules that match the filter from the storage.
 func (a *AdapterMock) RemoveFilteredPolicy(sec string, ptype string, fieldIndex int, fieldValues ...string) error {
 	return a.GetMockErr()
diff --git a/persist/update_adapter.go b/persist/update_adapter.go
new file mode 100644
index 0000000..fb0487b
--- /dev/null
+++ b/persist/update_adapter.go
@@ -0,0 +1,23 @@
+// Copyright 2020 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 persist
+
+// UpdatableAdapter is the interface for Casbin adapters with add update policy function.
+type UpdatableAdapter interface {
+	Adapter
+	// UpdatePolicy updates a policy rule from storage.
+	// This is part of the Auto-Save feature.
+	UpdatePolicy(sec string, ptype string, oldRule, newPolicy []string) error
+}
diff --git a/persist/watcher_update.go b/persist/watcher_update.go
new file mode 100644
index 0000000..c7dd216
--- /dev/null
+++ b/persist/watcher_update.go
@@ -0,0 +1,23 @@
+// Copyright 2020 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 persist
+
+// WatcherUpdatable is the strengthen for Casbin watchers.
+type WatcherUpdatable interface {
+	Watcher
+	// UPdateForUpdaticy calls the update callback of other instances to synchronize their policy.
+	// It is called after Enforcer.UpdatePolicy()
+	UpdateForUpdatePolicy(oldRule, newRule []string) error
+}


Test patch:
diff --git a/management_api_test.go b/management_api_test.go
index 40964c1..f5cb0d2 100644
--- a/management_api_test.go
+++ b/management_api_test.go
@@ -192,6 +192,10 @@ func TestModifyPolicyAPI(t *testing.T) {
 	_, _ = e.RemoveFilteredPolicy(1, "data2")
 
 	testGetPolicy(t, e, [][]string{{"eve", "data3", "read"}})
+
+	_, _ = e.UpdatePolicy([]string{"eve", "data3", "read"}, []string{"eve", "data3", "write"})
+
+	testGetPolicy(t, e, [][]string{{"eve", "data3", "write"}})
 }
 
 func TestModifyGroupingPolicyAPI(t *testing.T) {
diff --git a/watcher_update_test.go b/watcher_update_test.go
new file mode 100644
index 0000000..3909260
--- /dev/null
+++ b/watcher_update_test.go
@@ -0,0 +1,41 @@
+// Copyright 2020 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 (
+	"testing"
+)
+
+type SampleWatcherUpdatable struct {
+	SampleWatcher
+}
+
+func (w SampleWatcherUpdatable) UpdateForUpdatePolicy(params ...string) error {
+	return nil
+}
+
+func TestSetWatcherUpdatable(t *testing.T) {
+	e, _ := NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
+
+	sampleWatcherEx := SampleWatcherUpdatable{}
+	err := e.SetWatcher(sampleWatcherEx)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_ = e.SavePolicy()                                                                            // calls watcherEx.UpdateForSavePolicy()
+	_, _ = e.UpdatePolicy([]string{"admin", "data1", "read"}, []string{"admin", "data2", "read"}) // calls watcherEx.UpdateForUpdatePolicy()
+
+}