!!! TASK: Workspace aware commands

[nezaniel]

This makes commands operate on workspaces instead of content streams, as this

• better reflects the User's intention
• gives the command handlers options to remediate merge conflicts, e.g when the workspace has been rebased in the meantime and its previous content stream no longer exists

Notice: For Rebasing, a surrogate WorkspaceWasRebased is used. This passes the tests but does not make much sense, instead we should introduce a new event WorkspaceRebaseWasStarted that sets the workspace to the new content stream so that during rebase the commands can be applied to the proper content stream.

Resolves: #4694

Neos Ui Part: neos/neos-ui#3661

Upgrade instructions

The php api of most content repository commands was adjusted. The content stream id argument was removed and replaced with the workspace name.
(To migrate you can also look into the Ui adjustments for inspiration)

$contentRepository->handle(
    SetNodeProperties::create(
        WorkspaceName::forLive(), // instead of $contentStreamId
        $subject->nodeAggregateId,
        $originDimensionSpacePoint,
        PropertyValuesToWrite::fromArray(['foo' => 'bar'])
    )
);

Important

This change is partially breaking to your existing events. To allow discarding or publishing a workspace the migration has to be applied:

flow migrateevents:migrateMetaDataToWorkspaceName

Checklist

• Code follows the PSR-12 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the 9.0 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

[kitsunet]

So much work with all the tests, but this looks great!

[bwaidelich]

Amazing, thanks for your hard work!

Can you give a bit more context to your notice re "For Rebasing, a surrogate WorkspaceWasRebased is used."?

[nezaniel]

Amazing, thanks for your hard work!

Can you give a bit more context to your notice re "For Rebasing, a surrogate WorkspaceWasRebased is used."?

Sure.
When rebasing (see https://github.com/neos/neos-development-collection/pull/4708/files#diff-3e5de1a138586c8c85cdd71ed1a765ecc0ba0d79ab2579a6000475d427cefda2L391), we forked a content stream, modified the commands to operate on that content stream and then applied them.

This no longer works since the commands now operate on workspaces and the workspace doesn't change during rebase. Thus, before the commands get applied, the workspace must already point to the new content stream. This can currently only be done by WorkspaceWasRebased, which is semantically wrong and should be replaced with something like WorkspaceRebaseWasStarted

[mhsdesign]

So okay i ran the neos ui e2e test locally in combination with neos/neos-ui#3661 and they pass.

At first i had little trouble getting this branch to work and emptied my db:

Warning: Undefined array key "workspaceName" in Neos.ContentRepository.Core/Classes/Feature/WorkspacePublication/Dto/NodeIdToPublishOrDiscard.php line 48

This WorkspaceWasPartiallyDiscarded event could not be deserialized when catching up:

{
    "workspaceName": "user-admin",
    "newContentStreamId": "ed1314b0-93cc-4bf1-b249-48e5f0c0449e",
    "previousContentStreamId": "543da977-ad93-4785-ae6c-b68333b16f8c",
    "discardedNodes": [
        {
            "contentStreamId": "543da977-ad93-4785-ae6c-b68333b16f8c",
            "nodeAggregateId": "5b0d6ac0-40ab-47e8-b79e-39de6c0700df",
            "dimensionSpacePoint": {
                "language": "en_US"
            }
        },
  ...

i have to test again that this change is really not breaking if swapped out on the fly

[kitsunet]

@mhsdesign did you retest this?

[kitsunet]

I tried it as well and so far didn't notice problems, so I would realyl like to get this in ASAP.

[ahaeslich]

At first i had little trouble getting this branch to work and emptied my db:

@kitsunet so you didn't ran into this error?

This passes the tests but does not make much sense, instead we should introduce a new event WorkspaceRebaseWasStarted that sets the workspace to the new content stream so that during rebase the commands can be applied to the proper content stream.

So this should be added in a follow up or do we need to introduce this event here? If we can add this later I'm with you on merging this ASAP.

[kitsunet]

Seemed fine for me, but my dataset is small. I applied patch, did cr:setup on my exisating database for good measure and tried the website.

[dlubitz]

Thank you very much for all the work!

[mhsdesign]

1. the event migration flow migrateevents:fillWorkspaceNameInCommandPayloadOfEventMetaData is ready to merged here into it TASK: Workspace aware commands events migration #4890

2. in the pr i tried to make the override cs stream flow more obvious, but its just a personal nitpick

3. im not sure if we have to adjust any inline code documentation about the publishing, i could only find these sketches which dont seem to be affected so all good ;)

./Neos.ContentRepository.Core/Classes/Feature/NodeMove/Event/NodeAggregateWasMoved.monopic
./Neos.ContentRepository.Core/Classes/Projection/ContentStream/ContentStreamFinder.monopic
./Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.monopic
./Neos.Neos/Classes/FrontendRouting/EventSourcedFrontendNodeRoutePartHandler.monopic

4. i cannot reproduce reliable the neos ui e2e failure documented here !!! TASK: Workspace aware commands #4708 (comment) so this should not be a blocker and might be another problem that existed beforehand.
   edit2: yes i can reproduce it, i had a sleep(1) statement in the code
   edit3: the problem was fixed with the merge of 4583-variantRecreation because the code was refactored and contained previously unsafe publishEvents which was not blocked in the handleDiscardIndividualNodesFromWorkspace in this line

   neos-development-collection/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php

   Lines 637 to 650 in b194763

   	$this->eventPersister->publishEvents(
   	new EventsToPublish(
   	$streamName,
   	Events::with(
   	new WorkspaceWasPartiallyDiscarded(
   	$command->workspaceName,
   	$command->newContentStreamId,
   	$workspace->currentContentStreamId,
   	$command->nodesToDiscard,
   	)
   	),
   	ExpectedVersion::ANY()
   	)
   	);

-> this was fixed

5. ~now the ui is failing at this moment with ~

Content stream "766799e1-2a06-4b15-8986-e156e9d74b60" is closed.

-> this was fixed

[mhsdesign]

Thank you so much for taking care.

[bwaidelich]

Yeah!! Thanks so much, @nezaniel and congratulations!!