Merge branch '7.2' into 7.3

ezecosystem · Dec 11, 2018 · a98012d · a98012d
2 parents 3f4f034 + 0c78bee
commit a98012d
Show file tree

Hide file tree

Showing 23 changed files with 292 additions and 201 deletions.
diff --git a/eZ/Publish/API/Repository/LocationService.php b/eZ/Publish/API/Repository/LocationService.php
@@ -44,11 +44,12 @@ public function copySubtree(Location $subtree, Location $targetParentLocation);
      * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException If the specified location is not found
      *
      * @param mixed $locationId
-     * @param string[]|null $prioritizedLanguages Used as prioritized language code on translated properties of returned object.
+     * @param string[]|null $prioritizedLanguages Filter on and use as prioritized language code on translated properties of returned object.
+     * @param bool|null $useAlwaysAvailable Respect always available flag on content when filtering on $prioritizedLanguages.
      *
      * @return \eZ\Publish\API\Repository\Values\Content\Location
      */
-    public function loadLocation($locationId, array $prioritizedLanguages = null);
+    public function loadLocation($locationId, array $prioritizedLanguages = null, bool $useAlwaysAvailable = null);
 
     /**
      * Loads a location object from its $remoteId.
@@ -57,11 +58,12 @@ public function loadLocation($locationId, array $prioritizedLanguages = null);
      * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException If the specified location is not found
      *
      * @param string $remoteId
-     * @param string[]|null $prioritizedLanguages Used as prioritized language code on translated properties of returned object.
+     * @param string[]|null $prioritizedLanguages Filter on and use as prioritized language code on translated properties of returned object.
+     * @param bool|null $useAlwaysAvailable Respect always available flag on content when filtering on $prioritizedLanguages.
      *
      * @return \eZ\Publish\API\Repository\Values\Content\Location
      */
-    public function loadLocationByRemoteId($remoteId, array $prioritizedLanguages = null);
+    public function loadLocationByRemoteId($remoteId, array $prioritizedLanguages = null, bool $useAlwaysAvailable = null);
 
     /**
      * Loads the locations for the given content object.

diff --git a/eZ/Publish/API/Repository/Tests/BaseTest.php b/eZ/Publish/API/Repository/Tests/BaseTest.php
@@ -11,7 +11,7 @@
 use Doctrine\DBAL\Connection;
 use eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException;
 use eZ\Publish\API\Repository\Tests\PHPUnitConstraint\ValidationErrorOccurs as PHPUnitConstraintValidationErrorOccurs;
-use eZ\Publish\API\Repository\Values\Content\Location;
+use eZ\Publish\API\Repository\Values\Content\Language;
 use EzSystems\EzPlatformSolrSearchEngine\Tests\SetupFactory\LegacySetupFactory as LegacySolrSetupFactory;
 use PHPUnit\Framework\TestCase;
 use eZ\Publish\API\Repository\Repository;
@@ -717,4 +717,26 @@ protected function createFolder(array $names, $parentLocationId)
 
         return $contentService->publishVersion($contentDraft->versionInfo);
     }
+
+    /**
+     * Add new Language to the Repository.
+     *
+     * @param string $languageCode
+     * @param string $name
+     * @param bool $enabled
+     *
+     * @return \eZ\Publish\API\Repository\Values\Content\Language
+     */
+    protected function createLanguage(string $languageCode, string $name, bool $enabled = true): Language
+    {
+        $repository = $this->getRepository(false);
+
+        $languageService = $repository->getContentLanguageService();
+        $languageStruct = $languageService->newLanguageCreateStruct();
+        $languageStruct->name = $name;
+        $languageStruct->languageCode = $languageCode;
+        $languageStruct->enabled = $enabled;
+
+        return $languageService->createLanguage($languageStruct);
+    }
 }
diff --git a/eZ/Publish/API/Repository/Tests/LanguageServiceTest.php b/eZ/Publish/API/Repository/Tests/LanguageServiceTest.php
@@ -548,26 +548,6 @@ public function testGetDefaultLanguageCode()
         );
     }
 
