Merge "Expose extension point for generic OAuth providers" into stable-2.10
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index f121e1f..4ad7d21 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2282,6 +2282,16 @@
Default is unset for RFC 2307 servers (disabled)
and `memberOf` for Active Directory.
+[[ldap.fetchMemberOfEagerly]]ldap.fetchMemberOfEagerly::
++
+_(Optional)_ Whether to fetch the `memberOf` account attribute on
+login. Setups which use LDAP for user authentication but don't make
+use of the LDAP groups may benefit from setting this option to `false`
+as this will result in a much faster LDAP login.
++
+Default is unset for RFC 2307 servers (disabled) and `true` for
+Active Directory.
+
[[ldap.groupBase]]ldap.groupBase::
+
Root of the tree containing all group objects. This is typically
@@ -2392,6 +2402,47 @@
must have the DWORD value `allowtgtsessionkey` set to 1 and the account must not
have local administrator privileges.
+[[ldap.useConnectionPooling]]ldap.useConnectionPooling::
++
+_(Optional)_ Enable the LDAP connection pooling or not.
++
+If it is true, the LDAP service provider maintains a pool of (possibly)
+previously used connections and assigns them to a Context instance as
+needed. When a Context instance is done with a connection (closed or
+garbage collected), the connection is returned to the pool for future use.
++
+For details, see link:http://docs.oracle.com/javase/tutorial/jndi/ldap/pool.html[
+LDAP connection management (Pool)] and link:http://docs.oracle.com/javase/tutorial/jndi/ldap/config.html[
+LDAP connection management (Configuration)]
++
+By default, false.
+
+[[ldap.connectTimeout]]ldap.connectTimeout::
++
+_(Optional)_ Timeout period for establishment of an LDAP connection.
++
+The value is in the usual time-unit format like "1 s", "100 ms",
+etc...
++
+By default there is no timeout and Gerrit will wait indefinitely.
+
+[[ldap-connection-pooling]]
+==== LDAP Connection Pooling
+Once LDAP connection pooling is enabled by setting the link:#ldap.useConnectionPooling[
+ldap.useConnectionPooling] configuration property to `true`, the connection pool
+can be configured using JVM system properties as explained in the
+link:http://docs.oracle.com/javase/7/docs/technotes/guides/jndi/jndi-ldap.html#POOL[
+Java SE Documentation].
+
+For standalone Gerrit (running with the embedded Jetty), JVM system properties
+are specified in the link:#container[container section]:
+
+----
+ javaOptions = -Dcom.sun.jndi.ldap.connect.pool.maxsize=20
+ javaOptions = -Dcom.sun.jndi.ldap.connect.pool.prefsize=10
+ javaOptions = -Dcom.sun.jndi.ldap.connect.pool.timeout=300000
+----
+
[[mimetype]]
=== Section mimetype
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java
index d130243..6ba6bf0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java
@@ -21,14 +21,14 @@
/** Tracks group inclusions in memory for efficient access. */
public interface GroupIncludeCache {
/** @return groups directly a member of the passed group. */
- public Set<AccountGroup.UUID> membersOf(AccountGroup.UUID group);
+ public Set<AccountGroup.UUID> subgroupsOf(AccountGroup.UUID group);
/** @return any groups the passed group belongs to. */
- public Set<AccountGroup.UUID> memberIn(AccountGroup.UUID groupId);
+ public Set<AccountGroup.UUID> parentGroupsOf(AccountGroup.UUID groupId);
/** @return set of any UUIDs that are not internal groups. */
public Set<AccountGroup.UUID> allExternalMembers();
- public void evictMembersOf(AccountGroup.UUID groupId);
- public void evictMemberIn(AccountGroup.UUID groupId);
+ public void evictSubgroupsOf(AccountGroup.UUID groupId);
+ public void evictParentGroupsOf(AccountGroup.UUID groupId);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java
index 37d407c..9e7918d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java
@@ -42,23 +42,23 @@
public class GroupIncludeCacheImpl implements GroupIncludeCache {
private static final Logger log = LoggerFactory
.getLogger(GroupIncludeCacheImpl.class);
- private static final String BYINCLUDE_NAME = "groups_byinclude";
- private static final String MEMBERS_NAME = "groups_members";
+ private static final String PARENT_GROUPS_NAME = "groups_byinclude";
+ private static final String SUBGROUPS_NAME = "groups_members";
private static final String EXTERNAL_NAME = "groups_external";
public static Module module() {
return new CacheModule() {
@Override
protected void configure() {
- cache(BYINCLUDE_NAME,
+ cache(PARENT_GROUPS_NAME,
AccountGroup.UUID.class,
new TypeLiteral<Set<AccountGroup.UUID>>() {})
- .loader(MemberInLoader.class);
+ .loader(ParentGroupsLoader.class);
- cache(MEMBERS_NAME,
+ cache(SUBGROUPS_NAME,
AccountGroup.UUID.class,
new TypeLiteral<Set<AccountGroup.UUID>>() {})
- .loader(MembersOfLoader.class);
+ .loader(SubgroupsLoader.class);
cache(EXTERNAL_NAME,
String.class,
@@ -71,24 +71,24 @@
};
}
- private final LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> membersOf;
- private final LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> memberIn;
+ private final LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> subgroups;
+ private final LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> parentGroups;
private final LoadingCache<String, Set<AccountGroup.UUID>> external;
@Inject
GroupIncludeCacheImpl(
- @Named(MEMBERS_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> membersOf,
- @Named(BYINCLUDE_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> memberIn,
+ @Named(SUBGROUPS_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> subgroups,
+ @Named(PARENT_GROUPS_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> parentGroups,
@Named(EXTERNAL_NAME) LoadingCache<String, Set<AccountGroup.UUID>> external) {
- this.membersOf = membersOf;
- this.memberIn = memberIn;
+ this.subgroups = subgroups;
+ this.parentGroups = parentGroups;
this.external = external;
}
@Override
- public Set<AccountGroup.UUID> membersOf(AccountGroup.UUID groupId) {
+ public Set<AccountGroup.UUID> subgroupsOf(AccountGroup.UUID groupId) {
try {
- return membersOf.get(groupId);
+ return subgroups.get(groupId);
} catch (ExecutionException e) {
log.warn("Cannot load members of group", e);
return Collections.emptySet();
@@ -96,9 +96,9 @@
}
@Override
- public Set<AccountGroup.UUID> memberIn(AccountGroup.UUID groupId) {
+ public Set<AccountGroup.UUID> parentGroupsOf(AccountGroup.UUID groupId) {
try {
- return memberIn.get(groupId);
+ return parentGroups.get(groupId);
} catch (ExecutionException e) {
log.warn("Cannot load included groups", e);
return Collections.emptySet();
@@ -106,16 +106,16 @@
}
@Override
- public void evictMembersOf(AccountGroup.UUID groupId) {
+ public void evictSubgroupsOf(AccountGroup.UUID groupId) {
if (groupId != null) {
- membersOf.invalidate(groupId);
+ subgroups.invalidate(groupId);
}
}
@Override
- public void evictMemberIn(AccountGroup.UUID groupId) {
+ public void evictParentGroupsOf(AccountGroup.UUID groupId) {
if (groupId != null) {
- memberIn.invalidate(groupId);
+ parentGroups.invalidate(groupId);
if (!AccountGroup.isInternalGroup(groupId)) {
external.invalidate(EXTERNAL_NAME);
@@ -133,12 +133,12 @@
}
}
- static class MembersOfLoader extends
+ static class SubgroupsLoader extends
CacheLoader<AccountGroup.UUID, Set<AccountGroup.UUID>> {
private final SchemaFactory<ReviewDb> schema;
@Inject
- MembersOfLoader(final SchemaFactory<ReviewDb> sf) {
+ SubgroupsLoader(final SchemaFactory<ReviewDb> sf) {
schema = sf;
}
@@ -163,12 +163,12 @@
}
}
- static class MemberInLoader extends
+ static class ParentGroupsLoader extends
CacheLoader<AccountGroup.UUID, Set<AccountGroup.UUID>> {
private final SchemaFactory<ReviewDb> schema;
@Inject
- MemberInLoader(final SchemaFactory<ReviewDb> sf) {
+ ParentGroupsLoader(final SchemaFactory<ReviewDb> sf) {
schema = sf;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/IncludingGroupMembership.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/IncludingGroupMembership.java
index 2e278c3..b8a67ff 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/IncludingGroupMembership.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/IncludingGroupMembership.java
@@ -90,7 +90,7 @@
}
memberOf.put(id, false);
- if (search(includeCache.membersOf(id))) {
+ if (search(includeCache.subgroupsOf(id))) {
memberOf.put(id, true);
return true;
}
@@ -131,7 +131,7 @@
while (!q.isEmpty()) {
AccountGroup.UUID id = q.remove(q.size() - 1);
- for (AccountGroup.UUID g : includeCache.memberIn(id)) {
+ for (AccountGroup.UUID g : includeCache.parentGroupsOf(id)) {
if (g != null && r.add(g)) {
q.add(g);
memberOf.put(g, true);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java
index a54a97b..86c840b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java
@@ -116,7 +116,7 @@
if (createGroupArgs.initialGroups != null) {
addGroups(groupId, createGroupArgs.initialGroups);
- groupIncludeCache.evictMembersOf(uuid);
+ groupIncludeCache.evictSubgroupsOf(uuid);
}
groupCache.onCreateGroup(createGroupArgs.getGroup());
@@ -162,7 +162,7 @@
db.accountGroupByIdAud().insert(includesAudit);
for (AccountGroup.UUID uuid : groups) {
- groupIncludeCache.evictMemberIn(uuid);
+ groupIncludeCache.evictParentGroupsOf(uuid);
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java
index 5a19814..f8ecf25 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java
@@ -17,6 +17,7 @@
import com.google.common.base.Throwables;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
import com.google.gerrit.common.data.ParameterizedString;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.account.AccountException;
@@ -37,6 +38,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.TimeUnit;
@@ -58,7 +60,7 @@
@Singleton class Helper {
static final String LDAP_UUID = "ldap:";
- private final Cache<String, ImmutableSet<String>> groupsByInclude;
+ private final Cache<String, ImmutableSet<String>> parentGroups;
private final Config config;
private final String server;
private final String username;
@@ -67,28 +69,41 @@
private final boolean sslVerify;
private final String authentication;
private volatile LdapSchema ldapSchema;
- private final String readTimeOutMillis;
+ private final String readTimeoutMillis;
+ private final String connectTimeoutMillis;
+ private final boolean useConnectionPooling;
@Inject
Helper(@GerritServerConfig final Config config,
- @Named(LdapModule.GROUPS_BYINCLUDE_CACHE)
- Cache<String, ImmutableSet<String>> groupsByInclude) {
+ @Named(LdapModule.PARENT_GROUPS_CACHE)
+ Cache<String, ImmutableSet<String>> parentGroups) {
this.config = config;
this.server = LdapRealm.optional(config, "server");
this.username = LdapRealm.optional(config, "username");
- this.password = LdapRealm.optional(config, "password");
- this.referral = LdapRealm.optional(config, "referral");
+ this.password = LdapRealm.optional(config, "password", "");
+ this.referral = LdapRealm.optional(config, "referral", "ignore");
this.sslVerify = config.getBoolean("ldap", "sslverify", true);
- this.authentication = LdapRealm.optional(config, "authentication");
- String timeout = LdapRealm.optional(config, "readTimeout");
- if (timeout != null) {
- readTimeOutMillis =
- Long.toString(ConfigUtil.getTimeUnit(timeout, 0,
+ this.authentication =
+ LdapRealm.optional(config, "authentication", "simple");
+ String readTimeout = LdapRealm.optional(config, "readTimeout");
+ if (readTimeout != null) {
+ readTimeoutMillis =
+ Long.toString(ConfigUtil.getTimeUnit(readTimeout, 0,
TimeUnit.MILLISECONDS));
} else {
- readTimeOutMillis = null;
+ readTimeoutMillis = null;
}
- this.groupsByInclude = groupsByInclude;
+ String connectTimeout = LdapRealm.optional(config, "connectTimeout");
+ if (connectTimeout != null) {
+ connectTimeoutMillis =
+ Long.toString(ConfigUtil.getTimeUnit(connectTimeout, 0,
+ TimeUnit.MILLISECONDS));
+ } else {
+ connectTimeoutMillis = null;
+ }
+ this.parentGroups = parentGroups;
+ this.useConnectionPooling =
+ LdapRealm.optional(config, "useConnectionPooling", false);
}
private Properties createContextProperties() {
@@ -99,22 +114,28 @@
Class<? extends SSLSocketFactory> factory = BlindSSLSocketFactory.class;
env.put("java.naming.ldap.factory.socket", factory.getName());
}
- if (readTimeOutMillis != null) {
- env.put("com.sun.jndi.ldap.read.timeout", readTimeOutMillis);
+ if (readTimeoutMillis != null) {
+ env.put("com.sun.jndi.ldap.read.timeout", readTimeoutMillis);
+ }
+ if (connectTimeoutMillis != null) {
+ env.put("com.sun.jndi.ldap.connect.timeout", connectTimeoutMillis);
+ }
+ if (useConnectionPooling) {
+ env.put("com.sun.jndi.ldap.connect.pool", "true");
}
return env;
}
DirContext open() throws NamingException, LoginException {
final Properties env = createContextProperties();
- env.put(Context.SECURITY_AUTHENTICATION, authentication != null ? authentication : "simple");
- env.put(Context.REFERRAL, referral != null ? referral : "ignore");
+ env.put(Context.SECURITY_AUTHENTICATION, authentication);
+ env.put(Context.REFERRAL, referral);
if ("GSSAPI".equals(authentication)) {
return kerberosOpen(env);
} else {
if (username != null) {
env.put(Context.SECURITY_PRINCIPAL, username);
- env.put(Context.SECURITY_CREDENTIALS, password != null ? password : "");
+ env.put(Context.SECURITY_CREDENTIALS, password);
}
return new InitialDirContext(env);
}
@@ -146,8 +167,8 @@
final Properties env = createContextProperties();
env.put(Context.SECURITY_AUTHENTICATION, "simple");
env.put(Context.SECURITY_PRINCIPAL, dn);
- env.put(Context.SECURITY_CREDENTIALS, password != null ? password : "");
- env.put(Context.REFERRAL, referral != null ? referral : "ignore");
+ env.put(Context.SECURITY_CREDENTIALS, password);
+ env.put(Context.REFERRAL, referral);
try {
return new InitialDirContext(env);
} catch (NamingException e) {
@@ -166,27 +187,28 @@
return ldapSchema;
}
- LdapQuery.Result findAccount(final Helper.LdapSchema schema,
- final DirContext ctx, final String username) throws NamingException,
- AccountException {
+ LdapQuery.Result findAccount(Helper.LdapSchema schema,
+ DirContext ctx, String username, boolean fetchMemberOf)
+ throws NamingException, AccountException {
final HashMap<String, String> params = new HashMap<>();
params.put(LdapRealm.USERNAME, username);
- final List<LdapQuery.Result> res = new ArrayList<>();
- for (LdapQuery accountQuery : schema.accountQueryList) {
- res.addAll(accountQuery.query(ctx, params));
+ List<LdapQuery> accountQueryList;
+ if (fetchMemberOf && schema.type.accountMemberField() != null) {
+ accountQueryList = schema.accountWithMemberOfQueryList;
+ } else {
+ accountQueryList = schema.accountQueryList;
}
- switch (res.size()) {
- case 0:
- throw new NoSuchUserException(username);
-
- case 1:
+ for (LdapQuery accountQuery : accountQueryList) {
+ List<LdapQuery.Result> res = accountQuery.query(ctx, params);
+ if (res.size() == 1) {
return res.get(0);
-
- default:
+ } else if (res.size() > 1) {
throw new AccountException("Duplicate users: " + username);
+ }
}
+ throw new NoSuchUserException(username);
}
Set<AccountGroup.UUID> queryForGroups(final DirContext ctx,
@@ -200,7 +222,7 @@
if (account == null) {
try {
- account = findAccount(schema, ctx, username);
+ account = findAccount(schema, ctx, username, false);
} catch (AccountException e) {
LdapRealm.log.warn("Account " + username +
" not found, assuming empty group membership");
@@ -221,9 +243,9 @@
}
if (schema.accountMemberField != null) {
- if (account == null) {
+ if (account == null || account.getAll(schema.accountMemberField) == null) {
try {
- account = findAccount(schema, ctx, username);
+ account = findAccount(schema, ctx, username, true);
} catch (AccountException e) {
LdapRealm.log.warn("Account " + username +
" not found, assuming empty group membership");
@@ -259,14 +281,15 @@
private void recursivelyExpandGroups(final Set<String> groupDNs,
final LdapSchema schema, final DirContext ctx, final String groupDN) {
if (groupDNs.add(groupDN) && schema.accountMemberField != null) {
- ImmutableSet<String> cachedGroupDNs = groupsByInclude.getIfPresent(groupDN);
- if (cachedGroupDNs == null) {
+ ImmutableSet<String> cachedParentsDNs = parentGroups.getIfPresent(groupDN);
+ if (cachedParentsDNs == null) {
// Recursively identify the groups it is a member of.
ImmutableSet.Builder<String> dns = ImmutableSet.builder();
try {
final Name compositeGroupName = new CompositeName().add(groupDN);
final Attribute in =
- ctx.getAttributes(compositeGroupName).get(schema.accountMemberField);
+ ctx.getAttributes(compositeGroupName, schema.accountMemberFieldArray)
+ .get(schema.accountMemberField);
if (in != null) {
final NamingEnumeration<?> groups = in.getAll();
try {
@@ -279,10 +302,10 @@
} catch (NamingException e) {
LdapRealm.log.warn("Could not find group " + groupDN, e);
}
- cachedGroupDNs = dns.build();
- groupsByInclude.put(groupDN, cachedGroupDNs);
+ cachedParentsDNs = dns.build();
+ parentGroups.put(groupDN, cachedParentsDNs);
}
- for (String dn : cachedGroupDNs) {
+ for (String dn : cachedParentsDNs) {
recursivelyExpandGroups(groupDNs, schema, ctx, dn);
}
}
@@ -295,7 +318,9 @@
final ParameterizedString accountEmailAddress;
final ParameterizedString accountSshUserName;
final String accountMemberField;
+ final String[] accountMemberFieldArray;
final List<LdapQuery> accountQueryList;
+ final List<LdapQuery> accountWithMemberOfQueryList;
final List<String> groupBases;
final SearchScope groupScope;
@@ -307,6 +332,7 @@
type = discoverLdapType(ctx);
groupMemberQueryList = new ArrayList<>();
accountQueryList = new ArrayList<>();
+ accountWithMemberOfQueryList = new ArrayList<>();
final Set<String> accountAtts = new HashSet<>();
@@ -359,15 +385,24 @@
accountMemberField =
LdapRealm.optdef(config, "accountMemberField", type.accountMemberField());
if (accountMemberField != null) {
- accountAtts.add(accountMemberField);
+ accountMemberFieldArray = new String[] {accountMemberField};
+ } else {
+ accountMemberFieldArray = null;
}
final SearchScope accountScope = LdapRealm.scope(config, "accountScope");
final String accountPattern =
LdapRealm.reqdef(config, "accountPattern", type.accountPattern());
+ Set<String> accountWithMemberOfAtts;
+ if (accountMemberField != null) {
+ accountWithMemberOfAtts = new HashSet<>(accountAtts);
+ accountWithMemberOfAtts.add(accountMemberField);
+ } else {
+ accountWithMemberOfAtts = null;
+ }
for (String accountBase : LdapRealm.requiredList(config, "accountBase")) {
- final LdapQuery accountQuery =
+ LdapQuery accountQuery =
new LdapQuery(accountBase, accountScope, new ParameterizedString(
accountPattern), accountAtts);
if (accountQuery.getParameters().isEmpty()) {
@@ -375,6 +410,13 @@
"No variables in ldap.accountPattern");
}
accountQueryList.add(accountQuery);
+
+ if (accountWithMemberOfAtts != null) {
+ LdapQuery accountWithMemberOfQuery =
+ new LdapQuery(accountBase, accountScope, new ParameterizedString(
+ accountPattern), accountWithMemberOfAtts);
+ accountWithMemberOfQueryList.add(accountWithMemberOfQuery);
+ }
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java
index eb6249c..1d90f0c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java
@@ -83,7 +83,7 @@
}
try {
final Helper.LdapSchema schema = helper.getSchema(ctx);
- final LdapQuery.Result m = helper.findAccount(schema, ctx, username);
+ final LdapQuery.Result m = helper.findAccount(schema, ctx, username, false);
if (authConfig.getAuthType() == AuthType.LDAP) {
// We found the user account, but we need to verify
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
index 7731b7d..6cdce8d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
@@ -31,7 +31,6 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupMembership;
-import com.google.gerrit.server.account.ListGroupMembership;
import com.google.gerrit.server.auth.ldap.Helper.LdapSchema;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
@@ -60,7 +59,7 @@
* Implementation of GroupBackend for the LDAP group system.
*/
public class LdapGroupBackend implements GroupBackend {
- private static final Logger log = LoggerFactory.getLogger(LdapGroupBackend.class);
+ static final Logger log = LoggerFactory.getLogger(LdapGroupBackend.class);
private static final String LDAP_NAME = "ldap/";
private static final String GROUPNAME = "groupname";
@@ -185,21 +184,7 @@
if (id == null) {
return GroupMembership.EMPTY;
}
-
- try {
- final Set<AccountGroup.UUID> groups = membershipCache.get(id);
- return new ListGroupMembership(groups) {
- @Override
- public Set<AccountGroup.UUID> getKnownGroups() {
- Set<AccountGroup.UUID> g = Sets.newHashSet(groups);
- g.retainAll(projectCache.guessRelevantGroupUUIDs());
- return g;
- }
- };
- } catch (ExecutionException e) {
- log.warn(String.format("Cannot lookup membershipsOf %s in LDAP", id), e);
- return GroupMembership.EMPTY;
- }
+ return new LdapGroupMembership(membershipCache, projectCache, id);
}
private static String findId(final Collection<AccountExternalId> ids) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupMembership.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupMembership.java
new file mode 100644
index 0000000..7853320
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupMembership.java
@@ -0,0 +1,76 @@
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.auth.ldap;
+
+import com.google.common.cache.LoadingCache;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.account.GroupMembership;
+import com.google.gerrit.server.account.ListGroupMembership;
+import com.google.gerrit.server.project.ProjectCache;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+
+class LdapGroupMembership implements GroupMembership {
+ private final LoadingCache<String, Set<AccountGroup.UUID>> membershipCache;
+ private final ProjectCache projectCache;
+ private final String id;
+ private GroupMembership membership;
+
+ LdapGroupMembership(
+ LoadingCache<String, Set<AccountGroup.UUID>> membershipCache,
+ ProjectCache projectCache,
+ String id) {
+ this.membershipCache = membershipCache;
+ this.projectCache = projectCache;
+ this.id = id;
+ }
+
+ @Override
+ public boolean contains(AccountGroup.UUID groupId) {
+ return get().contains(groupId);
+ }
+
+ @Override
+ public boolean containsAnyOf(Iterable<AccountGroup.UUID> groupIds) {
+ return get().containsAnyOf(groupIds);
+ }
+
+ @Override
+ public Set<AccountGroup.UUID> intersection(Iterable<AccountGroup.UUID> groupIds) {
+ return get().intersection(groupIds);
+ }
+
+ @Override
+ public Set<AccountGroup.UUID> getKnownGroups() {
+ Set<AccountGroup.UUID> g = new HashSet<>(get().getKnownGroups());
+ g.retainAll(projectCache.guessRelevantGroupUUIDs());
+ return g;
+ }
+
+ private synchronized GroupMembership get() {
+ if (membership == null) {
+ try {
+ membership = new ListGroupMembership(membershipCache.get(id));
+ } catch (ExecutionException e) {
+ LdapGroupBackend.log.warn(String.format(
+ "Cannot lookup membershipsOf %s in LDAP", id), e);
+ membership = GroupMembership.EMPTY;
+ }
+ }
+ return membership;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java
index 88cf45b..eaaafd6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java
@@ -33,7 +33,7 @@
static final String USERNAME_CACHE = "ldap_usernames";
static final String GROUP_CACHE = "ldap_groups";
static final String GROUP_EXIST_CACHE = "ldap_group_existence";
- static final String GROUPS_BYINCLUDE_CACHE = "ldap_groups_byinclude";
+ static final String PARENT_GROUPS_CACHE = "ldap_groups_byinclude";
@Override
@@ -55,7 +55,7 @@
.expireAfterWrite(1, HOURS)
.loader(LdapRealm.ExistenceLoader.class);
- cache(GROUPS_BYINCLUDE_CACHE,
+ cache(PARENT_GROUPS_CACHE,
String.class,
new TypeLiteral<ImmutableSet<String>>() {})
.expireAfterWrite(1, HOURS);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
index 8bb481c..7b79add 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
@@ -69,6 +69,7 @@
private final EmailExpander emailExpander;
private final LoadingCache<String, Optional<Account.Id>> usernameCache;
private final Set<Account.FieldName> readOnlyAccountFields;
+ private final boolean fetchMemberOfEagerly;
private final Config config;
private final LoadingCache<String, Set<AccountGroup.UUID>> membershipCache;
@@ -96,6 +97,8 @@
if (optdef(config, "accountSshUserName", "DEFAULT") != null) {
readOnlyAccountFields.add(Account.FieldName.USER_NAME);
}
+
+ fetchMemberOfEagerly = optional(config, "fetchMemberOfEagerly", true);
}
static SearchScope scope(final Config c, final String setting) {
@@ -106,6 +109,22 @@
return config.getString("ldap", null, name);
}
+ static int optional(Config config, String name, int defaultValue) {
+ return config.getInt("ldap", name, defaultValue);
+ }
+
+ static String optional(Config config, String name, String defaultValue) {
+ final String v = optional(config, name);
+ if (Strings.isNullOrEmpty(v)) {
+ return defaultValue;
+ }
+ return v;
+ }
+
+ static boolean optional(Config config, String name, boolean defaultValue) {
+ return config.getBoolean("ldap", name, defaultValue);
+ }
+
static String required(final Config config, final String name) {
final String v = optional(config, name);
if (v == null || "".equals(v)) {
@@ -200,7 +219,8 @@
}
try {
final Helper.LdapSchema schema = helper.getSchema(ctx);
- final LdapQuery.Result m = helper.findAccount(schema, ctx, username);
+ final LdapQuery.Result m = helper.findAccount(schema, ctx, username,
+ fetchMemberOfEagerly);
if (authConfig.getAuthType() == AuthType.LDAP && !who.isSkipAuthentication()) {
// We found the user account, but we need to verify
@@ -229,7 +249,9 @@
// in the middle of authenticating the user, its likely we will
// need to know what access rights they have soon.
//
- membershipCache.put(username, helper.queryForGroups(ctx, username, m));
+ if (fetchMemberOfEagerly) {
+ membershipCache.put(username, helper.queryForGroups(ctx, username, m));
+ }
return who;
} finally {
try {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java
index 614138a..163b335 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java
@@ -128,9 +128,9 @@
db.get().accountGroupByIdAud().insert(newIncludedGroupsAudits);
db.get().accountGroupById().insert(newIncludedGroups.values());
for (AccountGroupById agi : newIncludedGroups.values()) {
- groupIncludeCache.evictMemberIn(agi.getIncludeUUID());
+ groupIncludeCache.evictParentGroupsOf(agi.getIncludeUUID());
}
- groupIncludeCache.evictMembersOf(group.getGroupUUID());
+ groupIncludeCache.evictSubgroupsOf(group.getGroupUUID());
}
return result;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteIncludedGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteIncludedGroups.java
index 555744e..8cc1a4a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteIncludedGroups.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteIncludedGroups.java
@@ -91,9 +91,9 @@
writeAudits(toRemove);
db.get().accountGroupById().delete(toRemove);
for (final AccountGroupById g : toRemove) {
- groupIncludeCache.evictMemberIn(g.getIncludeUUID());
+ groupIncludeCache.evictParentGroupsOf(g.getIncludeUUID());
}
- groupIncludeCache.evictMembersOf(internalGroup.getGroupUUID());
+ groupIncludeCache.evictSubgroupsOf(internalGroup.getGroupUUID());
}
return Response.none();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
index 41dfba5..f067e27 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
@@ -436,7 +436,10 @@
@Override
public Integer get(ChangeData input, FillArgs args)
throws OrmException {
- return input.changedLines().insertions;
+
+ return input.changedLines() != null
+ ? input.changedLines().insertions
+ : null;
}
};
@@ -447,7 +450,9 @@
@Override
public Integer get(ChangeData input, FillArgs args)
throws OrmException {
- return input.changedLines().deletions;
+ return input.changedLines() != null
+ ? input.changedLines().deletions
+ : null;
}
};
@@ -459,7 +464,9 @@
public Integer get(ChangeData input, FillArgs args)
throws OrmException {
ChangedLines changedLines = input.changedLines();
- return changedLines.insertions + changedLines.deletions;
+ return changedLines != null
+ ? changedLines.insertions + changedLines.deletions
+ : null;
}
};
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ProjectWatch.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ProjectWatch.java
index 3d16372..872b84a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ProjectWatch.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ProjectWatch.java
@@ -166,7 +166,7 @@
for (AccountGroupMember m : db.accountGroupMembers().byGroup(ig.getId())) {
matching.accounts.add(m.getAccountId());
}
- for (AccountGroup.UUID m : args.groupIncludes.membersOf(uuid)) {
+ for (AccountGroup.UUID m : args.groupIncludes.subgroupsOf(uuid)) {
if (seen.add(m)) {
q.add(m);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListBranches.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListBranches.java
index 7c8b29f..b09e39f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListBranches.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListBranches.java
@@ -32,10 +32,10 @@
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
@@ -72,30 +72,34 @@
}
try {
- final Map<String, Ref> all = db.getRefDatabase().getRefs(RefDatabase.ALL);
+ List<Ref> refs =
+ new ArrayList<>(db.getRefDatabase().getRefs(Constants.R_HEADS)
+ .values());
- if (!all.containsKey(Constants.HEAD)) {
- // The branch pointed to by HEAD doesn't exist yet, so getAllRefs
- // filtered it out. If we ask for it individually we can find the
- // underlying target and put it into the map anyway.
- //
try {
Ref head = db.getRef(Constants.HEAD);
if (head != null) {
- all.put(Constants.HEAD, head);
+ refs.add(head);
}
} catch (IOException e) {
// Ignore the failure reading HEAD.
}
- }
+ try {
+ Ref config = db.getRef(RefNames.REFS_CONFIG);
+ if (config != null) {
+ refs.add(config);
+ }
+ } catch (IOException e) {
+ // Ignore the failure reading refs/meta/config.
+ }
- for (final Ref ref : all.values()) {
+ for (Ref ref : refs) {
if (ref.isSymbolic()) {
targets.add(ref.getTarget().getName());
}
}
- for (final Ref ref : all.values()) {
+ for (Ref ref : refs) {
if (ref.isSymbolic()) {
// A symbolic reference to another branch, instead of
// showing the resolved value, show the name it references.
@@ -122,10 +126,10 @@
final RefControl refControl = rsrc.getControl().controlForRef(ref.getName());
if (refControl.isVisible()) {
- if (ref.getName().startsWith(Constants.R_HEADS)) {
- branches.add(createBranchInfo(ref, refControl, targets));
- } else if (RefNames.REFS_CONFIG.equals(ref.getName())) {
+ if (RefNames.REFS_CONFIG.equals(ref.getName())) {
configBranch = createBranchInfo(ref, refControl, targets);
+ } else {
+ branches.add(createBranchInfo(ref, refControl, targets));
}
}
}
