Author: mreutegg
Date: Tue Jan 19 15:02:57 2016
New Revision: 1725542
URL: http://svn.apache.org/viewvc?rev=1725542&view=rev
Log:
OAK-3897: Branch reset does not revert all changes
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ResetDiff.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentMKResetTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java?rev=1725542&r1=1725541&r2=1725542&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java
Tue Jan 19 15:02:57 2016
@@ -335,7 +335,7 @@ public class DocumentMK {
throw new DocumentStoreException("Not a branch revision: " +
ancestorRevisionId);
}
try {
- return nodeStore.reset(branch, ancestor, null).toString();
+ return nodeStore.reset(branch, ancestor).toString();
} catch (DocumentStoreException e) {
throw new DocumentStoreException(e);
}
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1725542&r1=1725541&r2=1725542&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
Tue Jan 19 15:02:57 2016
@@ -22,6 +22,8 @@ import static com.google.common.base.Pre
import static com.google.common.collect.Iterables.filter;
import static com.google.common.collect.Iterables.toArray;
import static com.google.common.collect.Iterables.transform;
+import static com.google.common.collect.Lists.newArrayList;
+import static com.google.common.collect.Lists.reverse;
import static java.util.Collections.singletonList;
import static org.apache.jackrabbit.oak.commons.PathUtils.concat;
import static org.apache.jackrabbit.oak.plugins.document.Collection.JOURNAL;
@@ -81,6 +83,7 @@ import org.apache.jackrabbit.oak.commons
import org.apache.jackrabbit.oak.plugins.blob.BlobStoreBlob;
import org.apache.jackrabbit.oak.plugins.blob.MarkSweepGarbageCollector;
import org.apache.jackrabbit.oak.plugins.blob.ReferencedBlob;
+import org.apache.jackrabbit.oak.plugins.document.Branch.BranchCommit;
import org.apache.jackrabbit.oak.plugins.document.cache.CacheInvalidationStats;
import
org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCache;
import
org.apache.jackrabbit.oak.plugins.document.persistentCache.broadcast.DynamicBroadcastConfig;
@@ -1249,8 +1252,7 @@ public final class DocumentNodeStore
@Nonnull
RevisionVector reset(@Nonnull RevisionVector branchHead,
- @Nonnull RevisionVector ancestor,
- @Nullable DocumentNodeStoreBranch branch) {
+ @Nonnull RevisionVector ancestor) {
checkNotNull(branchHead);
checkNotNull(ancestor);
Branch b = getBranches().getBranch(branchHead);
@@ -1261,61 +1263,44 @@ public final class DocumentNodeStore
throw new DocumentStoreException(branchHead + " is not the head " +
"of a branch");
}
- if (!b.containsCommit(ancestor.getBranchRevision())) {
+ if (!b.containsCommit(ancestor.getBranchRevision())
+ &&
!b.getBase().asBranchRevision(getClusterId()).equals(ancestor)) {
throw new DocumentStoreException(ancestor + " is not " +
"an ancestor revision of " + branchHead);
}
- if (branchHead.equals(ancestor)) {
+ // tailSet is inclusive -> use an ancestorRev with a
+ // counter incremented by one to make the call exclusive
+ Revision ancestorRev = ancestor.getBranchRevision();
+ ancestorRev = new Revision(ancestorRev.getTimestamp(),
+ ancestorRev.getCounter() + 1, ancestorRev.getClusterId(),
true);
+ List<Revision> revs =
newArrayList(b.getCommits().tailSet(ancestorRev));
+ if (revs.isEmpty()) {
// trivial
return branchHead;
}
- boolean success = false;
- Commit commit = newCommit(branchHead, branch);
- try {
- Iterator<Revision> it =
b.getCommits().tailSet(ancestor.getBranchRevision()).iterator();
- // first revision is the ancestor (tailSet is inclusive)
- // do not undo changes for this revision
- it.next();
- Map<String, UpdateOp> operations = Maps.newHashMap();
- if (it.hasNext()) {
- Revision reset = it.next();
- // TODO: correct?
- getRoot(b.getCommit(reset).getBase().update(reset))
- .compareAgainstBaseState(getRoot(ancestor),
- new ResetDiff(reset.asTrunkRevision(),
operations));
- UpdateOp rootOp = operations.get("/");
- if (rootOp == null) {
- rootOp = new UpdateOp(Utils.getIdFromPath("/"), false);
- NodeDocument.setModified(rootOp, commit.getRevision());
- operations.put("/", rootOp);
- }
- NodeDocument.removeCollision(rootOp, reset.asTrunkRevision());
- NodeDocument.removeRevision(rootOp, reset.asTrunkRevision());
- }
- // update root document first
- if (store.findAndUpdate(Collection.NODES, operations.get("/")) !=
null) {
- // clean up in-memory branch data
- // first revision is the ancestor (tailSet is inclusive)
- List<Revision> revs =
Lists.newArrayList(b.getCommits().tailSet(ancestor.getBranchRevision()));
- for (Revision r : revs.subList(1, revs.size())) {
- b.removeCommit(r);
- }
- // successfully updating the root document can be considered
- // as success because the changes are not marked as committed
- // anymore
- success = true;
+ UpdateOp rootOp = new UpdateOp(Utils.getIdFromPath("/"), false);
+ // reset each branch commit in reverse order
+ Map<String, UpdateOp> operations = Maps.newHashMap();
+ for (Revision r : reverse(revs)) {
+ NodeDocument.removeCollision(rootOp, r.asTrunkRevision());
+ NodeDocument.removeRevision(rootOp, r.asTrunkRevision());
+ operations.clear();
+ BranchCommit bc = b.getCommit(r);
+ if (bc.isRebase()) {
+ continue;
}
- operations.remove("/");
- // update remaining documents
+ getRoot(bc.getBase().update(r))
+ .compareAgainstBaseState(getRoot(bc.getBase()),
+ new ResetDiff(r.asTrunkRevision(), operations));
+ // apply reset operations
for (UpdateOp op : operations.values()) {
store.findAndUpdate(Collection.NODES, op);
}
- } finally {
- if (!success) {
- canceled(commit);
- } else {
- done(commit, true, null);
- }
+ }
+ store.findAndUpdate(Collection.NODES, rootOp);
+ // clean up in-memory branch data
+ for (Revision r : revs) {
+ b.removeCommit(r);
}
return ancestor;
}
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java?rev=1725542&r1=1725541&r2=1725542&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
Tue Jan 19 15:02:57 2016
@@ -620,8 +620,7 @@ class DocumentNodeStoreBranch implements
try {
head = store.getRoot(
store.reset(branchHead.getRevision(),
- ancestor.getRevision(),
- DocumentNodeStoreBranch.this));
+ ancestor.getRevision()));
} catch (Exception e) {
CommitFailedException ex = new CommitFailedException(
OAK, 100, "Branch reset failed", e);
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ResetDiff.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ResetDiff.java?rev=1725542&r1=1725541&r2=1725542&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ResetDiff.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ResetDiff.java
Tue Jan 19 15:02:57 2016
@@ -74,6 +74,7 @@ class ResetDiff implements NodeStateDiff
@Override
public boolean childNodeAdded(String name, NodeState after) {
+ NodeDocument.removeCommitRoot(getUpdateOp(), revision);
String p = PathUtils.concat(path, name);
ResetDiff diff = new ResetDiff(revision, p, operations);
UpdateOp op = diff.getUpdateOp();
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentMKResetTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentMKResetTest.java?rev=1725542&r1=1725541&r2=1725542&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentMKResetTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentMKResetTest.java
Tue Jan 19 15:02:57 2016
@@ -18,7 +18,6 @@ package org.apache.jackrabbit.oak.plugin
import java.util.Map;
-import org.junit.Ignore;
import org.junit.Test;
import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
@@ -131,7 +130,6 @@ public class DocumentMKResetTest extends
assertPropExists(rev, "/foo", "p2");
}
- @Ignore
@Test
public void resetToBaseOfBranch() {
addNodes(null, "/foo");
@@ -142,7 +140,6 @@ public class DocumentMKResetTest extends
assertNodesNotExist(null, "/foo/bar");
}
- @Ignore
@Test
public void resetRemovesCommitRootOnParent() {
DocumentStore store = mk.getDocumentStore();
@@ -167,7 +164,6 @@ public class DocumentMKResetTest extends
Revision.fromString(b1).asTrunkRevision()));
}
- @Ignore
@Test
public void resetMultipleCommits() {
DocumentStore store = mk.getDocumentStore();
