PackWriter: use lib.ObjectIdSet to avoid wrapper

Hoist ObjectIdSet up to lib as part of the public API and add
the interface to some common types like PackIndex and JGit custom
ObjectId map types.  This cleans up wrapper code in a number of
places by allowing direct use of the types as an ObjectIdSet.

Future commits can now rely on ObjectIdSet as a simple read-only
type to check a set of objects from a number of storage options.

Change-Id: Ib62b062421d475bd52abd6c84a73916ef36e084b

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java

@@ -43,12 +43,12 @@
 
 package org.eclipse.jgit.internal.storage.file;
 
+import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
@@ -67,13 +67,12 @@ import java.util.Set;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry;
 import org.eclipse.jgit.internal.storage.pack.PackWriter;
-import org.eclipse.jgit.internal.storage.pack.PackWriter.ObjectIdSet;
 import org.eclipse.jgit.junit.JGitTestUtil;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.junit.TestRepository.BranchBuilder;
-import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectIdSet;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.revwalk.RevBlob;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -528,7 +527,7 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 		RevCommit c2 = bb.commit().add("f", contentB).create();
 		testRepo.getRevWalk().parseHeaders(c2);
 		PackIndex pf2 = writePack(repo, Collections.singleton(c2),
-				Collections.singleton(objectIdSet(pf1)));
+				Collections.<ObjectIdSet> singleton(pf1));
 		assertContent(
 				pf2,
 				Arrays.asList(c2.getId(), c2.getTree().getId(),
@@ -733,12 +732,4 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 			assertEquals(objectsOrder[i++].toObjectId(), me.toObjectId());
 		}
 	}
