UploadPackTest: refactor capability config test

UploadPackTest.java contains tests that check behavior when
"allowfilter" and "allowrefinwant" are not set, are set, and are not set
but the client insists on using them anyway. Because another capability
is to be included in a subsequent patch, refactor the common code in
these tests.

Remove setBoolean calls with "false", as they are no-ops.

Also take the opportunity to eliminate the overspecification of the
"fetch=" line returned by the capability advertisement.

Change-Id: I289bbd11c902a513cd8d53bc34767e61ebbd5f17
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
This commit is contained in:
Jonathan Tan 2019-01-22 13:17:12 -08:00
parent 848d9f0d71
commit e5f7d5a136
1 changed files with 62 additions and 51 deletions

View File

@ -1,6 +1,7 @@
package org.eclipse.jgit.transport;
import static org.eclipse.jgit.lib.MoreAsserts.assertThrows;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.is;
@ -531,54 +532,66 @@ public void testV2Capabilities() throws Exception {
assertTrue(PacketLineIn.isEnd(pckIn.readString()));
}
@Test
public void testV2CapabilitiesAllowFilter() throws Exception {
server.getConfig().setBoolean("uploadpack", null, "allowfilter", true);
private void checkAdvertisedIfAllowed(String configSection, String configName,
String fetchCapability) throws Exception {
server.getConfig().setBoolean(configSection, null, configName, true);
ByteArrayInputStream recvStream =
uploadPackV2Setup(null, null, null, PacketLineIn.end());
PacketLineIn pckIn = new PacketLineIn(recvStream);
assertThat(pckIn.readString(), is("version 2"));
assertThat(
Arrays.asList(pckIn.readString(), pckIn.readString(),
pckIn.readString()),
// TODO(jonathantanmy) This check overspecifies the
// order of the capabilities of "fetch".
hasItems("ls-refs", "fetch=filter shallow", "server-option"));
assertTrue(PacketLineIn.isEnd(pckIn.readString()));
ArrayList<String> lines = new ArrayList<>();
String line;
while (!PacketLineIn.isEnd((line = pckIn.readString()))) {
if (line.startsWith("fetch=")) {
assertThat(
Arrays.asList(line.substring(6).split(" ")),
containsInAnyOrder(fetchCapability, "shallow"));
lines.add("fetch");
} else {
lines.add(line);
}
}
assertThat(lines, containsInAnyOrder("ls-refs", "fetch", "server-option"));
}
private void checkUnadvertisedIfUnallowed(String fetchCapability) throws Exception {
ByteArrayInputStream recvStream =
uploadPackV2Setup(null, null, null, PacketLineIn.end());
PacketLineIn pckIn = new PacketLineIn(recvStream);
assertThat(pckIn.readString(), is("version 2"));
ArrayList<String> lines = new ArrayList<>();
String line;
while (!PacketLineIn.isEnd((line = pckIn.readString()))) {
if (line.startsWith("fetch=")) {
assertThat(
Arrays.asList(line.substring(6).split(" ")),
hasItems("shallow"));
lines.add("fetch");
} else {
lines.add(line);
}
}
assertThat(lines, hasItems("ls-refs", "fetch", "server-option"));
}
@Test
public void testV2CapabilitiesAllowFilter() throws Exception {
checkAdvertisedIfAllowed("uploadpack", "allowfilter", "filter");
checkUnadvertisedIfUnallowed("filter");
}
@Test
public void testV2CapabilitiesRefInWant() throws Exception {
server.getConfig().setBoolean("uploadpack", null, "allowrefinwant", true);
ByteArrayInputStream recvStream =
uploadPackV2Setup(null, null, null, PacketLineIn.end());
PacketLineIn pckIn = new PacketLineIn(recvStream);
assertThat(pckIn.readString(), is("version 2"));
assertThat(
Arrays.asList(pckIn.readString(), pckIn.readString(),
pckIn.readString()),
// TODO(jonathantanmy) This check overspecifies the
// order of the capabilities of "fetch".
hasItems("ls-refs", "fetch=ref-in-want shallow",
"server-option"));
assertTrue(PacketLineIn.isEnd(pckIn.readString()));
checkAdvertisedIfAllowed("uploadpack", "allowrefinwant", "ref-in-want");
}
@Test
public void testV2CapabilitiesRefInWantNotAdvertisedIfUnallowed() throws Exception {
server.getConfig().setBoolean("uploadpack", null, "allowrefinwant", false);
ByteArrayInputStream recvStream =
uploadPackV2Setup(null, null, null, PacketLineIn.end());
PacketLineIn pckIn = new PacketLineIn(recvStream);
assertThat(pckIn.readString(), is("version 2"));
assertThat(
Arrays.asList(pckIn.readString(), pckIn.readString(),
pckIn.readString()),
hasItems("ls-refs", "fetch=shallow", "server-option"));
assertTrue(PacketLineIn.isEnd(pckIn.readString()));
checkUnadvertisedIfUnallowed("ref-in-want");
}
@Test
@ -1833,34 +1846,32 @@ public void testWantFilteredObject() throws Exception {
.has(preparator.subtree3.toObjectId()));
}
@Test
public void testV2FetchFilterWhenNotAllowed() throws Exception {
private void checkV2FetchWhenNotAllowed(String fetchLine, String expectedMessage)
throws Exception {
RevCommit commit = remote.commit().message("0").create();
remote.update("master", commit);
server.getConfig().setBoolean("uploadpack", null, "allowfilter", false);
UploadPackInternalServerErrorException e = assertThrows(
UploadPackInternalServerErrorException.class,
() -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(),
"want " + commit.toObjectId().getName() + "\n",
"filter blob:limit=5\n", "done\n", PacketLineIn.end()));
fetchLine, "done\n", PacketLineIn.end()));
assertThat(e.getCause().getMessage(),
containsString("unexpected filter blob:limit=5"));
containsString(expectedMessage));
}
@Test
public void testV2FetchFilterWhenNotAllowed() throws Exception {
checkV2FetchWhenNotAllowed(
"filter blob:limit=5\n",
"unexpected filter blob:limit=5");
}
@Test
public void testV2FetchWantRefIfNotAllowed() throws Exception {
RevCommit one = remote.commit().message("1").create();
remote.update("one", one);
UploadPackInternalServerErrorException e = assertThrows(
UploadPackInternalServerErrorException.class,
() -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(),
"want-ref refs/heads/one\n", "done\n",
PacketLineIn.end()));
assertThat(e.getCause().getMessage(),
containsString("unexpected want-ref refs/heads/one"));
checkV2FetchWhenNotAllowed(
"want-ref refs/heads/one\n",
"unexpected want-ref refs/heads/one");
}
@Test