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

TASK: Provide alternative for NodeDiscriminator::fromNode that doesnt use Nodes content stream #5144

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
use Neos\ContentRepository\Core\ContentGraphFinder;
use Neos\ContentRepository\Core\ContentRepository;
use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint;
use Neos\ContentRepository\Core\Projection\ContentGraph\ContentGraphInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\ContentSubgraphInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
use Neos\ContentRepository\Core\Projection\ContentGraph\NodeAggregate;
use Neos\ContentRepository\Core\Projection\ContentGraph\NodePath;
use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints;
Expand All @@ -29,6 +29,8 @@
use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName;
use Neos\ContentRepository\TestSuite\Behavior\Features\Bootstrap\Helpers\FakeClock;
use Neos\ContentRepository\TestSuite\Behavior\Features\Bootstrap\Helpers\FakeUserIdProvider;
use Neos\ContentRepository\TestSuite\Behavior\Features\Bootstrap\Helpers\ContentStreamAwareNode;
use PHPUnit\Framework\Assert;

/**
* The node creation trait for behavioral tests
Expand All @@ -49,7 +51,11 @@ trait CRTestSuiteRuntimeVariables

protected ?\Exception $lastCommandException = null;

protected ?Node $currentNode = null;
/**
* The Node's content stream id doesn't necessarily have to match with {@see $currentContentStreamId} if the node was initialized via the content graph from another workspace
* thus we have to track it as well.
*/
protected ?ContentStreamAwareNode $currentNode = null;

protected ?NodeAggregate $currentNodeAggregate = null;

Expand Down Expand Up @@ -147,16 +153,22 @@ public function visibilityConstraintsAreSetTo(string $restrictionType): void
};
}

public function getCurrentSubgraph(): ContentSubgraphInterface
public function getCurrentContentGraph(): ContentGraphInterface
{
// todo cache content graph per test run??? Otherwise it will be flushed too often?
$contentGraphFinder = $this->currentContentRepository->projectionState(ContentGraphFinder::class);
$contentGraphFinder->forgetInstances();
if (isset($this->currentContentStreamId)) {
if ($this->currentContentStreamId !== null) {
// This must still be supported for low level tests, e.g. for content stream forking
return $contentGraphFinder->getByWorkspaceNameAndContentStreamId($this->currentWorkspaceName, $this->currentContentStreamId)->getSubgraph($this->currentDimensionSpacePoint, $this->currentVisibilityConstraints);
return $contentGraphFinder->getByWorkspaceNameAndContentStreamId($this->currentWorkspaceName, $this->currentContentStreamId);
}

return $contentGraphFinder->getByWorkspaceName($this->currentWorkspaceName)->getSubgraph(
return $contentGraphFinder->getByWorkspaceName($this->currentWorkspaceName);
}

public function getCurrentSubgraph(): ContentSubgraphInterface
{
return $this->getCurrentContentGraph()->getSubgraph(
$this->currentDimensionSpacePoint,
$this->currentVisibilityConstraints
);
Expand All @@ -175,7 +187,12 @@ public function iRememberNodeAggregateIdOfNodesChildAs(string $parentNodeAggrega

protected function getCurrentNodeAggregateId(): NodeAggregateId
{
assert($this->currentNode instanceof Node);
$this->expectCurrentNode();
return $this->currentNode->aggregateId;
}

protected function expectCurrentNode(): void
{
Assert::assertNotNull($this->currentNode, 'No current node present');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function theCommandCopyNodesRecursivelyIsExecutedCopyingTheCurrentNodeAgg
$command = CopyNodesRecursively::createFromSubgraphAndStartNode(
$subgraph,
$workspaceName,
$this->currentNode,
$this->currentNode->nodeInstance,
$targetDimensionSpacePoint,
NodeAggregateId::fromString($commandArguments['targetParentNodeAggregateId']),
$targetSucceedingSiblingNodeAggregateId,
Expand Down
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We agreed not to have this kind of DTOs today (see #5034 (comment))

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

declare(strict_types=1);

namespace Neos\ContentRepository\TestSuite\Behavior\Features\Bootstrap\Helpers;

use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;

final readonly class ContentStreamAwareNode
{
private function __construct(
public ContentStreamId $contentStreamId,
public Node $nodeInstance,
/** Alias for $currentNode->instance->aggregateId */
public NodeAggregateId $aggregateId,
Comment on lines +16 to +17
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that way we dont need to rewrite too much code ...

) {
}

public static function create(ContentStreamId $contentStreamId, Node $node): self
{
return new self($contentStreamId, $node, $node->aggregateId);
}

public function builder(): ContentStreamAwareNodeBuilder
{
return ContentStreamAwareNodeBuilder::create($this->contentStreamId);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

declare(strict_types=1);

namespace Neos\ContentRepository\TestSuite\Behavior\Features\Bootstrap\Helpers;

use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;

final readonly class ContentStreamAwareNodeBuilder
{
private function __construct(
private ContentStreamId $contentStreamId
) {
}

public static function create(ContentStreamId $contentStreamId): self
{
return new self($contentStreamId);
}

public function buildNode(Node $node): ContentStreamAwareNode
{
return ContentStreamAwareNode::create($this->contentStreamId, $node);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
namespace Neos\ContentRepository\TestSuite\Behavior\Features\Bootstrap\Helpers;

use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePoint;
use Neos\ContentRepository\Core\Projection\ContentGraph\ContentGraphInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
Expand Down Expand Up @@ -47,12 +48,12 @@ public static function fromShorthand(string $shorthand): self
);
}

public static function fromNode(Node $node): self
public static function fromNode(ContentStreamAwareNode $contentStreamAwareNode): self
{
return new self(
$node->subgraphIdentity->contentStreamId,
$node->aggregateId,
$node->originDimensionSpacePoint
$contentStreamAwareNode->contentStreamId,
$contentStreamAwareNode->nodeInstance->aggregateId,
$contentStreamAwareNode->nodeInstance->originDimensionSpacePoint
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,6 @@ public static function fromArray(array $array): self
));
}

public static function fromNodes(Nodes $nodes): self
{
return new self(...array_map(
fn (Node $node): NodeDiscriminator => NodeDiscriminator::fromNode($node),
iterator_to_array($nodes)
));
}

public function equal(self $other): bool
{
return $this->discriminators == $other->discriminators;
Expand Down
Loading
Loading