Merge "Return parsed objects from TestRepository.commit/tree/blob()"

5 files changed, 129 insertions, 122 deletions

diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java
index 29a4100193..49715e5596 100644
--- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java
+++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java
@@ -276,7 +276,7 @@ public class TestRepository<R extends Repository> {
 	 *
 	 * @param content
 	 *            binary file content.
-	 * @return reference to the blob.
+	 * @return the new, fully parsed blob.
 	 * @throws Exception
 	 */
 	public RevBlob blob(byte[] content) throws Exception {
@@ -285,7 +285,7 @@ public class TestRepository<R extends Repository> {
 			id = ins.insert(Constants.OBJ_BLOB, content);
 			ins.flush();
 		}
-		return pool.lookupBlob(id);
+		return (RevBlob) pool.parseAny(id);
 	}
 
 	/**
@@ -312,21 +312,22 @@ public class TestRepository<R extends Repository> {
 	 * @param entries
 	 *            the files to include in the tree. The collection does not need
 	 *            to be sorted properly and may be empty.
-	 * @return reference to the tree specified by the entry list.
+	 * @return the new, fully parsed tree specified by the entry list.
 	 * @throws Exception
 	 */
 	public RevTree tree(DirCacheEntry... entries) throws Exception {
 		final DirCache dc = DirCache.newInCore();
 		final DirCacheBuilder b = dc.builder();
-		for (DirCacheEntry e : entries)
+		for (DirCacheEntry e : entries) {
 			b.add(e);
+		}
 		b.finish();
 		ObjectId root;
 		try (ObjectInserter ins = inserter) {
 			root = dc.writeTree(ins);
 			ins.flush();
 		}
-		return pool.lookupTree(root);
+		return pool.parseTree(root);
 	}
 
 	/**
@@ -422,7 +423,7 @@ public class TestRepository<R extends Repository> {
 	 *            the root tree for the commit.
 	 * @param parents
 	 *            zero or more parents of the commit.
-	 * @return the new commit.
+	 * @return the new, fully parsed commit.
 	 * @throws Exception
 	 */
 	public RevCommit commit(final int secDelta, final RevTree tree,
@@ -442,7 +443,7 @@ public class TestRepository<R extends Repository> {
 			id = ins.insert(c);
 			ins.flush();
 		}
-		return pool.lookupCommit(id);
+		return pool.parseCommit(id);
 	}
 
 	/**
@@ -467,7 +468,7 @@ public class TestRepository<R extends Repository> {
 	 *            with {@code refs/tags/}.
 	 * @param dst
 	 *            object the tag should be pointed at.
-	 * @return the annotated tag object.
+	 * @return the new, fully parsed annotated tag object.
 	 * @throws Exception
 	 */
 	public RevTag tag(String name, RevObject dst) throws Exception {
@@ -481,7 +482,7 @@ public class TestRepository<R extends Repository> {
 			id = ins.insert(t);
 			ins.flush();
 		}
-		return (RevTag) pool.lookupAny(id, Constants.OBJ_TAG);
+		return pool.parseTag(id);
 	}
 
 	/**
@@ -705,8 +706,8 @@ public class TestRepository<R extends Repository> {
 	 *
 	 * @param id
 	 *            commit-ish to cherry-pick.
-	 * @return newly created commit, or null if no work was done due to the
-	 *         resulting tree being identical.
+	 * @return the new, fully parsed commit, or null if no work was done due to
+	 *         the resulting tree being identical.
 	 * @throws Exception
 	 */
 	public RevCommit cherryPick(AnyObjectId id) throws Exception {
@@ -1197,7 +1198,7 @@ public class TestRepository<R extends Repository> {
 					commitId = ins.insert(c);
 					ins.flush();
 				}
-				self = pool.lookupCommit(commitId);
+				self = pool.parseCommit(commitId);
 
 				if (branch != null)
 					branch.update(self);
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java
index c50e88f97c..ffb6f4ee77 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java
@@ -360,7 +360,6 @@ public class GcCommitSelectionTest extends GcTestCase {
 			}
 		}
 		RevCommit c = commit.create();
-		tr.parseBody(c);
 		commits.add(c);
 		return c;
 	}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java
