From da0d9a3c5f2989988c35b2136d25b35f910776d0 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 21 Feb 2017 13:44:46 -0500 Subject: [PATCH] Reduce FieldDef boilerplate with a builder Inspired by Han-Wen's entirely reasonable annotation of the "stored" argument to the FieldDef constructor[1], I thought this smells like it could use the builder pattern to improve readability. Passing a functional interface to the build method also means we can use lambdas, for an overall significant boilerplate reduction: many FieldDefs can now be one-liners. [1] https://gerrit-review.googlesource.com/c/98014/9/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java#606 Change-Id: I845a7d9a28dda7f3e0cc0c049b94372118eb4480 --- .../elasticsearch/ElasticChangeIndex.java | 13 +- .../gerrit/lucene/LuceneChangeIndex.java | 12 +- .../google/gerrit/server/index/FieldDef.java | 128 +++- .../server/index/account/AccountField.java | 112 ++- .../server/index/change/ChangeField.java | 687 ++++++------------ .../gerrit/server/index/group/GroupField.java | 56 +- .../server/query/change/AuthorPredicate.java | 7 +- .../query/change/CommitterPredicate.java | 7 +- 8 files changed, 388 insertions(+), 634 deletions(-) diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java index 83bb3afd96..870b88ac80 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java @@ -14,6 +14,9 @@ package com.google.gerrit.elasticsearch; +import static com.google.gerrit.server.index.change.ChangeField.APPROVAL_CODEC; +import static com.google.gerrit.server.index.change.ChangeField.CHANGE_CODEC; +import static com.google.gerrit.server.index.change.ChangeField.PATCH_SET_CODEC; import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES; import static com.google.gerrit.server.index.change.ChangeIndexRewriter.OPEN_STATUSES; import static java.nio.charset.StandardCharsets.UTF_8; @@ -39,9 +42,6 @@ import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.index.QueryOptions; import com.google.gerrit.server.index.Schema; import com.google.gerrit.server.index.change.ChangeField; -import com.google.gerrit.server.index.change.ChangeField.ChangeProtoField; -import com.google.gerrit.server.index.change.ChangeField.PatchSetApprovalProtoField; -import com.google.gerrit.server.index.change.ChangeField.PatchSetProtoField; import com.google.gerrit.server.index.change.ChangeIndex; import com.google.gerrit.server.index.change.ChangeIndexRewriter; import com.google.gerrit.server.project.SubmitRuleOptions; @@ -280,16 +280,15 @@ class ElasticChangeIndex extends AbstractElasticIndex ChangeData cd = changeDataFactory.create( - db.get(), ChangeProtoField.CODEC.decode(Base64.decodeBase64(c.getAsString()))); + db.get(), CHANGE_CODEC.decode(Base64.decodeBase64(c.getAsString()))); // Patch sets. - cd.setPatchSets( - decodeProtos(source, ChangeField.PATCH_SET.getName(), PatchSetProtoField.CODEC)); + cd.setPatchSets(decodeProtos(source, ChangeField.PATCH_SET.getName(), PATCH_SET_CODEC)); // Approvals. if (source.get(ChangeField.APPROVAL.getName()) != null) { cd.setCurrentApprovals( - decodeProtos(source, ChangeField.APPROVAL.getName(), PatchSetApprovalProtoField.CODEC)); + decodeProtos(source, ChangeField.APPROVAL.getName(), APPROVAL_CODEC)); } else if (fields.contains(ChangeField.APPROVAL.getName())) { cd.setCurrentApprovals(Collections.emptyList()); } diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java index ba5780ee41..96986a9830 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -17,7 +17,10 @@ package com.google.gerrit.lucene; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.gerrit.lucene.AbstractLuceneIndex.sortFieldName; import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE; +import static com.google.gerrit.server.index.change.ChangeField.APPROVAL_CODEC; +import static com.google.gerrit.server.index.change.ChangeField.CHANGE_CODEC; import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID; +import static com.google.gerrit.server.index.change.ChangeField.PATCH_SET_CODEC; import static com.google.gerrit.server.index.change.ChangeField.PROJECT; import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES; import static com.google.gerrit.server.index.change.ChangeIndexRewriter.OPEN_STATUSES; @@ -47,9 +50,6 @@ import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.index.QueryOptions; import com.google.gerrit.server.index.Schema; import com.google.gerrit.server.index.change.ChangeField; -import com.google.gerrit.server.index.change.ChangeField.ChangeProtoField; -import com.google.gerrit.server.index.change.ChangeField.PatchSetApprovalProtoField; -import com.google.gerrit.server.index.change.ChangeField.PatchSetProtoField; import com.google.gerrit.server.index.change.ChangeIndex; import com.google.gerrit.server.index.change.ChangeIndexRewriter; import com.google.gerrit.server.project.SubmitRuleOptions; @@ -421,7 +421,7 @@ public class LuceneChangeIndex implements ChangeIndex { BytesRef proto = cb.binaryValue(); cd = changeDataFactory.create( - db.get(), ChangeProtoField.CODEC.decode(proto.bytes, proto.offset, proto.length)); + db.get(), CHANGE_CODEC.decode(proto.bytes, proto.offset, proto.length)); } else { IndexableField f = Iterables.getFirst(doc.get(idFieldName), null); Change.Id id = new Change.Id(f.numericValue().intValue()); @@ -475,7 +475,7 @@ public class LuceneChangeIndex implements ChangeIndex { } private void decodePatchSets(ListMultimap doc, ChangeData cd) { - List patchSets = decodeProtos(doc, PATCH_SET_FIELD, PatchSetProtoField.CODEC); + List patchSets = decodeProtos(doc, PATCH_SET_FIELD, PATCH_SET_CODEC); if (!patchSets.isEmpty()) { // Will be an empty list for schemas prior to when this field was stored; // this cannot be valid since a change needs at least one patch set. @@ -484,7 +484,7 @@ public class LuceneChangeIndex implements ChangeIndex { } private void decodeApprovals(ListMultimap doc, ChangeData cd) { - cd.setCurrentApprovals(decodeProtos(doc, APPROVAL_FIELD, PatchSetApprovalProtoField.CODEC)); + cd.setCurrentApprovals(decodeProtos(doc, APPROVAL_FIELD, APPROVAL_CODEC)); } private void decodeChangedLines(ListMultimap doc, ChangeData cd) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/FieldDef.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/FieldDef.java index de654d5e4b..d5f109102f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/FieldDef.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/FieldDef.java @@ -15,14 +15,16 @@ package com.google.gerrit.server.index; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.base.CharMatcher; -import com.google.common.base.Preconditions; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.TrackingFooters; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; +import java.io.IOException; +import java.sql.Timestamp; import org.eclipse.jgit.lib.Config; /** @@ -32,31 +34,43 @@ import org.eclipse.jgit.lib.Config; * @param type that should be extracted from the input object when converting to an index * document. */ -public abstract class FieldDef { - /** Definition of a single (non-repeatable) field. */ - public abstract static class Single extends FieldDef { - protected Single(String name, FieldType type, boolean stored) { - super(name, type, stored); - } - - @Override - public final boolean isRepeatable() { - return false; - } +public final class FieldDef { + public static FieldDef.Builder exact(String name) { + return new FieldDef.Builder<>(FieldType.EXACT, name); } - /** Definition of a repeatable field. */ - public abstract static class Repeatable extends FieldDef> { - protected Repeatable(String name, FieldType type, boolean stored) { - super(name, type, stored); - Preconditions.checkArgument( - type != FieldType.INTEGER_RANGE, "Range queries against repeated fields are unsupported"); - } + public static FieldDef.Builder fullText(String name) { + return new FieldDef.Builder<>(FieldType.FULL_TEXT, name); + } - @Override - public final boolean isRepeatable() { - return true; - } + public static FieldDef.Builder intRange(String name) { + return new FieldDef.Builder<>(FieldType.INTEGER_RANGE, name).stored(); + } + + public static FieldDef.Builder integer(String name) { + return new FieldDef.Builder<>(FieldType.INTEGER, name); + } + + public static FieldDef.Builder prefix(String name) { + return new FieldDef.Builder<>(FieldType.PREFIX, name); + } + + public static FieldDef.Builder storedOnly(String name) { + return new FieldDef.Builder<>(FieldType.STORED_ONLY, name).stored(); + } + + public static FieldDef.Builder timestamp(String name) { + return new FieldDef.Builder<>(FieldType.TIMESTAMP, name); + } + + @FunctionalInterface + public interface Getter { + T get(I input) throws OrmException, IOException; + } + + @FunctionalInterface + public interface GetterWithArgs { + T get(I input, FillArgs args) throws OrmException, IOException; } /** Arguments needed to fill in missing data in the input object. */ @@ -74,34 +88,78 @@ public abstract class FieldDef { } } + public static class Builder { + private final FieldType type; + private final String name; + private boolean stored; + + public Builder(FieldType type, String name) { + this.type = checkNotNull(type); + this.name = checkNotNull(name); + } + + public Builder stored() { + this.stored = true; + return this; + } + + public FieldDef build(Getter getter) { + return build((in, a) -> getter.get(in)); + } + + public FieldDef build(GetterWithArgs getter) { + return new FieldDef<>(name, type, stored, false, getter); + } + + public FieldDef> buildRepeatable(Getter> getter) { + return buildRepeatable((in, a) -> getter.get(in)); + } + + public FieldDef> buildRepeatable(GetterWithArgs> getter) { + return new FieldDef<>(name, type, stored, true, getter); + } + } + private final String name; private final FieldType type; private final boolean stored; + private final boolean repeatable; + private final GetterWithArgs getter; - private FieldDef(String name, FieldType type, boolean stored) { + private FieldDef( + String name, + FieldType type, + boolean stored, + boolean repeatable, + GetterWithArgs getter) { + checkArgument( + !(repeatable && type == FieldType.INTEGER_RANGE), + "Range queries against repeated fields are unsupported"); this.name = checkName(name); - this.type = type; + this.type = checkNotNull(type); this.stored = stored; + this.repeatable = repeatable; + this.getter = checkNotNull(getter); } private static String checkName(String name) { CharMatcher m = CharMatcher.anyOf("abcdefghijklmnopqrstuvwxyz0123456789_"); - checkArgument(m.matchesAllOf(name), "illegal field name: %s", name); + checkArgument(name != null && m.matchesAllOf(name), "illegal field name: %s", name); return name; } /** @return name of the field. */ - public final String getName() { + public String getName() { return name; } /** @return type of the field; for repeatable fields, the inner type, not the iterable type. */ - public final FieldType getType() { + public FieldType getType() { return type; } /** @return whether the field should be stored in the index. */ - public final boolean isStored() { + public boolean isStored() { return stored; } @@ -113,8 +171,16 @@ public abstract class FieldDef { * @return the field value(s) to index. * @throws OrmException */ - public abstract T get(I input, FillArgs args) throws OrmException; + public T get(I input, FillArgs args) throws OrmException { + try { + return getter.get(input, args); + } catch (IOException e) { + throw new OrmException(e); + } + } /** @return whether the field is repeatable. */ - public abstract boolean isRepeatable(); + public boolean isRepeatable() { + return repeatable; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountField.java index e3de1700fd..96aec3f900 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountField.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountField.java @@ -14,6 +14,11 @@ package com.google.gerrit.server.index.account; +import static com.google.gerrit.server.index.FieldDef.exact; +import static com.google.gerrit.server.index.FieldDef.integer; +import static com.google.gerrit.server.index.FieldDef.prefix; +import static com.google.gerrit.server.index.FieldDef.timestamp; + import com.google.common.base.Predicates; import com.google.common.base.Strings; import com.google.common.collect.FluentIterable; @@ -21,7 +26,6 @@ import com.google.common.collect.Iterables; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.ExternalId; import com.google.gerrit.server.index.FieldDef; -import com.google.gerrit.server.index.FieldType; import com.google.gerrit.server.index.SchemaUtil; import java.sql.Timestamp; import java.util.Collections; @@ -31,94 +35,60 @@ import java.util.Set; /** Secondary index schemas for accounts. */ public class AccountField { public static final FieldDef ID = - new FieldDef.Single("id", FieldType.INTEGER, true) { - @Override - public Integer get(AccountState input, FillArgs args) { - return input.getAccount().getId().get(); - } - }; + integer("id").stored().build(a -> a.getAccount().getId().get()); public static final FieldDef> EXTERNAL_ID = - new FieldDef.Repeatable("external_id", FieldType.EXACT, false) { - @Override - public Iterable get(AccountState input, FillArgs args) { - return Iterables.transform(input.getExternalIds(), id -> id.key().get()); - } - }; + exact("external_id") + .buildRepeatable(a -> Iterables.transform(a.getExternalIds(), id -> id.key().get())); /** Fuzzy prefix match on name and email parts. */ public static final FieldDef> NAME_PART = - new FieldDef.Repeatable("name", FieldType.PREFIX, false) { - @Override - public Iterable get(AccountState input, FillArgs args) { - String fullName = input.getAccount().getFullName(); - Set parts = - SchemaUtil.getNameParts( - fullName, Iterables.transform(input.getExternalIds(), ExternalId::email)); + prefix("name") + .buildRepeatable( + a -> { + String fullName = a.getAccount().getFullName(); + Set parts = + SchemaUtil.getNameParts( + fullName, Iterables.transform(a.getExternalIds(), ExternalId::email)); - // Additional values not currently added by getPersonParts. - // TODO(dborowitz): Move to getPersonParts and remove this hack. - if (fullName != null) { - parts.add(fullName.toLowerCase(Locale.US)); - } - return parts; - } - }; + // Additional values not currently added by getPersonParts. + // TODO(dborowitz): Move to getPersonParts and remove this hack. + if (fullName != null) { + parts.add(fullName.toLowerCase(Locale.US)); + } + return parts; + }); public static final FieldDef FULL_NAME = - new FieldDef.Single("full_name", FieldType.EXACT, false) { - @Override - public String get(AccountState input, FillArgs args) { - return input.getAccount().getFullName(); - } - }; + exact("full_name").build(a -> a.getAccount().getFullName()); public static final FieldDef ACTIVE = - new FieldDef.Single("inactive", FieldType.EXACT, false) { - @Override - public String get(AccountState input, FillArgs args) { - return input.getAccount().isActive() ? "1" : "0"; - } - }; + exact("inactive").build(a -> a.getAccount().isActive() ? "1" : "0"); public static final FieldDef> EMAIL = - new FieldDef.Repeatable("email", FieldType.PREFIX, false) { - @Override - public Iterable get(AccountState input, FillArgs args) { - return FluentIterable.from(input.getExternalIds()) - .transform(ExternalId::email) - .append(Collections.singleton(input.getAccount().getPreferredEmail())) - .filter(Predicates.notNull()) - .transform(String::toLowerCase) - .toSet(); - } - }; + prefix("email") + .buildRepeatable( + a -> + FluentIterable.from(a.getExternalIds()) + .transform(ExternalId::email) + .append(Collections.singleton(a.getAccount().getPreferredEmail())) + .filter(Predicates.notNull()) + .transform(String::toLowerCase) + .toSet()); public static final FieldDef REGISTERED = - new FieldDef.Single("registered", FieldType.TIMESTAMP, false) { - @Override - public Timestamp get(AccountState input, FillArgs args) { - return input.getAccount().getRegisteredOn(); - } - }; + timestamp("registered").build(a -> a.getAccount().getRegisteredOn()); public static final FieldDef USERNAME = - new FieldDef.Single("username", FieldType.EXACT, false) { - @Override - public String get(AccountState input, FillArgs args) { - return Strings.nullToEmpty(input.getUserName()).toLowerCase(); - } - }; + exact("username").build(a -> Strings.nullToEmpty(a.getUserName()).toLowerCase()); public static final FieldDef> WATCHED_PROJECT = - new FieldDef.Repeatable("watchedproject", FieldType.EXACT, false) { - @Override - public Iterable get(AccountState input, FillArgs args) { - return FluentIterable.from(input.getProjectWatches().keySet()) - .transform(k -> k.project().get()) - .toSet(); - } - }; + exact("watchedproject") + .buildRepeatable( + a -> + FluentIterable.from(a.getProjectWatches().keySet()) + .transform(k -> k.project().get()) + .toSet()); private AccountField() {} } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java index e2ca3dd4cf..b8acadc6e9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java @@ -16,6 +16,13 @@ package com.google.gerrit.server.index.change; import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.gerrit.server.index.FieldDef.exact; +import static com.google.gerrit.server.index.FieldDef.fullText; +import static com.google.gerrit.server.index.FieldDef.intRange; +import static com.google.gerrit.server.index.FieldDef.integer; +import static com.google.gerrit.server.index.FieldDef.prefix; +import static com.google.gerrit.server.index.FieldDef.storedOnly; +import static com.google.gerrit.server.index.FieldDef.timestamp; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; @@ -42,7 +49,7 @@ import com.google.gerrit.server.OutputFormat; import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.index.FieldDef; -import com.google.gerrit.server.index.FieldType; +import com.google.gerrit.server.index.FieldDef.FillArgs; import com.google.gerrit.server.index.SchemaUtil; import com.google.gerrit.server.index.change.StalenessChecker.RefState; import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern; @@ -66,8 +73,8 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; +import java.util.function.Function; import org.eclipse.jgit.revwalk.FooterLine; /** @@ -87,138 +94,52 @@ public class ChangeField { /** Legacy change ID. */ public static final FieldDef LEGACY_ID = - new FieldDef.Single("legacy_id", FieldType.INTEGER, true) { - @Override - public Integer get(ChangeData input, FillArgs args) { - return input.getId().get(); - } - }; + integer("legacy_id").stored().build(cd -> cd.getId().get()); /** Newer style Change-Id key. */ public static final FieldDef ID = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_CHANGE_ID, FieldType.PREFIX, false) { - @Override - public String get(ChangeData input, FillArgs args) throws OrmException { - Change c = input.change(); - if (c == null) { - return null; - } - return c.getKey().get(); - } - }; + prefix(ChangeQueryBuilder.FIELD_CHANGE_ID).build(changeGetter(c -> c.getKey().get())); /** Change status string, in the same format as {@code status:}. */ public static final FieldDef STATUS = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_STATUS, FieldType.EXACT, false) { - @Override - public String get(ChangeData input, FillArgs args) throws OrmException { - Change c = input.change(); - if (c == null) { - return null; - } - return ChangeStatusPredicate.canonicalize(c.getStatus()); - } - }; + exact(ChangeQueryBuilder.FIELD_STATUS) + .build(changeGetter(c -> ChangeStatusPredicate.canonicalize(c.getStatus()))); /** Project containing the change. */ public static final FieldDef PROJECT = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_PROJECT, FieldType.EXACT, true) { - @Override - public String get(ChangeData input, FillArgs args) throws OrmException { - Change c = input.change(); - if (c == null) { - return null; - } - return c.getProject().get(); - } - }; + exact(ChangeQueryBuilder.FIELD_PROJECT) + .stored() + .build(changeGetter(c -> c.getProject().get())); /** Project containing the change, as a prefix field. */ public static final FieldDef PROJECTS = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_PROJECTS, FieldType.PREFIX, false) { - @Override - public String get(ChangeData input, FillArgs args) throws OrmException { - Change c = input.change(); - if (c == null) { - return null; - } - return c.getProject().get(); - } - }; + prefix(ChangeQueryBuilder.FIELD_PROJECTS).build(changeGetter(c -> c.getProject().get())); /** Reference (aka branch) the change will submit onto. */ public static final FieldDef REF = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_REF, FieldType.EXACT, false) { - @Override - public String get(ChangeData input, FillArgs args) throws OrmException { - Change c = input.change(); - if (c == null) { - return null; - } - return c.getDest().get(); - } - }; + exact(ChangeQueryBuilder.FIELD_REF).build(changeGetter(c -> c.getDest().get())); /** Topic, a short annotation on the branch. */ public static final FieldDef EXACT_TOPIC = - new FieldDef.Single("topic4", FieldType.EXACT, false) { - @Override - public String get(ChangeData input, FillArgs args) throws OrmException { - return getTopic(input); - } - }; + exact("topic4").build(ChangeField::getTopic); /** Topic, a short annotation on the branch. */ public static final FieldDef FUZZY_TOPIC = - new FieldDef.Single("topic5", FieldType.FULL_TEXT, false) { - @Override - public String get(ChangeData input, FillArgs args) throws OrmException { - return getTopic(input); - } - }; + fullText("topic5").build(ChangeField::getTopic); /** Submission id assigned by MergeOp. */ public static final FieldDef SUBMISSIONID = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_SUBMISSIONID, FieldType.EXACT, false) { - @Override - public String get(ChangeData input, FillArgs args) throws OrmException { - Change c = input.change(); - if (c == null) { - return null; - } - return c.getSubmissionId(); - } - }; + exact(ChangeQueryBuilder.FIELD_SUBMISSIONID).build(changeGetter(Change::getSubmissionId)); /** Last update time since January 1, 1970. */ public static final FieldDef UPDATED = - new FieldDef.Single("updated2", FieldType.TIMESTAMP, true) { - @Override - public Timestamp get(ChangeData input, FillArgs args) throws OrmException { - Change c = input.change(); - if (c == null) { - return null; - } - return c.getLastUpdatedOn(); - } - }; + timestamp("updated2").stored().build(changeGetter(Change::getLastUpdatedOn)); /** List of full file paths modified in the current patch set. */ public static final FieldDef> PATH = - new FieldDef.Repeatable( - // Named for backwards compatibility. - ChangeQueryBuilder.FIELD_FILE, FieldType.EXACT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return firstNonNull(input.currentFilePaths(), ImmutableList.of()); - } - }; + // Named for backwards compatibility. + exact(ChangeQueryBuilder.FIELD_FILE) + .buildRepeatable(cd -> firstNonNull(cd.currentFilePaths(), ImmutableList.of())); public static Set getFileParts(ChangeData cd) throws OrmException { List paths = cd.currentFilePaths(); @@ -237,66 +158,31 @@ public class ChangeField { /** Hashtags tied to a change */ public static final FieldDef> HASHTAG = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_HASHTAG, FieldType.EXACT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return input.hashtags().stream().map(String::toLowerCase).collect(toSet()); - } - }; + exact(ChangeQueryBuilder.FIELD_HASHTAG) + .buildRepeatable(cd -> cd.hashtags().stream().map(String::toLowerCase).collect(toSet())); /** Hashtags with original case. */ public static final FieldDef> HASHTAG_CASE_AWARE = - new FieldDef.Repeatable("_hashtag", FieldType.STORED_ONLY, true) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return input.hashtags().stream().map(t -> t.getBytes(UTF_8)).collect(toSet()); - } - }; + storedOnly("_hashtag") + .buildRepeatable( + cd -> cd.hashtags().stream().map(t -> t.getBytes(UTF_8)).collect(toSet())); /** Components of each file path modified in the current patch set. */ public static final FieldDef> FILE_PART = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_FILEPART, FieldType.EXACT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return getFileParts(input); - } - }; + exact(ChangeQueryBuilder.FIELD_FILEPART).buildRepeatable(ChangeField::getFileParts); /** Owner/creator of the change. */ public static final FieldDef OWNER = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_OWNER, FieldType.INTEGER, false) { - @Override - public Integer get(ChangeData input, FillArgs args) throws OrmException { - Change c = input.change(); - if (c == null) { - return null; - } - return c.getOwner().get(); - } - }; + integer(ChangeQueryBuilder.FIELD_OWNER).build(changeGetter(c -> c.getOwner().get())); /** The user assigned to the change. */ public static final FieldDef ASSIGNEE = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_ASSIGNEE, FieldType.INTEGER, false) { - @Override - public Integer get(ChangeData input, FillArgs args) throws OrmException { - Account.Id id = input.change().getAssignee(); - return id != null ? id.get() : NO_ASSIGNEE; - } - }; + integer(ChangeQueryBuilder.FIELD_ASSIGNEE) + .build(changeGetter(c -> c.getAssignee() != null ? c.getAssignee().get() : NO_ASSIGNEE)); /** Reviewer(s) associated with the change. */ public static final FieldDef> REVIEWER = - new FieldDef.Repeatable("reviewer2", FieldType.EXACT, true) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return getReviewerFieldValues(input.reviewers()); - } - }; + exact("reviewer2").stored().buildRepeatable(cd -> getReviewerFieldValues(cd.reviewers())); @VisibleForTesting static List getReviewerFieldValues(ReviewerSet reviewers) { @@ -336,23 +222,11 @@ public class ChangeField { /** Commit ID of any patch set on the change, using prefix match. */ public static final FieldDef> COMMIT = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_COMMIT, FieldType.PREFIX, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return getRevisions(input); - } - }; + prefix(ChangeQueryBuilder.FIELD_COMMIT).buildRepeatable(ChangeField::getRevisions); /** Commit ID of any patch set on the change, using exact match. */ public static final FieldDef> EXACT_COMMIT = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_EXACTCOMMIT, FieldType.EXACT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return getRevisions(input); - } - }; + exact(ChangeQueryBuilder.FIELD_EXACTCOMMIT).buildRepeatable(ChangeField::getRevisions); private static Set getRevisions(ChangeData cd) throws OrmException { Set revisions = new HashSet<>(); @@ -366,49 +240,32 @@ public class ChangeField { /** Tracking id extracted from a footer. */ public static final FieldDef> TR = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_TR, FieldType.EXACT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - try { - List footers = input.commitFooters(); - if (footers == null) { - return ImmutableSet.of(); - } - return Sets.newHashSet(args.trackingFooters.extract(footers).values()); - } catch (IOException e) { - throw new OrmException(e); - } - } - }; + exact(ChangeQueryBuilder.FIELD_TR) + .buildRepeatable( + (ChangeData cd, FillArgs a) -> { + List footers = cd.commitFooters(); + if (footers == null) { + return ImmutableSet.of(); + } + return Sets.newHashSet(a.trackingFooters.extract(footers).values()); + }); /** List of labels on the current patch set. */ @Deprecated public static final FieldDef> LABEL = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_LABEL, FieldType.EXACT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return getLabels(input, false); - } - }; + exact(ChangeQueryBuilder.FIELD_LABEL).buildRepeatable(cd -> getLabels(cd, false)); /** List of labels on the current patch set including change owner votes. */ public static final FieldDef> LABEL2 = - new FieldDef.Repeatable("label2", FieldType.EXACT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return getLabels(input, true); - } - }; + exact("label2").buildRepeatable(cd -> getLabels(cd, true)); - private static Iterable getLabels(ChangeData input, boolean owners) throws OrmException { + private static Iterable getLabels(ChangeData cd, boolean owners) throws OrmException { Set allApprovals = new HashSet<>(); Set distinctApprovals = new HashSet<>(); - for (PatchSetApproval a : input.currentApprovals()) { + for (PatchSetApproval a : cd.currentApprovals()) { if (a.getValue() != 0 && !a.isLegacySubmit()) { allApprovals.add(formatLabel(a.getLabel(), a.getValue(), a.getAccountId())); - if (owners && input.change().getOwner().equals(a.getAccountId())) { + if (owners && cd.change().getOwner().equals(a.getAccountId())) { allApprovals.add( formatLabel(a.getLabel(), a.getValue(), ChangeQueryBuilder.OWNER_ACCOUNT_ID)); } @@ -419,20 +276,12 @@ public class ChangeField { return allApprovals; } - public static Set getAuthorParts(ChangeData cd) throws OrmException { - try { - return SchemaUtil.getPersonParts(cd.getAuthor()); - } catch (IOException e) { - throw new OrmException(e); - } + public static Set getAuthorParts(ChangeData cd) throws OrmException, IOException { + return SchemaUtil.getPersonParts(cd.getAuthor()); } - public static Set getCommitterParts(ChangeData cd) throws OrmException { - try { - return SchemaUtil.getPersonParts(cd.getCommitter()); - } catch (IOException e) { - throw new OrmException(e); - } + public static Set getCommitterParts(ChangeData cd) throws OrmException, IOException { + return SchemaUtil.getPersonParts(cd.getCommitter()); } /** @@ -440,63 +289,28 @@ public class ChangeField { * set. */ public static final FieldDef> AUTHOR = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_AUTHOR, FieldType.FULL_TEXT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return getAuthorParts(input); - } - }; + fullText(ChangeQueryBuilder.FIELD_AUTHOR).buildRepeatable(ChangeField::getAuthorParts); /** * The exact email address, or any part of the committer name or email address, in the current * patch set. */ public static final FieldDef> COMMITTER = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_COMMITTER, FieldType.FULL_TEXT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return getCommitterParts(input); - } - }; + fullText(ChangeQueryBuilder.FIELD_COMMITTER).buildRepeatable(ChangeField::getCommitterParts); - public static class ChangeProtoField extends FieldDef.Single { - public static final ProtobufCodec CODEC = CodecFactory.encoder(Change.class); - - private ChangeProtoField() { - super("_change", FieldType.STORED_ONLY, true); - } - - @Override - public byte[] get(ChangeData input, FieldDef.FillArgs args) throws OrmException { - Change c = input.change(); - if (c == null) { - return null; - } - return CODEC.encodeToByteArray(c); - } - } + public static final ProtobufCodec CHANGE_CODEC = CodecFactory.encoder(Change.class); /** Serialized change object, used for pre-populating results. */ - public static final ChangeProtoField CHANGE = new ChangeProtoField(); + public static final FieldDef CHANGE = + storedOnly("_change").build(changeGetter(CHANGE_CODEC::encodeToByteArray)); - public static class PatchSetApprovalProtoField extends FieldDef.Repeatable { - public static final ProtobufCodec CODEC = - CodecFactory.encoder(PatchSetApproval.class); - - private PatchSetApprovalProtoField() { - super("_approval", FieldType.STORED_ONLY, true); - } - - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return toProtos(CODEC, input.currentApprovals()); - } - } + public static final ProtobufCodec APPROVAL_CODEC = + CodecFactory.encoder(PatchSetApproval.class); /** Serialized approvals for the current patch set, used for pre-populating results. */ - public static final PatchSetApprovalProtoField APPROVAL = new PatchSetApprovalProtoField(); + public static final FieldDef> APPROVAL = + storedOnly("_approval") + .buildRepeatable(cd -> toProtos(APPROVAL_CODEC, cd.currentApprovals())); public static String formatLabel(String label, int value) { return formatLabel(label, value, null); @@ -518,181 +332,123 @@ public class ChangeField { /** Commit message of the current patch set. */ public static final FieldDef COMMIT_MESSAGE = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_MESSAGE, FieldType.FULL_TEXT, false) { - @Override - public String get(ChangeData input, FillArgs args) throws OrmException { - try { - return input.commitMessage(); - } catch (IOException e) { - throw new OrmException(e); - } - } - }; + fullText(ChangeQueryBuilder.FIELD_MESSAGE).build(ChangeData::commitMessage); /** Summary or inline comment. */ public static final FieldDef> COMMENT = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_COMMENT, FieldType.FULL_TEXT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - Set r = new HashSet<>(); - for (Comment c : input.publishedComments()) { - r.add(c.message); - } - for (ChangeMessage m : input.messages()) { - r.add(m.getMessage()); - } - return r; - } - }; + fullText(ChangeQueryBuilder.FIELD_COMMENT) + .buildRepeatable( + cd -> { + Set r = new HashSet<>(); + for (Comment c : cd.publishedComments()) { + r.add(c.message); + } + for (ChangeMessage m : cd.messages()) { + r.add(m.getMessage()); + } + return r; + }); /** Number of unresolved comments of the change. */ public static final FieldDef UNRESOLVED_COMMENT_COUNT = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_UNRESOLVED_COMMENT_COUNT, FieldType.INTEGER_RANGE, true) { - @Override - public Integer get(ChangeData input, FillArgs args) throws OrmException { - return input.unresolvedCommentCount(); - } - }; + intRange(ChangeQueryBuilder.FIELD_UNRESOLVED_COMMENT_COUNT) + .stored() + .build(ChangeData::unresolvedCommentCount); /** Whether the change is mergeable. */ public static final FieldDef MERGEABLE = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_MERGEABLE, FieldType.EXACT, true) { - @Override - public String get(ChangeData input, FillArgs args) throws OrmException { - Boolean m = input.isMergeable(); - if (m == null) { - return null; - } - return m ? "1" : "0"; - } - }; + exact(ChangeQueryBuilder.FIELD_MERGEABLE) + .stored() + .build( + cd -> { + Boolean m = cd.isMergeable(); + if (m == null) { + return null; + } + return m ? "1" : "0"; + }); /** The number of inserted lines in this change. */ public static final FieldDef ADDED = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_ADDED, FieldType.INTEGER_RANGE, true) { - @Override - public Integer get(ChangeData input, FillArgs args) throws OrmException { - return input.changedLines().isPresent() ? input.changedLines().get().insertions : null; - } - }; + intRange(ChangeQueryBuilder.FIELD_ADDED) + .stored() + .build(cd -> cd.changedLines().isPresent() ? cd.changedLines().get().insertions : null); /** The number of deleted lines in this change. */ public static final FieldDef DELETED = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_DELETED, FieldType.INTEGER_RANGE, true) { - @Override - public Integer get(ChangeData input, FillArgs args) throws OrmException { - return input.changedLines().isPresent() ? input.changedLines().get().deletions : null; - } - }; + intRange(ChangeQueryBuilder.FIELD_DELETED) + .stored() + .build(cd -> cd.changedLines().isPresent() ? cd.changedLines().get().deletions : null); /** The total number of modified lines in this change. */ public static final FieldDef DELTA = - new FieldDef.Single( - ChangeQueryBuilder.FIELD_DELTA, FieldType.INTEGER_RANGE, false) { - @Override - public Integer get(ChangeData input, FillArgs args) throws OrmException { - return input.changedLines().map(c -> c.insertions + c.deletions).orElse(null); - } - }; + intRange(ChangeQueryBuilder.FIELD_DELTA) + .build(cd -> cd.changedLines().map(c -> c.insertions + c.deletions).orElse(null)); /** Users who have commented on this change. */ public static final FieldDef> COMMENTBY = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_COMMENTBY, FieldType.INTEGER, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - Set r = new HashSet<>(); - for (ChangeMessage m : input.messages()) { - if (m.getAuthor() != null) { - r.add(m.getAuthor().get()); - } - } - for (Comment c : input.publishedComments()) { - r.add(c.author.getId().get()); - } - return r; - } - }; + integer(ChangeQueryBuilder.FIELD_COMMENTBY) + .buildRepeatable( + cd -> { + Set r = new HashSet<>(); + for (ChangeMessage m : cd.messages()) { + if (m.getAuthor() != null) { + r.add(m.getAuthor().get()); + } + } + for (Comment c : cd.publishedComments()) { + r.add(c.author.getId().get()); + } + return r; + }); /** Star labels on this change in the format: <account-id>:<label> */ public static final FieldDef> STAR = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_STAR, FieldType.EXACT, true) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return Iterables.transform( - input.stars().entries(), - (Map.Entry e) -> { - return StarredChangesUtil.StarField.create(e.getKey(), e.getValue()).toString(); - }); - } - }; + exact(ChangeQueryBuilder.FIELD_STAR) + .stored() + .buildRepeatable( + cd -> + Iterables.transform( + cd.stars().entries(), + e -> + StarredChangesUtil.StarField.create(e.getKey(), e.getValue()) + .toString())); /** Users that have starred the change with any label. */ public static final FieldDef> STARBY = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_STARBY, FieldType.INTEGER, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return Iterables.transform(input.stars().keySet(), Account.Id::get); - } - }; + integer(ChangeQueryBuilder.FIELD_STARBY) + .buildRepeatable(cd -> Iterables.transform(cd.stars().keySet(), Account.Id::get)); /** Opaque group identifiers for this change's patch sets. */ public static final FieldDef> GROUP = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_GROUP, FieldType.EXACT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - Set r = Sets.newHashSetWithExpectedSize(1); - for (PatchSet ps : input.patchSets()) { - r.addAll(ps.getGroups()); - } - return r; - } - }; + exact(ChangeQueryBuilder.FIELD_GROUP) + .buildRepeatable( + cd -> { + Set r = Sets.newHashSetWithExpectedSize(1); + for (PatchSet ps : cd.patchSets()) { + r.addAll(ps.getGroups()); + } + return r; + }); - public static class PatchSetProtoField extends FieldDef.Repeatable { - public static final ProtobufCodec CODEC = CodecFactory.encoder(PatchSet.class); - - private PatchSetProtoField() { - super("_patch_set", FieldType.STORED_ONLY, true); - } - - @Override - public Iterable get(ChangeData input, FieldDef.FillArgs args) throws OrmException { - return toProtos(CODEC, input.patchSets()); - } - } + public static final ProtobufCodec PATCH_SET_CODEC = + CodecFactory.encoder(PatchSet.class); /** Serialized patch set object, used for pre-populating results. */ - public static final PatchSetProtoField PATCH_SET = new PatchSetProtoField(); + public static final FieldDef> PATCH_SET = + storedOnly("_patch_set").buildRepeatable(cd -> toProtos(PATCH_SET_CODEC, cd.patchSets())); /** Users who have edits on this change. */ public static final FieldDef> EDITBY = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_EDITBY, FieldType.INTEGER, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return input.editsByUser().stream().map(Account.Id::get).collect(toSet()); - } - }; + integer(ChangeQueryBuilder.FIELD_EDITBY) + .buildRepeatable(cd -> cd.editsByUser().stream().map(Account.Id::get).collect(toSet())); /** Users who have draft comments on this change. */ public static final FieldDef> DRAFTBY = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_DRAFTBY, FieldType.INTEGER, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return input.draftsByUser().stream().map(Account.Id::get).collect(toSet()); - } - }; + integer(ChangeQueryBuilder.FIELD_DRAFTBY) + .buildRepeatable(cd -> cd.draftsByUser().stream().map(Account.Id::get).collect(toSet())); + + public static final Integer NOT_REVIEWED = -1; /** * Users the change was reviewed by since the last author update. @@ -705,21 +461,20 @@ public class ChangeField { * emitted. */ public static final FieldDef> REVIEWEDBY = - new FieldDef.Repeatable( - ChangeQueryBuilder.FIELD_REVIEWEDBY, FieldType.INTEGER, true) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - Set reviewedBy = input.reviewedBy(); - if (reviewedBy.isEmpty()) { - return ImmutableSet.of(NOT_REVIEWED); - } - List result = new ArrayList<>(reviewedBy.size()); - for (Account.Id id : reviewedBy) { - result.add(id.get()); - } - return result; - } - }; + integer(ChangeQueryBuilder.FIELD_REVIEWEDBY) + .stored() + .buildRepeatable( + cd -> { + Set reviewedBy = cd.reviewedBy(); + if (reviewedBy.isEmpty()) { + return ImmutableSet.of(NOT_REVIEWED); + } + List result = new ArrayList<>(reviewedBy.size()); + for (Account.Id id : reviewedBy) { + result.add(id.get()); + } + return result; + }); // Submit rule options in this class should never use fastEvalLabels. This // slows down indexing slightly but produces correct search results. @@ -780,30 +535,15 @@ public class ChangeField { } public static final FieldDef> SUBMIT_RECORD = - new FieldDef.Repeatable("submit_record", FieldType.EXACT, false) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return formatSubmitRecordValues(input); - } - }; + exact("submit_record").buildRepeatable(cd -> formatSubmitRecordValues(cd)); public static final FieldDef> STORED_SUBMIT_RECORD_STRICT = - new FieldDef.Repeatable( - "full_submit_record_strict", FieldType.STORED_ONLY, true) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return storedSubmitRecords(input, SUBMIT_RULE_OPTIONS_STRICT); - } - }; + storedOnly("full_submit_record_strict") + .buildRepeatable(cd -> storedSubmitRecords(cd, SUBMIT_RULE_OPTIONS_STRICT)); public static final FieldDef> STORED_SUBMIT_RECORD_LENIENT = - new FieldDef.Repeatable( - "full_submit_record_lenient", FieldType.STORED_ONLY, true) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - return storedSubmitRecords(input, SUBMIT_RULE_OPTIONS_LENIENT); - } - }; + storedOnly("full_submit_record_lenient") + .buildRepeatable(cd -> storedSubmitRecords(cd, SUBMIT_RULE_OPTIONS_LENIENT)); public static void parseSubmitRecords( Collection values, SubmitRuleOptions opts, ChangeData out) { @@ -873,35 +613,35 @@ public class ChangeField { *

