opendev/gerrit
Code Issues Proposed changes

Convert some value classes to AutoValue

Browse Source 
These classes were found by searching for custom hashCode
implementations, and omitting some cases:

 - Classes requiring custom serialization, which is not supported[1][2].
 - Most instances with custom hashCode or equals implementations,
   where the code savings is not as significant.
 - All classes in the extension API. We may migrate these eventually,
   but let's avoid this large backwards incompatible change until
   we're more used to AutoValue elsewhere.
 - All classes in the UI package.[3]

There are likely still more value classes that were missed by this
search that do not implement equals or hashCode.

[1] https://github.com/google/auto/tree/master/value#serialization
[2] This excludes, among other things, all persistent cache keys. It
might be possible to convert persistent caches to use a key
marshalling strategy other than Java serialization, but probably not
without invalidating all existing entries.
[3] This should still be possible as generated classes are generally
GWT compatible.

Change-Id: I96796b9879b7e487b80949b63115ac4032180f8b
This commit is contained in:
Dave Borowitz
2014-11-09 17:57:45 -08:00
parent c87d29ec16
commit eba26614e5
24 changed files with 159 additions and 332 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
gerrit-httpd/BUCK
View File

@@ -26,6 +26,7 @@ java_library(
'//lib:gwtorm', '//lib:gwtorm',
'//lib:jsch', '//lib:jsch',
'//lib:mime-util', '//lib:mime-util',
'//lib/auto:auto-value',
'//lib/commons:codec', '//lib/commons:codec',
'//lib/guice:guice', '//lib/guice:guice',
'//lib/guice:guice-assistedinject', '//lib/guice:guice-assistedinject',

27
gerrit-httpd/src/main/java/com/google/gerrit/httpd/AdvertisedObjectsCacheKey.java
View File

@@ -14,29 +14,16 @@
package com.google.gerrit.httpd; package com.google.gerrit.httpd;
import com.google.auto.value.AutoValue;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
class AdvertisedObjectsCacheKey { @AutoValue
private final Account.Id account; abstract class AdvertisedObjectsCacheKey {
private final Project.NameKey project; static AdvertisedObjectsCacheKey create(Account.Id account, Project.NameKey project) {
return new AutoValue_AdvertisedObjectsCacheKey(account, project);
AdvertisedObjectsCacheKey(Account.Id account, Project.NameKey project) {
this.account = account;
this.project = project;
} }
@Override public abstract Account.Id account();
public int hashCode() { public abstract Project.NameKey project();
return account.hashCode();
}
@Override
public boolean equals(Object other) {
if (other instanceof AdvertisedObjectsCacheKey) {
AdvertisedObjectsCacheKey o = (AdvertisedObjectsCacheKey) other;
return account.equals(o.account) && project.equals(o.project);
}
return false;
}
} }

2
gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java
View File

@@ -372,7 +372,7 @@ public class GitOverHttpServlet extends GitServlet {
return; return;
} }
AdvertisedObjectsCacheKey cacheKey = new AdvertisedObjectsCacheKey( AdvertisedObjectsCacheKey cacheKey = AdvertisedObjectsCacheKey.create(
((IdentifiedUser) pc.getCurrentUser()).getAccountId(), ((IdentifiedUser) pc.getCurrentUser()).getAccountId(),
projectName); projectName);

2
gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java
View File

