From 8d3a49a3191211df574838e8f98074f1012fb023 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 4 Oct 2019 17:54:19 +0200
Subject: [PATCH] Fixed issue with postgres when fetching resultset ordering by
 nullable column

---
 module/Core/src/Repository/ShortUrlRepository.php     | 11 ++++++++---
 .../test-db/Repository/ShortUrlRepositoryTest.php     |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php
index fb0a0625..d11f95d2 100644
--- a/module/Core/src/Repository/ShortUrlRepository.php
+++ b/module/Core/src/Repository/ShortUrlRepository.php
@@ -119,6 +119,11 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI
 
     public function findOneByShortCode(string $shortCode, ?string $domain = null): ?ShortUrl
     {
+        // When ordering DESC, Postgres puts nulls at the beginning while the rest of supported DB engines put them at
+        // the bottom
+        $dbPlatform = $this->getEntityManager()->getConnection()->getDatabasePlatform()->getName();
+        $ordering = $dbPlatform === 'postgresql' ? 'ASC' : 'DESC';
+
         $dql= <<<DQL
             SELECT s
               FROM Shlinkio\Shlink\Core\Entity\ShortUrl AS s
@@ -127,7 +132,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI
                AND (s.validSince <= :now OR s.validSince IS NULL)
                AND (s.validUntil >= :now OR s.validUntil IS NULL)
                AND (s.domain IS NULL OR d.authority = :domain)
-          ORDER BY s.domain DESC
+          ORDER BY s.domain {$ordering}
 DQL;
 
         $query = $this->getEntityManager()->createQuery($dql);
@@ -138,8 +143,8 @@ DQL;
                   'domain' => $domain,
               ]);
 
-        // Since we ordered by domain DESC, we will have first the URL matching the domain, followed
-        // by the one with no domain (if any), so it is safe to fetch 1 max result and we will get:
+        // Since we ordered by domain, we will have first the URL matching provided domain, followed by the one
+        // with no domain (if any), so it is safe to fetch 1 max result and we will get:
         //  * The short URL matching both the short code and the domain, or
         //  * The short URL matching the short code but without any domain, or
         //  * No short URL at all
diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php
index 2f7e27b6..b1fac238 100644
--- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php
+++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php
@@ -63,7 +63,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
         $withDomain->setShortCode('domain-short-code');
         $this->getEntityManager()->persist($withDomain);
 
-        $withDomainDuplicatingRegular = new ShortUrl('foo', ShortUrlMeta::createFromRawData([
+        $withDomainDuplicatingRegular = new ShortUrl('foo_with_domain', ShortUrlMeta::createFromRawData([
             'domain' => 'doma.in',
         ]));
         $withDomainDuplicatingRegular->setShortCode('foo');