-    /**
-     * Helper method that creates a new language test fixture in the
-     * API implementation under test.
-     *
-     * @return \eZ\Publish\API\Repository\Values\Content\Language
-     */
-    private function createLanguage()
-    {
-        $repository = $this->getRepository();
-
-        $languageService = $repository->getContentLanguageService();
-        $languageCreate = $languageService->newLanguageCreateStruct();
-
-        $languageCreate->enabled = false;
-        $languageCreate->name = 'English';
-        $languageCreate->languageCode = 'eng-US';
-
-        return $languageService->createLanguage($languageCreate);
-    }
-
     /**
      * Test for the createLanguage() method.
      *

diff --git a/eZ/Publish/API/Repository/Tests/LocationServiceTest.php b/eZ/Publish/API/Repository/Tests/LocationServiceTest.php
@@ -478,11 +478,7 @@ public function testLoadLocationPrioritizedLanguagesFallback()
         $repository = $this->getRepository();
 
         // Add a language
-        $languageService = $repository->getContentLanguageService();
-        $languageStruct = $languageService->newLanguageCreateStruct();
-        $languageStruct->name = 'Norsk';
-        $languageStruct->languageCode = 'nor-NO';
-        $languageService->createLanguage($languageStruct);
+        $this->createLanguage('nor-NO', 'Norsk');
 
         $locationService = $repository->getLocationService();
         $contentService = $repository->getContentService();
@@ -495,7 +491,7 @@ public function testLoadLocationPrioritizedLanguagesFallback()
         $draft = $contentService->updateContent($draft->getVersionInfo(), $struct);
         $contentService->publishVersion($draft->getVersionInfo());
 
-        // Load with prioritc language (fallback will be the old one)
+        // Load with priority language (fallback will be the old one)
         $location = $locationService->loadLocation(5, ['nor-NO']);
 
         $this->assertInstanceOf(
@@ -513,6 +509,24 @@ public function testLoadLocationPrioritizedLanguagesFallback()
         $this->assertEquals($content->getVersionInfo()->getName('eng-US'), 'Users');
     }
 
+    /**
+     * Test that accessing lazy-loaded Content without a translation in the specific
+     * not available language throws NotFoundException.
+     */
+    public function testLoadLocationThrowsNotFoundExceptionForNotAvailableContent(): void
+    {
+        $repository = $this->getRepository();
+
+        $locationService = $repository->getLocationService();
+
+        $this->createLanguage('pol-PL', 'Polski');
+
+        $this->expectException(NotFoundException::class);
+
+        // Note: relying on existing database fixtures to make test case more readable
+        $locationService->loadLocation(60, ['pol-PL']);
+    }
+
     /**
      * Test for the loadLocation() method.
      *

diff --git a/eZ/Publish/Core/Persistence/Cache/LocationHandler.php b/eZ/Publish/Core/Persistence/Cache/LocationHandler.php
@@ -21,15 +21,19 @@ class LocationHandler extends AbstractHandler implements LocationHandlerInterfac
     /**
      * {@inheritdoc}
      */
-    public function load($locationId)
+    public function load($locationId, array $translations = null, bool $useAlwaysAvailable = true)
     {
-        $cacheItem = $this->cache->getItem("ez-location-${locationId}");
+        $translationsKey = $this->getCacheTranslationKey($translations, $useAlwaysAvailable);
+        $cacheItem = $this->cache->getItem("ez-location-${locationId}-${translationsKey}");
         if ($cacheItem->isHit()) {
             return $cacheItem->get();
         }
 
-        $this->logger->logCall(__METHOD__, array('location' => $locationId));
-        $location = $this->persistenceHandler->locationHandler()->load($locationId);
+        $this->logger->logCall(
+            __METHOD__,
+            ['location' => $locationId, 'translations' => $translations, 'always-available' => $useAlwaysAvailable]
+        );
+        $location = $this->persistenceHandler->locationHandler()->load($locationId, $translations, $useAlwaysAvailable);
 
         $cacheItem->set($location);
         $cacheItem->tag($this->getCacheTags($location));
@@ -53,9 +57,9 @@ public function loadSubtreeIds($locationId)
 
         $cacheItem->set($locationIds);
         $cacheTags = ['location-' . $locationId, 'location-path-' . $locationId];
-        foreach ($locationIds as $locationId) {
-            $cacheTags[] = 'location-' . $locationId;
-            $cacheTags[] = 'location-path-' . $locationId;
+        foreach ($locationIds as $id) {
+            $cacheTags[] = 'location-' . $id;
+            $cacheTags[] = 'location-path-' . $id;
         }
         $cacheItem->tag($cacheTags);
         $this->cache->save($cacheItem);
@@ -120,15 +124,19 @@ public function loadParentLocationsForDraftContent($contentId)
     /**
      * {@inheritdoc}
      */
-    public function loadByRemoteId($remoteId)
+    public function loadByRemoteId($remoteId, array $translations = null, bool $useAlwaysAvailable = true)
     {
-        $cacheItem = $this->cache->getItem("ez-location-${remoteId}-by-remoteid");
+        $translationsKey = $this->getCacheTranslationKey($translations, $useAlwaysAvailable);
+        $cacheItem = $this->cache->getItem("ez-location-remoteid-${remoteId}-${translationsKey}");
         if ($cacheItem->isHit()) {
             return $cacheItem->get();
         }
 
-        $this->logger->logCall(__METHOD__, array('location' => $remoteId));
-        $location = $this->persistenceHandler->locationHandler()->loadByRemoteId($remoteId);
+        $this->logger->logCall(
+            __METHOD__,
+            ['location' => $remoteId, 'translations' => $translations, 'always-available' => $useAlwaysAvailable]
+        );
+        $location = $this->persistenceHandler->locationHandler()->loadByRemoteId($remoteId, $translations, $useAlwaysAvailable);
 
         $cacheItem->set($location);
         $cacheItem->tag($this->getCacheTags($location));
@@ -280,27 +288,6 @@ public function changeMainLocation($contentId, $locationId)
         $this->cache->invalidateTags(['content-' . $contentId]);
     }
 
-    /**
-     * Return relevant content and location tags so cache can be purged reliably.
-     *
-     * @param Location $location
-     * @param array $tags Optional, can be used to specify additional tags.
-     *
-     * @return array
-     */
-    private function getCacheTags(Location $location, $tags = [])
-    {
-        $tags[] = 'content-' . $location->contentId;
-        $tags[] = 'location-' . $location->id;
-        $tags[] = 'location-data-' . $location->id;
-        foreach (explode('/', trim($location->pathString, '/')) as $pathId) {
-            $tags[] = 'location-path-' . $pathId;
-            $tags[] = 'location-path-data-' . $pathId;
-        }
-
-        return $tags;
-    }
-
     /**
      * Get the total number of all existing Locations. Can be combined with loadAllLocations.
      *
@@ -327,4 +314,37 @@ public function loadAllLocations($offset, $limit)
 
         return $this->persistenceHandler->locationHandler()->loadAllLocations($offset, $limit);
     }
+
+    /**
+     * Return relevant content and location tags so cache can be purged reliably.
+     *
+     * @param \eZ\Publish\SPI\Persistence\Content\Location $location
+     * @param array $tags Optional, can be used to specify additional tags.
+     *
+     * @return array
+     */
+    private function getCacheTags(Location $location, $tags = [])
+    {
+        $tags[] = 'content-' . $location->contentId;
+        $tags[] = 'location-' . $location->id;
+        $tags[] = 'location-data-' . $location->id;
+        foreach (explode('/', trim($location->pathString, '/')) as $pathId) {
+            $tags[] = 'location-path-' . $pathId;
+            $tags[] = 'location-path-data-' . $pathId;
+        }
+
+        return $tags;
+    }
+
+    private function getCacheTranslationKey(array $translations = null, bool $useAlwaysAvailable = true): string
+    {
+        if (empty($translations)) {
+            return (int)$useAlwaysAvailable;
+        }
+
+        // Sort array as we don't care about order in location handler usage & want to optimize for cache hits.
+        sort($translations);
+
+        return implode('|', $translations) . '|' . (int)$useAlwaysAvailable;
+    }
 }
diff --git a/eZ/Publish/Core/Persistence/Cache/Tests/LocationHandlerTest.php b/eZ/Publish/Core/Persistence/Cache/Tests/LocationHandlerTest.php
@@ -30,7 +30,7 @@ public function getHandlerClassName(): string
 
     public function providerForUnCachedMethods(): array
     {
-        // string $method, array $arguments, array? $tags, string? $key
+        // string $method, array $arguments, array? $tags, string? $key, mixed? $returnValue
         return [
             ['copySubtree', [12, 45]],
             ['move', [12, 45], ['location-path-12']],
@@ -51,14 +51,16 @@ public function providerForCachedLoadMethods(): array
     {
         $location = new Location(['id' => 12]);
 
-        // string $method, array $arguments, string $key, mixed? $data
+        // string $method, array $arguments, string $key, mixed? $data, bool? $multi = false
         return [
-            ['load', [12], 'ez-location-12', $location],
+            ['load', [12], 'ez-location-12-1', $location],
+            ['load', [12, ['eng-GB', 'bra-PG'], false], 'ez-location-12-bra-PG|eng-GB|0', $location],
             ['loadSubtreeIds', [12], 'ez-location-subtree-12', [33, 44]],
             ['loadLocationsByContent', [4, 12], 'ez-content-locations-4-root-12', [$location]],
             ['loadLocationsByContent', [4], 'ez-content-locations-4', [$location]],
             ['loadParentLocationsForDraftContent', [4], 'ez-content-locations-4-parentForDraft', [$location]],
-            ['loadByRemoteId', ['34fe5y4'], 'ez-location-34fe5y4-by-remoteid', $location],
+            ['loadByRemoteId', ['34fe5y4'], 'ez-location-remoteid-34fe5y4-1', $location],
+            ['loadByRemoteId', ['34fe5y4', ['eng-GB', 'arg-ES']], 'ez-location-remoteid-34fe5y4-arg-ES|eng-GB|1', $location],
         ];
     }
 }
diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Language/MaskGenerator.php b/eZ/Publish/Core/Persistence/Legacy/Content/Language/MaskGenerator.php
@@ -9,6 +9,7 @@
 namespace eZ\Publish\Core\Persistence\Legacy\Content\Language;
 
 use eZ\Publish\SPI\Persistence\Content\Language\Handler as LanguageHandler;
+use RuntimeException;
 
 /**
  * Language MaskGenerator.
@@ -48,6 +49,11 @@ public function generateLanguageMask(array $languages)
         }
 
         foreach ($languages as $language => $value) {
+            if (is_int($language)) {
+                throw new RuntimeException(
+                    "Expected flipped array with language codes as keys, got int key: $language"
+                );
+            }
             $mask |= $this->languageHandler->loadByLanguageCode($language)->id;
         }
 

diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway.php b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway.php
@@ -33,27 +33,28 @@ abstract class Gateway
     /**
      * Returns an array with basic node data.
      *
-     * We might want to cache this, since this method is used by about every
-     * method in the location handler.
-     *
-     * @todo optimize
+     * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
      *
      * @param mixed $nodeId
+     * @param string[]|null $translations
+     * @param bool $useAlwaysAvailable Respect always available flag on content when filtering on $translations.
      *
      * @return array
      */
-    abstract public function getBasicNodeData($nodeId);
+    abstract public function getBasicNodeData($nodeId, array $translations = null, bool $useAlwaysAvailable = true);
 
     /**
      * Returns an array with basic node data for the node with $remoteId.
      *
-     * @todo optimize
+     * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
      *
      * @param mixed $remoteId
+     * @param string[]|null $translations
+     * @param bool $useAlwaysAvailable Respect always available flag on content when filtering on $translations.
      *
      * @return array
      */
-    abstract public function getBasicNodeDataByRemoteId($remoteId);
+    abstract public function getBasicNodeDataByRemoteId($remoteId, array $translations = null, bool $useAlwaysAvailable = true);
 
     /**
      * Loads data for all Locations for $contentId, optionally only in the