opendev/gerrit
Code Issues Proposed changes

Drop MemberInfo and use AccountInfo instead

Browse Source 
Both entities represent accounts and have similar fields. MemberInfo has
more fields but they aren't used so we can just drop them.

Move AccountInfo (on client and server side) from the 'change' package
into the 'account' package since it is now also used outside of the
'/changes/' scope.

Change-Id: I8e8732981ed09851ea0db848ad63a45b983954fe
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2013-02-27 12:39:32 +01:00
parent b5b5c79b43
commit 963dfd07d0
25 changed files with 151 additions and 239 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
gerrit-server/src/main/java/com/google/gerrit/server/change/AccountInfo.java → gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfo.java
View File

@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.change;
package com.google.gerrit.server.account;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Iterables;
@@ -21,8 +21,6 @@ import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -33,8 +31,8 @@ import java.util.List;
import java.util.Map;
public class AccountInfo {
static class Loader {
interface Factory {
public static class Loader {
public interface Factory {
Loader create(boolean detailed);
}
@@ -76,7 +74,7 @@ public class AccountInfo {
for (AccountInfo info : Iterables.concat(created.values(), provided)) {
AccountState state = accountCache.getIfPresent(info._id);
if (state != null) {
fill(info, state.getAccount());
info.fill(state.getAccount(), detailed);
} else {
missing.put(info._id, info);
}
@@ -84,7 +82,7 @@ public class AccountInfo {
if (!missing.isEmpty()) {
for (Account account : db.get().accounts().get(missing.keySet())) {
for (AccountInfo info : missing.get(account.getId())) {
fill(info, account);
info.fill(account, detailed);
}
}
}
@@ -97,23 +95,29 @@ public class AccountInfo {
}
fill();
}
private void fill(AccountInfo info, Account account) {
info.name = account.getFullName();
if (detailed) {
info._account_id = account.getId().get();
info.email = account.getPreferredEmail();
}
}
}
transient Account.Id _id;
public static AccountInfo parse(Account a, boolean detailed) {
AccountInfo ai = new AccountInfo(a.getId());
ai.fill(a, detailed);
return ai;
}
protected AccountInfo(Account.Id id) {
public transient Account.Id _id;
public AccountInfo(Account.Id id) {
_id = id;
}
public Integer _account_id;
public String name;
public String email;
private void fill(Account account, boolean detailed) {
name = account.getFullName();
if (detailed) {
_account_id = account.getId().get();
email = account.getPreferredEmail();
}
}
}

1
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@@ -56,6 +56,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountInfo;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.patch.PatchList;

1
gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
View File

@@ -22,6 +22,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.change.Reviewed.DeleteReviewed;
import com.google.gerrit.server.change.Reviewed.PutReviewed;
import com.google.gerrit.server.config.FactoryModule;

1
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
View File

@@ -39,6 +39,7 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
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.AccountInfo;
import com.google.gerrit.server.account.GroupMembers;
import com.google.gerrit.server.change.PostReviewers.Input;
import com.google.gerrit.server.change.ReviewerJson.PostResult;

1
gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
View File

@@ -27,6 +27,7 @@ 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.account.AccountInfo;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.workflow.CategoryFunction;

1
gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java
View File

@@ -29,6 +29,7 @@ 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.change.TestSubmitRule.Input;
import com.google.gerrit.server.project.RuleEvalException;
import com.google.gerrit.server.project.SubmitRuleEvaluator;

10
gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java
View File

@@ -35,6 +35,7 @@ import com.google.gerrit.server.BadRequestHandler;
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.AccountManager;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.AccountsCollection;
@@ -42,7 +43,6 @@ import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.group.AddMembers.Input;
import com.google.gerrit.server.group.MembersCollection.MemberInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -94,7 +94,7 @@ class AddMembers implements RestModifyView<GroupResource, Input> {
}
@Override
public List<MemberInfo> apply(GroupResource resource, Input input)
public List<AccountInfo> apply(GroupResource resource, Input input)
throws AuthException, MethodNotAllowedException, BadRequestException,
OrmException {
AccountGroup internalGroup = resource.toAccountGroup();
@@ -107,7 +107,7 @@ class AddMembers implements RestModifyView<GroupResource, Input> {
Map<Account.Id, AccountGroupMember> newAccountGroupMembers = Maps.newHashMap();
List<AccountGroupMemberAudit> newAccountGroupMemberAudits = Lists.newLinkedList();
BadRequestHandler badRequest = new BadRequestHandler("adding new group members");
List<MemberInfo> result = Lists.newLinkedList();
List<AccountInfo> result = Lists.newLinkedList();
Account.Id me = ((IdentifiedUser) control.getCurrentUser()).getAccountId();
for (String nameOrEmail : input.members) {
@@ -136,7 +136,7 @@ class AddMembers implements RestModifyView<GroupResource, Input> {
newAccountGroupMemberAudits.add(new AccountGroupMemberAudit(m, me));
}
}
result.add(MembersCollection.parse(a));
result.add(AccountInfo.parse(a, true));
}
badRequest.failOnError();
@@ -205,7 +205,7 @@ class AddMembers implements RestModifyView<GroupResource, Input> {
OrmException {
AddMembers.Input in = new AddMembers.Input();
in._oneMember = id;
List<MemberInfo> list = put.get().apply(resource, in);
List<AccountInfo> list = put.get().apply(resource, in);
if (list.size() == 1) {
return list.get(0);
}

6
gerrit-server/src/main/java/com/google/gerrit/server/group/GetMember.java
View File

@@ -15,11 +15,11 @@
package com.google.gerrit.server.group;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.group.MembersCollection.MemberInfo;
import com.google.gerrit.server.account.AccountInfo;
public class GetMember implements RestReadView<MemberResource> {
@Override
public MemberInfo apply(MemberResource resource) {
return MembersCollection.parse(resource.getMember().getAccount());
public AccountInfo apply(MemberResource resource) {
return AccountInfo.parse(resource.getMember().getAccount(), true);
}
}

38
gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java
View File

@@ -29,10 +29,9 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountInfo;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupDetailFactory;
import com.google.gerrit.server.group.MembersCollection.MemberInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -47,49 +46,49 @@ import java.util.Map;
public class ListMembers implements RestReadView<GroupResource> {
private final GroupCache groupCache;
private final GroupDetailFactory.Factory groupDetailFactory;
private final AccountCache accountCache;
private final AccountInfo.Loader accountLoader;
@Option(name = "--recursive", usage = "to resolve included groups recursively")
private boolean recursive;
@Inject
ListMembers(final GroupCache groupCache,
final GroupDetailFactory.Factory groupDetailFactory,
final AccountCache accountCache) {
ListMembers(GroupCache groupCache,
GroupDetailFactory.Factory groupDetailFactory,
AccountInfo.Loader.Factory accountLoaderFactory) {
this.groupCache = groupCache;
this.groupDetailFactory = groupDetailFactory;
this.accountCache = accountCache;
this.accountLoader = accountLoaderFactory.create(true);
}
@Override
public List<MemberInfo> apply(final GroupResource resource)
public List<AccountInfo> apply(final GroupResource resource)
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, Exception {
if (resource.toAccountGroup() == null) {
throw new ResourceNotFoundException(resource.getGroupUUID().get());
}
final Map<Account.Id, MemberInfo> members =
final Map<Account.Id, AccountInfo> members =
getMembers(resource.getGroupUUID(), new HashSet<AccountGroup.UUID>());
final List<MemberInfo> memberInfos = Lists.newArrayList(members.values());
Collections.sort(memberInfos, new Comparator<MemberInfo>() {
final List<AccountInfo> memberInfos = Lists.newArrayList(members.values());
Collections.sort(memberInfos, new Comparator<AccountInfo>() {
@Override
public int compare(MemberInfo a, MemberInfo b) {
public int compare(AccountInfo a, AccountInfo b) {
return ComparisonChain.start()
.compare(a.fullName, b.fullName, Ordering.natural().nullsFirst())
.compare(a.preferredEmail, b.preferredEmail, Ordering.natural().nullsFirst())
.compare(a.id, b.id, Ordering.natural().nullsFirst()).result();
.compare(a.name, b.name, Ordering.natural().nullsFirst())
.compare(a.email, b.email, Ordering.natural().nullsFirst())
.compare(a._account_id, b._account_id, Ordering.natural().nullsFirst()).result();
}
});
return memberInfos;
}
private Map<Account.Id, MemberInfo> getMembers(
private Map<Account.Id, AccountInfo> getMembers(
final AccountGroup.UUID groupUUID,
final HashSet<AccountGroup.UUID> seenGroups) throws OrmException,
NoSuchGroupException {
seenGroups.add(groupUUID);
final Map<Account.Id, MemberInfo> members = Maps.newHashMap();
final Map<Account.Id, AccountInfo> members = Maps.newHashMap();
final AccountGroup group = groupCache.get(groupUUID);
if (group == null) {
// the included group is an external group and can't be resolved
@@ -102,9 +101,7 @@ public class ListMembers implements RestReadView<GroupResource> {
if (groupDetail.members != null) {
for (final AccountGroupMember m : groupDetail.members) {
if (!members.containsKey(m.getAccountId())) {
final Account account =
accountCache.get(m.getAccountId()).getAccount();
members.put(account.getId(), MembersCollection.parse(account));
members.put(m.getAccountId(), accountLoader.get(m.getAccountId()));
}
}
}
@@ -118,6 +115,7 @@ public class ListMembers implements RestReadView<GroupResource> {
}
}
}
accountLoader.fill();
return members;
}
}

25
gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java
View File

@@ -21,7 +21,6 @@ import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -86,28 +85,4 @@ public class MembersCollection implements
public DynamicMap<RestView<MemberResource>> views() {
return views;
}
public static MemberInfo parse(Account account) {
MemberInfo accountInfo = new MemberInfo();
accountInfo.setId(account.getId());
accountInfo.fullName = account.getFullName();
accountInfo.preferredEmail = account.getPreferredEmail();
accountInfo.userName = account.getUserName();
return accountInfo;
}
static class MemberInfo {
final String kind = "gerritcodereview#member";
String fullName;
String id;
int accountId;
String preferredEmail;
String userName;
void setId(Account.Id i) {
accountId = i.get();
id = Url.encode(Integer.toString(accountId));
}
}
}