@@ -260,7 +260,7 @@ class HttpPluginServlet extends HttpServlet
} }
String file = pathInfo.substring(1); String file = pathInfo.substring(1);
PluginResourceKey key = new PluginResourceKey(holder.plugin, file); PluginResourceKey key = PluginResourceKey.create(holder.plugin, file);
Resource rsc = resourceCache.getIfPresent(key); Resource rsc = resourceCache.getIfPresent(key);
if (rsc != null && req.getHeader(HttpHeaders.IF_MODIFIED_SINCE) == null) { if (rsc != null && req.getHeader(HttpHeaders.IF_MODIFIED_SINCE) == null) {
rsc.send(req, res); rsc.send(req, res);

31
gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/PluginResourceKey.java
View File

@@ -14,34 +14,21 @@
package com.google.gerrit.httpd.plugins; package com.google.gerrit.httpd.plugins;
import com.google.auto.value.AutoValue;
import com.google.gerrit.httpd.resources.ResourceKey; import com.google.gerrit.httpd.resources.ResourceKey;
import com.google.gerrit.server.plugins.Plugin; import com.google.gerrit.server.plugins.Plugin;
final class PluginResourceKey implements ResourceKey { @AutoValue
private final Plugin.CacheKey plugin; abstract class PluginResourceKey implements ResourceKey {
private final String resource; static PluginResourceKey create(Plugin p, String r) {
return new AutoValue_PluginResourceKey(p.getCacheKey(), r);
PluginResourceKey(Plugin p, String r) {
this.plugin = p.getCacheKey();
this.resource = r;
} }
public abstract Plugin.CacheKey plugin();
public abstract String resource();
@Override @Override
public int weigh() { public int weigh() {
return resource.length() * 2; return resource().length() * 2;
}
@Override
public int hashCode() {
return plugin.hashCode() * 31 + resource.hashCode();
}
@Override
public boolean equals(Object other) {
if (other instanceof PluginResourceKey) {
PluginResourceKey rk = (PluginResourceKey) other;
return plugin == rk.plugin && resource.equals(rk.resource);
}
return false;
} }
} }

44
gerrit-server/src/main/java/com/google/gerrit/audit/AuditEvent.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.audit; package com.google.gerrit.audit;
import com.google.auto.value.AutoValue;
import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.common.collect.HashMultimap; import com.google.common.collect.HashMultimap;
@@ -36,41 +37,14 @@ public class AuditEvent {
public final long elapsed; public final long elapsed;
public final UUID uuid; public final UUID uuid;
public static class UUID { @AutoValue
public abstract static class UUID {
protected final String uuid; private static UUID create() {
return new AutoValue_AuditEvent_UUID(
protected UUID() { String.format("audit:%s", java.util.UUID.randomUUID().toString()));
uuid = String.format("audit:%s", java.util.UUID.randomUUID().toString());
} }
public UUID(final String n) { public abstract String uuid();
uuid = n;
}
public String get() {
return uuid;
}
@Override
public int hashCode() {
return uuid.hashCode();
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (!(obj instanceof UUID)) {
return false;
}
return uuid.equals(((UUID) obj).uuid);
}
} }
/** /**
@@ -93,7 +67,7 @@ public class AuditEvent {
this.when = when; this.when = when;
this.timeAtStart = this.when; this.timeAtStart = this.when;
this.params = MoreObjects.firstNonNull(params, EMPTY_PARAMS); this.params = MoreObjects.firstNonNull(params, EMPTY_PARAMS);
this.uuid = new UUID(); this.uuid = UUID.create();
this.result = result; this.result = result;
this.elapsed = TimeUtil.nowMs() - timeAtStart; this.elapsed = TimeUtil.nowMs() - timeAtStart;
} }
@@ -116,6 +90,6 @@ public class AuditEvent {
@Override @Override
public String toString() { public String toString() {
return String.format("AuditEvent UUID:%s, SID:%s, TS:%d, who:%s, what:%s", return String.format("AuditEvent UUID:%s, SID:%s, TS:%d, who:%s, what:%s",
uuid.get(), sessionId, when, who, what); uuid.uuid(), sessionId, when, who, what);
} }
} }

34
gerrit-server/src/main/java/com/google/gerrit/server/auth/AuthUser.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.auth;
import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkNotNull;
import com.google.auto.value.AutoValue;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
/** /**
@@ -26,40 +27,19 @@ public class AuthUser {
/** /**
* Globally unique identifier for the user. * Globally unique identifier for the user.
*/ */
public static final class UUID { @AutoValue
private final String uuid; public abstract static class UUID {
/** /**
* A new unique identifier. * A new unique identifier.
* *
* @param uuid the unique identifier. * @param uuid the unique identifier.
* @return identifier instance.
*/ */
public UUID(String uuid) { public static UUID create(String uuid) {
this.uuid = checkNotNull(uuid); return new AutoValue_AuthUser_UUID(uuid);
} }
/** @return the globally unique identifier. */ public abstract String uuid();
public String get() {
return uuid;
}
@Override
public boolean equals(Object obj) {
if (obj instanceof UUID) {
return get().equals(((UUID) obj).get());
}
return false;
}
@Override
public int hashCode() {
return get().hashCode();
}
@Override
public String toString() {
return String.format("AuthUser.UUID[%s]", get());
}
} }
private final UUID uuid; private final UUID uuid;

2
gerrit-server/src/main/java/com/google/gerrit/server/auth/InternalAuthBackend.java
View File

@@ -64,6 +64,6 @@ public class InternalAuthBackend implements AuthBackend {
} }
req.checkPassword(who.getPassword(username)); req.checkPassword(who.getPassword(username));
return new AuthUser(new AuthUser.UUID(username), username); return new AuthUser(AuthUser.UUID.create(username), username);
} }
} }

