Use a new RevWalk for validating not advertised wants

Shadow commits in the RevWalk in the UploadPack object may cause the
UNINTERESTING flag not being carried over to their parents commits since
they were marked NO_PARENTS during the assumeShallow or
initializeShallowCommits call.

A new RevWalk needs to be created for this reason, but instead of
creating a new RevWalk from Repository, we can reuse the ObjectReader in
the RevWalk of UploadPack to load objects.

Change-Id: Ic3fee0512d35b4f555c60e696a880f8b192e4439
Signed-off-by: Zhen Chen <czhen@google.com>
This commit is contained in:
Zhen Chen 2017-10-04 14:02:24 -07:00
parent a2ec6ccf0a
commit 65f9046547
2 changed files with 118 additions and 27 deletions

View File

@ -0,0 +1,90 @@
package org.eclipse.jgit.transport;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.util.Collections;
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.UploadPack.RequestPolicy;
import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
import org.eclipse.jgit.transport.resolver.UploadPackFactory;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
/**
* Tests for server upload-pack utilities.
*/
public class UploadPackTest {
private URIish uri;
private TestProtocol<Object> testProtocol;
private Object ctx = new Object();
private InMemoryRepository server;
private InMemoryRepository client;
private RevCommit commit0;
private RevCommit commit1;
private RevCommit tip;
@Before
public void setUp() throws Exception {
server = newRepo("server");
client = newRepo("client");
TestRepository<InMemoryRepository> remote =
new TestRepository<>(server);
commit0 = remote.commit().message("0").create();
commit1 = remote.commit().message("1").parent(commit0).create();
tip = remote.commit().message("2").parent(commit1).create();
remote.update("master", tip);
}
@After
public void tearDown() {
Transport.unregister(testProtocol);
}
private static InMemoryRepository newRepo(String name) {
return new InMemoryRepository(new DfsRepositoryDescription(name));
}
@Test
public void testFetchParentOfShallowCommit() throws Exception {
testProtocol = new TestProtocol<>(
new UploadPackFactory<Object>() {
@Override
public UploadPack create(Object req, Repository db)
throws ServiceNotEnabledException,
ServiceNotAuthorizedException {
UploadPack up = new UploadPack(db);
up.setRequestPolicy(RequestPolicy.REACHABLE_COMMIT);
// assume client has a shallow commit
up.getRevWalk().assumeShallow(
Collections.singleton(commit1.getId()));
return up;
}
}, null);
uri = testProtocol.register(ctx, server);
assertFalse(client.hasObject(commit0.toObjectId()));
// Fetch of the parent of the shallow commit
try (Transport tn = testProtocol.open(uri, client, "server")) {
tn.fetch(NullProgressMonitor.INSTANCE,
Collections.singletonList(new RefSpec(commit0.name())));
assertTrue(client.hasObject(commit0.toObjectId()));
}
}
}

View File

@ -71,7 +71,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.errors.CorruptObjectException;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
@ -1261,7 +1260,7 @@ public static final class ReachableCommitRequestValidator
@Override
public void checkWants(UploadPack up, List<ObjectId> wants)
throws PackProtocolException, IOException {
checkNotAdvertisedWants(up.getRevWalk(), wants,
checkNotAdvertisedWants(up, wants,
refIdSet(up.getAdvertisedRefs().values()));
}
}
@ -1298,7 +1297,7 @@ public static final class ReachableCommitTipRequestValidator
@Override
public void checkWants(UploadPack up, List<ObjectId> wants)
throws PackProtocolException, IOException {
checkNotAdvertisedWants(up.getRevWalk(), wants,
checkNotAdvertisedWants(up, wants,
refIdSet(up.getRepository().getRefDatabase().getRefs(ALL).values()));
}
}
@ -1316,7 +1315,7 @@ public void checkWants(UploadPack up, List<ObjectId> wants)
}
}
private static void checkNotAdvertisedWants(RevWalk walk,
private static void checkNotAdvertisedWants(UploadPack up,
List<ObjectId> notAdvertisedWants, Set<ObjectId> reachableFrom)
throws MissingObjectException, IncorrectObjectTypeException, IOException {
// Walk the requested commits back to the provided set of commits. If any
@ -1325,32 +1324,34 @@ private static void checkNotAdvertisedWants(RevWalk walk,
// into an advertised branch it will be marked UNINTERESTING and no commits
// return.
AsyncRevObjectQueue q = walk.parseAny(notAdvertisedWants, true);
try {
RevObject obj;
while ((obj = q.next()) != null) {
if (!(obj instanceof RevCommit))
throw new WantNotValidException(obj);
walk.markStart((RevCommit) obj);
}
} catch (MissingObjectException notFound) {
throw new WantNotValidException(notFound.getObjectId(), notFound);
} finally {
q.release();
}
for (ObjectId id : reachableFrom) {
try (RevWalk walk = new RevWalk(up.getRevWalk().getObjectReader())) {
AsyncRevObjectQueue q = walk.parseAny(notAdvertisedWants, true);
try {
walk.markUninteresting(walk.parseCommit(id));
} catch (IncorrectObjectTypeException notCommit) {
continue;
RevObject obj;
while ((obj = q.next()) != null) {
if (!(obj instanceof RevCommit))
throw new WantNotValidException(obj);
walk.markStart((RevCommit) obj);
}
} catch (MissingObjectException notFound) {
throw new WantNotValidException(notFound.getObjectId(),
notFound);
} finally {
q.release();
}
for (ObjectId id : reachableFrom) {
try {
walk.markUninteresting(walk.parseCommit(id));
} catch (IncorrectObjectTypeException notCommit) {
continue;
}
}
RevCommit bad = walk.next();
if (bad != null) {
throw new WantNotValidException(bad);
}
}
RevCommit bad = walk.next();
if (bad != null) {
throw new WantNotValidException(bad);
}
walk.reset();
}
private void addCommonBase(final RevObject o) {