Improve depthFirst() with integer keys

Co-authored-by: Amir Rami <[email protected]>
staudenmeir · Jan 4, 2022 · 1e27800 · 1e27800
1 parent 747c034
commit 1e27800
Show file tree

Hide file tree

Showing 13 changed files with 212 additions and 9 deletions.
diff --git a/src/Eloquent/Builder.php b/src/Eloquent/Builder.php
@@ -4,7 +4,10 @@
 
 use Illuminate\Database\Eloquent\Builder as Base;
 use Illuminate\Database\PostgresConnection;
+use Illuminate\Support\Str;
+use PDO;
 use RuntimeException;
+use Staudenmeir\LaravelAdjacencyList\Query\Grammars\MariaDbGrammar;
 use Staudenmeir\LaravelAdjacencyList\Query\Grammars\MySqlGrammar;
 use Staudenmeir\LaravelAdjacencyList\Query\Grammars\PostgresGrammar;
 use Staudenmeir\LaravelAdjacencyList\Query\Grammars\SQLiteGrammar;
@@ -79,13 +82,25 @@ public function getExpressionGrammar()
 
         switch ($driver) {
             case 'mysql':
-                return $this->query->getConnection()->withTablePrefix(new MySqlGrammar());
+                $version = $this->query->getConnection()->getReadPdo()->getAttribute(PDO::ATTR_SERVER_VERSION);
+
+                $grammar = Str::contains($version, 'MariaDB')
+                    ? new MariaDbGrammar($this->model)
+                    : new MySqlGrammar($this->model);
+
+                return $this->query->getConnection()->withTablePrefix($grammar);
             case 'pgsql':
-                return $this->query->getConnection()->withTablePrefix(new PostgresGrammar());
+                return $this->query->getConnection()->withTablePrefix(
+                    new PostgresGrammar($this->model)
+                );
             case 'sqlite':
-                return $this->query->getConnection()->withTablePrefix(new SQLiteGrammar());
+                return $this->query->getConnection()->withTablePrefix(
+                    new SQLiteGrammar($this->model)
+                );
             case 'sqlsrv':
-                return $this->query->getConnection()->withTablePrefix(new SqlServerGrammar());
+                return $this->query->getConnection()->withTablePrefix(
+                    new SqlServerGrammar($this->model)
+                );
         }
 
         throw new RuntimeException('This database is not supported.'); // @codeCoverageIgnore

diff --git a/src/Eloquent/HasRecursiveRelationshipScopes.php b/src/Eloquent/HasRecursiveRelationshipScopes.php
@@ -123,7 +123,9 @@ public function scopeBreadthFirst(Builder $query)
      */
     public function scopeDepthFirst(Builder $query)
     {
-        return $query->orderBy($this->getPathName());
+        $sql = $query->getExpressionGrammar()->compileOrderByPath();
+
+        return $query->orderByRaw($sql);
     }
 
     /**

diff --git a/src/Eloquent/HasRecursiveRelationships.php b/src/Eloquent/HasRecursiveRelationships.php
@@ -371,6 +371,19 @@ public function hasNestedPath()
         return Str::contains($path, $this->getPathSeparator());
     }
 
+    /**
+     * Determine if an attribute is an integer.
+     *
+     * @param string $attribute
+     * @return bool
+     */
+    public function isIntegerAttribute($attribute)
+    {
+        $casts = $this->getCasts();
+
+        return isset($casts[$attribute]) && in_array($casts[$attribute], ['int', 'integer']);
+    }
+
     /**
      * Create a new Eloquent query builder for the model.
      *

diff --git a/src/Query/Grammars/ExpressionGrammar.php b/src/Query/Grammars/ExpressionGrammar.php
@@ -43,4 +43,11 @@ public function getRecursivePathBindings($separator);
      * @return \Illuminate\Database\Query\Builder
      */
     public function selectPathList(Builder $query, $expression, $column, $pathSeparator, $listSeparator);
+
+    /**
+     * Compile an "order by path" clause.
+     *
+     * @return string
+     */
+    public function compileOrderByPath();
 }
