jackrabbit-oak-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Francesco Mari (JIRA)" <j...@apache.org>
Subject [jira] [Created] (OAK-7388) MergingNodeStateDiff may recreate nodes that were previously removed to resolve conflicts
Date Wed, 04 Apr 2018 13:12:00 GMT
Francesco Mari created OAK-7388:
-----------------------------------

             Summary: MergingNodeStateDiff may recreate nodes that were previously removed
to resolve conflicts
                 Key: OAK-7388
                 URL: https://issues.apache.org/jira/browse/OAK-7388
             Project: Jackrabbit Oak
          Issue Type: Improvement
          Components: core
            Reporter: Francesco Mari
            Assignee: Francesco Mari
             Fix For: 1.10


{{MergingNodeStateDiff}} might behave incorrectly when the resolution of a conflict involves
the deletion of the conflicting node. I spotted this issue in a use case that can be expressed
by the following code.

{noformat}
NodeState root = EmptyNodeState.EMPTY_NODE;

NodeState withProperty;
{
    NodeBuilder builder = root.builder();
    builder.child("c").setProperty("foo", "bar");
    withProperty = builder.getNodeState();
}

NodeState withUpdatedProperty;
{
    NodeBuilder builder = withProperty.builder();
    builder.child("c").setProperty("foo", "baz");
    withUpdatedProperty = builder.getNodeState();
}

NodeState withRemovedChild;
{
    NodeBuilder builder = withProperty.builder();
    builder.child("c").remove();
    withRemovedChild = builder.getNodeState();
}

NodeBuilder mergedBuilder = withUpdatedProperty.builder();
withRemovedChild.compareAgainstBaseState(withProperty, new ConflictAnnotatingRebaseDiff(mergedBuilder));
NodeState merged = ConflictHook.of(DefaultThreeWayConflictHandler.OURS).processCommit(
	mergedBuilder.getBaseState(), 
	mergedBuilder.getNodeState(), 
	CommitInfo.EMPTY
);
assertFalse(merged.hasChildNode("c"));
{noformat}

The assertion at the end of the code fails becauuse `merged` actually has a child node named
`c`, and `c` is an empty node. After digging into the issue, I figured out that the problem
is caused by the following steps.

# {{MergingNodeStateDiff#childNodeAdded}} is invoked because of {{:conflicts}}. This eventually
results in the deletion of the conflicting child node.
# {{MergingNodeStateDiff#childNodeChanged}} is called because in {{ModifiedNodeState#compareAgainstBaseState}}
the children are compared with the {{!=}} operator instead of using {{Object#equals}}.
# {{org.apache.jackrabbit.oak.spi.state.NodeBuilder#child}} is called in order to setup a
new {{MergingNodeStateDiff}} to descend into the subtree that was detected as modified.
# {{MemoryNodeBuilder#hasChildNode}} correctly returns {{false}}, because the child was removed
in step 1. The return value of {{false}} triggers the next step.
# {{MemoryNodeBuilder#setChildNode(java.lang.String)}} is invoked in order to setup a new,
empty child node.

In other words, the snippet above can be rewritten like the following.

{noformat}
NodeState root = EmptyNodeState.EMPTY_NODE;

NodeState withProperty;
{
    NodeBuilder builder = root.builder();
    builder.child("c").setProperty("foo", "bar");
    withProperty = builder.getNodeState();
}

NodeState withUpdatedProperty;
{
    NodeBuilder builder = withProperty.builder();
    builder.child("c").setProperty("foo", "baz");
    withUpdatedProperty = builder.getNodeState();
}

NodeState withRemovedChild;
{
    NodeBuilder builder = withProperty.builder();
    builder.child("c").remove();
    withRemovedChild = builder.getNodeState();
}

NodeBuilder mergedBuilder = withUpdatedProperty.builder();
// As per MergingNodeStateDiff.childNodeAdded()
mergedBuilder.child("c").remove();
// As per ModifiedNodeState#compareAgainstBaseState()
if (withUpdatedProperty.getChildNode("c") != withRemovedChild.getChildNode("c")) {
    // As per MergingNodeStateDiff.childNodeChanged()
    mergedBuilder.child("c");
}
NodeState merged = mergedBuilder.getNodeState();
assertFalse(merged.hasChildNode("c"));
{noformat}

The end result is that {{MergingNodeStateDiff}} inadvertently adds the node that was removed
in order to resolve a conflict.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message