TASK: Verify and optimize FlowQuery operations #4699

mficzel · 2023-11-03T16:09:30Z

Upmerge the flowQuery Tests from 8.3, fix bugs and optimize the implementation to reduce the amount of db-queries as far as possible without introducing breakiness. The tests for unique and remove are added here because those operations did not exist in Neos 8.3.

The optimizations had to be very defensive and could not improve the children and find much since the following fizzle features could not be supported by PropertyValueConstraints:

Case insensitive comparisons =~, ^=~, *=~ and $=~ (this may be fixed with Streamline case sensitive behavior of PropertyValueCriteria #4721)
Comparison of nested properties life foo.bar = "baz"
Comparison of array entities in ^=, *= and $=

This may be optimized further in a future release or a third party package that allows itself more breakiness.

Notes about changes compared to Neos 8.3:

The order of nodes returned by find with typeFilter has changed. Since this was never guaranteed this is ok

In addition:

Nodes::last method is added
Nodes::until is removed because of broken semantic. BetterAlternatives are previousAll and nextAll
Nodes::isEmpty add assertions that first and last will return not null
NodeNameFactory::forRoot is added

Relates: #4702, #4695

Upgrade instructions

Review instructions

This pr contains the upmerge of #4695 and #4702 (follow up) as this will otherwise cause lots of errors after upmerging.
You can compare this by checking out this pr and compare the content of Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature with the 8.3 branch.

I consider the current behavior (with this pr) ok for a beta 1 as find (with nodetypes) already returns the correct nodes but ordered differently.

Checklist

Code follows the PSR-2 coding style
Tests have been created, run and adjusted as needed
The PR is created against the lowest maintained branch
Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
Reviewer - The first section explains the change briefly for change-logs
Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

…ifyAndOptimizeFlowQuery # Conflicts: # Neos.Neos/Documentation/References/CommandReference.rst # Neos.Neos/Documentation/References/EelHelpersReference.rst # Neos.Neos/Documentation/References/FlowQueryOperationReference.rst # Neos.Neos/Documentation/References/Signals/ContentRepository.rst # Neos.Neos/Documentation/References/Signals/Flow.rst # Neos.Neos/Documentation/References/Signals/Media.rst # Neos.Neos/Documentation/References/Signals/Neos.rst # Neos.Neos/Documentation/References/Validators/Flow.rst # Neos.Neos/Documentation/References/Validators/Media.rst # Neos.Neos/Documentation/References/Validators/Party.rst # Neos.Neos/Documentation/References/ViewHelpers/ContentRepository.rst # Neos.Neos/Documentation/References/ViewHelpers/FluidAdaptor.rst # Neos.Neos/Documentation/References/ViewHelpers/Form.rst # Neos.Neos/Documentation/References/ViewHelpers/Fusion.rst # Neos.Neos/Documentation/References/ViewHelpers/Media.rst # Neos.Neos/Documentation/References/ViewHelpers/Neos.rst # Neos.Neos/Documentation/References/ViewHelpers/TYPO3Fluid.rst

…ifyAndOptimizeFlowQuery # Conflicts: # Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature

… `Prev`, `PrevAll` In addition: - `Nodes::last` method is added - `Nodes::until` is removed because of broken semantic. BetterAlternatives are `previousAll` and `nextAll` - `NodeNameFactory::forRoot` is added - `Nodes::isEmpty` add assertions that `first` and `last` will return not null

mhsdesign · 2023-11-03T17:43:19Z

Fyi to why until will be removed https://neos-project.slack.com/archives/C3MCBK6S2/p1699020664345879

Neos.ContentRepository.Core/Classes/Projection/ContentGraph/Nodes.php

mhsdesign · 2023-11-04T09:29:28Z

thanks for taking care. It looks quite logical and way more performant ^^. I like to wait for #4695 to be upmerged so we easier review the tests.

…ifyAndOptimizeFlowQuery # Conflicts: # Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature

…er expectation of type filter

Co-authored-by: Marc Henry Schultz <85400359+mhsdesign@users.noreply.github.com>

mficzel · 2023-11-04T17:20:35Z

@mhsdesign if you look into the commits you will find that this pr contains the upmerge of #4695 and #4702 (follow up) you can compare the test cases by checking out the pr and use compare branches on the feature file to see the differences.

mficzel · 2023-11-04T17:38:07Z

I suggest to look into the children and find operation again in a follow pr as those are more complex and still look weird inside.

…owQuery

mhsdesign · 2023-11-05T13:15:33Z

Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature

+      # @todo Decide wether the changed order of the results compared to Neos 8.3 is ok
+      # Result in Neos 8.3: "typeFilter: a1a,a1b1a,a1a2,a1b2,a1a3,a1a4,a1a5,a1a6"
+      # Result in Neos 9.0: "typeFilter: a1a,a1a2,a1b2,a1a3,a1a4,a1a5,a1a6,a1b1a"
      typeFilter = ${q(node).find('[instanceof Neos.Neos:Test.DocumentType2]').get()}
-      combinedFilter = ${q(node).find('[instanceof Neos.Neos:Test.DocumentType2][uriPathSegment*="b1"]').get()}
+      # @todo Fix and re enable `combinedFilter` case
+      # Result in Neos 8.3: "combinedFilter: a1b1a"
+      # Result in Neos 9.0: "combinedFilter: a1a,a1a2,a1b2,a1a3,a1a4,a1a5,a1a6,a1b1a"
+      # combinedFilter = ${q(node).find('[instanceof Neos.Neos:Test.DocumentType2][uriPathSegment*="b1"]').get()}
+      @process.render = Neos.Neos:Test.RenderNodesDataStructure
+    }


