Skip to content

Commit

Permalink
TASK: Make publish a no-op if there are no changes instead of a rebase
Browse files Browse the repository at this point in the history 
see discussion https://neos-project.slack.com/archives/C04PYL8H3/p1730111291595599

otherwise the logic got way to complex :D

previously a publishing without changes is a no-op if up to date and rebase if outdated
  • Loading branch information
@mhsdesign
mhsdesign committed Oct 28, 2024
1 parent 5339f97 commit f0e088e
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 61 deletions.
18 changes: 7 additions & 11 deletions ...havioralTests/Tests/Behavior/Features/W7-WorkspacePublication/02-PublishWorkspace.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ Feature: Workspace based content publishing
Then I expect exactly 4 events to be published on stream "ContentStream:cs-identifier"
Then I expect exactly 1 event to be published on stream "Workspace:live"

Scenario: Publish is a rebase if the workspace is outdated and no changes are to be published
Scenario: Publish is a no-op if there are no changes (and the workspace is outdated)
And the command SetNodeProperties is executed with payload:
| Key | Value |
| workspaceName | "live" |
Expand All @@ -200,17 +200,13 @@ Feature: Workspace based content publishing
| Key | Value |
| workspaceName | "user-test" |
| newContentStreamId | "user-cs-new" |
Then workspaces user-test has status OUTDATED

Then I expect exactly 2 events to be published on stream with prefix "Workspace:user-test"
And event at index 1 is of type "WorkspaceWasRebased" with payload:
| Key | Expected |
| workspaceName | "user-test" |
| newContentStreamId | "user-cs-new" |
| previousContentStreamId | "user-cs-identifier" |
Then I expect exactly 1 events to be published on stream with prefix "Workspace:user-test"

And I am in workspace "user-test" and dimension space point {}
Then I expect node aggregate identifier "nody-mc-nodeface" to lead to node user-cs-new;nody-mc-nodeface;{}
Then I expect node aggregate identifier "nody-mc-nodeface" to lead to node user-cs-identifier;nody-mc-nodeface;{}
And I expect this node to have the following properties:
| Key | Value |
| text | "Modified in live workspace" |
Then I expect the content stream "user-cs-identifier" to not exist
| Key | Value |
| text | "Original" |
Then I expect the content stream "user-cs-new" to not exist
18 changes: 5 additions & 13 deletions ...vioralTests/Tests/Behavior/Features/W8-IndividualNodePublication/02-BasicFeatures.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,11 @@ Feature: Individual node publication
Then I expect a node identified by cs-identifier;sir-david-nodenborough;{} to exist in the content graph


Scenario: Partial publish is a rebase if the workspace is outdated and no changes exist
Scenario: Partial publish is a no-op if the workspace doesnt contain any changes (and the workspace is outdated)

When the command CreateNodeAggregateWithNode is executed with payload:
| Key | Value |
| workspaceName | "live" |
| nodeAggregateId | "nody-mc-nodeface" |
| nodeTypeName | "Neos.ContentRepository.Testing:Content" |
| originDimensionSpacePoint | {} |
Expand All @@ -80,18 +81,9 @@ Feature: Individual node publication
| workspaceName | "user-test" |
| nodesToPublish | [{"dimensionSpacePoint": {}, "nodeAggregateId": "non-existing"}] |
| contentStreamIdForRemainingPart | "user-cs-new" |

Then I expect exactly 2 events to be published on stream with prefix "Workspace:user-test"
And event at index 1 is of type "WorkspaceWasRebased" with payload:
| Key | Expected |
| workspaceName | "user-test" |
| newContentStreamId | "user-cs-new" |
| previousContentStreamId | "user-cs-identifier" |
Then workspaces user-test has status OUTDATED

And I am in workspace "user-test" and dimension space point {}
Then I expect node aggregate identifier "nody-mc-nodeface" to lead to node user-cs-new;nody-mc-nodeface;{}
And I expect this node to have the following properties:
| Key | Value |
| text | "Original text" |
Then I expect node aggregate identifier "nody-mc-nodeface" to lead to no node

