From a750617d39d872434960f66a50e2863492c9ea93 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Tue, 23 Jul 2013 16:41:47 -0700 Subject: [PATCH] Define AccountDirectory as an abstract backend for user data Eventually this will fully support all account information, similar to GroupBackend. For now lets start by defining a simple base class that can supply name, email and avatar URLs to an AccountInfo for the REST API. Bind this in the Daemon and web application code so its at least partially pluggable at the static Guice injector level. Change-Id: I6e1b55fd46f3e99a982f7bd523f3e9d91b637b0d --- .../java/com/google/gerrit/pgm/Daemon.java | 2 + .../server/account/AccountDirectory.java | 59 +++++++++ .../gerrit/server/account/AccountInfo.java | 98 ++++++--------- .../account/InternalAccountDirectory.java | 116 ++++++++++++++++++ .../gerrit/httpd/WebAppInitializer.java | 2 + 5 files changed, 217 insertions(+), 60 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/account/AccountDirectory.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/account/InternalAccountDirectory.java diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java index a10e7e495f..24c33cdd6c 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java @@ -39,6 +39,7 @@ import com.google.gerrit.pgm.util.LogFileCompressor; import com.google.gerrit.pgm.util.RuntimeShutdown; import com.google.gerrit.pgm.util.SiteProgram; import com.google.gerrit.reviewdb.client.AuthType; +import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfigModule; @@ -250,6 +251,7 @@ public class Daemon extends SiteProgram { modules.add(new ReceiveCommitsExecutorModule()); modules.add(new IntraLineWorkerPool.Module()); modules.add(cfgInjector.getInstance(GerritGlobalModule.class)); + modules.add(new InternalAccountDirectory.Module()); modules.add(new DefaultCacheFactory.Module()); modules.add(new SmtpEmailSender.Module()); modules.add(new SignedTokenEmailTokenVerifier.Module()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountDirectory.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountDirectory.java new file mode 100644 index 0000000000..2bb1d1faa1 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountDirectory.java @@ -0,0 +1,59 @@ +// Copyright (C) 2013 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.account; + +import java.util.EnumSet; + +/** + * Directory of user account information. + * + * Implementations supply data to Gerrit about user accounts. + */ +public abstract class AccountDirectory { + /** Fields to be populated for a REST API response. */ + public enum FillOptions { + /** Human friendly display name presented in the web interface. */ + NAME, + + /** Preferred email address to contact the user at. */ + EMAIL, + + /** User profile images. */ + AVATARS, + + /** Unique user identity to login to Gerrit, may be deprecated. */ + USERNAME; + } + + public abstract void fillAccountInfo( + Iterable in, + EnumSet options) + throws DirectoryException; + + @SuppressWarnings("serial") + public static class DirectoryException extends Exception { + public DirectoryException(String message) { + super(message); + } + + public DirectoryException(String message, Throwable why) { + super(message, why); + } + + public DirectoryException(Throwable why) { + super(why); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfo.java index 4b8a9e8045..c4e72f348e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfo.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfo.java @@ -14,50 +14,41 @@ package com.google.gerrit.server.account; -import com.google.common.base.Strings; -import com.google.common.collect.ArrayListMultimap; +import com.google.common.base.Throwables; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import com.google.common.collect.Multimap; -import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.avatar.AvatarProvider; +import com.google.gerrit.server.account.AccountDirectory.DirectoryException; +import com.google.gerrit.server.account.AccountDirectory.FillOptions; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; import java.util.Collection; +import java.util.EnumSet; import java.util.List; import java.util.Map; public class AccountInfo { public static class Loader { + private static EnumSet DETAILED_OPTIONS = EnumSet.of( + FillOptions.NAME, + FillOptions.EMAIL, + FillOptions.AVATARS); + public interface Factory { Loader create(boolean detailed); } - private final Provider db; - private final AccountCache accountCache; - private final DynamicItem avatar; - private final IdentifiedUser.GenericFactory userFactory; + private final InternalAccountDirectory directory; private final boolean detailed; private final Map created; private final List provided; @Inject - Loader(Provider db, - AccountCache accountCache, - DynamicItem avatar, - IdentifiedUser.GenericFactory userFactory, - @Assisted boolean detailed) { - this.db = db; - this.accountCache = accountCache; - this.avatar = avatar; - this.userFactory = userFactory; + Loader(InternalAccountDirectory directory, @Assisted boolean detailed) { + this.directory = directory; this.detailed = detailed; created = Maps.newHashMap(); provided = Lists.newArrayList(); @@ -70,31 +61,29 @@ public class AccountInfo { AccountInfo info = created.get(id); if (info == null) { info = new AccountInfo(id); + if (detailed) { + info._account_id = id.get(); + } created.put(id, info); } return info; } public void put(AccountInfo info) { + if (detailed) { + info._account_id = info._id.get(); + } provided.add(info); } public void fill() throws OrmException { - Multimap missing = ArrayListMultimap.create(); - for (AccountInfo info : Iterables.concat(created.values(), provided)) { - AccountState state = accountCache.getIfPresent(info._id); - if (state != null) { - fill(info, state.getAccount()); - } else { - missing.put(info._id, info); - } - } - if (!missing.isEmpty()) { - for (Account account : db.get().accounts().get(missing.keySet())) { - for (AccountInfo info : missing.get(account.getId())) { - fill(info, account); - } - } + try { + directory.fillAccountInfo( + Iterables.concat(created.values(), provided), + DETAILED_OPTIONS); + } catch (DirectoryException e) { + Throwables.propagateIfPossible(e.getCause(), OrmException.class); + throw new OrmException(e); } } @@ -105,28 +94,6 @@ public class AccountInfo { } fill(); } - - private void fill(AccountInfo info, Account account) { - info.name = Strings.emptyToNull(account.getFullName()); - if (info.name == null) { - info.name = account.getUserName(); - } - - if (detailed) { - info._account_id = account.getId().get(); - info.email = account.getPreferredEmail(); - info.username = account.getUserName(); - } - - info.avatars = Lists.newArrayListWithCapacity(1); - AvatarProvider ap = avatar.get(); - if (ap != null) { - String u = ap.getUrl(userFactory.create(account.getId()), 26); - if (u != null) { - info.avatars.add(new AvatarInfo(u, 26)); - } - } - } } public transient Account.Id _id; @@ -142,8 +109,19 @@ public class AccountInfo { public List avatars; public static class AvatarInfo { - String url; - int height; + /** + * Size in pixels the UI prefers an avatar image to be. + * + * The web UI prefers avatar images to be square, both + * the height and width of the image should be this size. + * The height is the more important dimension to match + * than the width. + */ + public static final int DEFAULT_SIZE = 26; + + public String url; + public Integer height; + public Integer width; AvatarInfo(String url, int height) { this.url = url; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalAccountDirectory.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalAccountDirectory.java new file mode 100644 index 0000000000..90731d1ab7 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalAccountDirectory.java @@ -0,0 +1,116 @@ +// Copyright (C) 2013 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.account; + +import com.google.common.base.Strings; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Lists; +import com.google.common.collect.Multimap; +import com.google.gerrit.extensions.registration.DynamicItem; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.account.AccountInfo.AvatarInfo; +import com.google.gerrit.server.avatar.AvatarProvider; +import com.google.gwtorm.server.OrmException; +import com.google.inject.AbstractModule; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +import java.util.EnumSet; + +@Singleton +public class InternalAccountDirectory extends AccountDirectory { + public static class Module extends AbstractModule { + @Override + protected void configure() { + bind(AccountDirectory.class).to(InternalAccountDirectory.class); + } + } + + private final Provider db; + private final AccountCache accountCache; + private final DynamicItem avatar; + private final IdentifiedUser.GenericFactory userFactory; + + @Inject + InternalAccountDirectory(Provider db, + AccountCache accountCache, + DynamicItem avatar, + IdentifiedUser.GenericFactory userFactory) { + this.db = db; + this.accountCache = accountCache; + this.avatar = avatar; + this.userFactory = userFactory; + } + + @Override + public void fillAccountInfo( + Iterable in, + EnumSet options) + throws DirectoryException { + Multimap missing = ArrayListMultimap.create(); + for (AccountInfo info : in) { + AccountState state = accountCache.getIfPresent(info._id); + if (state != null) { + fill(info, state.getAccount(), options); + } else { + missing.put(info._id, info); + } + } + if (!missing.isEmpty()) { + try { + for (Account account : db.get().accounts().get(missing.keySet())) { + for (AccountInfo info : missing.get(account.getId())) { + fill(info, account, options); + } + } + } catch (OrmException e) { + throw new DirectoryException(e); + } + } + } + + private void fill(AccountInfo info, + Account account, + EnumSet options) { + if (options.contains(FillOptions.NAME)) { + info.name = Strings.emptyToNull(account.getFullName()); + if (info.name == null) { + info.name = account.getUserName(); + } + } + if (options.contains(FillOptions.EMAIL)) { + info.email = account.getPreferredEmail(); + } + if (options.contains(FillOptions.USERNAME)) { + info.username = account.getUserName(); + } + if (options.contains(FillOptions.AVATARS)) { + info.avatars = Lists.newArrayListWithCapacity(1); + AvatarProvider ap = avatar.get(); + if (ap != null) { + String u = ap.getUrl( + userFactory.create(account.getId()), + AvatarInfo.DEFAULT_SIZE); + if (u != null) { + info.avatars.add(new AvatarInfo(u, AvatarInfo.DEFAULT_SIZE)); + } + } + } + } +} diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java index da08555e55..e53a6fc945 100644 --- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java +++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java @@ -24,6 +24,7 @@ import com.google.gerrit.lifecycle.LifecycleManager; import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.lucene.LuceneIndexModule; import com.google.gerrit.reviewdb.client.AuthType; +import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfigModule; @@ -235,6 +236,7 @@ public class WebAppInitializer extends GuiceServletContextListener { modules.add(new ReceiveCommitsExecutorModule()); modules.add(new IntraLineWorkerPool.Module()); modules.add(cfgInjector.getInstance(GerritGlobalModule.class)); + modules.add(new InternalAccountDirectory.Module()); modules.add(new DefaultCacheFactory.Module()); modules.add(new SmtpEmailSender.Module()); modules.add(new SignedTokenEmailTokenVerifier.Module());