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.

Sign up for GitHub

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

BUGFIX: Handle nodes with missing UriPathSegments in uriPathProjection #5412

Open
wants to merge 3 commits into
base: 9.0
Choose a base branch
from
Open

BUGFIX: Handle nodes with missing UriPathSegments in uriPathProjection #5412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2445a69
TASK: Add testcase for documents that are projected without uriPathSe…
mficzel Dec 22, 2024
828908e
BUGFIX: Use nodeAggregateId as fallback for uriPathSegment instead of ""
mficzel Dec 22, 2024
64d52a7
TASK: Add site nodeName to testcases to avoid relying on invalid beha…
mficzel Dec 22, 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
2 changes: 1 addition & 1 deletion Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathProjection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ private function whenNodeAggregateWithNodeWasCreated(NodeAggregateWithNodeWasCre
}

$propertyValues = $event->initialPropertyValues->getPlainValues();
$uriPathSegment = $propertyValues['uriPathSegment'] ?? '';
$uriPathSegment = $propertyValues['uriPathSegment'] ?? $event->nodeAggregateId->value;
Copy link
Member

Choose a reason for hiding this comment

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

we discussed an alternative in line 273 but it doesnt really work :D

                $uriPath = $this->nodeTypeManager->getNodeType($parentNode->getNodeTypeName())?->isOfType(NodeTypeNameFactory::NAME_SITE)
                    ? $uriPathSegment
                    : $parentNode->getUriPath() . '/' . $uriPathSegment;

Copy link
Member Author

Choose a reason for hiding this comment

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

Still think resilience against missing uri path segments is better than adding an even tighter bond to the Neos NodeTypes. Even if this would work ;-)

The fallback way makes uriPathes more like a soft concern (which it is technically anyways) and allows routing of documents in cases where it is missing for any reason.

Only thing that would speak against that would be a case where the node-identifier based fallback does harm anything but i do not see a way for this to happen.