index 8e389ae252..4969305de0 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java
@@ -144,13 +144,13 @@ public class RevObjectTest extends RevWalkTestCase {
 		final RevCommit a = commit();
 		final RevFlag flag1 = rw.newFlag("flag1");
 		final RevFlag flag2 = rw.newFlag("flag2");
-		assertEquals(0, a.flags);
+		assertEquals(RevWalk.PARSED, a.flags);
 
 		a.add(flag1);
-		assertEquals(flag1.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask, a.flags);
 
 		a.add(flag2);
-		assertEquals(flag1.mask | flag2.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask | flag2.mask, a.flags);
 	}
 
 	@Test
@@ -162,10 +162,10 @@ public class RevObjectTest extends RevWalkTestCase {
 		s.add(flag1);
 		s.add(flag2);
 
-		assertEquals(0, a.flags);
+		assertEquals(RevWalk.PARSED, a.flags);
 
 		a.add(s);
-		assertEquals(flag1.mask | flag2.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask | flag2.mask, a.flags);
 	}
 
 	@Test
@@ -175,9 +175,9 @@ public class RevObjectTest extends RevWalkTestCase {
 		final RevFlag flag2 = rw.newFlag("flag2");
 		a.add(flag1);
 		a.add(flag2);
-		assertEquals(flag1.mask | flag2.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask | flag2.mask, a.flags);
 		a.remove(flag2);
-		assertEquals(flag1.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask, a.flags);
 	}
 
 	@Test