Then I expect the content stream "user-cs-identifier" to not exist
Then I expect the content stream "user-cs-new" to not exist
22 changes: 13 additions & 9 deletions ...alTests/Tests/Behavior/Features/W8-IndividualNodePublication/03-MoreBasicFeatures.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ Feature: Publishing individual nodes (basics)
| nodeAggregateId | "sir-unchanged" |
| nodeTypeName | "Neos.ContentRepository.Testing:Content" |
| parentNodeAggregateId | "lady-eleonode-rootford" |
| initialPropertyValues | {"text": "Initial text"} |
# Create user workspace
And the command CreateWorkspace is executed with payload:
Expand Down Expand Up @@ -285,7 +286,7 @@ Feature: Publishing individual nodes (basics)
| Key | Expected |
| contentStreamId | "user-cs-identifier-remaining" |
Scenario: Partial publish remaining changes are not lost
Scenario: Partial publish keeps remaining changes if nothing matches (and the workspace is outdated)
And the command SetNodeProperties is executed with payload:
| Key | Value |
| workspaceName | "live" |
Expand All @@ -294,20 +295,23 @@ Feature: Publishing individual nodes (basics)
| propertyValues | {"text": "Modified in live workspace"} |
When the command PublishIndividualNodesFromWorkspace is executed with payload:
| Key | Value |
| workspaceName | "user-test" |
| Key | Value |
| workspaceName | "user-test" |
| nodesToPublish | [{"dimensionSpacePoint": {}, "nodeAggregateId": "non-existing"}] |
| contentStreamIdForRemainingPart | "user-cs-new" |
| contentStreamIdForRemainingPart | "user-cs-new" |
Then workspaces user-test has status OUTDATED
Then I expect exactly 1 events to be published on stream with prefix "Workspace:user-test"
And I am in workspace "user-test" and dimension space point {}
Then I expect node aggregate identifier "sir-unchanged" to lead to node user-cs-new;sir-unchanged;{}
Then I expect node aggregate identifier "sir-unchanged" to lead to node user-cs-identifier;sir-unchanged;{}
And I expect this node to have the following properties:
| Key | Value |
| text | "Modified in live workspace" |
| Key | Value |
| text | "Initial text" |
Then I expect node aggregate identifier "sir-david-nodenborough" to lead to node user-cs-new;sir-david-nodenborough;{}
Then I expect node aggregate identifier "sir-david-nodenborough" to lead to node user-cs-identifier;sir-david-nodenborough;{}
And I expect this node to have the following properties:
| Key | Value |
| text | "Modified t1" |
# Then I expect the content stream "user-cs-identifier" to not exist
Then I expect the content stream "user-cs-new" to not exist
36 changes: 8 additions & 28 deletions Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,23 +198,14 @@ private function handlePublishWorkspace(
)
);

if ($workspace->status === WorkspaceStatus::UP_TO_DATE && $rebaseableCommands->isEmpty()) {
// we are up-to-date already and have no changes, we just reopen; partial no-op
if ($rebaseableCommands->isEmpty()) {
// we have no changes, we just reopen; partial no-op
yield $this->reopenContentStream(
$workspace->currentContentStreamId,
ContentStreamStatus::IN_USE_BY_WORKSPACE, // todo will be removed
$commandHandlingDependencies
);
return;
} elseif ($rebaseableCommands->isEmpty()) {
// we have no changes in the workspace, then we will just do a rebase
yield from $this->rebaseWorkspaceWithoutChanges(
$workspace,
$baseWorkspace,
$command->newContentStreamId,
$commandHandlingDependencies
);
return;
}

try {
Expand Down Expand Up @@ -354,7 +345,7 @@ private function handleRebaseWorkspace(
$workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies);
$baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies);
if (!$commandHandlingDependencies->contentStreamExists($workspace->currentContentStreamId)) {
throw new \DomainException('Cannot rebase a workspace with a stateless content stream', 1711718314);
throw new \RuntimeException('Cannot rebase a workspace with a stateless content stream', 1711718314);
}
$currentWorkspaceContentStreamState = $commandHandlingDependencies->getContentStreamStatus($workspace->currentContentStreamId);

Expand Down Expand Up @@ -459,7 +450,7 @@ private function handlePublishIndividualNodesFromWorkspace(

$workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies);
if (!$commandHandlingDependencies->contentStreamExists($workspace->currentContentStreamId)) {
throw new \DomainException('Cannot publish nodes on a workspace with a stateless content stream', 1710410114);
throw new \RuntimeException('Cannot publish nodes on a workspace with a stateless content stream', 1710410114);
}
$currentWorkspaceContentStreamState = $commandHandlingDependencies->getContentStreamStatus($workspace->currentContentStreamId);
$baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies);
Expand All @@ -478,21 +469,10 @@ private function handlePublishIndividualNodesFromWorkspace(
)
);

if ($rebaseableCommands->isEmpty() && $workspace->status === WorkspaceStatus::OUTDATED) {
// we are not up-to-date and don't have any changes, but we want to rebase
yield from $this->rebaseWorkspaceWithoutChanges(
$workspace,
$baseWorkspace,
$command->contentStreamIdForRemainingPart,
$commandHandlingDependencies
);
return;
}

[$matchingCommands, $remainingCommands] = $rebaseableCommands->separateMatchingAndRemainingCommands($command->nodesToPublish);

if ($matchingCommands->isEmpty() && $workspace->status === WorkspaceStatus::UP_TO_DATE) {
// almost a noop (e.g. random node ids were specified and we are up-to-date) ;)
if ($matchingCommands->isEmpty()) {
// almost a noop (e.g. random node ids were specified) ;)
yield $this->reopenContentStream(
$workspace->currentContentStreamId,
$currentWorkspaceContentStreamState,
Expand Down Expand Up @@ -608,7 +588,7 @@ private function handleDiscardIndividualNodesFromWorkspace(

$workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies);
if (!$commandHandlingDependencies->contentStreamExists($workspace->currentContentStreamId)) {
throw new \DomainException('Cannot discard nodes on a workspace with a stateless content stream', 1710408112);
throw new \RuntimeException('Cannot discard nodes on a workspace with a stateless content stream', 1710408112);
}
$currentWorkspaceContentStreamState = $commandHandlingDependencies->getContentStreamStatus($workspace->currentContentStreamId);
$baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies);
Expand Down Expand Up @@ -638,7 +618,7 @@ private function handleDiscardIndividualNodesFromWorkspace(
}

if ($commandsToKeep->isEmpty()) {
// quick path everything was discarded we just branch of from the base
// quick path everything was discarded
yield from $this->discardWorkspace(
$workspace,
$baseWorkspace,
Expand Down

0 comments on commit f0e088e

Please sign in to comment.