Skip to content

Commit

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

API:
* Initial simple addition of `Content->getContentType()` API
* Implement getContentType() on Core User & UserGroup object
* Add SPI method for  multi load content types
* Improve PHP doc on SPI/Persistence/Content/Type
* Adapt content domain mappers to bulk load content types
* Add unit test for Legacy/Content/Type/Handler::loadContentTypeList()
* Change MemoryCachingHandler to correctly use loadContentTypeList() and not single load()
* Change to use Doctrine QueryBuilder to fix loadTypesDataList(), use it for all loads for test coverage and consistency
* Fix Legacy Search test to keep connection, also refactor to avoid duplication

API:
* Expose `ContentTypeService->loadContentTypeList()` in order to adapt for new loadContentListByContentInfo()

Usage in Core, aka get rid of ContentType load operations:
* Adapt FieldRenderingExtension to use Content->getContentType()
* Optimize FieldHelper and it's usage in ContentExtension
* Optimize RestExecutedView to use content->getContentType()
  • Loading branch information
andrerom committed Dec 4, 2018
1 parent 727c999 commit 0dc4f63
Show file tree
Hide file tree
Showing 46 changed files with 785 additions and 636 deletions.
Expand Up @@ -179,7 +179,6 @@ services:
class: "%ezpublish.twig.extension.field_rendering.class%"
arguments:
- "@ezpublish.templating.field_block_renderer"
- "@ezpublish.api.service.content_type"
- "@ezpublish.fieldType.parameterProviderRegistry"
- "@ezpublish.translation_helper"
tags:
Expand Down
Expand Up @@ -709,7 +709,7 @@ services:
ezpublish_rest.output.value_object_visitor.RestExecutedView:
parent: ezpublish_rest.output.value_object_visitor.base
class: "%ezpublish_rest.output.value_object_visitor.RestExecutedView.class%"
arguments: [ "@ezpublish.api.service.location", "@ezpublish.api.service.content", "@ezpublish.api.service.content_type" ]
arguments: [ "@ezpublish.api.service.location", "@ezpublish.api.service.content" ]
tags:
- { name: ezpublish_rest.output.value_object_visitor, type: eZ\Publish\Core\REST\Server\Values\RestExecutedView }

