sulu / SuluContentBundle

Add content behaviours to a Doctrine Entity
14 stars 19 forks source link

Throw Exception if there is no DimensionContent for the given dimensionAttributes #145

Open niklasnatter opened 4 years ago

niklasnatter commented 4 years ago

The ContentResolver service allows to resolve the effective content for a given ContentRichEntity and given dimensionAttributes. In this context, effective content means that the object returned by the ContentResolver is a merged representation/projection of all relevant DimensionContents (eg. localized DimensionContent and unlocalized DimensionContent) for the given dimensionAttributes.

The ContentResolver throws an ContentNotFoundException, when there are no relevant DimensionContents found. At the moment, no exception is thrown if there is at least one relevant DimensionContent.

This means that the ContentResolver does not throw an exception when a developer queries for the published german content with the dimensionAttributes ['stage' => 'live', 'locale' => 'de'] if there is at least one published language (even if the german content is not published). Because of this, we need to explicitly check the resolved content with the following code in various places of the bundle and also in our projects:

/** @var ThingProjection $thingProjection */
$thingProjection = $this->contentManager->resolve($thing, [
    'locale' => $locale,
    'stage' => DimensionInterface::STAGE_LIVE,
]);

$dimension = $thingProjection->getDimension();

if (DimensionInterface::STAGE_LIVE !== $dimension->getStage() || $locale !== $dimension->getLocale()) {
    return null;
}

return $thingProjection;

In my opinion, this is not not a great DX. Firstly, as a developer, I do not expect this behaviour and therefore it is really easy to forget about the check. Secondly, the consequences of forgetting about the check are quite big.

As there are still places where the current behaviour is desired, a way for solving this would be adding a throwIfDimensionAttributesDoNotMatch flag (hopefully we will find a better name) to the resolve() method of the ContentResolver. This would lead to the following method signature:

public function resolve(
    ContentRichEntityInterface $contentRichEntity, 
    array $dimensionAttributes, 
    bool $throwIfDimensionAttributesDoNotMatch = true
): ContentProjectionInterface;
alexander-schranz commented 4 years ago

For me this is a little bit related to the options parameters #136. The document manager provides also options e.g.: ['load_ghost_content' => false]. I think something similar does make sense.

public function resolve(
    ContentRichEntityInterface $contentRichEntity, 
    array $dimensionAttributes, 
    array $options = []
): ContentProjectionInterface;