Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/ConflictValidator.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/ConflictValidator.java (revision 1693875) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/ConflictValidator.java (working copy) @@ -17,6 +17,7 @@ package org.apache.jackrabbit.oak.plugins.commit; import static org.apache.jackrabbit.oak.api.Type.STRINGS; +import static org.apache.jackrabbit.oak.commons.PathUtils.concat; import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.MIX_REP_MERGE_CONFLICT; import org.apache.jackrabbit.JcrConstants; @@ -24,7 +25,6 @@ import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.api.Type; -import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants; import org.apache.jackrabbit.oak.spi.commit.DefaultValidator; import org.apache.jackrabbit.oak.spi.commit.Validator; @@ -60,23 +60,11 @@ } ConflictValidator() { - if (log.isDebugEnabled()) { - this.path = "/"; - } else { - this.path = null; - } + this.path = "/"; } private ConflictValidator(String path, String name) { - if (path != null) { - this.path = PathUtils.concat(path, name); - } else { - this.path = null; - } - } - - private boolean isLogEnabled() { - return path != null; + this.path = concat(path, name); } @Override @@ -130,7 +118,7 @@ //Conflict details are not made part of ExceptionMessage instead they are //logged. This to avoid exposing property details to the caller as it might not have //permission to access it - if (isLogEnabled()) { + if (log.isDebugEnabled()) { log.debug(getConflictMessage(), ex); } throw ex; Index: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ConflictResolutionTest.java =================================================================== --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ConflictResolutionTest.java (revision 1693875) +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ConflictResolutionTest.java (working copy) @@ -23,7 +23,6 @@ import static org.junit.Assert.fail; import static org.junit.matchers.JUnitMatchers.containsString; -import java.util.List; import java.util.Set; import javax.jcr.InvalidItemStateException; @@ -32,13 +31,12 @@ import org.apache.jackrabbit.oak.commons.junit.LogCustomizer; import org.junit.After; -import org.junit.Before; import org.junit.Test; -import com.google.common.collect.Sets; - import ch.qos.logback.classic.Level; +import com.google.common.collect.Sets; + public class ConflictResolutionTest extends AbstractRepositoryTest { // TODO add tests for all ConflictType types to observe generated logs @@ -56,50 +54,33 @@ super(fixture); } - @Before - public void setup() throws RepositoryException { - logMergingNodeStateDiff.starting(); - logConflictValidator.starting(); - } - @After public void after() { super.logout(); - logMergingNodeStateDiff.finished(); - logConflictValidator.finished(); } @Test public void deleteChangedNode() throws RepositoryException { - getAdminSession().getRootNode().addNode("node").addNode("jcr:content") - .addNode("metadata"); - getAdminSession().save(); + // first INFO level + deleteChangedNodeOps("node0"); - Session session1 = createAdminSession(); - Session session2 = createAdminSession(); + // DEBUG level + Set mnsdLogs; + Set cvLogs; try { - session1.getNode("/node/jcr:content").remove(); - session2.getNode("/node/jcr:content/metadata").setProperty( - "updated", "myself"); - session2.save(); - try { - session1.save(); - fail("Expected InvalidItemStateException"); - } catch (InvalidItemStateException expected) { - assertThat( - "Expecting 'Unresolved conflicts in /node'", - expected.getMessage(), - containsString("OakState0001: Unresolved conflicts in /node")); - } + logMergingNodeStateDiff.starting(); + logConflictValidator.starting(); + deleteChangedNodeOps("node1"); } finally { - session1.logout(); - session2.logout(); + mnsdLogs = Sets.newHashSet(logMergingNodeStateDiff.getLogs()); + cvLogs = Sets.newHashSet(logConflictValidator.getLogs()); + logMergingNodeStateDiff.finished(); + logConflictValidator.finished(); } // MergingNodeStateDif debug: NodeConflictHandler // resolved conflict of type DELETE_CHANGED_NODE with resolution THEIRS // on node jcr:content, conflict trace ^"/metadata/updated":"myself" - Set mnsdLogs = Sets.newHashSet(logMergingNodeStateDiff.getLogs()); assertTrue(mnsdLogs.size() == 1); assertThat( "MergingNodeStateDiff log message must contain a reference to the handler", @@ -115,12 +96,38 @@ containsString("^\"/metadata/updated\":\"myself\"]")); // ConflictValidator debug: Commit failed due to unresolved - // conflicts in /node = {deleteChangedNode = {jcr:content}} - Set cvLogs = Sets.newHashSet(logConflictValidator.getLogs()); + // conflicts in /node1 = {deleteChangedNode = {jcr:content}} assertTrue(cvLogs.size() == 1); assertThat( "ConflictValidator log message must contain a reference to the path", cvLogs.toString(), - containsString("/node = {deleteChangedNode = {jcr:content}}")); + containsString("/node1 = {deleteChangedNode = {jcr:content}}")); + } + + private void deleteChangedNodeOps(String node) throws RepositoryException { + getAdminSession().getRootNode().addNode(node).addNode("jcr:content") + .addNode("metadata"); + getAdminSession().save(); + + Session session1 = createAdminSession(); + Session session2 = createAdminSession(); + try { + session1.getNode("/"+node+"/jcr:content").remove(); + session2.getNode("/"+node+"/jcr:content/metadata").setProperty( + "updated", "myself"); + session2.save(); + try { + session1.save(); + fail("Expected InvalidItemStateException"); + } catch (InvalidItemStateException expected) { + assertThat( + "Expecting 'Unresolved conflicts in /"+node+"'", + expected.getMessage(), + containsString("OakState0001: Unresolved conflicts in /"+node+"")); + } + } finally { + session1.logout(); + session2.logout(); + } } }