Merge branch 'stable-5.0' into stable-5.1

* stable-5.0:
  Call AdvertiseRefsHook for protocol v2
  Prepare 4.11.7-SNAPSHOT builds
  JGit v4.11.6.201812241910-r
  Prepare 4.9.9-SNAPSHOT builds
  JGit v4.9.8.201812241815-r
  UploadPack: Test filtering by AdvertiseRefsHook in stateless transports
  Prepare 4.7.8-SNAPSHOT builds
  JGit v4.7.7.201812240805-r
  Fix feature versions imported by feature org.eclipse.jgit.pgm
  Prepare 4.5.6-SNAPSHOT builds
  JGit v4.5.5.201812240535-r
  Call AdvertiseRefsHook before validating wants

Change-Id: Icdc212bf5be2485d0f8028acf6c62fb8531d0e3c
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.http.test/META-INF/MANIFEST.MF

@@ -50,5 +50,7 @@ Import-Package: javax.servlet;version="[2.5.0,3.2.0)",
  org.hamcrest;version="[1.1.0,2.0.0)",
  org.hamcrest.core;version="[1.1.0,2.0.0)",
  org.junit;version="[4.12,5.0.0)",
+ org.junit.rules;version="[4.12,5.0.0)",
  org.junit.runner;version="[4.12,5.0.0)",
  org.junit.runners;version="[4.12,5.0.0)"
+Require-Bundle: org.hamcrest.library;bundle-version="[1.1.0,2.0.0)"

org.eclipse.jgit.http.test/pom.xml

@@ -70,6 +70,13 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <groupId>org.hamcrest</groupId>
+      <artifactId>hamcrest-library</artifactId>
+      <scope>test</scope>
+      <version>[1.1.0,2.0.0)</version>
+    </dependency>
+
     <dependency>
       <groupId>org.eclipse.jgit</groupId>
       <artifactId>org.eclipse.jgit</artifactId>
@@ -83,13 +90,6 @@
       <scope>test</scope>
     </dependency>
 
-    <dependency>
-      <groupId>org.hamcrest</groupId>
-      <artifactId>hamcrest-library</artifactId>
-      <scope>test</scope>
-      <version>[1.1.0,2.0.0)</version>
-    </dependency>
-
     <dependency>
       <groupId>org.eclipse.jgit</groupId>
       <artifactId>org.eclipse.jgit.junit.http</artifactId>

org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java

@@ -85,6 +85,7 @@ import org.eclipse.jgit.errors.RemoteRepositoryException;
 import org.eclipse.jgit.errors.TransportException;
 import org.eclipse.jgit.errors.UnsupportedCredentialItem;
 import org.eclipse.jgit.http.server.GitServlet;
+import org.eclipse.jgit.http.server.resolver.DefaultUploadPackFactory;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
 import org.eclipse.jgit.junit.TestRepository;
@@ -105,24 +106,35 @@ import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.revwalk.RevBlob;
 import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.transport.AbstractAdvertiseRefsHook;
+import org.eclipse.jgit.transport.AdvertiseRefsHook;
 import org.eclipse.jgit.transport.CredentialItem;
 import org.eclipse.jgit.transport.CredentialsProvider;
 import org.eclipse.jgit.transport.FetchConnection;
 import org.eclipse.jgit.transport.HttpTransport;
+import org.eclipse.jgit.transport.RefSpec;
 import org.eclipse.jgit.transport.RemoteRefUpdate;
 import org.eclipse.jgit.transport.Transport;
 import org.eclipse.jgit.transport.TransportHttp;
 import org.eclipse.jgit.transport.URIish;
+import org.eclipse.jgit.transport.UploadPack;
 import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
 import org.eclipse.jgit.transport.http.HttpConnectionFactory;
 import org.eclipse.jgit.transport.http.JDKHttpConnectionFactory;
 import org.eclipse.jgit.transport.http.apache.HttpClientConnectionFactory;
+import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
+import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
+import org.eclipse.jgit.transport.resolver.UploadPackFactory;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.HttpSupport;
 import org.eclipse.jgit.util.SystemReader;
