Skip to content

Commit

Permalink
EZP-28956: Expose lazy loaded Location->getContent() on API for bette…
Browse files Browse the repository at this point in the history
…r DX (ezsystems#2328)

* Add SPI ContentHandler::loadContentList() for bulk loading content

Note on cache:
- By design here version is skipped, as use case for loading lots of content is in current version.
- So one downside for cache here is that it won't reuse cache from load() method, causing cache to be duplcated in current design.

Possible ways to remedy:
- Force having to provide versions also on loadContentList() making it less efficient for some use cases (Page builder scenarios for instance)
- Allow $version on load() to be false, or expose a loadContentInCurrentVersion() kind of method, either way adapt usage for this so cache will be shared.

* Add Location->getContent() using lazy properties

* [Integration] Add coverage for Location->getContent()

* Change SearchService->findContent() to bulk load content

Now that we have method to load several at once, take advantage in findContent().

* [ContentViewBuilder] Take advantage of Location->getContent() to avoid duplicate loading

* Refactor for LoadStruct usage in SPI

* fix review comments

* Add internal ContentInfo proxy for ContentProxy usage

* Make Trash item contain Content as well (as it extends Location)

* Fix review comments

* Don't use unset() on properties when they are no longer needed

* Fix phpdoc on Abstract(Cache)Handler
  • Loading branch information
andrerom authored and Łukasz Serwatka committed Jun 25, 2018
1 parent 65f512f commit fe0390a
Show file tree
Hide file tree
Showing 29 changed files with 1,003 additions and 341 deletions.
15 changes: 10 additions & 5 deletions eZ/Publish/API/Repository/LocationService.php
Expand Up @@ -44,10 +44,11 @@ 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.
*
* @return \eZ\Publish\API\Repository\Values\Content\Location
*/
public function loadLocation($locationId);
public function loadLocation($locationId, array $prioritizedLanguages = null);

/**
* Loads a location object from its $remoteId.
Expand All @@ -56,10 +57,11 @@ public function loadLocation($locationId);
* @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.
*
* @return \eZ\Publish\API\Repository\Values\Content\Location
*/
public function loadLocationByRemoteId($remoteId);
public function loadLocationByRemoteId($remoteId, array $prioritizedLanguages = null);

/**
* Loads the locations for the given content object.
Expand All @@ -71,30 +73,33 @@ public function loadLocationByRemoteId($remoteId);
*
* @param \eZ\Publish\API\Repository\Values\Content\ContentInfo $contentInfo
* @param \eZ\Publish\API\Repository\Values\Content\Location $rootLocation
* @param string[]|null $prioritizedLanguages Used as prioritized language code on translated properties of returned object.
*
* @return \eZ\Publish\API\Repository\Values\Content\Location[] An array of {@link Location}
*/
public function loadLocations(ContentInfo $contentInfo, Location $rootLocation = null);
public function loadLocations(ContentInfo $contentInfo, Location $rootLocation = null, array $prioritizedLanguages = null);

/**
* Loads children which are readable by the current user of a location object sorted by sortField and sortOrder.
*
* @param \eZ\Publish\API\Repository\Values\Content\Location $location
* @param int $offset the start offset for paging
* @param int $limit the number of locations returned
* @param string[]|null $prioritizedLanguages Used as prioritized language code on translated properties of returned object.
*
* @return \eZ\Publish\API\Repository\Values\Content\LocationList
*/
public function loadLocationChildren(Location $location, $offset = 0, $limit = 25);
public function loadLocationChildren(Location $location, $offset = 0, $limit = 25, array $prioritizedLanguages = null);

/**
* Load parent Locations for Content Draft.
*
* @param \eZ\Publish\API\Repository\Values\Content\VersionInfo $versionInfo
* @param string[]|null $prioritizedLanguages Used as prioritized language code on translated properties of returned object.
*
* @return \eZ\Publish\API\Repository\Values\Content\Location[] List of parent Locations
*/
public function loadParentLocationsForDraftContent(VersionInfo $versionInfo);
public function loadParentLocationsForDraftContent(VersionInfo $versionInfo, array $prioritizedLanguages = null);

/**
* Returns the number of children which are readable by the current user of a location object.
Expand Down
47 changes: 47 additions & 0 deletions eZ/Publish/API/Repository/Tests/LocationServiceTest.php
Expand Up @@ -464,6 +464,53 @@ public function testLoadLocationStructValues(Location $location)
$location->contentInfo
);
$this->assertEquals($this->generateId('object', 4), $location->contentInfo->id);

// Check lazy loaded proxy on ->content
$this->assertInstanceOf(
Content::class,
$content = $location->getContent()
);
$this->assertEquals(4, $content->contentInfo->id);
}

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);

$locationService = $repository->getLocationService();
$contentService = $repository->getContentService();
$location = $locationService->loadLocation(5);

// Translate "Users"
$draft = $contentService->createContentDraft($location->contentInfo);
$struct = $contentService->newContentUpdateStruct();
$struct->setField('name', 'Brukere', 'nor-NO');
$draft = $contentService->updateContent($draft->getVersionInfo(), $struct);
$contentService->publishVersion($draft->getVersionInfo());

// Load with prioritc language (fallback will be the old one)
$location = $locationService->loadLocation(5, ['nor-NO']);

$this->assertInstanceOf(
Location::class,
$location
);
self::assertEquals(5, $location->id);
$this->assertInstanceOf(
Content::class,
$content = $location->getContent()
);
$this->assertEquals(4, $content->contentInfo->id);

$this->assertEquals($content->getVersionInfo()->getName(), 'Brukere');
$this->assertEquals($content->getVersionInfo()->getName('eng-US'), 'Users');
}

/**
Expand Down
7 changes: 7 additions & 0 deletions eZ/Publish/API/Repository/Tests/TrashServiceTest.php
Expand Up @@ -10,6 +10,7 @@

use eZ\Publish\API\Repository\Repository;
use eZ\Publish\API\Repository\URLAliasService;
use eZ\Publish\API\Repository\Values\Content\Content;
use eZ\Publish\API\Repository\Values\Content\Location as APILocation;
use eZ\Publish\API\Repository\Values\Content\LocationCreateStruct;
use eZ\Publish\API\Repository\Values\Content\Query;
Expand Down Expand Up @@ -254,6 +255,12 @@ public function testLoadTrashItem()
$trashItem,
$trashItemReloaded
);

$this->assertInstanceOf(
Content::class,
$content = $trashItemReloaded->getContent()
);
$this->assertEquals($trashItem->contentId, $content->contentInfo->id);
}

/**
Expand Down
13 changes: 13 additions & 0 deletions eZ/Publish/API/Repository/Values/Content/Location.php
Expand Up @@ -193,6 +193,19 @@ public function isDraft()
*/
protected $sortOrder;

/**
* @var \eZ\Publish\API\Repository\Values\Content\Content
*/
protected $content;

/**
* @return \eZ\Publish\API\Repository\Values\Content\Content
*/
public function getContent(): Content
{
return $this->content;
}

/**
* Get SortClause objects built from Locations's sort options.
*
Expand Down
10 changes: 10 additions & 0 deletions eZ/Publish/Core/MVC/Symfony/View/Builder/ContentViewBuilder.php
Expand Up @@ -97,6 +97,9 @@ public function buildView(array $parameters)

if (isset($parameters['content'])) {
$content = $parameters['content'];
} elseif ($location instanceof Location) {
// if we already have location load content true it so we avoid dual loading in case user does that in view
$content = $location->getContent();
} else {
if (isset($parameters['contentId'])) {
$contentId = $parameters['contentId'];
Expand All @@ -115,6 +118,13 @@ public function buildView(array $parameters)
if ($location->contentId !== $content->id) {
throw new InvalidArgumentException('Location', 'Provided location does not belong to selected content');
}

if (isset($parameters['contentId']) && $location->contentId !== $parameters['contentId']) {
throw new InvalidArgumentException(
'Location',
'Provided location does not belong to selected content as requested via contentId parameter'
);
}
} elseif (isset($this->locationLoader)) {
try {
$location = $this->locationLoader->loadLocation($content->contentInfo);
Expand Down
15 changes: 12 additions & 3 deletions eZ/Publish/Core/Persistence/Cache/AbstractHandler.php
Expand Up @@ -56,19 +56,24 @@ public function __construct(
* Cache items must be stored with a key in the following format "${keyPrefix}${id}", like "ez-content-info-${id}",
* in order for this method to be able to prefix key on id's and also extract key prefix afterwards.
*
* It also optionally supports a key suffixs, for use on a variable argument that affects all lookups,
* like translations, i.e. "ez-content-${id}-${translationKey}" where $keySuffixes = [$id => "-${translationKey}"].
*
* @param array $ids
* @param string $keyPrefix
* @param string $keyPrefix E.g "ez-content-"
* @param callable $missingLoader Function for loading missing objects, gets array with missing id's as argument,
* expects return value to be array with id as key. Missing items should be missing.
* @param callable $loadedTagger Function for tagging loaded object, gets object as argument, return array of tags.
* @param array $keySuffixes Optional, key is id as provided in $ids, and value is a key suffix e.g. "-eng-Gb"
*
* @return array
*/
final protected function getMultipleCacheItems(
array $ids,
string $keyPrefix,
callable $missingLoader,
callable $loadedTagger
callable $loadedTagger,
array $keySuffixes = []
): array {
if (empty($ids)) {
return [];
Expand All @@ -77,7 +82,7 @@ final protected function getMultipleCacheItems(
// Generate unique cache keys
$cacheKeys = [];
foreach (array_unique($ids) as $id) {
$cacheKeys[] = $keyPrefix . $id;
$cacheKeys[] = $keyPrefix . $id . ($keySuffixes[$id] ?? '');
}

// Load cache items by cache keys (will contain hits and misses)
Expand All @@ -86,6 +91,10 @@ final protected function getMultipleCacheItems(
$keyPrefixLength = strlen($keyPrefix);
foreach ($this->cache->getItems($cacheKeys) as $key => $cacheItem) {
$id = substr($key, $keyPrefixLength);
if (!empty($keySuffixes)) {
$id = explode('-', $id, 2)[0];
}

if ($cacheItem->isHit()) {
$list[$id] = $cacheItem->get();
} else {
Expand Down
35 changes: 35 additions & 0 deletions eZ/Publish/Core/Persistence/Cache/ContentHandler.php
Expand Up @@ -10,6 +10,7 @@

use eZ\Publish\API\Repository\Values\Content\Relation as APIRelation;
use eZ\Publish\SPI\Persistence\Content\Handler as ContentHandlerInterface;
use eZ\Publish\SPI\Persistence\Content;
use eZ\Publish\SPI\Persistence\Content\VersionInfo;
use eZ\Publish\SPI\Persistence\Content\ContentInfo;
use eZ\Publish\SPI\Persistence\Content\CreateStruct;
Expand Down Expand Up @@ -77,6 +78,40 @@ public function load($contentId, $versionNo, array $translations = null)
return $content;
}

public function loadContentList(array $contentLoadStructs): array
{
// Extract id's and make key suffix for each one (handling undefined versionNo and languages)
$contentIds = [];
$keySuffixes = [];
foreach ($contentLoadStructs as $struct) {
$contentIds[] = $struct->id;
$keySuffixes[$struct->id] = ($struct->versionNo ? "-{$struct->versionNo}-" : '-') .
(empty($struct->languages) ? self::ALL_TRANSLATIONS_KEY : implode('|', $struct->languages));
}

return $this->getMultipleCacheItems(
$contentIds,
'ez-content-',
function (array $cacheMissIds) use ($contentLoadStructs) {
$this->logger->logCall(__CLASS__ . '::loadContentList', ['content' => $cacheMissIds]);

$filteredStructs = [];
/* @var $contentLoadStructs \eZ\Publish\SPI\Persistence\Content\LoadStruct[] */
foreach ($contentLoadStructs as $struct) {
if (in_array($struct->id, $cacheMissIds)) {
$filteredStructs[] = $struct;
}
}

return $this->persistenceHandler->contentHandler()->loadContentList($filteredStructs);
},
function (Content $content) {
return $this->getCacheTags($content->versionInfo->contentInfo, true);
},
$keySuffixes
);
}

/**
* {@inheritdoc}
*/
Expand Down
Expand Up @@ -69,10 +69,12 @@ public function providerForCachedLoadMethods(): array
$version = new VersionInfo(['versionNo' => 1, 'contentInfo' => $info]);
$content = new Content(['fields' => [], 'versionInfo' => $version]);

// string $method, array $arguments, string $key, mixed? $data
// string $method, array $arguments, string $key, mixed? $data, bool $multi = false
return [
['load', [2, 1], 'ez-content-2-1-' . ContentHandler::ALL_TRANSLATIONS_KEY, $content],
['load', [2, 1, ['eng-GB', 'eng-US']], 'ez-content-2-1-eng-GB|eng-US', $content],
['loadContentList', [[new Content\LoadStruct(['id' => 2, 'versionNo' => 3])]], 'ez-content-2-3-' . ContentHandler::ALL_TRANSLATIONS_KEY, [2 => $content], true],
['loadContentList', [[new Content\LoadStruct(['id' => 5, 'languages' => ['eng-GB', 'eng-US']])]], 'ez-content-5-eng-GB|eng-US', [5 => $content], true],
['loadContentInfo', [2], 'ez-content-info-2', $info],
['loadContentInfoList', [[2]], 'ez-content-info-2', [2 => $info], true],
['loadContentInfoByRemoteId', ['3d8jrj'], 'ez-content-info-byRemoteId-3d8jrj', $info],
Expand Down
10 changes: 10 additions & 0 deletions eZ/Publish/Core/Persistence/Legacy/Content/Gateway.php
Expand Up @@ -151,6 +151,16 @@ abstract public function updateNonTranslatableField(
*/
abstract public function load($contentId, $version, array $translations = null);

/**
* Loads current version for a list of content objects.
*
* @param array[] $IdVersionTranslationPairs Hashes with 'id', optionally 'version', & optionally 'languages'
* If version is not set current version will be loaded, if languages is not set ALL will be loaded.
*
* @return array[]
*/
abstract public function loadContentList(array $IdVersionTranslationPairs): array;

/**
* Loads info for a content object identified by its remote ID.
*
Expand Down

0 comments on commit fe0390a

Please sign in to comment.