From 5cc3023b606f8ee05ec31be590ca1f085a10c66d Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 28 Jul 2015 10:50:19 +0900 Subject: [PATCH] ChangeField: Add LEGACY_ID2 The LEGACY_ID field is named "_id" which cannot be used in Elasticsearch because that field name is reserved. Deprecate LEGACY_ID and add a new field LEGACY_ID2 which is named "legacy_id" instead. Add a new schema version using LEGACY_ID2 and deprecate the previous version. Change-Id: I0b89365b6af74360aeece00c61d29416951fecb4 --- .../gerrit/lucene/LuceneChangeIndex.java | 23 +++++++----- .../google/gerrit/lucene/QueryBuilder.java | 12 +++---- .../gerrit/server/index/ChangeField.java | 11 ++++++ .../gerrit/server/index/ChangeSchemas.java | 36 ++++++++++++++++++- .../query/change/ChangeQueryBuilder.java | 3 +- .../server/query/change/CommentPredicate.java | 4 +-- .../query/change/ConflictsPredicate.java | 2 +- .../query/change/FuzzyTopicPredicate.java | 6 ++-- .../query/change/IsStarredByPredicate.java | 8 +++-- .../query/change/LegacyChangeIdPredicate.java | 22 ++++++++++-- .../server/query/change/MessagePredicate.java | 4 +-- 11 files changed, 101 insertions(+), 30 deletions(-) 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 53d6713177..55ed22d388 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 @@ -54,6 +54,7 @@ import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeDataSource; +import com.google.gerrit.server.query.change.LegacyChangeIdPredicate; import com.google.gwtorm.protobuf.ProtobufCodec; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; @@ -129,9 +130,7 @@ public class LuceneChangeIndex implements ChangeIndex { private static final String APPROVAL_FIELD = ChangeField.APPROVAL.getName(); private static final String CHANGE_FIELD = ChangeField.CHANGE.getName(); private static final String DELETED_FIELD = ChangeField.DELETED.getName(); - private static final String ID_FIELD = ChangeField.LEGACY_ID.getName(); - private static final String ID_SORT_FIELD = - sortFieldName(ChangeField.LEGACY_ID); + private static final String ID_FIELD = ChangeField.LEGACY_ID2.getName(); private static final String MERGEABLE_FIELD = ChangeField.MERGEABLE.getName(); private static final String PATCH_SET_FIELD = ChangeField.PATCH_SET.getName(); private static final String REVIEWEDBY_FIELD = @@ -211,6 +210,7 @@ public class LuceneChangeIndex implements ChangeIndex { private final QueryBuilder queryBuilder; private final SubIndex openIndex; private final SubIndex closedIndex; + private final String idSortField; /** * Whether to use DocValues for range/sorted numeric fields. @@ -240,6 +240,7 @@ public class LuceneChangeIndex implements ChangeIndex { this.changeDataFactory = changeDataFactory; this.schema = schema; this.useDocValuesForSorting = schema.getVersion() >= 15; + this.idSortField = sortFieldName(LegacyChangeIdPredicate.idField(schema)); CustomMappingAnalyzer analyzer = new CustomMappingAnalyzer(new StandardAnalyzer(CharArraySet.EMPTY_SET), @@ -274,6 +275,7 @@ public class LuceneChangeIndex implements ChangeIndex { if (useDocValuesForSorting) { return new SearcherFactory(); } + @SuppressWarnings("deprecation") final Map mapping = ImmutableMap.of( ChangeField.LEGACY_ID.getName(), UninvertingReader.Type.INTEGER, ChangeField.UPDATED.getName(), UninvertingReader.Type.LONG); @@ -314,7 +316,7 @@ public class LuceneChangeIndex implements ChangeIndex { @Override public void replace(ChangeData cd) throws IOException { - Term id = QueryBuilder.idTerm(cd); + Term id = QueryBuilder.idTerm(schema, cd); Document doc = toDocument(cd); try { if (cd.change().getStatus().isOpen()) { @@ -333,7 +335,7 @@ public class LuceneChangeIndex implements ChangeIndex { @Override public void delete(Change.Id id) throws IOException { - Term idTerm = QueryBuilder.idTerm(id); + Term idTerm = QueryBuilder.idTerm(schema, id); try { Futures.allAsList( openIndex.delete(idTerm), @@ -369,11 +371,12 @@ public class LuceneChangeIndex implements ChangeIndex { setReady(sitePaths, schema.getVersion(), ready); } + @SuppressWarnings("deprecation") private Sort getSort() { if (useDocValuesForSorting) { return new Sort( new SortField(UPDATED_SORT_FIELD, SortField.Type.LONG, true), - new SortField(ID_SORT_FIELD, SortField.Type.LONG, true)); + new SortField(idSortField, SortField.Type.LONG, true)); } else { return new Sort( new SortField( @@ -550,16 +553,18 @@ public class LuceneChangeIndex implements ChangeIndex { return result; } + @SuppressWarnings("deprecation") private void add(Document doc, Values values) { String name = values.getField().getName(); FieldType type = values.getField().getType(); Store store = store(values.getField()); if (useDocValuesForSorting) { - if (values.getField() == ChangeField.LEGACY_ID) { + FieldDef f = values.getField(); + if (f == ChangeField.LEGACY_ID || f == ChangeField.LEGACY_ID2) { int v = (Integer) getOnlyElement(values.getValues()); - doc.add(new NumericDocValuesField(ID_SORT_FIELD, v)); - } else if (values.getField() == ChangeField.UPDATED) { + doc.add(new NumericDocValuesField(sortFieldName(f), v)); + } else if (f == ChangeField.UPDATED) { long t = ((Timestamp) getOnlyElement(values.getValues())).getTime(); doc.add(new NumericDocValuesField(UPDATED_SORT_FIELD, t)); } diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java index 39a9e10cf2..7fd98aa84f 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java @@ -14,17 +14,18 @@ package com.google.gerrit.lucene; +import static com.google.gerrit.server.query.change.LegacyChangeIdPredicate.idField; import static org.apache.lucene.search.BooleanClause.Occur.MUST; import static org.apache.lucene.search.BooleanClause.Occur.MUST_NOT; import static org.apache.lucene.search.BooleanClause.Occur.SHOULD; import com.google.common.collect.Lists; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.server.index.ChangeField; import com.google.gerrit.server.index.FieldType; import com.google.gerrit.server.index.IndexPredicate; import com.google.gerrit.server.index.IntegerRangePredicate; import com.google.gerrit.server.index.RegexPredicate; +import com.google.gerrit.server.index.Schema; import com.google.gerrit.server.index.TimestampRangePredicate; import com.google.gerrit.server.query.AndPredicate; import com.google.gerrit.server.query.NotPredicate; @@ -49,14 +50,13 @@ import java.util.Date; import java.util.List; public class QueryBuilder { - private static final String ID_FIELD = ChangeField.LEGACY_ID.getName(); - public static Term idTerm(ChangeData cd) { - return intTerm(ID_FIELD, cd.getId().get()); + public static Term idTerm(Schema schema, ChangeData cd) { + return intTerm(idField(schema).getName(), cd.getId().get()); } - public static Term idTerm(Change.Id id) { - return intTerm(ID_FIELD, id.get()); + public static Term idTerm(Schema schema, Change.Id id) { + return intTerm(idField(schema).getName(), id.get()); } private final org.apache.lucene.util.QueryBuilder queryBuilder; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java index daf0c105d7..5d7229a514 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java @@ -62,6 +62,7 @@ import java.util.Set; * characters. */ public class ChangeField { + @Deprecated /** Legacy change ID. */ public static final FieldDef LEGACY_ID = new FieldDef.Single("_id", @@ -72,6 +73,16 @@ public class ChangeField { } }; + /** Legacy change ID without underscore prefix. */ + public static final FieldDef LEGACY_ID2 = + new FieldDef.Single("legacy_id", + FieldType.INTEGER, true) { + @Override + public Integer get(ChangeData input, FillArgs args) { + return input.getId().get(); + } + }; + /** Newer style Change-Id key. */ public static final FieldDef ID = new FieldDef.Single("change_id", diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java index 2e858470d5..4915a663c7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java @@ -241,7 +241,7 @@ public class ChangeSchemas { ChangeField.EDITBY, ChangeField.REVIEWEDBY); - @Deprecated + @SuppressWarnings("deprecation") static final Schema V21 = schema( ChangeField.LEGACY_ID, ChangeField.ID, @@ -274,6 +274,7 @@ public class ChangeSchemas { ChangeField.EDITBY, ChangeField.REVIEWEDBY); + @Deprecated static final Schema V22 = schema( ChangeField.LEGACY_ID, ChangeField.ID, @@ -307,6 +308,39 @@ public class ChangeSchemas { ChangeField.REVIEWEDBY, ChangeField.EXACT_COMMIT); + static final Schema V23 = schema( + ChangeField.LEGACY_ID2, + ChangeField.ID, + ChangeField.STATUS, + ChangeField.PROJECT, + ChangeField.PROJECTS, + ChangeField.REF, + ChangeField.EXACT_TOPIC, + ChangeField.FUZZY_TOPIC, + ChangeField.UPDATED, + ChangeField.FILE_PART, + ChangeField.PATH, + ChangeField.OWNER, + ChangeField.REVIEWER, + ChangeField.COMMIT, + ChangeField.TR, + ChangeField.LABEL, + ChangeField.COMMIT_MESSAGE, + ChangeField.COMMENT, + ChangeField.CHANGE, + ChangeField.APPROVAL, + ChangeField.MERGEABLE, + ChangeField.ADDED, + ChangeField.DELETED, + ChangeField.DELTA, + ChangeField.HASHTAG, + ChangeField.COMMENTBY, + ChangeField.PATCH_SET, + ChangeField.GROUP, + ChangeField.EDITBY, + ChangeField.REVIEWEDBY, + ChangeField.EXACT_COMMIT); + private static Schema schema(Collection> fields) { return new Schema<>(ImmutableList.copyOf(fields)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index 5210577735..b9dab9e4d1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -341,7 +341,8 @@ public class ChangeQueryBuilder extends QueryBuilder { @Operator public Predicate change(String query) throws QueryParseException { if (PAT_LEGACY_ID.matcher(query).matches()) { - return new LegacyChangeIdPredicate(Change.Id.parse(query)); + return new LegacyChangeIdPredicate( + args.getSchema(), Change.Id.parse(query)); } else if (PAT_CHANGE_ID.matcher(query).matches()) { return new ChangeIdPredicate(parseChangeId(query)); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommentPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommentPredicate.java index 3983f62393..e829e53f31 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommentPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommentPredicate.java @@ -33,8 +33,8 @@ class CommentPredicate extends IndexPredicate { public boolean match(ChangeData object) throws OrmException { try { for (ChangeData cData : index.getSource( - Predicate.and(new LegacyChangeIdPredicate(object.getId()), this), 0, 1) - .read()) { + Predicate.and(new LegacyChangeIdPredicate( + index.getSchema(), object.getId()), this), 0, 1).read()) { if (cData.getId().equals(object.getId())) { return true; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java index e63b05cf05..8bea099a04 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java @@ -76,7 +76,7 @@ class ConflictsPredicate extends OrPredicate { List> predicatesForOneChange = Lists.newArrayListWithCapacity(5); predicatesForOneChange.add( - not(new LegacyChangeIdPredicate(c.getId()))); + not(new LegacyChangeIdPredicate(args.getSchema(), c.getId()))); predicatesForOneChange.add( new ProjectPredicate(c.getProject().get())); predicatesForOneChange.add( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java index f65b3b8773..565515420c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java @@ -55,8 +55,10 @@ class FuzzyTopicPredicate extends IndexPredicate { } if (getField() == FUZZY_TOPIC || getField() == LEGACY_TOPIC3) { try { - Predicate thisId = new LegacyChangeIdPredicate(cd.getId()); - Iterable results = index.getSource(and(thisId, this), 0, 1).read(); + Predicate thisId = + new LegacyChangeIdPredicate(index.getSchema(), cd.getId()); + Iterable results = + index.getSource(and(thisId, this), 0, 1).read(); return !Iterables.isEmpty(results); } catch (QueryParseException e) { throw new OrmException(e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java index b5bef0735d..b990091bd6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java @@ -18,6 +18,7 @@ import com.google.common.collect.Lists; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.index.Schema; import com.google.gerrit.server.query.OrPredicate; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; @@ -37,10 +38,11 @@ class IsStarredByPredicate extends OrPredicate implements return user.toString(); } - private static List> predicates(Set ids) { + private static List> predicates( + Schema schema, Set ids) { List> r = Lists.newArrayListWithCapacity(ids.size()); for (Change.Id id : ids) { - r.add(new LegacyChangeIdPredicate(id)); + r.add(new LegacyChangeIdPredicate(schema, id)); } return r; } @@ -53,7 +55,7 @@ class IsStarredByPredicate extends OrPredicate implements } private IsStarredByPredicate(Arguments args, IdentifiedUser user) { - super(predicates(user.getStarredChanges())); + super(predicates(args.getSchema(), user.getStarredChanges())); this.args = args; this.user = user; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LegacyChangeIdPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LegacyChangeIdPredicate.java index 9ecc7b2dbb..bf595533bd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LegacyChangeIdPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LegacyChangeIdPredicate.java @@ -14,16 +14,32 @@ package com.google.gerrit.server.query.change; +import static com.google.gerrit.server.index.ChangeField.LEGACY_ID; +import static com.google.gerrit.server.index.ChangeField.LEGACY_ID2; + import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.server.index.ChangeField; +import com.google.gerrit.server.index.FieldDef; import com.google.gerrit.server.index.IndexPredicate; +import com.google.gerrit.server.index.Schema; /** Predicate over change number (aka legacy ID or Change.Id). */ -class LegacyChangeIdPredicate extends IndexPredicate { +public class LegacyChangeIdPredicate extends IndexPredicate { private final Change.Id id; - LegacyChangeIdPredicate(Change.Id id) { - super(ChangeField.LEGACY_ID, ChangeQueryBuilder.FIELD_CHANGE, id.toString()); + @SuppressWarnings("deprecation") + public static FieldDef idField(Schema schema) { + if (schema == null) { + return ChangeField.LEGACY_ID2; + } else if (schema.hasField(LEGACY_ID2)) { + return schema.getFields().get(LEGACY_ID2.getName()); + } else { + return schema.getFields().get(LEGACY_ID.getName()); + } + } + + LegacyChangeIdPredicate(Schema schema, Change.Id id) { + super(idField(schema), ChangeQueryBuilder.FIELD_CHANGE, id.toString()); this.id = id; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/MessagePredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/MessagePredicate.java index 0a16d02766..72230f6f37 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/MessagePredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/MessagePredicate.java @@ -37,8 +37,8 @@ class MessagePredicate extends IndexPredicate { public boolean match(ChangeData object) throws OrmException { try { for (ChangeData cData : index.getSource( - Predicate.and(new LegacyChangeIdPredicate(object.getId()), this), 0, 1) - .read()) { + Predicate.and(new LegacyChangeIdPredicate( + index.getSchema(), object.getId()), this), 0, 1).read()) { if (cData.getId().equals(object.getId())) { return true; }