PackWriter: Declare preparePack object sets as @NonNull

Require callers to pass in valid sets for both want and have
collections. Offer PackWriter.NONE as a handy constant for an
empty collection for the have part of preparePack instead of null.

Change-Id: Ifda4450f5e488cbfefd728382b7d30797e229217

5 files changed, 38 insertions, 42 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 c941afc786..3a4f9c7884 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
@@ -866,7 +866,7 @@ public class TestRepository<R extends Repository> {
 				Set<ObjectId> all = new HashSet<ObjectId>();
 				for (Ref r : db.getAllRefs().values())
 					all.add(r.getObjectId());
-				pw.preparePack(m, all, Collections.<ObjectId> emptySet());
+				pw.preparePack(m, all, PackWriter.NONE);
 
 				final ObjectId name = pw.computeName();
 
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java
index 3c44799b62..01d6ee68e8 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java
@@ -49,6 +49,7 @@ 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.internal.storage.pack.PackWriter.NONE;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
@@ -88,9 +89,6 @@ import org.junit.Test;
 
 public class PackWriterTest extends SampleDataRepositoryTestCase {
 
-	private static final Set<ObjectId> EMPTY_SET_OBJECT = Collections
-			.<ObjectId> emptySet();
-
 	private static final List<RevObject> EMPTY_LIST_REVS = Collections
 			.<RevObject> emptyList();
 
@@ -171,7 +169,7 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 	 */
 	@Test
 	public void testWriteEmptyPack1() throws IOException {
-		createVerifyOpenPack(EMPTY_SET_OBJECT, EMPTY_SET_OBJECT, false, false);
+		createVerifyOpenPack(NONE, NONE, false, false);
 
 		assertEquals(0, writer.getObjectCount());
 		assertEquals(0, pack.getObjectCount());
@@ -204,8 +202,8 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 		final ObjectId nonExisting = ObjectId
 				.fromString("0000000000000000000000000000000000000001");
 		try {
-			createVerifyOpenPack(EMPTY_SET_OBJECT, Collections.singleton(
-					nonExisting), false, false);
+			createVerifyOpenPack(NONE, Collections.singleton(nonExisting),
+					false, false);
 			fail("Should have thrown MissingObjectException");
 		} catch (MissingObjectException x) {
 			// expected
@@ -221,8 +219,8 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 	public void testIgnoreNonExistingObjects() throws IOException {
 		final ObjectId nonExisting = ObjectId
 				.fromString("0000000000000000000000000000000000000001");
-		createVerifyOpenPack(EMPTY_SET_OBJECT, Collections.singleton(
-				nonExisting), false, true);
+		createVerifyOpenPack(NONE, Collections.singleton(nonExisting),
+				false, true);
 		// shouldn't throw anything
 	}
 
@@ -240,8 +238,8 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 		final ObjectId nonExisting = ObjectId
 				.fromString("0000000000000000000000000000000000000001");
 		new GC(db).gc();
-		createVerifyOpenPack(EMPTY_SET_OBJECT,
-				Collections.singleton(nonExisting), false, true, true);
+		createVerifyOpenPack(NONE, Collections.singleton(nonExisting), false,
+				true, true);
 		// shouldn't throw anything
 	}
 
@@ -552,8 +550,7 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 			pw.setReuseDeltaCommits(false);
 			for (ObjectIdSet idx : excludeObjects)
 				pw.excludeObjects(idx);
-			pw.preparePack(NullProgressMonitor.INSTANCE, want,
-					Collections.<ObjectId> emptySet());
+			pw.preparePack(NullProgressMonitor.INSTANCE, want, NONE);
 			String id = pw.computeName().getName();
 			File packdir = new File(repo.getObjectsDirectory(), "pack");
 			File packFile = new File(packdir, "pack-" + id + ".pack");
@@ -576,7 +573,7 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 		final HashSet<ObjectId> interestings = new HashSet<ObjectId>();
 		interestings.add(ObjectId
 				.fromString("82c6b885ff600be425b4ea96dee75dca255b69e7"));
-		createVerifyOpenPack(interestings, EMPTY_SET_OBJECT, false, false);
+		createVerifyOpenPack(interestings, NONE, false, false);
 
 		final ObjectId expectedOrder[] = new ObjectId[] {
 				ObjectId.fromString("82c6b885ff600be425b4ea96dee75dca255b69e7"),
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java
index c48a49da7a..784507d88c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java
@@ -53,7 +53,6 @@ import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -278,7 +277,7 @@ public class DfsGarbageCollector {
 
 		try (PackWriter pw = newPackWriter()) {
 			pw.setTagTargets(tagTargets);
-			pw.preparePack(pm, allHeads, none());
+			pw.preparePack(pm, allHeads, PackWriter.NONE);
 			if (0 < pw.getObjectCount())
 				writePack(GC, pw, pm);
 		}
@@ -303,16 +302,12 @@ public class DfsGarbageCollector {
 		try (PackWriter pw = newPackWriter()) {
 			for (ObjectIdSet packedObjs : newPackObj)
 				pw.excludeObjects(packedObjs);
-			pw.preparePack(pm, txnHeads, none());
+			pw.preparePack(pm, txnHeads, PackWriter.NONE);
 			if (0 < pw.getObjectCount())
 				writePack(GC_TXN, pw, pm);
 		}
 	}
 
-	private static Set<ObjectId> none() {
-		return Collections.<ObjectId> emptySet();
-	}
-
 	private void packGarbage(ProgressMonitor pm) throws IOException {
 		// TODO(sop) This is ugly. The garbage pack needs to be deleted.
 		PackConfig cfg = new PackConfig(packConfig);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
index 8677164a67..2ce0d47348 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
@@ -69,6 +69,7 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.TreeMap;
 
+import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.dircache.DirCacheIterator;
 import org.eclipse.jgit.errors.CorruptObjectException;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@@ -578,7 +579,7 @@ public class GC {
 				ret.add(rest);
 		}
 		if (!txnHeads.isEmpty()) {
-			PackFile txn = writePack(txnHeads, null, null, excluded);
+			PackFile txn = writePack(txnHeads, PackWriter.NONE, null, excluded);
 			if (txn != null)
 				ret.add(txn);
 		}
@@ -698,8 +699,8 @@ public class GC {
 		}
 	}
 
-	private PackFile writePack(Set<? extends ObjectId> want,
-			Set<? extends ObjectId> have, Set<ObjectId> tagTargets,
+	private PackFile writePack(@NonNull Set<? extends ObjectId> want,
+			@NonNull Set<? extends ObjectId> have, Set<ObjectId> tagTargets,
 			List<ObjectIdSet> excludeObjects) throws IOException {
 		File tmpPack = null;
 		Map<PackExt, File> tmpExts = new TreeMap<PackExt, File>(
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
index 763f35c21a..525f9aecc7 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
@@ -80,6 +80,7 @@ import java.util.zip.CheckedOutputStream;
 import java.util.zip.Deflater;
 import java.util.zip.DeflaterOutputStream;
 
+import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.errors.CorruptObjectException;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.LargeObjectException;
@@ -162,6 +163,9 @@ import org.eclipse.jgit.util.TemporaryBuffer;
 public class PackWriter implements AutoCloseable {
 	private static final int PACK_VERSION_GENERATED = 2;
 
+	/** Empty set of objects for {@code preparePack()}. */
+	public static Set<ObjectId> NONE = Collections.emptySet();
+
 	private static final Map<WeakReference<PackWriter>, Boolean> instances =
 			new ConcurrentHashMap<WeakReference<PackWriter>, Boolean>();
 
@@ -670,7 +674,7 @@ public class PackWriter implements AutoCloseable {
 	 * @throws IOException
 	 *             when some I/O problem occur during reading objects.
 	 */
-	public void preparePack(final Iterator<RevObject> objectsSource)
+	public void preparePack(@NonNull Iterator<RevObject> objectsSource)
 			throws IOException {
 		while (objectsSource.hasNext()) {
 			addObject(objectsSource.next());
@@ -693,16 +697,18 @@ public class PackWriter implements AutoCloseable {
 	 *            progress during object enumeration.
 	 * @param want
 	 *            collection of objects to be marked as interesting (start
-	 *            points of graph traversal).
+	 *            points of graph traversal). Must not be {@code null}.
 	 * @param have
 	 *            collection of objects to be marked as uninteresting (end
-	 *            points of graph traversal).
+	 *            points of graph traversal). Pass {@link #NONE} if all objects
+	 *            reachable from {@code want} are desired, such as when serving
+	 *            a clone.
 	 * @throws IOException
 	 *             when some I/O problem occur during reading objects.
 	 */
 	public void preparePack(ProgressMonitor countingMonitor,
-			Set<? extends ObjectId> want,
-			Set<? extends ObjectId> have) throws IOException {
+			@NonNull Set<? extends ObjectId> want,
+			@NonNull Set<? extends ObjectId> have) throws IOException {
 		ObjectWalk ow;
 		if (shallowPack)
 			ow = new DepthWalk.ObjectWalk(reader, depth);
@@ -729,17 +735,19 @@ public class PackWriter implements AutoCloseable {
 	 *            ObjectWalk to perform enumeration.
 	 * @param interestingObjects
 	 *            collection of objects to be marked as interesting (start
-	 *            points of graph traversal).
+	 *            points of graph traversal). Must not be {@code null}.
 	 * @param uninterestingObjects
 	 *            collection of objects to be marked as uninteresting (end
-	 *            points of graph traversal).
+	 *            points of graph traversal). Pass {@link #NONE} if all objects
+	 *            reachable from {@code want} are desired, such as when serving
+	 *            a clone.
 	 * @throws IOException
 	 *             when some I/O problem occur during reading objects.
 	 */
 	public void preparePack(ProgressMonitor countingMonitor,
-			ObjectWalk walk,
-			final Set<? extends ObjectId> interestingObjects,
-			final Set<? extends ObjectId> uninterestingObjects)
+			@NonNull ObjectWalk walk,
+			@NonNull Set<? extends ObjectId> interestingObjects,
+			@NonNull Set<? extends ObjectId> uninterestingObjects)
 			throws IOException {
 		if (countingMonitor == null)
 			countingMonitor = NullProgressMonitor.INSTANCE;
@@ -1597,17 +1605,12 @@ public class PackWriter implements AutoCloseable {
 		out.write(packcsum);
 	}
 
-	private void findObjectsToPack(final ProgressMonitor countingMonitor,
-			final ObjectWalk walker, final Set<? extends ObjectId> want,
-			Set<? extends ObjectId> have)
-			throws MissingObjectException, IOException,
-			IncorrectObjectTypeException {
+	private void findObjectsToPack(@NonNull ProgressMonitor countingMonitor,
+			@NonNull ObjectWalk walker, @NonNull Set<? extends ObjectId> want,
+			@NonNull Set<? extends ObjectId> have) throws IOException {
 		final long countingStart = System.currentTimeMillis();
 		beginPhase(PackingPhase.COUNTING, countingMonitor, ProgressMonitor.UNKNOWN);
 
-		if (have == null)
-			have = Collections.emptySet();
-
 		stats.interestingObjects = Collections.unmodifiableSet(new HashSet<ObjectId>(want));
 		stats.uninterestingObjects = Collections.unmodifiableSet(new HashSet<ObjectId>(have));