@@ -191,8 +191,8 @@ public class RevObjectTest extends RevWalkTestCase {
 		s.add(flag2);
 		a.add(flag3);
 		a.add(s);
-		assertEquals(flag1.mask | flag2.mask | flag3.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask | flag2.mask | flag3.mask, a.flags);
 		a.remove(s);
-		assertEquals(flag3.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag3.mask, a.flags);
 	}
 }
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCullTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCullTest.java
index fb52828c5b..7984a37193 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCullTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCullTest.java
@@ -85,7 +85,7 @@ public class RevWalkCullTest extends RevWalkTestCase {
 
 	@Test
 	public void testProperlyCullAllAncestors_LongHistory() throws Exception {
-		final RevCommit a = commit();
+		RevCommit a = commit();
 		RevCommit b = commit(a);
 		for (int i = 0; i < 24; i++) {
 			b = commit(b);
@@ -94,6 +94,12 @@ public class RevWalkCullTest extends RevWalkTestCase {
 		}
 		final RevCommit c = commit(b);
 
+		// TestRepository eagerly parses newly created objects. The current rw
+		// is caching that parsed state. To verify that RevWalk itself is lazy,
+		// set up a new one.
+		rw.close();
+		rw = createRevWalk();
+		RevCommit a2 = rw.lookupCommit(a);
 		markStart(c);
 		markUninteresting(b);
 		assertCommit(c, rw.next());
@@ -102,6 +108,6 @@ public class RevWalkCullTest extends RevWalkTestCase {
 		// We should have aborted before we got back so far that "a"
 		// would be parsed. Thus, its parents shouldn't be allocated.
 		//
-		assertNull(a.parents);
+		assertNull(a2.parents);
 	}
 }
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkShallowTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkShallowTest.java
index 37243e1fa7..7554d7a479 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkShallowTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkShallowTest.java
@@ -58,130 +58,140 @@ public class RevWalkShallowTest extends RevWalkTestCase {
 
 	@Test
 	public void testDepth1() throws Exception {
-		final RevCommit a = commit();
-		final RevCommit b = commit(a);
-		final RevCommit c = commit(b);
-		final RevCommit d = commit(c);
+		RevCommit[] commits = setupLinearChain();
 
-		createShallowFile(d);
+		createShallowFile(commits[3]);
+		updateCommits(commits);
 
-		rw.reset();
-		markStart(d);
-		assertCommit(d, rw.next());
+		rw.markStart(commits[3]);
+		assertCommit(commits[3], rw.next());
 		assertNull(rw.next());
 	}
 
 	@Test
 	public void testDepth2() throws Exception {
-		final RevCommit a = commit();
-		final RevCommit b = commit(a);
-		final RevCommit c = commit(b);
-		final RevCommit d = commit(c);
+		RevCommit[] commits = setupLinearChain();
 
-		createShallowFile(c);
+		createShallowFile(commits[2]);
+		updateCommits(commits);
 
-		rw.reset();
-		markStart(d);
-		assertCommit(d, rw.next());
-		assertCommit(c, rw.next());
+		rw.markStart(commits[3]);
+		assertCommit(commits[3], rw.next());
+		assertCommit(commits[2], rw.next());
 		assertNull(rw.next());
 	}
 
 	@Test
 	public void testDepth3() throws Exception {
-		final RevCommit a = commit();
-		final RevCommit b = commit(a);
-		final RevCommit c = commit(b);
-		final RevCommit d = commit(c);
+		RevCommit[] commits = setupLinearChain();
 
-		createShallowFile(b);
+		createShallowFile(commits[1]);
+		updateCommits(commits);
 
-		rw.reset();
-		markStart(d);
-		assertCommit(d, rw.next());
-		assertCommit(c, rw.next());
-		assertCommit(b, rw.next());
+		rw.markStart(commits[3]);
+		assertCommit(commits[3], rw.next());
+		assertCommit(commits[2], rw.next());
+		assertCommit(commits[1], rw.next());
 		assertNull(rw.next());
 	}
 
 	@Test
-	public void testMergeCommitOneParentShallow() throws Exception {
-		final RevCommit a = commit();
-		final RevCommit b = commit(a);
-		final RevCommit c = commit(b);
-		final RevCommit d = commit(b);
-		final RevCommit e = commit(d);
-		final RevCommit merge = commit(c, e);
-
-		createShallowFile(e);
-
-		rw.reset();
-		markStart(merge);
-		assertCommit(merge, rw.next());
-		assertCommit(e, rw.next());
-		assertCommit(c, rw.next());
-		assertCommit(b, rw.next());
-		assertCommit(a, rw.next());
+	public void testObjectDirectorySnapshot() throws Exception {
+		RevCommit[] commits = setupLinearChain();
+
+		createShallowFile(commits[3]);
+		updateCommits(commits);
+
+		markStart(commits[3]);
+		assertCommit(commits[3], rw.next());
+		assertNull(rw.next());
+
+		createShallowFile(commits[2]);
+		updateCommits(commits);
+
+		markStart(commits[3]);
+		assertCommit(commits[3], rw.next());
+		assertCommit(commits[2], rw.next());
 		assertNull(rw.next());
 	}
 
+	private RevCommit[] setupLinearChain() throws Exception {
+		RevCommit[] commits = new RevCommit[4];
+		RevCommit parent = null;
+		for (int i = 0; i < commits.length; i++) {
+			commits[i] = parent != null ? commit(parent) : commit();
+			parent = commits[i];
+		}
+		return commits;
+	}
+
 	@Test
-	public void testMergeCommitEntirelyShallow() throws Exception {
-		final RevCommit a = commit();
-		final RevCommit b = commit(a);
-		final RevCommit c = commit(b);
-		final RevCommit d = commit(b);
-		final RevCommit e = commit(d);
-		final RevCommit merge = commit(c, e);
-
-		createShallowFile(c, e);
-
-		rw.reset();
-		markStart(merge);
-		assertCommit(merge, rw.next());
-		assertCommit(e, rw.next());
-		assertCommit(c, rw.next());
+	public void testMergeCommitOneParentShallow() throws Exception {
+		RevCommit[] commits = setupMergeChain();
+
+		createShallowFile(commits[4]);
+		updateCommits(commits);
+
+		markStart(commits[5]);
+		assertCommit(commits[5], rw.next());
+		assertCommit(commits[4], rw.next());
+		assertCommit(commits[2], rw.next());
+		assertCommit(commits[1], rw.next());
+		assertCommit(commits[0], rw.next());
 		assertNull(rw.next());
 	}
 
 	@Test
-	public void testObjectDirectorySnapshot() throws Exception {
-		RevCommit a = commit();
-		RevCommit b = commit(a);
-		RevCommit c = commit(b);
-		RevCommit d = commit(c);
+	public void testMergeCommitEntirelyShallow() throws Exception {
+		RevCommit[] commits = setupMergeChain();
 
-		createShallowFile(d);
+		createShallowFile(commits[2], commits[4]);
+		updateCommits(commits);
 
-		rw.reset();
-		markStart(d);
-		assertCommit(d, rw.next());
+		markStart(commits[5]);
+		assertCommit(commits[5], rw.next());
+		assertCommit(commits[4], rw.next());
+		assertCommit(commits[2], rw.next());
 		assertNull(rw.next());
+	}
 
-		rw = createRevWalk();
-		a = rw.lookupCommit(a);
-		b = rw.lookupCommit(b);
-		c = rw.lookupCommit(c);
-		d = rw.lookupCommit(d);
-
-		rw.reset();
-		markStart(d);
-		assertCommit(d, rw.next());
-		assertNull(rw.next());
+	private RevCommit[] setupMergeChain() throws Exception {
+		/*-
+		 * Create a history like this, diverging at 1 and merging at 5:
+		 *
+		 *      ---o--o       commits 3,4
+		 *     /       \
+		 * o--o--o------o   commits 0,1,2,5
+		 */
+		RevCommit[] commits = new RevCommit[6];
+		commits[0] = commit();
+		commits[1] = commit(commits[0]);
+		commits[2] = commit(commits[1]);
+		commits[3] = commit(commits[1]);
+		commits[4] = commit(commits[3]);
+		commits[5] = commit(commits[2], commits[4]);
+		return commits;
+	}
 
-		createShallowFile(c);
+	private void updateCommits(RevCommit[] commits) {
+		// Relookup commits using the new RevWalk
+		for (int i = 0; i < commits.length; i++) {
+			commits[i] = rw.lookupCommit(commits[i].getId());
+		}
+	}
 
+	private void createShallowFile(ObjectId... shallowCommits)
+			throws IOException {
+		// Reset the RevWalk since the new shallow file invalidates the existing
+		// RevWalk's shallow state.
+		rw.close();
 		rw = createRevWalk();
-		a = rw.lookupCommit(a);
-		b = rw.lookupCommit(b);
-		c = rw.lookupCommit(c);
-		d = rw.lookupCommit(d);
-
-		rw.reset();
-		markStart(d);
-		assertCommit(d, rw.next());
-		assertCommit(c, rw.next());
-		assertNull(rw.next());
+		StringBuilder builder = new StringBuilder();
+		for (ObjectId commit : shallowCommits) {
+			builder.append(commit.getName() + "\n");
+		}
+		JGitTestUtil.write(new File(db.getDirectory(), "shallow"),
+				builder.toString());
 	}
 
 	@Test
@@ -199,13 +209,4 @@ public class RevWalkShallowTest extends RevWalkTestCase {
 		assertCommit(b, rw.next());
 		assertNull(rw.next());
 	}
-
-	private void createShallowFile(ObjectId... shallowCommits)
-			throws IOException {
-		final StringBuilder builder = new StringBuilder();
-		for (ObjectId commit : shallowCommits)
-			builder.append(commit.getName() + "\n");
-		JGitTestUtil.write(new File(db.getDirectory(), "shallow"),
-				builder.toString());
-	}
 }