Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EZP-29613: As a developer I want access to ContentType on Content to avoid re-loading it #2444

Merged
merged 25 commits into from Dec 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4200ba5
Inital simple addtion of ->getContentType API
andrerom Aug 27, 2018
5f57d9b
Adapt FieldRenderingExtension to use Contnet->getCotnentType()
andrerom Aug 27, 2018
7e12146
Optimize FieldHelper and it's usage in ContentExtension
andrerom Aug 31, 2018
b2d3c54
Optimize RestExecutedView to use cotnent->getContentType()
andrerom Aug 31, 2018
9e8f3ff
Implement getContentType() on Core User & UserGroup object
andrerom Sep 8, 2018
1310656
Add test coverage for ->getContentType()
andrerom Sep 8, 2018
374fc3d
Improve PHP doc on SPI/Persistence/Content/Type
andrerom Sep 11, 2018
06199f6
Add SPI method for multi load content types
andrerom Sep 8, 2018
f7fce4f
Adapt content domain mappers to bulk load content types
andrerom Sep 12, 2018
c96e3c3
Move back to use array for key suffixes
andrerom Sep 12, 2018
64df1ab
Adapt unit tests
andrerom Sep 14, 2018
c44bacc
Add unit test for Legacy/Content/Type/Handler::loadContentTypeList()
andrerom Sep 15, 2018
205a006
fix review comments
andrerom Sep 18, 2018
cbda26e
Change MemoryCachingHandler to correctly use loadContentTypeList() an…
andrerom Sep 18, 2018
5683a26
Change to use Doctrine QueryBuilder to fix loadTypesDataList(), use i…
andrerom Sep 18, 2018
498b8df
Fix Leagacy Search test to keep connection, also refactor to avoid du…
andrerom Sep 18, 2018
1cf087a
Expose ContentTypeService->loadContentTypeList() in order to adapt fo…
andrerom Sep 18, 2018
cde64c5
Remove a not so relevant todo
andrerom Sep 18, 2018
48fd929
CS + REST client fix
andrerom Sep 18, 2018
e06fa6b
Make sure MemoryCachingHandler cache is array type to avoid operand i…
andrerom Sep 19, 2018
a3f8697
Fix casting issue in Persistence/Legacy/Content/Type/MemoryCachingHan…
andrerom Sep 21, 2018
6e197f7
Change back to sort by content class identifier
andrerom Sep 23, 2018
f9259df
Fix tests after merge issue
andrerom Oct 19, 2018
fa9ee61
Fix review comments
andrerom Nov 16, 2018
c89fb22
Use QueryBuilder for where() conditions
andrerom Nov 17, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -180,7 +180,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