diff --git a/src/Query/Grammars/MariaDbGrammar.php b/src/Query/Grammars/MariaDbGrammar.php
@@ -0,0 +1,38 @@
+<?php
+
+namespace Staudenmeir\LaravelAdjacencyList\Query\Grammars;
+
+class MariaDbGrammar extends MySqlGrammar
+{
+    /**
+     * Compile an "order by path" clause.
+     *
+     * @return string
+     */
+    public function compileOrderByPath()
+    {
+        $column = $this->model->getLocalKeyName();
+
+        $path = $this->wrap(
+            $this->model->getPathName()
+        );
+
+        $pathSeparator = $this->model->getPathSeparator();
+
+        if (!$this->model->isIntegerAttribute($column)) {
+            return "$path asc";
+        }
+
+        return <<<SQL
+regexp_replace(
+    regexp_replace(
+        $path,
+        '(^|[$pathSeparator])(\\\\d+)',
+        '\\\\100000000000000000000\\\\2'
+    ),
+    '0+(\\\\d\{20\})([$pathSeparator]|$)',
+    '\\\\1\\\\2'
+) asc
+SQL;
+    }
+}
diff --git a/src/Query/Grammars/MySqlGrammar.php b/src/Query/Grammars/MySqlGrammar.php
@@ -7,6 +7,8 @@
 
 class MySqlGrammar extends Base implements ExpressionGrammar
 {
+    use OrdersByPath;
+
     /**
      * Compile an initial path.
      *
@@ -16,7 +18,7 @@ class MySqlGrammar extends Base implements ExpressionGrammar
      */
     public function compileInitialPath($column, $alias)
     {
-        return 'cast('.$this->wrap($column).' as char(65535)) as '.$this->wrap($alias);
+        return 'cast(' . $this->wrap($column) . ' as char(65535)) as ' . $this->wrap($alias);
     }
 
     /**
@@ -28,7 +30,7 @@ public function compileInitialPath($column, $alias)
      */
     public function compileRecursivePath($column, $alias)
     {
-        return 'concat('.$this->wrap($alias).', ?, '.$this->wrap($column).')';
+        return 'concat(' . $this->wrap($alias) . ', ?, ' . $this->wrap($column) . ')';
     }
 
     /**
@@ -55,7 +57,39 @@ public function getRecursivePathBindings($separator)
     public function selectPathList(Builder $query, $expression, $column, $pathSeparator, $listSeparator)
     {
         return $query->selectRaw(
-            'group_concat('.$this->wrap($column)." separator '$listSeparator')"
+            'group_concat(' . $this->wrap($column) . " separator '$listSeparator')"
         )->from($expression);
     }
+
+    /**
+     * Compile an "order by path" clause.
+     *
+     * @return string
+     */
+    public function compileOrderByPath()
+    {
+        $column = $this->model->getLocalKeyName();
+
+        $path = $this->wrap(
+            $this->model->getPathName()
+        );
+
+        $pathSeparator = $this->model->getPathSeparator();
+
+        if (!$this->model->isIntegerAttribute($column)) {
+            return "$path asc";
+        }
+
+        return <<<SQL
+regexp_replace(
+    regexp_replace(
+        $path,
+        '(^|[$pathSeparator])(\\\\d+)',
+        '$100000000000000000000$2'
+    ),
+    '0+(\\\\d\{20\})([$pathSeparator]|$)',
+    '$1$2'
+) asc
+SQL;
+    }
 }