2
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java
View File

@@ -91,7 +91,7 @@ public class LdapAuthBackend implements AuthBackend {
// //
helper.authenticate(m.getDN(), req.getPassword()).close(); helper.authenticate(m.getDN(), req.getPassword()).close();
} }
return new AuthUser(new AuthUser.UUID(username), username); return new AuthUser(AuthUser.UUID.create(username), username);
} finally { } finally {
try { try {
ctx.close(); ctx.close();

2
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@@ -525,7 +525,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
} }
private void addLabelDelta(String name, short value) { private void addLabelDelta(String name, short value) {
labelDelta.add(new LabelVote(name, value).format()); labelDelta.add(LabelVote.create(name, value).format());
} }
private boolean insertMessage(RevisionResource rsrc, String msg, private boolean insertMessage(RevisionResource rsrc, String msg,

6
gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
View File

@@ -442,7 +442,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
update.putApproval(submit.getLabel(), submit.getValue()); update.putApproval(submit.getLabel(), submit.getValue());
dbProvider.get().patchSetApprovals().upsert(normalized.getNormalized()); dbProvider.get().patchSetApprovals().upsert(normalized.getNormalized());
dbProvider.get().patchSetApprovals().delete(normalized.getDeleted()); dbProvider.get().patchSetApprovals().delete(normalized.deleted());
try { try {
return saveToBatch(rsrc, update, normalized, timestamp); return saveToBatch(rsrc, update, normalized, timestamp);
@@ -455,11 +455,11 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
ChangeUpdate callerUpdate, LabelNormalizer.Result normalized, ChangeUpdate callerUpdate, LabelNormalizer.Result normalized,
Timestamp timestamp) throws IOException { Timestamp timestamp) throws IOException {
Table<Account.Id, String, Optional<Short>> byUser = HashBasedTable.create(); Table<Account.Id, String, Optional<Short>> byUser = HashBasedTable.create();
for (PatchSetApproval psa : normalized.getUpdated()) { for (PatchSetApproval psa : normalized.updated()) {
byUser.put(psa.getAccountId(), psa.getLabel(), byUser.put(psa.getAccountId(), psa.getLabel(),
Optional.of(psa.getValue())); Optional.of(psa.getValue()));
} }
for (PatchSetApproval psa : normalized.getDeleted()) { for (PatchSetApproval psa : normalized.deleted()) {
byUser.put(psa.getAccountId(), psa.getLabel(), Optional.<Short> absent()); byUser.put(psa.getAccountId(), psa.getLabel(), Optional.<Short> absent());
} }

62
gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
View File

@@ -16,8 +16,8 @@ package com.google.gerrit.server.git;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
@@ -37,7 +37,6 @@ import com.google.inject.Singleton;
import java.util.Collection; import java.util.Collection;
import java.util.List; import java.util.List;
import java.util.Objects;
/** /**
* Normalizes votes on labels according to project config and permissions. * Normalizes votes on labels according to project config and permissions.
@@ -50,60 +49,25 @@ import java.util.Objects;
*/ */
@Singleton @Singleton
public class LabelNormalizer { public class LabelNormalizer {
public static class Result { @AutoValue
private final ImmutableList<PatchSetApproval> unchanged; public abstract static class Result {
private final ImmutableList<PatchSetApproval> updated;
private final ImmutableList<PatchSetApproval> deleted;
@VisibleForTesting @VisibleForTesting
Result( static Result create(
List<PatchSetApproval> unchanged, List<PatchSetApproval> unchanged,
List<PatchSetApproval> updated, List<PatchSetApproval> updated,
List<PatchSetApproval> deleted) { List<PatchSetApproval> deleted) {
this.unchanged = ImmutableList.copyOf(unchanged); return new AutoValue_LabelNormalizer_Result(
this.updated = ImmutableList.copyOf(updated); ImmutableList.copyOf(unchanged),
this.deleted = ImmutableList.copyOf(deleted); ImmutableList.copyOf(updated),
ImmutableList.copyOf(deleted));
} }
public ImmutableList<PatchSetApproval> getUnchanged() { public abstract ImmutableList<PatchSetApproval> unchanged();
return unchanged; public abstract ImmutableList<PatchSetApproval> updated();
} public abstract ImmutableList<PatchSetApproval> deleted();
public ImmutableList<PatchSetApproval> getUpdated() {
return updated;
}
public ImmutableList<PatchSetApproval> getDeleted() {
return deleted;
}
public Iterable<PatchSetApproval> getNormalized() { public Iterable<PatchSetApproval> getNormalized() {
return Iterables.concat(unchanged, updated); return Iterables.concat(unchanged(), updated());
}
@Override
public boolean equals(Object o) {
if (o instanceof Result) {
Result r = (Result) o;
return Objects.equals(unchanged, r.unchanged)
&& Objects.equals(updated, r.updated)
&& Objects.equals(deleted, r.deleted);
}
return false;
}
@Override
public int hashCode() {
return Objects.hash(unchanged, updated, deleted);
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("unchanged", unchanged)
.add("updated", updated)
.add("deleted", deleted)
.toString();
} }
} }
@@ -174,7 +138,7 @@ public class LabelNormalizer {
unchanged.add(psa); unchanged.add(psa);
} }
} }
return new Result(unchanged, updated, deleted); return Result.create(unchanged, updated, deleted);
} }
/** /**

6
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -1142,12 +1142,12 @@ public class ReceiveCommits {
void addLabel(final String token) throws CmdLineException { void addLabel(final String token) throws CmdLineException {
LabelVote v = LabelVote.parse(token); LabelVote v = LabelVote.parse(token);
try { try {
LabelType.checkName(v.getLabel()); LabelType.checkName(v.label());
ApprovalsUtil.checkLabel(labelTypes, v.getLabel(), v.getValue()); ApprovalsUtil.checkLabel(labelTypes, v.label(), v.value());
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
throw clp.reject(e.getMessage()); throw clp.reject(e.getMessage());
} }
labels.put(v.getLabel(), v.getValue()); labels.put(v.label(), v.value());
} }
@Option(name = "--hashtag", aliases = {"-t"}, metaVar = "HASHTAG", @Option(name = "--hashtag", aliases = {"-t"}, metaVar = "HASHTAG",

8
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
View File

@@ -310,13 +310,13 @@ class ChangeNotesParser implements AutoCloseable {
pe.initCause(e); pe.initCause(e);
throw pe; throw pe;
} }
if (!curr.contains(accountId, l.getLabel())) { if (!curr.contains(accountId, l.label())) {
curr.put(accountId, l.getLabel(), Optional.of(new PatchSetApproval( curr.put(accountId, l.label(), Optional.of(new PatchSetApproval(
new PatchSetApproval.Key( new PatchSetApproval.Key(
psId, psId,
accountId, accountId,
new LabelId(l.getLabel())), new LabelId(l.label())),
l.getValue(), l.value(),
new Timestamp(commit.getCommitterIdent().getWhen().getTime())))); new Timestamp(commit.getCommitterIdent().getWhen().getTime()))));
} }
} }

4
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@@ -462,8 +462,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
if (!e.getValue().isPresent()) { if (!e.getValue().isPresent()) {
addFooter(msg, FOOTER_LABEL, '-', e.getKey()); addFooter(msg, FOOTER_LABEL, '-', e.getKey());
} else { } else {
addFooter(msg, FOOTER_LABEL, addFooter(msg, FOOTER_LABEL, LabelVote.create(
new LabelVote(e.getKey(), e.getValue().get()).formatWithEquals()); e.getKey(), e.getValue().get()).formatWithEquals());
} }
} }

52
gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
View File

@@ -17,8 +17,10 @@ package com.google.gerrit.server.project;
import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.gerrit.server.project.RefControl.isRE; import static com.google.gerrit.server.project.RefControl.isRE;
import com.google.auto.value.AutoValue;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.common.data.PermissionRule;
@@ -127,7 +129,7 @@ public class PermissionCollection {
exclusiveGroupPermissions.contains(permission.getName()); exclusiveGroupPermissions.contains(permission.getName());
for (PermissionRule rule : permission.getRules()) { for (PermissionRule rule : permission.getRules()) {
SeenRule s = new SeenRule(section, permission, rule); SeenRule s = SeenRule.create(section, permission, rule);
boolean addRule; boolean addRule;
if (rule.isBlock()) { if (rule.isBlock()) {
addRule = true; addRule = true;
@@ -149,7 +151,7 @@ public class PermissionCollection {
p.put(permission.getName(), r); p.put(permission.getName(), r);
} }
r.add(rule); r.add(rule);
ruleProps.put(rule, new ProjectRef(project, section.getName())); ruleProps.put(rule, ProjectRef.create(project, section.getName()));
} }
} }
@@ -224,41 +226,19 @@ public class PermissionCollection {
} }
/** Tracks whether or not a permission has been overridden. */ /** Tracks whether or not a permission has been overridden. */
private static class SeenRule { @AutoValue
final String refPattern; abstract static class SeenRule {
final String permissionName; public abstract String refPattern();
final AccountGroup.UUID group; public abstract String permissionName();
@Nullable public abstract AccountGroup.UUID group();
SeenRule(AccessSection section, Permission permission, PermissionRule rule) { static SeenRule create(AccessSection section, Permission permission,
refPattern = section.getName(); @Nullable PermissionRule rule) {
permissionName = permission.getName(); AccountGroup.UUID group = rule != null && rule.getGroup() != null
group = rule.getGroup().getUUID(); ? rule.getGroup().getUUID()
} : null;
return new AutoValue_PermissionCollection_SeenRule(
@Override section.getName(), permission.getName(), group);
public int hashCode() {
int hc = refPattern.hashCode();
hc = hc * 31 + permissionName.hashCode();
if (group != null) {
hc = hc * 31 + group.hashCode();
}
return hc;
}
@Override
public boolean equals(Object other) {
if (other instanceof SeenRule) {
SeenRule a = this;
SeenRule b = (SeenRule) other;
return a.refPattern.equals(b.refPattern) //
&& a.permissionName.equals(b.permissionName) //
&& eq(a.group, b.group);
}
return false;
}
private boolean eq(AccountGroup.UUID a, AccountGroup.UUID b) {
return a != null && b != null && a.equals(b);
} }
} }
} }

