Merge pull request #12 from basakest/FilteredAdapter

fix: refactor the logic of loadFilteredPolicy method

Author: leeqvip

src/Adapter.php

@@ -229,51 +229,47 @@ public function removeFilteredPolicy(string $sec, string $ptype, int $fieldIndex
      */
     public function loadFilteredPolicy(Model $model, $filter): void
     {
-        // if $filter is empty, load all policies
-        if (is_null($filter)) {
-            $this->loadPolicy($model);
-            return;
-        }
         // the basic sql
         $sql = 'SELECT ptype, v0, v1, v2, v3, v4, v5 FROM '.$this->casbinRuleTableName . ' WHERE ';
-        
-        if ($filter instanceof Filter) {
-            $type = '';
-            $filter = (array) $filter;
-            // choose which ptype to use
-            foreach($filter as $i => $v) {
-                if (!empty($v)) {
-                    array_unshift($filter[$i], $i);
-                    $type = $i;
-                    break;
-                }
-            }
-            $items = ['ptype', 'v0', 'v1', 'v2', 'v3', 'v4', 'v5'];
-            $temp = [];
-            $values = [];
-            foreach($items as $i => $item) {
-                if (isset($filter[$type][$i]) && !empty($filter[$type][$i])) {
-                    array_push($temp, $item . '=:' . $item);
-                    $values[$item] = $filter[$type][$i];
-                }
+
+        $bind = [];
+
+        if (is_string($filter)) {
+            $filter = str_replace(' ', '', $filter);
+            $filter = str_replace('\'', '', $filter);
+            $filter = explode('=', $filter);
+            $sql .= "$filter[0] = :{$filter[0]}";
+            $bind[$filter[0]] = $filter[1];
+        } else if ($filter instanceof Filter) {
+            foreach($filter->p as $k => $v) {
+                $where[] = $v . ' = :' . $v;
+                $bind[$v] = $filter->g[$k];
             }
-            $sql .= implode(' and ', $temp);
-            $rows = $this->connection->query($sql, $values);
-        } elseif (is_string($filter)) {
-            $sql .= $filter;
-            $rows = $this->connection->query($sql);
+            $where = implode(' AND ', $where);
+            $sql .= $where;
         } else if ($filter instanceof Closure) {
-            $filter($this->connection, $sql, $this->rows);
+            $where = '';
+            $filter($where);
+            $where = str_replace(' ', '', $where);
+            $where = str_replace('\'', '', $where);
+            $where = explode('=', $where);
+            $sql .= "$where[0] = :{$where[0]}";
+            $bind[$where[0]] = $where[1];
         } else {
             throw new InvalidFilterTypeException('invalid filter type');
         }
 
-        $rows = $rows ?? $this->rows;
+        $rows = $this->connection->query($sql, $bind);
         foreach($rows as $row) {
-            $line = implode(', ', $row);
-            $this->loadPolicyLine($line, $model);
+            $row = array_filter($row, function($value) { return !is_null($value) && $value !== ''; });
+            unset($row['id']);
+            $line = implode(', ', array_filter($row, function ($val) {
+                return '' != $val && !is_null($val);
+            }));
+            $this->loadPolicyLine(trim($line), $model);
         }
-        $this->filtered = true;
+
+        $this->setFiltered(true);
     }
 
     /**

tests/AdapterTest.php

@@ -9,6 +9,7 @@
 use PHPUnit\Framework\TestCase;
 use TechOne\Database\Manager;
 use Casbin\Persist\Adapters\Filter;
+use Casbin\Exceptions\InvalidFilterTypeException;
 
 class AdapterTest extends TestCase
 {
@@ -85,34 +86,42 @@ public function testLoadPolicy()
     public function testLoadFilteredPolicy()
     {
         $this->initConfig();
-        $adapter = DatabaseAdapter::newAdapter($this->config);
-        $adapter->setFiltered(true);
-        $e = $this->getEnforcerWithAdapter($adapter);
+        $this->initDb(DatabaseAdapter::newAdapter($this->config));
+        $e = $this->getEnforcer();
+        $e->clearPolicy();
         $this->assertEquals([], $e->getPolicy());
 
+        // invalid filter type
+        try {
+            $filter = ['alice', 'data1', 'read'];
+            $e->loadFilteredPolicy($filter);
+            $exception = InvalidFilterTypeException::class;
+            $this->fail("Expected exception $exception not thrown");
+        } catch (InvalidFilterTypeException $exception) {
+            $this->assertEquals("invalid filter type", $exception->getMessage());
+        }
+
         // string
         $filter = "v0 = 'bob'";
         $e->loadFilteredPolicy($filter);
         $this->assertEquals([
-            //
-            ['bob', 'data2', 'write', '', '', '']
+            ['bob', 'data2', 'write']
         ], $e->getPolicy());
 
         // Filter
-        $filter = new Filter(['', '', 'read']);
+        $filter = new Filter(['v2'], ['read']);
         $e->loadFilteredPolicy($filter);
         $this->assertEquals([
-            ['alice', 'data1', 'read', '', '', ''],
-            ['data2_admin', 'data2', 'read', '', '', ''],
+            ['alice', 'data1', 'read'],
+            ['data2_admin', 'data2', 'read'],
         ], $e->getPolicy());
 
-        // Closure
-        $e->loadFilteredPolicy(function ($connection, $sql, &$rows) {
-            $rows = $connection->query($sql . "v0 = 'alice'");
+        $e->loadFilteredPolicy(function (&$sql) {
+            $sql .= "v0 = 'alice'";
         });
 
         $this->assertEquals([
-            ['alice', 'data1', 'read', '', '', ''],
+            ['alice', 'data1', 'read'],
         ], $e->getPolicy());
     }