From 5e85d34bbb9081523d7b3a75f48465386bc786f8 Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Sun, 28 Feb 2021 10:49:48 +0100 Subject: [PATCH 1/5] Add nulls_always_first and nulls_always_last to nulls_comparison --- .../Common/Filter/OrderFilterInterface.php | 2 + .../Doctrine/Orm/Filter/OrderFilter.php | 6 +- .../Common/Filter/OrderFilterTestTrait.php | 56 +++++++++++++++++++ .../Doctrine/Orm/Filter/OrderFilterTest.php | 20 +++++++ 4 files changed, 83 insertions(+), 1 deletion(-) diff --git a/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php b/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php index 4edbeaad129..049569027f7 100644 --- a/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php +++ b/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php @@ -24,6 +24,8 @@ interface OrderFilterInterface { public const DIRECTION_ASC = 'ASC'; public const DIRECTION_DESC = 'DESC'; + public const NULLS_ALWAYS_FIRST = 'nulls_always_first'; + public const NULLS_ALWAYS_LAST = 'nulls_always_last'; public const NULLS_SMALLEST = 'nulls_smallest'; public const NULLS_LARGEST = 'nulls_largest'; public const NULLS_DIRECTION_MAP = [ diff --git a/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php b/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php index e9829396abc..299a064203e 100644 --- a/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php +++ b/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php @@ -103,7 +103,11 @@ protected function filterProperty(string $property, $direction, QueryBuilder $qu } if (null !== $nullsComparison = $this->properties[$property]['nulls_comparison'] ?? null) { - $nullsDirection = self::NULLS_DIRECTION_MAP[$nullsComparison][$direction]; + if (\in_array($nullsComparison, [self::NULLS_ALWAYS_FIRST, self::NULLS_ALWAYS_LAST], true)) { + $nullsDirection = self::NULLS_ALWAYS_FIRST === $nullsComparison ? 'ASC' : 'DESC'; + } else { + $nullsDirection = self::NULLS_DIRECTION_MAP[$nullsComparison][$direction]; + } $nullRankHiddenField = sprintf('_%s_%s_null_rank', $alias, $field); diff --git a/tests/Bridge/Doctrine/Common/Filter/OrderFilterTestTrait.php b/tests/Bridge/Doctrine/Common/Filter/OrderFilterTestTrait.php index f9b4e6f6507..3e1e9464459 100644 --- a/tests/Bridge/Doctrine/Common/Filter/OrderFilterTestTrait.php +++ b/tests/Bridge/Doctrine/Common/Filter/OrderFilterTestTrait.php @@ -232,6 +232,62 @@ private function provideApplyTestArguments(): array ], ], ], + 'nulls_always_first (asc)' => [ + [ + 'dummyDate' => [ + 'nulls_comparison' => 'nulls_always_first', + ], + 'name' => null, + ], + [ + 'order' => [ + 'dummyDate' => 'asc', + 'name' => 'desc', + ], + ], + ], + 'nulls_always_first (desc)' => [ + [ + 'dummyDate' => [ + 'nulls_comparison' => 'nulls_always_first', + ], + 'name' => null, + ], + [ + 'order' => [ + 'dummyDate' => 'desc', + 'name' => 'desc', + ], + ], + ], + 'nulls_always_last (asc)' => [ + [ + 'dummyDate' => [ + 'nulls_comparison' => 'nulls_always_last', + ], + 'name' => null, + ], + [ + 'order' => [ + 'dummyDate' => 'asc', + 'name' => 'desc', + ], + ], + ], + 'nulls_always_last (desc)' => [ + [ + 'dummyDate' => [ + 'nulls_comparison' => 'nulls_always_last', + ], + 'name' => null, + ], + [ + 'order' => [ + 'dummyDate' => 'desc', + 'name' => 'desc', + ], + ], + ], 'not having order should not throw a deprecation (select unchanged)' => [ [ 'id' => null, diff --git a/tests/Bridge/Doctrine/Orm/Filter/OrderFilterTest.php b/tests/Bridge/Doctrine/Orm/Filter/OrderFilterTest.php index 77417ba1714..fa5c16cdc3c 100644 --- a/tests/Bridge/Doctrine/Orm/Filter/OrderFilterTest.php +++ b/tests/Bridge/Doctrine/Orm/Filter/OrderFilterTest.php @@ -265,6 +265,26 @@ public function provideApplyTestData(): array null, $orderFilterFactory, ], + 'nulls_always_first (asc)' => [ + sprintf('SELECT o, CASE WHEN o.dummyDate IS NULL THEN 0 ELSE 1 END AS HIDDEN _o_dummyDate_null_rank FROM %s o ORDER BY _o_dummyDate_null_rank ASC, o.dummyDate ASC, o.name DESC', Dummy::class), + null, + $orderFilterFactory, + ], + 'nulls_always_first (desc)' => [ + sprintf('SELECT o, CASE WHEN o.dummyDate IS NULL THEN 0 ELSE 1 END AS HIDDEN _o_dummyDate_null_rank FROM %s o ORDER BY _o_dummyDate_null_rank ASC, o.dummyDate DESC, o.name DESC', Dummy::class), + null, + $orderFilterFactory, + ], + 'nulls_always_last (asc)' => [ + sprintf('SELECT o, CASE WHEN o.dummyDate IS NULL THEN 0 ELSE 1 END AS HIDDEN _o_dummyDate_null_rank FROM %s o ORDER BY _o_dummyDate_null_rank DESC, o.dummyDate ASC, o.name DESC', Dummy::class), + null, + $orderFilterFactory, + ], + 'nulls_always_last (desc)' => [ + sprintf('SELECT o, CASE WHEN o.dummyDate IS NULL THEN 0 ELSE 1 END AS HIDDEN _o_dummyDate_null_rank FROM %s o ORDER BY _o_dummyDate_null_rank DESC, o.dummyDate DESC, o.name DESC', Dummy::class), + null, + $orderFilterFactory, + ], 'not having order should not throw a deprecation (select unchanged)' => [ sprintf('SELECT o FROM %s o', Dummy::class), null, From ddb2c389198e5d9b9ded7a57544cdb55dd4e15ee Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Sun, 28 Feb 2021 11:01:24 +0100 Subject: [PATCH 2/5] Add new `nulls_comparison` options --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 148362352a0..88ad8b83d63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * MongoDB: `date_immutable` support (#3940) * DataProvider: Add `TraversablePaginator` (#3783) * Swagger UI: Add `swagger_ui_extra_configuration` to Swagger / OpenAPI configuration (#3731) +* OrderFilter: Add `nulls_always_first` and `nulls_always_last` to `nulls_comparison` (#4103) ## 2.6.3 From 969681fafc2998208fe1b9f765655c2509e4113f Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Mon, 1 Mar 2021 20:23:47 +0100 Subject: [PATCH 3/5] Reusing direction map --- .../Doctrine/Common/Filter/OrderFilterInterface.php | 8 ++++++++ src/Bridge/Doctrine/Orm/Filter/OrderFilter.php | 6 +----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php b/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php index 049569027f7..2c043895728 100644 --- a/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php +++ b/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php @@ -37,5 +37,13 @@ interface OrderFilterInterface 'ASC' => 'DESC', 'DESC' => 'ASC', ], + self::NULLS_ALWAYS_FIRST => [ + 'ASC' => 'ASC', + 'DESC' => 'ASC', + ], + self::NULLS_ALWAYS_LAST => [ + 'ASC' => 'DESC', + 'DESC' => 'DESC', + ], ]; } diff --git a/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php b/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php index 299a064203e..e9829396abc 100644 --- a/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php +++ b/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php @@ -103,11 +103,7 @@ protected function filterProperty(string $property, $direction, QueryBuilder $qu } if (null !== $nullsComparison = $this->properties[$property]['nulls_comparison'] ?? null) { - if (\in_array($nullsComparison, [self::NULLS_ALWAYS_FIRST, self::NULLS_ALWAYS_LAST], true)) { - $nullsDirection = self::NULLS_ALWAYS_FIRST === $nullsComparison ? 'ASC' : 'DESC'; - } else { - $nullsDirection = self::NULLS_DIRECTION_MAP[$nullsComparison][$direction]; - } + $nullsDirection = self::NULLS_DIRECTION_MAP[$nullsComparison][$direction]; $nullRankHiddenField = sprintf('_%s_%s_null_rank', $alias, $field); From 2e8acbf12dc9099d9140244140249759d23ee002 Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Mon, 1 Mar 2021 20:28:02 +0100 Subject: [PATCH 4/5] Reorder for consistency --- src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php b/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php index 2c043895728..d348c10bd02 100644 --- a/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php +++ b/src/Bridge/Doctrine/Common/Filter/OrderFilterInterface.php @@ -24,10 +24,10 @@ interface OrderFilterInterface { public const DIRECTION_ASC = 'ASC'; public const DIRECTION_DESC = 'DESC'; - public const NULLS_ALWAYS_FIRST = 'nulls_always_first'; - public const NULLS_ALWAYS_LAST = 'nulls_always_last'; public const NULLS_SMALLEST = 'nulls_smallest'; public const NULLS_LARGEST = 'nulls_largest'; + public const NULLS_ALWAYS_FIRST = 'nulls_always_first'; + public const NULLS_ALWAYS_LAST = 'nulls_always_last'; public const NULLS_DIRECTION_MAP = [ self::NULLS_SMALLEST => [ 'ASC' => 'ASC', From 49732fcda03359e73b788411597ea3cc71624b79 Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Mon, 1 Mar 2021 20:46:41 +0100 Subject: [PATCH 5/5] Add missing mongoDB tests --- .../MongoDbOdm/Filter/OrderFilterTest.php | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/Bridge/Doctrine/MongoDbOdm/Filter/OrderFilterTest.php b/tests/Bridge/Doctrine/MongoDbOdm/Filter/OrderFilterTest.php index bdddb3877fa..9279532f8e6 100644 --- a/tests/Bridge/Doctrine/MongoDbOdm/Filter/OrderFilterTest.php +++ b/tests/Bridge/Doctrine/MongoDbOdm/Filter/OrderFilterTest.php @@ -436,6 +436,70 @@ public function provideApplyTestData(): array ], $orderFilterFactory, ], + 'nulls_always_first (asc)' => [ + [ + [ + '$sort' => [ + 'dummyDate' => 1, + ], + ], + [ + '$sort' => [ + 'dummyDate' => 1, + 'name' => -1, + ], + ], + ], + $orderFilterFactory, + ], + 'nulls_always_first (desc)' => [ + [ + [ + '$sort' => [ + 'dummyDate' => -1, + ], + ], + [ + '$sort' => [ + 'dummyDate' => -1, + 'name' => -1, + ], + ], + ], + $orderFilterFactory, + ], + 'nulls_always_last (asc)' => [ + [ + [ + '$sort' => [ + 'dummyDate' => 1, + ], + ], + [ + '$sort' => [ + 'dummyDate' => 1, + 'name' => -1, + ], + ], + ], + $orderFilterFactory, + ], + 'nulls_always_last (desc)' => [ + [ + [ + '$sort' => [ + 'dummyDate' => -1, + ], + ], + [ + '$sort' => [ + 'dummyDate' => -1, + 'name' => -1, + ], + ], + ], + $orderFilterFactory, + ], 'not having order should not throw a deprecation (select unchanged)' => [ [], $orderFilterFactory,