Skip to content

Commit

Permalink
IBX-5119: Added logging for invalid Content's Locations setup
Browse files Browse the repository at this point in the history
For more details see https://issues.ibexa.co/browse/IBX-5119 and #364

* Added logging for invalid Content's Locations setup
  • Loading branch information
barw4 authored Mar 28, 2023
1 parent 306c4d3 commit 1138d50
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 2 deletions.
7 changes: 6 additions & 1 deletion eZ/Publish/API/Repository/Values/Content/ContentInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* @property-read bool $alwaysAvailable Indicates if the Content object is shown in the mainlanguage if its not present in an other requested language
* @property-read string $remoteId a global unique id of the Content object
* @property-read string $mainLanguageCode The main language code of the Content object. If the available flag is set to true the Content is shown in this language if the requested language does not exist.
* @property-read int|null $mainLocationId Identifier of the main location.
* @property-read int|null $mainLocationId @deprecated Use {@see ContentInfo::getMainLocationId} instead
* @property-read int $status status of the Content object
* @property-read bool $isHidden status of the Content object
*/
Expand Down Expand Up @@ -215,6 +215,11 @@ public function getOwner(): User
return $this->owner;
}

public function getMainLocationId(): ?int
{
return $this->mainLocationId;
}

public function getId(): int
{
return $this->id;
Expand Down
7 changes: 7 additions & 0 deletions eZ/Publish/Core/Persistence/Cache/LocationHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -502,4 +502,11 @@ private function getCacheTranslationKey(array $translations = null, bool $useAlw

return implode('|', $translations) . '|' . (int)$useAlwaysAvailable;
}

public function countLocationsByContent(int $contentId): int
{
$this->logger->logCall(__METHOD__, ['contentId' => $contentId]);

return $this->persistenceHandler->locationHandler()->countLocationsByContent($contentId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public function providerForUnCachedMethods(): array
['removeSubtree', [12], [['location_path', [12], false]], null, ['lp-12']],
['setSectionForSubtree', [12, 2], [['location_path', [12], false]], null, ['lp-12']],
['changeMainLocation', [4, 12], [['content', [4], false]], null, ['c-4']],
['countLocationsByContent', [4]],
];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -608,4 +608,12 @@ public function loadAllLocations($offset, $limit)

return $this->locationMapper->createLocationsFromRows($rows);
}

/**
* {@inheritdoc}
*/
public function countLocationsByContent(int $contentId): int
{
return $this->locationGateway->countLocationsByContentId($contentId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,23 @@ public function testCopySubtree()
);
}

/**
* @covers \eZ\Publish\Core\Persistence\Legacy\Content\Location::countLocationsByContent
*/
public function testCountLocationsByContent(): void
{
$handler = $this->getLocationHandler();

$contentId = 41;

$this->locationGateway
->expects(self::once())
->method('countLocationsByContentId')
->with($contentId);

$handler->countLocationsByContent($contentId);
}

/**
* Returns the handler to test with $methods mocked.
*
Expand Down
23 changes: 22 additions & 1 deletion eZ/Publish/Core/Repository/Mapper/ContentDomainMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,20 @@
use eZ\Publish\SPI\Persistence\Content\Type\Handler as TypeHandler;
use eZ\Publish\SPI\Persistence\Content\VersionInfo as SPIVersionInfo;
use eZ\Publish\SPI\Repository\Strategy\ContentThumbnail\ThumbnailStrategy;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;

/**
* ContentDomainMapper is an internal service.
*
* @internal Meant for internal use by Repository.
*/
class ContentDomainMapper extends ProxyAwareDomainMapper
class ContentDomainMapper extends ProxyAwareDomainMapper implements LoggerAwareInterface
{
use LoggerAwareTrait;

public const MAX_LOCATION_PRIORITY = 2147483647;
public const MIN_LOCATION_PRIORITY = -2147483648;

Expand Down Expand Up @@ -76,6 +82,7 @@ public function __construct(
LanguageHandler $contentLanguageHandler,
FieldTypeRegistry $fieldTypeRegistry,
ThumbnailStrategy $thumbnailStrategy,
?LoggerInterface $logger = null,
?ProxyDomainMapperInterface $proxyFactory = null
) {
$this->contentHandler = $contentHandler;
Expand All @@ -85,6 +92,7 @@ public function __construct(
$this->contentLanguageHandler = $contentLanguageHandler;
$this->fieldTypeRegistry = $fieldTypeRegistry;
$this->thumbnailStrategy = $thumbnailStrategy;
$this->logger = $logger ?? new NullLogger();
parent::__construct($proxyFactory);
}

Expand Down Expand Up @@ -119,6 +127,19 @@ public function buildContentDomainObject(

$versionInfo = $this->buildVersionInfoDomainObject($spiContent->versionInfo, $prioritizedLanguages);

$contentInfo = $versionInfo->getContentInfo();
$mainLocation = $contentInfo->getMainLocation();

// For performance reasons 'countLocationsByContent' is moved to if
if ($mainLocation === null && $this->locationHandler->countLocationsByContent($contentInfo->getId()) > 0) {
$this->logger->error(
sprintf(
'Main location for content of ID = %d doesn\'t exist yet this content has locations assigned.',
$contentInfo->getId()
)
);
}

return new Content(
[
'thumbnail' => $this->thumbnailStrategy->getThumbnail($contentType, $internalFields, $versionInfo),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use eZ\Publish\SPI\Persistence\Content\ContentInfo as SPIContentInfo;
use eZ\Publish\SPI\Persistence\Content\Location;
use eZ\Publish\SPI\Persistence\Content\VersionInfo as SPIVersionInfo;
use Psr\Log\LoggerInterface;

/**
* Mock test case for internal ContentDomainMapper.
Expand Down Expand Up @@ -276,6 +277,7 @@ protected function getContentDomainMapper(): ContentDomainMapper
$this->getLanguageHandlerMock(),
$this->getFieldTypeRegistryMock(),
$this->getThumbnailStrategy(),
$this->getLoggerMock(),
$this->getProxyFactoryMock()
);
}
Expand Down Expand Up @@ -308,4 +310,9 @@ protected function getProxyFactoryMock(): ProxyDomainMapperInterface
{
return $this->createMock(ProxyDomainMapperInterface::class);
}

protected function getLoggerMock(): LoggerInterface
{
return $this->createMock(LoggerInterface::class);
}
}
4 changes: 4 additions & 0 deletions eZ/Publish/Core/settings/repository/inner.yml
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ services:
$contentLanguageHandler: '@ezpublish.spi.persistence.language_handler'
$fieldTypeRegistry: '@eZ\Publish\Core\FieldType\FieldTypeRegistry'
$thumbnailStrategy: '@eZ\Publish\Core\Repository\Strategy\ContentThumbnail\ThumbnailChainStrategy'
calls:
- [setLogger, ['@?logger']]
tags:
- { name: 'monolog.logger', channel: 'ibexa.core' }

eZ\Publish\Core\Repository\Mapper\RoleDomainMapper:
arguments:
Expand Down
5 changes: 5 additions & 0 deletions eZ/Publish/SPI/Persistence/Content/Location/Handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -252,4 +252,9 @@ public function countAllLocations();
* @return \eZ\Publish\SPI\Persistence\Content\Location[]
*/
public function loadAllLocations($offset, $limit);

/**
* Counts locations for a given content represented by its id.
*/
public function countLocationsByContent(int $contentId): int;
}

0 comments on commit 1138d50

Please sign in to comment.