feat: refactor LoadPolicy, delete LoadPolicyFast (#1402)

* feat: cut down the lock range in `LoadPolicy`, delete outdated `LoadPolicyFast`

* fix: fix logic

enforcer.go

@@ -323,11 +323,39 @@ func (e *Enforcer) ClearPolicy() {
 
 // LoadPolicy reloads the policy from file/database.
 func (e *Enforcer) LoadPolicy() error {
-	needToRebuild := false
-	newModel := e.model.Copy()
+	newModel, err := e.loadPolicyFromAdapter(e.model)
+	if err != nil {
+		return err
+	}
+	err = e.applyModifiedModel(newModel)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+func (e *Enforcer) loadPolicyFromAdapter(baseModel model.Model) (model.Model, error) {
+	newModel := baseModel.Copy()
 	newModel.ClearPolicy()
 
+	if err := e.adapter.LoadPolicy(newModel); err != nil && err.Error() != "invalid file path, file path cannot be empty" {
+		return nil, err
+	}
+
+	if err := newModel.SortPoliciesBySubjectHierarchy(); err != nil {
+		return nil, err
+	}
+
+	if err := newModel.SortPoliciesByPriority(); err != nil {
+		return nil, err
+	}
+
+	return newModel, nil
+}
+
+func (e *Enforcer) applyModifiedModel(newModel model.Model) error {
 	var err error
+	needToRebuild := false
 	defer func() {
 		if err != nil {
 			if e.autoBuildRoleLinks && needToRebuild {
@@ -336,20 +364,9 @@ func (e *Enforcer) LoadPolicy() error {
 		}
 	}()
 
-	if err = e.adapter.LoadPolicy(newModel); err != nil && err.Error() != "invalid file path, file path cannot be empty" {
-		return err
-	}
-
-	if err = newModel.SortPoliciesBySubjectHierarchy(); err != nil {
-		return err
-	}
-
-	if err = newModel.SortPoliciesByPriority(); err != nil {
-		return err
-	}
-
 	if e.autoBuildRoleLinks {
 		needToRebuild = true
+
 		if err := e.rebuildRoleLinks(newModel); err != nil {
 			return err
 		}
@@ -358,6 +375,7 @@ func (e *Enforcer) LoadPolicy() error {
 			return err
 		}
 	}
+
 	e.model = newModel
 	e.invalidateMatcherMap()
 	return nil

enforcer_synced.go

@@ -22,8 +22,6 @@ import (
 	"github.com/casbin/govaluate"
 
 	"github.com/casbin/casbin/v2/persist"
-	"github.com/casbin/casbin/v2/rbac"
-	defaultrolemanager "github.com/casbin/casbin/v2/rbac/default-role-manager"
 )
 
 // SyncedEnforcer wraps Enforcer and provides synchronized access.
@@ -117,48 +115,18 @@ func (e *SyncedEnforcer) ClearPolicy() {
 
 // LoadPolicy reloads the policy from file/database.
 func (e *SyncedEnforcer) LoadPolicy() error {
-	e.m.Lock()
-	defer e.m.Unlock()
-	return e.Enforcer.LoadPolicy()
-}
-
-// LoadPolicyFast is not blocked when adapter calls LoadPolicy.
-func (e *SyncedEnforcer) LoadPolicyFast() error {
 	e.m.RLock()
-	newModel := e.model.Copy()
+	newModel, err := e.loadPolicyFromAdapter(e.model)
 	e.m.RUnlock()
-
-	newModel.ClearPolicy()
-	newRmMap := map[string]rbac.RoleManager{}
-	var err error
-
-	if err = e.adapter.LoadPolicy(newModel); err != nil && err.Error() != "invalid file path, file path cannot be empty" {
-		return err
-	}
-
-	if err = newModel.SortPoliciesBySubjectHierarchy(); err != nil {
+	if err != nil {
 		return err
 	}
-
-	if err = newModel.SortPoliciesByPriority(); err != nil {
+	e.m.Lock()
+	err = e.applyModifiedModel(newModel)
+	e.m.Unlock()
+	if err != nil {
 		return err
 	}
-
-	if e.autoBuildRoleLinks {
-		for ptype := range newModel["g"] {
-			newRmMap[ptype] = defaultrolemanager.NewRoleManager(10)
-		}
-		err = newModel.BuildRoleLinks(newRmMap)
-		if err != nil {
-			return err
-		}
-	}
-
-	// reduce the lock range
-	e.m.Lock()
-	defer e.m.Unlock()
-	e.model = newModel
-	e.rmMap = newRmMap
 	return nil
 }