diff --git a/src/Query/Grammars/OrdersByPath.php b/src/Query/Grammars/OrdersByPath.php
@@ -0,0 +1,27 @@
+<?php
+
+namespace Staudenmeir\LaravelAdjacencyList\Query\Grammars;
+
+use Illuminate\Database\Eloquent\Model;
+
+trait OrdersByPath
+{
+    protected $model;
+
+    public function __construct(Model $model)
+    {
+        $this->model = $model;
+    }
+
+    /**
+     * Compile an "order by path" clause.
+     *
+     * @return string
+     */
+    public function compileOrderByPath()
+    {
+        $path = $this->model->getPathName();
+
+        return $this->wrap($path) . ' asc';
+    }
+}
diff --git a/src/Query/Grammars/PostgresGrammar.php b/src/Query/Grammars/PostgresGrammar.php
@@ -7,6 +7,8 @@
 
 class PostgresGrammar extends Base implements ExpressionGrammar
 {
+    use OrdersByPath;
+
     /**
      * Compile an initial path.
      *
@@ -16,6 +18,10 @@ class PostgresGrammar extends Base implements ExpressionGrammar
      */
     public function compileInitialPath($column, $alias)
     {
+        if ($this->model->isIntegerAttribute($column)) {
+            return 'array['.$this->wrap($column).'] as '.$this->wrap($alias);
+        }
+
         return 'array[('.$this->wrap($column)." || '')::varchar] as ".$this->wrap($alias);
     }
 
@@ -28,7 +34,13 @@ public function compileInitialPath($column, $alias)
      */
     public function compileRecursivePath($column, $alias)
     {
-        return $this->wrap($alias).' || '.$this->wrap($column).'::varchar';
+        $attribute = explode('.', $column)[1];
+
+        if ($this->model->isIntegerAttribute($attribute)) {
+            return $this->wrap($alias).' || '.$this->wrap($column);
+        }
+
+        return $this->wrap($alias) . ' || ' . $this->wrap($column) . '::varchar';
     }
 
     /**

diff --git a/src/Query/Grammars/SQLiteGrammar.php b/src/Query/Grammars/SQLiteGrammar.php
@@ -7,6 +7,8 @@
 
 class SQLiteGrammar extends Base implements ExpressionGrammar
 {
+    use OrdersByPath;
+
     /**
      * Compile an initial path.
      *

diff --git a/src/Query/Grammars/SqlServerGrammar.php b/src/Query/Grammars/SqlServerGrammar.php
@@ -7,6 +7,8 @@
 
 class SqlServerGrammar extends Base implements ExpressionGrammar
 {
+    use OrdersByPath;
+
     /**
      * Compile an initial path.
      *

diff --git a/tests/EloquentTest.php b/tests/EloquentTest.php
@@ -3,6 +3,7 @@
 namespace Tests;
 
 use Illuminate\Database\Eloquent\Builder;
+use Tests\Models\Category;
 use Tests\Models\User;
 
 class EloquentTest extends TestCase
@@ -130,4 +131,24 @@ public function testScopeDepthFirst()
 
         $this->assertEquals([1, 2, 5, 8, 3, 6, 9, 4, 7, 11, 12], $users->pluck('id')->all());
     }
+
+    public function testScopeDepthFirstWithNaturalSorting()
+    {
+        if (in_array($this->database, ['sqlite', 'sqlsrv'])) {
+            $this->markTestSkipped();
+        }
+
+        User::forceCreate(['id' => 70, 'slug' => 'user-70', 'parent_id' => 5, 'deleted_at' => null]);
+
+        $users = User::tree()->depthFirst()->get();
+
+        $this->assertEquals([1, 2, 5, 8, 70, 3, 6, 9, 4, 7, 11, 12], $users->pluck('id')->all());
+    }
+
+    public function testScopeDepthFirstWithStringKey()
+    {
+        $categories = Category::tree()->depthFirst()->get();
+
+        $this->assertEquals(['a', 'b', 'c', 'd'], $categories->pluck('id')->all());
+    }
 }
diff --git a/tests/Models/Category.php b/tests/Models/Category.php
@@ -0,0 +1,15 @@
+<?php
+
+namespace Tests\Models;
+
+use Illuminate\Database\Eloquent\Model;
+use Staudenmeir\LaravelAdjacencyList\Eloquent\HasRecursiveRelationships;
+
+class Category extends Model
+{
+    use HasRecursiveRelationships;
+
+    public $incrementing = false;
+
+    protected $keyType = 'string';
+}
diff --git a/tests/TestCase.php b/tests/TestCase.php
@@ -7,6 +7,7 @@
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Schema\Blueprint;
 use PHPUnit\Framework\TestCase as Base;
+use Tests\Models\Category;
 use Tests\Models\Post;
 use Tests\Models\Role;
 use Tests\Models\Tag;
@@ -122,6 +123,15 @@ function (Blueprint $table) {
                 $table->morphs('authorable');
             }
         );
+
+        DB::schema()->create(
+            'categories',
+            function (Blueprint $table) {
+                $table->string('id');
+                $table->string('parent_id')->nullable();
+                $table->timestamps();
+            }
+        );
     }
 
     /**
@@ -251,6 +261,11 @@ protected function seed()
             ]
         );
 
+        Category::create(['id' => 'a', 'parent_id' => null]);
+        Category::create(['id' => 'd', 'parent_id' => 'a']);
+        Category::create(['id' => 'c', 'parent_id' => 'a']);
+        Category::create(['id' => 'b', 'parent_id' => 'a']);
+
         Model::reguard();
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -7,6 +7,8 @@ @@
     class SQLiteGrammar extends Base implements ExpressionGrammar
     {
+        use OrdersByPath;
         /**
          * Compile an initial path.
          *
@@ Expand Down @@