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: Remove leftovers from magic underscore access for internal node properties #5015

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
17f04d7
TASK: Provide helper to find out if node is disabled in fusion
mhsdesign Apr 26, 2024
9884e6c
!!! TASK: Remove underscore node property access `_*` from property a…
mhsdesign Apr 26, 2024
00d82d1
TASK: Remove legacy b/c for `property('_path')`
mhsdesign Apr 26, 2024
0746dba
TASK: Remove legacy b/c for `property('_identifier')`
mhsdesign Apr 26, 2024
cd77d45
TASK: Move orphaned `SortOperation` to other Node eel flow-queries
mhsdesign Apr 26, 2024
8e4298f
!!! TASK: Remove underscore node property access `sort('_*')` from so…
mhsdesign Apr 26, 2024
5f8579c
TASK: Adjust docs of `PropertyOperation`
mhsdesign May 12, 2024
7554048
TASK: Add behat test for sort operation
mhsdesign Jul 3, 2024
1be047a
TASK: Readd unit test for sort operation
mhsdesign Jul 3, 2024
9b2b607
TASK: Reimplement _creationDateTime, _lastModificationDateTime and _l…
mhsdesign Jul 3, 2024
3ebe2a4
TASK: Simplify happy path for q().property
mhsdesign Jul 3, 2024
4efafb8
Merge remote-tracking branch 'origin/9.0' into task/4208-remove-magic…
mhsdesign Sep 24, 2024
8c72a52
TASK: Remove obsolete ParentsOperationTest
mhsdesign Sep 26, 2024
c1fb301
FEATURE: Introduce sortByTimestamp fq
mhsdesign Sep 26, 2024
00c9ee4
TASK: Correct casing of `SortByTimeStampOperation`
mhsdesign Sep 27, 2024
546f398
TASK: Correct casing of SortByTimestampOperation
dlubitz Sep 27, 2024
98cf58c
TASK: Add `originalCreated` as option
mhsdesign Oct 9, 2024
60b11b2
Merge remote-tracking branch 'origin/9.0' into task/4208-remove-magic…
mhsdesign Oct 10, 2024
08b0ba1
TASK: Test `sortByTimestamp` by manipulating date time
mhsdesign Oct 10, 2024
6d5c1bc
Merge remote-tracking branch 'origin/9.0' into task/4208-remove-magic…
mhsdesign Oct 11, 2024
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 @@ -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(),
mhsdesign marked this conversation as resolved.
Show resolved Hide resolved
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
Loading