diff --git a/enforcer.go b/enforcer.go index 91e4c420e..42328d3d3 100644 --- a/enforcer.go +++ b/enforcer.go @@ -281,10 +281,38 @@ func (e *Enforcer) ClearPolicy() { // LoadPolicy reloads the policy from file/database. func (e *Enforcer) LoadPolicy() error { - needToRebuild := false + newModel, err := e.prepareNewModel() + if err != nil { + return err + } + + return e.setNewModel(newModel) +} + +// prepareNewModel prepares a new model with the policy from file/database. +func (e *Enforcer) prepareNewModel() (model.Model, error) { newModel := e.model.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 +} + +// setNewModel replaces the current model with a new one +func (e *Enforcer) setNewModel(newModel model.Model) error { + needToRebuild := false + var err error defer func() { if err != nil { @@ -294,18 +322,6 @@ 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 for _, rm := range e.rmMap { diff --git a/enforcer_synced.go b/enforcer_synced.go index 9bd192ac5..07fc37b61 100644 --- a/enforcer_synced.go +++ b/enforcer_synced.go @@ -69,7 +69,7 @@ func (e *SyncedEnforcer) StartAutoLoadPolicy(d time.Duration) { select { case <-ticker.C: // error intentionally ignored - _ = e.LoadPolicy() + _ = e.reloadPolicy() // Uncomment this line to see when the policy is loaded. // log.Print("Load policy for time: ", n) n++ @@ -116,6 +116,18 @@ func (e *SyncedEnforcer) LoadPolicy() error { return e.Enforcer.LoadPolicy() } +// reloadPolicy reloads the policy from file/database, but only locks when replacing the model +func (e *SyncedEnforcer) reloadPolicy() error { + newModel, err := e.prepareNewModel() + if err != nil { + return err + } + + e.m.Lock() + defer e.m.Unlock() + return e.setNewModel(newModel) +} + // LoadFilteredPolicy reloads a filtered policy from file/database. func (e *SyncedEnforcer) LoadFilteredPolicy(filter interface{}) error { e.m.Lock() diff --git a/enforcer_synced_test.go b/enforcer_synced_test.go index 4377869b9..9576632a3 100644 --- a/enforcer_synced_test.go +++ b/enforcer_synced_test.go @@ -40,6 +40,15 @@ 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() }