Expand Down
14 changes: 14 additions & 0 deletions eZ/Publish/API/Repository/ContentTypeService.php
Expand Up @@ -161,6 +161,20 @@ public function loadContentTypeByRemoteId($remoteId, array $prioritizedLanguages
*/
public function loadContentTypeDraft($contentTypeId);

/**
* Bulk-load Content Type objects by ids.
*
* Note: it does not throw exceptions on load, just ignores erroneous items.
*
* @since 7.3
*
* @param mixed[] $contentTypeIds
* @param string[] $prioritizedLanguages Used as prioritized language code on translated properties of returned object.
*
* @return \eZ\Publish\API\Repository\Values\ContentType\ContentType[]|iterable
*/
public function loadContentTypeList(array $contentTypeIds, array $prioritizedLanguages = []): iterable;

/**
* Get Content Type objects which belong to the given content type group.
*
Expand Down
58 changes: 58 additions & 0 deletions eZ/Publish/API/Repository/Tests/ContentServiceTest.php
Expand Up @@ -264,6 +264,35 @@ public function testCreateContentSetsExpectedVersionInfo($content)
$this->assertFalse($content->getVersionInfo()->isArchived());
}

/**
* Test for the createContent() method.
*
* @param \eZ\Publish\API\Repository\Values\Content\Content $content
*
* @see \eZ\Publish\API\Repository\ContentService::createContent()
* @depends testCreateContent
*/
public function testCreateContentSetsExpectedContentType($content)
{
$contentType = $content->getContentType();

$this->assertEquals(
[
$contentType->id,
// Won't match as it's set to true in createContentDraftVersion1()
//$contentType->defaultAlwaysAvailable,
//$contentType->defaultSortField,
//$contentType->defaultSortOrder,
],
[
$content->contentInfo->contentTypeId,
//$content->contentInfo->alwaysAvailable,
//$location->sortField,
//$location->sortOrder,
]
);
}

/**
* Test for the createContent() method.
*
Expand Down Expand Up @@ -1008,6 +1037,35 @@ public function testPublishVersionSetsExpectedVersionInfo($content)
$this->assertFalse($content->getVersionInfo()->isArchived());
}

/**
* Test for the publishVersion() method.
*
* @param \eZ\Publish\API\Repository\Values\Content\Content $content
*
* @see \eZ\Publish\API\Repository\ContentService::publishVersion()
* @depends testPublishVersion
*/
public function testPublishVersionSetsExpectedContentType($content)
{
$contentType = $content->getContentType();

$this->assertEquals(
[
$contentType->id,
// won't be a match as it's set to true in createContentDraftVersion1()
//$contentType->defaultAlwaysAvailable,
//$contentType->defaultSortField,
//$contentType->defaultSortOrder,
],
[
$content->contentInfo->contentTypeId,
//$content->contentInfo->alwaysAvailable,
//$location->sortField,
//$location->sortOrder,
]
);
}

/**
* Test for the publishVersion() method.
*
Expand Down
38 changes: 35 additions & 3 deletions eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php
Expand Up @@ -1132,7 +1132,7 @@ public function testCreateContentTypeThrowsContentTypeFieldDefinitionValidationE
/**
* Test for the createContentTypeGroup() method called with no groups.
*
* @depends \eZ\Publish\API\Repository\Tests\ContentTypeServiceTest::testCreateContentType
* @depends testCreateContentType
* @expectedException \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
* @expectedExceptionMessage Argument '$contentTypeGroups' is invalid: Argument must contain at least one ContentTypeGroup
* @covers \eZ\Publish\Core\Repository\ContentTypeService::createContentTypeGroup
Expand Down Expand Up @@ -2348,8 +2348,12 @@ public function testPublishContentTypeDraftRefreshesContentTypesList()

$contentTypeDraft = $this->createContentTypeDraft();

// Make sure to 1. check draft is not part of lists, and 2. warm cache to make sure it invalidates
$contentTypes = $contentTypeService->loadContentTypeList([1, $contentTypeDraft->id]);
self::assertArrayNotHasKey($contentTypeDraft->id, $contentTypes);
self::assertCount(1, $contentTypes);

$contentTypeGroups = $contentTypeDraft->getContentTypeGroups();
// load Content Types for Groups of the new Content Type Draft to populate cache before publishing
foreach ($contentTypeGroups as $contentTypeGroup) {
$contentTypes = $contentTypeService->loadContentTypes($contentTypeGroup);
// check if not published Content Type does not exist on published Content Types list
Expand All @@ -2366,7 +2370,11 @@ function (ContentType $contentType) {

$contentTypeService->publishContentTypeDraft($contentTypeDraft);

// load Content Types for the Groups of the new Content Type
// After publishing it should be part of lists
$contentTypes = $contentTypeService->loadContentTypeList([1, $contentTypeDraft->id]);
self::assertArrayHasKey($contentTypeDraft->id, $contentTypes);
self::assertCount(2, $contentTypes);

foreach ($contentTypeGroups as $contentTypeGroup) {
$contentTypes = $contentTypeService->loadContentTypes($contentTypeGroup);
// check if published Content is available in published Content Types list
Expand Down Expand Up @@ -2800,6 +2808,30 @@ public function testLoadContentTypeByRemoteIdThrowsNotFoundException()
/* END: Use Case */
}

/**
* Test for the loadContentTypeList() method.
*
* @see \eZ\Publish\API\Repository\ContentTypeService::loadContentTypeList()
* @depends testLoadContentType
*/
public function testLoadContentTypeList()
{
$repository = $this->getRepository();
$contentTypeService = $repository->getContentTypeService();

$types = $contentTypeService->loadContentTypeList([3, 4]);

$this->assertInternalType('iterable', $types);

$this->assertEquals(
[
3 => $contentTypeService->loadContentType(3),
4 => $contentTypeService->loadContentType(4),
],
$types
);
}

