git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>
Cc: git@vger.kernel.org
Subject: [JGIT PATCH 3/5] Unify RevWalk parsing code to be more consistent across types
Date: Fri, 12 Jun 2009 16:00:17 -0700	[thread overview]
Message-ID: <1244847619-7364-4-git-send-email-spearce@spearce.org> (raw)
In-Reply-To: <1244847619-7364-3-git-send-email-spearce@spearce.org>

The parse() method now verifies the object actually exists, and
not just that we have a handle to it (e.g. in the case of a tree
or a blob type).  This allowed us to push a chunk of the loading
code up into the RevObject base class, removing some duplication.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/revwalk/RevBlob.java      |    5 ----
 .../src/org/spearce/jgit/revwalk/RevCommit.java    |   11 +--------
 .../src/org/spearce/jgit/revwalk/RevObject.java    |   23 +++++++++++++++++--
 .../src/org/spearce/jgit/revwalk/RevTag.java       |    9 +-------
 .../src/org/spearce/jgit/revwalk/RevTree.java      |    5 ----
 .../src/org/spearce/jgit/revwalk/RevWalk.java      |   14 ++---------
 6 files changed, 25 insertions(+), 42 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java
index cf241cf..70eeac0 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevBlob.java
@@ -53,11 +53,6 @@ protected RevBlob(final AnyObjectId id) {
 	}
 
 	@Override
