From 6df5d3397c5c9354409d21a8e207a061f2e6efc2 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 23 Aug 2010 10:13:25 -0700 Subject: [PATCH] Move commit and tag formatting to CommitBuilder, TagBuilder These objects should be responsible for their own formatting, rather than delegating it to some obtuse type called ObjectInserter. While we are at it, simplify the way we insert these into a database. Passing in the type and calling format in application code turned out to be a huge mistake in terms of ease-of-use of the insert API. Change-Id: Id5bb95ee56aa2a002243e9b7853b84ec8df1d7bf Signed-off-by: Shawn O. Pearce --- .../eclipse/jgit/junit/TestRepository.java | 7 +- .../src/org/eclipse/jgit/pgm/Tag.java | 2 +- .../jgit/pgm/debug/RebuildCommitGraph.java | 2 +- .../eclipse/jgit/merge/CherryPickTest.java | 3 +- .../eclipse/jgit/merge/SimpleMergeTest.java | 3 +- .../jgit/revwalk/RevCommitParseTest.java | 2 +- .../eclipse/jgit/revwalk/RevTagParseTest.java | 2 +- .../jgit/storage/file/T0003_Basic.java | 8 +- .../org/eclipse/jgit/api/CommitCommand.java | 3 +- .../org/eclipse/jgit/lib/CommitBuilder.java | 99 ++++++++++++ .../org/eclipse/jgit/lib/ObjectInserter.java | 148 ++++-------------- .../org/eclipse/jgit/lib/ObjectWriter.java | 6 +- .../src/org/eclipse/jgit/lib/TagBuilder.java | 68 ++++++++ 13 files changed, 214 insertions(+), 139 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 6e1f1db39..7a95ccd36 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 @@ -368,7 +368,7 @@ public RevCommit commit(final int secDelta, final RevTree tree, c.setMessage(""); ObjectId id; try { - id = inserter.insert(Constants.OBJ_COMMIT, inserter.format(c)); + id = inserter.insert(c); inserter.flush(); } finally { inserter.release(); @@ -405,7 +405,7 @@ public RevTag tag(final String name, final RevObject dst) throws Exception { t.setMessage(""); ObjectId id; try { - id = inserter.insert(Constants.OBJ_TAG, inserter.format(t)); + id = inserter.insert(t); inserter.flush(); } finally { inserter.release(); @@ -822,8 +822,7 @@ public RevCommit create() throws Exception { ObjectId commitId; try { c.setTreeId(tree.writeTree(inserter)); - commitId = inserter.insert(Constants.OBJ_COMMIT, inserter - .format(c)); + commitId = inserter.insert(c); inserter.flush(); } finally { inserter.release(); diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Tag.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Tag.java index 0ab19b6bb..19454b186 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Tag.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Tag.java @@ -100,7 +100,7 @@ protected void run() throws Exception { tag.setTagger(new PersonIdent(db)); tag.setMessage(message.replaceAll("\r", "")); tag.setTag(shortName); - id = inserter.insert(Constants.OBJ_TAG, inserter.format(tag)); + id = inserter.insert(tag); inserter.flush(); } finally { inserter.release(); diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java index ab4412871..b01734e00 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java @@ -197,7 +197,7 @@ private void recreateCommitGraph() throws IOException { newc.setCommitter(newc.getAuthor()); newc.setParentIds(newParents); newc.setMessage("ORIGINAL " + t.oldId.name() + "\n"); - t.newId = oi.insert(Constants.OBJ_COMMIT,oi.format(newc)); + t.newId = oi.insert(newc); rewrites.put(t.oldId, t.newId); pm.update(1); } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/CherryPickTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/CherryPickTest.java index 876b98636..12fdec2b6 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/CherryPickTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/CherryPickTest.java @@ -45,7 +45,6 @@ package org.eclipse.jgit.merge; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; -import static org.eclipse.jgit.lib.Constants.OBJ_COMMIT; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.dircache.DirCacheBuilder; @@ -137,7 +136,7 @@ private ObjectId commit(final ObjectInserter odi, final DirCache treeB, c.setCommitter(c.getAuthor()); c.setParentIds(parentIds); c.setMessage("Tree " + c.getTreeId().name()); - ObjectId id = odi.insert(OBJ_COMMIT, odi.format(c)); + ObjectId id = odi.insert(c); odi.flush(); return id; } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/SimpleMergeTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/SimpleMergeTest.java index 5e7bcee1e..659c9e3b2 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/SimpleMergeTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/SimpleMergeTest.java @@ -45,7 +45,6 @@ package org.eclipse.jgit.merge; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; -import static org.eclipse.jgit.lib.Constants.OBJ_COMMIT; import java.io.IOException; @@ -373,7 +372,7 @@ private ObjectId commit(final ObjectInserter odi, final DirCache treeB, c.setCommitter(c.getAuthor()); c.setParentIds(parentIds); c.setMessage("Tree " + c.getTreeId().name()); - ObjectId id = odi.insert(OBJ_COMMIT, odi.format(c)); + ObjectId id = odi.insert(c); odi.flush(); return id; } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevCommitParseTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevCommitParseTest.java index 36730c153..7be7dbc81 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevCommitParseTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevCommitParseTest.java @@ -327,7 +327,7 @@ public void testParse_PublicParseMethod() src.setCommitter(committer); src.setMessage("Test commit\n\nThis is a test.\n"); - RevCommit p = RevCommit.parse(fmt.format(src)); + RevCommit p = RevCommit.parse(src.format()); assertEquals(src.getTreeId(), p.getTree()); assertEquals(0, p.getParentCount()); assertEquals(author, p.getAuthorIdent()); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevTagParseTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevTagParseTest.java index 727c48e8e..c694add44 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevTagParseTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevTagParseTest.java @@ -407,7 +407,7 @@ public void testParse_PublicParseMethod() throws CorruptObjectException { src.setTag("a.test"); src.setMessage("Test tag\n\nThis is a test.\n"); - RevTag p = RevTag.parse(fmt.format(src)); + RevTag p = RevTag.parse(src.format()); assertEquals(src.getObjectId(), p.getObject()); assertEquals(committer, p.getTaggerIdent()); assertEquals("a.test", p.getTagName()); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/T0003_Basic.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/T0003_Basic.java index cd802d41e..0352a40bf 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/T0003_Basic.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/T0003_Basic.java @@ -693,13 +693,12 @@ public void test30_stripWorkDir() { } - private ObjectId insertCommit(final CommitBuilder commit) throws IOException, + private ObjectId insertCommit(final CommitBuilder builder) throws IOException, UnsupportedEncodingException { ObjectInserter oi = db.newObjectInserter(); try { - ObjectId id = oi.insert(Constants.OBJ_COMMIT, oi.format(commit)); + ObjectId id = oi.insert(builder); oi.flush(); - commit.setCommitId(id); return id; } finally { oi.release(); @@ -721,9 +720,8 @@ private ObjectId insertTag(final TagBuilder tag) throws IOException, UnsupportedEncodingException { ObjectInserter oi = db.newObjectInserter(); try { - ObjectId id = oi.insert(Constants.OBJ_TAG, oi.format(tag)); + ObjectId id = oi.insert(tag); oi.flush(); - tag.setTagId(id); return id; } finally { oi.release(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java index a88899cc5..e385e7cb8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java @@ -169,8 +169,7 @@ public RevCommit call() throws NoHeadException, NoMessageException, commit.setParentIds(parents); commit.setTreeId(indexTreeId); - ObjectId commitId = odi.insert(Constants.OBJ_COMMIT, odi - .format(commit)); + ObjectId commitId = odi.insert(commit); odi.flush(); RevWalk revWalk = new RevWalk(repo); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitBuilder.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitBuilder.java index 8457d4549..8bd02ee29 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitBuilder.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitBuilder.java @@ -45,6 +45,10 @@ package org.eclipse.jgit.lib; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStreamWriter; +import java.io.UnsupportedEncodingException; import java.nio.charset.Charset; import java.util.List; @@ -62,6 +66,16 @@ public class CommitBuilder { private static final ObjectId[] EMPTY_OBJECTID_LIST = new ObjectId[0]; + private static final byte[] htree = Constants.encodeASCII("tree"); + + private static final byte[] hparent = Constants.encodeASCII("parent"); + + private static final byte[] hauthor = Constants.encodeASCII("author"); + + private static final byte[] hcommitter = Constants.encodeASCII("committer"); + + private static final byte[] hencoding = Constants.encodeASCII("encoding"); + private ObjectId commitId; private ObjectId treeId; @@ -262,6 +276,91 @@ public Charset getEncoding() { return encoding; } + /** + * 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 + * the encoding specified by {@link #getEncoding()} is not + * supported by this Java runtime. + */ + public byte[] format() throws UnsupportedEncodingException { + return format(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[] format(ObjectInserter oi) throws UnsupportedEncodingException { + ByteArrayOutputStream os = new ByteArrayOutputStream(); + OutputStreamWriter w = new OutputStreamWriter(os, getEncoding()); + try { + os.write(htree); + os.write(' '); + getTreeId().copyTo(os); + os.write('\n'); + + for (ObjectId p : getParentIds()) { + os.write(hparent); + os.write(' '); + p.copyTo(os); + os.write('\n'); + } + + os.write(hauthor); + os.write(' '); + w.write(getAuthor().toExternalString()); + w.flush(); + os.write('\n'); + + os.write(hcommitter); + os.write(' '); + w.write(getCommitter().toExternalString()); + w.flush(); + os.write('\n'); + + if (getEncoding() != Constants.CHARSET) { + os.write(hencoding); + os.write(' '); + os.write(Constants.encodeASCII(getEncoding().name())); + os.write('\n'); + } + + os.write('\n'); + + if (getMessage() != null) { + w.write(getMessage()); + w.flush(); + } + } catch (IOException err) { + // This should never occur, the only way to get it above is + // for the ByteArrayOutputStream to throw, but it doesn't. + // + throw new RuntimeException(err); + } + + byte[] content = os.toByteArray(); + setCommitId(oi.idFor(Constants.OBJ_COMMIT, content)); + return content; + } + @Override public String toString() { StringBuilder r = new StringBuilder(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java index 94d792858..102445922 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java @@ -51,9 +51,6 @@ import java.io.EOFException; import java.io.IOException; import java.io.InputStream; -import java.io.OutputStreamWriter; -import java.io.UnsupportedEncodingException; -import java.nio.charset.Charset; import java.security.MessageDigest; import java.text.MessageFormat; @@ -73,16 +70,6 @@ * otherwise making the returned ObjectIds visible to other code. */ public abstract class ObjectInserter { - private static final byte[] htree = Constants.encodeASCII("tree"); - - private static final byte[] hparent = Constants.encodeASCII("parent"); - - private static final byte[] hauthor = Constants.encodeASCII("author"); - - private static final byte[] hcommitter = Constants.encodeASCII("committer"); - - private static final byte[] hencoding = Constants.encodeASCII("encoding"); - /** An inserter that can be used for formatting and id generation only. */ public static class Formatter extends ObjectInserter { @Override @@ -194,6 +181,38 @@ public ObjectId idFor(int objectType, long length, InputStream in) return ObjectId.fromRaw(md.digest()); } + /** + * 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. + * @throws IOException + * the object could not be stored. + */ + public final ObjectId insert(CommitBuilder builder) throws IOException { + return insert(Constants.OBJ_COMMIT, builder.format(this)); + } + + /** + * 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. + * @throws IOException + * the object could not be stored. + */ + public final ObjectId insert(TagBuilder builder) throws IOException { + return insert(Constants.OBJ_TAG, builder.format(this)); + } + /** * Insert a single object into the store, returning its unique name. * @@ -293,107 +312,4 @@ public final byte[] format(Tree tree) throws IOException { } return o.toByteArray(); } - - /** - * Format a Commit in canonical format. - * - * @param commit - * the commit object to format - * @return canonical encoding of the commit object. - * @throws UnsupportedEncodingException - * the commit's chosen encoding isn't supported on this JVM. - */ - public final byte[] format(CommitBuilder commit) - throws UnsupportedEncodingException { - Charset encoding = commit.getEncoding(); - ByteArrayOutputStream os = new ByteArrayOutputStream(); - OutputStreamWriter w = new OutputStreamWriter(os, encoding); - try { - os.write(htree); - os.write(' '); - commit.getTreeId().copyTo(os); - os.write('\n'); - - for (ObjectId p : commit.getParentIds()) { - os.write(hparent); - os.write(' '); - p.copyTo(os); - os.write('\n'); - } - - os.write(hauthor); - os.write(' '); - w.write(commit.getAuthor().toExternalString()); - w.flush(); - os.write('\n'); - - os.write(hcommitter); - os.write(' '); - w.write(commit.getCommitter().toExternalString()); - w.flush(); - os.write('\n'); - - if (encoding != Constants.CHARSET) { - os.write(hencoding); - os.write(' '); - os.write(Constants.encodeASCII(encoding.name())); - os.write('\n'); - } - - os.write('\n'); - - if (commit.getMessage() != null) { - w.write(commit.getMessage()); - w.flush(); - } - } catch (IOException err) { - // This should never occur, the only way to get it above is - // for the ByteArrayOutputStream to throw, but it doesn't. - // - throw new RuntimeException(err); - } - return os.toByteArray(); - } - - /** - * Format a Tag in canonical format. - * - * @param tag - * the tag object to format - * @return canonical encoding of the tag object. - */ - public final byte[] format(TagBuilder tag) { - ByteArrayOutputStream os = new ByteArrayOutputStream(); - OutputStreamWriter w = new OutputStreamWriter(os, Constants.CHARSET); - try { - w.write("object "); - tag.getObjectId().copyTo(w); - w.write('\n'); - - w.write("type "); - w.write(Constants.typeString(tag.getObjectType())); - w.write("\n"); - - w.write("tag "); - w.write(tag.getTag()); - w.write("\n"); - - if (tag.getTagger() != null) { - w.write("tagger "); - w.write(tag.getTagger().toExternalString()); - w.write('\n'); - } - - w.write('\n'); - if (tag.getMessage() != null) - w.write(tag.getMessage()); - w.close(); - } catch (IOException err) { - // This should never occur, the only way to get it above is - // for the ByteArrayOutputStream to throw, but it doesn't. - // - throw new RuntimeException(err); - } - return os.toByteArray(); - } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectWriter.java index 5cb4b2168..339986be0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectWriter.java @@ -46,8 +46,6 @@ package org.eclipse.jgit.lib; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; -import static org.eclipse.jgit.lib.Constants.OBJ_COMMIT; -import static org.eclipse.jgit.lib.Constants.OBJ_TAG; import static org.eclipse.jgit.lib.Constants.OBJ_TREE; import java.io.File; @@ -174,7 +172,7 @@ public ObjectId writeCanonicalTree(byte[] treeData) throws IOException { */ public ObjectId writeCommit(CommitBuilder commit) throws IOException { try { - ObjectId id = inserter.insert(OBJ_COMMIT, inserter.format(commit)); + ObjectId id = inserter.insert(commit); inserter.flush(); return id; } finally { @@ -192,7 +190,7 @@ public ObjectId writeCommit(CommitBuilder commit) throws IOException { */ public ObjectId writeTag(TagBuilder tag) throws IOException { try { - ObjectId id = inserter.insert(OBJ_TAG, inserter.format(tag)); + ObjectId id = inserter.insert(tag); inserter.flush(); return id; } finally { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/TagBuilder.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/TagBuilder.java index 6cd0e9e88..71e46494c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/TagBuilder.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/TagBuilder.java @@ -45,6 +45,10 @@ package org.eclipse.jgit.lib; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStreamWriter; + import org.eclipse.jgit.revwalk.RevObject; /** @@ -169,6 +173,70 @@ public void setMessage(final String 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[] format() { + return format(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[] format(ObjectInserter oi) { + ByteArrayOutputStream os = new ByteArrayOutputStream(); + OutputStreamWriter w = new OutputStreamWriter(os, Constants.CHARSET); + try { + w.write("object "); + getObjectId().copyTo(w); + w.write('\n'); + + w.write("type "); + w.write(Constants.typeString(getObjectType())); + w.write("\n"); + + w.write("tag "); + w.write(getTag()); + w.write("\n"); + + if (getTagger() != null) { + w.write("tagger "); + w.write(getTagger().toExternalString()); + w.write('\n'); + } + + w.write('\n'); + if (getMessage() != null) + w.write(getMessage()); + w.close(); + } catch (IOException err) { + // This should never occur, the only way to get it above is + // for the ByteArrayOutputStream to throw, but it doesn't. + // + throw new RuntimeException(err); + } + + byte[] content = os.toByteArray(); + setTagId(oi.idFor(Constants.OBJ_TAG, content)); + return content; + } + @Override public String toString() { StringBuilder r = new StringBuilder();