-
-	private static ObjectIdSet objectIdSet(final PackIndex idx) {
-		return new ObjectIdSet() {
-			public boolean contains(AnyObjectId objectId) {
-				return idx.hasObject(objectId);
-			}
-		};
-	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java

@@ -67,7 +67,7 @@ import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectIdOwnerMap;
+import org.eclipse.jgit.lib.ObjectIdSet;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -87,7 +87,7 @@ public class DfsGarbageCollector {
 
 	private final List<PackStatistics> newPackStats;
 
-	private final List<PackWriter.ObjectIdSet> newPackObj;
+	private final List<ObjectIdSet> newPackObj;
 
 	private DfsReader ctx;
 
@@ -117,7 +117,7 @@ public class DfsGarbageCollector {
 		objdb = repo.getObjectDatabase();
 		newPackDesc = new ArrayList<DfsPackDescription>(4);
 		newPackStats = new ArrayList<PackStatistics>(4);
-		newPackObj = new ArrayList<PackWriter.ObjectIdSet>(4);
+		newPackObj = new ArrayList<ObjectIdSet>(4);
 
 		packConfig = new PackConfig(repo);
 		packConfig.setIndexVersion(2);
@@ -288,7 +288,7 @@ public class DfsGarbageCollector {
 			return;
 
 		try (PackWriter pw = newPackWriter()) {
-			for (PackWriter.ObjectIdSet packedObjs : newPackObj)
+			for (ObjectIdSet packedObjs : newPackObj)
 				pw.excludeObjects(packedObjs);
 			pw.preparePack(pm, nonHeads, allHeads);
 			if (0 < pw.getObjectCount())
@@ -328,7 +328,7 @@ public class DfsGarbageCollector {
 	}
 
 	private boolean anyPackHas(AnyObjectId id) {
-		for (PackWriter.ObjectIdSet packedObjs : newPackObj)
+		for (ObjectIdSet packedObjs : newPackObj)
 			if (packedObjs.contains(id))
 				return true;
 		return false;
@@ -389,17 +389,10 @@ public class DfsGarbageCollector {
 			}
 		}
 
-		final ObjectIdOwnerMap<ObjectIdOwnerMap.Entry> packedObjs = pw
-				.getObjectSet();
-		newPackObj.add(new PackWriter.ObjectIdSet() {
-			public boolean contains(AnyObjectId objectId) {
-				return packedObjs.contains(objectId);
-			}
-		});
-
 		PackStatistics stats = pw.getStatistics();
 		pack.setPackStats(stats);
 		newPackStats.add(stats);
+		newPackObj.add(pw.getObjectSet());
 
 		DfsBlockCache.getInstance().getOrCreate(pack, null);
 		return pack;

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java

@@ -62,6 +62,7 @@ import org.eclipse.jgit.internal.storage.pack.PackWriter;
 import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectIdSet;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.revwalk.RevFlag;
 import org.eclipse.jgit.revwalk.RevObject;
@@ -91,7 +92,7 @@ public class DfsPackCompactor {
 
 	private final List<DfsPackFile> srcPacks;
 
-	private final List<PackWriter.ObjectIdSet> exclude;
+	private final List<ObjectIdSet> exclude;
 
 	private final List<DfsPackDescription> newPacks;
 
@@ -113,7 +114,7 @@ public class DfsPackCompactor {
 		repo = repository;
 		autoAddSize = 5 * 1024 * 1024; // 5 MiB
 		srcPacks = new ArrayList<DfsPackFile>();
-		exclude = new ArrayList<PackWriter.ObjectIdSet>(4);
+		exclude = new ArrayList<ObjectIdSet>(4);
 		newPacks = new ArrayList<DfsPackDescription>(1);
 		newStats = new ArrayList<PackStatistics>(1);
 	}
@@ -164,7 +165,7 @@ public class DfsPackCompactor {
 	 *            objects to not include.
 	 * @return {@code this}.
 	 */
-	public DfsPackCompactor exclude(PackWriter.ObjectIdSet set) {
+	public DfsPackCompactor exclude(ObjectIdSet set) {
 		exclude.add(set);
 		return this;
 	}
@@ -183,11 +184,7 @@ public class DfsPackCompactor {
 		try (DfsReader ctx = (DfsReader) repo.newObjectReader()) {
 			idx = pack.getPackIndex(ctx);
 		}
-		return exclude(new PackWriter.ObjectIdSet() {
-			public boolean contains(AnyObjectId id) {
-				return idx.hasObject(id);
-			}
-		});
+		return exclude(idx);
 	}
 
 	/**
@@ -343,7 +340,7 @@ public class DfsPackCompactor {
 			RevObject obj = rw.lookupOrNull(id);
 			if (obj != null && (obj.has(added) || obj.has(isBase)))
 				continue;
-			for (PackWriter.ObjectIdSet e : exclude)
+			for (ObjectIdSet e : exclude)
 				if (e.contains(id))
 					continue SCAN;
 			want.add(new ObjectIdWithOffset(id, ent.getOffset()));

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java

@@ -79,13 +79,12 @@ import org.eclipse.jgit.errors.NoWorkTreeException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.pack.PackExt;
 import org.eclipse.jgit.internal.storage.pack.PackWriter;
-import org.eclipse.jgit.internal.storage.pack.PackWriter.ObjectIdSet;
-import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectIdSet;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Ref.Storage;
@@ -552,7 +551,7 @@ public class GC {
 		List<ObjectIdSet> excluded = new LinkedList<ObjectIdSet>();
 		for (final PackFile f : repo.getObjectDatabase().getPacks())
 			if (f.shouldBeKept())
-				excluded.add(objectIdSet(f.getIndex()));
+				excluded.add(f.getIndex());
 
 		tagTargets.addAll(allHeads);
 		nonHeads.addAll(indexObjects);
@@ -564,7 +563,7 @@ public class GC {
 					tagTargets, excluded);
 			if (heads != null) {
 				ret.add(heads);
-				excluded.add(0, objectIdSet(heads.getIndex()));
+				excluded.add(0, heads.getIndex());
 			}
 		}
 		if (!nonHeads.isEmpty()) {
@@ -1000,12 +999,4 @@ public class GC {
 		this.expire = expire;
 		expireAgeMillis = -1;
 	}
-
-	private static ObjectIdSet objectIdSet(final PackIndex idx) {
-		return new ObjectIdSet() {
-			public boolean contains(AnyObjectId objectId) {
-				return idx.hasObject(objectId);
-			}
-		};
-	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackIndex.java

@@ -60,6 +60,7 @@ import org.eclipse.jgit.lib.AbbreviatedObjectId;
 import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.MutableObjectId;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectIdSet;
 import org.eclipse.jgit.util.IO;
 import org.eclipse.jgit.util.NB;
 
@@ -72,7 +73,8 @@ import org.eclipse.jgit.util.NB;
  * by ObjectId.
  * </p>
  */
-public abstract class PackIndex implements Iterable<PackIndex.MutableEntry> {
+public abstract class PackIndex
+		implements Iterable<PackIndex.MutableEntry>, ObjectIdSet {
 	/**
 	 * Open an existing pack <code>.idx</code> file for reading.
 	 * <p>
@@ -166,6 +168,11 @@ public abstract class PackIndex implements Iterable<PackIndex.MutableEntry> {
 		return findOffset(id) != -1;
 	}
 
+	@Override
+	public boolean contains(AnyObjectId id) {
+		return findOffset(id) != -1;
+	}
+
 	/**
 	 * Provide iterator that gives access to index entries. Note, that iterator
 	 * returns reference to mutable object, the same reference in each call -

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java

@@ -99,6 +99,7 @@ import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdOwnerMap;
+import org.eclipse.jgit.lib.ObjectIdSet;
 import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.ProgressMonitor;
@@ -161,18 +162,6 @@ import org.eclipse.jgit.util.TemporaryBuffer;
 public class PackWriter implements AutoCloseable {
 	private static final int PACK_VERSION_GENERATED = 2;
 
-	/** A collection of object ids. */
-	public interface ObjectIdSet {
-		/**
-		 * Returns true if the objectId is contained within the collection.
-		 *
-		 * @param objectId
-		 *            the objectId to find
-		 * @return whether the collection contains the objectId.
-		 */
-		boolean contains(AnyObjectId objectId);
-	}
-
 	private static final Map<WeakReference<PackWriter>, Boolean> instances =
 			new ConcurrentHashMap<WeakReference<PackWriter>, Boolean>();
 

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

@@ -67,8 +67,8 @@ import java.util.NoSuchElementException;
  * @param <V>
  *            type of subclass of ObjectId that will be stored in the map.
  */
-public class ObjectIdOwnerMap<V extends ObjectIdOwnerMap.Entry> implements
-		Iterable<V> {
+public class ObjectIdOwnerMap<V extends ObjectIdOwnerMap.Entry>
+		implements Iterable<V>, ObjectIdSet {
 	/** Size of the initial directory, will grow as necessary. */
 	private static final int INITIAL_DIRECTORY = 1024;
 

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

@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2015, 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;
+
+/**
+ * Simple set of ObjectIds.
+ * <p>
+ * Usually backed by a read-only data structure such as
+ * {@link org.eclipse.jgit.internal.storage.file.PackIndex}. Mutable types like
+ * {@link ObjectIdOwnerMap} also implement the interface by checking keys.
+ *
+ * @since 4.2
+ */
+public interface ObjectIdSet {
+	/**
+	 * Returns true if the objectId is contained within the collection.
+	 *
+	 * @param objectId
+	 *            the objectId to find
+	 * @return whether the collection contains the objectId.
+	 */
+	boolean contains(AnyObjectId objectId);
+}

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

@@ -60,7 +60,8 @@ import java.util.NoSuchElementException;
  * @param <V>
  *            type of subclass of ObjectId that will be stored in the map.
  */
-public class ObjectIdSubclassMap<V extends ObjectId> implements Iterable<V> {
+public class ObjectIdSubclassMap<V extends ObjectId>
+		implements Iterable<V>, ObjectIdSet {
 	private static final int INITIAL_TABLE_SIZE = 2048;
 
 	int size;