/**
* Test for the loadContentTypes() method.
*
Expand Down
8 changes: 8 additions & 0 deletions eZ/Publish/API/Repository/Values/Content/Content.php
Expand Up @@ -9,6 +9,7 @@
namespace eZ\Publish\API\Repository\Values\Content;

use eZ\Publish\API\Repository\Values\ValueObject;
use eZ\Publish\API\Repository\Values\ContentType\ContentType;

/**
* this class represents a content object in a specific version.
Expand Down Expand Up @@ -97,4 +98,11 @@ abstract public function getFieldsByLanguage($languageCode = null);
* @return \eZ\Publish\API\Repository\Values\Content\Field|null A {@link Field} or null if nothing is found
*/
abstract public function getField($fieldDefIdentifier, $languageCode = null);

/**
* Returns the ContentType for this content.
*
* @return \eZ\Publish\API\Repository\Values\ContentType\ContentType
*/
abstract public function getContentType(): ContentType;
}
4 changes: 3 additions & 1 deletion eZ/Publish/Core/Helper/FieldHelper.php
Expand Up @@ -49,7 +49,7 @@ public function __construct(TranslationHelper $translationHelper, ContentTypeSer
public function isFieldEmpty(Content $content, $fieldDefIdentifier, $forcedLanguage = null)
{
$field = $this->translationHelper->getTranslatedField($content, $fieldDefIdentifier, $forcedLanguage);
$fieldDefinition = $this->getFieldDefinition($content->contentInfo, $fieldDefIdentifier);
$fieldDefinition = $content->getContentType()->getFieldDefinition($fieldDefIdentifier);

return $this
->fieldTypeService
Expand All @@ -60,6 +60,8 @@ public function isFieldEmpty(Content $content, $fieldDefIdentifier, $forcedLangu
/**
* Returns FieldDefinition object based on $contentInfo and $fieldDefIdentifier.
*
* @deprecated If you have Content you can instead do: $content->getContentType()->getFieldDefinition($identifier)
*
* @param ContentInfo $contentInfo
* @param string $fieldDefIdentifier
*
Expand Down
18 changes: 8 additions & 10 deletions eZ/Publish/Core/Helper/Tests/FieldHelperTest.php
Expand Up @@ -78,11 +78,10 @@ public function testIsFieldEmpty()
->with($fieldDefIdentifier)
->will($this->returnValue($fieldDefinition));

$this->contentTypeServiceMock
->expects($this->once())
->method('loadContentType')
->with($contentTypeId)
->will($this->returnValue($contentType));
$content
->expects($this->any())
->method('getContentType')
->willReturn($contentType);

$this->translationHelper
->expects($this->once())
Expand Down Expand Up @@ -125,11 +124,10 @@ public function testIsFieldNotEmpty()
->with($fieldDefIdentifier)
->will($this->returnValue($fieldDefinition));

$this->contentTypeServiceMock
->expects($this->once())
->method('loadContentType')
->with($contentTypeId)
->will($this->returnValue($contentType));
$content
->expects($this->any())
->method('getContentType')
->willReturn($contentType);

$this->translationHelper
->expects($this->once())
Expand Down
Expand Up @@ -9,7 +9,6 @@
namespace eZ\Publish\Core\MVC\Symfony\Templating\Tests\Twig\Extension;

use eZ\Publish\API\Repository\ContentService;
use eZ\Publish\API\Repository\ContentTypeService;
use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\API\Repository\Values\Content\Field;
use eZ\Publish\Core\Helper\TranslationHelper;
Expand Down Expand Up @@ -69,7 +68,6 @@ public function getExtensions()
return array(
new FieldRenderingExtension(
$fieldBlockRenderer,
$this->getContentTypeServiceMock(),
$this->createMock(ParameterProviderRegistryInterface::class),
new TranslationHelper(
$configResolver,
Expand Down Expand Up @@ -129,6 +127,12 @@ protected function getContent($contentTypeIdentifier, array $fieldsData, array $
$content = new Content(
array(
'internalFields' => $fields,
'contentType' => new ContentType([
'id' => $contentTypeIdentifier,
'identifier' => $contentTypeIdentifier,
'mainLanguageCode' => 'fre-FR',
'fieldDefinitions' => $this->fieldDefinitions[$contentTypeIdentifier],
]),
'versionInfo' => new VersionInfo(
array(
'versionNo' => 64,
Expand Down Expand Up @@ -176,30 +180,4 @@ private function getConfigResolverMock()

return $mock;
}

/**
* @return \PHPUnit\Framework\MockObject\MockObject
*/
protected function getContentTypeServiceMock()
{
$mock = $this->createMock(ContentTypeService::class);

$mock->expects($this->any())
->method('loadContentType')
->will(
$this->returnCallback(
function ($contentTypeId) {
return new ContentType(
array(
'identifier' => $contentTypeId,
'mainLanguageCode' => 'fre-FR',
'fieldDefinitions' => $this->fieldDefinitions[$contentTypeId],
)
);
}
)
);

return $mock;
}
}
Expand Up @@ -288,10 +288,9 @@ private function getContentType(ValueObject $content)
public function getFirstFilledImageFieldIdentifier(Content $content)
{
foreach ($content->getFieldsByLanguage() as $field) {
$fieldTypeIdentifier = $this->fieldHelper->getFieldDefinition(
$content->contentInfo,
$field->fieldDefIdentifier
)->fieldTypeIdentifier;
$fieldTypeIdentifier = $content->getContentType()
->getFieldDefinition($field->fieldDefIdentifier)
->fieldTypeIdentifier;

if ($fieldTypeIdentifier !== 'ezimage') {
continue;
Expand Down
Expand Up @@ -8,7 +8,6 @@
*/
namespace eZ\Publish\Core\MVC\Symfony\Templating\Twig\Extension;

use eZ\Publish\API\Repository\ContentTypeService;
use eZ\Publish\API\Repository\Values\Content\Content;
use eZ\Publish\API\Repository\Values\Content\Field;
use eZ\Publish\API\Repository\Values\ContentType\FieldDefinition;
Expand All @@ -30,11 +29,6 @@ class FieldRenderingExtension extends Twig_Extension
*/
private $fieldBlockRenderer;

/**
* @var ContentTypeService
*/
private $contentTypeService;

/**
* @var ParameterProviderRegistryInterface
*/
Expand All @@ -54,12 +48,10 @@ class FieldRenderingExtension extends Twig_Extension

public function __construct(
FieldBlockRendererInterface $fieldBlockRenderer,
ContentTypeService $contentTypeService,
ParameterProviderRegistryInterface $parameterProviderRegistry,
TranslationHelper $translationHelper
) {
$this->fieldBlockRenderer = $fieldBlockRenderer;
$this->contentTypeService = $contentTypeService;
$this->parameterProviderRegistry = $parameterProviderRegistry;
$this->translationHelper = $translationHelper;
}
Expand Down Expand Up @@ -152,7 +144,7 @@ private function getRenderFieldBlockParameters(Content $content, Field $field, a

$versionInfo = $content->getVersionInfo();
$contentInfo = $versionInfo->getContentInfo();
$contentType = $this->contentTypeService->loadContentType($contentInfo->contentTypeId);
$contentType = $content->getContentType();
$fieldDefinition = $contentType->getFieldDefinition($field->fieldDefIdentifier);
// Adding Field, FieldSettings and ContentInfo objects to
// parameters to be passed to the template
Expand Down Expand Up @@ -192,11 +184,10 @@ private function getRenderFieldBlockParameters(Content $content, Field $field, a
*/
private function getFieldTypeIdentifier(Content $content, Field $field)
{
$contentInfo = $content->getVersionInfo()->getContentInfo();
$key = $contentInfo->contentTypeId . ' ' . $field->fieldDefIdentifier;
$contentType = $content->getContentType();
$key = $contentType->id . ' ' . $field->fieldDefIdentifier;

if (!isset($this->fieldTypeIdentifiers[$key])) {
$contentType = $this->contentTypeService->loadContentType($contentInfo->contentTypeId);
$this->fieldTypeIdentifiers[$key] = $contentType
->getFieldDefinition($field->fieldDefIdentifier)
->fieldTypeIdentifier;
Expand Down

0 comments on commit 0dc4f63

Please sign in to comment.