fix: lost model data if loadPolicy failed

fix: lost model data if loadPolicy failed fix: lost model data if loadPolicy failed
php-casbin · Sep 22, 2021 · 463080c · 463080c
1 parent 1eb5fd0
commit 463080c
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 9 deletions.
diff --git a/src/CoreEnforcer.php b/src/CoreEnforcer.php
@@ -354,18 +354,35 @@ public function clearPolicy(): void
      */
     public function loadPolicy(): void
     {
-        $this->model->clearPolicy();
+        $flag = false;
+        $needToRebuild = false;
+        $newModel = clone $this->model;
+        $newModel->clearPolicy();
 
         try {
-            $this->adapter->loadPolicy($this->model);
-        } catch (InvalidFilePathException $e) {
-            //throw $e;
-        }
+            $this->adapter->loadPolicy($newModel);
+            $newModel->printPolicy();
+            $newModel->sortPoliciesByPriority();
 
-        $this->model->printPolicy();
-        $this->model->sortPoliciesByPriority();
-        if ($this->autoBuildRoleLinks) {
-            $this->buildRoleLinks();
+            if ($this->autoBuildRoleLinks) {
+                $needToRebuild = true;
+                foreach ($this->rmMap as $rm) {
+                    $rm->clear();
+                }
+                $newModel->buildRoleLinks($this->rmMap);
+            }
+            $this->model = $newModel;
+        } catch (InvalidFilePathException $e) {
+            // Ignore throw $e;
+        } catch (\Throwable $e) {
+            $flag = true;
+            throw $e;
+        } finally {
+            if ($flag) {
+                if ($this->autoBuildRoleLinks && $needToRebuild) {
+                    $this->buildRoleLinks();
+                }
+            }
         }
     }
 

diff --git a/src/Model/Model.php b/src/Model/Model.php
@@ -34,6 +34,18 @@ public function __construct()
     {
     }
 
+    public function __clone()
+    {
+        $this->sectionNameMap = $this->sectionNameMap;
+        $newAstMap = [];
+        foreach ($this->items as $ptype => $ast) {
+            foreach ($ast as $i => $v) {
+                $newAstMap[$ptype][$i] = clone $v;
+            }
+        }
+        $this->items = $newAstMap;
+    }
+
     /**
      * @param ConfigContract $cfg
      * @param string $sec

diff --git a/tests/EnforcerTest.php b/tests/EnforcerTest.php
@@ -33,6 +33,8 @@ public function testKeyMatchModelInMemory()
         $this->assertFalse($e->enforce('bob', '/alice_data/resource1', 'GET'));
 
         $e = new Enforcer($m);
+        $a->loadPolicy($e->getModel());
+
         $this->assertTrue($e->enforce('alice', '/alice_data/resource1', 'GET'));
         $this->assertFalse($e->enforce('bob', '/alice_data/resource1', 'GET'));
     }

diff --git a/tests/Unit/EnforcerTest.php b/tests/Unit/EnforcerTest.php
@@ -6,6 +6,7 @@
 use Casbin\Util\BuiltinOperations;
 use PHPUnit\Framework\TestCase;
 use Casbin\Enforcer;
+use Casbin\Persist\Adapters\FileAdapter;
 
 /**
  * EnforcerTest.
@@ -345,4 +346,31 @@ public function testGetPermissionsForUserInDomain()
         $this->assertEquals($e->getPermissionsForUserInDomain('admin', 'domain2'), [['admin', 'domain2', 'data2', 'read'], ['admin', 'domain2', 'data2', 'write']]);
         $this->assertEquals($e->getPermissionsForUserInDomain('non_exist', 'domain2'), []);
     }
+
+    public function testFailedToLoadPolicy()
+    {
+        $e = new Enforcer($this->modelAndPolicyPath . '/rbac_with_pattern_model.conf', $this->modelAndPolicyPath . '/rbac_with_pattern_policy.csv');
+        $e->addNamedMatchingFunc('g2', 'matchingFunc', function (string $key1, string $key2) {
+            return BuiltinOperations::keyMatch2($key1, $key2);
+        });
+        $this->assertTrue($e->enforce('alice', '/pen/1', 'GET'));
+        $this->assertTrue($e->enforce('alice', '/pen2/1', 'GET'));
+        $e->setAdapter(new FileAdapter('not found'));
+        $e->loadPolicy();
+        $this->assertTrue($e->enforce('alice', '/pen/1', 'GET'));
+        $this->assertTrue($e->enforce('alice', '/pen2/1', 'GET'));
+    }
+
+    public function testReloadPolicyWithFunc()
+    {
+        $e = new Enforcer($this->modelAndPolicyPath . '/rbac_with_pattern_model.conf', $this->modelAndPolicyPath . '/rbac_with_pattern_policy.csv');
+        $e->addNamedMatchingFunc('g2', 'matchingFunc', function (string $key1, string $key2) {
+            return BuiltinOperations::keyMatch2($key1, $key2);
+        });
+        $this->assertTrue($e->enforce('alice', '/pen/1', 'GET'));
+        $this->assertTrue($e->enforce('alice', '/pen2/1', 'GET'));
+        $e->loadPolicy();
+        $this->assertTrue($e->enforce('alice', '/pen/1', 'GET'));
+        $this->assertTrue($e->enforce('alice', '/pen2/1', 'GET'));
+    }
 }