Move ls-refs parsing code to the ProtocolV2Parser

Fetch code has been moved to a ProtocolV2Parser, but ls-refs code is
still in UploadPack.

Moving it to the parser makes it easier to test, keeps the parsing
together and makes the two commands follow similar structure.

Change-Id: I573ce543e804ddeb9f83303b4af250b7cddc8cad
Signed-off-by: Ivan Frade <ifrade@google.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java

@@ -45,6 +45,7 @@ package org.eclipse.jgit.transport;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasItems;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
@@ -333,4 +334,60 @@ public class ProtocolV2ParserTest {
 				testRepo.getRepository().getRefDatabase());
 	}
 
+	@Test
+	public void testLsRefsMinimalReq() throws IOException {
+		PacketLineIn pckIn = formatAsPacketLine(PacketLineIn.DELIM,
+				PacketLineIn.END);
+
+		ProtocolV2Parser parser = new ProtocolV2Parser(
+				ConfigBuilder.getDefault());
+		LsRefsV2Request req = parser.parseLsRefsRequest(pckIn);
+		assertFalse(req.getPeel());
+		assertFalse(req.getSymrefs());
+		assertEquals(0, req.getRefPrefixes().size());
+	}
+
+	@Test
+	public void testLsRefsSymrefs() throws IOException {
+		PacketLineIn pckIn = formatAsPacketLine(PacketLineIn.DELIM, "symrefs",
+				PacketLineIn.END);
+
+		ProtocolV2Parser parser = new ProtocolV2Parser(
+				ConfigBuilder.getDefault());
+		LsRefsV2Request req = parser.parseLsRefsRequest(pckIn);
+		assertFalse(req.getPeel());
+		assertTrue(req.getSymrefs());
+		assertEquals(0, req.getRefPrefixes().size());
+
+	}
+
+	@Test
+	public void testLsRefsPeel() throws IOException {
+		PacketLineIn pckIn = formatAsPacketLine(
+				PacketLineIn.DELIM,
+				"peel",
+				PacketLineIn.END);
+
+		ProtocolV2Parser parser = new ProtocolV2Parser(
+				ConfigBuilder.getDefault());
+		LsRefsV2Request req = parser.parseLsRefsRequest(pckIn);
+		assertTrue(req.getPeel());
+		assertFalse(req.getSymrefs());
+		assertEquals(0, req.getRefPrefixes().size());
+	}
+
+	@Test
+	public void testLsRefsRefPrefixes() throws IOException {
+		PacketLineIn pckIn = formatAsPacketLine(PacketLineIn.DELIM,
+				"ref-prefix refs/for", "ref-prefix refs/heads",
+				PacketLineIn.END);
+
+		ProtocolV2Parser parser = new ProtocolV2Parser(
+				ConfigBuilder.getDefault());
+		LsRefsV2Request req = parser.parseLsRefsRequest(pckIn);
+		assertFalse(req.getPeel());
+		assertFalse(req.getSymrefs());
+		assertEquals(2, req.getRefPrefixes().size());
+		assertThat(req.getRefPrefixes(), hasItems("refs/for", "refs/heads"));
+	}
 }

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

@@ -53,6 +53,8 @@ import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_WANT_REF;
 
 import java.io.IOException;
 import java.text.MessageFormat;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.eclipse.jgit.errors.PackProtocolException;
 import org.eclipse.jgit.internal.JGitText;
@@ -204,6 +206,51 @@ final class ProtocolV2Parser {
 	}
 
 	/**
+	 * Parse the incoming ls-refs request arguments from the wire. This is meant
+	 * for calling immediately after the caller has consumed a "command=ls-refs"
+	 * line indicating the beginning of a ls-refs request.
+	 *
+	 * The incoming PacketLineIn is consumed until an END line, but the caller
+	 * is responsible for closing it (if needed)
+	 *
+	 * @param pckIn
+	 *            incoming lines. This method will read until an END line.
+	 * @return a LsRefsV2Request object with the data received in the wire.
+	 * @throws PackProtocolException
+	 *             for inconsistencies in the protocol (e.g. unexpected lines)
+	 * @throws IOException
+	 *             reporting problems reading the incoming messages from the
+	 *             wire
+	 */
+	LsRefsV2Request parseLsRefsRequest(PacketLineIn pckIn)
+			throws PackProtocolException, IOException {
+		LsRefsV2Request.Builder builder = LsRefsV2Request.builder();
+		List<String> prefixes = new ArrayList<>();
+		String line = pckIn.readString();
+		// Currently, we do not support any capabilities, so the next
+		// line is DELIM if there are arguments or END if not.
+		if (line == PacketLineIn.DELIM) {
+			while ((line = pckIn.readString()) != PacketLineIn.END) {
+				if (line.equals("peel")) { //$NON-NLS-1$
+					builder.setPeel(true);
+				} else if (line.equals("symrefs")) { //$NON-NLS-1$
+					builder.setSymrefs(true);
+				} else if (line.startsWith("ref-prefix ")) { //$NON-NLS-1$
+					prefixes.add(line.substring("ref-prefix ".length())); //$NON-NLS-1$
+				} else {
+					throw new PackProtocolException(MessageFormat
+							.format(JGitText.get().unexpectedPacketLine, line));
+				}
+			}
+		} else if (line != PacketLineIn.END) {
+			throw new PackProtocolException(MessageFormat
+					.format(JGitText.get().unexpectedPacketLine, line));
+		}
+
+		return builder.setRefPrefixes(prefixes).build();
+	}
+
+	/*
 	 * Process the content of "filter" line from the protocol. It has a shape
 	 * like "blob:none" or "blob:limit=N", with limit a positive number.
 	 *

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

@@ -907,30 +907,8 @@ public class UploadPack {
 	}
 
 	private void lsRefsV2() throws IOException {
-		LsRefsV2Request.Builder builder = LsRefsV2Request.builder();
+		ProtocolV2Parser parser = new ProtocolV2Parser(transferConfig);
-		List<String> prefixes = new ArrayList<>();
+		LsRefsV2Request req = parser.parseLsRefsRequest(pckIn);
-		String line = pckIn.readString();
-		// Currently, we do not support any capabilities, so the next
-		// line is DELIM if there are arguments or END if not.
-		if (line == PacketLineIn.DELIM) {
-			while ((line = pckIn.readString()) != PacketLineIn.END) {
-				if (line.equals("peel")) { //$NON-NLS-1$
-					builder.setPeel(true);
-				} else if (line.equals("symrefs")) { //$NON-NLS-1$
-					builder.setSymrefs(true);
-				} else if (line.startsWith("ref-prefix ")) { //$NON-NLS-1$
-					prefixes.add(line.substring("ref-prefix ".length())); //$NON-NLS-1$
-				} else {
-					throw new PackProtocolException(MessageFormat
-							.format(JGitText.get().unexpectedPacketLine, line));
-				}
-			}
-		} else if (line != PacketLineIn.END) {
-			throw new PackProtocolException(MessageFormat
-					.format(JGitText.get().unexpectedPacketLine, line));
-		}
-		LsRefsV2Request req = builder.setRefPrefixes(prefixes).build();
-
 		protocolV2Hook.onLsRefs(req);
 
 		rawOut.stopBuffering();