Improved code related to EZP-29139 (#2454)

* Improved Query selection logic in Content Gateway * [SPI] Improved list of thrown exceptions in PhpDocs of URLAlias Handler
ezsystems · Sep 24, 2018 · b4417cd · b4417cd
1 parent 398a3bc
commit b4417cd
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 29 deletions.
diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php b/eZ/Publish/Core/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php
@@ -9,7 +9,6 @@
 namespace eZ\Publish\Core\Persistence\Legacy\Content\Gateway;
 
 use Doctrine\DBAL\Connection;
-use Doctrine\DBAL\Query\QueryBuilder as DoctrineQueryBuilder;
 use eZ\Publish\Core\Persistence\Legacy\Content\Gateway;
 use eZ\Publish\Core\Persistence\Legacy\Content\Gateway\DoctrineDatabase\QueryBuilder;
 use eZ\Publish\Core\Persistence\Database\DatabaseHandler;
@@ -954,25 +953,30 @@ private function internalLoadContent(array $contentIds, $version = null, array $
     }
 
     /**
-     * @see loadContentInfo(), loadContentInfoByRemoteId(), loadContentInfoList(), loadContentInfoByLocationId()
+     * Get query builder to load Content Info data.
      *
-     * @param \Doctrine\DBAL\Query\QueryBuilder $query
+     * @see loadContentInfo(), loadContentInfoByRemoteId(), loadContentInfoList(), loadContentInfoByLocationId()
      *
-     * @return array
+     * @return \Doctrine\DBAL\Query\QueryBuilder
      */
-    private function internalLoadContentInfo(DoctrineQueryBuilder $query)
+    private function createLoadContentInfoQueryBuilder()
     {
-        $query
+        $queryBuilder = $this->connection->createQueryBuilder();
+        $expr = $queryBuilder->expr();
+        $queryBuilder
             ->select('c.*', 't.main_node_id AS ezcontentobject_tree_main_node_id')
             ->from('ezcontentobject', 'c')
             ->leftJoin(
                 'c',
                 'ezcontentobject_tree',
                 't',
-                'c.id = t.contentobject_id AND t.node_id = t.main_node_id'
+                $expr->andX(
+                    $expr->eq('c.id', 't.contentobject_id'),
+                    $expr->eq('t.node_id', 't.main_node_id')
+                )
             );
 
-        return $query->execute()->fetchAll();
+        return $queryBuilder;
     }
 
     /**
@@ -989,11 +993,12 @@ private function internalLoadContentInfo(DoctrineQueryBuilder $query)
      */
     public function loadContentInfo($contentId)
     {
-        $query = $this->connection->createQueryBuilder();
-        $query->where('c.id = :id')
-              ->setParameter('id', $contentId, PDO::PARAM_INT);
+        $queryBuilder = $this->createLoadContentInfoQueryBuilder();
+        $queryBuilder
+            ->where('c.id = :id')
+            ->setParameter('id', $contentId, PDO::PARAM_INT);
 
-        $results = $this->internalLoadContentInfo($query);
+        $results = $queryBuilder->execute()->fetchAll(PDO::FETCH_ASSOC);
         if (empty($results)) {
             throw new NotFound('content', "id: $contentId");
         }
@@ -1003,11 +1008,12 @@ public function loadContentInfo($contentId)
 
     public function loadContentInfoList(array $contentIds)
     {
-        $query = $this->connection->createQueryBuilder();
-        $query->where('c.id IN (:ids)')
-              ->setParameter('ids', $contentIds, Connection::PARAM_INT_ARRAY);
+        $queryBuilder = $this->createLoadContentInfoQueryBuilder();
+        $queryBuilder
+            ->where('c.id IN (:ids)')
+            ->setParameter('ids', $contentIds, Connection::PARAM_INT_ARRAY);
 
-        return $this->internalLoadContentInfo($query);
+        return $queryBuilder->execute()->fetchAll(PDO::FETCH_ASSOC);
     }
 
     /**
@@ -1023,11 +1029,12 @@ public function loadContentInfoList(array $contentIds)
      */
     public function loadContentInfoByRemoteId($remoteId)
     {
-        $query = $this->connection->createQueryBuilder();
-        $query->where('c.remote_id = :id')
-              ->setParameter('id', $remoteId, PDO::PARAM_STR);
+        $queryBuilder = $this->createLoadContentInfoQueryBuilder();
+        $queryBuilder
+            ->where('c.remote_id = :id')
+            ->setParameter('id', $remoteId, PDO::PARAM_STR);
 
-        $results = $this->internalLoadContentInfo($query);
+        $results = $queryBuilder->execute()->fetchAll(PDO::FETCH_ASSOC);
         if (empty($results)) {
             throw new NotFound('content', "remote_id: $remoteId");
         }
@@ -1048,11 +1055,12 @@ public function loadContentInfoByRemoteId($remoteId)
      */
     public function loadContentInfoByLocationId($locationId)
     {
-        $query = $this->connection->createQueryBuilder();
-        $query->where('t.main_node_id = :id')
-              ->setParameter('id', $locationId, PDO::PARAM_INT);
+        $queryBuilder = $this->createLoadContentInfoQueryBuilder();
+        $queryBuilder
+            ->where('t.main_node_id = :id')
+            ->setParameter('id', $locationId, PDO::PARAM_INT);
 
-        $results = $this->internalLoadContentInfo($query);
+        $results = $queryBuilder->execute()->fetchAll(PDO::FETCH_ASSOC);
         if (empty($results)) {
             throw new NotFound('content', "main_node_id: $locationId");
         }

diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/UrlAlias/Handler.php b/eZ/Publish/Core/Persistence/Legacy/Content/UrlAlias/Handler.php
@@ -307,6 +307,10 @@ private function internalPublishUrlAliasForLocation(
      * If $languageCode is null the $alias is created in the system's default
      * language. $alwaysAvailable makes the alias available in all languages.
      *
+     * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException
+     * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
+     * @throws \eZ\Publish\API\Repository\Exceptions\ForbiddenException
+     *
      * @param mixed $locationId
      * @param string $path
      * @param bool $forwarding
@@ -335,6 +339,8 @@ public function createCustomUrlAlias($locationId, $path, $forwarding = false, $l
      * language. $alwaysAvailable makes the alias available in all languages.
      *
      * @throws \eZ\Publish\API\Repository\Exceptions\ForbiddenException if the path already exists for the given language
+     * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException if the path is broken
+     * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
      *
      * @param string $resource
      * @param string $path
@@ -536,9 +542,7 @@ public function removeURLAliases(array $urlAliases)
      * Looks up a url alias for the given url.
      *
      * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
-     * @throws \RuntimeException
-     * @throws \eZ\Publish\Core\Base\Exceptions\NotFoundException
-     * @throws \eZ\Publish\Core\Base\Exceptions\InvalidArgumentException
+     * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
      * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException
      *
      * @param string $url

diff --git a/eZ/Publish/SPI/Persistence/Content/UrlAlias/Handler.php b/eZ/Publish/SPI/Persistence/Content/UrlAlias/Handler.php
@@ -69,6 +69,8 @@ public function createGlobalUrlAlias($resource, $path, $forwarding = false, $lan
     /**
      * List global aliases.
      *
+     * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException if path for any of the global URL aliases is broken
+     *
      * @param string|null $languageCode
      * @param int $offset
      * @param int $limit
@@ -80,6 +82,8 @@ public function listGlobalURLAliases($languageCode = null, $offset = 0, $limit =
     /**
      * List of url entries of $urlType, pointing to $locationId.
      *
+     * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException if any path for the Location is broken
+     *
      * @param mixed $locationId
      * @param bool $custom if true the user generated aliases are listed otherwise the autogenerated
      *
@@ -102,6 +106,8 @@ public function removeURLAliases(array $urlAliases);
      * Looks up a url alias for the given url.
      *
      * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
+     * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
+     * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException if the stored path for the given URL is broken
      *
      * @param string $url
      *
@@ -112,9 +118,9 @@ public function lookup($url);
     /**
      * Loads URL alias by given $id.
      *
-     * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
+     * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException if path for the given URL alias is broken
      *
-     * @param string $id
+     * @param string $id unique identifier in the form of "<parentId>-<text_md5>"
      *
      * @return \eZ\Publish\SPI\Persistence\Content\UrlAlias
      */