From 1e2780087e9b286f94d9fa58475477cce5f2a98e Mon Sep 17 00:00:00 2001 From: Jonas Staudenmeir Date: Tue, 4 Jan 2022 20:42:52 +0100 Subject: [PATCH] Improve depthFirst() with integer keys Co-authored-by: Amir Rami --- src/Eloquent/Builder.php | 23 +++++++++-- .../HasRecursiveRelationshipScopes.php | 4 +- src/Eloquent/HasRecursiveRelationships.php | 13 ++++++ src/Query/Grammars/ExpressionGrammar.php | 7 ++++ src/Query/Grammars/MariaDbGrammar.php | 38 ++++++++++++++++++ src/Query/Grammars/MySqlGrammar.php | 40 +++++++++++++++++-- src/Query/Grammars/OrdersByPath.php | 27 +++++++++++++ src/Query/Grammars/PostgresGrammar.php | 14 ++++++- src/Query/Grammars/SQLiteGrammar.php | 2 + src/Query/Grammars/SqlServerGrammar.php | 2 + tests/EloquentTest.php | 21 ++++++++++ tests/Models/Category.php | 15 +++++++ tests/TestCase.php | 15 +++++++ 13 files changed, 212 insertions(+), 9 deletions(-) create mode 100644 src/Query/Grammars/MariaDbGrammar.php create mode 100644 src/Query/Grammars/OrdersByPath.php create mode 100644 tests/Models/Category.php diff --git a/src/Eloquent/Builder.php b/src/Eloquent/Builder.php index 51eb2a5..22ad5ab 100644 --- 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 index b5c3b7d..7fa2f4f 100644 --- 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 index 2e31119..6642d53 100644 --- 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 index 07ce6e3..26cc7b5 100644 --- 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 new file mode 100644 index 0000000..231464f --- /dev/null +++ b/src/Query/Grammars/MariaDbGrammar.php @@ -0,0 +1,38 @@ +model->getLocalKeyName(); + + $path = $this->wrap( + $this->model->getPathName() + ); + + $pathSeparator = $this->model->getPathSeparator(); + + if (!$this->model->isIntegerAttribute($column)) { + return "$path asc"; + } + + return <<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 <<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 index 62af820..168e7d3 100644 --- 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 index 91ca44a..4f1a293 100644 --- 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 index 3921492..3ebc67d 100644 --- 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 index 9d234cb..e9f73b6 100644 --- 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 new file mode 100644 index 0000000..e5e1bb6 --- /dev/null +++ b/tests/Models/Category.php @@ -0,0 +1,15 @@ +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(); } }