-	void parse(final RevWalk walk) {
-		flags |= PARSED;
-	}
-	
-	@Override
 	public final int getType() {
 		return Constants.OBJ_BLOB;
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
index 2a59ec4..679718e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
@@ -46,7 +46,6 @@
 import org.spearce.jgit.lib.Commit;
 import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.MutableObjectId;
-import org.spearce.jgit.lib.ObjectLoader;
 import org.spearce.jgit.lib.PersonIdent;
 import org.spearce.jgit.util.RawParseUtils;
 
@@ -54,8 +53,6 @@
 public class RevCommit extends RevObject {
 	static final RevCommit[] NO_PARENTS = {};
 
-	private static final String TYPE_COMMIT = Constants.TYPE_COMMIT;
-
 	private RevTree tree;
 
 	RevCommit[] parents;
@@ -79,13 +76,7 @@ protected RevCommit(final AnyObjectId id) {
 	@Override
 	void parse(final RevWalk walk) throws MissingObjectException,
 			IncorrectObjectTypeException, IOException {
-		final ObjectLoader ldr = walk.db.openObject(walk.curs, this);
-		if (ldr == null)
-			throw new MissingObjectException(this, TYPE_COMMIT);
-		final byte[] data = ldr.getCachedBytes();
-		if (Constants.OBJ_COMMIT != ldr.getType())
-			throw new IncorrectObjectTypeException(this, TYPE_COMMIT);
-		parseCanonical(walk, data);
+		parseCanonical(walk, loadCanonical(walk));
 	}
 
 	void parseCanonical(final RevWalk walk, final byte[] raw) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
index 1a13d0a..0e0386c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
@@ -39,11 +39,13 @@
 
 import java.io.IOException;
 
+import org.spearce.jgit.errors.CorruptObjectException;
 import org.spearce.jgit.errors.IncorrectObjectTypeException;
 import org.spearce.jgit.errors.MissingObjectException;
 import org.spearce.jgit.lib.AnyObjectId;
 import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.ObjectLoader;
 
 /** Base object type accessed during revision walking. */
 public abstract class RevObject extends ObjectId {
@@ -55,9 +57,24 @@ RevObject(final AnyObjectId name) {
 		super(name);
 	}
 
-	abstract void parse(RevWalk walk) throws MissingObjectException,
-			IncorrectObjectTypeException, IOException;
-	
+	void parse(final RevWalk walk) throws MissingObjectException,
+			IncorrectObjectTypeException, IOException {
+		loadCanonical(walk);
+		flags |= PARSED;
+	}
+
+	final byte[] loadCanonical(final RevWalk walk) throws IOException,
+			MissingObjectException, IncorrectObjectTypeException,
+			CorruptObjectException {
+		final ObjectLoader ldr = walk.db.openObject(walk.curs, this);
+		if (ldr == null)
+			throw new MissingObjectException(this, getType());
+		final byte[] data = ldr.getCachedBytes();
+		if (getType() != ldr.getType())
+			throw new IncorrectObjectTypeException(this, getType());
+		return data;
+	}
+
 	/**
 	 * Get Git object type. See {@link Constants}.
 	 * 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
index 83fd873..e876025 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
@@ -45,7 +45,6 @@
 import org.spearce.jgit.errors.MissingObjectException;
 import org.spearce.jgit.lib.AnyObjectId;
 import org.spearce.jgit.lib.Constants;
-import org.spearce.jgit.lib.ObjectLoader;
 import org.spearce.jgit.lib.PersonIdent;
 import org.spearce.jgit.lib.Tag;
 import org.spearce.jgit.util.MutableInteger;
@@ -72,13 +71,7 @@ protected RevTag(final AnyObjectId id) {
 	@Override
 	void parse(final RevWalk walk) throws MissingObjectException,
 			IncorrectObjectTypeException, IOException {
-		final ObjectLoader ldr = walk.db.openObject(walk.curs, this);
-		if (ldr == null)
-			throw new MissingObjectException(this, Constants.TYPE_TAG);
-		final byte[] data = ldr.getCachedBytes();
-		if (Constants.OBJ_TAG != ldr.getType())
-			throw new IncorrectObjectTypeException(this, Constants.TYPE_TAG);
-		parseCanonical(walk, data);
+		parseCanonical(walk, loadCanonical(walk));
 	}
 
 	void parseCanonical(final RevWalk walk, final byte[] rawTag)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java
index 4d767e4..e9f5d91 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTree.java
@@ -53,11 +53,6 @@ protected RevTree(final AnyObjectId id) {
 	}
 
 	@Override
-	void parse(final RevWalk walk) {
-		flags |= PARSED;
-	}
-	
-	@Override
 	public final int getType() {
 		return Constants.OBJ_TREE;
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
index f567a33..40bdb4e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
@@ -667,15 +667,7 @@ else if (!(c instanceof RevTree))
 					Constants.TYPE_TREE);
 		else
 			t = (RevTree) c;
-
-		if ((t.flags & PARSED) != 0)
-			return t;
-		final ObjectLoader ldr = db.openObject(curs, t);
-		if (ldr == null)
-			throw new MissingObjectException(t, Constants.TYPE_TREE);
-		if (ldr.getType() != Constants.OBJ_TREE)
-			throw new IncorrectObjectTypeException(t, Constants.TYPE_TREE);
-		t.flags |= PARSED;
+		parse(t);
 		return t;
 	}
 
@@ -731,8 +723,8 @@ public RevObject parseAny(final AnyObjectId id)
 				throw new IllegalArgumentException("Bad object type: " + type);
 			}
 			objects.add(r);
-		} else if ((r.flags & PARSED) == 0)
-			r.parse(this);
+		} else
+			parse(r);
 		return r;
 	}
 
-- 
1.6.3.2.367.gf0de

  reply	other threads:[~2009-06-12 23:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-12 23:00 [JGIT PATCH 0/5] Fix major performance problems in UploadPack Shawn O. Pearce
2009-06-12 23:00 ` [JGIT PATCH 1/5] Make RevTag getObject(), getName() final to prevent overrides Shawn O. Pearce
2009-06-12 23:00   ` [JGIT PATCH 2/5] Allow exceptions to be created with integer type codes Shawn O. Pearce
2009-06-12 23:00     ` Shawn O. Pearce [this message]
2009-06-12 23:00       ` [JGIT PATCH 4/5] Change RevObject dispose() semantics to avoid reparses Shawn O. Pearce
2009-06-12 23:00         ` [JGIT PATCH 5/5] UploadPack: Only recompute okToGiveUp() if bases changed Shawn O. Pearce

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1244847619-7364-4-git-send-email-spearce@spearce.org \
    --to=spearce@spearce.org \
    --cc=git@vger.kernel.org \
    --cc=robin.rosenberg@dewire.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).