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.

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

Discarding nodes tries to discard node from base workspace #4577

Closed
Sebobo opened this issue Oct 5, 2023 · 7 comments
Closed

Discarding nodes tries to discard node from base workspace #4577

Sebobo opened this issue Oct 5, 2023 · 7 comments

Comments

@Sebobo
Copy link
Member

Sebobo commented Oct 5, 2023

I'm currently working on a bug for a customer in which they get errors when trying to discard/delete several workspaces.
Neos runs into an exception as nodes from "live" cannot be discarded as it has no base workspace.

So the PublishingService->doDiscardNode collects child nodes of the nodes in the desired workspace, but nodeDataRepository->findByParentAndNodeType returns a mixed list of child nodes from "live" and the target workspace.
See

$affectedChildNodeDataInSameWorkspace = $this->nodeDataRepository->findByParentAndNodeType($parentBasePath, null, $node->getWorkspace(), null, false, true);

I currently don't see exactly where findByParentAndNodeType makes sure that only matching nodes are returned.

So IMO the correct solution must be to add the following check before trying to discard the found child nodes:

if (!$affectedChildNodeData->matchesWorkspaceAndDimensions($node->getWorkspace(), $node->getDimensions())) {
    continue;
}

The bug only appears since they upgraded to 8.3, but it might affect 7.3 too and something else had changed.

@dlubitz
Copy link
Contributor

dlubitz commented Oct 6, 2023

Is there a way to reproduce that behavior?

@Sebobo
Copy link
Member Author

Sebobo commented Oct 6, 2023

So far I was only able to reproduce this in my customers project but not in the demo yet as it got a bit late yesterday.
Will try to do that today or Monday to fully understand the history of the issue.

@Sebobo
Copy link
Member Author

Sebobo commented Oct 6, 2023

But it must be something like this:

You have the workspace chain "live" -> "A" -> "B"

You modify a content element in A which has children and also modify one of it's children.
Then you modify the same element in B but not the children.

Then try to discard the changes from B.
Now I expect the changed child from A to appear in the nodes to discard which would discard it, as a base workspace exists.

Still thinking about the Problem with only live + A and the thrown exception that live has no base workspace.

@dlubitz
Copy link
Contributor

dlubitz commented Oct 6, 2023

Based on the place in the code, it needs to be some moved node, right?

@Sebobo
Copy link
Member Author

Sebobo commented Oct 6, 2023

Modified should be enough, why do you think moved?
It's definitely not added or removed from what I saw in the database.

@dlubitz
Copy link
Contributor

dlubitz commented Oct 6, 2023

It's all located in the iteration of the result of findOneByMovedTo .

$possibleShadowNodeData = $this->nodeDataRepository->findOneByMovedTo($node->getNodeData());
if ($possibleShadowNodeData instanceof NodeData) {
if ($possibleShadowNodeData->getMovedTo() !== null) {
$parentBasePath = $node->getPath();
$affectedChildNodeDataInSameWorkspace = $this->nodeDataRepository->findByParentAndNodeType($parentBasePath, null, $node->getWorkspace(), null, false, true);
foreach ($affectedChildNodeDataInSameWorkspace as $affectedChildNodeData) {
if (!$affectedChildNodeData->matchesWorkspaceAndDimensions($node->getWorkspace(), $node->getDimensions())) {
continue;
}
/** @var NodeData $affectedChildNodeData */
$affectedChildNode = $this->nodeFactory->createFromNodeData($affectedChildNodeData, $node->getContext());
$this->doDiscardNode($affectedChildNode, $alreadyDiscardedNodeIdentifiers);
}
}
$this->nodeDataRepository->remove($possibleShadowNodeData);
}

@Sebobo
Copy link
Member Author

Sebobo commented Oct 6, 2023

Ah yes 🤦
Could explain, why it wasn't noticed more often.

Sebobo added a commit that referenced this issue Oct 6, 2023
If a node is moved in a sub workspace it’s possible that childnodes from the base workspace are returned and discarded. If the parent workspace is live, this will lead to an exception in the workspace module, if it’s another shared workspace it might even discard nodes from it.

Resolves: #4577
@Sebobo Sebobo added the 7.3 label Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants