diff --git a/eZ/Bundle/EzPublishCoreBundle/ApiLoader/RepositoryFactory.php b/eZ/Bundle/EzPublishCoreBundle/ApiLoader/RepositoryFactory.php index 76707d1b2ca..b5ceb665e56 100644 --- a/eZ/Bundle/EzPublishCoreBundle/ApiLoader/RepositoryFactory.php +++ b/eZ/Bundle/EzPublishCoreBundle/ApiLoader/RepositoryFactory.php @@ -9,6 +9,7 @@ namespace eZ\Bundle\EzPublishCoreBundle\ApiLoader; use eZ\Publish\Core\MVC\ConfigResolverInterface; +use eZ\Publish\Core\Repository\Helper\RelationProcessor; use eZ\Publish\Core\Repository\Values\User\UserReference; use eZ\Publish\Core\Search\Common\BackgroundIndexer; use eZ\Publish\SPI\Persistence\Handler as PersistenceHandler; @@ -84,13 +85,15 @@ public function __construct( * @param \eZ\Publish\SPI\Persistence\Handler $persistenceHandler * @param \eZ\Publish\SPI\Search\Handler $searchHandler * @param \eZ\Publish\Core\Search\Common\BackgroundIndexer $backgroundIndexer + * @param \eZ\Publish\Core\Repository\Helper\RelationProcessor $relationProcessor * * @return \eZ\Publish\API\Repository\Repository */ public function buildRepository( PersistenceHandler $persistenceHandler, SearchHandler $searchHandler, - BackgroundIndexer $backgroundIndexer + BackgroundIndexer $backgroundIndexer, + RelationProcessor $relationProcessor ) { $config = $this->container->get('ezpublish.api.repository_configuration_provider')->getRepositoryConfig(); @@ -98,6 +101,7 @@ public function buildRepository( $persistenceHandler, $searchHandler, $backgroundIndexer, + $relationProcessor, array( 'fieldType' => $this->fieldTypeCollectionFactory->getFieldTypes(), 'nameableFieldTypes' => $this->fieldTypeNameableCollectionFactory->getNameableFieldTypes(), diff --git a/eZ/Publish/API/Repository/Tests/BaseTest.php b/eZ/Publish/API/Repository/Tests/BaseTest.php index e89ff4eda90..002e73bb011 100644 --- a/eZ/Publish/API/Repository/Tests/BaseTest.php +++ b/eZ/Publish/API/Repository/Tests/BaseTest.php @@ -8,6 +8,8 @@ */ namespace eZ\Publish\API\Repository\Tests; +use eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException; +use eZ\Publish\API\Repository\Tests\PHPUnitConstraint\ValidationErrorOccurs as PHPUnitConstraintValidationErrorOccurs; use EzSystems\EzPlatformSolrSearchEngine\Tests\SetupFactory\LegacySetupFactory as LegacySolrSetupFactory; use PHPUnit\Framework\TestCase; use eZ\Publish\API\Repository\Repository; @@ -564,4 +566,19 @@ public function createRoleWithPolicies($roleName, array $policiesData) return $roleService->loadRole($roleDraft->id); } + + /** + * Traverse all errors for all fields in all Translations to find expected one. + * + * @param \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException $exception + * @param string $expectedValidationErrorMessage + */ + protected function assertValidationErrorOccurs( + ContentFieldValidationException $exception, + $expectedValidationErrorMessage + ) { + $constraint = new PHPUnitConstraintValidationErrorOccurs($expectedValidationErrorMessage); + + self::assertThat($exception, $constraint); + } } diff --git a/eZ/Publish/API/Repository/Tests/ContentServiceTest.php b/eZ/Publish/API/Repository/Tests/ContentServiceTest.php index 78e18735002..b4d0c456f04 100644 --- a/eZ/Publish/API/Repository/Tests/ContentServiceTest.php +++ b/eZ/Publish/API/Repository/Tests/ContentServiceTest.php @@ -21,6 +21,7 @@ use eZ\Publish\API\Repository\Values\User\Limitation\LocationLimitation; use eZ\Publish\API\Repository\Values\User\Limitation\ContentTypeLimitation; use eZ\Publish\API\Repository\Exceptions\NotFoundException; +use DOMDocument; use Exception; /** @@ -1655,6 +1656,60 @@ public function testUpdateContentThrowsContentFieldValidationException() /* END: Use Case */ } + /** + * Test for the updateContent() method. + * + * @covers \eZ\Publish\API\Repository\ContentService::updateContent() + * @depends eZ\Publish\API\Repository\Tests\ContentServiceTest::testUpdateContent + */ + public function testUpdateContentValidatorIgnoresRequiredFieldsOfNotUpdatedLanguages() + { + $repository = $this->getRepository(); + /* BEGIN: Use Case */ + $contentTypeService = $repository->getContentTypeService(); + $contentType = $contentTypeService->loadContentTypeByIdentifier('folder'); + + // Create multilangual content + $contentService = $repository->getContentService(); + $contentCreate = $contentService->newContentCreateStruct($contentType, 'eng-US'); + $contentCreate->setField('name', 'An awesome Sidelfingen folder', 'eng-US'); + $contentCreate->setField('name', 'An awesome Sidelfingen folder', 'eng-GB'); + + $contentDraft = $contentService->createContent($contentCreate); + + // 2. Update content type definition + $contentTypeDraft = $contentTypeService->createContentTypeDraft($contentType); + + $fieldDefinition = $contentType->getFieldDefinition('description'); + $fieldDefinitionUpdate = $contentTypeService->newFieldDefinitionUpdateStruct(); + $fieldDefinitionUpdate->identifier = 'description'; + $fieldDefinitionUpdate->isRequired = true; + + $contentTypeService->updateFieldDefinition( + $contentTypeDraft, + $fieldDefinition, + $fieldDefinitionUpdate + ); + $contentTypeService->publishContentTypeDraft($contentTypeDraft); + + // 3. Update only eng-US translation + $description = new DOMDocument(); + $description->loadXML(<< +
+ Lorem ipsum dolor +
+XML + ); + + $contentUpdate = $contentService->newContentUpdateStruct(); + $contentUpdate->setField('name', 'An awesome Sidelfingen folder (updated)', 'eng-US'); + $contentUpdate->setField('description', $description); + + $contentService->updateContent($contentDraft->getVersionInfo(), $contentUpdate); + /* END: Use Case */ + } + /** * Test for the updateContent() method. * diff --git a/eZ/Publish/API/Repository/Tests/FieldType/BaseIntegrationTest.php b/eZ/Publish/API/Repository/Tests/FieldType/BaseIntegrationTest.php index 4438964d974..ab42ab37504 100644 --- a/eZ/Publish/API/Repository/Tests/FieldType/BaseIntegrationTest.php +++ b/eZ/Publish/API/Repository/Tests/FieldType/BaseIntegrationTest.php @@ -309,8 +309,16 @@ protected function createContentType($fieldSettings, $validatorConfiguration, ar $repository = $this->getRepository(); $contentTypeService = $repository->getContentTypeService(); + $contentTypeIdentifier = 'test-' . $this->getTypeName(); + + try { + return $contentTypeService->loadContentTypeByIdentifier($contentTypeIdentifier); + } catch (Repository\Exceptions\NotFoundException $e) { + // Move on to creating Content Type + } + $createStruct = $contentTypeService->newContentTypeCreateStruct( - 'test-' . $this->getTypeName() + $contentTypeIdentifier ); $createStruct->mainLanguageCode = $this->getOverride('mainLanguageCode', $typeCreateOverride, 'eng-GB'); $createStruct->remoteId = $this->getTypeName(); @@ -539,10 +547,16 @@ protected function createContent($fieldData, $contentType = null) * * @param array $names Content names in the form of [languageCode => name] * @param array $fieldData FT-specific data in the form of [languageCode => data] + * @param \eZ\Publish\API\Repository\Values\Content\LocationCreateStruct[] $locationCreateStructs * * @return \eZ\Publish\API\Repository\Values\Content\Content + * + * @throws \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException + * @throws \eZ\Publish\API\Repository\Exceptions\ContentValidationException + * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException + * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException */ - protected function createMultilingualContent(array $names, array $fieldData) + protected function createMultilingualContent(array $names, array $fieldData, array $locationCreateStructs = []) { self::assertEquals(array_keys($names), array_keys($fieldData), 'Languages passed to names and data differ'); @@ -567,7 +581,7 @@ protected function createMultilingualContent(array $names, array $fieldData) $createStruct->remoteId = md5(uniqid('', true) . microtime()); $createStruct->alwaysAvailable = true; - return $contentService->createContent($createStruct); + return $contentService->createContent($createStruct, $locationCreateStructs); } /** diff --git a/eZ/Publish/API/Repository/Tests/FieldType/RichTextIntegrationTest.php b/eZ/Publish/API/Repository/Tests/FieldType/RichTextIntegrationTest.php index cf22c238513..4e8584064c1 100644 --- a/eZ/Publish/API/Repository/Tests/FieldType/RichTextIntegrationTest.php +++ b/eZ/Publish/API/Repository/Tests/FieldType/RichTextIntegrationTest.php @@ -10,6 +10,8 @@ use DirectoryIterator; use eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException; +use eZ\Publish\API\Repository\Repository; +use eZ\Publish\API\Repository\Values\Content\Location; use eZ\Publish\Core\FieldType\RichText\Value as RichTextValue; use eZ\Publish\API\Repository\Values\Content\Field; use DOMDocument; @@ -722,6 +724,157 @@ protected function createDocument($filename) return $document; } + /** + * Prepare Content structure with link to deleted Location. + * + * @param \eZ\Publish\API\Repository\Repository $repository + * + * @return array [$deletedLocation, $brokenContent] + * + * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException + * @throws \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException + * @throws \eZ\Publish\API\Repository\Exceptions\ContentValidationException + * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException + * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException + */ + private function prepareInternalLinkValidatorBrokenLinksTestCase(Repository $repository) + { + $contentService = $repository->getContentService(); + $locationService = $repository->getLocationService(); + + // Create first content with single Language + $primaryContent = $contentService->publishVersion( + $this->createMultilingualContent( + ['eng-US' => 'ContentA'], + ['eng-US' => $this->getValidCreationFieldData()], + [$locationService->newLocationCreateStruct(2)] + )->versionInfo + ); + // Create secondary Location (to be deleted) for the first Content + $deletedLocation = $locationService->createLocation( + $primaryContent->contentInfo, + $locationService->newLocationCreateStruct(60) + ); + + // Create second Content with two Languages, one of them linking to secondary Location + $brokenContent = $contentService->publishVersion( + $this->createMultilingualContent( + [ + 'eng-US' => 'ContentB', + 'eng-GB' => 'ContentB', + ], + [ + 'eng-US' => $this->getValidCreationFieldData(), + 'eng-GB' => $this->getDocumentWithLocationLink($deletedLocation), + ], + [$locationService->newLocationCreateStruct(2)] + )->versionInfo + ); + + // delete Location making second Content broken + $locationService->deleteLocation($deletedLocation); + + return [$deletedLocation, $brokenContent]; + } + + /** + * Test updating Content which contains links to deleted Location doesn't fail when updating not broken field only. + * + * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException + * @throws \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException + * @throws \eZ\Publish\API\Repository\Exceptions\ContentValidationException + * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException + * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException + */ + public function testInternalLinkValidatorIgnoresMissingRelationOnNotUpdatedField() + { + $repository = $this->getRepository(); + $contentService = $repository->getContentService(); + + list(, $contentB) = $this->prepareInternalLinkValidatorBrokenLinksTestCase($repository); + + // update field w/o erroneous link to trigger validation + $contentUpdateStruct = $contentService->newContentUpdateStruct(); + $contentUpdateStruct->setField('data', $this->getValidUpdateFieldData(), 'eng-US'); + + $contentDraftB = $contentService->updateContent( + $contentService->createContentDraft($contentB->contentInfo)->versionInfo, + $contentUpdateStruct + ); + + $contentService->publishVersion($contentDraftB->versionInfo); + } + + /** + * Test updating Content which contains links to deleted Location fails when updating broken field. + * + * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException + * @throws \eZ\Publish\API\Repository\Exceptions\ContentValidationException + * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException + * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException + * @throws \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException + */ + public function testInternalLinkValidatorReturnsErrorOnMissingRelationInUpdatedField() + { + $repository = $this->getRepository(); + $contentService = $repository->getContentService(); + + list($deletedLocation, $brokenContent) = $this->prepareInternalLinkValidatorBrokenLinksTestCase( + $repository + ); + + // update field containing erroneous link to trigger validation + /** @var \DOMDocument $document */ + $document = $brokenContent->getField('data', 'eng-GB')->value->xml; + $newParagraph = $document->createElement('para', 'Updated content'); + $document + ->getElementsByTagName('section')->item(0) + ->appendChild($newParagraph); + + $contentUpdateStruct = $contentService->newContentUpdateStruct(); + $contentUpdateStruct->setField('data', new RichTextValue($document), 'eng-GB'); + + $expectedValidationErrorMessage = sprintf( + 'Invalid link "ezlocation://%s": target location cannot be found', + $deletedLocation->id + ); + try { + $contentDraftB = $contentService->updateContent( + $contentService->createContentDraft($brokenContent->contentInfo)->versionInfo, + $contentUpdateStruct + ); + + $contentService->publishVersion($contentDraftB->versionInfo); + } catch (ContentFieldValidationException $e) { + $this->assertValidationErrorOccurs($e, $expectedValidationErrorMessage); + + return; + } + + self::fail("Expected ValidationError '{$expectedValidationErrorMessage}' didn't occur"); + } + + /** + * Get XML Document in DocBook format, containing link to the given Location. + * + * @param \eZ\Publish\API\Repository\Values\Content\Location $location + * + * @return \DOMDocument + */ + private function getDocumentWithLocationLink(Location $location) + { + $document = new DOMDocument(); + $document->loadXML(<< +
+ link1 +
+XML + ); + + return $document; + } + protected function checkSearchEngineSupport() { if (ltrim(get_class($this->getSetupFactory()), '\\') === 'eZ\\Publish\\API\\Repository\\Tests\\SetupFactory\\Legacy') { diff --git a/eZ/Publish/API/Repository/Tests/PHPUnitConstraint/ValidationErrorOccurs.php b/eZ/Publish/API/Repository/Tests/PHPUnitConstraint/ValidationErrorOccurs.php new file mode 100644 index 00000000000..bf1c406d943 --- /dev/null +++ b/eZ/Publish/API/Repository/Tests/PHPUnitConstraint/ValidationErrorOccurs.php @@ -0,0 +1,79 @@ +expectedValidationErrorMessage = $expectedValidationErrorMessage; + } + + /** + * @param \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException $other + * + * @return bool + */ + protected function matches($other) + { + foreach ($other->getFieldErrors() as $fieldId => $errors) { + foreach ($errors as $languageCode => $fieldErrors) { + foreach ($fieldErrors as $fieldError) { + /** @var \eZ\Publish\Core\FieldType\ValidationError $fieldError */ + if ($fieldError->getTranslatableMessage()->message === $this->expectedValidationErrorMessage) { + return true; + } + } + } + } + + return false; + } + + /** + * @param \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException $other + * + * @return string + */ + protected function failureDescription($other) + { + return sprintf( + '%s::getFieldErrors = %s %s', + get_class($other), + var_export($other->getFieldErrors(), true), + $this->toString() + ); + } + + /** + * Returns a string representation of the object. + * + * @return string + */ + public function toString() + { + return "contains a ValidationError with the message '{$this->expectedValidationErrorMessage}'"; + } +} diff --git a/eZ/Publish/Core/Base/Container/ApiLoader/RepositoryFactory.php b/eZ/Publish/Core/Base/Container/ApiLoader/RepositoryFactory.php index e99717f4efe..a4922e22e7b 100644 --- a/eZ/Publish/Core/Base/Container/ApiLoader/RepositoryFactory.php +++ b/eZ/Publish/Core/Base/Container/ApiLoader/RepositoryFactory.php @@ -8,6 +8,7 @@ */ namespace eZ\Publish\Core\Base\Container\ApiLoader; +use eZ\Publish\Core\Repository\Helper\RelationProcessor; use eZ\Publish\Core\Repository\Values\User\UserReference; use eZ\Publish\Core\Search\Common\BackgroundIndexer; use eZ\Publish\SPI\Persistence\Handler as PersistenceHandler; @@ -82,12 +83,14 @@ public function __construct( public function buildRepository( PersistenceHandler $persistenceHandler, SearchHandler $searchHandler, - BackgroundIndexer $backgroundIndexer + BackgroundIndexer $backgroundIndexer, + RelationProcessor $relationProcessor ) { $repository = new $this->repositoryClass( $persistenceHandler, $searchHandler, $backgroundIndexer, + $relationProcessor, array( 'fieldType' => $this->fieldTypeCollectionFactory->getFieldTypes(), 'nameableFieldTypes' => $this->fieldTypeNameableCollectionFactory->getNameableFieldTypes(), diff --git a/eZ/Publish/Core/Repository/ContentService.php b/eZ/Publish/Core/Repository/ContentService.php index 02eb17bc8d2..990e58c67c9 100644 --- a/eZ/Publish/Core/Repository/ContentService.php +++ b/eZ/Publish/Core/Repository/ContentService.php @@ -1140,7 +1140,16 @@ public function updateContent(APIVersionInfo $versionInfo, APIContentUpdateStruc } $mainLanguageCode = $content->contentInfo->mainLanguageCode; - $languageCodes = $this->getLanguageCodesForUpdate($contentUpdateStruct, $content); + if ($contentUpdateStruct->initialLanguageCode === null) { + $contentUpdateStruct->initialLanguageCode = $mainLanguageCode; + } + + $allLanguageCodes = $this->getLanguageCodesForUpdate($contentUpdateStruct, $content); + foreach ($allLanguageCodes as $languageCode) { + $this->persistenceHandler->contentLanguageHandler()->loadByLanguageCode($languageCode); + } + + $updatedLanguageCodes = $this->getUpdatedLanguageCodes($contentUpdateStruct); $contentType = $this->repository->getContentTypeService()->loadContentType( $content->contentInfo->contentTypeId ); @@ -1162,9 +1171,10 @@ public function updateContent(APIVersionInfo $versionInfo, APIContentUpdateStruc $fieldDefinition->fieldTypeIdentifier ); - foreach ($languageCodes as $languageCode) { + foreach ($allLanguageCodes as $languageCode) { $isCopied = $isEmpty = $isRetained = false; $isLanguageNew = !in_array($languageCode, $content->versionInfo->languageCodes); + $isLanguageUpdated = in_array($languageCode, $updatedLanguageCodes); $valueLanguageCode = $fieldDefinition->isTranslatable ? $languageCode : $mainLanguageCode; $isFieldUpdated = isset($fields[$fieldDefinition->identifier][$valueLanguageCode]); $isProcessed = isset($fieldValues[$fieldDefinition->identifier][$valueLanguageCode]); @@ -1185,7 +1195,7 @@ public function updateContent(APIVersionInfo $versionInfo, APIContentUpdateStruc if ($fieldType->isEmptyValue($fieldValue)) { $isEmpty = true; - if ($fieldDefinition->isRequired) { + if ($isLanguageUpdated && $fieldDefinition->isRequired) { $allFieldErrors[$fieldDefinition->id][$languageCode] = new ValidationError( "Value for required field definition '%identifier%' with language '%languageCode%' is empty", null, @@ -1193,7 +1203,7 @@ public function updateContent(APIVersionInfo $versionInfo, APIContentUpdateStruc 'empty' ); } - } else { + } elseif ($isLanguageUpdated) { $fieldErrors = $fieldType->validate( $fieldDefinition, $fieldValue @@ -1244,7 +1254,7 @@ public function updateContent(APIVersionInfo $versionInfo, APIContentUpdateStruc 'name' => $this->nameSchemaService->resolveNameSchema( $content, $fieldValues, - $languageCodes, + $allLanguageCodes, $contentType ), 'creatorId' => $contentUpdateStruct->creatorId ?: $this->repository->getCurrentUserReference()->getUserId(), @@ -1283,6 +1293,30 @@ public function updateContent(APIVersionInfo $versionInfo, APIContentUpdateStruc ); } + /** + * Returns only updated language codes. + * + * @param \eZ\Publish\API\Repository\Values\Content\ContentUpdateStruct $contentUpdateStruct + * + * @return array + */ + private function getUpdatedLanguageCodes(APIContentUpdateStruct $contentUpdateStruct) + { + $languageCodes = [ + $contentUpdateStruct->initialLanguageCode => true, + ]; + + foreach ($contentUpdateStruct->fields as $field) { + if ($field->languageCode === null || isset($languageCodes[$field->languageCode])) { + continue; + } + + $languageCodes[$field->languageCode] = true; + } + + return array_keys($languageCodes); + } + /** * Returns all language codes used in given $fields. * @@ -1295,26 +1329,12 @@ public function updateContent(APIVersionInfo $versionInfo, APIContentUpdateStruc */ protected function getLanguageCodesForUpdate(APIContentUpdateStruct $contentUpdateStruct, APIContent $content) { - if ($contentUpdateStruct->initialLanguageCode !== null) { - $this->persistenceHandler->contentLanguageHandler()->loadByLanguageCode( - $contentUpdateStruct->initialLanguageCode - ); - } else { - $contentUpdateStruct->initialLanguageCode = $content->contentInfo->mainLanguageCode; - } - $languageCodes = array_fill_keys($content->versionInfo->languageCodes, true); $languageCodes[$contentUpdateStruct->initialLanguageCode] = true; - foreach ($contentUpdateStruct->fields as $field) { - if ($field->languageCode === null || isset($languageCodes[$field->languageCode])) { - continue; - } - - $this->persistenceHandler->contentLanguageHandler()->loadByLanguageCode( - $field->languageCode - ); - $languageCodes[$field->languageCode] = true; + $updatedLanguageCodes = $this->getUpdatedLanguageCodes($contentUpdateStruct); + foreach ($updatedLanguageCodes as $languageCode) { + $languageCodes[$languageCode] = true; } return array_keys($languageCodes); diff --git a/eZ/Publish/Core/Repository/Helper/RelationProcessor.php b/eZ/Publish/Core/Repository/Helper/RelationProcessor.php index 5256a8eefdd..1392df597c0 100644 --- a/eZ/Publish/Core/Repository/Helper/RelationProcessor.php +++ b/eZ/Publish/Core/Repository/Helper/RelationProcessor.php @@ -8,12 +8,15 @@ */ namespace eZ\Publish\Core\Repository\Helper; +use eZ\Publish\API\Repository\Exceptions\NotFoundException; use eZ\Publish\SPI\Persistence\Handler; use eZ\Publish\API\Repository\Values\ContentType\ContentType; use eZ\Publish\Core\Repository\Values\Content\Relation; use eZ\Publish\Core\FieldType\Value as BaseValue; use eZ\Publish\SPI\FieldType\FieldType as SPIFieldType; use eZ\Publish\SPI\Persistence\Content\Relation\CreateStruct as SPIRelationCreateStruct; +use Psr\Log\LoggerAwareTrait; +use Psr\Log\NullLogger; /** * RelationProcessor is an internal service used for handling field relations upon Content creation or update. @@ -22,6 +25,8 @@ */ class RelationProcessor { + use LoggerAwareTrait; + /** * @var \eZ\Publish\SPI\Persistence\Handler */ @@ -35,6 +40,7 @@ class RelationProcessor public function __construct(Handler $handler) { $this->persistenceHandler = $handler; + $this->logger = new NullLogger(); } /** @@ -70,12 +76,19 @@ public function appendFieldRelations( if (isset($destinationIds['locationIds'])) { foreach ($destinationIds['locationIds'] as $locationId) { - if (!isset($locationIdToContentIdMapping[$locationId])) { - $location = $this->persistenceHandler->locationHandler()->load($locationId); - $locationIdToContentIdMapping[$locationId] = $location->contentId; - } + try { + if (!isset($locationIdToContentIdMapping[$locationId])) { + $location = $this->persistenceHandler->locationHandler()->load($locationId); + $locationIdToContentIdMapping[$locationId] = $location->contentId; + } - $relations[$relationType][$locationIdToContentIdMapping[$locationId]] = true; + $relations[$relationType][$locationIdToContentIdMapping[$locationId]] = true; + } catch (NotFoundException $e) { + $this->logger->error('Invalid relation: destination location not found', [ + 'fieldDefinitionId' => $fieldDefinitionId, + 'locationId' => $locationId, + ]); + } } } diff --git a/eZ/Publish/Core/Repository/Repository.php b/eZ/Publish/Core/Repository/Repository.php index 4d2725774ed..66c720fa6eb 100644 --- a/eZ/Publish/Core/Repository/Repository.php +++ b/eZ/Publish/Core/Repository/Repository.php @@ -12,6 +12,7 @@ use eZ\Publish\API\Repository\Values\User\UserReference as APIUserReference; use eZ\Publish\Core\Base\Exceptions\InvalidArgumentValue; use eZ\Publish\Core\Base\Exceptions\InvalidArgumentType; +use eZ\Publish\Core\Repository\Helper\RelationProcessor; use eZ\Publish\Core\Repository\Permission\CachedPermissionService; use eZ\Publish\Core\Repository\Permission\PermissionCriterionResolver; use eZ\Publish\Core\Repository\Values\User\UserReference; @@ -243,12 +244,14 @@ public function __construct( PersistenceHandler $persistenceHandler, SearchHandler $searchHandler, BackgroundIndexer $backgroundIndexer, + RelationProcessor $relationProcessor, array $serviceSettings = array(), APIUserReference $user = null ) { $this->persistenceHandler = $persistenceHandler; $this->searchHandler = $searchHandler; $this->backgroundIndexer = $backgroundIndexer; + $this->relationProcessor = $relationProcessor; $this->serviceSettings = $serviceSettings + array( 'content' => array(), 'contentType' => array(), @@ -843,12 +846,6 @@ public function getNameSchemaService() */ protected function getRelationProcessor() { - if ($this->relationProcessor !== null) { - return $this->relationProcessor; - } - - $this->relationProcessor = new Helper\RelationProcessor($this->persistenceHandler); - return $this->relationProcessor; } diff --git a/eZ/Publish/Core/Repository/Tests/Service/Mock/Base.php b/eZ/Publish/Core/Repository/Tests/Service/Mock/Base.php index 6585ec6eeaf..9458c8e7094 100644 --- a/eZ/Publish/Core/Repository/Tests/Service/Mock/Base.php +++ b/eZ/Publish/Core/Repository/Tests/Service/Mock/Base.php @@ -6,6 +6,7 @@ */ namespace eZ\Publish\Core\Repository\Tests\Service\Mock; +use eZ\Publish\Core\Repository\Helper\RelationProcessor; use eZ\Publish\Core\Search\Common\BackgroundIndexer\NullIndexer; use PHPUnit\Framework\TestCase; use eZ\Publish\Core\Repository\Repository; @@ -62,6 +63,7 @@ protected function getRepository(array $serviceSettings = array()) $this->getPersistenceMock(), $this->getSPIMockHandler('Search\\Handler'), new NullIndexer(), + $this->getRelationProcessorMock(), $serviceSettings, $this->getStubbedUser(14) ); @@ -188,6 +190,11 @@ protected function getPersistenceMock() return $this->persistenceMock; } + protected function getRelationProcessorMock() + { + return $this->createMock(RelationProcessor::class); + } + /** * Returns a SPI Handler mock. * diff --git a/eZ/Publish/Core/Repository/Tests/Service/Mock/ContentTest.php b/eZ/Publish/Core/Repository/Tests/Service/Mock/ContentTest.php index 50fad5408e3..4280a0ceb61 100644 --- a/eZ/Publish/Core/Repository/Tests/Service/Mock/ContentTest.php +++ b/eZ/Publish/Core/Repository/Tests/Service/Mock/ContentTest.php @@ -5013,7 +5013,6 @@ public function assertForTestUpdateContentThrowsContentFieldValidationException( /** @var \PHPUnit\Framework\MockObject\MockObject $languageHandlerMock */ $languageHandlerMock = $this->getPersistenceMock()->contentLanguageHandler(); $contentTypeServiceMock = $this->getContentTypeServiceMock(); - $fieldTypeServiceMock = $this->getFieldTypeServiceMock(); $fieldTypeMock = $this->createMock(SPIFieldType::class); $existingLanguageCodes = array_map( function (Field $field) { @@ -5094,44 +5093,32 @@ function () { $languageCodes ); $allFieldErrors = array(); - $validateCount = 0; $emptyValue = self::EMPTY_FIELD_VALUE; - foreach ($contentType->getFieldDefinitions() as $fieldDefinition) { - foreach ($fieldValues[$fieldDefinition->identifier] as $languageCode => $value) { - $fieldTypeMock->expects($this->at($validateCount++)) - ->method('acceptValue') - ->will( - $this->returnCallback( - function ($valueString) { - return new ValueStub($valueString); - } - ) - ); - - $fieldTypeMock->expects($this->at($validateCount++)) - ->method('isEmptyValue') - ->will( - $this->returnCallback( - function (ValueStub $value) use ($emptyValue) { - return $emptyValue === (string)$value; - } - ) - ); - if (self::EMPTY_FIELD_VALUE === (string)$value) { - continue; - } + $fieldTypeMock->expects($this->exactly(count($fieldValues) * count($languageCodes))) + ->method('acceptValue') + ->will( + $this->returnCallback( + function ($valueString) { + return new ValueStub($valueString); + } + ) + ); - $fieldTypeMock->expects($this->at($validateCount++)) - ->method('validate') - ->with( - $this->equalTo($fieldDefinition), - $this->equalTo($value) - )->will($this->returnArgument(1)); + $fieldTypeMock->expects($this->exactly(count($fieldValues) * count($languageCodes))) + ->method('isEmptyValue') + ->will( + $this->returnCallback( + function (ValueStub $value) use ($emptyValue) { + return $emptyValue === (string)$value; + } + ) + ); - $allFieldErrors[$fieldDefinition->id][$languageCode] = $value; - } - } + $fieldTypeMock + ->expects($this->any()) + ->method('validate') + ->willReturnArgument(1); $this->getFieldTypeRegistryMock()->expects($this->any()) ->method('getFieldType') @@ -5149,7 +5136,124 @@ function (ValueStub $value) use ($emptyValue) { public function providerForTestUpdateContentThrowsContentFieldValidationException() { - return $this->providerForTestUpdateContentNonRedundantFieldSetComplex(); + $allFieldErrors = [ + [ + 'fieldDefinitionId1' => [ + 'eng-GB' => 'newValue1-eng-GB', + 'eng-US' => 'newValue1-eng-GB', + ], + 'fieldDefinitionId2' => [ + 'eng-GB' => 'initialValue2', + ], + 'fieldDefinitionId3' => [ + 'eng-GB' => 'initialValue3', + 'eng-US' => 'initialValue3', + ], + 'fieldDefinitionId4' => [ + 'eng-GB' => 'initialValue4', + 'eng-US' => 'newValue4', + ], + ], + [ + 'fieldDefinitionId1' => [ + 'eng-GB' => 'newValue1-eng-GB', + 'eng-US' => 'newValue1-eng-GB', + ], + 'fieldDefinitionId2' => [ + 'eng-GB' => 'initialValue2', + ], + 'fieldDefinitionId3' => [ + 'eng-GB' => 'initialValue3', + 'eng-US' => 'initialValue3', + ], + 'fieldDefinitionId4' => [ + 'eng-GB' => 'initialValue4', + 'eng-US' => 'newValue4', + ], + ], + [ + 'fieldDefinitionId1' => [ + 'eng-GB' => 'newValue1-eng-GB', + 'eng-US' => 'newValue1-eng-GB', + ], + 'fieldDefinitionId2' => [ + 'eng-GB' => 'initialValue2', + 'eng-US' => 'newValue2', + ], + 'fieldDefinitionId3' => [ + 'eng-GB' => 'initialValue3', + 'eng-US' => 'initialValue3', + ], + 'fieldDefinitionId4' => [ + 'eng-GB' => 'initialValue4', + 'eng-US' => 'defaultValue4', + ], + ], + [ + 'fieldDefinitionId1' => [ + 'eng-GB' => 'newValue1-eng-GB', + 'eng-US' => 'newValue1-eng-GB', + ], + 'fieldDefinitionId2' => [ + 'eng-GB' => 'initialValue2', + 'eng-US' => 'newValue2', + ], + 'fieldDefinitionId3' => [ + 'eng-GB' => 'initialValue3', + 'eng-US' => 'initialValue3', + ], + 'fieldDefinitionId4' => [ + 'eng-GB' => 'initialValue4', + 'eng-US' => 'defaultValue4', + ], + ], + [ + 'fieldDefinitionId1' => [ + 'eng-GB' => 'newValue1-eng-GB', + 'ger-DE' => 'newValue1-eng-GB', + 'eng-US' => 'newValue1-eng-GB', + ], + 'fieldDefinitionId2' => [ + 'eng-GB' => 'initialValue2', + 'eng-US' => 'newValue2', + ], + 'fieldDefinitionId3' => [ + 'eng-GB' => 'initialValue3', + 'ger-DE' => 'initialValue3', + 'eng-US' => 'initialValue3', + ], + 'fieldDefinitionId4' => [ + 'eng-GB' => 'initialValue4', + 'eng-US' => 'defaultValue4', + 'ger-DE' => 'defaultValue4', + ], + ], + [ + 'fieldDefinitionId1' => [ + 'eng-US' => 'newValue1-eng-GB', + 'ger-DE' => 'newValue1-eng-GB', + ], + 'fieldDefinitionId2' => [ + 'eng-US' => 'newValue2', + ], + 'fieldDefinitionId3' => [ + 'ger-DE' => 'initialValue3', + 'eng-US' => 'initialValue3', + ], + 'fieldDefinitionId4' => [ + 'ger-DE' => 'defaultValue4', + 'eng-US' => 'defaultValue4', + ], + ], + ]; + + $data = $this->providerForTestUpdateContentNonRedundantFieldSetComplex(); + $count = count($data); + for ($i = 0; $i < $count; ++$i) { + $data[$i][] = $allFieldErrors[$i]; + } + + return $data; } /** @@ -5162,10 +5266,10 @@ public function providerForTestUpdateContentThrowsContentFieldValidationExceptio * @expectedException \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException * @expectedExceptionMessage Content fields did not validate */ - public function testUpdateContentThrowsContentFieldValidationException($initialLanguageCode, $structFields) + public function testUpdateContentThrowsContentFieldValidationException($initialLanguageCode, $structFields, $spiField, $allFieldErrors) { list($existingFields, $fieldDefinitions) = $this->fixturesForTestUpdateContentNonRedundantFieldSetComplex(); - list($versionInfo, $contentUpdateStruct, $allFieldErrors) = + list($versionInfo, $contentUpdateStruct) = $this->assertForTestUpdateContentThrowsContentFieldValidationException( $initialLanguageCode, $structFields, diff --git a/eZ/Publish/Core/Repository/Tests/Service/Mock/RelationProcessorTest.php b/eZ/Publish/Core/Repository/Tests/Service/Mock/RelationProcessorTest.php index cb5d7ece096..c90bd7d69c1 100644 --- a/eZ/Publish/Core/Repository/Tests/Service/Mock/RelationProcessorTest.php +++ b/eZ/Publish/Core/Repository/Tests/Service/Mock/RelationProcessorTest.php @@ -8,6 +8,7 @@ */ namespace eZ\Publish\Core\Repository\Tests\Service\Mock; +use eZ\Publish\API\Repository\Exceptions\NotFoundException; use eZ\Publish\API\Repository\Values\ContentType\ContentType; use eZ\Publish\Core\Repository\FieldTypeService; use eZ\Publish\Core\Repository\Helper\RelationProcessor; @@ -19,6 +20,7 @@ use eZ\Publish\SPI\FieldType\FieldType; use eZ\Publish\SPI\Persistence\Content\Relation\CreateStruct; use eZ\Publish\SPI\Persistence\Content\Location; +use Psr\Log\LoggerInterface; /** * Mock Test case for RelationProcessor service. @@ -263,6 +265,58 @@ public function testAppendFieldRelationsLocationMappingWorks() ); } + public function testAppendFieldRelationsLogsMissingLocations() + { + $fieldValueMock = $this->getMockForAbstractClass(Value::class); + $fieldTypeMock = $this->createMock(FieldType::class); + + $locationId = 123465; + $fieldDefinitionId = 42; + + $fieldTypeMock + ->expects($this->once()) + ->method('getRelations') + ->with($this->equalTo($fieldValueMock)) + ->will( + $this->returnValue( + [ + Relation::LINK => [ + 'locationIds' => [$locationId], + ], + ] + ) + ); + + $locationHandler = $this->getPersistenceMock()->locationHandler(); + $locationHandler + ->expects($this->any()) + ->method('load') + ->with($locationId) + ->willThrowException($this->createMock(NotFoundException::class)); + + $logger = $this->createMock(LoggerInterface::class); + $logger + ->expects($this->once()) + ->method('error') + ->with('Invalid relation: destination location not found', [ + 'fieldDefinitionId' => $fieldDefinitionId, + 'locationId' => $locationId, + ]); + + $relations = []; + $locationIdToContentIdMapping = []; + + $relationProcessor = $this->getPartlyMockedRelationProcessor(); + $relationProcessor->setLogger($logger); + $relationProcessor->appendFieldRelations( + $relations, + $locationIdToContentIdMapping, + $fieldTypeMock, + $fieldValueMock, + $fieldDefinitionId + ); + } + /** * Test for the processFieldRelations() method. * diff --git a/eZ/Publish/Core/settings/repository/inner.yml b/eZ/Publish/Core/settings/repository/inner.yml index a681ac781e9..586d45a9468 100644 --- a/eZ/Publish/Core/settings/repository/inner.yml +++ b/eZ/Publish/Core/settings/repository/inner.yml @@ -40,6 +40,7 @@ services: - "@ezpublish.api.persistence_handler" - "@ezpublish.spi.search" - '@ezpublish.search.background_indexer' + - '@ezpublish.repository.relation_processor' lazy: true ezpublish.api.service.inner_content: @@ -127,3 +128,10 @@ services: ezpublish.search.background_indexer: class: eZ\Publish\Core\Search\Common\BackgroundIndexer\NullIndexer + + ezpublish.repository.relation_processor: + class: eZ\Publish\Core\Repository\Helper\RelationProcessor + arguments: + - '@ezpublish.api.persistence_handler' + calls: + - ['setLogger', ['@?logger']]