From ac72eca77b6f4a02e26fa0ed7fc535a5c45cf434 Mon Sep 17 00:00:00 2001 From: Nasser Grainawi Date: Fri, 24 May 2019 09:51:52 -0600 Subject: [PATCH] RefAdvertisementIT: Add test for non-commit tag Some tags created with long ago git versions (like those used in the kernel project) point to trees instead of commits. Add a tag that points to a tree to ensure we correctly advertise these tags when all refs are visible. All other tests show that we do not currently advertise these tags when not all refs are visible and ensure that we don't have any errors when we encounter them. Change-Id: Id7a1adf443b72c3cfe3cac34ce58533f52f80c8f --- .../acceptance/git/RefAdvertisementIT.java | 57 ++++++++++++++++++- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java index 4f55046fc1..891d6409cb 100644 --- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java +++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java @@ -74,6 +74,7 @@ import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Before; import org.junit.Test; @@ -89,6 +90,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest { private AccountGroup.UUID nonInteractiveUsers; private ChangeData cd1; + private RevCommit rc1; private String psRef1; private String metaRef1; @@ -156,6 +158,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest { pushFactory.create(admin.newIdent(), testRepo).to("refs/for/master%submit"); mr.assertOkStatus(); cd1 = mr.getChange(); + rc1 = mr.getCommit(); psRef1 = cd1.currentPatchSet().id().toRefName(); metaRef1 = RefNames.changeMetaRef(cd1.getId()); PushOneCommit.Result br = @@ -189,10 +192,18 @@ public class RefAdvertisementIT extends AbstractDaemonTest { btu.setExpectedOldObjectId(ObjectId.zeroId()); btu.setNewObjectId(repo.exactRef("refs/heads/branch").getObjectId()); assertThat(btu.update()).isEqualTo(RefUpdate.Result.NEW); + + // Create a tag for the tree of the commit on 'master' + // tree-tag -> master.tree + RefUpdate ttu = repo.updateRef("refs/tags/tree-tag"); + ttu.setExpectedOldObjectId(ObjectId.zeroId()); + ttu.setNewObjectId(rc1.getTree().toObjectId()); + assertThat(ttu.update()).isEqualTo(RefUpdate.Result.NEW); } } @Test + @GerritConfig(name = "auth.skipFullRefEvaluationIfAllRefsAreVisible", value = "false") public void uploadPackAllRefsVisibleNoRefsMetaConfig() throws Exception { projectOperations .project(project) @@ -217,6 +228,36 @@ public class RefAdvertisementIT extends AbstractDaemonTest { "refs/heads/master", "refs/tags/branch-tag", "refs/tags/master-tag"); + // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead. + } + + @Test + @GerritConfig(name = "auth.skipFullRefEvaluationIfAllRefsAreVisible", value = "true") + public void uploadPackAllRefsVisibleNoRefsMetaConfigSkipFullRefEval() throws Exception { + projectOperations + .project(project) + .forUpdate() + .add(allow(Permission.READ).ref("refs/*").group(REGISTERED_USERS)) + .add(allow(Permission.READ).ref(RefNames.REFS_CONFIG).group(admins)) + .setExclusiveGroup(permissionKey(Permission.READ).ref(RefNames.REFS_CONFIG), true) + .update(); + + requestScopeOperations.setApiUser(user.id()); + assertUploadPackRefs( + "HEAD", + psRef1, + metaRef1, + psRef2, + metaRef2, + psRef3, + metaRef3, + psRef4, + metaRef4, + "refs/heads/branch", + "refs/heads/master", + "refs/tags/branch-tag", + "refs/tags/master-tag", + "refs/tags/tree-tag"); } @Test @@ -242,7 +283,8 @@ public class RefAdvertisementIT extends AbstractDaemonTest { "refs/heads/master", RefNames.REFS_CONFIG, "refs/tags/branch-tag", - "refs/tags/master-tag"); + "refs/tags/master-tag", + "refs/tags/tree-tag"); } @Test @@ -257,6 +299,8 @@ public class RefAdvertisementIT extends AbstractDaemonTest { requestScopeOperations.setApiUser(user.id()); assertUploadPackRefs( "HEAD", psRef1, metaRef1, psRef3, metaRef3, "refs/heads/master", "refs/tags/master-tag"); + // tree-tag is not visible because we don't look at trees reachable from + // refs } @Test @@ -279,6 +323,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest { // master branch is not visible but master-tag is reachable from branch // (since PushOneCommit always bases changes on each other). "refs/tags/master-tag"); + // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead. } @Test @@ -306,6 +351,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest { "refs/heads/master", "refs/tags/master-tag", "refs/users/01/1000001/edit-" + cd3.getId() + "/1"); + // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead. } @Test @@ -338,6 +384,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest { "refs/tags/master-tag", "refs/users/00/1000000/edit-" + cd3.getId() + "/1", "refs/users/01/1000001/edit-" + cd3.getId() + "/1"); + // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead. } @Test @@ -374,6 +421,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest { "refs/tags/master-tag", // All edits are visible due to accessDatabase capability. "refs/users/00/1000000/edit-" + cd3.getId() + "/1"); + // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead. } @Test @@ -413,6 +461,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest { "refs/heads/master", "refs/tags/branch-tag", "refs/tags/master-tag"); + // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead. } @Test @@ -449,7 +498,8 @@ public class RefAdvertisementIT extends AbstractDaemonTest { metaRef3, "refs/heads/master", "refs/tags/branch-tag", - "refs/tags/master-tag"); + "refs/tags/master-tag", + "refs/tags/tree-tag"); } @Test @@ -463,7 +513,8 @@ public class RefAdvertisementIT extends AbstractDaemonTest { "refs/heads/master", "refs/meta/config", "refs/tags/branch-tag", - "refs/tags/master-tag"); + "refs/tags/master-tag", + "refs/tags/tree-tag"); assertThat(r.additionalHaves()).containsExactly(obj(cd3, 1), obj(cd4, 1)); }