Remove result id from CommitBuilder, TagBuilder

These objects don't need to be updated with the resulting ObjectId of
the formatted content, callers can get that from the ObjectInserter on
their own.

Change-Id: Idc5f097de9f7beafc5e54e597383d82daf9d7db4
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Reviewed-by: Chris Aniszczyk <caniszczyk@gmail.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReflogConfigTest.java

@@ -110,8 +110,9 @@ public class ReflogConfigTest extends RepositoryTestCase {
 		commit.setMessage(commitMsg);
 		commit.setTreeId(t.getTreeId());
 		ObjectInserter inserter = db.newObjectInserter();
+		ObjectId id;
 		try {
-			inserter.insert(commit);
+			id = inserter.insert(commit);
 			inserter.flush();
 		} finally {
 			inserter.release();
@@ -119,7 +120,7 @@ public class ReflogConfigTest extends RepositoryTestCase {
 
 		int nl = commitMsg.indexOf('\n');
 		final RefUpdate ru = db.updateRef(Constants.HEAD);
-		ru.setNewObjectId(commit.getCommitId());
+		ru.setNewObjectId(id);
 		ru.setRefLogMessage("commit : "
 				+ ((nl == -1) ? commitMsg : commitMsg.substring(0, nl)), false);
 		ru.forceUpdate();

org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/T0003_Basic.java

@@ -377,11 +377,11 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
 		c.setTreeId(t.getTreeId());
 		assertEquals(t.getTreeId(), c.getTreeId());
 
-		insertCommit(c);
+		ObjectId actid = insertCommit(c);
 
 		final ObjectId cmtid = ObjectId.fromString(
 				"803aec4aba175e8ab1d666873c984c0308179099");
-		assertEquals(cmtid, c.getCommitId());
+		assertEquals(cmtid, actid);
 
 		// Verify the commit we just wrote is in the correct format.
 		ObjectDatabase odb = db.getObjectDatabase();
@@ -397,7 +397,7 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
 		}
 
 		// Verify we can read it.
-		RevCommit c2 = parseCommit(c.getCommitId());
+		RevCommit c2 = parseCommit(actid);
 		assertNotNull(c2);
 		assertEquals(c.getMessage(), c2.getFullMessage());
 		assertEquals(c.getTreeId(), c2.getTree());
@@ -432,10 +432,10 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
 		t.setTag("test020");
 		t.setTagger(new PersonIdent(author, 1154236443000L, -4 * 60));
 		t.setMessage("test020 tagged\n");
-		insertTag(t);
-		assertEquals("6759556b09fbb4fd8ae5e315134481cc25d46954", t.getTagId().name());
+		ObjectId actid = insertTag(t);
+		assertEquals("6759556b09fbb4fd8ae5e315134481cc25d46954", actid.name());
 
-		RevTag mapTag = parseTag(t.getTagId());
+		RevTag mapTag = parseTag(actid);
 		assertEquals(Constants.OBJ_BLOB, mapTag.getObject().getType());
 		assertEquals("test020 tagged\n", mapTag.getFullMessage());
 		assertEquals(new PersonIdent(author, 1154236443000L, -4 * 60), mapTag.getTaggerIdent());
@@ -452,10 +452,10 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
 		t.setTag("test021");
 		t.setTagger(new PersonIdent(author, 1154236443000L, -4 * 60));
 		t.setMessage("test021 tagged\n");
-		insertTag(t);
-		assertEquals("b0517bc8dbe2096b419d42424cd7030733f4abe5", t.getTagId().name());
+		ObjectId actid = insertTag(t);
+		assertEquals("b0517bc8dbe2096b419d42424cd7030733f4abe5", actid.name());
 
-		RevTag mapTag = parseTag(t.getTagId());
+		RevTag mapTag = parseTag(actid);
 		assertEquals(Constants.OBJ_TREE, mapTag.getObject().getType());
 		assertEquals("test021 tagged\n", mapTag.getFullMessage());
 		assertEquals(new PersonIdent(author, 1154236443000L, -4 * 60), mapTag.getTaggerIdent());
@@ -478,10 +478,10 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
 		t.setTag("test022");
 		t.setTagger(new PersonIdent(author, 1154236443000L, -4 * 60));
 		t.setMessage("test022 tagged\n");
-		insertTag(t);
-		assertEquals("0ce2ebdb36076ef0b38adbe077a07d43b43e3807", t.getTagId().name());
+		ObjectId actid = insertTag(t);
+		assertEquals("0ce2ebdb36076ef0b38adbe077a07d43b43e3807", actid.name());
 
-		RevTag mapTag = parseTag(t.getTagId());
+		RevTag mapTag = parseTag(actid);
 		assertEquals(Constants.OBJ_COMMIT, mapTag.getObject().getType());
 		assertEquals("test022 tagged\n", mapTag.getFullMessage());
 		assertEquals(new PersonIdent(author, 1154236443000L, -4 * 60), mapTag.getTaggerIdent());
@@ -543,10 +543,10 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
 		c1.setMessage("A Commit\n");
 		c1.setTreeId(t.getTreeId());
 		assertEquals(t.getTreeId(), c1.getTreeId());
-		insertCommit(c1);
+		ObjectId actid1 = insertCommit(c1);
 		final ObjectId cmtid1 = ObjectId.fromString(
 				"803aec4aba175e8ab1d666873c984c0308179099");
-		assertEquals(cmtid1, c1.getCommitId());
+		assertEquals(cmtid1, actid1);
 
 		final CommitBuilder c2 = new CommitBuilder();
 		c2.setAuthor(new PersonIdent(author, 1154236443000L, -4 * 60));
@@ -554,20 +554,20 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
 		c2.setMessage("A Commit 2\n");
 		c2.setTreeId(t.getTreeId());
 		assertEquals(t.getTreeId(), c2.getTreeId());
-		c2.setParentIds(c1.getCommitId());
-		insertCommit(c2);
+		c2.setParentIds(actid1);
+		ObjectId actid2 = insertCommit(c2);
 		final ObjectId cmtid2 = ObjectId.fromString(
 				"95d068687c91c5c044fb8c77c5154d5247901553");
-		assertEquals(cmtid2, c2.getCommitId());
+		assertEquals(cmtid2, actid2);
 
 		RevCommit rm2 = parseCommit(cmtid2);
 		assertNotSame(c2, rm2); // assert the parsed objects is not from the cache
 		assertEquals(c2.getAuthor(), rm2.getAuthorIdent());
-		assertEquals(c2.getCommitId(), rm2.getId());
+		assertEquals(actid2, rm2.getId());
 		assertEquals(c2.getMessage(), rm2.getFullMessage());
 		assertEquals(c2.getTreeId(), rm2.getTree().getId());
 		assertEquals(1, rm2.getParentCount());
-		assertEquals(c1.getCommitId(), rm2.getParent(0));
+		assertEquals(actid1, rm2.getParent(0));
 
 		final CommitBuilder c3 = new CommitBuilder();
 		c3.setAuthor(new PersonIdent(author, 1154236443000L, -4 * 60));
@@ -575,21 +575,21 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
 		c3.setMessage("A Commit 3\n");
 		c3.setTreeId(t.getTreeId());
 		assertEquals(t.getTreeId(), c3.getTreeId());
-		c3.setParentIds(c1.getCommitId(), c2.getCommitId());
-		insertCommit(c3);
+		c3.setParentIds(actid1, actid2);
+		ObjectId actid3 = insertCommit(c3);
 		final ObjectId cmtid3 = ObjectId.fromString(
 				"ce6e1ce48fbeeb15a83f628dc8dc2debefa066f4");
-		assertEquals(cmtid3, c3.getCommitId());
+		assertEquals(cmtid3, actid3);
 
 		RevCommit rm3 = parseCommit(cmtid3);
 		assertNotSame(c3, rm3); // assert the parsed objects is not from the cache
 		assertEquals(c3.getAuthor(), rm3.getAuthorIdent());
-		assertEquals(c3.getCommitId(), rm3.getId());
+		assertEquals(actid3, rm3.getId());
 		assertEquals(c3.getMessage(), rm3.getFullMessage());
 		assertEquals(c3.getTreeId(), rm3.getTree().getId());
 		assertEquals(2, rm3.getParentCount());
-		assertEquals(c1.getCommitId(), rm3.getParent(0));
-		assertEquals(c2.getCommitId(), rm3.getParent(1));
+		assertEquals(actid1, rm3.getParent(0));
+		assertEquals(actid2, rm3.getParent(1));
 
 		final CommitBuilder c4 = new CommitBuilder();
 		c4.setAuthor(new PersonIdent(author, 1154236443000L, -4 * 60));
@@ -597,22 +597,22 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
 		c4.setMessage("A Commit 4\n");
 		c4.setTreeId(t.getTreeId());
 		assertEquals(t.getTreeId(), c3.getTreeId());
-		c4.setParentIds(c1.getCommitId(), c2.getCommitId(), c3.getCommitId());
-		insertCommit(c4);
+		c4.setParentIds(actid1, actid2, actid3);
+		ObjectId actid4 = insertCommit(c4);
 		final ObjectId cmtid4 = ObjectId.fromString(
 				"d1fca9fe3fef54e5212eb67902c8ed3e79736e27");
-		assertEquals(cmtid4, c4.getCommitId());
+		assertEquals(cmtid4, actid4);
 
 		RevCommit rm4 = parseCommit(cmtid4);
 		assertNotSame(c4, rm3); // assert the parsed objects is not from the cache
 		assertEquals(c4.getAuthor(), rm4.getAuthorIdent());
-		assertEquals(c4.getCommitId(), rm4.getId());
+		assertEquals(actid4, rm4.getId());
 		assertEquals(c4.getMessage(), rm4.getFullMessage());
 		assertEquals(c4.getTreeId(), rm4.getTree().getId());
 		assertEquals(3, rm4.getParentCount());
-		assertEquals(c1.getCommitId(), rm4.getParent(0));
-		assertEquals(c2.getCommitId(), rm4.getParent(1));
-		assertEquals(c3.getCommitId(), rm4.getParent(2));
+		assertEquals(actid1, rm4.getParent(0));
+		assertEquals(actid2, rm4.getParent(1));
+		assertEquals(actid3, rm4.getParent(2));
 	}
 
 	public void test027_UnpackedRefHigherPriorityThanPacked() throws IOException {

org.eclipse.jgit/src/org/eclipse/jgit/api/TagCommand.java

@@ -140,7 +140,7 @@ public class TagCommand extends GitCommand<RevTag> {
 
 				RevWalk revWalk = new RevWalk(repo);
 				try {
-					RevTag revTag = revWalk.parseTag(newTag.getTagId());
+					RevTag revTag = revWalk.parseTag(tagId);
 					String refName = Constants.R_TAGS + newTag.getTag();
 					RefUpdate tagRef = repo.updateRef(refName);
 					tagRef.setNewObjectId(tagId);

org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitBuilder.java

@@ -76,8 +76,6 @@ public class CommitBuilder {
 
 	private static final byte[] hencoding = Constants.encodeASCII("encoding");
 
-	private ObjectId commitId;
-
 	private ObjectId treeId;
 
 	private ObjectId[] parentIds;
@@ -96,21 +94,6 @@ public class CommitBuilder {
 		encoding = Constants.CHARSET;
 	}
 
-	/** @return this commit's object id. */
-	public ObjectId getCommitId() {
-		return commitId;
-	}
-
-	/**
-	 * Set the id of this commit object.
-	 *
-	 * @param id
-	 *            the id that we calculated for this object.
-	 */
-	public void setCommitId(final ObjectId id) {
-		commitId = id;
-	}
-
 	/** @return id of the root tree listing this commit's snapshot. */
 	public ObjectId getTreeId() {
 		return treeId;
@@ -124,7 +107,6 @@ public class CommitBuilder {
 	 */
 	public void setTreeId(AnyObjectId id) {
 		treeId = id.copy();
-		commitId = null;
 	}
 
 	/** @return the author of this commit (who wrote it). */
@@ -140,7 +122,6 @@ public class CommitBuilder {
 	 */
 	public void setAuthor(PersonIdent newAuthor) {
 		author = newAuthor;
-		commitId = null;
 	}
 
 	/** @return the committer and commit time for this object. */
@@ -156,7 +137,6 @@ public class CommitBuilder {
 	 */
 	public void setCommitter(PersonIdent newCommitter) {
 		committer = newCommitter;
-		commitId = null;
 	}
 
 	/** @return the ancestors of this commit. Never null. */
@@ -172,7 +152,6 @@ public class CommitBuilder {
 	 */
 	public void setParentId(AnyObjectId newParent) {
 		parentIds = new ObjectId[] { newParent.copy() };
-		commitId = null;
 	}
 
 	/**
@@ -188,7 +167,6 @@ public class CommitBuilder {
 	 */
 	public void setParentIds(AnyObjectId parent1, AnyObjectId parent2) {
 		parentIds = new ObjectId[] { parent1.copy(), parent2.copy() };
-		commitId = null;
 	}
 
 	/**
@@ -201,7 +179,6 @@ public class CommitBuilder {
 		parentIds = new ObjectId[newParents.length];
 		for (int i = 0; i < newParents.length; i++)
 			parentIds[i] = newParents[i].copy();
-		commitId = null;
 	}
 
 	/**
@@ -214,7 +191,6 @@ public class CommitBuilder {
 		parentIds = new ObjectId[newParents.size()];
 		for (int i = 0; i < newParents.size(); i++)
 			parentIds[i] = newParents.get(i).copy();
-		commitId = null;
 	}
 
 	/**
@@ -232,7 +208,6 @@ public class CommitBuilder {
 				newParents[i] = parentIds[i];
 			newParents[parentIds.length] = additionalParent.copy();
 			parentIds = newParents;
-			commitId = null;
 		}
 	}
 
@@ -279,9 +254,6 @@ public class CommitBuilder {
 	/**
 	 * Format this builder's state as a commit object.
 	 *
-	 * As a side effect, {@link #getCommitId()} will be populated with the
-	 * proper ObjectId for the formatted content.
-	 *
 	 * @return this object in the canonical commit format, suitable for storage
 	 *         in a repository.
 	 * @throws UnsupportedEncodingException
@@ -289,26 +261,6 @@ public class CommitBuilder {
 	 *             supported by this Java runtime.
 	 */
 	public byte[] build() throws UnsupportedEncodingException {
-		return build(new ObjectInserter.Formatter());
-	}
-
-	/**
-	 * Format this builder's state as a commit object.
-	 *
-	 * As a side effect, {@link #getCommitId()} will be populated with the
-	 * proper ObjectId for the formatted content.
-	 *
-	 * @param oi
-	 *            the inserter whose formatting support will be reused. The
-	 *            inserter itself is not affected, and the commit is not
-	 *            actually inserted into the repository.
-	 * @return this object in the canonical commit format, suitable for storage
-	 *         in a repository.
-	 * @throws UnsupportedEncodingException
-	 *             the encoding specified by {@link #getEncoding()} is not
-	 *             supported by this Java runtime.
-	 */
-	public byte[] build(ObjectInserter oi) throws UnsupportedEncodingException {
 		ByteArrayOutputStream os = new ByteArrayOutputStream();
 		OutputStreamWriter w = new OutputStreamWriter(os, getEncoding());
 		try {
@@ -355,18 +307,13 @@ public class CommitBuilder {
 			//
 			throw new RuntimeException(err);
 		}
-
-		byte[] content = os.toByteArray();
-		setCommitId(oi.idFor(Constants.OBJ_COMMIT, content));
-		return content;
+		return os.toByteArray();
 	}
 
 	@Override
 	public String toString() {
 		StringBuilder r = new StringBuilder();
 		r.append("Commit");
-		if (commitId != null)
-			r.append("[" + commitId.name() + "]");
 		r.append("={\n");
 
 		r.append("tree ");

org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java

@@ -179,9 +179,6 @@ public abstract class ObjectInserter {
 	/**
 	 * Insert a single commit into the store, returning its unique name.
 	 *
-	 * As a side effect, {@link CommitBuilder#getCommitId()} will also be
-	 * populated with the returned ObjectId.
-	 *
 	 * @param builder
 	 *            the builder containing the proposed commit's data.
 	 * @return the name of the commit object.
@@ -189,15 +186,12 @@ public abstract class ObjectInserter {
 	 *             the object could not be stored.
 	 */
 	public final ObjectId insert(CommitBuilder builder) throws IOException {
-		return insert(Constants.OBJ_COMMIT, builder.build(this));
+		return insert(Constants.OBJ_COMMIT, builder.build());
 	}
 
 	/**
 	 * Insert a single annotated tag into the store, returning its unique name.
 	 *
-	 * As a side effect, {@link TagBuilder#getTagId()} will also be populated
-	 * with the returned ObjectId.
-	 *
 	 * @param builder
 	 *            the builder containing the proposed tag's data.
 	 * @return the name of the tag object.
@@ -205,7 +199,7 @@ public abstract class ObjectInserter {
 	 *             the object could not be stored.
 	 */
 	public final ObjectId insert(TagBuilder builder) throws IOException {
-		return insert(Constants.OBJ_TAG, builder.build(this));
+		return insert(Constants.OBJ_TAG, builder.build());
 	}
 
 	/**

org.eclipse.jgit/src/org/eclipse/jgit/lib/TagBuilder.java

@@ -62,8 +62,6 @@ import org.eclipse.jgit.revwalk.RevObject;
  * {@link org.eclipse.jgit.revwalk.RevWalk#parseTag(AnyObjectId)}.
  */
 public class TagBuilder {
-	private ObjectId tagId;
-
 	private ObjectId object;
 
 	private int type = Constants.OBJ_BAD;
@@ -74,21 +72,6 @@ public class TagBuilder {
 
 	private String message;
 
-	/** @return this tag's object id. */
-	public ObjectId getTagId() {
-		return tagId;
-	}
-
-	/**
-	 * Set the id of this tag object.
-	 *
-	 * @param id
-	 *            the id that we calculated for this object.
-	 */
-	public void setTagId(ObjectId id) {
-		tagId = id;
-	}
-
 	/** @return the type of object this tag refers to. */
 	public int getObjectType() {
 		return type;
@@ -110,7 +93,6 @@ public class TagBuilder {
 	public void setObjectId(AnyObjectId obj, int objType) {
 		object = obj.copy();
 		type = objType;
-		tagId = null;
 	}
 
 	/**
@@ -138,7 +120,6 @@ public class TagBuilder {
 	 */
 	public void setTag(String shortName) {
 		this.tag = shortName;
-		tagId = null;
 	}
 
 	/** @return creator of this tag. May be null. */
@@ -154,7 +135,6 @@ public class TagBuilder {
 	 */
 	public void setTagger(PersonIdent taggerIdent) {
 		tagger = taggerIdent;
-		tagId = null;
 	}
 
 	/** @return the complete commit message. */
@@ -170,36 +150,15 @@ public class TagBuilder {
 	 */
 	public void setMessage(final String newMessage) {
 		message = newMessage;
-		tagId = null;
 	}
 
 	/**
 	 * Format this builder's state as an annotated tag object.
 	 *
-	 * As a side effect, {@link #getTagId()} will be populated with the proper
-	 * ObjectId for the formatted content.
-	 *
 	 * @return this object in the canonical annotated tag format, suitable for
 	 *         storage in a repository.
 	 */
 	public byte[] build() {
-		return build(new ObjectInserter.Formatter());
-	}
-
-	/**
-	 * Format this builder's state as an annotated tag object.
-	 *
-	 * As a side effect, {@link #getTagId()} will be populated with the proper
-	 * ObjectId for the formatted content.
-	 *
-	 * @param oi
-	 *            the inserter whose formatting support will be reused. The
-	 *            inserter itself is not affected, and the annotated tag is not
-	 *            actually inserted into the repository.
-	 * @return this object in the canonical annotated tag format, suitable for
-	 *         storage in a repository.
-	 */
-	public byte[] build(ObjectInserter oi) {
 		ByteArrayOutputStream os = new ByteArrayOutputStream();
 		OutputStreamWriter w = new OutputStreamWriter(os, Constants.CHARSET);
 		try {
@@ -231,18 +190,13 @@ public class TagBuilder {
 			//
 			throw new RuntimeException(err);
 		}
-
-		byte[] content = os.toByteArray();
-		setTagId(oi.idFor(Constants.OBJ_TAG, content));
-		return content;
+		return os.toByteArray();
 	}
 
 	@Override
 	public String toString() {
 		StringBuilder r = new StringBuilder();
 		r.append("Tag");
-		if (tagId != null)
-			r.append("[" + tagId.name() + "]");
 		r.append("={\n");
 
 		r.append("object ");