From 5cfc29b491eece0ceb71e653763b6cc771e7bdfe Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 18 Jun 2010 23:02:57 -0700 Subject: [PATCH] Replace WindowCache with ObjectReader The WindowCache is an implementation detail of PackFile and how its used by ObjectDirectory. Lets start to hide it and replace the public API with a more generic concept, ObjectReader. Because PackedObjectLoader is also considered a private detail of PackFile, we have to make PackWriter temporarily dependent upon the WindowCursor and thus FileRepository and ObjectDirectory in order to just start the refactoring. In later changes we will clean up the APIs more, exposing sufficient support to PackWriter without needing the file specific implementation details. Change-Id: I676be12b57f3534f1285854ee5de1aa483895398 Signed-off-by: Shawn O. Pearce --- .../eclipse/jgit/iplog/IpLogGenerator.java | 7 +- .../pgm/opt/AbstractTreeIteratorHandler.java | 4 +- .../eclipse/jgit/lib/T0004_PackReader.java | 2 +- .../transport/ReceivePackRefFilterTest.java | 2 +- .../jgit/dircache/DirCacheBuilder.java | 4 +- .../jgit/lib/CachedObjectDirectory.java | 9 +- .../eclipse/jgit/lib/FileObjectDatabase.java | 14 ++- .../org/eclipse/jgit/lib/FileRepository.java | 39 ------ .../org/eclipse/jgit/lib/ObjectDatabase.java | 66 ++++++---- .../org/eclipse/jgit/lib/ObjectDirectory.java | 4 +- .../org/eclipse/jgit/lib/ObjectReader.java | 114 ++++++++++++++++++ .../src/org/eclipse/jgit/lib/PackWriter.java | 7 +- .../src/org/eclipse/jgit/lib/Repository.java | 63 ++++------ .../org/eclipse/jgit/lib/WindowCursor.java | 41 +++++-- .../src/org/eclipse/jgit/merge/Merger.java | 4 +- .../src/org/eclipse/jgit/revwalk/RevWalk.java | 6 +- .../org/eclipse/jgit/transport/IndexPack.java | 22 +++- .../jgit/treewalk/AbstractTreeIterator.java | 4 +- .../jgit/treewalk/CanonicalTreeParser.java | 14 +-- .../org/eclipse/jgit/treewalk/TreeWalk.java | 5 +- 20 files changed, 274 insertions(+), 157 deletions(-) create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java diff --git a/org.eclipse.jgit.iplog/src/org/eclipse/jgit/iplog/IpLogGenerator.java b/org.eclipse.jgit.iplog/src/org/eclipse/jgit/iplog/IpLogGenerator.java index f64c32984..f40c3896a 100644 --- a/org.eclipse.jgit.iplog/src/org/eclipse/jgit/iplog/IpLogGenerator.java +++ b/org.eclipse.jgit.iplog/src/org/eclipse/jgit/iplog/IpLogGenerator.java @@ -86,7 +86,7 @@ import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.WindowCursor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; @@ -144,7 +144,7 @@ public class IpLogGenerator { private NameConflictTreeWalk tw; - private final WindowCursor curs = new WindowCursor(); + private ObjectReader curs; private final MutableObjectId idbuf = new MutableObjectId(); @@ -184,6 +184,7 @@ public void scan(Repository repo, RevCommit startCommit, String version) throws IOException, ConfigInvalidException { try { db = repo; + curs = db.newObjectReader(); rw = new RevWalk(db); tw = new NameConflictTreeWalk(db); @@ -194,7 +195,7 @@ public void scan(Repository repo, RevCommit startCommit, String version) scanProjectCommits(meta.getProjects().get(0), c); commits.add(c); } finally { - WindowCursor.release(curs); + curs.release(); db = null; rw = null; tw = null; diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/AbstractTreeIteratorHandler.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/AbstractTreeIteratorHandler.java index 2043ac209..d7b98c509 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/AbstractTreeIteratorHandler.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/AbstractTreeIteratorHandler.java @@ -59,7 +59,7 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.WindowCursor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.pgm.CLIText; import org.eclipse.jgit.treewalk.AbstractTreeIterator; import org.eclipse.jgit.treewalk.CanonicalTreeParser; @@ -121,7 +121,7 @@ public int parseArguments(final Parameters params) throws CmdLineException { throw new CmdLineException(MessageFormat.format(CLIText.get().notATree, name)); final CanonicalTreeParser p = new CanonicalTreeParser(); - final WindowCursor curs = new WindowCursor(); + final ObjectReader curs = clp.getRepository().newObjectReader(); try { p.reset(clp.getRepository(), clp.getRevWalk().parseTree(id), curs); } catch (MissingObjectException e) { diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0004_PackReader.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0004_PackReader.java index 336bba22c..54816f6f5 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0004_PackReader.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0004_PackReader.java @@ -63,7 +63,7 @@ public void test003_lookupCompressedObject() throws IOException { id = ObjectId.fromString("902d5476fa249b7abc9d84c611577a81381f0327"); pr = new PackFile(TEST_IDX, TEST_PACK); - or = pr.get(new WindowCursor(), id); + or = pr.get(new WindowCursor(null), id); assertNotNull(or); assertEquals(Constants.OBJ_TREE, or.getType()); assertEquals(35, or.getSize()); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java index b87cc7a99..03726cba0 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java @@ -176,7 +176,7 @@ public void testSuccess() throws Exception { // Verify the only storage of b is our packed delta above. // ObjectDirectory od = (ObjectDirectory) src.getObjectDatabase(); - assertTrue("has b", od.hasObject(b)); + assertTrue("has b", src.hasObject(b)); assertFalse("b not loose", od.fileFor(b).exists()); // Now use b but in a different commit than what is hidden. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheBuilder.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheBuilder.java index e6b619781..adb8a8d77 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheBuilder.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheBuilder.java @@ -51,7 +51,7 @@ import org.eclipse.jgit.JGitText; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.WindowCursor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.treewalk.AbstractTreeIterator; import org.eclipse.jgit.treewalk.CanonicalTreeParser; import org.eclipse.jgit.treewalk.TreeWalk; @@ -166,7 +166,7 @@ public void addTree(final byte[] pathPrefix, final int stage, final Repository db, final AnyObjectId tree) throws IOException { final TreeWalk tw = new TreeWalk(db); tw.reset(); - final WindowCursor curs = new WindowCursor(); + final ObjectReader curs = db.newObjectReader(); try { tw.addTree(new CanonicalTreeParser(pathPrefix, db, tree .toObjectId(), curs)); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CachedObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CachedObjectDirectory.java index 37a96d74f..351d6817f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CachedObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CachedObjectDirectory.java @@ -46,7 +46,6 @@ import java.io.File; import java.io.IOException; -import java.util.Collection; /** * The cached instance of an {@link ObjectDirectory}. @@ -116,12 +115,6 @@ FileObjectDatabase newCachedFileObjectDatabase() { return this; } - @Override - void openObjectInAllPacks(Collection out, - WindowCursor curs, AnyObjectId objectId) throws IOException { - wrapped.openObjectInAllPacks(out, curs, objectId); - } - @Override File getDirectory() { return wrapped.getDirectory(); @@ -157,7 +150,7 @@ boolean hasObject1(AnyObjectId objectId) { } @Override - public ObjectLoader openObject(final WindowCursor curs, + ObjectLoader openObject(final WindowCursor curs, final AnyObjectId objectId) throws IOException { return openObjectImpl1(curs, objectId); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/FileObjectDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/FileObjectDatabase.java index a7bf60370..36e808c23 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/FileObjectDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/FileObjectDatabase.java @@ -45,8 +45,14 @@ import java.io.File; import java.io.IOException; +import java.util.Collection; abstract class FileObjectDatabase extends ObjectDatabase { + @Override + public ObjectReader newReader() { + return new WindowCursor(this); + } + /** * Does the requested object exist in this database? *

@@ -98,8 +104,8 @@ final boolean hasObjectImpl2(final String objectId) { * object, or null if the object does not exist. * @throws IOException */ - public ObjectLoader openObject(final WindowCursor curs, - final AnyObjectId objectId) throws IOException { + ObjectLoader openObject(final WindowCursor curs, final AnyObjectId objectId) + throws IOException { ObjectLoader ldr; ldr = openObjectImpl1(curs, objectId); @@ -154,6 +160,10 @@ final ObjectLoader openObjectImpl2(final WindowCursor curs, return null; } + void openObjectInAllPacks(Collection reuseLoaders, + WindowCursor windowCursor, AnyObjectId otp) throws IOException { + } + abstract File getDirectory(); abstract AlternateHandle[] myAlternates(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/FileRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/FileRepository.java index 12248fb42..32f516f7d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/FileRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/FileRepository.java @@ -49,9 +49,7 @@ import java.io.File; import java.io.IOException; import java.text.MessageFormat; -import java.util.Collection; import java.util.HashSet; -import java.util.LinkedList; import java.util.Set; import org.eclipse.jgit.JGitText; @@ -396,43 +394,6 @@ public File toFile(final AnyObjectId objectId) { return objectDatabase.fileFor(objectId); } - /** - * Open object in all packs containing specified object. - * - * @param objectId - * id of object to search for - * @param curs - * temporary working space associated with the calling thread. - * @return collection of loaders for this object, from all packs containing - * this object - * @throws IOException - */ - public Collection openObjectInAllPacks( - final AnyObjectId objectId, final WindowCursor curs) - throws IOException { - Collection result = new LinkedList(); - openObjectInAllPacks(objectId, result, curs); - return result; - } - - /** - * Open object in all packs containing specified object. - * - * @param objectId - * id of object to search for - * @param resultLoaders - * result collection of loaders for this object, filled with - * loaders from all packs containing specified object - * @param curs - * temporary working space associated with the calling thread. - * @throws IOException - */ - void openObjectInAllPacks(final AnyObjectId objectId, - final Collection resultLoaders, - final WindowCursor curs) throws IOException { - objectDatabase.openObjectInAllPacks(resultLoaders, curs, objectId); - } - /** * Objects known to exist but not expressed by {@link #getAllRefs()}. *

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectDatabase.java index c44a7ac6f..c2d2beda9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectDatabase.java @@ -44,7 +44,8 @@ package org.eclipse.jgit.lib; import java.io.IOException; -import java.util.Collection; + +import org.eclipse.jgit.errors.MissingObjectException; /** * Abstraction of arbitrary object storage. @@ -89,6 +90,17 @@ public void create() throws IOException { */ public abstract ObjectInserter newInserter(); + /** + * Create a new {@code ObjectReader} to read existing objects. + *

+ * The returned reader is not itself thread-safe, but multiple concurrent + * reader instances created from the same {@code ObjectDatabase} must be + * thread-safe. + * + * @return reader the caller can use to load objects from this database. + */ + public abstract ObjectReader newReader(); + /** * Close any resources held by this database. */ @@ -96,42 +108,48 @@ public void create() throws IOException { /** * Does the requested object exist in this database? + *

+ * This is a one-shot call interface which may be faster than allocating a + * {@link #newReader()} to perform the lookup. * * @param objectId * identity of the object to test for existence of. * @return true if the specified object is stored in this database. + * @throws IOException + * the object store cannot be accessed. */ - public abstract boolean hasObject(AnyObjectId objectId); + public boolean hasObject(final AnyObjectId objectId) throws IOException { + final ObjectReader or = newReader(); + try { + return or.hasObject(objectId); + } finally { + or.release(); + } + } /** * Open an object from this database. + *

+ * This is a one-shot call interface which may be faster than allocating a + * {@link #newReader()} to perform the lookup. * - * @param curs - * temporary working space associated with the calling thread. * @param objectId * identity of the object to open. - * @return a {@link ObjectLoader} for accessing the data of the named - * object, or null if the object does not exist. + * @return a {@link ObjectLoader} for accessing the object. + * @throws MissingObjectException + * the object does not exist. * @throws IOException + * the object store cannot be accessed. */ - public abstract ObjectLoader openObject(WindowCursor curs, - AnyObjectId objectId) throws IOException; - - /** - * Open the object from all packs containing it. - * - * @param out - * result collection of loaders for this object, filled with - * loaders from all packs containing specified object - * @param curs - * temporary working space associated with the calling thread. - * @param objectId - * id of object to search for - * @throws IOException - */ - abstract void openObjectInAllPacks(final Collection out, - final WindowCursor curs, final AnyObjectId objectId) - throws IOException; + public ObjectLoader openObject(final AnyObjectId objectId) + throws IOException { + final ObjectReader or = newReader(); + try { + return or.openObject(objectId); + } finally { + or.release(); + } + } /** * Create a new cached database instance over this database. This instance might diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectDirectory.java index c605df9b8..810e48890 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectDirectory.java @@ -283,7 +283,6 @@ ObjectLoader openObject1(final WindowCursor curs, } } - @Override void openObjectInAllPacks(final Collection out, final WindowCursor curs, final AnyObjectId objectId) throws IOException { @@ -308,6 +307,9 @@ void openObjectInAllPacks(final Collection out, } break SEARCH; } + + for (AlternateHandle h : myAlternates()) + h.db.openObjectInAllPacks(out, curs, objectId); } boolean hasObject2(final String objectName) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java new file mode 100644 index 000000000..c95130ce2 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java @@ -0,0 +1,114 @@ +/* + * Copyright (C) 2010, Google Inc. + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.lib; + +import java.io.IOException; + +import org.eclipse.jgit.errors.MissingObjectException; + +/** Reads an {@link ObjectDatabase} for a single thread. */ +public abstract class ObjectReader { + /** Type hint indicating the caller doesn't know the type. */ + protected static final int OBJ_ANY = -1; + + /** + * Does the requested object exist in this database? + * + * @param objectId + * identity of the object to test for existence of. + * @return true if the specified object is stored in this database. + * @throws IOException + * the object store cannot be accessed. + */ + public boolean hasObject(AnyObjectId objectId) throws IOException { + try { + openObject(objectId); + return true; + } catch (MissingObjectException notFound) { + return false; + } + } + + /** + * Open an object from this database. + * + * @param objectId + * identity of the object to open. + * @return a {@link ObjectLoader} for accessing the object. + * @throws MissingObjectException + * the object does not exist. + * @throws IOException + */ + public ObjectLoader openObject(AnyObjectId objectId) + throws MissingObjectException, IOException { + return openObject(objectId, OBJ_ANY); + } + + /** + * Open an object from this database. + * + * @param objectId + * identity of the object to open. + *@param typeHint + * hint about the type of object being requested; + * {@link #OBJ_ANY} if the object type is not known, or does not + * matter to the caller. + * @return a {@link ObjectLoader} for accessing the object. + * @throws MissingObjectException + * the object does not exist. + * @throws IOException + */ + public abstract ObjectLoader openObject(AnyObjectId objectId, int typeHint) + throws MissingObjectException, IOException; + + /** + * Release any resources used by this reader. + *

+ * A reader that has been released can be used again, but may need to be + * released after the subsequent usage. + */ + public void release() { + // Do nothing. + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackWriter.java index e8e5fc21e..15ac17425 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PackWriter.java @@ -179,7 +179,7 @@ public class PackWriter { private final byte[] buf = new byte[16384]; // 16 KB - private final WindowCursor windowCursor = new WindowCursor(); + private final WindowCursor windowCursor; private List sortedByName; @@ -236,6 +236,9 @@ public PackWriter(final Repository repo, final ProgressMonitor monitor) { public PackWriter(final Repository repo, final ProgressMonitor imonitor, final ProgressMonitor wmonitor) { this.db = repo; + windowCursor = new WindowCursor((ObjectDirectory) repo + .getObjectDatabase()); + initMonitor = imonitor == null ? NullProgressMonitor.INSTANCE : imonitor; writeMonitor = wmonitor == null ? NullProgressMonitor.INSTANCE : wmonitor; @@ -621,7 +624,7 @@ private void searchForReuse() throws IOException { private void searchForReuse( final Collection reuseLoaders, final ObjectToPack otp) throws IOException { - db.openObjectInAllPacks(otp, reuseLoaders, windowCursor); + windowCursor.openObjectInAllPacks(otp, reuseLoaders); if (reuseDeltas) { selectDeltaReuseForObject(otp, reuseLoaders); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index 334581415..400cae43d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -50,7 +50,6 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -64,6 +63,7 @@ import org.eclipse.jgit.JGitText; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.errors.IncorrectObjectTypeException; +import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.RevisionSyntaxException; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; @@ -153,6 +153,11 @@ public ObjectInserter newObjectInserter() { return getObjectDatabase().newInserter(); } + /** @return a new inserter to create objects in {@link #getObjectDatabase()} */ + public ObjectReader newObjectReader() { + return getObjectDatabase().newReader(); + } + /** @return the reference database which stores the reference namespace. */ public abstract RefDatabase getRefDatabase(); @@ -186,7 +191,12 @@ public FS getFS() { * known shared repositories. */ public boolean hasObject(AnyObjectId objectId) { - return getObjectDatabase().hasObject(objectId); + try { + return getObjectDatabase().hasObject(objectId); + } catch (IOException e) { + // Legacy API, assume error means "no" + return false; + } } /** @@ -199,11 +209,11 @@ public boolean hasObject(AnyObjectId objectId) { */ public ObjectLoader openObject(final AnyObjectId id) throws IOException { - final WindowCursor wc = new WindowCursor(); try { - return openObject(wc, id); - } finally { - wc.release(); + return getObjectDatabase().openObject(id); + } catch (MissingObjectException notFound) { + // Legacy API, return null + return null; } } @@ -216,43 +226,18 @@ public ObjectLoader openObject(final AnyObjectId id) * @return a {@link ObjectLoader} for accessing the data of the named * object, or null if the object does not exist. * @throws IOException + * @deprecated Use {code newObjectReader().open(id)}. */ - public ObjectLoader openObject(WindowCursor curs, AnyObjectId id) + @Deprecated + public ObjectLoader openObject(ObjectReader curs, AnyObjectId id) throws IOException { - return getObjectDatabase().openObject(curs, id); + try { + return curs.openObject(id); + } catch (MissingObjectException notFound) { + return null; + } } - /** - * Open object in all packs containing specified object. - * - * @param objectId - * id of object to search for - * @param curs - * temporary working space associated with the calling thread. - * @return collection of loaders for this object, from all packs containing - * this object - * @throws IOException - */ - public abstract Collection openObjectInAllPacks( - AnyObjectId objectId, WindowCursor curs) - throws IOException; - - /** - * Open object in all packs containing specified object. - * - * @param objectId - * id of object to search for - * @param resultLoaders - * result collection of loaders for this object, filled with - * loaders from all packs containing specified object - * @param curs - * temporary working space associated with the calling thread. - * @throws IOException - */ - abstract void openObjectInAllPacks(final AnyObjectId objectId, - final Collection resultLoaders, - final WindowCursor curs) throws IOException; - /** * @param id * SHA'1 of a blob diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/WindowCursor.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/WindowCursor.java index 968c92e5c..afc7f7186 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/WindowCursor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/WindowCursor.java @@ -45,11 +45,14 @@ package org.eclipse.jgit.lib; import java.io.IOException; +import java.util.Collection; import java.util.zip.DataFormatException; import java.util.zip.Inflater; +import org.eclipse.jgit.errors.MissingObjectException; + /** Active handle to a ByteWindow. */ -public final class WindowCursor { +final class WindowCursor extends ObjectReader { /** Temporary buffer large enough for at least one raw object id. */ final byte[] tempId = new byte[Constants.OBJECT_ID_LENGTH]; @@ -57,6 +60,32 @@ public final class WindowCursor { private ByteWindow window; + private final FileObjectDatabase db; + + WindowCursor(FileObjectDatabase db) { + this.db = db; + } + + public boolean hasObject(AnyObjectId objectId) throws IOException { + return db.hasObject(objectId); + } + + public ObjectLoader openObject(AnyObjectId objectId, int typeHint) + throws MissingObjectException, IOException { + final ObjectLoader ldr = db.openObject(this, objectId); + if (ldr == null) { + if (typeHint == OBJ_ANY) + throw new MissingObjectException(objectId.copy(), "unknown"); + throw new MissingObjectException(objectId.copy(), typeHint); + } + return ldr; + } + + void openObjectInAllPacks(AnyObjectId otp, + Collection reuseLoaders) throws IOException { + db.openObjectInAllPacks(reuseLoaders, this, otp); + } + /** * Copy bytes from the window to a caller supplied buffer. * @@ -168,14 +197,4 @@ public void release() { inf = null; } } - - /** - * @param curs cursor to release; may be null. - * @return always null. - */ - public static WindowCursor release(final WindowCursor curs) { - if (curs != null) - curs.release(); - return null; - } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/Merger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/Merger.java index 59b826438..38a8b8eae 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/Merger.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/Merger.java @@ -53,7 +53,7 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.WindowCursor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevTree; @@ -202,7 +202,7 @@ protected AbstractTreeIterator mergeBase(final int aIdx, final int bIdx) */ protected AbstractTreeIterator openTree(final AnyObjectId treeId) throws IncorrectObjectTypeException, IOException { - final WindowCursor curs = new WindowCursor(); + final ObjectReader curs = db.newObjectReader(); try { return new CanonicalTreeParser(null, db, treeId, curs); } finally { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java index e42554811..122418296 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java @@ -62,7 +62,7 @@ import org.eclipse.jgit.lib.ObjectIdSubclassMap; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.WindowCursor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.revwalk.filter.RevFilter; import org.eclipse.jgit.treewalk.filter.TreeFilter; @@ -159,7 +159,7 @@ public class RevWalk implements Iterable { final Repository db; - final WindowCursor curs; + final ObjectReader curs; final MutableObjectId idBuffer; @@ -193,7 +193,7 @@ public class RevWalk implements Iterable { */ public RevWalk(final Repository repo) { db = repo; - curs = new WindowCursor(); + curs = db.newObjectReader(); idBuffer = new MutableObjectId(); objects = new ObjectIdSubclassMap(); roots = new ArrayList(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java index f6e04107f..0cd673369 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java @@ -79,7 +79,7 @@ import org.eclipse.jgit.lib.PackLock; import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.WindowCursor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.util.NB; /** Indexes Git pack files for local use. */ @@ -213,7 +213,7 @@ public static IndexPack create(final Repository db, final InputStream is) /** If {@link #fixThin} this is the last byte of the original checksum. */ private long originalEOF; - private WindowCursor readCurs; + private ObjectReader readCurs; /** * Create a new pack indexer utility. @@ -232,7 +232,7 @@ public IndexPack(final Repository db, final InputStream src, objectDatabase = db.getObjectDatabase().newCachedDatabase(); in = src; inflater = InflaterCache.get(); - readCurs = new WindowCursor(); + readCurs = objectDatabase.newReader(); buf = new byte[BUFFER_SIZE]; objectData = new byte[BUFFER_SIZE]; objectDigest = Constants.newMessageDigest(); @@ -430,7 +430,13 @@ public void index(final ProgressMonitor progress) throws IOException { inflater = null; objectDatabase.close(); } - readCurs = WindowCursor.release(readCurs); + + try { + if (readCurs != null) + readCurs.release(); + } finally { + readCurs = null; + } progress.endTask(); if (packOut != null) @@ -847,12 +853,16 @@ private void verifySafeObject(final AnyObjectId id, final int type, } } - final ObjectLoader ldr = objectDatabase.openObject(readCurs, id); - if (ldr != null) { + try { + final ObjectLoader ldr = readCurs.openObject(id, type); final byte[] existingData = ldr.getCachedBytes(); if (ldr.getType() != type || !Arrays.equals(data, existingData)) { throw new IOException(MessageFormat.format(JGitText.get().collisionOn, id.name())); } + } catch (MissingObjectException notLocal) { + // This is OK, we don't have a copy of the object locally + // but the API throws when we try to read it as usually its + // an error to read something that doesn't exist. } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java index 90cea0f1b..73357a4df 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java @@ -56,7 +56,7 @@ import org.eclipse.jgit.lib.MutableObjectId; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.WindowCursor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.treewalk.filter.TreeFilter; /** @@ -474,7 +474,7 @@ public EmptyTreeIterator createEmptyTreeIterator() { * a loose object or pack file could not be read. */ public AbstractTreeIterator createSubtreeIterator(final Repository repo, - final MutableObjectId idBuffer, final WindowCursor curs) + final MutableObjectId idBuffer, final ObjectReader curs) throws IncorrectObjectTypeException, IOException { return createSubtreeIterator(repo); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java index 0b9dc0044..b7f980cca 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java @@ -56,7 +56,7 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.WindowCursor; +import org.eclipse.jgit.lib.ObjectReader; /** Parses raw Git trees from the canonical semi-text/semi-binary format. */ public class CanonicalTreeParser extends AbstractTreeIterator { @@ -102,7 +102,7 @@ public CanonicalTreeParser() { * a loose object or pack file could not be read. */ public CanonicalTreeParser(final byte[] prefix, final Repository repo, - final AnyObjectId treeId, final WindowCursor curs) + final AnyObjectId treeId, final ObjectReader curs) throws IncorrectObjectTypeException, IOException { super(prefix); reset(repo, treeId, curs); @@ -148,7 +148,7 @@ public void reset(final byte[] treeData) { * a loose object or pack file could not be read. */ public CanonicalTreeParser resetRoot(final Repository repo, - final AnyObjectId id, final WindowCursor curs) + final AnyObjectId id, final ObjectReader curs) throws IncorrectObjectTypeException, IOException { CanonicalTreeParser p = this; while (p.parent != null) @@ -197,7 +197,7 @@ public CanonicalTreeParser next() { * a loose object or pack file could not be read. */ public void reset(final Repository repo, final AnyObjectId id, - final WindowCursor curs) + final ObjectReader curs) throws IncorrectObjectTypeException, IOException { final ObjectLoader ldr = repo.openObject(curs, id); if (ldr == null) { @@ -214,7 +214,7 @@ public void reset(final Repository repo, final AnyObjectId id, @Override public CanonicalTreeParser createSubtreeIterator(final Repository repo, - final MutableObjectId idBuffer, final WindowCursor curs) + final MutableObjectId idBuffer, final ObjectReader curs) throws IncorrectObjectTypeException, IOException { idBuffer.fromRaw(idBuffer(), idOffset()); if (!FileMode.TREE.equals(mode)) { @@ -242,7 +242,7 @@ public CanonicalTreeParser createSubtreeIterator(final Repository repo, * a loose object or pack file could not be read. */ public final CanonicalTreeParser createSubtreeIterator0( - final Repository repo, final AnyObjectId id, final WindowCursor curs) + final Repository repo, final AnyObjectId id, final ObjectReader curs) throws IOException { final CanonicalTreeParser p = new CanonicalTreeParser(this); p.reset(repo, id, curs); @@ -251,7 +251,7 @@ public final CanonicalTreeParser createSubtreeIterator0( public CanonicalTreeParser createSubtreeIterator(final Repository repo) throws IncorrectObjectTypeException, IOException { - final WindowCursor curs = new WindowCursor(); + final ObjectReader curs = repo.newObjectReader(); try { return createSubtreeIterator(repo, new MutableObjectId(), curs); } finally { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/TreeWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/TreeWalk.java index 25ab78ef0..aefa79c3a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/TreeWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/TreeWalk.java @@ -57,7 +57,7 @@ import org.eclipse.jgit.lib.MutableObjectId; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.WindowCursor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.treewalk.filter.PathFilterGroup; import org.eclipse.jgit.treewalk.filter.TreeFilter; @@ -155,7 +155,7 @@ public static TreeWalk forPath(final Repository db, final String path, private final MutableObjectId idBuffer = new MutableObjectId(); - private final WindowCursor curs = new WindowCursor(); + private final ObjectReader curs; private TreeFilter filter; @@ -181,6 +181,7 @@ public static TreeWalk forPath(final Repository db, final String path, */ public TreeWalk(final Repository repo) { db = repo; + curs = repo.newObjectReader(); filter = TreeFilter.ALL; trees = new AbstractTreeIterator[] { new EmptyTreeIterator() }; }