Revert "fix: lock less when syncing (casbin#988)"

This reverts commit 756b994.

enforcer.go

@@ -287,37 +287,46 @@ func (e *Enforcer) ClearPolicy() {
 
 // LoadPolicy reloads the policy from file/database.
 func (e *Enforcer) LoadPolicy() error {
-	newModel, err := e.prepareNewModel(e.model.Copy())
-	if err != nil {
-		return err
-	}
+	needToRebuild := false
+	newModel := e.model.Copy()
+	newModel.ClearPolicy()
 
-	if e.autoBuildRoleLinks {
-		if err := e.tryBuildingRoleLinksWithModel(newModel); err != nil {
-			return err
+	var err error
+	defer func() {
+		if err != nil {
+			if e.autoBuildRoleLinks && needToRebuild {
+				_ = e.BuildRoleLinks()
+			}
 		}
-	}
-	e.model = newModel
-	return nil
-}
-
-// prepareNewModel prepares a new model with the policy from file/database.
-func (e *Enforcer) prepareNewModel(newModel model.Model) (model.Model, error) {
-	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 = 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 nil, err
+	if err = newModel.SortPoliciesBySubjectHierarchy(); err != nil {
+		return err
 	}
 
-	if err := newModel.SortPoliciesByPriority(); err != nil {
-		return nil, err
+	if err = newModel.SortPoliciesByPriority(); err != nil {
+		return err
 	}
 
-	return newModel, nil
+	if e.autoBuildRoleLinks {
+		needToRebuild = true
+		for _, rm := range e.rmMap {
+			err := rm.Clear()
+			if err != nil {
+				return err
+			}
+		}
+		err = newModel.BuildRoleLinks(e.rmMap)
+		if err != nil {
+			return err
+		}
+	}
+	e.model = newModel
+	return nil
 }
 
 func (e *Enforcer) loadFilteredPolicy(filter interface{}) error {
@@ -435,30 +444,16 @@ func (e *Enforcer) EnableAutoBuildRoleLinks(autoBuildRoleLinks bool) {
 	e.autoBuildRoleLinks = autoBuildRoleLinks
 }
 
-// buildingRoleLinksWithModel attempts to build the role links for a new model
-func (e *Enforcer) buildingRoleLinksWithModel(newModel model.Model) error {
+// BuildRoleLinks manually rebuild the role inheritance relations.
+func (e *Enforcer) BuildRoleLinks() error {
 	for _, rm := range e.rmMap {
 		err := rm.Clear()
 		if err != nil {
 			return err
 		}
 	}
 
-	return newModel.BuildRoleLinks(e.rmMap)
-}
-
-// BuildRoleLinks manually rebuild the role inheritance relations.
-func (e *Enforcer) BuildRoleLinks() error {
-	return e.buildingRoleLinksWithModel(e.model)
-}
-
-// tryBuildingRoleLinksWithModel attempts to build the role links for a new model falls back to the existing model in case of errors
-func (e *Enforcer) tryBuildingRoleLinksWithModel(newModel model.Model) error {
-	if err := e.buildingRoleLinksWithModel(newModel); err != nil {
-		_ = e.buildingRoleLinksWithModel(e.model)
-		return err
-	}
-	return nil
+	return e.model.BuildRoleLinks(e.rmMap)
 }
 
 // BuildIncrementalRoleLinks provides incremental build the role inheritance relations.

enforcer_synced.go

@@ -110,23 +110,9 @@ func (e *SyncedEnforcer) ClearPolicy() {
 
 // LoadPolicy reloads the policy from file/database.
 func (e *SyncedEnforcer) LoadPolicy() error {
-	e.m.RLock()
-	cleanedNewModel := e.model.Copy()
-	e.m.RUnlock()
-	newModel, err := e.prepareNewModel(cleanedNewModel)
-	if err != nil {
-		return err
-	}
-
 	e.m.Lock()
 	defer e.m.Unlock()
-	if e.autoBuildRoleLinks {
-		if err := e.tryBuildingRoleLinksWithModel(newModel); err != nil {
-			return err
-		}
-	}
-	e.model = newModel
-	return nil
+	return e.Enforcer.LoadPolicy()
 }
 
 // LoadFilteredPolicy reloads a filtered policy from file/database.

enforcer_synced_test.go

@@ -40,15 +40,6 @@ func TestSync(t *testing.T) {
 	testEnforceSync(t, e, "bob", "data2", "read", false)
 	testEnforceSync(t, e, "bob", "data2", "write", true)
 
-	// Simulate a policy change
-	e.ClearPolicy()
-	testEnforceSync(t, e, "bob", "data2", "write", false)
-
-	// Wait for at least one sync
-	time.Sleep(time.Millisecond * 300)
-
-	testEnforceSync(t, e, "bob", "data2", "write", true)
-
 	// Stop the reloading policy periodically.
 	e.StopAutoLoadPolicy()
 }

go.mod

@@ -5,4 +5,4 @@ require (
 	github.com/golang/mock v1.4.4
 )
 
-go 1.13
+go 1.13