Skip to content

Commit

Permalink
EZP-29613: As a developer I want access ContentType on Content to avo…
Browse files Browse the repository at this point in the history
…id re-loading it

Uses ezsystems/ezpublish-kernel#2444, and $location->getContent()
(v2.2) in order avoid several repository loads not needed anymore.

Here are some numbers for cache lookups on clean install, expect difference to be bigger when you have more data (demo or project):

dashboard:
418 / 1240 (33.71%) _(master)_
468 / 1315 (35.59%) _(with kernel PR, only)_*
364 / 1043 (34.9%) _(with this PR + kernel PR)_

content structure:
478 / 1401 (34.12%)  _(master)_
450 / 1359 (33.11%) _(with kernel PR, only)_
436 / 1327 (32.86%) _(with this PR + kernel PR)_

_* So withouth this patch there is a slight regression with more cache loads with the kernel PR on dashboard.
Probably due to kernel now always loading content types from API instead of falling back to using SPI,
and via api language loading and mapping is needed. With this PR we get rid of the duplicated loads,
and for other apps the extra lanfguage load and much more will go away once we have inMemory cache again (v2.4)._
  • Loading branch information
andrerom committed Sep 19, 2018
1 parent 6180670 commit 8d502aa
Show file tree
Hide file tree
Showing 19 changed files with 59 additions and 96 deletions.
5 changes: 1 addition & 4 deletions src/bundle/Controller/ContentViewController.php
Expand Up @@ -183,10 +183,7 @@ private function supplyPathLocations(ContentView $view): void
*/
private function supplyContentType(ContentView $view): void
{
$content = $view->getContent();
$contentType = $this->contentTypeService->loadContentType($content->contentInfo->contentTypeId, $this->siteAccessLanguages);

$view->addParameters(['contentType' => $contentType]);
$view->addParameters(['contentType' => $view->getContent()->getContentType()]);
}

/**
Expand Down
14 changes: 3 additions & 11 deletions src/bundle/Controller/LocationController.php
Expand Up @@ -116,11 +116,7 @@ public function moveAction(Request $request): Response
$location = $data->getLocation();
$newParentLocation = $data->getNewParentLocation();

$newParentContentType = $this->contentTypeService->loadContentType(
$newParentLocation->getContentInfo()->contentTypeId
);

if (!$newParentContentType->isContainer) {
if (!$newParentLocation->getContent()->getContentType()->isContainer) {
throw new InvalidArgumentException(
'$newParentLocation',
'Cannot move location to a parent that is not a container'
Expand Down Expand Up @@ -172,11 +168,7 @@ public function copyAction(Request $request): Response
$location = $data->getLocation();
$newParentLocation = $data->getNewParentLocation();

$newParentContentType = $this->contentTypeService->loadContentType(
$newParentLocation->getContentInfo()->contentTypeId
);

if (!$newParentContentType->isContainer) {
if (!$newParentLocation->getContent()->getContentType()->isContainer) {
throw new InvalidArgumentException(
'$newParentLocation',
'Cannot copy location to a parent that is not a container'
Expand Down Expand Up @@ -282,7 +274,7 @@ public function swapAction(Request $request): Response
$newLocation = $data->getNewLocation();

$childCount = $this->locationService->getLocationChildCount($currentLocation);
$contentType = $this->contentTypeService->loadContentType($newLocation->getContentInfo()->contentTypeId);
$contentType = $newLocation->getContent()->getContentType();

if (!$contentType->isContainer && $childCount) {
throw new \InvalidArgumentException(
Expand Down
2 changes: 1 addition & 1 deletion src/bundle/Controller/SectionController.php
Expand Up @@ -227,7 +227,7 @@ public function viewSectionContentAction(Section $section, int $page = 1, int $l
$assignedContent[] = [
'id' => $content->id,
'name' => $content->getName(),
'type' => $this->contentTypeService->loadContentType($content->contentInfo->contentTypeId)->getName(),
'type' => $content->getContentType()->getName(),
'path' => $this->pathService->loadPathLocations(
$this->locationService->loadLocation($content->contentInfo->mainLocationId)
),
Expand Down
2 changes: 1 addition & 1 deletion src/bundle/Controller/TrashController.php
Expand Up @@ -145,7 +145,7 @@ public function listAction(Request $request): Response

/** @var \eZ\Publish\API\Repository\Values\Content\TrashItem $item */
foreach ($pagerfanta->getCurrentPageResults() as $item) {
$contentType = $this->contentTypeService->loadContentType($item->contentInfo->contentTypeId);
$contentType = $item->getContent()->getContentType();
$ancestors = $this->uiPathService->loadPathLocations($item);

$trashItemsList[] = new TrashItemData($item, $contentType, $ancestors);
Expand Down
Expand Up @@ -53,9 +53,8 @@ public function onFilterViewParameters(FilterViewParametersEvent $event)
}