31
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectRef.java
View File

@@ -14,32 +14,15 @@
package com.google.gerrit.server.project; package com.google.gerrit.server.project;
import com.google.auto.value.AutoValue;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
class ProjectRef { @AutoValue
abstract class ProjectRef {
public abstract Project.NameKey project();
public abstract String ref();
final Project.NameKey project; static ProjectRef create(Project.NameKey project, String ref) {
final String ref; return new AutoValue_ProjectRef(project, ref);
ProjectRef(Project.NameKey project, String ref) {
this.project = project;
this.ref = ref;
}
@Override
public boolean equals(Object other) {
return other instanceof ProjectRef
&& project.equals(((ProjectRef) other).project)
&& ref.equals(((ProjectRef) other).ref);
}
@Override
public int hashCode() {
return project.hashCode() * 31 + ref.hashCode();
}
@Override
public String toString() {
return project + ", " + ref;
} }
} }

40
gerrit-server/src/main/java/com/google/gerrit/server/project/SectionSortCache.java
View File

@@ -14,7 +14,9 @@
package com.google.gerrit.server.project; package com.google.gerrit.server.project;
import com.google.auto.value.AutoValue;
import com.google.common.cache.Cache; import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.util.MostSpecificComparator; import com.google.gerrit.server.util.MostSpecificComparator;
@@ -26,7 +28,7 @@ import com.google.inject.name.Named;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import java.util.Arrays; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.IdentityHashMap; import java.util.IdentityHashMap;
import java.util.List; import java.util.List;
@@ -62,7 +64,7 @@ public class SectionSortCache {
return; return;
} }
EntryKey key = new EntryKey(ref, sections); EntryKey key = EntryKey.create(ref, sections);
EntryVal val = cache.getIfPresent(key); EntryVal val = cache.getIfPresent(key);
if (val != null) { if (val != null) {
int[] srcIdx = val.order; int[] srcIdx = val.order;
@@ -116,35 +118,27 @@ public class SectionSortCache {
return true; return true;
} }
static final class EntryKey { @AutoValue
private final String ref; static abstract class EntryKey {
private final String[] patterns; public abstract String ref();
private final int hashCode; public abstract List<String> patterns();
public abstract int cachedHashCode();
EntryKey(String refName, List<AccessSection> sections) { static EntryKey create(String refName, List<AccessSection> sections) {
int hc = refName.hashCode(); int hc = refName.hashCode();
ref = refName; List<String> patterns = new ArrayList<>(sections.size());
patterns = new String[sections.size()]; for (AccessSection s : sections) {
for (int i = 0; i < patterns.length; i++) { String n = s.getName();
String n = sections.get(i).getName(); patterns.add(n);
patterns[i] = n;
hc = hc * 31 + n.hashCode(); hc = hc * 31 + n.hashCode();
} }
hashCode = hc; return new AutoValue_SectionSortCache_EntryKey(
refName, ImmutableList.copyOf(patterns), hc);
} }
@Override @Override
public int hashCode() { public int hashCode() {
return hashCode; return cachedHashCode();
}
@Override
public boolean equals(Object other) {
if (other instanceof EntryKey) {
EntryKey b = (EntryKey) other;
return ref.equals(b.ref) && Arrays.equals(patterns, b.patterns);
}
return false;
} }
} }

4
gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
View File

@@ -90,14 +90,14 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
try { try {
LabelVote lv = LabelVote.parse(v); LabelVote lv = LabelVote.parse(v);
parsed = new Parsed(lv.getLabel(), "=", lv.getValue()); parsed = new Parsed(lv.label(), "=", lv.value());
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
// Try next format. // Try next format.
} }
try { try {
LabelVote lv = LabelVote.parseWithEquals(v); LabelVote lv = LabelVote.parseWithEquals(v);
parsed = new Parsed(lv.getLabel(), "=", lv.getValue()); parsed = new Parsed(lv.label(), "=", lv.value());
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
// Try next format. // Try next format.
} }

66
gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java
View File

@@ -16,18 +16,18 @@ package com.google.gerrit.server.util;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import com.google.auto.value.AutoValue;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import java.util.Objects;
/** A single vote on a label, consisting of a label name and a value. */ /** A single vote on a label, consisting of a label name and a value. */
public class LabelVote { @AutoValue
public abstract class LabelVote {
public static LabelVote parse(String text) { public static LabelVote parse(String text) {
checkArgument(!Strings.isNullOrEmpty(text), "Empty label vote"); checkArgument(!Strings.isNullOrEmpty(text), "Empty label vote");
if (text.charAt(0) == '-') { if (text.charAt(0) == '-') {
return new LabelVote(text.substring(1), (short) 0); return create(text.substring(1), (short) 0);
} }
short sign = 0; short sign = 0;
int i; int i;
@@ -44,9 +44,9 @@ public class LabelVote {
} }
} }
if (sign == 0) { if (sign == 0) {
return new LabelVote(text, (short) 1); return create(text, (short) 1);
} }
return new LabelVote(text.substring(0, i), return create(text.substring(0, i),
(short)(sign * Short.parseShort(text.substring(i + 1)))); (short)(sign * Short.parseShort(text.substring(i + 1))));
} }
@@ -54,63 +54,39 @@ public class LabelVote {
checkArgument(!Strings.isNullOrEmpty(text), "Empty label vote"); checkArgument(!Strings.isNullOrEmpty(text), "Empty label vote");
int e = text.lastIndexOf('='); int e = text.lastIndexOf('=');
checkArgument(e >= 0, "Label vote missing '=': %s", text); checkArgument(e >= 0, "Label vote missing '=': %s", text);
return new LabelVote(text.substring(0, e), return create(text.substring(0, e),
Short.parseShort(text.substring(e + 1), text.length())); Short.parseShort(text.substring(e + 1), text.length()));
} }
private final String name; public static LabelVote create(String label, short value) {
private final short value; return new AutoValue_LabelVote(LabelType.checkNameInternal(label), value);
public LabelVote(String name, short value) {
this.name = LabelType.checkNameInternal(name);
this.value = value;
} }
public LabelVote(PatchSetApproval psa) { public static LabelVote create(PatchSetApproval psa) {
this(psa.getLabel(), psa.getValue()); return create(psa.getLabel(), psa.getValue());
} }
public String getLabel() { public abstract String label();
return name; public abstract short value();
}
public short getValue() {
return value;
}
public String format() { public String format() {
if (value == (short) 0) { if (value() == (short) 0) {
return '-' + name; return '-' + label();
} else if (value < 0) { } else if (value() < 0) {
return name + value; return label() + value();
} else { } else {
return name + '+' + value; return label() + '+' + value();
} }
} }
public String formatWithEquals() { public String formatWithEquals() {
if (value <= (short) 0) { if (value() <= (short) 0) {
return name + '=' + value; return label() + '=' + value();
} else { } else {
return name + "=+" + value; return label() + "=+" + value();
} }
} }
@Override
public boolean equals(Object o) {
if (o instanceof LabelVote) {
LabelVote l = (LabelVote) o;
return Objects.equals(name, l.name)
&& value == l.value;
}
return false;
}
@Override
public int hashCode() {
return 17 * value + name.hashCode();
}
@Override @Override
public String toString() { public String toString() {
return format(); return format();

8
gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java
View File

@@ -137,7 +137,7 @@ public class LabelNormalizerTest {
PatchSetApproval cr = psa(userId, "Code-Review", 2); PatchSetApproval cr = psa(userId, "Code-Review", 2);
PatchSetApproval v = psa(userId, "Verified", 1); PatchSetApproval v = psa(userId, "Verified", 1);
assertEquals(new Result( assertEquals(Result.create(
list(v), list(v),
list(copy(cr, 1)), list(copy(cr, 1)),
list()), list()),
@@ -153,7 +153,7 @@ public class LabelNormalizerTest {
PatchSetApproval cr = psa(userId, "Code-Review", 5); PatchSetApproval cr = psa(userId, "Code-Review", 5);
PatchSetApproval v = psa(userId, "Verified", 5); PatchSetApproval v = psa(userId, "Verified", 5);
assertEquals(new Result( assertEquals(Result.create(
list(), list(),
list(copy(cr, 2), copy(v, 1)), list(copy(cr, 2), copy(v, 1)),
list()), list()),
@@ -164,7 +164,7 @@ public class LabelNormalizerTest {
public void emptyPermissionRangeOmitsResult() throws Exception { public void emptyPermissionRangeOmitsResult() throws Exception {
PatchSetApproval cr = psa(userId, "Code-Review", 1); PatchSetApproval cr = psa(userId, "Code-Review", 1);
PatchSetApproval v = psa(userId, "Verified", 1); PatchSetApproval v = psa(userId, "Verified", 1);
assertEquals(new Result( assertEquals(Result.create(
list(), list(),
list(), list(),
list(cr, v)), list(cr, v)),
@@ -179,7 +179,7 @@ public class LabelNormalizerTest {
PatchSetApproval cr = psa(userId, "Code-Review", 0); PatchSetApproval cr = psa(userId, "Code-Review", 0);
PatchSetApproval v = psa(userId, "Verified", 0); PatchSetApproval v = psa(userId, "Verified", 0);
assertEquals(new Result( assertEquals(Result.create(
list(cr), list(cr),
list(), list(),
list(v)), list(v)),

52
gerrit-server/src/test/java/com/google/gerrit/server/util/LabelVoteTest.java
View File

@@ -23,23 +23,23 @@ public class LabelVoteTest {
public void parse() { public void parse() {
LabelVote l; LabelVote l;
l = LabelVote.parse("Code-Review-2"); l = LabelVote.parse("Code-Review-2");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) -2, l.getValue()); assertEquals((short) -2, l.value());
l = LabelVote.parse("Code-Review-1"); l = LabelVote.parse("Code-Review-1");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) -1, l.getValue()); assertEquals((short) -1, l.value());
l = LabelVote.parse("-Code-Review"); l = LabelVote.parse("-Code-Review");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) 0, l.getValue()); assertEquals((short) 0, l.value());
l = LabelVote.parse("Code-Review"); l = LabelVote.parse("Code-Review");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) 1, l.getValue()); assertEquals((short) 1, l.value());
l = LabelVote.parse("Code-Review+1"); l = LabelVote.parse("Code-Review+1");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) 1, l.getValue()); assertEquals((short) 1, l.value());
l = LabelVote.parse("Code-Review+2"); l = LabelVote.parse("Code-Review+2");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) 2, l.getValue()); assertEquals((short) 2, l.value());
} }
@Test @Test
@@ -55,26 +55,26 @@ public class LabelVoteTest {
public void parseWithEquals() { public void parseWithEquals() {
LabelVote l; LabelVote l;
l = LabelVote.parseWithEquals("Code-Review=-2"); l = LabelVote.parseWithEquals("Code-Review=-2");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) -2, l.getValue()); assertEquals((short) -2, l.value());
l = LabelVote.parseWithEquals("Code-Review=-1"); l = LabelVote.parseWithEquals("Code-Review=-1");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) -1, l.getValue()); assertEquals((short) -1, l.value());
l = LabelVote.parseWithEquals("Code-Review=0"); l = LabelVote.parseWithEquals("Code-Review=0");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) 0, l.getValue()); assertEquals((short) 0, l.value());
l = LabelVote.parseWithEquals("Code-Review=1"); l = LabelVote.parseWithEquals("Code-Review=1");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) 1, l.getValue()); assertEquals((short) 1, l.value());
l = LabelVote.parseWithEquals("Code-Review=+1"); l = LabelVote.parseWithEquals("Code-Review=+1");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) 1, l.getValue()); assertEquals((short) 1, l.value());
l = LabelVote.parseWithEquals("Code-Review=2"); l = LabelVote.parseWithEquals("Code-Review=2");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) 2, l.getValue()); assertEquals((short) 2, l.value());
l = LabelVote.parseWithEquals("Code-Review=+2"); l = LabelVote.parseWithEquals("Code-Review=+2");
assertEquals("Code-Review", l.getLabel()); assertEquals("Code-Review", l.label());
assertEquals((short) 2, l.getValue()); assertEquals((short) 2, l.value());
} }
@Test @Test

1
gerrit-sshd/BUCK
View File

@@ -17,6 +17,7 @@ java_library(
'//lib:guava', '//lib:guava',
'//lib:gwtorm', '//lib:gwtorm',
'//lib:jsch', '//lib:jsch',
'//lib/auto:auto-value',
'//lib/commons:codec', '//lib/commons:codec',
'//lib/commons:collections', '//lib/commons:collections',
'//lib/guice:guice', '//lib/guice:guice',

4
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
View File

@@ -122,8 +122,8 @@ public class ReviewCommand extends SshCommand {
@Option(name = "--label", aliases = "-l", usage = "custom label(s) to assign", metaVar = "LABEL=VALUE") @Option(name = "--label", aliases = "-l", usage = "custom label(s) to assign", metaVar = "LABEL=VALUE")
void addLabel(final String token) { void addLabel(final String token) {
LabelVote v = LabelVote.parseWithEquals(token); LabelVote v = LabelVote.parseWithEquals(token);
LabelType.checkName(v.getLabel()); // Disallow SUBM. LabelType.checkName(v.label()); // Disallow SUBM.
customLabels.put(v.getLabel(), v.getValue()); customLabels.put(v.label(), v.value());
} }
@Inject @Inject