+import org.hamcrest.Matchers;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameters;
@@ -131,6 +143,11 @@ import org.junit.runners.Parameterized.Parameters;
 public class SmartClientSmartServerTest extends HttpTestCase {
 	private static final String HDR_TRANSFER_ENCODING = "Transfer-Encoding";
 
+	@Rule
+	public ExpectedException thrown = ExpectedException.none();
+
+	private AdvertiseRefsHook advertiseRefsHook;
+
 	private Repository remoteRepository;
 
 	private CredentialsProvider testCredentials = new UsernamePasswordCredentialsProvider(
@@ -148,7 +165,7 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 
 	private RevBlob A_txt;
 
-	private RevCommit A, B;
+	private RevCommit A, B, unreachableCommit;
 
 	@Parameters
 	public static Collection<Object[]> data() {
@@ -175,6 +192,19 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 						ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, true);
 
 		GitServlet gs = new GitServlet();
+		gs.setUploadPackFactory(new UploadPackFactory<HttpServletRequest>() {
+			@Override
+			public UploadPack create(HttpServletRequest req, Repository db)
+					throws ServiceNotEnabledException,
+					ServiceNotAuthorizedException {
+				DefaultUploadPackFactory f = new DefaultUploadPackFactory();
+				UploadPack up = f.create(req, db);
+				if (advertiseRefsHook != null) {
+					up.setAdvertiseRefsHook(advertiseRefsHook);
+				}
+				return up;
+			}
+		});
 
 		ServletContextHandler app = addNormalContext(gs, src, srcName);
 
@@ -200,6 +230,8 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 		B = src.commit().parent(A).add("A_txt", "C").add("B", "B").create();
 		src.update(master, B);
 
+		unreachableCommit = src.commit().add("A_txt", A_txt).create();
+
 		src.update("refs/garbage/a/very/long/ref/name/to/compress", B);
 	}
 
@@ -452,6 +484,56 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 				info.getResponseHeader(HDR_CONTENT_TYPE));
 	}
 