$contentInfo = $view->getContent()->contentInfo;
$contentType = $this->contentTypeService->loadContentType(
$contentInfo->contentTypeId
);
$contentType = $view->getContent()->getContentType();

$event->getParameterBag()->add([
'form' => $view->getFormView(),
'location' => $view->getLocation(),
Expand Down
5 changes: 1 addition & 4 deletions src/lib/EventListener/RequestAttributesListener.php
Expand Up @@ -72,10 +72,7 @@ public function addRequestAttributes(FilterViewBuilderParametersEvent $event)
/** @var Location $location */
$location = $parameterBag->get('location');

$languageCode = $parameterBag->get('languageCode') ?? $location->contentInfo->mainLanguageCode;

$content = $this->loadContent($location->contentInfo->id, $languageCode);
$parameterBag->set('content', $content);
$parameterBag->set('content', $location->getContent());
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/Menu/ContentCreateRightSidebarBuilder.php
Expand Up @@ -91,7 +91,7 @@ public function createStructure(array $options): ItemInterface
$parentLocation = $options['parentLocation'];
/** @var \eZ\Publish\API\Repository\Values\ContentType\ContentType $contentType */
$contentType = $options['content_type'];
$parentContentType = $this->contentTypeService->loadContentType($parentLocation->contentInfo->contentTypeId);
$parentContentType = $parentLocation->getContent()->getContentType();
/** @var \eZ\Publish\API\Repository\Values\Content\Language $language */
$language = $options['language'];
/** @var \Knp\Menu\ItemInterface|\Knp\Menu\ItemInterface[] $menu */
Expand Down
7 changes: 3 additions & 4 deletions src/lib/Specification/Location/IsContainer.php
Expand Up @@ -9,6 +9,7 @@
namespace EzSystems\EzPlatformAdminUi\Specification\Location;

use EzSystems\EzPlatformAdminUi\Specification\AbstractSpecification;
use eZ\Publish\API\Repository\Values\Content\Location;

class IsContainer extends AbstractSpecification
{
Expand All @@ -30,10 +31,8 @@ public function __construct($contentTypeService)
*
* @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
*/
public function isSatisfiedBy($item): bool
public function isSatisfiedBy(Location $item): bool
{
return $this->contentTypeService->loadContentType(
$item->getContentInfo()->contentTypeId
)->isContainer;
return $item->getContent()->getContentType()->isContainer;
}
}
12 changes: 10 additions & 2 deletions src/lib/Tab/Dashboard/MyDraftsTab.php
Expand Up @@ -128,10 +128,18 @@ public function renderView(array $parameters): string
$pager->setCurrentPage($page);

$data = [];
$contentTypes = $contentTypeIds = [];
/** @var \eZ\Publish\API\Repository\Values\Content\VersionInfo $version */
foreach ($pager as $version) {
$contentTypeIds = $version->getContentInfo()->contentTypeId;
}

if (!empty($contentTypeIds)) {
$contentTypes = $this->contentTypeService->loadContentType(array_unique($contentTypeIds));
}

foreach ($pager as $version) {
$contentInfo = $version->getContentInfo();
$contentType = $this->contentTypeService->loadContentType($contentInfo->contentTypeId);

if (null === $contentInfo->mainLocationId) {
$locations = $this->locationService->loadParentLocationsForDraftContent($version);
Expand All @@ -144,7 +152,7 @@ public function renderView(array $parameters): string
$data[] = [
'contentId' => $contentInfo->id,
'name' => $version->getName(),
'type' => $contentType->getName(),
'type' => $contentTypes[$contentInfo->contentTypeId]->getName(),
'language' => $version->initialLanguageCode,
'version' => $version->versionNo,
'modified' => $version->modificationDate,
Expand Down
4 changes: 2 additions & 2 deletions src/lib/Tab/Dashboard/PagerContentToDataMapper.php
Expand Up @@ -50,7 +50,7 @@ public function map(Pagerfanta $pager): array
$data = [];

foreach ($pager as $content) {
$contentInfo = $this->contentService->loadContentInfo($content->id);
$contentInfo = $content->getVersionInfo()->getContentInfo();

$contributor = (new UserExists($this->userService))->isSatisfiedBy($contentInfo->ownerId)
? $this->userService->loadUser($contentInfo->ownerId) : null;
Expand All @@ -61,7 +61,7 @@ public function map(Pagerfanta $pager): array
'language' => $contentInfo->mainLanguageCode,
'contributor' => $contributor,
'version' => $content->versionInfo->versionNo,
'type' => $this->contentTypeService->loadContentType($contentInfo->contentTypeId)->getName(),
'type' => $content->getContentType()->getName(),
'modified' => $content->versionInfo->modificationDate,
'initialLanguageCode' => $content->versionInfo->initialLanguageCode,
];
Expand Down
23 changes: 9 additions & 14 deletions src/lib/Tab/LocationView/RelationsTab.php
Expand Up @@ -106,22 +106,17 @@ public function renderView(array $parameters): string
{
/** @var Content $content */
$content = $parameters['content'];
$versionInfo = $content->getVersionInfo();
$relationsDataset = $this->datasetFactory->relations();
$relationsDataset->load($versionInfo);
$relationsDataset->load($content);

$contentTypes = [];
$contentTypeIds = [];

$relations = $relationsDataset->getRelations();

$viewParameters = [];

foreach ($relations as $relation) {
$contentTypeId = $relation->getDestinationContentInfo()->contentTypeId;

if (!isset($contentTypes[$contentTypeId])) {
$contentTypes[$contentTypeId] = $this->contentTypeService->loadContentType($contentTypeId);
}
$contentTypeIds[] = $relation->getDestinationContentInfo()->contentTypeId;
}

$viewParameters['relations'] = $relations;
Expand All @@ -130,17 +125,17 @@ public function renderView(array $parameters): string
$reverseRelations = $relationsDataset->getReverseRelations();

foreach ($reverseRelations as $relation) {
$contentTypeId = $relation->getSourceContentInfo()->contentTypeId;

if (!isset($contentTypes[$contentTypeId])) {
$contentTypes[$contentTypeId] = $this->contentTypeService->loadContentType($contentTypeId);
}
$contentTypeIds[] = $relation->getSourceContentInfo()->contentTypeId;
}

$viewParameters['reverse_relations'] = $reverseRelations;
}

$viewParameters['contentTypes'] = $contentTypes;
if (!empty($contentTypeIds)) {
$viewParameters['contentTypes'] = $this->contentTypeService->loadContentTypeList(array_unique($contentTypeIds));
} else {
$viewParameters['contentTypes'] = [];
}

return $this->twig->render(
'@ezdesign/content/tab/relations/tab.html.twig',
Expand Down
7 changes: 1 addition & 6 deletions src/lib/UI/Config/Provider/User.php
Expand Up @@ -70,12 +70,7 @@ public function getConfig(): array
*/
private function resolveProfilePictureField(ApiUser $user): ?Field
{
try {
$contentType = $this->contentTypeService->loadContentType($user->contentInfo->contentTypeId);
} catch (\Exception $e) {
return null;
}

$contentType = $user->getContentType();
foreach ($user->getFields() as $field) {
$fieldDef = $contentType->getFieldDefinition($field->fieldDefIdentifier);

Expand Down
4 changes: 2 additions & 2 deletions src/lib/UI/Dataset/PoliciesDataset.php
Expand Up @@ -84,8 +84,8 @@ public function __construct(
public function load(Location $location): self
{
$roleAssignments = [];
$content = $this->contentService->loadContentByContentInfo($location->contentInfo);
$contentType = $this->contentTypeService->loadContentType($content->contentInfo->contentTypeId);
$content = $location->getContent();
$contentType = $content->getContentType();

if ((new ContentTypeIsUser($this->userContentTypeIdentifier))->isSatisfiedBy($contentType)) {
$user = $this->userService->loadUser($content->id);
Expand Down
10 changes: 5 additions & 5 deletions src/lib/UI/Dataset/RelationsDataset.php
Expand Up @@ -10,7 +10,7 @@

use eZ\Publish\API\Repository\ContentService;
use eZ\Publish\API\Repository\Exceptions\UnauthorizedException;
use eZ\Publish\API\Repository\Values\Content\VersionInfo;
use eZ\Publish\API\Repository\Values\Content\Content;
use EzSystems\EzPlatformAdminUi\UI\Value\ValueFactory;
use EzSystems\EzPlatformAdminUi\UI\Value as UIValue;

Expand Down Expand Up @@ -47,16 +47,16 @@ public function __construct(ContentService $contentService, ValueFactory $valueF
*
* @throws UnauthorizedException
*/
public function load(VersionInfo $versionInfo): self
public function load(Content $content): self
{
$contentInfo = $versionInfo->getContentInfo();
$versionInfo = $content->getVersionInfo();

foreach ($this->contentService->loadRelations($versionInfo) as $relation) {
$this->relations[] = $this->valueFactory->createRelation($relation, $contentInfo);
$this->relations[] = $this->valueFactory->createRelation($relation, $content);
}

foreach ($this->contentService->loadReverseRelations($versionInfo->getContentInfo()) as $reverseRelation) {
$this->reverseRelations[] = $this->valueFactory->createRelation($reverseRelation, $contentInfo);
$this->reverseRelations[] = $this->valueFactory->createRelation($reverseRelation, $content);
}

return $this;
Expand Down
5 changes: 3 additions & 2 deletions src/lib/UI/Dataset/RolesDataset.php
Expand Up @@ -83,9 +83,10 @@ public function __construct(
public function load(Location $location): self
{
$roleAssignment = [];
$content = $this->contentService->loadContentByContentInfo($location->contentInfo);
$contentType = $this->contentTypeService->loadContentType($content->contentInfo->contentTypeId);
$content = $location->getContent();
$contentType = $content->getContentType();

// @todo $content should just have been instance of User or UserGroup direclty so we don't need to re-load data
if ((new ContentTypeIsUser($this->userContentTypeIdentifier))->isSatisfiedBy($contentType)) {
$user = $this->userService->loadUser($content->id);
$roleAssignment = $this->roleService->getRoleAssignmentsForUser($user, true);
Expand Down
15 changes: 5 additions & 10 deletions src/lib/UI/Module/Subitems/ContentViewParameterSupplier.php
Expand Up @@ -108,14 +108,13 @@ public function supply(ContentView $view)

$locationChildren = $this->locationService->loadLocationChildren($location, 0, $this->subitemsLimit);
foreach ($locationChildren->locations as $locationChild) {
$contentInfo = $locationChild->getContentInfo();
$contentType = $this->contentTypeService->loadContentType($contentInfo->contentTypeId);
$contentType = $locationChild->getContent()->getContentType();

if (!isset($contentTypes[$contentType->identifier])) {
$contentTypes[$contentType->identifier] = $contentType;
}

$subitemsRows[] = $this->createSubitemsRow($locationChild, $contentInfo, $contentType);
$subitemsRows[] = $this->createSubitemsRow($locationChild, $contentType);
}

$subitemsList = new SubitemsList($subitemsRows, $childrenCount);
Expand All @@ -135,7 +134,6 @@ public function supply(ContentView $view)
}

/**
* @param \eZ\Publish\API\Repository\Values\Content\ContentInfo $contentInfo
* @param \eZ\Publish\API\Repository\Values\Content\Location $location
* @param \eZ\Publish\API\Repository\Values\ContentType\ContentType $contentType
*
Expand All @@ -145,14 +143,13 @@ public function supply(ContentView $view)
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
*/
private function createRestContent(
ContentInfo $contentInfo,
Location $location,
ContentType $contentType
): RestContent {
return new RestContent(
$contentInfo,
$location->getContentInfo(),
$location,
$this->contentService->loadContentByContentInfo($contentInfo),
$location->getContent(),
$contentType,
[]
);
Expand All @@ -173,7 +170,6 @@ private function createRestLocation(Location $location): RestLocation

/**
* @param \eZ\Publish\API\Repository\Values\Content\Location $location
* @param \eZ\Publish\API\Repository\Values\Content\ContentInfo $contentInfo
* @param \eZ\Publish\API\Repository\Values\ContentType\ContentType $contentType
*
* @return \EzSystems\EzPlatformAdminUi\UI\Module\Subitems\Values\SubitemsRow
Expand All @@ -183,11 +179,10 @@ private function createRestLocation(Location $location): RestLocation
*/
private function createSubitemsRow(
Location $location,
ContentInfo $contentInfo,
ContentType $contentType
): SubitemsRow {
$restLocation = $this->createRestLocation($location);
$restContent = $this->createRestContent($contentInfo, $location, $contentType);
$restContent = $this->createRestContent($location, $contentType);

return new SubitemsRow($restLocation, $restContent);
}
Expand Down
13 changes: 6 additions & 7 deletions src/lib/UI/Value/ValueFactory.php
Expand Up @@ -15,6 +15,7 @@
use eZ\Publish\API\Repository\PermissionResolver;
use eZ\Publish\API\Repository\SearchService;
use eZ\Publish\API\Repository\UserService;
use eZ\Publish\API\Repository\Values\Content\Content;
use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\API\Repository\Values\Content\Language;
use eZ\Publish\API\Repository\Values\Content\Location;
Expand Down Expand Up @@ -136,16 +137,16 @@ public function createLanguage(Language $language, VersionInfo $versionInfo): UI

/**
* @param Relation $relation
* @param ContentInfo $contentInfo
* @param Content $content
*
* @return UIValue\Content\Relation
*
* @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
*/
public function createRelation(Relation $relation, ContentInfo $contentInfo): UIValue\Content\Relation
public function createRelation(Relation $relation, Content $content): UIValue\Content\Relation
{
$contentType = $this->contentTypeService->loadContentType($contentInfo->contentTypeId);
$contentType = $content->getContentType();
$fieldDefinition = $contentType->getFieldDefinition($relation->sourceFieldDefinitionIdentifier);

return new UIValue\Content\Relation($relation, [
Expand Down Expand Up @@ -248,10 +249,8 @@ public function createBookmark(Location $location): UIValue\Location\Bookmark
return new UIValue\Location\Bookmark(
$location,
[
'contentType' => $this->contentTypeService->loadContentType($location->contentInfo->contentTypeId),
'pathLocations' => $this->pathService->loadPathLocations(
$this->locationService->loadLocation($location->contentInfo->mainLocationId)
),
'contentType' => $location->getContent()->getContentType(),
'pathLocations' => $this->pathService->loadPathLocations($location),
'userCanEdit' => $this->permissionResolver->canUser('content', 'edit', $location->contentInfo),
]
);
Expand Down

0 comments on commit 8d502aa

Please sign in to comment.