Fix ReceivePack connectivity validation with alternates

If a repository has an alternate object database, the alternate has
its references advertised as ".have" lines, which permits the client
to use these as delta base candidates when generating the pack.  If
setCheckReferencedObjectsAreReachable(true) is used, these additional
have lines need to be considered in addition to the advertised refs.

Change-Id: Ie39c6696f9d3ff147ef4405cd5624f6011700ce5
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

2 files changed, 29 insertions, 24 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
index 324e5604d6..e2ab5f6780 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
@@ -166,6 +166,9 @@ public class ReceivePack {
 	/** The refs we advertised as existing at the start of the connection. */
 	private Map<String, Ref> refs;
 
+	/** All SHA-1s shown to the client, which can be possible edges. */
+	private Set<ObjectId> advertisedHaves;
+
 	/** Capabilities requested by the client. */
 	private Set<String> enabledCapablities;
 
@@ -208,6 +211,7 @@ public class ReceivePack {
 		refFilter = RefFilter.DEFAULT;
 		preReceive = PreReceiveHook.NULL;
 		postReceive = PostReceiveHook.NULL;
+		advertisedHaves = new HashSet<ObjectId>();
 	}
 
 	private static class ReceiveConfig {
@@ -251,9 +255,28 @@ public class ReceivePack {
 
 	/** @return all refs which were advertised to the client. */
 	public final Map<String, Ref> getAdvertisedRefs() {
+		if (refs == null) {
+			refs = refFilter.filter(db.getAllRefs());
+
+			Ref head = refs.get(Constants.HEAD);
+			if (head != null && head.isSymbolic())
+				refs.remove(Constants.HEAD);
+
+			for (Ref ref : refs.values()) {
+				if (ref.getObjectId() != null)
+					advertisedHaves.add(ref.getObjectId());
+			}
+			advertisedHaves.addAll(db.getAdditionalHaves());
+		}
 		return refs;
 	}
 
+	/** @return the set of objects advertised as present in this repository. */
+	public final Set<ObjectId> getAdvertisedObjects() {
+		getAdvertisedRefs();
+		return advertisedHaves;
+	}
+
 	/**
 	 * @return true if this instance will validate all referenced, but not
 	 *         supplied by the client, objects are reachable from another
@@ -629,7 +652,7 @@ public class ReceivePack {
 			sendAdvertisedRefs(new PacketLineOutRefAdvertiser(pckOut));
 			pckOut.flush();
 		} else
-			refs = refFilter.filter(db.getAllRefs());
+			getAdvertisedRefs();
 		if (advertiseError != null)
 			return;
 		recvCommands();
@@ -707,12 +730,9 @@ public class ReceivePack {
 		adv.advertiseCapability(CAPABILITY_REPORT_STATUS);
 		if (allowOfsDelta)
 			adv.advertiseCapability(CAPABILITY_OFS_DELTA);
-		refs = refFilter.filter(db.getAllRefs());
-		final Ref head = refs.remove(Constants.HEAD);
-		adv.send(refs);
-		if (head != null && !head.isSymbolic())
-			adv.advertiseHave(head.getObjectId());
-		adv.includeAdditionalHaves(db);
+		adv.send(getAdvertisedRefs());
+		for (ObjectId obj : advertisedHaves)
+			adv.advertiseHave(obj);
 		if (adv.isEmpty())
 			adv.advertiseId(ObjectId.zeroId(), "capabilities^{}");
 		adv.end();
@@ -847,8 +867,8 @@ public class ReceivePack {
 				continue;
 			ow.markStart(ow.parseAny(cmd.getNewId()));
 		}
-		for (final Ref ref : refs.values()) {
-			RevObject o = ow.parseAny(ref.getObjectId());
+		for (final ObjectId have : advertisedHaves) {
+			RevObject o = ow.parseAny(have);
 			ow.markUninteresting(o);
 
 			if (checkReferencedIsReachable && !baseObjects.isEmpty()) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java
index a77910bb9e..b04311659f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java
@@ -133,7 +133,6 @@ public abstract class RefAdvertiser {
 	 * <ul>
 	 * <li>{@link #send(Map)}
 	 * <li>{@link #advertiseHave(AnyObjectId)}
-	 * <li>{@link #includeAdditionalHaves(Repository)}
 	 * </ul>
 	 *
 	 * @param name
@@ -205,20 +204,6 @@ public abstract class RefAdvertiser {
 		advertiseAnyOnce(id, ".have");
 	}
 
-	/**
-	 * Include references of alternate repositories as {@code .have} lines.
-	 *
-	 * @param src
-	 *            repository to get the additional reachable objects from.
-	 * @throws IOException
-	 *             the underlying output stream failed to write out an
-	 *             advertisement record.
-	 */
-	public void includeAdditionalHaves(Repository src) throws IOException {
-		for (ObjectId id : src.getAdditionalHaves())
-			advertiseHave(id);
-	}
-
 	/** @return true if no advertisements have been sent yet. */
 	public boolean isEmpty() {
 		return first;