neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
264 stars 222 forks source link

9.0: how to solve unique uri path segments? #4581

Open ahaeslich opened 1 year ago

ahaeslich commented 1 year ago

Test what happens if two nodes on the same level share the same uriPathSegment value and evaluate ways to prevent / resolve this

Accidentally tested on 31.03.2023: Both pages will get the same uriPathSegment and the first node found in the table cr_default_p_neos_documenturipath_uri will be accessed when navigating in the frontend.

Christian - Some thoughts 03.05.2023 We don't really want to constrain this at creation time I suppose. So I don't know how dirty this would be but what about the following? If we encounter an already existing uri path while updating the projection we append the node aggregate identifier to absolutely deduplicate the path and immediately issue an event propagating this "new" uri path to the actual node, which in turn allows editors to see this effect and react accordingly by (possibly) changing to another path. If someone wants to implement something on top eg. for imports they can always ask the projection before setting an uri path, but that is obviously not perfectly save, so we need that fallback described above. WDYT?

Related: neos/neos-ui#3676

ahaeslich commented 1 year ago

In Neos 8 a suffix will be generated for an identical uriPathSegment.

bwaidelich commented 8 months ago

A few ideas after briefly discussing this in todays weekly:

I can think of a couple of ways to solve this with increasing consistency/complexity:

a) ignore it

Obviously not a satisfactory solution, but maybe it is not too bad as long as we make sure that the UriPathProjection detects this case and does not override an existing URL if a new node with the same uriPathSegment is created.

We could improve UX by flagging duplicates in the read model and highlighting it in the UI

b) dealing with it in the projection(s)

As Christian suggested, we could detect duplicates at projection time and append a suffix in the UriPathProjection read model to make the path unique. There are a couple of issues though:

c) use the read model to detect duplicates at write time

As long as the read model is up-to-date we can use it for constraints like this. I assume that we already do this when we create nodes, but apparently not when we copy/move them.

There would still be a chance of a duplicate due to the eventual consistent nature – but we use the graph for constraint checks already so this should be OK.

d) detecting duplicates via process manager

To make the suffix creation deterministic we could postpone it to some process manager that detects duplicates and emits corresponding NodePropertiesWhereSet events with the modified uriPathSegment.

While this seems like a legit way to go, this would require us to introduce the notion of plain event listeners (that cannot be replayed).

e) introducing a hard constraint

We currently don't have actual hard constraints (based on events, not on read models) so this is not a short term option. In the long run, this would probably the way to go.

General considerations

back to problem space

Before we can decide on a solution we should properly document the status quo. @ahaeslich wants to test the scenario, a failing behat test would be perfect of course.

In general we should look at this from the perspective of an editor using the system.

Only live?

In theory duplicated uriPathSegment values should not be a problem anywhere except for the live content stream.. Maybe that fact can be used to reduce the performance/complexity impact

Introduce unique properties?

"uriPathSegment" only has a special meaning in Neos – which makes some of the above considerations harder to implement. We could think about a general flag in the Node type schema that marks a property as unique (globally or on one level) to generalize this feature. But obviously there be dragons and it's probably not a good idea :)

ahaeslich commented 8 months ago

I assume that we already do this when we create nodes, ...

As of now the uriPathSegment generation isn't that consistent in Neos 8 (will update the test cases in the issue description). But at least a new page will get a postfix on creation. This is not happening in Neos 9 as of now.

bwaidelich commented 8 months ago

@ahaeslich Thanks for investigating. What exactly happens in Neos 9 today if you create two pages with the same URI? Will the old one keeps working (better) or is it replaced with the new one (no-go IMO)

bwaidelich commented 7 months ago

@ahaeslich ping

sbruggmann commented 7 months ago

@bwaidelich Currently with Neos 9 in the UI both Nodes are still working. For the frontend there are two identical uripath entries created in the documentUriPath_uri projection.

In general we should look at this from the perspective of an editor using the system.

I strongly belive that the editor does have to see the final url in the UI even if he just created an unpublished node. This could, and maybe has to be updated automatically again while publishing to live (maybe someone else created a node with the same uriPathSegment in the meantime).

I'd go with "c".

bwaidelich commented 7 months ago

@sbruggmann Thanks for the update. I'll have a look into this if nobody else does before