Skip to content

Commit

Permalink
Merge pull request #5015 from mhsdesign/task/4208-remove-magic-unders…
Browse files Browse the repository at this point in the history
…core-access-for-internal-node-properties

!!! TASK: Remove leftovers from magic underscore access for internal node properties
  • Loading branch information
bwaidelich authored Oct 11, 2024
2 parents 276f1cb + 6d5c1bc commit bc4de7f
Show file tree
Hide file tree
Showing 9 changed files with 246 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
use Neos\Eel\FlowQuery\FlowQuery;
use Neos\Flow\Annotations as Flow;
use Neos\Neos\Utility\NodeTypeWithFallbackProvider;
use Neos\Utility\ObjectAccess;

/**
* This filter implementation contains specific behavior for use on ContentRepository
Expand Down Expand Up @@ -122,14 +121,7 @@ protected function matchesIdentifierFilter($element, $identifier)
*/
protected function getPropertyPath($element, $propertyPath)
{
if ($propertyPath === '_identifier') {
// TODO: deprecated (Neos <9 case)
return $element->aggregateId->value;
} elseif ($propertyPath[0] === '_') {
return ObjectAccess::getPropertyPath($element, substr($propertyPath, 1));
} else {
return $element->getProperty($propertyPath);
}
return $element->getProperty($propertyPath);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,9 @@
use Neos\Eel\FlowQuery\Operations\AbstractOperation;
use Neos\Flow\Annotations as Flow;
use Neos\Neos\Utility\NodeTypeWithFallbackProvider;
use Neos\Utility\ObjectAccess;

/**
* Used to access properties of a ContentRepository Node. If the property mame is
* prefixed with _, internal node properties like start time, end time,
* hidden are accessed.
* Used to access properties of a ContentRepository Node.
*/
class PropertyOperation extends AbstractOperation
{
Expand Down Expand Up @@ -84,7 +81,7 @@ public function canEvaluate($context): bool
public function evaluate(FlowQuery $flowQuery, array $arguments): mixed
{
if (empty($arguments[0])) {
throw new FlowQueryException('property() does not support returning all attributes yet', 1332492263);
throw new FlowQueryException(static::$shortName . '() does not allow returning all properties.', 1332492263);
}
/** @var array<int,mixed> $context */
$context = $flowQuery->getContext();
Expand All @@ -96,22 +93,8 @@ public function evaluate(FlowQuery $flowQuery, array $arguments): mixed

/* @var $element Node */
$element = $context[0];
if ($propertyName === '_path') {
$subgraph = $this->contentRepositoryRegistry->subgraphForNode($element);
$ancestors = $subgraph->findAncestorNodes(
$element->aggregateId,
FindAncestorNodesFilter::create()
)->reverse();

return AbsoluteNodePath::fromLeafNodeAndAncestors($element, $ancestors)->serializeToString();
}
if ($propertyName === '_identifier') {
// TODO: deprecated (Neos <9 case)
return $element->aggregateId->value;
}

if ($propertyName[0] === '_') {
return ObjectAccess::getPropertyPath($element, substr($propertyName, 1));
if ($element->hasProperty($propertyName)) {
return $element->getProperty($propertyName);
}

$contentRepository = $this->contentRepositoryRegistry->get($element->contentRepositoryId);
Expand All @@ -136,6 +119,6 @@ public function evaluate(FlowQuery $flowQuery, array $arguments): mixed
return $references;
}

return $element->getProperty($propertyName);
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<?php

/*
* This file is part of the Neos.Neos package.
*
* (c) Contributors of the Neos Project - www.neos.io
*
* This package is Open Source Software. For the full copyright and license
* information, please view the LICENSE file which was distributed with this
* source code.
*/

declare(strict_types=1);

namespace Neos\ContentRepository\NodeAccess\FlowQueryOperations;

use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
use Neos\ContentRepository\Core\Projection\ContentGraph\Timestamps;
use Neos\Eel\FlowQuery\FlowQuery;
use Neos\Eel\FlowQuery\FlowQueryException;
use Neos\Eel\FlowQuery\Operations\AbstractOperation;

/**
* "sortByTimestamp" operation working on ContentRepository nodes.
* Sorts nodes by specified timestamp.
*/
class SortByTimestampOperation extends AbstractOperation
{
/**
* {@inheritdoc}
*
* @var string
*/
protected static $shortName = 'sortByTimestamp';

/**
* {@inheritdoc}
*
* We can only handle ContentRepository Nodes.
*
* @param mixed $context
* @return boolean
*/
public function canEvaluate($context)
{
return count($context) === 0 || (is_array($context) === true && (current($context) instanceof Node));
}

/**
* First argument is the timestamp to sort by like created, lastModified, originalCreated and originalLastModified
* Second argument is the sort direction (ASC or DESC).
*
* sortByTimestamp("created", "ASC")
* sortByTimestamp("lastModified", "DESC")
*
* @see Timestamps for further documentation
*
* @param FlowQuery $flowQuery the FlowQuery object
* @param array<int,mixed> $arguments the arguments for this operation.
* @return void
* @throws FlowQueryException
*/
public function evaluate(FlowQuery $flowQuery, array $arguments)
{
/** @var array|Node[] $nodes */
$nodes = $flowQuery->getContext();

$sortedNodes = [];
$sortSequence = [];
$nodesByIdentifier = [];

// Determine the property value to sort by
foreach ($nodes as $node) {
$timeStamp = match($arguments[0] ?? null) {
'created' => $node->timestamps->created->getTimestamp(),
'lastModified' => $node->timestamps->lastModified?->getTimestamp(),
'originalCreated' => $node->timestamps->originalCreated->getTimestamp(),
'originalLastModified' => $node->timestamps->originalLastModified?->getTimestamp(),
default => throw new FlowQueryException('Please provide a timestamp (created, lastModified, originalLastModified) to sort by.', 1727367726)
};

$sortSequence[$node->aggregateId->value] = $timeStamp;
$nodesByIdentifier[$node->aggregateId->value] = $node;
}

$sortOrder = is_string($arguments[1] ?? null) ? strtoupper($arguments[1]) : null;
if ($sortOrder === 'DESC') {
arsort($sortSequence);
} elseif ($sortOrder === 'ASC') {
asort($sortSequence);
} else {
throw new FlowQueryException('Please provide a valid sort direction (ASC or DESC)', 1727367837);
}

// Build the sorted context that is returned
foreach ($sortSequence as $nodeIdentifier => $value) {
$sortedNodes[] = $nodesByIdentifier[$nodeIdentifier];
}

$flowQuery->setContext($sortedNodes);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

declare(strict_types=1);

namespace Neos\Neos\Eel\FlowQueryOperations;
namespace Neos\ContentRepository\NodeAccess\FlowQueryOperations;

use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
use Neos\Eel\FlowQuery\FlowQuery;
Expand Down Expand Up @@ -46,9 +46,7 @@ public function canEvaluate($context)
}

/**
* {@inheritdoc}
*
* First argument is the node property to sort by. Works with internal arguments (_xyz) as well.
* First argument is the node property to sort by.
* Second argument is the sort direction (ASC or DESC).
* Third optional argument are the sort options (see https://www.php.net/manual/en/function.sort):
* - 'SORT_REGULAR'
Expand Down Expand Up @@ -113,15 +111,7 @@ public function evaluate(FlowQuery $flowQuery, array $arguments)

// Determine the property value to sort by
foreach ($nodes as $node) {
if ($sortProperty[0] === '_') {
$propertyValue = \Neos\Utility\ObjectAccess::getPropertyPath($node, substr($sortProperty, 1));
} else {
$propertyValue = $node->getProperty($sortProperty);
}

if ($propertyValue instanceof \DateTime) {
$propertyValue = $propertyValue->getTimestamp();
}
$propertyValue = $node->getProperty($sortProperty);

$sortSequence[$node->aggregateId->value] = $propertyValue;
$nodesByIdentifier[$node->aggregateId->value] = $node;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

declare(strict_types=1);

namespace Neos\ContentRepository\NodeAccess\Tests\Unit\FlowQueryOperations;

use Neos\ContentRepository\NodeAccess\FlowQueryOperations\SortByTimestampOperation;
use Neos\Eel\FlowQuery\FlowQueryException;
use PHPUnit\Framework\TestCase;

/**
* SortOperation test
*/
class SortByTimestampOperationTest extends TestCase
{
/**
* @test+
*/
public function callWithoutArgumentsCausesException()
{
$this->expectException(FlowQueryException::class);
$flowQuery = new \Neos\Eel\FlowQuery\FlowQuery([]);
$operation = new SortByTimestampOperation();
$operation->evaluate($flowQuery, []);
}

/**
* @test+
*/
public function callWithoutWrongTimeStampArgumentsCausesException()
{
$this->expectException(FlowQueryException::class);
$flowQuery = new \Neos\Eel\FlowQuery\FlowQuery([]);
$operation = new SortByTimestampOperation();
$operation->evaluate($flowQuery, ['erstellt']);
}

/**
* @test
*/
public function invalidSortDirectionCausesException()
{
$this->expectException(FlowQueryException::class);
$flowQuery = new \Neos\Eel\FlowQuery\FlowQuery([]);
$operation = new SortByTimestampOperation();
$operation->evaluate($flowQuery, ['created', 'FOO']);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

declare(strict_types=1);

namespace Neos\ContentRepository\NodeAccess\Tests\Unit\FlowQueryOperations;

use Neos\ContentRepository\NodeAccess\FlowQueryOperations\SortOperation;
use Neos\Eel\FlowQuery\FlowQueryException;
use PHPUnit\Framework\TestCase;

/**
* SortOperation test
*/
class SortOperationTest extends TestCase
{
/**
* @test+
*/
public function callWithoutArgumentsCausesException()
{
$this->expectException(FlowQueryException::class);
$flowQuery = new \Neos\Eel\FlowQuery\FlowQuery([]);
$operation = new SortOperation();
$operation->evaluate($flowQuery, []);
}

/**
* @test
*/
public function invalidSortDirectionCausesException()
{
$this->expectException(FlowQueryException::class);
$flowQuery = new \Neos\Eel\FlowQuery\FlowQuery([]);
$operation = new SortOperation();
$operation->evaluate($flowQuery, ['title', 'FOO']);
}

/**
* @test
*/
public function invalidSortOptionCausesException()
{
$this->expectException(FlowQueryException::class);
$flowQuery = new \Neos\Eel\FlowQuery\FlowQuery([]);
$operation = new SortOperation();
$operation->evaluate($flowQuery, ['title', 'ASC', 'SORT_BAR']);
}
}
6 changes: 6 additions & 0 deletions Neos.Neos/Classes/Fusion/Helper/NodeHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

namespace Neos\Neos\Fusion\Helper;

use Neos\ContentRepository\Core\Feature\SubtreeTagging\Dto\SubtreeTag;
use Neos\ContentRepository\Core\NodeType\NodeType;
use Neos\ContentRepository\Core\Projection\ContentGraph\AbsoluteNodePath;
use Neos\ContentRepository\Core\Projection\ContentGraph\ContentSubgraphInterface;
Expand Down Expand Up @@ -162,6 +163,11 @@ public function subgraphForNode(Node $node): ContentSubgraphInterface
return $this->contentRepositoryRegistry->subgraphForNode($node);
}

public function isDisabled(Node $node): bool
{
return $node->tags->contain(SubtreeTag::disabled());
}

/**
* @param string $methodName
* @return boolean
Expand Down
33 changes: 33 additions & 0 deletions Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ Feature: Tests for the "Neos.ContentRepository" Flow Query methods.
| Key | Value |
| nodeAggregateId | "root" |
| nodeTypeName | "Neos.Neos:Sites" |
Given the current date and time is "2024-09-22T12:00:00+01:00"
And the following CreateNodeAggregateWithNode commands are executed:
| nodeAggregateId | parentNodeAggregateId | nodeTypeName | initialPropertyValues | nodeName |
| a | root | Neos.Neos:Site | {"title": "Node a"} | a |
Expand All @@ -71,6 +72,11 @@ Feature: Tests for the "Neos.ContentRepository" Flow Query methods.
| a1b3 | a1b | Neos.Neos:Test.DocumentType1 | {"uriPathSegment": "a1b3", "title": "Node a1b3"} | a1b3 |
| a1c | a1 | Neos.Neos:Test.DocumentType1 | {"uriPathSegment": "a1c", "title": "Node a1c", "hiddenInMenu": true} | a1c |
| a1c1 | a1c | Neos.Neos:Test.DocumentType1 | {"uriPathSegment": "a1c1", "title": "Node a1c1"} | a1c1 |
Given the current date and time is "2024-09-22T18:00:00+01:00"
And the following CreateNodeAggregateWithNode commands are executed:
| nodeAggregateId | parentNodeAggregateId | nodeTypeName | initialPropertyValues | nodeName |
| a2 | a | Neos.Neos:Test.DocumentType1 | {"uriPathSegment": "a2", "title": "Node a2"} | a2 |

And A site exists for node name "a" and domain "http://localhost"
And the sites configuration is:
"""yaml
Expand Down Expand Up @@ -395,6 +401,33 @@ Feature: Tests for the "Neos.ContentRepository" Flow Query methods.
nothingToRemove: a1a4,a1a4,a1a4
"""

Scenario: Sort
When I execute the following Fusion code:
"""fusion
test = Neos.Fusion:DataStructure {
@context {
a2 = ${q(site).find('#a2').get(0)}
a1a1 = ${q(site).find('#a1a1').get(0)}
a1a2 = ${q(site).find('#a1a2').get(0)}
a1a3 = ${q(site).find('#a1a3').get(0)}
a1a4 = ${q(site).find('#a1a4').get(0)}
}
unsorted = ${q([a1a3, a1a4, a1a1, a1a2]).get()}
sortByTitleAsc = ${q([a1a3, a1a4, a1a1, a1a2]).sort("title", "ASC").get()}
sortByUriDesc = ${q([a1a3, a1a4, a1a1, a1a2]).sort("uriPathSegment", "DESC").get()}
# a2 is "older"
sortByDateAsc = ${q([a2, a1a1]).sortByTimestamp("created", "ASC").get()}
@process.render = Neos.Neos:Test.RenderNodesDataStructure
}
"""
Then I expect the following Fusion rendering result:
"""
unsorted: a1a3,a1a4,a1a1,a1a2
sortByTitleAsc: a1a1,a1a2,a1a3,a1a4
sortByUriDesc: a1a4,a1a3,a1a2,a1a1
sortByDateAsc: a1a1,a2
"""

Scenario: Node accessors (final Node access operations)
When the Fusion context node is "a1"
When I execute the following Fusion code:
Expand Down
Loading

0 comments on commit bc4de7f

Please sign in to comment.