From 54ba43a51dc00260dc6ceb72a329174aa6f2c17f Mon Sep 17 00:00:00 2001 From: Dave Borowitz <dborowitz@google.com> Date: Tue, 25 Nov 2014 14:41:05 -0500 Subject: [PATCH] Replace account.AccountInfo with extensions.common.AccountInfo These have identical public fields; the only difference is that AccountInfo.Loader was using the transient _id field store the ID before filling in values. We can instead reuse the Integer _accountId field for this purpose. This may incur an additional boxing or so per account per request, but that cost should be negligible given how many allocations we do per request already. AccountInfoMapper is no longer required, so remove that entirely. We are now enforcing that AccountInfos passed to the loader have an _accountId. Since by default the old account.AccountInfo worked without an _accountId, add this as a separate FillOption and clear it if that option is not set. To verify this, check the owner fields in ChangeIT#get. Change-Id: Ied01750485cc728e91e585a31f3db8863bae7e40 --- .../acceptance/api/change/ChangeIT.java | 6 + .../rest/account/AccountAssert.java | 2 +- .../acceptance/rest/account/GetAccountIT.java | 2 +- .../rest/group/AddRemoveGroupMembersIT.java | 2 +- .../rest/group/ListGroupMembersIT.java | 2 +- .../gerrit/extensions/common/AccountInfo.java | 7 + .../extensions/common/ApprovalInfo.java | 4 + .../gerrit/extensions/common/AvatarInfo.java | 34 ++--- .../server/account/AccountDirectory.java | 7 +- .../gerrit/server/account/AccountInfo.java | 130 ------------------ .../gerrit/server/account/AccountLoader.java | 99 +++++++++++++ .../gerrit/server/account/CreateAccount.java | 7 +- .../gerrit/server/account/GetAccount.java | 7 +- .../account/InternalAccountDirectory.java | 14 +- .../server/api/accounts/AccountApiImpl.java | 11 +- .../server/api/changes/ChangeInfoMapper.java | 28 ++-- .../gerrit/server/change/ChangeJson.java | 29 ++-- .../google/gerrit/server/change/Check.java | 4 +- .../gerrit/server/change/CommentInfo.java | 5 +- .../gerrit/server/change/GetComment.java | 8 +- .../google/gerrit/server/change/GetDraft.java | 8 +- .../gerrit/server/change/ListComments.java | 4 +- .../gerrit/server/change/ListDrafts.java | 8 +- .../google/gerrit/server/change/Module.java | 4 +- .../gerrit/server/change/PostReviewers.java | 6 +- .../gerrit/server/change/ReviewerJson.java | 14 +- .../server/change/SuggestReviewers.java | 17 +-- .../gerrit/server/change/TestSubmitRule.java | 11 +- .../gerrit/server/group/AddMembers.java | 9 +- .../google/gerrit/server/group/GetMember.java | 9 +- .../google/gerrit/server/group/GroupJson.java | 2 +- .../gerrit/server/group/ListMembers.java | 7 +- .../gerrit/server/change/CommentsTest.java | 17 +-- .../sshd/commands/ListMembersCommand.java | 7 +- 34 files changed, 259 insertions(+), 272 deletions(-) rename gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountInfoMapper.java => gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AvatarInfo.java (51%) delete mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfo.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java index f1454c503a..931e9a7dcb 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -57,6 +57,12 @@ public class ChangeIT extends AbstractDaemonTest { assertThat(c.changeId).isEqualTo(r.getChangeId()); assertThat(c.created).isEqualTo(c.updated); assertThat(c._number).is(1); + + assertThat(c.owner.name).isEqualTo(admin.fullName); + assertThat(c.owner._accountId).isNull(); + assertThat(c.owner.email).isNull(); + assertThat(c.owner.username).isNull(); + assertThat(c.owner.avatars).isNull(); } @Test diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/AccountAssert.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/AccountAssert.java index 2c41f9aa14..1cc9e5b1c7 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/AccountAssert.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/AccountAssert.java @@ -18,7 +18,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import com.google.gerrit.acceptance.TestAccount; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.extensions.common.AccountInfo; public class AccountAssert { diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/GetAccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/GetAccountIT.java index 627fd58051..1e64860e22 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/GetAccountIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/GetAccountIT.java @@ -20,8 +20,8 @@ import static org.junit.Assert.assertEquals; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.TestAccount; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.Url; -import com.google.gerrit.server.account.AccountInfo; import org.apache.http.HttpStatus; import org.junit.Test; diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/AddRemoveGroupMembersIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/AddRemoveGroupMembersIT.java index 7c8a3a35f9..74a30f7a80 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/AddRemoveGroupMembersIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/AddRemoveGroupMembersIT.java @@ -27,11 +27,11 @@ import com.google.common.collect.Sets; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.TestAccount; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroupById; import com.google.gerrit.reviewdb.client.AccountGroupMember; -import com.google.gerrit.server.account.AccountInfo; import com.google.gerrit.server.group.AddIncludedGroups; import com.google.gerrit.server.group.AddMembers; import com.google.gerrit.server.group.CreateGroup; diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/ListGroupMembersIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/ListGroupMembersIT.java index 542574f66c..3c2ad4faa4 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/ListGroupMembersIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/ListGroupMembersIT.java @@ -22,7 +22,7 @@ import com.google.common.collect.Collections2; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.common.Nullable; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.server.group.CreateGroup; import com.google.gson.reflect.TypeToken; diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountInfo.java index 5130f9f708..39d98de149 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountInfo.java @@ -14,9 +14,16 @@ package com.google.gerrit.extensions.common; +import java.util.List; + public class AccountInfo { public Integer _accountId; public String name; public String email; public String username; + public List<AvatarInfo> avatars; + + public AccountInfo(Integer id) { + this._accountId = id; + } } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java index 2176e8f22b..ce120cd78b 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java @@ -19,4 +19,8 @@ import java.sql.Timestamp; public class ApprovalInfo extends AccountInfo { public Integer value; public Timestamp date; + + public ApprovalInfo(Integer id) { + super(id); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountInfoMapper.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AvatarInfo.java similarity index 51% rename from gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountInfoMapper.java rename to gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AvatarInfo.java index 10a9116ead..793aa2498c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountInfoMapper.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AvatarInfo.java @@ -12,26 +12,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.gerrit.server.api.accounts; +package com.google.gerrit.extensions.common; -import com.google.gerrit.extensions.common.AccountInfo; +public class AvatarInfo { + /** + * 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 class AccountInfoMapper { - public static AccountInfo fromAcountInfo( - com.google.gerrit.server.account.AccountInfo i) { - if (i == null) { - return null; - } - AccountInfo ai = new AccountInfo(); - fromAccount(i, ai); - return ai; - } - - public static void fromAccount( - com.google.gerrit.server.account.AccountInfo i, AccountInfo ai) { - ai._accountId = i._accountId; - ai.email = i.email; - ai.name = i.name; - ai.username = i.username; - } + public String url; + public Integer height; + public Integer width; } 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 index 4dc9d793ee..445ac6e83c 100644 --- 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 @@ -14,6 +14,8 @@ package com.google.gerrit.server.account; +import com.google.gerrit.extensions.common.AccountInfo; + import java.util.Set; /** @@ -34,7 +36,10 @@ public abstract class AccountDirectory { AVATARS, /** Unique user identity to login to Gerrit, may be deprecated. */ - USERNAME + USERNAME, + + /** Numeric account ID, may be deprecated. */ + ID; } public abstract void fillAccountInfo( 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 deleted file mode 100644 index 97abbf65f3..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfo.java +++ /dev/null @@ -1,130 +0,0 @@ -// 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.Throwables; -import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; -import com.google.gerrit.reviewdb.client.Account; -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.assistedinject.Assisted; - -import java.util.Collection; -import java.util.Collections; -import java.util.EnumSet; -import java.util.List; -import java.util.Map; -import java.util.Set; - -public class AccountInfo { - public static class Loader { - private static final Set<FillOptions> DETAILED_OPTIONS = - Collections.unmodifiableSet(EnumSet.of( - FillOptions.NAME, - FillOptions.EMAIL, - FillOptions.USERNAME, - FillOptions.AVATARS)); - - public interface Factory { - Loader create(boolean detailed); - } - - private final InternalAccountDirectory directory; - private final boolean detailed; - private final Map<Account.Id, AccountInfo> created; - private final List<AccountInfo> provided; - - @Inject - Loader(InternalAccountDirectory directory, @Assisted boolean detailed) { - this.directory = directory; - this.detailed = detailed; - created = Maps.newHashMap(); - provided = Lists.newArrayList(); - } - - public AccountInfo get(Account.Id id) { - if (id == null) { - return null; - } - AccountInfo info = created.get(id); - if (info == null) { - info = new AccountInfo(id); - if (detailed) { - info._accountId = id.get(); - } - created.put(id, info); - } - return info; - } - - public void put(AccountInfo info) { - if (detailed) { - info._accountId = info._id.get(); - } - provided.add(info); - } - - public void fill() throws OrmException { - try { - directory.fillAccountInfo( - Iterables.concat(created.values(), provided), - detailed ? DETAILED_OPTIONS : EnumSet.of(FillOptions.NAME)); - } catch (DirectoryException e) { - Throwables.propagateIfPossible(e.getCause(), OrmException.class); - throw new OrmException(e); - } - } - - public void fill(Collection<? extends AccountInfo> infos) - throws OrmException { - for (AccountInfo info : infos) { - put(info); - } - fill(); - } - } - - public transient Account.Id _id; - - public AccountInfo(Account.Id id) { - _id = id; - } - - public Integer _accountId; - public String name; - public String email; - public String username; - public List<AvatarInfo> avatars; - - public static class AvatarInfo { - /** - * 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; - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java new file mode 100644 index 0000000000..e8d4327a10 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java @@ -0,0 +1,99 @@ +// Copyright (C) 2014 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 static com.google.common.base.Preconditions.checkArgument; + +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.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.reviewdb.client.Account; +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.assistedinject.Assisted; + +import java.util.Collection; +import java.util.Collections; +import java.util.EnumSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class AccountLoader { + private static final Set<FillOptions> DETAILED_OPTIONS = + Collections.unmodifiableSet(EnumSet.of( + FillOptions.ID, + FillOptions.NAME, + FillOptions.EMAIL, + FillOptions.USERNAME, + FillOptions.AVATARS)); + + public interface Factory { + AccountLoader create(boolean detailed); + } + + private final InternalAccountDirectory directory; + private final boolean detailed; + private final Map<Account.Id, AccountInfo> created; + private final List<AccountInfo> provided; + + @Inject + AccountLoader(InternalAccountDirectory directory, @Assisted boolean detailed) { + this.directory = directory; + this.detailed = detailed; + created = Maps.newHashMap(); + provided = Lists.newArrayList(); + } + + public AccountInfo get(Account.Id id) { + if (id == null) { + return null; + } + AccountInfo info = created.get(id); + if (info == null) { + info = new AccountInfo(id.get()); + created.put(id, info); + } + return info; + } + + public void put(AccountInfo info) { + checkArgument(info._accountId != null, "_accountId field required"); + provided.add(info); + } + + public void fill() throws OrmException { + try { + directory.fillAccountInfo( + Iterables.concat(created.values(), provided), + detailed ? DETAILED_OPTIONS : EnumSet.of(FillOptions.NAME)); + } catch (DirectoryException e) { + Throwables.propagateIfPossible(e.getCause(), OrmException.class); + throw new OrmException(e); + } + } + + public void fill(Collection<? extends AccountInfo> infos) + throws OrmException { + for (AccountInfo info : infos) { + put(info); + } + fill(); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java index 53dec1b701..38fda4cceb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java @@ -21,6 +21,7 @@ import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.data.GroupDescriptions; import com.google.gerrit.common.errors.InvalidSshKeyException; import com.google.gerrit.extensions.annotations.RequiresCapability; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.ResourceConflictException; @@ -72,7 +73,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, Input> { private final SshKeyCache sshKeyCache; private final AccountCache accountCache; private final AccountByEmailCache byEmailCache; - private final AccountInfo.Loader.Factory infoLoader; + private final AccountLoader.Factory infoLoader; private final String username; private final AuditService auditService; @@ -80,7 +81,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, Input> { CreateAccount(ReviewDb db, Provider<IdentifiedUser> currentUser, GroupsCollection groupsCollection, SshKeyCache sshKeyCache, AccountCache accountCache, AccountByEmailCache byEmailCache, - AccountInfo.Loader.Factory infoLoader, + AccountLoader.Factory infoLoader, @Assisted String username, AuditService auditService) { this.db = db; this.currentUser = currentUser; @@ -180,7 +181,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, Input> { accountCache.evictByUsername(username); byEmailCache.evict(input.email); - AccountInfo.Loader loader = infoLoader.create(true); + AccountLoader loader = infoLoader.create(true); AccountInfo info = loader.get(id); loader.fill(); return Response.created(info); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetAccount.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetAccount.java index 200595f92d..05f830001c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetAccount.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetAccount.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.account; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -21,16 +22,16 @@ import com.google.inject.Singleton; @Singleton public class GetAccount implements RestReadView<AccountResource> { - private final AccountInfo.Loader.Factory infoFactory; + private final AccountLoader.Factory infoFactory; @Inject - GetAccount(AccountInfo.Loader.Factory infoFactory) { + GetAccount(AccountLoader.Factory infoFactory) { this.infoFactory = infoFactory; } @Override public AccountInfo apply(AccountResource rsrc) throws OrmException { - AccountInfo.Loader loader = infoFactory.create(true); + AccountLoader loader = infoFactory.create(true); AccountInfo info = loader.get(rsrc.getUser().getAccountId()); loader.fill(); return info; 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 index 1e3e4b11db..60763fdfdd 100644 --- 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 @@ -19,11 +19,12 @@ 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.common.AccountInfo; +import com.google.gerrit.extensions.common.AvatarInfo; 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; @@ -65,11 +66,12 @@ public class InternalAccountDirectory extends AccountDirectory { throws DirectoryException { Multimap<Account.Id, AccountInfo> missing = ArrayListMultimap.create(); for (AccountInfo info : in) { - AccountState state = accountCache.getIfPresent(info._id); + Account.Id id = new Account.Id(info._accountId); + AccountState state = accountCache.getIfPresent(id); if (state != null) { fill(info, state.getAccount(), options); } else { - missing.put(info._id, info); + missing.put(id, info); } } if (!missing.isEmpty()) { @@ -88,6 +90,12 @@ public class InternalAccountDirectory extends AccountDirectory { private void fill(AccountInfo info, Account account, Set<FillOptions> options) { + if (options.contains(FillOptions.ID)) { + info._accountId = account.getId().get(); + } else { + // Was previously set to look up account for filling. + info._accountId = null; + } if (options.contains(FillOptions.NAME)) { info.name = Strings.emptyToNull(account.getFullName()); if (info.name == null) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java index b1fd979b70..44413b748d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java @@ -15,10 +15,11 @@ package com.google.gerrit.server.api.accounts; import com.google.gerrit.extensions.api.accounts.AccountApi; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.TopLevelResource; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.AccountResource; import com.google.gerrit.server.account.StarredChanges; import com.google.gerrit.server.change.ChangeResource; @@ -34,12 +35,12 @@ public class AccountApiImpl extends AccountApi.NotImplemented implements Account private final AccountResource account; private final ChangesCollection changes; - private final AccountInfo.Loader.Factory accountLoaderFactory; + private final AccountLoader.Factory accountLoaderFactory; private final StarredChanges.Create starredChangesCreate; private final StarredChanges.Delete starredChangesDelete; @Inject - AccountApiImpl(AccountInfo.Loader.Factory ailf, + AccountApiImpl(AccountLoader.Factory ailf, ChangesCollection changes, StarredChanges.Create starredChangesCreate, StarredChanges.Delete starredChangesDelete, @@ -54,11 +55,11 @@ public class AccountApiImpl extends AccountApi.NotImplemented implements Account @Override public com.google.gerrit.extensions.common.AccountInfo get() throws RestApiException { - AccountInfo.Loader accountLoader = accountLoaderFactory.create(true); + AccountLoader accountLoader = accountLoaderFactory.create(true); try { AccountInfo ai = accountLoader.get(account.getUser().getAccountId()); accountLoader.fill(); - return AccountInfoMapper.fromAcountInfo(ai); + return ai; } catch (OrmException e) { throw new RestApiException("Cannot parse change", e); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java index 8e6c20bb79..dd01d847e8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java @@ -18,16 +18,15 @@ import com.google.common.base.Function; import com.google.common.collect.EnumBiMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import com.google.gerrit.extensions.common.ApprovalInfo; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeMessageInfo; import com.google.gerrit.extensions.common.ChangeStatus; import com.google.gerrit.extensions.common.LabelInfo; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change.Status; -import com.google.gerrit.server.api.accounts.AccountInfoMapper; import com.google.gerrit.server.change.ChangeJson; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -81,7 +80,7 @@ public class ChangeInfoMapper o.mergeable = i.mergeable; o.insertions = i.insertions; o.deletions = i.deletions; - o.owner = AccountInfoMapper.fromAcountInfo(i.owner); + o.owner = i.owner; o.currentRevision = i.currentRevision; o._number = i._number; } @@ -95,7 +94,7 @@ public class ChangeInfoMapper for (ChangeJson.ChangeMessageInfo m : i.messages) { ChangeMessageInfo cmi = new ChangeMessageInfo(); cmi.id = m.id; - cmi.author = AccountInfoMapper.fromAcountInfo(m.author); + cmi.author = m.author; cmi.date = m.date; cmi.message = m.message; cmi._revisionNumber = m._revisionNumber; @@ -112,31 +111,20 @@ public class ChangeInfoMapper for (Map.Entry<String, ChangeJson.LabelInfo> e : i.labels.entrySet()) { ChangeJson.LabelInfo li = e.getValue(); LabelInfo lo = new LabelInfo(); - lo.approved = AccountInfoMapper.fromAcountInfo(li.approved); - lo.rejected = AccountInfoMapper.fromAcountInfo(li.rejected); - lo.recommended = AccountInfoMapper.fromAcountInfo(li.recommended); - lo.disliked = AccountInfoMapper.fromAcountInfo(li.disliked); + lo.approved = li.approved; + lo.rejected = li.rejected; + lo.recommended = li.recommended; + lo.disliked = li.disliked; lo.value = li.value; lo.defaultValue = li.defaultValue; lo.optional = li.optional; lo.blocking = li.blocking; lo.values = li.values; if (li.all != null) { - lo.all = Lists.newArrayListWithExpectedSize(li.all.size()); - for (ChangeJson.ApprovalInfo ai : li.all) { - lo.all.add(fromApprovalInfo(ai)); - } + lo.all = new ArrayList<>(li.all); } r.put(e.getKey(), lo); } o.labels = r; } - - private static ApprovalInfo fromApprovalInfo(ChangeJson.ApprovalInfo ai) { - ApprovalInfo ao = new ApprovalInfo(); - ao.value = ai.value; - ao.date = ai.date; - AccountInfoMapper.fromAccount(ai, ao); - return ao; - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index 844ff6fef4..1d146192d1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -51,7 +51,9 @@ import com.google.gerrit.common.data.LabelValue; import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.common.data.SubmitRecord; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.common.ActionInfo; +import com.google.gerrit.extensions.common.ApprovalInfo; import com.google.gerrit.extensions.common.CommitInfo; import com.google.gerrit.extensions.common.FetchInfo; import com.google.gerrit.extensions.common.GitPerson; @@ -81,7 +83,7 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.PatchLineCommentsUtil; import com.google.gerrit.server.WebLinks; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.extensions.webui.UiActions; import com.google.gerrit.server.git.LabelNormalizer; import com.google.gerrit.server.notedb.ChangeNotes; @@ -122,7 +124,7 @@ public class ChangeJson { private final ChangeData.Factory changeDataFactory; private final PatchSetInfoFactory patchSetInfoFactory; private final FileInfoJson fileInfoJson; - private final AccountInfo.Loader.Factory accountLoaderFactory; + private final AccountLoader.Factory accountLoaderFactory; private final DynamicMap<DownloadScheme> downloadSchemes; private final DynamicMap<DownloadCommand> downloadCommands; private final DynamicMap<RestView<ChangeResource>> changeViews; @@ -132,7 +134,7 @@ public class ChangeJson { private final ChangeMessagesUtil cmUtil; private final PatchLineCommentsUtil plcUtil; - private AccountInfo.Loader accountLoader; + private AccountLoader accountLoader; @Inject ChangeJson( @@ -144,7 +146,7 @@ public class ChangeJson { ChangeData.Factory cdf, PatchSetInfoFactory psi, FileInfoJson fileInfoJson, - AccountInfo.Loader.Factory ailf, + AccountLoader.Factory ailf, DynamicMap<DownloadScheme> downloadSchemes, DynamicMap<DownloadCommand> downloadCommands, DynamicMap<RestView<ChangeResource>> changeViews, @@ -582,7 +584,7 @@ public class ChangeJson { } private ApprovalInfo approvalInfo(Account.Id id, Integer value, Timestamp date) { - ApprovalInfo ai = new ApprovalInfo(id); + ApprovalInfo ai = new ApprovalInfo(id.get()); ai.value = value; ai.date = date; accountLoader.put(ai); @@ -686,11 +688,11 @@ public class ChangeJson { continue; } for (ApprovalInfo ai : label.all) { - if (ctl.canRemoveReviewer(ai._id, - MoreObjects.firstNonNull(ai.value, 0))) { - removable.add(ai._id); + Account.Id id = new Account.Id(ai._accountId); + if (ctl.canRemoveReviewer(id, MoreObjects.firstNonNull(ai.value, 0))) { + removable.add(id); } else { - fixed.add(ai._id); + fixed.add(id); } } } @@ -987,15 +989,6 @@ public class ChangeJson { } } - public static class ApprovalInfo extends AccountInfo { - public Integer value; - public Timestamp date; - - ApprovalInfo(Account.Id id) { - super(id); - } - } - public static class ChangeMessageInfo { public String id; public AccountInfo author; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java index 8d613fba15..9877ba2415 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java @@ -14,9 +14,9 @@ package com.google.gerrit.server.change; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.server.account.AccountInfo; import com.google.gerrit.server.change.ChangeJson.ChangeInfo; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -66,7 +66,7 @@ public class Check implements RestReadView<ChangeResource> { info.changeId = c.getKey().get(); info.subject = c.getSubject(); info.status = c.getStatus(); - info.owner = new AccountInfo(c.getOwner()); + info.owner = new AccountInfo(c.getOwner().get()); info.created = c.getCreatedOn(); info.updated = c.getLastUpdatedOn(); info._number = c.getId().get(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentInfo.java index 4c5d8489f5..23da8385a9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentInfo.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentInfo.java @@ -16,10 +16,11 @@ package com.google.gerrit.server.change; import com.google.common.base.Strings; import com.google.gerrit.common.changes.Side; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.reviewdb.client.CommentRange; import com.google.gerrit.reviewdb.client.PatchLineComment; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import java.sql.Timestamp; @@ -34,7 +35,7 @@ public class CommentInfo { AccountInfo author; CommentRange range; - CommentInfo(PatchLineComment c, AccountInfo.Loader accountLoader) { + CommentInfo(PatchLineComment c, AccountLoader accountLoader) { id = Url.encode(c.getKey().get()); path = c.getKey().getParentKey().getFileName(); if (c.getSide() == 0) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetComment.java index 27de91caad..96390d651e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetComment.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetComment.java @@ -15,7 +15,7 @@ package com.google.gerrit.server.change; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -23,16 +23,16 @@ import com.google.inject.Singleton; @Singleton class GetComment implements RestReadView<CommentResource> { - private final AccountInfo.Loader.Factory accountLoaderFactory; + private final AccountLoader.Factory accountLoaderFactory; @Inject - GetComment(AccountInfo.Loader.Factory accountLoaderFactory) { + GetComment(AccountLoader.Factory accountLoaderFactory) { this.accountLoaderFactory = accountLoaderFactory; } @Override public CommentInfo apply(CommentResource rsrc) throws OrmException { - AccountInfo.Loader accountLoader = accountLoaderFactory.create(true); + AccountLoader accountLoader = accountLoaderFactory.create(true); CommentInfo ci = new CommentInfo(rsrc.getComment(), accountLoader); accountLoader.fill(); return ci; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDraft.java index 12c50ae603..8cff9f8b5e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDraft.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDraft.java @@ -15,7 +15,7 @@ package com.google.gerrit.server.change; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -23,16 +23,16 @@ import com.google.inject.Singleton; @Singleton class GetDraft implements RestReadView<DraftResource> { - private final AccountInfo.Loader.Factory accountLoaderFactory; + private final AccountLoader.Factory accountLoaderFactory; @Inject - GetDraft(AccountInfo.Loader.Factory accountLoaderFactory) { + GetDraft(AccountLoader.Factory accountLoaderFactory) { this.accountLoaderFactory = accountLoaderFactory; } @Override public CommentInfo apply(DraftResource rsrc) throws OrmException { - AccountInfo.Loader accountLoader = accountLoaderFactory.create(true); + AccountLoader accountLoader = accountLoaderFactory.create(true); CommentInfo ci = new CommentInfo(rsrc.getComment(), accountLoader); accountLoader.fill(); return ci; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java index 7896403917..6222d82067 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java @@ -17,7 +17,7 @@ package com.google.gerrit.server.change; import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.PatchLineCommentsUtil; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -27,7 +27,7 @@ import com.google.inject.Singleton; @Singleton class ListComments extends ListDrafts { @Inject - ListComments(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf, + ListComments(Provider<ReviewDb> db, AccountLoader.Factory alf, PatchLineCommentsUtil plcUtil) { super(db, alf, plcUtil); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java index b3c36fd42c..be047db645 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java @@ -23,7 +23,7 @@ import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.PatchLineCommentsUtil; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -38,10 +38,10 @@ import java.util.Map; class ListDrafts implements RestReadView<RevisionResource> { protected final Provider<ReviewDb> db; protected final PatchLineCommentsUtil plcUtil; - private final AccountInfo.Loader.Factory accountLoaderFactory; + private final AccountLoader.Factory accountLoaderFactory; @Inject - ListDrafts(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf, + ListDrafts(Provider<ReviewDb> db, AccountLoader.Factory alf, PatchLineCommentsUtil plcUtil) { this.db = db; this.accountLoaderFactory = alf; @@ -62,7 +62,7 @@ class ListDrafts implements RestReadView<RevisionResource> { public Map<String, List<CommentInfo>> apply(RevisionResource rsrc) throws OrmException { Map<String, List<CommentInfo>> out = Maps.newTreeMap(); - AccountInfo.Loader accountLoader = + AccountLoader accountLoader = includeAuthorInfo() ? accountLoaderFactory.create(true) : null; for (PatchLineComment c : listComments(rsrc)) { CommentInfo o = new CommentInfo(c, accountLoader); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java index 48b256d50c..ce38778523 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java @@ -24,7 +24,7 @@ import static com.google.gerrit.server.change.RevisionResource.REVISION_KIND; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.RestApiModule; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.change.Reviewed.DeleteReviewed; import com.google.gerrit.server.change.Reviewed.PutReviewed; import com.google.gerrit.server.config.FactoryModule; @@ -116,7 +116,7 @@ public class Module extends RestApiModule { @Override protected void configure() { factory(ReviewerResource.Factory.class); - factory(AccountInfo.Loader.Factory.class); + factory(AccountLoader.Factory.class); factory(EmailReviewComments.Factory.class); factory(ChangeInserter.Factory.class); factory(PatchSetInserter.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java index 5a223f743d..2c40ec3852 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java @@ -39,9 +39,9 @@ import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountCache; -import com.google.gerrit.server.account.AccountInfo; import com.google.gerrit.server.account.AccountsCollection; import com.google.gerrit.server.account.GroupMembers; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.change.ReviewerJson.PostResult; import com.google.gerrit.server.change.ReviewerJson.ReviewerInfo; import com.google.gerrit.server.config.GerritServerConfig; @@ -81,7 +81,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer private final AddReviewerSender.Factory addReviewerSenderFactory; private final GroupsCollection groupsCollection; private final GroupMembers.Factory groupMembersFactory; - private final AccountInfo.Loader.Factory accountLoaderFactory; + private final AccountLoader.Factory accountLoaderFactory; private final Provider<ReviewDb> dbProvider; private final ChangeUpdate.Factory updateFactory; private final Provider<CurrentUser> currentUser; @@ -99,7 +99,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer AddReviewerSender.Factory addReviewerSenderFactory, GroupsCollection groupsCollection, GroupMembers.Factory groupMembersFactory, - AccountInfo.Loader.Factory accountLoaderFactory, + AccountLoader.Factory accountLoaderFactory, Provider<ReviewDb> db, ChangeUpdate.Factory updateFactory, Provider<CurrentUser> currentUser, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java index 804af4e447..65b390ae80 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java @@ -23,12 +23,13 @@ import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.common.data.SubmitRecord; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.query.change.ChangeData; @@ -47,13 +48,13 @@ public class ReviewerJson { private final Provider<ReviewDb> db; private final ChangeData.Factory changeDataFactory; private final ApprovalsUtil approvalsUtil; - private final AccountInfo.Loader.Factory accountLoaderFactory; + private final AccountLoader.Factory accountLoaderFactory; @Inject ReviewerJson(Provider<ReviewDb> db, ChangeData.Factory changeDataFactory, ApprovalsUtil approvalsUtil, - AccountInfo.Loader.Factory accountLoaderFactory) { + AccountLoader.Factory accountLoaderFactory) { this.db = db; this.changeDataFactory = changeDataFactory; this.approvalsUtil = approvalsUtil; @@ -63,7 +64,7 @@ public class ReviewerJson { public List<ReviewerInfo> format(Collection<ReviewerResource> rsrcs) throws OrmException { List<ReviewerInfo> infos = Lists.newArrayListWithCapacity(rsrcs.size()); - AccountInfo.Loader loader = accountLoaderFactory.create(true); + AccountLoader loader = accountLoaderFactory.create(true); for (ReviewerResource rsrc : rsrcs) { ReviewerInfo info = format(new ReviewerInfo( rsrc.getUser().getAccountId()), @@ -82,7 +83,8 @@ public class ReviewerJson { public ReviewerInfo format(ReviewerInfo out, ChangeControl ctl) throws OrmException { PatchSet.Id psId = ctl.getChange().currentPatchSetId(); return format(out, ctl, - approvalsUtil.byPatchSetUser(db.get(), ctl, psId, out._id)); + approvalsUtil.byPatchSetUser(db.get(), ctl, psId, + new Account.Id(out._accountId))); } public ReviewerInfo format(ReviewerInfo out, ChangeControl ctl, @@ -136,7 +138,7 @@ public class ReviewerJson { Map<String, String> approvals; protected ReviewerInfo(Account.Id id) { - super(id); + super(id.get()); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java index ff54e09fb8..a6b3945b47 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java @@ -21,6 +21,7 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.common.errors.NoSuchGroupException; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.Url; @@ -32,7 +33,7 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountControl; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.AccountVisibility; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupMembers; @@ -60,7 +61,7 @@ public class SuggestReviewers implements RestReadView<ChangeResource> { private static final int DEFAULT_MAX_SUGGESTED = 10; private static final int DEFAULT_MAX_MATCHES = 100; - private final AccountInfo.Loader.Factory accountLoaderFactory; + private final AccountLoader.Factory accountLoaderFactory; private final AccountControl.Factory accountControlFactory; private final GroupMembers.Factory groupMembersFactory; private final AccountCache accountCache; @@ -94,7 +95,7 @@ public class SuggestReviewers implements RestReadView<ChangeResource> { @Inject SuggestReviewers(AccountVisibility av, - AccountInfo.Loader.Factory accountLoaderFactory, + AccountLoader.Factory accountLoaderFactory, AccountControl.Factory accountControlFactory, AccountCache accountCache, GroupMembers.Factory groupMembersFactory, @@ -216,13 +217,13 @@ public class SuggestReviewers implements RestReadView<ChangeResource> { LinkedHashMap<Account.Id, AccountInfo> r = Maps.newLinkedHashMap(); for (Account p : dbProvider.get().accounts() .suggestByFullName(a, b, limit)) { - addSuggestion(r, p, new AccountInfo(p.getId()), visibilityControl); + addSuggestion(r, p, new AccountInfo(p.getId().get()), visibilityControl); } if (r.size() < limit) { for (Account p : dbProvider.get().accounts() .suggestByPreferredEmail(a, b, limit - r.size())) { - addSuggestion(r, p, new AccountInfo(p.getId()), visibilityControl); + addSuggestion(r, p, new AccountInfo(p.getId().get()), visibilityControl); } } @@ -231,7 +232,7 @@ public class SuggestReviewers implements RestReadView<ChangeResource> { .suggestByEmailAddress(a, b, limit - r.size())) { if (!r.containsKey(e.getAccountId())) { Account p = accountCache.get(e.getAccountId()).getAccount(); - AccountInfo info = new AccountInfo(p.getId()); + AccountInfo info = new AccountInfo(p.getId().get()); addSuggestion(r, p, info, visibilityControl); } } @@ -260,14 +261,14 @@ public class SuggestReviewers implements RestReadView<ChangeResource> { } } for (Account a : fullNameMatches) { - addSuggestion(accountMap, a, new AccountInfo(a.getId()), visibilityControl); + addSuggestion(accountMap, a, new AccountInfo(a.getId().get()), visibilityControl); if (accountMap.size() >= limit) { break; } } if (accountMap.size() < limit) { for (Account a : emailMatches) { - addSuggestion(accountMap, a, new AccountInfo(a.getId()), visibilityControl); + addSuggestion(accountMap, a, new AccountInfo(a.getId().get()), visibilityControl); if (accountMap.size() >= limit) { break; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java index fd7ecad2d1..8d798075ec 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java @@ -18,13 +18,14 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.gerrit.common.data.SubmitRecord; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.rules.RulesCache; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.change.TestSubmitRule.Input; import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.query.change.ChangeData; @@ -51,7 +52,7 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, Input> { private final Provider<ReviewDb> db; private final ChangeData.Factory changeDataFactory; private final RulesCache rules; - private final AccountInfo.Loader.Factory accountInfoFactory; + private final AccountLoader.Factory accountInfoFactory; @Option(name = "--filters", usage = "impact of filters in parent projects") private Filters filters = Filters.RUN; @@ -60,7 +61,7 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, Input> { TestSubmitRule(Provider<ReviewDb> db, ChangeData.Factory changeDataFactory, RulesCache rules, - AccountInfo.Loader.Factory infoFactory) { + AccountLoader.Factory infoFactory) { this.db = db; this.changeDataFactory = changeDataFactory; this.rules = rules; @@ -86,7 +87,7 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, Input> { .setRule(input.rule) .canSubmit(); List<Record> out = Lists.newArrayListWithCapacity(records.size()); - AccountInfo.Loader accounts = accountInfoFactory.create(true); + AccountLoader accounts = accountInfoFactory.create(true); for (SubmitRecord r : records) { out.add(new Record(r, accounts)); } @@ -103,7 +104,7 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, Input> { Map<String, AccountInfo> may; Map<String, None> impossible; - Record(SubmitRecord r, AccountInfo.Loader accounts) { + Record(SubmitRecord r, AccountLoader accounts) { this.status = r.status; this.errorMessage = r.errorMessage; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java index 6002088124..5c7fcbca23 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java @@ -18,6 +18,7 @@ import com.google.common.base.Strings; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.gerrit.audit.AuditService; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; @@ -31,7 +32,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountException; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.AccountsCollection; @@ -79,7 +80,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> { private final AccountsCollection accounts; private final AccountResolver accountResolver; private final AccountCache accountCache; - private final AccountInfo.Loader.Factory infoFactory; + private final AccountLoader.Factory infoFactory; private final Provider<ReviewDb> db; private final AuditService auditService; @@ -89,7 +90,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> { AccountsCollection accounts, AccountResolver accountResolver, AccountCache accountCache, - AccountInfo.Loader.Factory infoFactory, + AccountLoader.Factory infoFactory, Provider<ReviewDb> db, AuditService auditService) { this.accountManager = accountManager; @@ -116,7 +117,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> { Map<Account.Id, AccountGroupMember> newAccountGroupMembers = Maps.newHashMap(); List<AccountInfo> result = Lists.newLinkedList(); Account.Id me = ((IdentifiedUser) control.getCurrentUser()).getAccountId(); - AccountInfo.Loader loader = infoFactory.create(true); + AccountLoader loader = infoFactory.create(true); for (String nameOrEmail : input.members) { Account a = findAccount(nameOrEmail); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetMember.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetMember.java index 2782932e06..9d270ec689 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetMember.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetMember.java @@ -14,24 +14,25 @@ package com.google.gerrit.server.group; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Singleton; @Singleton public class GetMember implements RestReadView<MemberResource> { - private final AccountInfo.Loader.Factory infoFactory; + private final AccountLoader.Factory infoFactory; @Inject - GetMember(AccountInfo.Loader.Factory infoFactory) { + GetMember(AccountLoader.Factory infoFactory) { this.infoFactory = infoFactory; } @Override public AccountInfo apply(MemberResource rsrc) throws OrmException { - AccountInfo.Loader loader = infoFactory.create(true); + AccountLoader loader = infoFactory.create(true); AccountInfo info = loader.get(rsrc.getMember().getAccountId()); loader.fill(); return info; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupJson.java index 9f851f4648..96b4234138 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupJson.java @@ -21,10 +21,10 @@ import com.google.common.base.Strings; import com.google.gerrit.common.data.GroupDescription; import com.google.gerrit.common.data.GroupDescriptions; import com.google.gerrit.common.groups.ListGroupsOption; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.reviewdb.client.AccountGroup; -import com.google.gerrit.server.account.AccountInfo; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupControl; import com.google.gwtorm.server.OrmException; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java index ad16a7e136..aab9efeafc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java @@ -20,13 +20,14 @@ import com.google.common.collect.Maps; import com.google.common.collect.Ordering; import com.google.gerrit.common.data.GroupDetail; import com.google.gerrit.common.errors.NoSuchGroupException; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroupById; import com.google.gerrit.reviewdb.client.AccountGroupMember; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.GroupDetailFactory; import com.google.gwtorm.server.OrmException; @@ -43,7 +44,7 @@ import java.util.Map; public class ListMembers implements RestReadView<GroupResource> { private final GroupCache groupCache; private final GroupDetailFactory.Factory groupDetailFactory; - private final AccountInfo.Loader accountLoader; + private final AccountLoader accountLoader; @Option(name = "--recursive", usage = "to resolve included groups recursively") private boolean recursive; @@ -51,7 +52,7 @@ public class ListMembers implements RestReadView<GroupResource> { @Inject protected ListMembers(GroupCache groupCache, GroupDetailFactory.Factory groupDetailFactory, - AccountInfo.Loader.Factory accountLoaderFactory) { + AccountLoader.Factory accountLoaderFactory) { this.groupCache = groupCache; this.groupDetailFactory = groupDetailFactory; this.accountLoader = accountLoaderFactory.create(true); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java index 4f64a67db1..1897db9d94 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java @@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.changes.Side; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; @@ -49,7 +50,7 @@ import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.PatchLineCommentsUtil; import com.google.gerrit.server.account.AccountCache; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.config.AllUsersNameProvider; @@ -143,8 +144,8 @@ public class CommentsTest { final TypeLiteral<DynamicMap<RestView<DraftResource>>> draftViewsType = new TypeLiteral<DynamicMap<RestView<DraftResource>>>() {}; - final AccountInfo.Loader.Factory alf = - createMock(AccountInfo.Loader.Factory.class); + final AccountLoader.Factory alf = + createMock(AccountLoader.Factory.class); db = createMock(ReviewDb.class); final FakeAccountCache accountCache = new FakeAccountCache(); final PersonIdent serverIdent = new PersonIdent( @@ -168,7 +169,7 @@ public class CommentsTest { protected void configure() { bind(commentViewsType).toInstance(commentViews); bind(draftViewsType).toInstance(draftViews); - bind(AccountInfo.Loader.Factory.class).toInstance(alf); + bind(AccountLoader.Factory.class).toInstance(alf); bind(ReviewDb.class).toInstance(db); bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(config); bind(ProjectCache.class).toProvider(Providers.<ProjectCache> of(null)); @@ -206,13 +207,13 @@ public class CommentsTest { changeOwner = userFactory.create(ownerId); IdentifiedUser otherUser = userFactory.create(otherUserId); - AccountInfo.Loader accountLoader = createMock(AccountInfo.Loader.class); + AccountLoader accountLoader = createMock(AccountLoader.class); accountLoader.fill(); expectLastCall().anyTimes(); expect(accountLoader.get(ownerId)) - .andReturn(new AccountInfo(ownerId)).anyTimes(); + .andReturn(new AccountInfo(ownerId.get())).anyTimes(); expect(accountLoader.get(otherUserId)) - .andReturn(new AccountInfo(otherUserId)).anyTimes(); + .andReturn(new AccountInfo(otherUserId.get())).anyTimes(); expect(alf.create(true)).andReturn(accountLoader).anyTimes(); replay(accountLoader, alf); @@ -432,7 +433,7 @@ public class CommentsTest { assertEquals(plc.getMessage(), ci.message); if (isPublished) { assertNotNull(ci.author); - assertEquals(plc.getAuthor(), ci.author._id); + assertEquals(plc.getAuthor(), new Account.Id(ci.author._accountId)); } assertEquals(plc.getLine(), (int) ci.line); assertEquals(plc.getSide() == 0 ? Side.PARENT : Side.REVISION, diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListMembersCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListMembersCommand.java index db497f42c7..b24c4bfccf 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListMembersCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListMembersCommand.java @@ -18,8 +18,9 @@ import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER_OR_SLAVE; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; +import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.reviewdb.client.AccountGroup; -import com.google.gerrit.server.account.AccountInfo; +import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.GroupDetailFactory.Factory; import com.google.gerrit.server.group.ListMembers; @@ -62,7 +63,7 @@ public class ListMembersCommand extends SshCommand { @Inject protected ListMembersCommandImpl(GroupCache groupCache, Factory groupDetailFactory, - AccountInfo.Loader.Factory accountLoaderFactory) { + AccountLoader.Factory accountLoaderFactory) { super(groupCache, groupDetailFactory, accountLoaderFactory); this.groupCache = groupCache; } @@ -89,7 +90,7 @@ public class ListMembersCommand extends SshCommand { continue; } - formatter.addColumn(member._id.toString()); + formatter.addColumn(Integer.toString(member._accountId)); formatter.addColumn(MoreObjects.firstNonNull( member.username, "n/a")); formatter.addColumn(MoreObjects.firstNonNull(