Should we resolve this todo here or in a followup?

@mhsdesign in a follow up. I did no changes for those operations so this only documents the deviation from 8.3 to 9.0 that was there. The fixes for the other operations already optimize quite a bit that should go to beta1

children and find will have to wait for beta 2

Might also be that filter could benefit from this #4701

okay i thought so already so lets not block this any further ;)

mhsdesign

Thanks great work. Some operations have different priorities either 0 or 100 - probably just due to copy pasta at some point.

i also want to point out that the order of returned items doesnt seem to always reflect the cr philosophy of the closest first, see PrevUntil expected a1a3,a1a2 actual a1a2,a1a3

So maybe we should fix this - in another cleanup? - and advise people to just reverse if they need the old behaviour ...

Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/PrevUntilOperation.php

mhsdesign

;)

mficzel · 2023-11-07T16:00:24Z

@mhsdesign pushed a change to

restore the operation priority as it was in 8.3 (100 for operatins that override derfault flowQuery operations and 0 otherwise)
define shortNames for all flowQuery operations

... pretty sure the complaints of the linter are not caused by me ...

mficzel · 2023-11-07T16:14:05Z

Just seen that the same errors now pop up in the 9.0 branch. ... now i am sure i am not guilty

mhsdesign · 2023-11-07T17:49:27Z

Thanks @mficzel now its perfect ;)

…not like the new syntax (yet)

Absolute pathes are handled separately as the fizzle parser cannot handle the syntax

…tation

mhsdesign · 2023-11-14T18:34:12Z

I made behat test now run in strict mode, as previously a non implemented snipped would not cause behat to fail???
Cant do it on the 9.0 branch as the current 8.3 upmerged flowquery test would fail in that case 😂

…owQuery

kitsunet

Very nice, thank youuuuuu! :)

mficzel added 4 commits November 3, 2023 12:51

TASK: Adjust test for the new cr

1d986c9

TASK: Fix ParentsUntil Operation and add tests for unique and remove

8e25777

Merge branch 'task/addBehatTestsForFlowQueryCrOperations' into 90/ver…

50182cd

…ifyAndOptimizeFlowQuery # Conflicts: # Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature

github-actions bot added Task 9.0 labels Nov 3, 2023

mficzel force-pushed the 90/verifyAndOptimizeFlowQuery branch from 4940424 to 37d901e Compare November 3, 2023 16:36

mficzel marked this pull request as ready for review November 3, 2023 16:48

mficzel added the 9.0beta1 label Nov 3, 2023

mficzel requested review from skurfuerst, bwaidelich, mhsdesign and ahaeslich November 3, 2023 16:49

mhsdesign changed the title ~~TASK: Verify and optimize flowQery operations~~ TASK: Verify and optimize FlowQuery operations Nov 4, 2023

mhsdesign reviewed Nov 4, 2023

View reviewed changes

Neos.ContentRepository.Core/Classes/Projection/ContentGraph/Nodes.php Outdated Show resolved Hide resolved

mhsdesign reviewed Nov 4, 2023

View reviewed changes

Neos.ContentRepository.Core/Classes/Projection/ContentGraph/Nodes.php Outdated Show resolved Hide resolved

mficzel and others added 3 commits November 4, 2023 17:59