Emitted as UTF-8 encoded strings of the form {@code project:ref/name:[hex sha]}. */ public static final FieldDef> REF_STATE = - new FieldDef.Repeatable("ref_state", FieldType.STORED_ONLY, true) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - List result = new ArrayList<>(); - Project.NameKey project = input.change().getProject(); + storedOnly("ref_state") + .buildRepeatable( + (cd, a) -> { + List result = new ArrayList<>(); + Project.NameKey project = cd.change().getProject(); - input.editRefs().values().forEach(r -> result.add(RefState.of(r).toByteArray(project))); - input - .starRefs() - .values() - .forEach(r -> result.add(RefState.of(r.ref()).toByteArray(args.allUsers))); + cd.editRefs() + .values() + .forEach(r -> result.add(RefState.of(r).toByteArray(project))); + cd.starRefs() + .values() + .forEach(r -> result.add(RefState.of(r.ref()).toByteArray(a.allUsers))); - if (PrimaryStorage.of(input.change()) == PrimaryStorage.NOTE_DB) { - ChangeNotes notes = input.notes(); - result.add(RefState.create(notes.getRefName(), notes.getMetaId()).toByteArray(project)); - notes.getRobotComments(); // Force loading robot comments. - RobotCommentNotes robotNotes = notes.getRobotCommentNotes(); - result.add( - RefState.create(robotNotes.getRefName(), robotNotes.getMetaId()) - .toByteArray(project)); - input - .draftRefs() - .values() - .forEach(r -> result.add(RefState.of(r).toByteArray(args.allUsers))); - } + if (PrimaryStorage.of(cd.change()) == PrimaryStorage.NOTE_DB) { + ChangeNotes notes = cd.notes(); + result.add( + RefState.create(notes.getRefName(), notes.getMetaId()).toByteArray(project)); + notes.getRobotComments(); // Force loading robot comments. + RobotCommentNotes robotNotes = notes.getRobotCommentNotes(); + result.add( + RefState.create(robotNotes.getRefName(), robotNotes.getMetaId()) + .toByteArray(project)); + cd.draftRefs() + .values() + .forEach(r -> result.add(RefState.of(r).toByteArray(a.allUsers))); + } - return result; - } - }; + return result; + }); /** * All ref wildcard patterns that were used in the course of indexing this document. @@ -910,32 +650,29 @@ public class ChangeField { * RefStatePattern} for the pattern format. */ public static final FieldDef> REF_STATE_PATTERN = - new FieldDef.Repeatable( - "ref_state_pattern", FieldType.STORED_ONLY, true) { - @Override - public Iterable get(ChangeData input, FillArgs args) throws OrmException { - Change.Id id = input.getId(); - Project.NameKey project = input.change().getProject(); - List result = new ArrayList<>(3); - result.add( - RefStatePattern.create(RefNames.REFS_USERS + "*/" + RefNames.EDIT_PREFIX + id + "/*") - .toByteArray(project)); - result.add( - RefStatePattern.create(RefNames.refsStarredChangesPrefix(id) + "*") - .toByteArray(args.allUsers)); - if (PrimaryStorage.of(input.change()) == PrimaryStorage.NOTE_DB) { - result.add( - RefStatePattern.create(RefNames.refsDraftCommentsPrefix(id) + "*") - .toByteArray(args.allUsers)); - } - return result; - } - }; + storedOnly("ref_state_pattern") + .buildRepeatable( + (cd, a) -> { + Change.Id id = cd.getId(); + Project.NameKey project = cd.change().getProject(); + List result = new ArrayList<>(3); + result.add( + RefStatePattern.create( + RefNames.REFS_USERS + "*/" + RefNames.EDIT_PREFIX + id + "/*") + .toByteArray(project)); + result.add( + RefStatePattern.create(RefNames.refsStarredChangesPrefix(id) + "*") + .toByteArray(a.allUsers)); + if (PrimaryStorage.of(cd.change()) == PrimaryStorage.NOTE_DB) { + result.add( + RefStatePattern.create(RefNames.refsDraftCommentsPrefix(id) + "*") + .toByteArray(a.allUsers)); + } + return result; + }); - public static final Integer NOT_REVIEWED = -1; - - private static String getTopic(ChangeData input) throws OrmException { - Change c = input.change(); + private static String getTopic(ChangeData cd) throws OrmException { + Change c = cd.change(); if (c == null) { return null; } @@ -959,4 +696,8 @@ public class ChangeField { } return result; } + + private static FieldDef.Getter changeGetter(Function func) { + return in -> in.change() != null ? func.apply(in.change()) : null; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/group/GroupField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/group/GroupField.java index cc07dfd5f4..5e72327487 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/group/GroupField.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/group/GroupField.java @@ -14,74 +14,42 @@ package com.google.gerrit.server.index.group; +import static com.google.gerrit.server.index.FieldDef.exact; +import static com.google.gerrit.server.index.FieldDef.fullText; +import static com.google.gerrit.server.index.FieldDef.integer; +import static com.google.gerrit.server.index.FieldDef.prefix; + import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.index.FieldDef; -import com.google.gerrit.server.index.FieldType; import com.google.gerrit.server.index.SchemaUtil; -import com.google.gwtorm.server.OrmException; /** Secondary index schemas for groups. */ public class GroupField { /** Legacy group ID. */ public static final FieldDef ID = - new FieldDef.Single("id", FieldType.INTEGER, false) { - @Override - public Integer get(AccountGroup input, FillArgs args) { - return input.getId().get(); - } - }; + integer("id").build(g -> g.getId().get()); /** Group UUID. */ public static final FieldDef UUID = - new FieldDef.Single("uuid", FieldType.EXACT, true) { - @Override - public String get(AccountGroup input, FillArgs args) { - return input.getGroupUUID().get(); - } - }; + exact("uuid").stored().build(g -> g.getGroupUUID().get()); /** Group owner UUID. */ public static final FieldDef OWNER_UUID = - new FieldDef.Single("owner_uuid", FieldType.EXACT, false) { - @Override - public String get(AccountGroup input, FillArgs args) { - return input.getOwnerGroupUUID().get(); - } - }; + exact("owner_uuid").build(g -> g.getOwnerGroupUUID().get()); /** Group name. */ public static final FieldDef NAME = - new FieldDef.Single("name", FieldType.EXACT, false) { - @Override - public String get(AccountGroup input, FillArgs args) { - return input.getName(); - } - }; + exact("name").build(AccountGroup::getName); /** Prefix match on group name parts. */ public static final FieldDef> NAME_PART = - new FieldDef.Repeatable("name_part", FieldType.PREFIX, false) { - @Override - public Iterable get(AccountGroup input, FillArgs args) { - return SchemaUtil.getNameParts(input.getName()); - } - }; + prefix("name_part").buildRepeatable(g -> SchemaUtil.getNameParts(g.getName())); /** Group description. */ public static final FieldDef DESCRIPTION = - new FieldDef.Single("description", FieldType.FULL_TEXT, false) { - @Override - public String get(AccountGroup input, FillArgs args) { - return input.getDescription(); - } - }; + fullText("description").build(AccountGroup::getDescription); /** Whether the group is visible to all users. */ public static final FieldDef IS_VISIBLE_TO_ALL = - new FieldDef.Single("is_visible_to_all", FieldType.EXACT, false) { - @Override - public String get(AccountGroup input, FillArgs args) throws OrmException { - return input.isVisibleToAll() ? "1" : "0"; - } - }; + exact("is_visible_to_all").build(g -> g.isVisibleToAll() ? "1" : "0"); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AuthorPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AuthorPredicate.java index 6a760db413..dccd17e51d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AuthorPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AuthorPredicate.java @@ -19,6 +19,7 @@ import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_AUT import com.google.gerrit.server.index.change.ChangeField; import com.google.gwtorm.server.OrmException; +import java.io.IOException; public class AuthorPredicate extends ChangeIndexPredicate { AuthorPredicate(String value) { @@ -27,7 +28,11 @@ public class AuthorPredicate extends ChangeIndexPredicate { @Override public boolean match(ChangeData object) throws OrmException { - return ChangeField.getAuthorParts(object).contains(getValue().toLowerCase()); + try { + return ChangeField.getAuthorParts(object).contains(getValue().toLowerCase()); + } catch (IOException e) { + throw new OrmException(e); + } } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommitterPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommitterPredicate.java index 8e13e202f0..cd1f3b2f61 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommitterPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommitterPredicate.java @@ -19,6 +19,7 @@ import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_COM import com.google.gerrit.server.index.change.ChangeField; import com.google.gwtorm.server.OrmException; +import java.io.IOException; public class CommitterPredicate extends ChangeIndexPredicate { CommitterPredicate(String value) { @@ -27,7 +28,11 @@ public class CommitterPredicate extends ChangeIndexPredicate { @Override public boolean match(ChangeData object) throws OrmException { - return ChangeField.getCommitterParts(object).contains(getValue().toLowerCase()); + try { + return ChangeField.getCommitterParts(object).contains(getValue().toLowerCase()); + } catch (IOException e) { + throw new OrmException(e); + } } @Override