neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
260 stars 220 forks source link

FEATURE: Add `ContentRepository::subgraphForNode` #5047

Closed mhsdesign closed 1 month ago

mhsdesign commented 1 month ago

Seeing code like this

$subgraph = $this->contentRepositoryRegistry->subgraphForNode($node);
$nodeTypeManager = $this->contentRepositoryRegistry->get($node->contentRepositoryId)->getNodeTypeManager();

this seems just double because the contentRepository is fetched twice and not passed

$subgraph = $this->contentRepositoryRegistry->subgraphForNode($node);
$contentRepository = $this->contentRepositoryRegistry->get($node->contentRepositoryId);
$contentRepository-> ...

makes me rethink if were not going to far with contentRepositoryRegistry->subgraphForNode, id like to fetch the cr more often in code directly and then get the subgraph:

$contentRepository = $this->contentRepositoryRegistry->get($node->contentRepositoryId);
$subgraph = $contentRepository->subgraphForNode($node);
$nodeTypeManager = $contentRepository->getNodeTypeManager();

Upgrade instructions

Review instructions

Checklist

nezaniel commented 1 month ago

I guess the reason for this not existing is that the node might not even belong to the given content repository

mhsdesign commented 1 month ago

touché we could add a content repository id check

mhsdesign commented 1 month ago
public function subgraphForNode(Node $node): ContentSubgraphInterface
{
    return $this->subgraphForIdentity(ContentSubgraphIdentity::fromNode($node));
}

public function subgraphForIdentity(ContentSubgraphIdentity $identity): ContentSubgraphInterface
{
    if (!$this->id->equals($identity->contentRepositoryId)) {
        throw new ForeignContentRepositoryUsed(
            sprintf('ContentRepository "%s" cannot get subgraph for Node of ContentRepository "%s".', $this->id->value, $node->contentRepositoryId->value),
            1715531407
        );
    }
    return $this->getContentGraph($identity->workspaceName)->getSubgraph(
        $identity->dimensionSpacePoint,
        $identity->visibilityConstraints
    );
}

public function subgraphFor(WorkspaceName $workspaceName, DimensionSpacePoint $dimensionSpacePoint, VisibilityConstraints $visibilityConstraints): ContentSubgraphInterface
{
    return $this->getContentGraph($workspaceName)->getSubgraph(
        $dimensionSpacePoint,
        $visibilityConstraints
    );
}

as of now we concluded that shortcuts like these are not worth it yet

mhsdesign commented 1 month ago

Discussion moved to https://github.com/neos/neos-development-collection/issues/5065