From 9a82caa362a7c67adc2cb2c46b0ff9af0377dc5e Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 13 Aug 2026 12:28:16 +0200 Subject: [PATCH 1/2] feat(db): Add support for SortDirection to addSortBy and sortBy Signed-off-by: Carl Schwan --- .../DB/QueryBuilder/ExtendedQueryBuilder.php | 7 ++-- lib/private/DB/QueryBuilder/QueryBuilder.php | 33 +++++++------------ .../Sharded/ShardedQueryBuilder.php | 29 +++++++++++----- lib/public/AppFramework/ORM/Repository.php | 2 +- lib/public/DB/QueryBuilder/IQueryBuilder.php | 8 ++--- .../DB/QueryBuilder/ITypedQueryBuilder.php | 6 ++-- 6 files changed, 45 insertions(+), 40 deletions(-) diff --git a/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php b/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php index 5cc0771c0bed0..5b63e1792bacb 100644 --- a/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php +++ b/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php @@ -10,7 +10,10 @@ use OCP\DB\IResult; use OCP\DB\QueryBuilder\ConflictResolutionMode; +use OCP\DB\QueryBuilder\ILiteral; +use OCP\DB\QueryBuilder\IParameter; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\DB\QueryBuilder\IQueryFunction; use OCP\IDBConnection; /** @@ -251,13 +254,13 @@ public function orHaving(...$having) { } #[\Override] - public function orderBy($sort, $order = null) { + public function orderBy(string|ILiteral|IParameter|IQueryFunction $sort, string|\SortDirection|null $order = null): self { $this->builder->orderBy($sort, $order); return $this; } #[\Override] - public function addOrderBy($sort, $order = null) { + public function addOrderBy(string|ILiteral|IParameter|IQueryFunction $sort, string|\SortDirection|null $order = null): self { $this->builder->addOrderBy($sort, $order); return $this; } diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index f5252fcfaccc7..96ff7a465f9bd 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -1115,18 +1115,13 @@ public function orHaving(...$having) { return $this; } - /** - * Specifies an ordering for the query results. - * Replaces any previously specified orderings, if any. - * - * @param string|IQueryFunction|ILiteral|IParameter $sort The ordering expression. - * @param string $order The ordering direction. - * - * @return $this This QueryBuilder instance. - */ #[\Override] - public function orderBy($sort, $order = null) { - if ($order !== null && !in_array(strtoupper((string)$order), ['ASC', 'DESC'], true)) { + public function orderBy(string|ILiteral|IParameter|IQueryFunction $sort, string|\SortDirection|null $order = null): self { + if ($order === \SortDirection::Ascending) { + $order = 'ASC'; + } elseif ($order === \SortDirection::Descending) { + $order = 'DESC'; + } elseif ($order !== null && !in_array(strtoupper($order), ['ASC', 'DESC'], true)) { $order = null; } @@ -1138,17 +1133,13 @@ public function orderBy($sort, $order = null) { return $this; } - /** - * Adds an ordering to the query results. - * - * @param string|ILiteral|IParameter|IQueryFunction $sort The ordering expression. - * @param string $order The ordering direction. - * - * @return $this This QueryBuilder instance. - */ #[\Override] - public function addOrderBy($sort, $order = null) { - if ($order !== null && !in_array(strtoupper((string)$order), ['ASC', 'DESC'], true)) { + public function addOrderBy(string|ILiteral|IParameter|IQueryFunction $sort, string|\SortDirection|null $order = null): self { + if ($order === \SortDirection::Ascending) { + $order = 'ASC'; + } elseif ($order === \SortDirection::Descending) { + $order = 'DESC'; + } elseif ($order !== null && !in_array(strtoupper($order), ['ASC', 'DESC'], true)) { $order = null; } diff --git a/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php b/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php index 0c2bf2fa1db14..96fdd53001abc 100644 --- a/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php +++ b/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php @@ -12,7 +12,10 @@ use OC\DB\QueryBuilder\ExtendedQueryBuilder; use OC\DB\QueryBuilder\Parameter; use OCP\DB\IResult; +use OCP\DB\QueryBuilder\ILiteral; +use OCP\DB\QueryBuilder\IParameter; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\DB\QueryBuilder\IQueryFunction; use OCP\IDBConnection; /** @@ -295,24 +298,34 @@ public function setFirstResult($firstResult) { } #[\Override] - public function addOrderBy($sort, $order = null) { - if ($order !== null && !in_array(strtoupper((string)$order), ['ASC', 'DESC'], true)) { + public function addOrderBy(string|ILiteral|IParameter|IQueryFunction $sort, string|\SortDirection|null $order = null): self { + if ($order === \SortDirection::Ascending) { + $order = 'ASC'; + } elseif ($order === \SortDirection::Descending) { + $order = 'DESC'; + } elseif ($order !== null && !in_array(strtoupper($order), ['ASC', 'DESC'], true)) { $order = null; } - $this->registerOrder((string)$sort, (string)($order ?? 'ASC')); - return parent::addOrderBy($sort, $order); + $this->registerOrder((string)$sort, $order ?? 'ASC'); + parent::addOrderBy($sort, $order); + return $this; } #[\Override] - public function orderBy($sort, $order = null) { - if ($order !== null && !in_array(strtoupper((string)$order), ['ASC', 'DESC'], true)) { + public function orderBy(string|ILiteral|IParameter|IQueryFunction $sort, string|\SortDirection|null $order = null): self { + if ($order === \SortDirection::Ascending) { + $order = 'ASC'; + } elseif ($order === \SortDirection::Descending) { + $order = 'DESC'; + } elseif ($order !== null && !in_array(strtoupper($order), ['ASC', 'DESC'], true)) { $order = null; } $this->sortList = []; - $this->registerOrder((string)$sort, (string)($order ?? 'ASC')); - return parent::orderBy($sort, $order); + $this->registerOrder((string)$sort, $order ?? 'ASC'); + parent::orderBy($sort, $order); + return $this; } private function registerOrder(string $column, string $order): void { diff --git a/lib/public/AppFramework/ORM/Repository.php b/lib/public/AppFramework/ORM/Repository.php index 3a007a9194c83..098ea261335d4 100644 --- a/lib/public/AppFramework/ORM/Repository.php +++ b/lib/public/AppFramework/ORM/Repository.php @@ -478,7 +478,7 @@ private function getJoinedSelectQueryBuilder(array $criteria, array $orderBy = [ foreach ($orderBy as $field => $direction) { $column = $entityInfo->mappingPropertyToColumn[$field]; - $qb->addOrderBy('e.' . $column, $direction === \SortDirection::Ascending ? 'ASC' : 'DESC'); + $qb->addOrderBy('e.' . $column, $direction); } return [$qb, $relations]; diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index 89d6b61e0abdf..6b9ed4a36280c 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -857,7 +857,7 @@ public function orHaving(...$having); * Replaces any previously specified orderings, if any. * * @param string|IQueryFunction|ILiteral|IParameter $sort The ordering expression. - * @param string $order The ordering direction. + * @param string|\SortDirection|null $order The ordering direction. * * @return $this This QueryBuilder instance. * @since 8.2.0 @@ -865,13 +865,13 @@ public function orHaving(...$having); * @psalm-taint-sink sql $sort * @psalm-taint-sink sql $order */ - public function orderBy($sort, $order = null); + public function orderBy(string|ILiteral|IParameter|IQueryFunction $sort, string|\SortDirection|null $order = null): self; /** * Adds an ordering to the query results. * * @param string|ILiteral|IParameter|IQueryFunction $sort The ordering expression. - * @param string $order The ordering direction. + * @param string|\SortDirection|null $order The ordering direction. * * @return $this This QueryBuilder instance. * @since 8.2.0 @@ -879,7 +879,7 @@ public function orderBy($sort, $order = null); * @psalm-taint-sink sql $sort * @psalm-taint-sink sql $order */ - public function addOrderBy($sort, $order = null); + public function addOrderBy(string|ILiteral|IParameter|IQueryFunction $sort, string|\SortDirection|null $order = null): self; /** * Gets a query part by its name. diff --git a/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php b/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php index f096b5cff0f41..823cb862786e8 100644 --- a/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php @@ -293,20 +293,18 @@ public function orHaving(...$having); /** * @inheritDoc * @return $this - * @psalm-suppress MissingParamType * @since 34.0.0 */ #[Override] - public function orderBy($sort, $order = null); + public function orderBy(string|ILiteral|IParameter|IQueryFunction $sort, string|\SortDirection|null $order = null): self; /** * @inheritDoc * @return $this - * @psalm-suppress MissingParamType * @since 34.0.0 */ #[Override] - public function addOrderBy($sort, $order = null); + public function addOrderBy(string|ILiteral|IParameter|IQueryFunction $sort, string|\SortDirection|null $order = null): self; /** * @inheritDoc From 6b482b8c0fd5e294a438ae12b5cbefcafd25c1aa Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 13 Aug 2026 12:39:17 +0200 Subject: [PATCH 2/2] refactor: Be a bit more strict about string input in order And throw if this doesn't match Signed-off-by: Carl Schwan --- lib/private/DB/QueryBuilder/QueryBuilder.php | 4 ++-- lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php | 4 ++-- lib/public/DB/QueryBuilder/IQueryBuilder.php | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index 96ff7a465f9bd..11264d28361e3 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -1122,7 +1122,7 @@ public function orderBy(string|ILiteral|IParameter|IQueryFunction $sort, string| } elseif ($order === \SortDirection::Descending) { $order = 'DESC'; } elseif ($order !== null && !in_array(strtoupper($order), ['ASC', 'DESC'], true)) { - $order = null; + throw new \InvalidArgumentException('Only ASC or DESC are supported'); } $this->queryBuilder->orderBy( @@ -1140,7 +1140,7 @@ public function addOrderBy(string|ILiteral|IParameter|IQueryFunction $sort, stri } elseif ($order === \SortDirection::Descending) { $order = 'DESC'; } elseif ($order !== null && !in_array(strtoupper($order), ['ASC', 'DESC'], true)) { - $order = null; + throw new \InvalidArgumentException('Only ASC or DESC are supported'); } $this->queryBuilder->addOrderBy( diff --git a/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php b/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php index 96fdd53001abc..9bdf08acd56cc 100644 --- a/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php +++ b/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php @@ -304,7 +304,7 @@ public function addOrderBy(string|ILiteral|IParameter|IQueryFunction $sort, stri } elseif ($order === \SortDirection::Descending) { $order = 'DESC'; } elseif ($order !== null && !in_array(strtoupper($order), ['ASC', 'DESC'], true)) { - $order = null; + throw new \InvalidArgumentException('Only ASC or DESC are supported'); } $this->registerOrder((string)$sort, $order ?? 'ASC'); @@ -319,7 +319,7 @@ public function orderBy(string|ILiteral|IParameter|IQueryFunction $sort, string| } elseif ($order === \SortDirection::Descending) { $order = 'DESC'; } elseif ($order !== null && !in_array(strtoupper($order), ['ASC', 'DESC'], true)) { - $order = null; + throw new \InvalidArgumentException('Only ASC or DESC are supported'); } $this->sortList = []; diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index 6b9ed4a36280c..8cb89d8f976d4 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -857,7 +857,7 @@ public function orHaving(...$having); * Replaces any previously specified orderings, if any. * * @param string|IQueryFunction|ILiteral|IParameter $sort The ordering expression. - * @param string|\SortDirection|null $order The ordering direction. + * @param 'ASC'|'DESC'|'asc'|'desc'|\SortDirection|null $order The ordering direction. * * @return $this This QueryBuilder instance. * @since 8.2.0 @@ -871,7 +871,7 @@ public function orderBy(string|ILiteral|IParameter|IQueryFunction $sort, string| * Adds an ordering to the query results. * * @param string|ILiteral|IParameter|IQueryFunction $sort The ordering expression. - * @param string|\SortDirection|null $order The ordering direction. + * @param 'ASC'|'DESC'|'asc'|'desc'|\SortDirection|null $order The ordering direction. * * @return $this This QueryBuilder instance. * @since 8.2.0