$shortcutTarget = null;
if ($documentTypeClassification === DocumentTypeClassification::CLASSIFICATION_SHORTCUT) {
Expand Down
104 changes: 104 additions & 0 deletions Neos.Neos/Tests/Behavior/Features/FrontendRouting/MissingPathSegments.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
@flowEntities @contentrepository
Feature: Routing functionality if path segments are missing like during tethered node creation

Background:
Given using no content dimensions
And using the following node types:
"""yaml
'Neos.Neos:Sites':
superTypes:
'Neos.ContentRepository:Root': true
'Neos.Neos:Document': {}
'Neos.Neos:Content': {}
'Neos.Neos:Test.Routing.Page':
superTypes:
'Neos.Neos:Document': true
properties:
uriPathSegment:
type: string
'Neos.Neos:Test.Routing.Content':
superTypes:
'Neos.Neos:Content': true
properties:
uriPathSegment:
type: string
"""
And using identifier "default", I define a content repository
And I am in content repository "default"
And I am user identified by "initiating-user-identifier"

When the command CreateRootWorkspace is executed with payload:
| Key | Value |
| workspaceName | "live" |
| newContentStreamId | "cs-identifier" |
And I am in workspace "live" and dimension space point {}
And the command CreateRootNodeAggregateWithNode is executed with payload:
| Key | Value |
| nodeAggregateId | "lady-eleonode-rootford" |
| nodeTypeName | "Neos.Neos:Sites" |

# lady-eleonode-rootford
# shernode-homes
# sir-david-nodenborough
# duke-of-contentshire (content node)
# earl-o-documentbourgh
# nody-mc-nodeface
#
And the following CreateNodeAggregateWithNode commands are executed:
| nodeAggregateId | parentNodeAggregateId | nodeTypeName | initialPropertyValues | nodeName |
| shernode-homes | lady-eleonode-rootford | Neos.Neos:Test.Routing.Page | {} | node1 |
| sir-david-nodenborough | shernode-homes | Neos.Neos:Test.Routing.Page | {} | node2 |
| duke-of-contentshire | sir-david-nodenborough | Neos.Neos:Test.Routing.Content | {} | node3 |
| earl-o-documentbourgh | sir-david-nodenborough | Neos.Neos:Test.Routing.Page | {} | node4 |
| nody-mc-nodeface | shernode-homes | Neos.Neos:Test.Routing.Page | {} | node5 |
And A site exists for node name "node1"
And the sites configuration is:
"""yaml
Neos:
Neos:
sites:
'node1':
preset: 'default'
uriPathSuffix: ''
contentDimensions:
resolver:
factoryClassName: Neos\Neos\FrontendRouting\DimensionResolution\Resolver\NoopResolverFactory
"""
Scenario: Match homepage URL
When I am on URL "/"
Then the matched node should be "shernode-homes" in content stream "cs-identifier" and dimension "{}"

Scenario: Resolve nodes correctly from homepage
When I am on URL "/"
Then the node "shernode-homes" in content stream "cs-identifier" and dimension "{}" should resolve to URL "/"
And the node "sir-david-nodenborough" in content stream "cs-identifier" and dimension "{}" should resolve to URL "/sir-david-nodenborough"
And the node "earl-o-documentbourgh" in content stream "cs-identifier" and dimension "{}" should resolve to URL "/sir-david-nodenborough/earl-o-documentbourgh"

Scenario: Match node lower in the tree
When I am on URL "/sir-david-nodenborough/earl-o-documentbourgh"
Then the matched node should be "earl-o-documentbourgh" in content stream "cs-identifier" and dimension "{}"

Scenario: Resolve from node lower in the tree
When I am on URL "/sir-david-nodenborough/earl-o-documentbourgh"
Then the node "shernode-homes" in content stream "cs-identifier" and dimension "{}" should resolve to URL "/"
And the node "sir-david-nodenborough" in content stream "cs-identifier" and dimension "{}" should resolve to URL "/sir-david-nodenborough"
And the node "earl-o-documentbourgh" in content stream "cs-identifier" and dimension "{}" should resolve to URL "/sir-david-nodenborough/earl-o-documentbourgh"

Scenario: Add uri path segment on first level
When the command SetNodeProperties is executed with payload:
| Key | Value |
| nodeAggregateId | "sir-david-nodenborough" |
| originDimensionSpacePoint | {} |
| propertyValues | {"uriPathSegment": "david-nodenborough-updated"} |
And I am on URL "/"
Then the node "sir-david-nodenborough" in content stream "cs-identifier" and dimension "{}" should resolve to URL "/david-nodenborough-updated"
And the node "earl-o-documentbourgh" in content stream "cs-identifier" and dimension "{}" should resolve to URL "/david-nodenborough-updated/earl-o-documentbourgh"

Scenario: Add uri path segment on second level
When the command SetNodeProperties is executed with payload:
| Key | Value |
| nodeAggregateId | "earl-o-documentbourgh" |
| originDimensionSpacePoint | {} |
| propertyValues | {"uriPathSegment": "earl-documentbourgh-updated"} |
And I am on URL "/"
Then the node "earl-o-documentbourgh" in content stream "cs-identifier" and dimension "{}" should resolve to URL "/sir-david-nodenborough/earl-documentbourgh-updated"
1 change: 1 addition & 0 deletions Neos.Neos/Tests/Behavior/Features/FrontendRouting/NodeCreationEdgeCases.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Feature: Test cases for node creation edge cases
| nodeTypeName | "Neos.Neos:Site" |
| parentNodeAggregateId | "lady-eleonode-rootford" |
| originDimensionSpacePoint | {"example":"source"} |
| nodeName | "site" |
And the following CreateNodeAggregateWithNode commands are executed:
| nodeAggregateId | nodeName | parentNodeAggregateId | succeedingSiblingNodeAggregateId | nodeTypeName | initialPropertyValues |
# Let's prepare some siblings to check orderings. Also, everything gets better with siblings.
Expand Down
3 changes: 3 additions & 0 deletions Neos.Neos/Tests/Behavior/Features/FrontendRouting/NodeVariationEdgeCases.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Feature: Test cases for node variation edge cases
| nodeTypeName | "Neos.Neos:Site" |
| parentNodeAggregateId | "lady-eleonode-rootford" |
| originDimensionSpacePoint | {"example":"source"} |
| nodeName | "site" |
And the command CreateNodeVariant is executed with payload:
| Key | Value |
| nodeAggregateId | "shernode-homes" |
Expand Down Expand Up @@ -135,6 +136,7 @@ Feature: Test cases for node variation edge cases
| nodeTypeName | "Neos.Neos:Site" |
| parentNodeAggregateId | "lady-eleonode-rootford" |
| originDimensionSpacePoint | {"example":"rootGeneral"} |
| nodeName | "site" |
And the following CreateNodeAggregateWithNode commands are executed:
| nodeAggregateId | originDimensionSpacePoint | nodeName | parentNodeAggregateId | succeedingSiblingNodeAggregateId | nodeTypeName | initialPropertyValues |
# Let's create some siblings, both in source and target, to check ordering
Expand Down Expand Up @@ -225,6 +227,7 @@ Feature: Test cases for node variation edge cases
| nodeTypeName | "Neos.Neos:Site" |
| parentNodeAggregateId | "lady-eleonode-rootford" |
| originDimensionSpacePoint | {"example":"source"} |
| nodeName | "site" |
And the following CreateNodeAggregateWithNode commands are executed:
| nodeAggregateId | nodeName | parentNodeAggregateId | succeedingSiblingNodeAggregateId | nodeTypeName | initialPropertyValues |
# Let's create our test subject...
Expand Down
5 changes: 3 additions & 2 deletions Neos.Neos/Tests/Behavior/Features/FrontendRouting/PartialPublish.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Feature: Test cases for partial publish to live and uri path generation
| nodeTypeName | "Neos.Neos:Site" |
| parentNodeAggregateId | "lady-eleonode-rootford" |
| originDimensionSpacePoint | {"example":"source"} |
| nodeName | "site" |
And the command CreateWorkspace is executed with payload:
| Key | Value |
| workspaceName | "myworkspace" |
Expand All @@ -49,7 +50,7 @@ Feature: Test cases for partial publish to live and uri path generation
| nodeTypeName | "Neos.Neos:Document" |
| parentNodeAggregateId | "shernode-homes" |
| originDimensionSpacePoint | {"example":"source"} |
| properties | {"uriPathSegment": "just"}|
| initialPropertyValues | {"uriPathSegment": "just"}|
| workspaceName | "myworkspace" |
And the command PublishIndividualNodesFromWorkspace is executed with payload:
| Key | Value |
Expand All @@ -65,4 +66,4 @@ Feature: Test cases for partial publish to live and uri path generation
| "65901ded4f068dac14ad0dce4f459b29" | "" | "lady-eleonode-rootford" | "lady-eleonode-rootford" | null | null | null | "Neos.Neos:Sites" |
| "fbe53ddc3305685fbb4dbf529f283a0e" | "" | "lady-eleonode-rootford" | "lady-eleonode-rootford" | null | null | null | "Neos.Neos:Sites" |
| "65901ded4f068dac14ad0dce4f459b29" | "" | "lady-eleonode-rootford/shernode-homes" | "shernode-homes" | "lady-eleonode-rootford" | null | null | "Neos.Neos:Site" |
| "65901ded4f068dac14ad0dce4f459b29" | "" | "lady-eleonode-rootford/shernode-homes/justsomepage" | "justsomepage" | "shernode-homes" | null | null | "Neos.Neos:Document" |
| "65901ded4f068dac14ad0dce4f459b29" | "just" | "lady-eleonode-rootford/shernode-homes/justsomepage" | "justsomepage" | "shernode-homes" | null | null | "Neos.Neos:Document" |
1 change: 1 addition & 0 deletions Neos.Neos/Tests/Behavior/Features/FrontendRouting/UnknownNodeTypes.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Feature: Basic routing functionality (match & resolve nodes with unknown types)
| nodeTypeName | "Neos.Neos:Test.Routing.Page" |
| parentNodeAggregateId | "lady-eleonode-rootford" |
| nodeAggregateClassification | "regular" |
| nodeName | "site" |
And the event NodeAggregateWithNodeWasCreated was published with payload:
| Key | Value |
| workspaceName | "live" |
Expand Down
Loading