Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java (revision 1692059) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java (working copy) @@ -448,10 +448,14 @@ DocumentStore store = nodeStore.getDocumentStore(); for (UpdateOp op : changed) { UpdateOp reverse = op.getReverseOperation(); + if (op.isNew()) { + NodeDocument.setDeletedOnce(reverse); + } store.findAndUpdate(NODES, reverse); } for (UpdateOp op : newDocuments) { UpdateOp reverse = op.getReverseOperation(); + NodeDocument.setDeletedOnce(reverse); store.findAndUpdate(NODES, reverse); } UpdateOp removeCollision = new UpdateOp(commitRoot.getId(), false); Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (revision 1692059) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (working copy) @@ -1310,8 +1310,7 @@ public static void setRevision(@Nonnull UpdateOp op, @Nonnull Revision revision, @Nonnull String commitValue) { - checkNotNull(op).setMapEntry(REVISIONS, - checkNotNull(revision), checkNotNull(commitValue)); + checkNotNull(op).setMapEntry(REVISIONS, checkNotNull(revision), checkNotNull(commitValue)); } public static void unsetRevision(@Nonnull UpdateOp op, @@ -1338,8 +1337,7 @@ public static void addCollision(@Nonnull UpdateOp op, @Nonnull Revision revision) { - checkNotNull(op).setMapEntry(COLLISIONS, checkNotNull(revision), - String.valueOf(true)); + checkNotNull(op).setMapEntry(COLLISIONS, checkNotNull(revision), String.valueOf(true)); } public static void removeCollision(@Nonnull UpdateOp op, @@ -1349,9 +1347,7 @@ public static void setLastRev(@Nonnull UpdateOp op, @Nonnull Revision revision) { - checkNotNull(op).setMapEntry(LAST_REV, - new Revision(0, 0, revision.getClusterId()), - revision.toString()); + checkNotNull(op).setMapEntry(LAST_REV, new Revision(0, 0, revision.getClusterId()), revision.toString()); } public static void setCommitRoot(@Nonnull UpdateOp op, @@ -1372,12 +1368,15 @@ if(deleted) { //DELETED_ONCE would be set upon every delete. //possibly we can avoid that - checkNotNull(op).set(DELETED_ONCE, Boolean.TRUE); + setDeletedOnce(op); } - checkNotNull(op).setMapEntry(DELETED, checkNotNull(revision), - String.valueOf(deleted)); + checkNotNull(op).setMapEntry(DELETED, checkNotNull(revision), String.valueOf(deleted)); } + public static void setDeletedOnce(@Nonnull UpdateOp op) { + checkNotNull(op).set(DELETED_ONCE, Boolean.TRUE); + } + public static void removeDeleted(@Nonnull UpdateOp op, @Nonnull Revision revision) { checkNotNull(op).removeMapEntry(DELETED, revision); Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java (revision 1692059) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java (working copy) @@ -242,15 +242,27 @@ final DocumentMK mk = new DocumentMK.Builder() .setDocumentStore(docStore).setAsyncDelay(0).open(); final DocumentNodeStore store = mk.getNodeStore(); - final String head = mk.commit("/", "+\"foo\":{}+\"bar\":{}", null, null); + + NodeBuilder builder = store.getRoot().builder(); + builder.child("deletedNode"); + builder.child("updateNode").setProperty("foo", "bar"); + merge(store, builder); + + builder = store.getRoot().builder(); + builder.child("deletedNode").remove(); + merge(store, builder); + + final Revision head = store.getHeadRevision(); + Thread writer = new Thread(new Runnable() { @Override public void run() { try { Revision r = store.newRevision(); - Commit c = new Commit(store, r, Revision.fromString(head), null); - c.addNode(new DocumentNodeState(store, "/foo/node", r)); - c.addNode(new DocumentNodeState(store, "/bar/node", r)); + Commit c = new Commit(store, r, head, null); + c.addNode(new DocumentNodeState(store, "/newConflictingNode", r)); + c.addNode(new DocumentNodeState(store, "/deletedNode", r)); + c.updateProperty("/updateNode", "foo", "baz"); c.apply(); } catch (DocumentStoreException e) { exceptions.add(e); @@ -265,9 +277,9 @@ created.acquireUninterruptibly(); // commit will succeed and add collision marker to writer commit Revision r = store.newRevision(); - Commit c = new Commit(store, r, Revision.fromString(head), null); - c.addNode(new DocumentNodeState(store, "/foo/node", r)); - c.addNode(new DocumentNodeState(store, "/bar/node", r)); + Commit c = new Commit(store, r, head, null); + c.addNode(new DocumentNodeState(store, "/newConflictingNode", r)); + c.addNode(new DocumentNodeState(store, "/newNonConflictingNode", r)); c.apply(); // allow writer to continue s.release(); @@ -274,13 +286,27 @@ writer.join(); assertEquals("expected exception", 1, exceptions.size()); - String id = Utils.getIdFromPath("/foo/node"); + String id = Utils.getIdFromPath("/newConflictingNode"); NodeDocument doc = docStore.find(NODES, id); assertNotNull("document with id " + id + " does not exist", doc); - id = Utils.getIdFromPath("/bar/node"); + assertTrue("document with id " + id + " should get _deletedOnce marked due to rollback", + doc.wasDeletedOnce()); + + id = Utils.getIdFromPath("/newNonConflictingNode"); doc = docStore.find(NODES, id); - assertNotNull("document with id " + id + " does not exist", doc); + assertNull("document with id " + id + " must not have _deletedOnce", + doc.get(NodeDocument.DELETED_ONCE)); + id = Utils.getIdFromPath("/deletedNode"); + doc = docStore.find(NODES, id); + assertTrue("document with id " + id + " should get _deletedOnce marked due to rollback", + doc.wasDeletedOnce()); + + id = Utils.getIdFromPath("/updateNode"); + doc = docStore.find(NODES, id); + assertNull("document with id " + id + " must not have _deletedOnce despite rollback", + doc.get(NodeDocument.DELETED_ONCE)); + mk.dispose(); }