Skip to content

fix: improve policyManager #636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nodece wants to merge 1 commit into from
Closed

fix: improve policyManager #636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions api/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// 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 api

// PolicyManager is the policy manager for model and adapter
type PolicyManager interface {
AddPolicies(sec string, ptype string, rules [][]string) ([][]string, error)
RemovePolicies(sec string, ptype string, rules [][]string) ([][]string, error)
RemoveFilteredPolicy(sec string, ptype string, fieldIndex int, fieldValues ...string) ([][]string, error)
}
15 changes: 8 additions & 7 deletions enforcer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"time"

"github.com/Knetic/govaluate"
"github.com/casbin/casbin/v3/api"
"github.com/casbin/casbin/v3/effect"
"github.com/casbin/casbin/v3/internal"
"github.com/casbin/casbin/v3/log"
Expand All @@ -45,7 +46,7 @@ type Enforcer struct {
dispatcher persist.Dispatcher
rm rbac.RoleManager

internal internal.PolicyManager
policyManager api.PolicyManager
enabled bool
autoSave bool
autoBuildRoleLinks bool
Expand Down Expand Up @@ -159,7 +160,7 @@ func (e *Enforcer) InitWithModelAndAdapter(m model.Model, adapter persist.Adapte
e.model.PrintModel()
e.fm = model.LoadFunctionMap()
e.initialize()
e.internal = internal.NewPolicyManager(m, adapter, e.rm)
e.policyManager = internal.NewPolicyManager(m, adapter, e.rm, e.shouldPersist)
// Do not initialize the full policy when using a filtered adapter
fa, ok := e.adapter.(persist.FilteredAdapter)
if e.adapter != nil && (!ok || ok && !fa.IsFiltered()) {
Expand Down Expand Up @@ -209,7 +210,7 @@ func (e *Enforcer) GetModel() model.Model {
func (e *Enforcer) SetModel(m model.Model) {
e.model = m
e.fm = model.LoadFunctionMap()
e.internal = internal.NewPolicyManager(m, e.adapter, e.rm)
e.policyManager = internal.NewPolicyManager(m, e.adapter, e.rm, e.shouldPersist)
e.initialize()
}

Expand All @@ -221,12 +222,12 @@ func (e *Enforcer) GetAdapter() persist.Adapter {
// SetAdapter sets the current adapter.
func (e *Enforcer) SetAdapter(adapter persist.Adapter) {
e.adapter = adapter
e.internal = internal.NewPolicyManager(e.model, adapter, e.rm)
e.policyManager = internal.NewPolicyManager(e.model, e.adapter, e.rm, e.shouldPersist)
}

// GetPolicyManager gets the current policy manager.
func (e *Enforcer) GetPolicyManager() internal.PolicyManager {
return e.internal
func (e *Enforcer) GetPolicyManager() api.PolicyManager {
return e.policyManager
}

// SetWatcher sets the current watcher.
Expand All @@ -249,7 +250,7 @@ func (e *Enforcer) GetRoleManager() rbac.RoleManager {
// SetRoleManager sets the current role manager.
func (e *Enforcer) SetRoleManager(rm rbac.RoleManager) {
e.rm = rm
e.internal = internal.NewPolicyManager(e.model, e.adapter, rm)
e.policyManager = internal.NewPolicyManager(e.model, e.adapter, rm, e.shouldPersist)
}

// SetEffector sets the current effector.
Expand Down
120 changes: 32 additions & 88 deletions internal/internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,77 +15,47 @@
package internal

import (
"github.com/casbin/casbin/v3/api"
"github.com/casbin/casbin/v3/errors"
"github.com/casbin/casbin/v3/model"
"github.com/casbin/casbin/v3/persist"
"github.com/casbin/casbin/v3/rbac"
)

// PolicyManager is the policy manager for model and adapter
type PolicyManager interface {
AddPolicy(sec string, ptype string, rule []string, shouldPersist bool) (bool, error)
AddPolicies(sec string, ptype string, rules [][]string, shouldPersist bool) (bool, [][]string, error)
RemovePolicy(sec string, ptype string, rule []string, shouldPersist bool) (bool, error)
RemovePolicies(sec string, ptype string, rules [][]string, shouldPersist bool) (bool, [][]string, error)
RemoveFilteredPolicy(sec string, ptype string, shouldPersist bool, fieldIndex int, fieldValues ...string) (bool, [][]string, error)
}
var _ api.PolicyManager = &policyManager{}

type policyManager struct {
model model.Model
adapter persist.Adapter
rm rbac.RoleManager
model model.Model
adapter persist.Adapter
rm rbac.RoleManager
shouldPersist func() bool
}

const (
notImplemented = "not implemented"
)

// NewPolicyManager is the constructor for PolicyManager
func NewPolicyManager(model model.Model, adapter persist.Adapter, rm rbac.RoleManager) PolicyManager {
func NewPolicyManager(model model.Model, adapter persist.Adapter, rm rbac.RoleManager, shouldPersist func() bool) api.PolicyManager {
return &policyManager{
model: model,
adapter: adapter,
rm: rm,
}
}

// AddPolicy adds a rule to model and adapter.
func (p *policyManager) AddPolicy(sec string, ptype string, rule []string, shouldPersist bool) (bool, error) {
if p.model.HasPolicy(sec, ptype, rule) {
return false, nil
}

if shouldPersist {
if err := p.adapter.AddPolicy(sec, ptype, rule); err != nil {
if err.Error() != notImplemented {
return false, err
}
}
}

p.model.AddPolicy(sec, ptype, rule)

if sec == "g" {
err := p.model.BuildIncrementalRoleLinks(p.rm, model.PolicyAdd, "g", ptype, [][]string{rule})
if err != nil {
return true, err
}
model: model,
adapter: adapter,
rm: rm,
shouldPersist: shouldPersist,
}

return true, nil
}

// AddPolicies adds rules to model and adapter.
func (p *policyManager) AddPolicies(sec string, ptype string, rules [][]string, shouldPersist bool) (bool, [][]string, error) {
rules = p.model.RemoveExistPolicy(sec, ptype, rules)
func (p *policyManager) AddPolicies(sec string, ptype string, rules [][]string) ([][]string, error) {
rules = p.model.FilterNotExistPolicy(sec, ptype, rules)
if len(rules) == 0 {
return true, nil, nil
return nil, nil
}

if shouldPersist {
if p.shouldPersist() {
if err := p.adapter.(persist.BatchAdapter).AddPolicies(sec, ptype, rules); err != nil {
if err.Error() != notImplemented {
return false, nil, err
return nil, err
}
}
}
Expand All @@ -95,93 +65,67 @@ func (p *policyManager) AddPolicies(sec string, ptype string, rules [][]string,
if sec == "g" {
err := p.model.BuildIncrementalRoleLinks(p.rm, model.PolicyAdd, "g", ptype, rules)
if err != nil {
return true, rules, err
}
}

return true, rules, nil
}

// RemovePolicy removes a rule from model and adapter.
func (p *policyManager) RemovePolicy(sec string, ptype string, rule []string, shouldPersist bool) (bool, error) {
if shouldPersist {
if err := p.adapter.RemovePolicy(sec, ptype, rule); err != nil {
if err.Error() != notImplemented {
return false, err
}
}
}

ruleRemoved := p.model.RemovePolicy(sec, ptype, rule)
if !ruleRemoved {
return ruleRemoved, nil
}

if sec == "g" {
err := p.model.BuildIncrementalRoleLinks(p.rm, model.PolicyRemove, "g", ptype, [][]string{rule})
if err != nil {
return ruleRemoved, err
return rules, err
}
}

return ruleRemoved, nil
return rules, nil
}

// RemovePolicies removes rules from model and adapter.
func (p *policyManager) RemovePolicies(sec string, ptype string, rules [][]string, shouldPersist bool) (bool, [][]string, error) {
rules = p.model.RemoveNotExistPolicy(sec, ptype, rules)
func (p *policyManager) RemovePolicies(sec string, ptype string, rules [][]string) ([][]string, error) {
if len(rules) == 0 {
return true, nil, nil
return nil, nil
}

if shouldPersist {
if p.shouldPersist() {
if err := p.adapter.(persist.BatchAdapter).RemovePolicies(sec, ptype, rules); err != nil {
if err.Error() != notImplemented {
return false, nil, err
return nil, err
}
}
}

rulesRemoved := p.model.RemovePolicies(sec, ptype, rules)
if !rulesRemoved {
return rulesRemoved, rules, nil
return rules, nil
}

if sec == "g" {
err := p.model.BuildIncrementalRoleLinks(p.rm, model.PolicyRemove, "g", ptype, rules)
if err != nil {
return rulesRemoved, rules, err
return rules, err
}
}

return rulesRemoved, rules, nil
return rules, nil
}

// RemoveFilteredPolicy removes rules based on field filters from model and adapter.
func (p *policyManager) RemoveFilteredPolicy(sec string, ptype string, shouldPersist bool, fieldIndex int, fieldValues ...string) (bool, [][]string, error) {
func (p *policyManager) RemoveFilteredPolicy(sec string, ptype string, fieldIndex int, fieldValues ...string) ([][]string, error) {
if len(fieldValues) == 0 {
return false, nil, errors.INVALID_FIELDVAULES_PARAMETER
return nil, errors.INVALID_FIELDVAULES_PARAMETER
}

if shouldPersist {
if p.shouldPersist() {
if err := p.adapter.RemoveFilteredPolicy(sec, ptype, fieldIndex, fieldValues...); err != nil {
if err.Error() != notImplemented {
return false, nil, err
return nil, err
}
}
}

ruleRemoved, effects := p.model.RemoveFilteredPolicy(sec, ptype, fieldIndex, fieldValues...)
if !ruleRemoved {
return ruleRemoved, effects, nil
return effects, nil
}

if sec == "g" {
err := p.model.BuildIncrementalRoleLinks(p.rm, model.PolicyRemove, "g", ptype, effects)
if err != nil {
return ruleRemoved, effects, err
return effects, err
}
}

return ruleRemoved, effects, nil
return effects, nil
}
41 changes: 21 additions & 20 deletions internal_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ func (e *Enforcer) addPolicy(sec string, ptype string, rule []string) (bool, err
return true, e.dispatcher.AddPolicies(sec, ptype, [][]string{rule})
}

if result, err := e.internal.AddPolicy(sec, ptype, rule, e.shouldPersist()); err != nil || !result {
return result, err
effects, err := e.policyManager.AddPolicies(sec, ptype, [][]string{rule})
if err != nil || len(effects) == 0 {
return false, err
}

if e.watcher != nil && e.autoNotifyWatcher {
Expand All @@ -57,9 +58,9 @@ func (e *Enforcer) addPolicies(sec string, ptype string, rules [][]string) (bool
return true, e.dispatcher.AddPolicies(sec, ptype, rules)
}

result, effects, err := e.internal.AddPolicies(sec, ptype, rules, e.shouldPersist())
if err != nil || !result {
return result, err
effects, err := e.policyManager.AddPolicies(sec, ptype, rules)
if err != nil || len(effects) == 0 {
return false, err
}

if e.watcher != nil && e.autoNotifyWatcher {
Expand All @@ -82,9 +83,9 @@ func (e *Enforcer) removePolicy(sec string, ptype string, rule []string) (bool,
return true, e.dispatcher.RemovePolicies(sec, ptype, [][]string{rule})
}

ruleRemoved, err := e.internal.RemovePolicy(sec, ptype, rule, e.shouldPersist())
if err != nil || !ruleRemoved {
return ruleRemoved, err
effects, err := e.policyManager.RemovePolicies(sec, ptype, [][]string{rule})
if err != nil || len(effects) == 0 {
return false, err
}

if e.watcher != nil && e.autoNotifyWatcher {
Expand All @@ -94,15 +95,15 @@ func (e *Enforcer) removePolicy(sec string, ptype string, rule []string) (bool,
} else {
err = e.watcher.Update()
}
return ruleRemoved, err
return true, err

}

if log.GetLogger().IsEnabled() {
log.LogPrintf("Policy Management, Type: RemovePolicy Assertion %s::%s\nrule: %s", sec, ptype, util.ArrayToString(rule))
}

return ruleRemoved, nil
return true, nil
}

// removePolicies removes rules from the current policy.
Expand All @@ -111,23 +112,23 @@ func (e *Enforcer) removePolicies(sec string, ptype string, rules [][]string) (b
return true, e.dispatcher.RemovePolicies(sec, ptype, rules)
}

rulesRemoved, effects, err := e.internal.RemovePolicies(sec, ptype, rules, e.shouldPersist())
if err != nil || !rulesRemoved {
return rulesRemoved, err
effects, err := e.policyManager.RemovePolicies(sec, ptype, rules)
if err != nil || len(effects) == 0 {
return false, err
}

if e.watcher != nil && e.autoNotifyWatcher {
err := e.watcher.Update()
if err != nil {
return rulesRemoved, err
return true, err
}
}

if log.GetLogger().IsEnabled() {
log.LogPrintf("Policy Management, Type: RemovePolicies Assertion %s::%s\nrules: %s", sec, ptype, util.Array2DToString(effects))
}

return rulesRemoved, nil
return true, nil
}

// removeFilteredPolicy removes rules based on field filters from the current policy.
Expand All @@ -136,9 +137,9 @@ func (e *Enforcer) removeFilteredPolicy(sec string, ptype string, fieldIndex int
return true, e.dispatcher.RemoveFilteredPolicy(sec, ptype, fieldIndex, fieldValues...)
}

ruleRemoved, effects, err := e.internal.RemoveFilteredPolicy(sec, ptype, e.shouldPersist(), fieldIndex, fieldValues...)
if err != nil || !ruleRemoved {
return ruleRemoved, err
effects, err := e.policyManager.RemoveFilteredPolicy(sec, ptype, fieldIndex, fieldValues...)
if err != nil || len(effects) == 0 {
return false, err
}

if e.watcher != nil && e.autoNotifyWatcher {
Expand All @@ -148,12 +149,12 @@ func (e *Enforcer) removeFilteredPolicy(sec string, ptype string, fieldIndex int
} else {
err = e.watcher.Update()
}
return ruleRemoved, err
return true, err
}

if log.GetLogger().IsEnabled() {
log.LogPrintf("Policy Management, Type: RemoveFilteredPolicy Assertion: %s::%s\nrules: %s", sec, ptype, util.Array2DToString(effects))
}

return ruleRemoved, nil
return true, nil
}
Loading