Merge branch 'task/addBehatTestsForFlowQueryCrOperations' into 90/ver…

42421b4

…ifyAndOptimizeFlowQuery # Conflicts: # Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature

TASK: Disable the failing parts of find operation and adjust the od…

0763ed8

…er expectation of type filter

Apply suggestions from code review

6c33434

Co-authored-by: Marc Henry Schultz <85400359+mhsdesign@users.noreply.github.com>

TASK: Adjust format of the todo notice in find operation

cbbfea3

mficzel and others added 2 commits November 4, 2023 18:46

TASK: Fix phpstan type assertation annotations

3c2e461

Merge remote-tracking branch 'origin/9.0' into 90/verifyAndOptimizeFl…

ba37e4a

…owQuery

mhsdesign reviewed Nov 5, 2023

View reviewed changes

mhsdesign reviewed Nov 7, 2023

View reviewed changes

Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/PrevUntilOperation.php Show resolved Hide resolved

mhsdesign approved these changes Nov 7, 2023

View reviewed changes

TASK: Add sortNames and priority to flowQuery operations

45d3bd4

ahaeslich removed the 9.0beta1 label Nov 7, 2023

mficzel mentioned this pull request Nov 7, 2023

TASK: Fix and improve find and children flowQuery operation #4712

Draft

6 tasks

Merge branch '9.0' into 90/verifyAndOptimizeFlowQuery

67f343e

ahaeslich requested review from kitsunet and dlubitz November 10, 2023 10:24

TASK: Add special handling of absolute node pathes since fizzle does …

5f288c6

…not like the new syntax (yet)

mficzel force-pushed the 90/verifyAndOptimizeFlowQuery branch from 061fe3a to e7de457 Compare November 10, 2023 17:10

TASK: Fix find operation

76d0eb8

Absolute pathes are handled separately as the fizzle parser cannot handle the syntax

mficzel force-pushed the 90/verifyAndOptimizeFlowQuery branch from e7de457 to 76d0eb8 Compare November 11, 2023 10:18

TASK: Enable behat strict mode to fail tests without snippet implemen…

b5a0fc7

…tation

bwaidelich removed their request for review January 17, 2024 14:52

Merge remote-tracking branch 'origin/9.0' into 90/verifyAndOptimizeFl…

b9ce619

…owQuery

kitsunet approved these changes Feb 2, 2024

View reviewed changes

kitsunet merged commit f4f9df6 into neos:9.0 Feb 2, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TASK: Verify and optimize FlowQuery operations #4699

TASK: Verify and optimize FlowQuery operations #4699

mficzel commented Nov 3, 2023 •

edited

Loading

mhsdesign commented Nov 3, 2023

mhsdesign commented Nov 4, 2023

mficzel commented Nov 4, 2023

mficzel commented Nov 4, 2023

mhsdesign Nov 5, 2023

mficzel Nov 6, 2023 •

edited

Loading

mficzel Nov 6, 2023 •

edited

Loading

mficzel Nov 6, 2023

mhsdesign Nov 7, 2023

mhsdesign left a comment

mhsdesign left a comment

mficzel commented Nov 7, 2023 •

edited

Loading

mficzel commented Nov 7, 2023

mhsdesign commented Nov 7, 2023

mhsdesign commented Nov 14, 2023

kitsunet left a comment

TASK: Verify and optimize FlowQuery operations #4699

TASK: Verify and optimize FlowQuery operations #4699

Conversation

mficzel commented Nov 3, 2023 • edited Loading

mhsdesign commented Nov 3, 2023

mhsdesign commented Nov 4, 2023

mficzel commented Nov 4, 2023

mficzel commented Nov 4, 2023

mhsdesign Nov 5, 2023

Choose a reason for hiding this comment

mficzel Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

mficzel Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

mficzel Nov 6, 2023

Choose a reason for hiding this comment

mhsdesign Nov 7, 2023

Choose a reason for hiding this comment

mhsdesign left a comment

Choose a reason for hiding this comment

mhsdesign left a comment

Choose a reason for hiding this comment

mficzel commented Nov 7, 2023 • edited Loading

mficzel commented Nov 7, 2023

mhsdesign commented Nov 7, 2023

mhsdesign commented Nov 14, 2023

kitsunet left a comment

Choose a reason for hiding this comment

mficzel commented Nov 3, 2023 •

edited

Loading

mficzel Nov 6, 2023 •

edited

Loading

mficzel Nov 6, 2023 •

edited

Loading

mficzel commented Nov 7, 2023 •

edited

Loading