+	@Test
+	public void testFetchBySHA1() throws Exception {
+		Repository dst = createBareRepository();
+		assertFalse(dst.hasObject(A_txt));
+
+		try (Transport t = Transport.open(dst, remoteURI)) {
+			t.fetch(NullProgressMonitor.INSTANCE,
+					Collections.singletonList(new RefSpec(B.name())));
+		}
+
+		assertTrue(dst.hasObject(A_txt));
+	}
+
+	@Test
+	public void testFetchBySHA1Unreachable() throws Exception {
+		Repository dst = createBareRepository();
+		assertFalse(dst.hasObject(A_txt));
+
+		try (Transport t = Transport.open(dst, remoteURI)) {
+			thrown.expect(TransportException.class);
+			thrown.expectMessage(Matchers.containsString(
+					"want " + unreachableCommit.name() + " not valid"));
+			t.fetch(NullProgressMonitor.INSTANCE, Collections
+					.singletonList(new RefSpec(unreachableCommit.name())));
+		}
+	}
+
+	@Test
+	public void testFetchBySHA1UnreachableByAdvertiseRefsHook()
+			throws Exception {
+		Repository dst = createBareRepository();
+		assertFalse(dst.hasObject(A_txt));
+
+		advertiseRefsHook = new AbstractAdvertiseRefsHook() {
+			@Override
+			protected Map<String, Ref> getAdvertisedRefs(Repository repository,
+					RevWalk revWalk) {
+				return Collections.emptyMap();
+			}
+		};
+
+		try (Transport t = Transport.open(dst, remoteURI)) {
+			thrown.expect(TransportException.class);
+			thrown.expectMessage(Matchers.containsString(
+					"want " + A.name() + " not valid"));
+			t.fetch(NullProgressMonitor.INSTANCE, Collections
+					.singletonList(new RefSpec(A.name())));
+		}
+	}
+
 	@Test
 	public void testInitialClone_Small() throws Exception {
 		Repository dst = createBareRepository();

org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java

@@ -43,8 +43,9 @@
 
 package org.eclipse.jgit.transport;
 
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toMap;
 import static org.eclipse.jgit.lib.Constants.R_TAGS;
-import static org.eclipse.jgit.lib.RefDatabase.ALL;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REF_IN_WANT;
 import static org.eclipse.jgit.transport.GitProtocolConstants.COMMAND_FETCH;
 import static org.eclipse.jgit.transport.GitProtocolConstants.COMMAND_LS_REFS;
@@ -262,7 +263,10 @@ public class UploadPack {
 
 	private OutputStream msgOut = NullOutputStream.INSTANCE;
 
-	/** The refs we advertised as existing at the start of the connection. */
+	/**
+	 * Refs eligible for advertising to the client, set using
+	 * {@link #setAdvertisedRefs}.
+	 */
 	private Map<String, Ref> refs;
 
 	/** Hook used while processing Git protocol v2 requests. */
@@ -271,6 +275,9 @@ public class UploadPack {
 	/** Hook used while advertising the refs to the client. */
 	private AdvertiseRefsHook advertiseRefsHook = AdvertiseRefsHook.DEFAULT;
 
+	/** Whether the {@link #advertiseRefsHook} has been invoked. */
+	private boolean advertiseRefsHookCalled;
+
 	/** Filter used while advertising the refs to the client. */
 	private RefFilter refFilter = RefFilter.DEFAULT;
 
@@ -793,11 +800,52 @@ public class UploadPack {
 	}
 
 	private Map<String, Ref> getAdvertisedOrDefaultRefs() throws IOException {
-		if (refs == null)
+		if (refs != null) {
-			setAdvertisedRefs(db.getRefDatabase().getRefs(ALL));
+			return refs;
+		}
+
+		advertiseRefsHook.advertiseRefs(this);
+		advertiseRefsHookCalled = true;
+		if (refs == null) {
+			// Fall back to all refs.
+			setAdvertisedRefs(
+					db.getRefDatabase().getRefs().stream()
+						.collect(toMap(Ref::getName, identity())));
+		}
 		return refs;
 	}
 
+	private Map<String, Ref> getFilteredRefs(Collection<String> refPrefixes)
+					throws IOException {
+		if (refPrefixes.isEmpty()) {
+			return getAdvertisedOrDefaultRefs();
+		}
+		if (refs == null && !advertiseRefsHookCalled) {
+			advertiseRefsHook.advertiseRefs(this);
+			advertiseRefsHookCalled = true;
+		}
+		if (refs == null) {
+			// Fast path: the advertised refs hook did not set advertised refs.
+			Map<String, Ref> rs = new HashMap<>();
+			for (String p : refPrefixes) {
+				for (Ref r : db.getRefDatabase().getRefsByPrefix(p)) {
+					rs.put(r.getName(), r);
+				}
+			}
+			if (refFilter != RefFilter.DEFAULT) {
+				return refFilter.filter(rs);
+			}
+			return transferConfig.getRefFilter().filter(rs);
+		}
+
+		// Slow path: filter the refs provided by the advertised refs hook.
+		// refFilter has already been applied to refs.
+		return refs.values().stream()
+				.filter(ref -> refPrefixes.stream()
+						.anyMatch(ref.getName()::startsWith))
+				.collect(toMap(Ref::getName, identity()));
+	}
+
 	private void service() throws IOException {
 		boolean sendPack = false;
 		// If it's a non-bidi request, we need to read the entire request before
@@ -923,17 +971,7 @@ public class UploadPack {
 		if (req.getPeel()) {
 			adv.setDerefTags(true);
 		}
-		Map<String, Ref> refsToSend;
+		Map<String, Ref> refsToSend = getFilteredRefs(req.getRefPrefixes());
-		if (req.getRefPrefixes().isEmpty()) {
-			refsToSend = getAdvertisedOrDefaultRefs();
-		} else {
-			refsToSend = new HashMap<>();
-			for (String refPrefix : req.getRefPrefixes()) {
-				for (Ref ref : db.getRefDatabase().getRefsByPrefix(refPrefix)) {
-					refsToSend.put(ref.getName(), ref);
-				}
-			}
-		}
 		if (req.getSymrefs()) {
 			findSymrefs(adv, refsToSend);
 		}
@@ -1281,15 +1319,7 @@ public class UploadPack {
 			return;
 		}
 
-		try {
+		Map<String, Ref> advertisedOrDefaultRefs = getAdvertisedOrDefaultRefs();
-			advertiseRefsHook.advertiseRefs(this);
-		} catch (ServiceMayNotContinueException fail) {
-			if (fail.getMessage() != null) {
-				adv.writeOne("ERR " + fail.getMessage()); //$NON-NLS-1$
-				fail.setOutput();
-			}
-			throw fail;
-		}
 
 		if (serviceName != null) {
 			adv.writeOne("# service=" + serviceName + '\n'); //$NON-NLS-1$
@@ -1321,7 +1351,6 @@ public class UploadPack {
 			adv.advertiseCapability(OPTION_FILTER);
 		}
 		adv.setDerefTags(true);
-		Map<String, Ref> advertisedOrDefaultRefs = getAdvertisedOrDefaultRefs();
 		findSymrefs(adv, advertisedOrDefaultRefs);
 		advertised = adv.send(advertisedOrDefaultRefs);
 		if (adv.isEmpty())