From e7ed19a62b9fc58a956e5a9e4e08fb762b1d6860 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 11 Nov 2015 12:22:40 -0800 Subject: [PATCH] Factor out an AccountInfoComparator class to avoid code duplication Add a new class that implements a comparator for AccountInfo that can be used in multiple places. The ordering for SuggestAccounts is slighty improved since it now also sorts by account ID when name and email are equal. Change-Id: If6fd06585e804ec1339f70d4928b87d9ac8cfa98 Signed-off-by: Edwin Kempin --- .../server/account/SuggestAccounts.java | 14 +---- .../api/accounts/AccountInfoComparator.java | 56 +++++++++++++++++++ .../gerrit/server/change/ChangeJson.java | 15 +---- .../gerrit/server/group/ListMembers.java | 14 +---- 4 files changed, 62 insertions(+), 37 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountInfoComparator.java diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SuggestAccounts.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SuggestAccounts.java index 837e1ede6c..1cc3c94379 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SuggestAccounts.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SuggestAccounts.java @@ -15,8 +15,6 @@ package com.google.gerrit.server.account; import com.google.common.base.Strings; -import com.google.common.collect.ComparisonChain; -import com.google.common.collect.Ordering; import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.RestReadView; @@ -24,6 +22,7 @@ import com.google.gerrit.extensions.restapi.TopLevelResource; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountExternalId; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.api.accounts.AccountInfoComparator; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -33,7 +32,6 @@ import org.kohsuke.args4j.Option; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -140,15 +138,7 @@ public class SuggestAccounts implements RestReadView { } List m = new ArrayList<>(matches.values()); - Collections.sort(m, new Comparator() { - @Override - public int compare(AccountInfo a, AccountInfo b) { - return ComparisonChain.start() - .compare(a.name, b.name, Ordering.natural().nullsLast()) - .compare(a.email, b.email, Ordering.natural().nullsLast()) - .result(); - } - }); + Collections.sort(m, AccountInfoComparator.ORDER_NULLS_LAST); return m; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountInfoComparator.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountInfoComparator.java new file mode 100644 index 0000000000..37947012e9 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountInfoComparator.java @@ -0,0 +1,56 @@ +// Copyright (C) 2015 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.api.accounts; + +import com.google.common.collect.ComparisonChain; +import com.google.common.collect.Ordering; +import com.google.gerrit.extensions.common.AccountInfo; + +import java.util.Comparator; + +public class AccountInfoComparator extends Ordering + implements Comparator { + public static final AccountInfoComparator ORDER_NULLS_FIRST = + new AccountInfoComparator(); + public static final AccountInfoComparator ORDER_NULLS_LAST = + new AccountInfoComparator().setNullsLast(); + + private boolean nullsLast; + + private AccountInfoComparator() { + } + + private AccountInfoComparator setNullsLast() { + this.nullsLast = true; + return this; + } + + @Override + public int compare(AccountInfo a, AccountInfo b) { + return ComparisonChain.start() + .compare(a.name, b.name, createOrdering()) + .compare(a.email, b.email, createOrdering()) + .compare(a._accountId, b._accountId, createOrdering()) + .result(); + } + + private > Ordering createOrdering() { + if (nullsLast) { + return Ordering.natural().nullsLast(); + } else { + return Ordering.natural().nullsFirst(); + } + } +} 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 a02f0db780..a17ef284c7 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 @@ -40,7 +40,6 @@ import com.google.common.base.Joiner; import com.google.common.base.MoreObjects; import com.google.common.base.Optional; import com.google.common.base.Throwables; -import com.google.common.collect.ComparisonChain; import com.google.common.collect.FluentIterable; import com.google.common.collect.HashBasedTable; import com.google.common.collect.HashMultimap; @@ -49,7 +48,6 @@ import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; -import com.google.common.collect.Ordering; import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; import com.google.common.collect.Table; @@ -92,6 +90,7 @@ import com.google.gerrit.server.GpgException; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.WebLinks; import com.google.gerrit.server.account.AccountLoader; +import com.google.gerrit.server.api.accounts.AccountInfoComparator; import com.google.gerrit.server.api.accounts.GpgApiAdapter; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.LabelNormalizer; @@ -121,7 +120,6 @@ import java.sql.Timestamp; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.EnumSet; import java.util.HashMap; import java.util.List; @@ -849,16 +847,7 @@ public class ChangeJson { return accountLoader.get(id); } }) - .toSortedList(new Comparator() { - @Override - public int compare(AccountInfo a, AccountInfo b) { - return ComparisonChain.start() - .compare(a.name, b.name, Ordering.natural().nullsFirst()) - .compare(a.email, b.email, Ordering.natural().nullsFirst()) - .compare(a._accountId, b._accountId, - Ordering.natural().nullsFirst()).result(); - } - }); + .toSortedList(AccountInfoComparator.ORDER_NULLS_FIRST); } private Map revisions(ChangeControl ctl, 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 aab9efeafc..fdededdb04 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 @@ -14,10 +14,8 @@ package com.google.gerrit.server.group; -import com.google.common.collect.ComparisonChain; import com.google.common.collect.Lists; 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; @@ -30,13 +28,13 @@ import com.google.gerrit.reviewdb.client.AccountGroupMember; import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.GroupDetailFactory; +import com.google.gerrit.server.api.accounts.AccountInfoComparator; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import org.kohsuke.args4j.Option; import java.util.Collections; -import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -82,15 +80,7 @@ public class ListMembers implements RestReadView { final Map members = getMembers(groupId, new HashSet()); final List memberInfos = Lists.newArrayList(members.values()); - Collections.sort(memberInfos, new Comparator() { - @Override - public int compare(AccountInfo a, AccountInfo b) { - return ComparisonChain.start() - .compare(a.name, b.name, Ordering.natural().nullsFirst()) - .compare(a.email, b.email, Ordering.natural().nullsFirst()) - .compare(a._accountId, b._accountId, Ordering.natural().nullsFirst()).result(); - } - }); + Collections.sort(memberInfos, AccountInfoComparator.ORDER_NULLS_FIRST); return memberInfos; }