From c668f4d46be1c71b63c0ae37f05e0ffae2354865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20=C5=BDivkov?= Date: Tue, 20 May 2014 16:59:51 +0200 Subject: [PATCH] Use allow/block/deny methods to improve test readability The readability of permission based tests is improved by introducing the utility methods: allow, block, deny. Instead of: grant(cfg, PUSH, REGISTERED_USERS, "refs/heads/*"); grant(cfg, FORGE_COMMITTER, ANONYMOUS_USERS, "refs/heads/*").setBlock(true); grant(cfg, READ, REGISTERED_USERS, "refs/*").setDeny(true); we now use: allow(cfg, PUSH, REGISTERED_USERS, "refs/heads/*"); block(cfg, FORGE_COMMITTER, ANONYMOUS_USERS, "refs/heads/*"); deny(cfg, READ, REGISTERED_USERS, "refs/*"); Usage of "allow/block/deny" also resembles the corresponding drop-down list from the permission editor UI. Change-Id: I0de1a9f7014fc2ff48bebff1ecc894c8b80ad682 --- .../acceptance/git/DraftChangeBlockedIT.java | 5 +- .../rest/project/CreateBranchIT.java | 51 +++--- .../rest/project/DeleteBranchIT.java | 51 +++--- .../rest/project/ListBranchesIT.java | 29 +-- .../server/project/CustomLabelIT.java | 4 +- .../google/gerrit/rules/GerritCommonTest.java | 6 +- .../server/git/LabelNormalizerTest.java | 12 +- .../gerrit/server/project/RefControlTest.java | 173 +++++++++--------- .../google/gerrit/server/project/Util.java | 29 ++- 9 files changed, 189 insertions(+), 171 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java index b69c26478b..20c8f76e73 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java @@ -15,7 +15,7 @@ package com.google.gerrit.acceptance.git; import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; -import static com.google.gerrit.server.project.Util.grant; +import static com.google.gerrit.server.project.Util.block; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; @@ -49,8 +49,7 @@ public class DraftChangeBlockedIT extends AbstractDaemonTest { @Before public void setUp() throws Exception { ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig(); - grant(cfg, Permission.PUSH, ANONYMOUS_USERS, - "refs/drafts/*").setBlock(); + block(cfg, Permission.PUSH, ANONYMOUS_USERS, "refs/drafts/*"); saveProjectConfig(cfg); projectCache.evict(cfg.getProject()); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java index a4bcdf21e8..23cd278494 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java @@ -14,18 +14,20 @@ package com.google.gerrit.acceptance.rest.project; +import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; +import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.server.project.Util.allow; +import static com.google.gerrit.server.project.Util.block; import static org.junit.Assert.assertEquals; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; -import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.Permission; -import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.client.Branch; -import com.google.gerrit.server.config.AllProjectsNameProvider; +import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; -import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.project.ProjectCache; import com.google.inject.Inject; @@ -44,7 +46,7 @@ public class CreateBranchIT extends AbstractDaemonTest { private ProjectCache projectCache; @Inject - private AllProjectsNameProvider allProjects; + private AllProjectsName allProjects; private Branch.NameKey branch; @@ -130,29 +132,26 @@ public class CreateBranchIT extends AbstractDaemonTest { } private void blockCreateReference() throws IOException, ConfigInvalidException { - MetaDataUpdate md = metaDataUpdateFactory.create(allProjects.get()); - md.setMessage(String.format("Block %s", Permission.CREATE)); - ProjectConfig config = ProjectConfig.read(md); - AccessSection s = config.getAccessSection("refs/*", true); - Permission p = s.getPermission(Permission.CREATE, true); - PermissionRule rule = new PermissionRule(config.resolve( - SystemGroupBackend.getGroup(SystemGroupBackend.ANONYMOUS_USERS))); - rule.setBlock(); - p.add(rule); - config.commit(md); - projectCache.evict(config.getProject()); + ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig(); + block(cfg, Permission.CREATE, ANONYMOUS_USERS, "refs/*"); + saveProjectConfig(allProjects, cfg); + projectCache.evict(cfg.getProject()); } private void grantOwner() throws IOException, ConfigInvalidException { - MetaDataUpdate md = metaDataUpdateFactory.create(project); - md.setMessage(String.format("Grant %s", Permission.OWNER)); - ProjectConfig config = ProjectConfig.read(md); - AccessSection s = config.getAccessSection("refs/*", true); - Permission p = s.getPermission(Permission.OWNER, true); - PermissionRule rule = new PermissionRule(config.resolve( - SystemGroupBackend.getGroup(SystemGroupBackend.REGISTERED_USERS))); - p.add(rule); - config.commit(md); - projectCache.evict(config.getProject()); + ProjectConfig cfg = projectCache.checkedGet(project).getConfig(); + allow(cfg, Permission.OWNER, REGISTERED_USERS, "refs/*"); + saveProjectConfig(project, cfg); + projectCache.evict(cfg.getProject()); + } + + private void saveProjectConfig(Project.NameKey p, ProjectConfig cfg) + throws IOException { + MetaDataUpdate md = metaDataUpdateFactory.create(p); + try { + cfg.commit(md); + } finally { + md.close(); + } } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java index 020fd43c10..0d6ec5b7fb 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java @@ -14,18 +14,20 @@ package com.google.gerrit.acceptance.rest.project; +import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; +import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.server.project.Util.allow; +import static com.google.gerrit.server.project.Util.block; import static org.junit.Assert.assertEquals; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; -import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.Permission; -import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.client.Branch; -import com.google.gerrit.server.config.AllProjectsNameProvider; +import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; -import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.project.ProjectCache; import com.google.inject.Inject; @@ -45,7 +47,7 @@ public class DeleteBranchIT extends AbstractDaemonTest { private ProjectCache projectCache; @Inject - private AllProjectsNameProvider allProjects; + private AllProjectsName allProjects; private Branch.NameKey branch; @@ -125,30 +127,25 @@ public class DeleteBranchIT extends AbstractDaemonTest { } private void blockForcePush() throws IOException, ConfigInvalidException { - MetaDataUpdate md = metaDataUpdateFactory.create(allProjects.get()); - md.setMessage(String.format("Block force %s", Permission.PUSH)); - ProjectConfig config = ProjectConfig.read(md); - AccessSection s = config.getAccessSection("refs/heads/*", true); - Permission p = s.getPermission(Permission.PUSH, true); - PermissionRule rule = new PermissionRule(config.resolve( - SystemGroupBackend.getGroup(SystemGroupBackend.ANONYMOUS_USERS))); - rule.setForce(true); - rule.setBlock(); - p.add(rule); - config.commit(md); - projectCache.evict(config.getProject()); + ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig(); + block(cfg, Permission.PUSH, ANONYMOUS_USERS, "refs/heads/*").setForce(true); + saveProjectConfig(allProjects, cfg); + projectCache.evict(cfg.getProject()); } private void grantOwner() throws IOException, ConfigInvalidException { - MetaDataUpdate md = metaDataUpdateFactory.create(project); - md.setMessage(String.format("Grant %s", Permission.OWNER)); - ProjectConfig config = ProjectConfig.read(md); - AccessSection s = config.getAccessSection("refs/*", true); - Permission p = s.getPermission(Permission.OWNER, true); - PermissionRule rule = new PermissionRule(config.resolve( - SystemGroupBackend.getGroup(SystemGroupBackend.REGISTERED_USERS))); - p.add(rule); - config.commit(md); - projectCache.evict(config.getProject()); + ProjectConfig cfg = projectCache.checkedGet(project).getConfig(); + allow(cfg, Permission.OWNER, REGISTERED_USERS, "refs/*"); + saveProjectConfig(project, cfg); + projectCache.evict(cfg.getProject()); + } + + private void saveProjectConfig(Project.NameKey p, ProjectConfig cfg) throws IOException { + MetaDataUpdate md = metaDataUpdateFactory.create(p); + try { + cfg.commit(md); + } finally { + md.close(); + } } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java index 52bb623a59..e6fde73a52 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java @@ -16,19 +16,18 @@ package com.google.gerrit.acceptance.rest.project; import static com.google.gerrit.acceptance.GitUtil.createProject; import static com.google.gerrit.acceptance.rest.project.BranchAssert.assertBranches; +import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.server.project.Util.block; import static org.junit.Assert.assertEquals; import com.google.common.collect.Lists; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.RestResponse; -import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.Permission; -import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; -import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.project.ListBranches.BranchInfo; import com.google.gerrit.server.project.ProjectCache; import com.google.gson.reflect.TypeToken; @@ -142,17 +141,10 @@ public class ListBranchesIT extends AbstractDaemonTest { private void blockRead(Project.NameKey project, String ref) throws RepositoryNotFoundException, IOException, ConfigInvalidException { - MetaDataUpdate md = metaDataUpdateFactory.create(project); - md.setMessage("Grant submit on " + ref); - ProjectConfig config = ProjectConfig.read(md); - AccessSection s = config.getAccessSection(ref, true); - Permission p = s.getPermission(Permission.READ, true); - PermissionRule rule = new PermissionRule(config.resolve( - SystemGroupBackend.getGroup(SystemGroupBackend.REGISTERED_USERS))); - rule.setBlock(); - p.add(rule); - config.commit(md); - projectCache.evict(config.getProject()); + ProjectConfig cfg = projectCache.checkedGet(project).getConfig(); + block(cfg, Permission.READ, REGISTERED_USERS, ref); + saveProjectConfig(project, cfg); + projectCache.evict(cfg.getProject()); } private static List toBranchInfoList(RestResponse r) @@ -168,4 +160,13 @@ public class ListBranchesIT extends AbstractDaemonTest { PushOneCommit push = pushFactory.create(db, admin.getIdent()); return push.to(git, ref); } + + private void saveProjectConfig(Project.NameKey p, ProjectConfig cfg) throws IOException { + MetaDataUpdate md = metaDataUpdateFactory.create(p); + try { + cfg.commit(md); + } finally { + md.close(); + } + } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java index f38ac89b3e..2a20a2cba8 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java @@ -15,8 +15,8 @@ package com.google.gerrit.acceptance.server.project; import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; +import static com.google.gerrit.server.project.Util.allow; import static com.google.gerrit.server.project.Util.category; -import static com.google.gerrit.server.project.Util.grant; import static com.google.gerrit.server.project.Util.value; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -64,7 +64,7 @@ public class CustomLabelIT extends AbstractDaemonTest { ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig(); AccountGroup.UUID anonymousUsers = SystemGroupBackend.getGroup(ANONYMOUS_USERS).getUUID(); - grant(cfg, Permission.forLabel(Q.getName()), -1, 1, anonymousUsers, + allow(cfg, Permission.forLabel(Q.getName()), -1, 1, anonymousUsers, "refs/heads/*"); saveProjectConfig(cfg); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java b/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java index d5db854673..1ed35bed7f 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java @@ -15,8 +15,8 @@ package com.google.gerrit.rules; import static com.google.gerrit.common.data.Permission.LABEL; +import static com.google.gerrit.server.project.Util.allow; import static com.google.gerrit.server.project.Util.category; -import static com.google.gerrit.server.project.Util.grant; import static com.google.gerrit.server.project.Util.value; import com.google.gerrit.common.data.LabelType; @@ -74,8 +74,8 @@ public class GerritCommonTest extends PrologTestCase { local.getLabelSections().put(V.getName(), V); local.getLabelSections().put(Q.getName(), Q); util.add(local); - grant(local, LABEL + V.getName(), -1, +1, SystemGroupBackend.REGISTERED_USERS, "refs/heads/*"); - grant(local, LABEL + Q.getName(), -1, +1, SystemGroupBackend.REGISTERED_USERS, "refs/heads/master"); + allow(local, LABEL + V.getName(), -1, +1, SystemGroupBackend.REGISTERED_USERS, "refs/heads/*"); + allow(local, LABEL + Q.getName(), -1, +1, SystemGroupBackend.REGISTERED_USERS, "refs/heads/master"); } @Override diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java index 5412c671d0..40088e9506 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java @@ -16,8 +16,8 @@ package com.google.gerrit.server.git; import static com.google.gerrit.common.data.Permission.forLabel; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.server.project.Util.allow; import static com.google.gerrit.server.project.Util.category; -import static com.google.gerrit.server.project.Util.grant; import static com.google.gerrit.server.project.Util.value; import static org.junit.Assert.assertEquals; @@ -133,8 +133,8 @@ public class LabelNormalizerTest { @Test public void normalizeByPermission() throws Exception { ProjectConfig pc = loadAllProjects(); - grant(pc, forLabel("Code-Review"), -1, 1, REGISTERED_USERS, "refs/heads/*"); - grant(pc, forLabel("Verified"), -1, 1, REGISTERED_USERS, "refs/heads/*"); + allow(pc, forLabel("Code-Review"), -1, 1, REGISTERED_USERS, "refs/heads/*"); + allow(pc, forLabel("Verified"), -1, 1, REGISTERED_USERS, "refs/heads/*"); save(pc); PatchSetApproval cr = psa(userId, "Code-Review", 2); @@ -149,8 +149,8 @@ public class LabelNormalizerTest { @Test public void normalizeByType() throws Exception { ProjectConfig pc = loadAllProjects(); - grant(pc, forLabel("Code-Review"), -5, 5, REGISTERED_USERS, "refs/heads/*"); - grant(pc, forLabel("Verified"), -5, 5, REGISTERED_USERS, "refs/heads/*"); + allow(pc, forLabel("Code-Review"), -5, 5, REGISTERED_USERS, "refs/heads/*"); + allow(pc, forLabel("Verified"), -5, 5, REGISTERED_USERS, "refs/heads/*"); save(pc); PatchSetApproval cr = psa(userId, "Code-Review", 5); @@ -176,7 +176,7 @@ public class LabelNormalizerTest { @Test public void explicitZeroVoteOnNonEmptyRangeIsPresent() throws Exception { ProjectConfig pc = loadAllProjects(); - grant(pc, forLabel("Code-Review"), -1, 1, REGISTERED_USERS, "refs/heads/*"); + allow(pc, forLabel("Code-Review"), -1, 1, REGISTERED_USERS, "refs/heads/*"); save(pc); PatchSetApproval cr = psa(userId, "Code-Review", 0); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index 54765ac558..7963ce45c8 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -25,8 +25,10 @@ import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; import static com.google.gerrit.server.project.Util.ADMIN; import static com.google.gerrit.server.project.Util.DEVS; +import static com.google.gerrit.server.project.Util.allow; +import static com.google.gerrit.server.project.Util.block; +import static com.google.gerrit.server.project.Util.deny; import static com.google.gerrit.server.project.Util.doNotInherit; -import static com.google.gerrit.server.project.Util.grant; import static com.google.gerrit.testutil.InMemoryRepositoryManager.newRepository; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -68,7 +70,7 @@ public class RefControlTest { @Test public void testOwnerProject() { - grant(local, OWNER, ADMIN, "refs/*"); + allow(local, OWNER, ADMIN, "refs/*"); ProjectControl uBlah = util.user(local, DEVS); ProjectControl uAdmin = util.user(local, DEVS, ADMIN); @@ -79,8 +81,8 @@ public class RefControlTest { @Test public void testBranchDelegation1() { - grant(local, OWNER, ADMIN, "refs/*"); - grant(local, OWNER, DEVS, "refs/heads/x/*"); + allow(local, OWNER, ADMIN, "refs/*"); + allow(local, OWNER, DEVS, "refs/heads/x/*"); ProjectControl uDev = util.user(local, DEVS); assertFalse("not owner", uDev.isOwner()); @@ -96,9 +98,9 @@ public class RefControlTest { @Test public void testBranchDelegation2() { - grant(local, OWNER, ADMIN, "refs/*"); - grant(local, OWNER, DEVS, "refs/heads/x/*"); - grant(local, OWNER, fixers, "refs/heads/x/y/*"); + allow(local, OWNER, ADMIN, "refs/*"); + allow(local, OWNER, DEVS, "refs/heads/x/*"); + allow(local, OWNER, fixers, "refs/heads/x/y/*"); doNotInherit(local, OWNER, "refs/heads/x/y/*"); ProjectControl uDev = util.user(local, DEVS); @@ -125,9 +127,9 @@ public class RefControlTest { @Test public void testInheritRead_SingleBranchDeniesUpload() { - grant(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); - grant(util.getParentConfig(), PUSH, REGISTERED_USERS, "refs/for/refs/*"); - grant(local, READ, REGISTERED_USERS, "refs/heads/foobar"); + allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); + allow(util.getParentConfig(), PUSH, REGISTERED_USERS, "refs/for/refs/*"); + allow(local, READ, REGISTERED_USERS, "refs/heads/foobar"); doNotInherit(local, READ, "refs/heads/foobar"); doNotInherit(local, PUSH, "refs/for/refs/heads/foobar"); @@ -143,9 +145,8 @@ public class RefControlTest { @Test public void testBlockPushDrafts() { - grant(util.getParentConfig(), PUSH, REGISTERED_USERS, "refs/for/refs/*"); - grant(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/drafts/*") - .setBlock(); + allow(util.getParentConfig(), PUSH, REGISTERED_USERS, "refs/for/refs/*"); + block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/drafts/*"); ProjectControl u = util.user(local); assertTrue("can upload refs/heads/master", @@ -156,9 +157,8 @@ public class RefControlTest { @Test public void testBlockPushDraftsUnblockAdmin() { - grant(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/drafts/*") - .setBlock(); - grant(util.getParentConfig(), PUSH, ADMIN, "refs/drafts/*"); + block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/drafts/*"); + allow(util.getParentConfig(), PUSH, ADMIN, "refs/drafts/*"); assertTrue("push is blocked for anonymous to refs/drafts/master", util.user(local).controlForRef("refs/drafts/refs/heads/master") @@ -170,9 +170,9 @@ public class RefControlTest { @Test public void testInheritRead_SingleBranchDoesNotOverrideInherited() { - grant(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); - grant(util.getParentConfig(), PUSH, REGISTERED_USERS, "refs/for/refs/*"); - grant(local, READ, REGISTERED_USERS, "refs/heads/foobar"); + allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); + allow(util.getParentConfig(), PUSH, REGISTERED_USERS, "refs/for/refs/*"); + allow(local, READ, REGISTERED_USERS, "refs/heads/foobar"); ProjectControl u = util.user(local); assertTrue("can upload", u.canPushToAtLeastOneRef() == Capable.OK); @@ -186,21 +186,21 @@ public class RefControlTest { @Test public void testInheritDuplicateSections() throws Exception { - grant(util.getParentConfig(), READ, ADMIN, "refs/*"); - grant(local, READ, DEVS, "refs/heads/*"); + allow(util.getParentConfig(), READ, ADMIN, "refs/*"); + allow(local, READ, DEVS, "refs/heads/*"); local.getProject().setParentName(util.getParentConfig().getProject().getName()); assertTrue("a can read", util.user(local, "a", ADMIN).isVisible()); local = new ProjectConfig(new Project.NameKey("local")); local.load(newRepository(localKey)); - grant(local, READ, DEVS, "refs/*"); + allow(local, READ, DEVS, "refs/*"); assertTrue("d can read", util.user(local, "d", DEVS).isVisible()); } @Test public void testInheritRead_OverrideWithDeny() { - grant(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); - grant(local, READ, REGISTERED_USERS, "refs/*").setDeny(); + allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); + deny(local, READ, REGISTERED_USERS, "refs/*"); ProjectControl u = util.user(local); assertFalse("can't read", u.isVisible()); @@ -208,8 +208,8 @@ public class RefControlTest { @Test public void testInheritRead_AppendWithDenyOfRef() { - grant(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); - grant(local, READ, REGISTERED_USERS, "refs/heads/*").setDeny(); + allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); + deny(local, READ, REGISTERED_USERS, "refs/heads/*"); ProjectControl u = util.user(local); assertTrue("can read", u.isVisible()); @@ -220,9 +220,9 @@ public class RefControlTest { @Test public void testInheritRead_OverridesAndDeniesOfRef() { - grant(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); - grant(local, READ, REGISTERED_USERS, "refs/*").setDeny(); - grant(local, READ, REGISTERED_USERS, "refs/heads/*"); + allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); + deny(local, READ, REGISTERED_USERS, "refs/*"); + allow(local, READ, REGISTERED_USERS, "refs/heads/*"); ProjectControl u = util.user(local); assertTrue("can read", u.isVisible()); @@ -233,9 +233,9 @@ public class RefControlTest { @Test public void testInheritSubmit_OverridesAndDeniesOfRef() { - grant(util.getParentConfig(), SUBMIT, REGISTERED_USERS, "refs/*"); - grant(local, SUBMIT, REGISTERED_USERS, "refs/*").setDeny(); - grant(local, SUBMIT, REGISTERED_USERS, "refs/heads/*"); + allow(util.getParentConfig(), SUBMIT, REGISTERED_USERS, "refs/*"); + deny(local, SUBMIT, REGISTERED_USERS, "refs/*"); + allow(local, SUBMIT, REGISTERED_USERS, "refs/heads/*"); ProjectControl u = util.user(local); assertFalse("can't submit", u.controlForRef("refs/foobar").canSubmit()); @@ -245,9 +245,9 @@ public class RefControlTest { @Test public void testCannotUploadToAnyRef() { - grant(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); - grant(local, READ, DEVS, "refs/heads/*"); - grant(local, PUSH, DEVS, "refs/for/refs/heads/*"); + allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*"); + allow(local, READ, DEVS, "refs/heads/*"); + allow(local, PUSH, DEVS, "refs/for/refs/heads/*"); ProjectControl u = util.user(local); assertFalse("cannot upload", u.canPushToAtLeastOneRef() == Capable.OK); @@ -257,7 +257,7 @@ public class RefControlTest { @Test public void testUsernamePatternNonRegex() { - grant(local, READ, DEVS, "refs/sb/${username}/heads/*"); + allow(local, READ, DEVS, "refs/sb/${username}/heads/*"); ProjectControl u = util.user(local, "u", DEVS), d = util.user(local, "d", DEVS); assertFalse("u can't read", u.controlForRef("refs/sb/d/heads/foobar").isVisible()); @@ -266,7 +266,7 @@ public class RefControlTest { @Test public void testUsernamePatternWithRegex() { - grant(local, READ, DEVS, "^refs/sb/${username}/heads/.*"); + allow(local, READ, DEVS, "^refs/sb/${username}/heads/.*"); ProjectControl u = util.user(local, "d.v", DEVS), d = util.user(local, "dev", DEVS); assertFalse("u can't read", u.controlForRef("refs/sb/dev/heads/foobar").isVisible()); @@ -275,7 +275,7 @@ public class RefControlTest { @Test public void testUsernameEmailPatternWithRegex() { - grant(local, READ, DEVS, "^refs/sb/${username}/heads/.*"); + allow(local, READ, DEVS, "^refs/sb/${username}/heads/.*"); ProjectControl u = util.user(local, "d.v@ger-rit.org", DEVS); ProjectControl d = util.user(local, "dev@ger-rit.org", DEVS); @@ -287,8 +287,8 @@ public class RefControlTest { @Test public void testSortWithRegex() { - grant(local, READ, DEVS, "^refs/heads/.*"); - grant(util.getParentConfig(), READ, ANONYMOUS_USERS, "^refs/heads/.*-QA-.*"); + allow(local, READ, DEVS, "^refs/heads/.*"); + allow(util.getParentConfig(), READ, ANONYMOUS_USERS, "^refs/heads/.*-QA-.*"); ProjectControl u = util.user(local, DEVS), d = util.user(local, DEVS); assertTrue("u can read", u.controlForRef("refs/heads/foo-QA-bar").isVisible()); @@ -297,17 +297,17 @@ public class RefControlTest { @Test public void testBlockRule_ParentBlocksChild() { - grant(local, PUSH, DEVS, "refs/tags/*"); - grant(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/tags/*").setBlock(); + allow(local, PUSH, DEVS, "refs/tags/*"); + block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/tags/*"); ProjectControl u = util.user(local, DEVS); assertFalse("u can't update tag", u.controlForRef("refs/tags/V10").canUpdate()); } @Test public void testBlockRule_ParentBlocksChildEvenIfAlreadyBlockedInChild() { - grant(local, PUSH, DEVS, "refs/tags/*"); - grant(local, PUSH, ANONYMOUS_USERS, "refs/tags/*").setBlock(); - grant(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/tags/*").setBlock(); + allow(local, PUSH, DEVS, "refs/tags/*"); + block(local, PUSH, ANONYMOUS_USERS, "refs/tags/*"); + block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/tags/*"); ProjectControl u = util.user(local, DEVS); assertFalse("u can't update tag", u.controlForRef("refs/tags/V10").canUpdate()); @@ -315,8 +315,8 @@ public class RefControlTest { @Test public void testBlockLabelRange_ParentBlocksChild() { - grant(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); - grant(util.getParentConfig(), LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*").setBlock(); + allow(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); + block(util.getParentConfig(), LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); ProjectControl u = util.user(local, DEVS); @@ -329,10 +329,10 @@ public class RefControlTest { @Test public void testBlockLabelRange_ParentBlocksChildEvenIfAlreadyBlockedInChild() { - grant(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); - grant(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*").setBlock(); - grant(util.getParentConfig(), LABEL + "Code-Review", -2, +2, DEVS, - "refs/heads/*").setBlock(); + allow(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); + block(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); + block(util.getParentConfig(), LABEL + "Code-Review", -2, +2, DEVS, + "refs/heads/*"); ProjectControl u = util.user(local, DEVS); @@ -346,8 +346,8 @@ public class RefControlTest { @Test public void testUnblockNoForce() { - grant(local, PUSH, ANONYMOUS_USERS, "refs/heads/*").setBlock(); - grant(local, PUSH, DEVS, "refs/heads/*"); + block(local, PUSH, ANONYMOUS_USERS, "refs/heads/*"); + allow(local, PUSH, DEVS, "refs/heads/*"); ProjectControl u = util.user(local, DEVS); assertTrue("u can push", u.controlForRef("refs/heads/master").canUpdate()); @@ -355,10 +355,9 @@ public class RefControlTest { @Test public void testUnblockForce() { - PermissionRule r = grant(local, PUSH, ANONYMOUS_USERS, "refs/heads/*"); - r.setBlock(); + PermissionRule r = block(local, PUSH, ANONYMOUS_USERS, "refs/heads/*"); r.setForce(true); - grant(local, PUSH, DEVS, "refs/heads/*").setForce(true); + allow(local, PUSH, DEVS, "refs/heads/*").setForce(true); ProjectControl u = util.user(local, DEVS); assertTrue("u can force push", u.controlForRef("refs/heads/master").canForceUpdate()); @@ -366,10 +365,9 @@ public class RefControlTest { @Test public void testUnblockForceWithAllowNoForce_NotPossible() { - PermissionRule r = grant(local, PUSH, ANONYMOUS_USERS, "refs/heads/*"); - r.setBlock(); + PermissionRule r = block(local, PUSH, ANONYMOUS_USERS, "refs/heads/*"); r.setForce(true); - grant(local, PUSH, DEVS, "refs/heads/*"); + allow(local, PUSH, DEVS, "refs/heads/*"); ProjectControl u = util.user(local, DEVS); assertFalse("u can't force push", u.controlForRef("refs/heads/master").canForceUpdate()); @@ -377,8 +375,8 @@ public class RefControlTest { @Test public void testUnblockMoreSpecificRef_Fails() { - grant(local, PUSH, ANONYMOUS_USERS, "refs/heads/*").setBlock(); - grant(local, PUSH, DEVS, "refs/heads/master"); + block(local, PUSH, ANONYMOUS_USERS, "refs/heads/*"); + allow(local, PUSH, DEVS, "refs/heads/master"); ProjectControl u = util.user(local, DEVS); assertFalse("u can't push", u.controlForRef("refs/heads/master").canUpdate()); @@ -386,8 +384,8 @@ public class RefControlTest { @Test public void testUnblockLargerScope_Fails() { - grant(local, PUSH, ANONYMOUS_USERS, "refs/heads/master").setBlock(); - grant(local, PUSH, DEVS, "refs/heads/*"); + block(local, PUSH, ANONYMOUS_USERS, "refs/heads/master"); + allow(local, PUSH, DEVS, "refs/heads/*"); ProjectControl u = util.user(local, DEVS); assertFalse("u can't push", u.controlForRef("refs/heads/master").canUpdate()); @@ -395,8 +393,8 @@ public class RefControlTest { @Test public void testUnblockInLocal_Fails() { - grant(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/heads/*").setBlock(); - grant(local, PUSH, fixers, "refs/heads/*"); + block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/heads/*"); + allow(local, PUSH, fixers, "refs/heads/*"); ProjectControl f = util.user(local, fixers); assertFalse("u can't push", f.controlForRef("refs/heads/master").canUpdate()); @@ -404,9 +402,9 @@ public class RefControlTest { @Test public void testUnblockInParentBlockInLocal() { - grant(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/heads/*").setBlock(); - grant(util.getParentConfig(), PUSH, DEVS, "refs/heads/*"); - grant(local, PUSH, DEVS, "refs/heads/*").setBlock(); + block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/heads/*"); + allow(util.getParentConfig(), PUSH, DEVS, "refs/heads/*"); + block(local, PUSH, DEVS, "refs/heads/*"); ProjectControl d = util.user(local, DEVS); assertFalse("u can't push", d.controlForRef("refs/heads/master").canUpdate()); @@ -414,8 +412,8 @@ public class RefControlTest { @Test public void testUnblockVisibilityByREGISTEREDUsers() { - grant(local, READ, ANONYMOUS_USERS, "refs/heads/*").setBlock(); - grant(local, READ, REGISTERED_USERS, "refs/heads/*"); + block(local, READ, ANONYMOUS_USERS, "refs/heads/*"); + allow(local, READ, REGISTERED_USERS, "refs/heads/*"); ProjectControl u = util.user(local, REGISTERED_USERS); assertTrue("u can read", u.controlForRef("refs/heads/master").isVisibleByRegisteredUsers()); @@ -423,8 +421,8 @@ public class RefControlTest { @Test public void testUnblockInLocalVisibilityByRegisteredUsers_Fails() { - grant(util.getParentConfig(), READ, ANONYMOUS_USERS, "refs/heads/*").setBlock(); - grant(local, READ, REGISTERED_USERS, "refs/heads/*"); + block(util.getParentConfig(), READ, ANONYMOUS_USERS, "refs/heads/*"); + allow(local, READ, REGISTERED_USERS, "refs/heads/*"); ProjectControl u = util.user(local, REGISTERED_USERS); assertFalse("u can't read", u.controlForRef("refs/heads/master").isVisibleByRegisteredUsers()); @@ -432,8 +430,8 @@ public class RefControlTest { @Test public void testUnblockForceEditTopicName() { - grant(local, EDIT_TOPIC_NAME, ANONYMOUS_USERS, "refs/heads/*").setBlock(); - grant(local, EDIT_TOPIC_NAME, DEVS, "refs/heads/*").setForce(true); + block(local, EDIT_TOPIC_NAME, ANONYMOUS_USERS, "refs/heads/*"); + allow(local, EDIT_TOPIC_NAME, DEVS, "refs/heads/*").setForce(true); ProjectControl u = util.user(local, DEVS); assertTrue("u can edit topic name", u.controlForRef("refs/heads/master") @@ -442,9 +440,8 @@ public class RefControlTest { @Test public void testUnblockInLocalForceEditTopicName_Fails() { - grant(util.getParentConfig(), EDIT_TOPIC_NAME, ANONYMOUS_USERS, "refs/heads/*") - .setBlock(); - grant(local, EDIT_TOPIC_NAME, DEVS, "refs/heads/*").setForce(true); + block(util.getParentConfig(), EDIT_TOPIC_NAME, ANONYMOUS_USERS, "refs/heads/*"); + allow(local, EDIT_TOPIC_NAME, DEVS, "refs/heads/*").setForce(true); ProjectControl u = util.user(local, REGISTERED_USERS); assertFalse("u can't edit topic name", u.controlForRef("refs/heads/master") @@ -453,8 +450,8 @@ public class RefControlTest { @Test public void testUnblockRange() { - grant(local, LABEL + "Code-Review", -1, +1, ANONYMOUS_USERS, "refs/heads/*").setBlock(); - grant(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); + block(local, LABEL + "Code-Review", -1, +1, ANONYMOUS_USERS, "refs/heads/*"); + allow(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); ProjectControl u = util.user(local, DEVS); PermissionRange range = u.controlForRef("refs/heads/master").getRange(LABEL + "Code-Review"); @@ -464,8 +461,8 @@ public class RefControlTest { @Test public void testUnblockRangeOnMoreSpecificRef_Fails() { - grant(local, LABEL + "Code-Review", -1, +1, ANONYMOUS_USERS, "refs/heads/*").setBlock(); - grant(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/master"); + block(local, LABEL + "Code-Review", -1, +1, ANONYMOUS_USERS, "refs/heads/*"); + allow(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/master"); ProjectControl u = util.user(local, DEVS); PermissionRange range = u.controlForRef("refs/heads/master").getRange(LABEL + "Code-Review"); @@ -475,8 +472,8 @@ public class RefControlTest { @Test public void testUnblockRangeOnLargerScope_Fails() { - grant(local, LABEL + "Code-Review", -1, +1, ANONYMOUS_USERS, "refs/heads/master").setBlock(); - grant(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); + block(local, LABEL + "Code-Review", -1, +1, ANONYMOUS_USERS, "refs/heads/master"); + allow(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); ProjectControl u = util.user(local, DEVS); PermissionRange range = u.controlForRef("refs/heads/master").getRange(LABEL + "Code-Review"); @@ -486,9 +483,9 @@ public class RefControlTest { @Test public void testUnblockInLocalRange_Fails() { - grant(util.getParentConfig(), LABEL + "Code-Review", -1, 1, ANONYMOUS_USERS, - "refs/heads/*").setBlock(); - grant(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); + block(util.getParentConfig(), LABEL + "Code-Review", -1, 1, ANONYMOUS_USERS, + "refs/heads/*"); + allow(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*"); ProjectControl u = util.user(local, DEVS); PermissionRange range = @@ -498,7 +495,7 @@ public class RefControlTest { } public void testUnblockRangeForChangeOwner() { - grant(local, LABEL + "Code-Review", -2, +2, CHANGE_OWNER, "refs/heads/*"); + allow(local, LABEL + "Code-Review", -2, +2, CHANGE_OWNER, "refs/heads/*"); ProjectControl u = util.user(local, DEVS); PermissionRange range = u.controlForRef("refs/heads/master") @@ -508,7 +505,7 @@ public class RefControlTest { } public void testUnblockRangeForNotChangeOwner() { - grant(local, LABEL + "Code-Review", -2, +2, CHANGE_OWNER, "refs/heads/*"); + allow(local, LABEL + "Code-Review", -2, +2, CHANGE_OWNER, "refs/heads/*"); ProjectControl u = util.user(local, DEVS); PermissionRange range = u.controlForRef("refs/heads/master") diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java index 9ac8ba9c33..2832d16139 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java @@ -100,7 +100,7 @@ public class Util { return new PermissionRule(group); } - static public PermissionRule grant(ProjectConfig project, + static public PermissionRule allow(ProjectConfig project, String permissionName, int min, int max, AccountGroup.UUID group, String ref) { PermissionRule rule = newRule(project, group); @@ -109,11 +109,36 @@ public class Util { return grant(project, permissionName, rule, ref); } - static public PermissionRule grant(ProjectConfig project, + static public PermissionRule block(ProjectConfig project, + String permissionName, int min, int max, AccountGroup.UUID group, + String ref) { + PermissionRule rule = newRule(project, group); + rule.setMin(min); + rule.setMax(max); + PermissionRule r = grant(project, permissionName, rule, ref); + r.setBlock(); + return r; + } + + static public PermissionRule allow(ProjectConfig project, String permissionName, AccountGroup.UUID group, String ref) { return grant(project, permissionName, newRule(project, group), ref); } + static public PermissionRule block(ProjectConfig project, + String permissionName, AccountGroup.UUID group, String ref) { + PermissionRule r = grant(project, permissionName, newRule(project, group), ref); + r.setBlock(); + return r; + } + + static public PermissionRule deny(ProjectConfig project, + String permissionName, AccountGroup.UUID group, String ref) { + PermissionRule r = grant(project, permissionName, newRule(project, group), ref); + r.setDeny(); + return r; + } + static public void doNotInherit(ProjectConfig project, String permissionName, String ref) { project.getAccessSection(ref, true) //