EZP-28663: Cannot swap location when viewing item from non-main locat…

…ion (#2531)

* [Tests] Created integration test for swapping secondary Location

* Dropped forcing Main Location when loading Content by Location data

* Refactored UrlAlias GW ::getLocationContentMainLanguageId to use Doctrine Query Builder

* [Tests] Impl. integration test for duplicate main Location

* Handled main node changes when swapping non-main Locations

eZ/Publish/API/Repository/Tests/LocationServiceTest.php

@@ -8,12 +8,15 @@
  */
 namespace eZ\Publish\API\Repository\Tests;
 
+use Exception;
+use eZ\Publish\API\Repository\Exceptions\NotFoundException;
+use eZ\Publish\API\Repository\Values\Content\Content;
+use eZ\Publish\API\Repository\Values\Content\ContentInfo;
 use eZ\Publish\API\Repository\Values\Content\Location;
 use eZ\Publish\API\Repository\Values\Content\LocationCreateStruct;
 use eZ\Publish\API\Repository\Values\Content\LocationList;
-use eZ\Publish\API\Repository\Values\Content\ContentInfo;
-use eZ\Publish\API\Repository\Exceptions\NotFoundException;
-use Exception;
+use eZ\Publish\API\Repository\Values\Content\Query;
+use eZ\Publish\API\Repository\Values\Content\Search\SearchHit;
 
 /**
  * Test case for operations in the LocationService using in memory storage.
@@ -1126,6 +1129,205 @@ public function testSwapLocation()
         );
     }
 
+    /**
+     * Test swapping secondary Location with main Location.
+     *
+     * @covers \eZ\Publish\API\Repository\LocationService::swapLocation
+     *
+     * @see https://jira.ez.no/browse/EZP-28663
+     *
+     * @throws \eZ\Publish\API\Repository\Exceptions\ForbiddenException
+     * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
+     * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
+     *
+     * @return int[]
+     */
+    public function testSwapLocationForMainAndSecondaryLocation()
+    {
+        $repository = $this->getRepository();
+        $locationService = $repository->getLocationService();
+        $contentService = $repository->getContentService();
+
+        $folder1 = $this->createFolder(['eng-GB' => 'Folder1'], 2);
+        $folder2 = $this->createFolder(['eng-GB' => 'Folder2'], 2);
+        $folder3 = $this->createFolder(['eng-GB' => 'Folder3'], 2);
+
+        $primaryLocation = $locationService->loadLocation($folder1->contentInfo->mainLocationId);
+        $parentLocation = $locationService->loadLocation($folder2->contentInfo->mainLocationId);
+        $secondaryLocation = $locationService->createLocation(
+            $folder1->contentInfo,
+            $locationService->newLocationCreateStruct($parentLocation->id)
+        );
+
+        $targetLocation = $locationService->loadLocation($folder3->contentInfo->mainLocationId);
+
+        // perform sanity checks
+        $folder1Locations = $locationService->loadLocations($folder1->contentInfo);
+        self::assertCount(2, $folder1Locations);
+        self::assertContains(
+            $primaryLocation,
+            $folder1Locations,
+            "Location {$primaryLocation->id} not found in Folder1 Locations",
+            false,
+            false
+        );
+        self::assertContains(
+            $secondaryLocation,
+            $folder1Locations,
+            "Location {$secondaryLocation->id} not found in Folder1 Locations",
+            false,
+            false
+        );
+
+        // begin use case
+        $locationService->swapLocation($secondaryLocation, $targetLocation);
+
+        // test results
+        $primaryLocation = $locationService->loadLocation($primaryLocation->id);
+        $secondaryLocation = $locationService->loadLocation($secondaryLocation->id);
+        $targetLocation = $locationService->loadLocation($targetLocation->id);
+
+        self::assertEquals($folder1->id, $primaryLocation->contentInfo->id);
+        self::assertEquals($folder1->id, $targetLocation->contentInfo->id);
+        self::assertEquals($folder3->id, $secondaryLocation->contentInfo->id);
+
+        $folder1Locations = $locationService->loadLocations($folder1->contentInfo);
+        self::assertCount(2, $folder1Locations);
+        self::assertContains(
+            $primaryLocation,
+            $folder1Locations,
+            "Location {$primaryLocation->id} not found in Folder1 Locations",
+            false,
+            false
+        );
+        self::assertContains(
+            $targetLocation,
+            $folder1Locations,
+            "Location {$targetLocation->id} not found in Folder1 Locations",
+            false,
+            false
+        );
+
+        self::assertEquals(
+            $folder1,
+            $contentService->loadContent($folder1->id)
+        );
+
+        self::assertEquals(
+            $folder2,
+            $contentService->loadContent($folder2->id)
+        );
+
+        // only in case of Folder 3, main location id changed due to swap
+        self::assertEquals(
+            $secondaryLocation->id,
+            $contentService->loadContent($folder3->id)->contentInfo->mainLocationId
+        );
+
+        return [$folder1, $folder2, $folder3];
+    }
+
+    /**
+     * @depends testSwapLocationForMainAndSecondaryLocation
+     *
+     * @param \eZ\Publish\API\Repository\Values\Content\Content[] $contentItems Content items created by testSwapLocationForSecondaryLocation
+     *
+     * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
+     */
+    public function testSwapLocationDoesNotCorruptSearchResults(
+        array $contentItems
+    ) {
+        $repository = $this->getRepository(false);
+        $searchService = $repository->getSearchService();
+
+        $this->refreshSearch($repository);
+
+        $contentIds = array_map(
+            function (Content $content) {
+                return $content->id;
+            },
+            $contentItems
+        );
+
+        $query = new Query();
+        $query->filter = new Query\Criterion\ContentId($contentIds);
+
+        $searchResult = $searchService->findContent($query);
+
+        self::assertEquals(count($contentItems), $searchResult->totalCount);
+        self::assertEquals(
+            $searchResult->totalCount,
+            count($searchResult->searchHits),
+            'Total count of search result hits does not match the actual number of found results'
+        );
+        $foundContentIds = array_map(
+            function (SearchHit $searchHit) {
+                return $searchHit->valueObject->id;
+            },
+            $searchResult->searchHits
+        );
+        sort($contentIds);
+        sort($foundContentIds);
+        self::assertSame(
+            $contentIds,
+            $foundContentIds,
+            'Got different than expected Content item Ids'
+        );
+    }
+
+    /**
+     * Test swapping two secondary (non-main) Locations.
+     *
+     * @covers \eZ\Publish\API\Repository\LocationService::swapLocation
+     *
+     * @throws \eZ\Publish\API\Repository\Exceptions\ForbiddenException
+     * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
+     * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
+     * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
+     */
+    public function testSwapLocationForSecondaryLocations()
+    {
+        $repository = $this->getRepository();
+        $locationService = $repository->getLocationService();
+        $contentService = $repository->getContentService();
+
+        $folder1 = $this->createFolder(['eng-GB' => 'Folder1'], 2);
+        $folder2 = $this->createFolder(['eng-GB' => 'Folder2'], 2);
+        $parentFolder1 = $this->createFolder(['eng-GB' => 'Parent1'], 2);
+        $parentFolder2 = $this->createFolder(['eng-GB' => 'Parent2'], 2);
+
+        $parentLocation1 = $locationService->loadLocation($parentFolder1->contentInfo->mainLocationId);
+        $parentLocation2 = $locationService->loadLocation($parentFolder2->contentInfo->mainLocationId);
+        $secondaryLocation1 = $locationService->createLocation(
+            $folder1->contentInfo,
+            $locationService->newLocationCreateStruct($parentLocation1->id)
+        );
+        $secondaryLocation2 = $locationService->createLocation(
+            $folder2->contentInfo,
+            $locationService->newLocationCreateStruct($parentLocation2->id)
+        );
+
+        // begin use case
+        $locationService->swapLocation($secondaryLocation1, $secondaryLocation2);
+
+        // test results
+        $secondaryLocation1 = $locationService->loadLocation($secondaryLocation1->id);
+        $secondaryLocation2 = $locationService->loadLocation($secondaryLocation2->id);
+
+        self::assertEquals($folder2->id, $secondaryLocation1->contentInfo->id);
+        self::assertEquals($folder1->id, $secondaryLocation2->contentInfo->id);
+
+        self::assertEquals(
+            $folder1,
+            $contentService->loadContent($folder1->id)
+        );
+
+        self::assertEquals(
+            $folder2,
+            $contentService->loadContent($folder2->id)
+        );
+    }
+
     /**
      * Test for the hideLocation() method.
      *

eZ/Publish/Core/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php

@@ -933,23 +933,32 @@ private function internalLoadContent(array $contentIds, $version = null, array $
      *
      * @see loadContentInfo(), loadContentInfoByRemoteId(), loadContentInfoList(), loadContentInfoByLocationId()
      *
+     * @param bool $joinMainLocation
+     *
      * @return \Doctrine\DBAL\Query\QueryBuilder
      */
-    private function createLoadContentInfoQueryBuilder()
+    private function createLoadContentInfoQueryBuilder($joinMainLocation = true)
     {
         $queryBuilder = $this->connection->createQueryBuilder();
         $expr = $queryBuilder->expr();
+
+        $joinCondition = $expr->eq('c.id', 't.contentobject_id');
+        if ($joinMainLocation) {
+            // wrap join condition with AND operator and join by a Main Location
+            $joinCondition = $expr->andX(
+                $joinCondition,
+                $expr->eq('t.node_id', 't.main_node_id')
+            );
+        }
+
         $queryBuilder
             ->select('c.*', 't.main_node_id AS ezcontentobject_tree_main_node_id')
             ->from('ezcontentobject', 'c')
             ->leftJoin(
                 'c',
                 'ezcontentobject_tree',
                 't',
-                $expr->andX(
-                    $expr->eq('c.id', 't.contentobject_id'),
-                    $expr->eq('t.node_id', 't.main_node_id')
-                )
+                $joinCondition
             );
 
         return $queryBuilder;
@@ -1031,14 +1040,14 @@ public function loadContentInfoByRemoteId($remoteId)
      */
     public function loadContentInfoByLocationId($locationId)
     {
-        $queryBuilder = $this->createLoadContentInfoQueryBuilder();
+        $queryBuilder = $this->createLoadContentInfoQueryBuilder(false);
         $queryBuilder
-            ->where('t.main_node_id = :id')
+            ->where('t.node_id = :id')
             ->setParameter('id', $locationId, PDO::PARAM_INT);
 
         $results = $queryBuilder->execute()->fetchAll(PDO::FETCH_ASSOC);
         if (empty($results)) {
-            throw new NotFound('content', "main_node_id: $locationId");
+            throw new NotFound('content', "node_id: $locationId");
         }
 
         return $results[0];