Merge "Resource exhausted because of unclosed LDAP connection" into stable-2.10
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
index 63d2b35..321c2ea 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.project;
+import static com.google.common.base.Objects.firstNonNull;
import static com.google.gerrit.server.project.RefControl.isRE;
import com.google.common.collect.Lists;
@@ -113,6 +114,7 @@
Set<String> exclusiveGroupPermissions = new HashSet<>();
HashMap<String, List<PermissionRule>> permissions = new HashMap<>();
+ HashMap<String, List<PermissionRule>> overridden = new HashMap<>();
Map<PermissionRule, ProjectRef> ruleProps = Maps.newIdentityHashMap();
for (AccessSection section : sections) {
Project.NameKey project = sectionToProject.get(section);
@@ -128,11 +130,19 @@
} else {
addRule = seen.add(s) && !rule.isDeny() && !exclusivePermissionExists;
}
+
+ HashMap<String, List<PermissionRule>> p = null;
if (addRule) {
- List<PermissionRule> r = permissions.get(permission.getName());
+ p = permissions;
+ } else if (!rule.isDeny() && !exclusivePermissionExists) {
+ p = overridden;
+ }
+
+ if (p != null) {
+ List<PermissionRule> r = p.get(permission.getName());
if (r == null) {
r = new ArrayList<>(2);
- permissions.put(permission.getName(), r);
+ p.put(permission.getName(), r);
}
r.add(rule);
ruleProps.put(rule, new ProjectRef(project, section.getName()));
@@ -145,18 +155,22 @@
}
}
- return new PermissionCollection(permissions, ruleProps, perUser);
+ return new PermissionCollection(permissions, overridden, ruleProps,
+ perUser);
}
}
private final Map<String, List<PermissionRule>> rules;
+ private final Map<String, List<PermissionRule>> overridden;
private final Map<PermissionRule, ProjectRef> ruleProps;
private final boolean perUser;
private PermissionCollection(Map<String, List<PermissionRule>> rules,
+ Map<String, List<PermissionRule>> overridden,
Map<PermissionRule, ProjectRef> ruleProps,
boolean perUser) {
this.rules = rules;
+ this.overridden = overridden;
this.ruleProps = ruleProps;
this.perUser = perUser;
}
@@ -182,6 +196,11 @@
return r != null ? r : Collections.<PermissionRule> emptyList();
}
+ List<PermissionRule> getOverridden(String permissionName) {
+ return firstNonNull(
+ overridden.get(permissionName), Collections.<PermissionRule> emptyList());
+ }
+
ProjectRef getRuleProps(PermissionRule rule) {
return ruleProps.get(rule);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 00ecee3..7dc7a2a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -120,6 +120,7 @@
*/
public boolean isVisibleByRegisteredUsers() {
List<PermissionRule> access = relevant.getPermission(Permission.READ);
+ List<PermissionRule> overridden = relevant.getOverridden(Permission.READ);
Set<ProjectRef> allows = Sets.newHashSet();
Set<ProjectRef> blocks = Sets.newHashSet();
for (PermissionRule rule : access) {
@@ -129,6 +130,11 @@
allows.add(relevant.getRuleProps(rule));
}
}
+ for (PermissionRule rule : overridden) {
+ if (SystemGroupBackend.isAnonymousOrRegistered(rule.getGroup())) {
+ blocks.remove(relevant.getRuleProps(rule));
+ }
+ }
blocks.removeAll(allows);
return blocks.isEmpty() && !allows.isEmpty();
}
@@ -492,6 +498,7 @@
private boolean doCanPerform(String permissionName, boolean blockOnly) {
List<PermissionRule> access = access(permissionName);
+ List<PermissionRule> overridden = relevant.getOverridden(permissionName);
Set<ProjectRef> allows = Sets.newHashSet();
Set<ProjectRef> blocks = Sets.newHashSet();
for (PermissionRule rule : access) {
@@ -501,6 +508,9 @@
allows.add(relevant.getRuleProps(rule));
}
}
+ for (PermissionRule rule : overridden) {
+ blocks.remove(relevant.getRuleProps(rule));
+ }
blocks.removeAll(allows);
return blocks.isEmpty() && (!allows.isEmpty() || blockOnly);
}
@@ -508,6 +518,7 @@
/** True if the user has force this permission. Works only for non labels. */
private boolean canForcePerform(String permissionName) {
List<PermissionRule> access = access(permissionName);
+ List<PermissionRule> overridden = relevant.getOverridden(permissionName);
Set<ProjectRef> allows = Sets.newHashSet();
Set<ProjectRef> blocks = Sets.newHashSet();
for (PermissionRule rule : access) {
@@ -517,6 +528,11 @@
allows.add(relevant.getRuleProps(rule));
}
}
+ for (PermissionRule rule : overridden) {
+ if (rule.getForce()) {
+ blocks.remove(relevant.getRuleProps(rule));
+ }
+ }
blocks.removeAll(allows);
return blocks.isEmpty() && !allows.isEmpty();
}
@@ -524,6 +540,7 @@
/** True if for this permission force is blocked for the user. Works only for non labels. */
private boolean isForceBlocked(String permissionName) {
List<PermissionRule> access = access(permissionName);
+ List<PermissionRule> overridden = relevant.getOverridden(permissionName);
Set<ProjectRef> allows = Sets.newHashSet();
Set<ProjectRef> blocks = Sets.newHashSet();
for (PermissionRule rule : access) {
@@ -533,6 +550,11 @@
allows.add(relevant.getRuleProps(rule));
}
}
+ for (PermissionRule rule : overridden) {
+ if (rule.getForce()) {
+ blocks.remove(relevant.getRuleProps(rule));
+ }
+ }
blocks.removeAll(allows);
return !blocks.isEmpty();
}
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 dc3529c..5478a6c 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
@@ -55,6 +55,8 @@
private final AccountGroup.UUID fixers = new AccountGroup.UUID("test.fixers");
private Project.NameKey localKey = new Project.NameKey("local");
private ProjectConfig local;
+ private Project.NameKey parentKey = new Project.NameKey("parent");
+ private ProjectConfig parent;
private final Util util;
public RefControlTest() {
@@ -63,9 +65,14 @@
@Before
public void setUp() throws Exception {
+ parent = new ProjectConfig(parentKey);
+ parent.load(newRepository(parentKey));
+ util.add(parent);
+
local = new ProjectConfig(localKey);
local.load(newRepository(localKey));
util.add(local);
+ local.getProject().setParentName(parentKey);
}
@Test
@@ -139,8 +146,8 @@
@Test
public void testInheritRead_SingleBranchDeniesUpload() {
- allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*");
- allow(util.getParentConfig(), PUSH, REGISTERED_USERS, "refs/for/refs/*");
+ allow(parent, READ, REGISTERED_USERS, "refs/*");
+ allow(parent, 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");
@@ -157,8 +164,8 @@
@Test
public void testBlockPushDrafts() {
- allow(util.getParentConfig(), PUSH, REGISTERED_USERS, "refs/for/refs/*");
- block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/drafts/*");
+ allow(parent, PUSH, REGISTERED_USERS, "refs/for/refs/*");
+ block(parent, PUSH, ANONYMOUS_USERS, "refs/drafts/*");
ProjectControl u = util.user(local);
assertTrue("can upload refs/heads/master",
@@ -169,8 +176,8 @@
@Test
public void testBlockPushDraftsUnblockAdmin() {
- block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/drafts/*");
- allow(util.getParentConfig(), PUSH, ADMIN, "refs/drafts/*");
+ block(parent, PUSH, ANONYMOUS_USERS, "refs/drafts/*");
+ allow(parent, PUSH, ADMIN, "refs/drafts/*");
assertTrue("push is blocked for anonymous to refs/drafts/master",
util.user(local).controlForRef("refs/drafts/refs/heads/master")
@@ -182,8 +189,8 @@
@Test
public void testInheritRead_SingleBranchDoesNotOverrideInherited() {
- allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*");
- allow(util.getParentConfig(), PUSH, REGISTERED_USERS, "refs/for/refs/*");
+ allow(parent, READ, REGISTERED_USERS, "refs/*");
+ allow(parent, PUSH, REGISTERED_USERS, "refs/for/refs/*");
allow(local, READ, REGISTERED_USERS, "refs/heads/foobar");
ProjectControl u = util.user(local);
@@ -198,20 +205,20 @@
@Test
public void testInheritDuplicateSections() throws Exception {
- allow(util.getParentConfig(), READ, ADMIN, "refs/*");
+ allow(parent, 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 = new ProjectConfig(localKey);
local.load(newRepository(localKey));
+ local.getProject().setParentName(parentKey);
allow(local, READ, DEVS, "refs/*");
assertTrue("d can read", util.user(local, "d", DEVS).isVisible());
}
@Test
public void testInheritRead_OverrideWithDeny() {
- allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*");
+ allow(parent, READ, REGISTERED_USERS, "refs/*");
deny(local, READ, REGISTERED_USERS, "refs/*");
ProjectControl u = util.user(local);
@@ -220,7 +227,7 @@
@Test
public void testInheritRead_AppendWithDenyOfRef() {
- allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*");
+ allow(parent, READ, REGISTERED_USERS, "refs/*");
deny(local, READ, REGISTERED_USERS, "refs/heads/*");
ProjectControl u = util.user(local);
@@ -232,7 +239,7 @@
@Test
public void testInheritRead_OverridesAndDeniesOfRef() {
- allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*");
+ allow(parent, READ, REGISTERED_USERS, "refs/*");
deny(local, READ, REGISTERED_USERS, "refs/*");
allow(local, READ, REGISTERED_USERS, "refs/heads/*");
@@ -245,7 +252,7 @@
@Test
public void testInheritSubmit_OverridesAndDeniesOfRef() {
- allow(util.getParentConfig(), SUBMIT, REGISTERED_USERS, "refs/*");
+ allow(parent, SUBMIT, REGISTERED_USERS, "refs/*");
deny(local, SUBMIT, REGISTERED_USERS, "refs/*");
allow(local, SUBMIT, REGISTERED_USERS, "refs/heads/*");
@@ -257,7 +264,7 @@
@Test
public void testCannotUploadToAnyRef() {
- allow(util.getParentConfig(), READ, REGISTERED_USERS, "refs/*");
+ allow(parent, READ, REGISTERED_USERS, "refs/*");
allow(local, READ, DEVS, "refs/heads/*");
allow(local, PUSH, DEVS, "refs/for/refs/heads/*");
@@ -307,7 +314,7 @@
@Test
public void testSortWithRegex() {
allow(local, READ, DEVS, "^refs/heads/.*");
- allow(util.getParentConfig(), READ, ANONYMOUS_USERS, "^refs/heads/.*-QA-.*");
+ allow(parent, 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());
@@ -317,7 +324,7 @@
@Test
public void testBlockRule_ParentBlocksChild() {
allow(local, PUSH, DEVS, "refs/tags/*");
- block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/tags/*");
+ block(parent, PUSH, ANONYMOUS_USERS, "refs/tags/*");
ProjectControl u = util.user(local, DEVS);
assertFalse("u can't update tag", u.controlForRef("refs/tags/V10").canUpdate());
}
@@ -326,7 +333,7 @@
public void testBlockRule_ParentBlocksChildEvenIfAlreadyBlockedInChild() {
allow(local, PUSH, DEVS, "refs/tags/*");
block(local, PUSH, ANONYMOUS_USERS, "refs/tags/*");
- block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/tags/*");
+ block(parent, PUSH, ANONYMOUS_USERS, "refs/tags/*");
ProjectControl u = util.user(local, DEVS);
assertFalse("u can't update tag", u.controlForRef("refs/tags/V10").canUpdate());
@@ -335,7 +342,7 @@
@Test
public void testBlockLabelRange_ParentBlocksChild() {
allow(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*");
- block(util.getParentConfig(), LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*");
+ block(parent, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*");
ProjectControl u = util.user(local, DEVS);
@@ -350,7 +357,7 @@
public void testBlockLabelRange_ParentBlocksChildEvenIfAlreadyBlockedInChild() {
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,
+ block(parent, LABEL + "Code-Review", -2, +2, DEVS,
"refs/heads/*");
ProjectControl u = util.user(local, DEVS);
@@ -364,6 +371,17 @@
}
@Test
+ public void testInheritSubmit_AllowInChildDoesntAffectUnblockInParent() {
+ block(parent, SUBMIT, ANONYMOUS_USERS, "refs/heads/*");
+ allow(parent, SUBMIT, REGISTERED_USERS, "refs/heads/*");
+ allow(local, SUBMIT, REGISTERED_USERS, "refs/heads/*");
+
+ ProjectControl u = util.user(local);
+ assertFalse("not blocked from submitting", u.controlForRef(
+ "refs/heads/master").isBlocked(SUBMIT));
+ }
+
+ @Test
public void testUnblockNoForce() {
block(local, PUSH, ANONYMOUS_USERS, "refs/heads/*");
allow(local, PUSH, DEVS, "refs/heads/*");
@@ -412,7 +430,7 @@
@Test
public void testUnblockInLocal_Fails() {
- block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/heads/*");
+ block(parent, PUSH, ANONYMOUS_USERS, "refs/heads/*");
allow(local, PUSH, fixers, "refs/heads/*");
ProjectControl f = util.user(local, fixers);
@@ -421,8 +439,8 @@
@Test
public void testUnblockInParentBlockInLocal() {
- block(util.getParentConfig(), PUSH, ANONYMOUS_USERS, "refs/heads/*");
- allow(util.getParentConfig(), PUSH, DEVS, "refs/heads/*");
+ block(parent, PUSH, ANONYMOUS_USERS, "refs/heads/*");
+ allow(parent, PUSH, DEVS, "refs/heads/*");
block(local, PUSH, DEVS, "refs/heads/*");
ProjectControl d = util.user(local, DEVS);
@@ -440,7 +458,7 @@
@Test
public void testUnblockInLocalVisibilityByRegisteredUsers_Fails() {
- block(util.getParentConfig(), READ, ANONYMOUS_USERS, "refs/heads/*");
+ block(parent, READ, ANONYMOUS_USERS, "refs/heads/*");
allow(local, READ, REGISTERED_USERS, "refs/heads/*");
ProjectControl u = util.user(local, REGISTERED_USERS);
@@ -459,7 +477,7 @@
@Test
public void testUnblockInLocalForceEditTopicName_Fails() {
- block(util.getParentConfig(), EDIT_TOPIC_NAME, ANONYMOUS_USERS, "refs/heads/*");
+ block(parent, EDIT_TOPIC_NAME, ANONYMOUS_USERS, "refs/heads/*");
allow(local, EDIT_TOPIC_NAME, DEVS, "refs/heads/*").setForce(true);
ProjectControl u = util.user(local, REGISTERED_USERS);
@@ -502,7 +520,7 @@
@Test
public void testUnblockInLocalRange_Fails() {
- block(util.getParentConfig(), LABEL + "Code-Review", -1, 1, ANONYMOUS_USERS,
+ block(parent, LABEL + "Code-Review", -1, 1, ANONYMOUS_USERS,
"refs/heads/*");
allow(local, LABEL + "Code-Review", -2, +2, DEVS, "refs/heads/*");
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 c0f35ba..ac1e0d7 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
@@ -180,17 +180,19 @@
private final PermissionCollection.Factory sectionSorter;
private final GitRepositoryManager repoManager;
- private final AllProjectsName allProjectsName = new AllProjectsName("parent");
- private final ProjectConfig parent = new ProjectConfig(allProjectsName);
+ private final AllProjectsName allProjectsName =
+ new AllProjectsName("All-Projects");
+ private final ProjectConfig allProjects;
public Util() {
all = new HashMap<>();
repoManager = new InMemoryRepositoryManager();
try {
Repository repo = repoManager.createRepository(allProjectsName);
- parent.load(repo);
- parent.getLabelSections().put(CR.getName(), CR);
- add(parent);
+ allProjects = new ProjectConfig(new Project.NameKey(allProjectsName.get()));
+ allProjects.load(repo);
+ allProjects.getLabelSections().put(CR.getName(), CR);
+ add(allProjects);
} catch (IOException | ConfigInvalidException e) {
throw new RuntimeException(e);
}
@@ -281,10 +283,6 @@
injector.getInstance(ChangeControl.AssistedFactory.class);
}
- public ProjectConfig getParentConfig() {
- return this.parent;
- }
-
public void add(ProjectConfig pc) {
PrologEnvironment.Factory envFactory = null;
ProjectControl.AssistedFactory projectControlFactory = null;
