Merge "Remove gr-change-requirements"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 05e7341..da15775 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6641,7 +6641,8 @@
by one of the following REST endpoints: link:#create-change[Create
Change], link:#create-merge-patch-set-for-change[Create Merge Patch Set
For Change], link:#cherry-pick[Cherry Pick Revision],
-link:rest-api-project.html#cherry-pick-commit[Cherry Pick Commit]
+link:rest-api-project.html#cherry-pick-commit[Cherry Pick Commit],
+link:#rebase-change[Rebase Change]
|==================================
[[change-input]]
diff --git a/java/com/google/gerrit/index/Schema.java b/java/com/google/gerrit/index/Schema.java
index 91c3f70..4235821 100644
--- a/java/com/google/gerrit/index/Schema.java
+++ b/java/com/google/gerrit/index/Schema.java
@@ -36,7 +36,6 @@
public static class Builder<T> {
private final List<FieldDef<T, ?>> fields = new ArrayList<>();
- private boolean useLegacyNumericFields;
public Builder<T> add(Schema<T> schema) {
this.fields.addAll(schema.getFields().values());
@@ -55,13 +54,8 @@
return this;
}
- public Builder<T> legacyNumericFields(boolean useLegacyNumericFields) {
- this.useLegacyNumericFields = useLegacyNumericFields;
- return this;
- }
-
public Schema<T> build() {
- return new Schema<>(useLegacyNumericFields, ImmutableList.copyOf(fields));
+ return new Schema<>(ImmutableList.copyOf(fields));
}
}
@@ -90,15 +84,14 @@
private final ImmutableMap<String, FieldDef<T, ?>> fields;
private final ImmutableMap<String, FieldDef<T, ?>> storedFields;
- private final boolean useLegacyNumericFields;
private int version;
- public Schema(boolean useLegacyNumericFields, Iterable<FieldDef<T, ?>> fields) {
- this(0, useLegacyNumericFields, fields);
+ public Schema(Iterable<FieldDef<T, ?>> fields) {
+ this(0, fields);
}
- public Schema(int version, boolean useLegacyNumericFields, Iterable<FieldDef<T, ?>> fields) {
+ public Schema(int version, Iterable<FieldDef<T, ?>> fields) {
this.version = version;
ImmutableMap.Builder<String, FieldDef<T, ?>> b = ImmutableMap.builder();
ImmutableMap.Builder<String, FieldDef<T, ?>> sb = ImmutableMap.builder();
@@ -110,17 +103,12 @@
}
this.fields = b.build();
this.storedFields = sb.build();
- this.useLegacyNumericFields = useLegacyNumericFields;
}
public final int getVersion() {
return version;
}
- public final boolean useLegacyNumericFields() {
- return useLegacyNumericFields;
- }
-
/**
* Get all fields in this schema.
*
diff --git a/java/com/google/gerrit/index/SchemaUtil.java b/java/com/google/gerrit/index/SchemaUtil.java
index 9599d6a..96fe4fc 100644
--- a/java/com/google/gerrit/index/SchemaUtil.java
+++ b/java/com/google/gerrit/index/SchemaUtil.java
@@ -67,30 +67,23 @@
}
public static <V> Schema<V> schema(Collection<FieldDef<V, ?>> fields) {
- return new Schema<>(true, ImmutableList.copyOf(fields));
+ return new Schema<>(ImmutableList.copyOf(fields));
}
- public static <V> Schema<V> schema(Schema<V> schema, boolean useLegacyNumericFields) {
- return new Schema<>(
- useLegacyNumericFields,
- new ImmutableList.Builder<FieldDef<V, ?>>().addAll(schema.getFields().values()).build());
+ @SafeVarargs
+ public static <V> Schema<V> schema(FieldDef<V, ?>... fields) {
+ return schema(ImmutableList.copyOf(fields));
}
@SafeVarargs
public static <V> Schema<V> schema(Schema<V> schema, FieldDef<V, ?>... moreFields) {
return new Schema<>(
- true,
new ImmutableList.Builder<FieldDef<V, ?>>()
.addAll(schema.getFields().values())
.addAll(ImmutableList.copyOf(moreFields))
.build());
}
- @SafeVarargs
- public static <V> Schema<V> schema(FieldDef<V, ?>... fields) {
- return new Schema<>(true, ImmutableList.copyOf(fields));
- }
-
public static Set<String> getPersonParts(PersonIdent person) {
if (person == null) {
return ImmutableSet.of();
diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
index 988d6fb..f8256bb 100644
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -66,8 +66,6 @@
import org.apache.lucene.document.Field;
import org.apache.lucene.document.Field.Store;
import org.apache.lucene.document.IntPoint;
-import org.apache.lucene.document.LegacyIntField;
-import org.apache.lucene.document.LegacyLongField;
import org.apache.lucene.document.LongPoint;
import org.apache.lucene.document.StoredField;
import org.apache.lucene.document.StringField;
@@ -88,7 +86,6 @@
import org.apache.lucene.store.Directory;
/** Basic Lucene index implementation. */
-@SuppressWarnings("deprecation")
public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -348,13 +345,9 @@
if (type == FieldType.INTEGER || type == FieldType.INTEGER_RANGE) {
for (Object value : values.getValues()) {
Integer intValue = (Integer) value;
- if (schema.useLegacyNumericFields()) {
- doc.add(new LegacyIntField(name, intValue, store));
- } else {
- doc.add(new IntPoint(name, intValue));
- if (store == Store.YES) {
- doc.add(new StoredField(name, intValue));
- }
+ doc.add(new IntPoint(name, intValue));
+ if (store == Store.YES) {
+ doc.add(new StoredField(name, intValue));
}
}
} else if (type == FieldType.LONG) {
@@ -383,13 +376,9 @@
}
private void addLongField(Document doc, String name, Store store, Long longValue) {
- if (schema.useLegacyNumericFields()) {
- doc.add(new LegacyLongField(name, longValue, store));
- } else {
- doc.add(new LongPoint(name, longValue));
- if (store == Store.YES) {
- doc.add(new StoredField(name, longValue));
- }
+ doc.add(new LongPoint(name, longValue));
+ if (store == Store.YES) {
+ doc.add(new StoredField(name, longValue));
}
}
diff --git a/java/com/google/gerrit/lucene/ChangeSubIndex.java b/java/com/google/gerrit/lucene/ChangeSubIndex.java
index 475dac4..661c0b0 100644
--- a/java/com/google/gerrit/lucene/ChangeSubIndex.java
+++ b/java/com/google/gerrit/lucene/ChangeSubIndex.java
@@ -15,8 +15,7 @@
package com.google.gerrit.lucene;
import static com.google.common.collect.Iterables.getOnlyElement;
-import static com.google.gerrit.lucene.LuceneChangeIndex.ID2_SORT_FIELD;
-import static com.google.gerrit.lucene.LuceneChangeIndex.ID_SORT_FIELD;
+import static com.google.gerrit.lucene.LuceneChangeIndex.ID_STR_SORT_FIELD;
import static com.google.gerrit.lucene.LuceneChangeIndex.MERGED_ON_SORT_FIELD;
import static com.google.gerrit.lucene.LuceneChangeIndex.UPDATED_SORT_FIELD;
import static com.google.gerrit.server.index.change.ChangeSchemaDefinitions.NAME;
@@ -120,12 +119,9 @@
void add(Document doc, Values<ChangeData> values) {
// Add separate DocValues fields for those fields needed for sorting.
FieldDef<ChangeData, ?> f = values.getField();
- if (f == ChangeField.LEGACY_ID) {
- int v = (Integer) getOnlyElement(values.getValues());
- doc.add(new NumericDocValuesField(ID_SORT_FIELD, v));
- } else if (f == ChangeField.LEGACY_ID_STR) {
+ if (f == ChangeField.LEGACY_ID_STR) {
String v = (String) getOnlyElement(values.getValues());
- doc.add(new NumericDocValuesField(ID2_SORT_FIELD, Integer.valueOf(v)));
+ doc.add(new NumericDocValuesField(ID_STR_SORT_FIELD, Integer.valueOf(v)));
} else if (f == ChangeField.UPDATED) {
long t = ((Timestamp) getOnlyElement(values.getValues())).getTime();
doc.add(new NumericDocValuesField(UPDATED_SORT_FIELD, t));
diff --git a/java/com/google/gerrit/lucene/LuceneAccountIndex.java b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
index c4a5240..934b27f 100644
--- a/java/com/google/gerrit/lucene/LuceneAccountIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
@@ -137,7 +137,7 @@
@Override
public void replace(AccountState as) {
try {
- replace(idTerm(getSchema().useLegacyNumericFields(), as), toDocument(as)).get();
+ replace(idTerm(getSchema().hasField(ID), as), toDocument(as)).get();
} catch (ExecutionException | InterruptedException e) {
throw new StorageException(e);
}
@@ -155,7 +155,7 @@
@Override
public void delete(Account.Id key) {
try {
- delete(idTerm(getSchema().useLegacyNumericFields(), key)).get();
+ delete(idTerm(getSchema().hasField(ID), key)).get();
} catch (ExecutionException | InterruptedException e) {
throw new StorageException(e);
}
@@ -164,15 +164,14 @@
@Override
public DataSource<AccountState> getSource(Predicate<AccountState> p, QueryOptions opts)
throws QueryParseException {
- queryBuilder.getSchema().useLegacyNumericFields();
return new LuceneQuerySource(
- opts.filterFields(o -> IndexUtils.accountFields(o, getSchema().useLegacyNumericFields())),
+ opts.filterFields(o -> IndexUtils.accountFields(o, getSchema().hasField(ID))),
queryBuilder.toQuery(p),
getSort());
}
private Sort getSort() {
- String idSortField = getSchema().useLegacyNumericFields() ? ID_SORT_FIELD : ID2_SORT_FIELD;
+ String idSortField = getSchema().hasField(ID) ? ID_SORT_FIELD : ID2_SORT_FIELD;
return new Sort(
new SortField(FULL_NAME_SORT_FIELD, SortField.Type.STRING, false),
new SortField(EMAIL_SORT_FIELD, SortField.Type.STRING, false),
@@ -181,10 +180,10 @@
@Override
protected AccountState fromDocument(Document doc) {
- FieldDef<AccountState, ?> idField = getSchema().useLegacyNumericFields() ? ID : ID_STR;
+ FieldDef<AccountState, ?> idField = getSchema().hasField(ID) ? ID : ID_STR;
Account.Id id =
Account.id(
- getSchema().useLegacyNumericFields()
+ getSchema().hasField(ID)
? doc.getField(idField.getName()).numericValue().intValue()
: Integer.valueOf(doc.getField(idField.getName()).stringValue()));
// Use the AccountCache rather than depending on any stored fields in the document (of which
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 9ea9d2e..cf176ee 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -17,7 +17,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
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.LEGACY_ID;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID_STR;
import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES;
@@ -100,30 +99,39 @@
static final String UPDATED_SORT_FIELD = sortFieldName(ChangeField.UPDATED);
static final String MERGED_ON_SORT_FIELD = sortFieldName(ChangeField.MERGED_ON);
- static final String ID_SORT_FIELD = sortFieldName(ChangeField.LEGACY_ID);
- static final String ID2_SORT_FIELD = sortFieldName(ChangeField.LEGACY_ID_STR);
+ static final String ID_STR_SORT_FIELD = sortFieldName(ChangeField.LEGACY_ID_STR);
private static final String CHANGES = "changes";
private static final String CHANGES_OPEN = "open";
private static final String CHANGES_CLOSED = "closed";
private static final String CHANGE_FIELD = ChangeField.CHANGE.getName();
- @FunctionalInterface
- interface IdTerm {
- Term get(String name, int id);
+ /*
+ @FunctionalInterface
+ interface IdTerm {
+ Term get(String name, int id);
+ }
+
+ static Term idTerm(IdTerm idTerm, FieldDef<ChangeData, ?> idField, ChangeData cd) {
+ return idTerm(idTerm, idField, cd.getId());
+ }
+
+ static Term idTerm(IdTerm idTerm, FieldDef<ChangeData, ?> idField, Change.Id id) {
+ return idTerm.get(idField.getName(), id.get());
+ }
+
+ @FunctionalInterface
+ interface ChangeIdExtractor {
+ Change.Id extract(IndexableField f);
+ }
+ */
+
+ static Term idTerm(ChangeData cd) {
+ return idTerm(cd.getId());
}
- static Term idTerm(IdTerm idTerm, FieldDef<ChangeData, ?> idField, ChangeData cd) {
- return idTerm(idTerm, idField, cd.getId());
- }
-
- static Term idTerm(IdTerm idTerm, FieldDef<ChangeData, ?> idField, Change.Id id) {
- return idTerm.get(idField.getName(), id.get());
- }
-
- @FunctionalInterface
- interface ChangeIdExtractor {
- Change.Id extract(IndexableField f);
+ static Term idTerm(Change.Id id) {
+ return QueryBuilder.stringTerm(LEGACY_ID_STR.getName(), Integer.toString(id.get()));
}
private final ListeningExecutorService executor;
@@ -132,12 +140,6 @@
private final QueryBuilder<ChangeData> queryBuilder;
private final ChangeSubIndex openIndex;
private final ChangeSubIndex closedIndex;
-
- // TODO(davido): Remove the below fields when support for legacy numeric fields is removed.
- private final FieldDef<ChangeData, ?> idField;
- private final String idSortFieldName;
- private final IdTerm idTerm;
- private final ChangeIdExtractor extractor;
private final ImmutableSet<String> skipFields;
@Inject
@@ -205,20 +207,6 @@
searcherFactory,
autoFlush);
}
-
- idField = this.schema.useLegacyNumericFields() ? LEGACY_ID : LEGACY_ID_STR;
- idSortFieldName = schema.useLegacyNumericFields() ? ID_SORT_FIELD : ID2_SORT_FIELD;
- idTerm =
- (name, id) ->
- this.schema.useLegacyNumericFields()
- ? QueryBuilder.intTerm(name, id)
- : QueryBuilder.stringTerm(name, Integer.toString(id));
- extractor =
- (f) ->
- Change.id(
- this.schema.useLegacyNumericFields()
- ? f.numericValue().intValue()
- : Integer.valueOf(f.stringValue()));
}
@Override
@@ -237,7 +225,7 @@
@Override
public void replace(ChangeData cd) {
- Term id = LuceneChangeIndex.idTerm(idTerm, idField, cd);
+ Term id = LuceneChangeIndex.idTerm(cd);
// toDocument is essentially static and doesn't depend on the specific
// sub-index, so just pick one.
Document doc = openIndex.toDocument(cd);
@@ -270,9 +258,9 @@
@Override
public void delete(Change.Id changeId) {
- Term id = LuceneChangeIndex.idTerm(idTerm, idField, changeId);
+ Term idTerm = LuceneChangeIndex.idTerm(changeId);
try {
- Futures.allAsList(openIndex.delete(id), closedIndex.delete(id)).get();
+ Futures.allAsList(openIndex.delete(idTerm), closedIndex.delete(idTerm)).get();
} catch (ExecutionException | InterruptedException e) {
throw new StorageException(e);
}
@@ -309,7 +297,7 @@
return new Sort(
new SortField(UPDATED_SORT_FIELD, SortField.Type.LONG, true),
new SortField(MERGED_ON_SORT_FIELD, SortField.Type.LONG, true),
- new SortField(idSortFieldName, SortField.Type.LONG, true));
+ new SortField(ID_STR_SORT_FIELD, SortField.Type.LONG, true));
}
private class QuerySource implements ChangeDataSource {
@@ -357,7 +345,7 @@
throw new StorageException("interrupted");
}
- final Set<String> fields = IndexUtils.changeFields(opts, schema.useLegacyNumericFields());
+ final Set<String> fields = IndexUtils.changeFields(opts);
return new ChangeDataResults(
executor.submit(
new Callable<List<Document>>() {
@@ -378,7 +366,7 @@
public ResultSet<FieldBundle> readRaw() {
List<Document> documents;
try {
- documents = doRead(IndexUtils.changeFields(opts, schema.useLegacyNumericFields()));
+ documents = doRead(IndexUtils.changeFields(opts));
} catch (IOException e) {
throw new StorageException(e);
}
@@ -457,7 +445,7 @@
ImmutableList.Builder<ChangeData> result =
ImmutableList.builderWithExpectedSize(docs.size());
for (Document doc : docs) {
- result.add(toChangeData(fields(doc, fields), fields, idField.getName()));
+ result.add(toChangeData(fields(doc, fields), fields, LEGACY_ID_STR.getName()));
}
return result.build();
} catch (InterruptedException e) {
@@ -499,9 +487,10 @@
} else {
IndexableField f = Iterables.getFirst(doc.get(idFieldName), null);
+ Change.Id id = Change.id(Integer.valueOf(f.stringValue()));
// IndexUtils#changeFields ensures either CHANGE or PROJECT is always present.
IndexableField project = doc.get(PROJECT.getName()).iterator().next();
- cd = changeDataFactory.create(Project.nameKey(project.stringValue()), extractor.extract(f));
+ cd = changeDataFactory.create(Project.nameKey(project.stringValue()), id);
}
for (FieldDef<ChangeData, ?> field : getSchema().getFields().values()) {
diff --git a/java/com/google/gerrit/lucene/QueryBuilder.java b/java/com/google/gerrit/lucene/QueryBuilder.java
index e1b56c6..bd34743 100644
--- a/java/com/google/gerrit/lucene/QueryBuilder.java
+++ b/java/com/google/gerrit/lucene/QueryBuilder.java
@@ -15,6 +15,7 @@
package com.google.gerrit.lucene;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.apache.lucene.search.BooleanClause.Occur.MUST;
import static org.apache.lucene.search.BooleanClause.Occur.MUST_NOT;
@@ -39,36 +40,18 @@
import org.apache.lucene.document.LongPoint;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanQuery;
-import org.apache.lucene.search.LegacyNumericRangeQuery;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.PrefixQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.RegexpQuery;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.util.BytesRefBuilder;
-import org.apache.lucene.util.LegacyNumericUtils;
-@SuppressWarnings("deprecation")
public class QueryBuilder<V> {
- @FunctionalInterface
- static interface IntTermQuery {
- Query get(String name, int value);
- }
-
- @FunctionalInterface
- static interface IntRangeQuery {
- Query get(String name, int min, int max);
- }
-
- @FunctionalInterface
- static interface LongRangeQuery {
- Query get(String name, long min, long max);
- }
-
- static Term intTerm(String name, int value) {
- BytesRefBuilder builder = new BytesRefBuilder();
- LegacyNumericUtils.intToPrefixCoded(value, 0, builder);
- return new Term(name, builder.get());
+ /** @param name field name qparam i key value */
+ static Term intTerm(String name, int i) {
+ checkState(false, "Lucene index implementation removed legacy numeric type");
+ return null;
}
static Term stringTerm(String name, String value) {
@@ -84,29 +67,9 @@
private final Schema<V> schema;
private final org.apache.lucene.util.QueryBuilder queryBuilder;
- // TODO(davido): Remove the below fields when support for legacy numeric fields is removed.
- private final IntTermQuery intTermQuery;
- private final IntRangeQuery intRangeTermQuery;
- private final LongRangeQuery longRangeQuery;
-
public QueryBuilder(Schema<V> schema, Analyzer analyzer) {
this.schema = schema;
queryBuilder = new org.apache.lucene.util.QueryBuilder(analyzer);
- intTermQuery =
- (name, value) ->
- this.schema.useLegacyNumericFields()
- ? new TermQuery(intTerm(name, value))
- : intPoint(name, value);
- intRangeTermQuery =
- (name, min, max) ->
- this.schema.useLegacyNumericFields()
- ? LegacyNumericRangeQuery.newIntRange(name, min, max, true, true)
- : IntPoint.newRangeQuery(name, min, max);
- longRangeQuery =
- (name, min, max) ->
- this.schema.useLegacyNumericFields()
- ? LegacyNumericRangeQuery.newLongRange(name, min, max, true, true)
- : LongPoint.newRangeQuery(name, min, max);
}
public Query toQuery(Predicate<V> p) throws QueryParseException {
@@ -209,7 +172,7 @@
} catch (NumberFormatException e) {
throw new QueryParseException("not an integer: " + p.getValue(), e);
}
- return intTermQuery.get(p.getField().getName(), value);
+ return intPoint(p.getField().getName(), value);
}
private Query intRangeQuery(IndexPredicate<V> p) throws QueryParseException {
@@ -220,9 +183,9 @@
int maximum = r.getMaximumValue();
if (minimum == maximum) {
// Just fall back to a standard integer query.
- return intTermQuery.get(name, minimum);
+ return intPoint(name, minimum);
}
- return intRangeTermQuery.get(name, minimum, maximum);
+ return IntPoint.newRangeQuery(name, minimum, maximum);
}
throw new QueryParseException("not an integer range: " + p);
}
@@ -230,7 +193,7 @@
private Query timestampQuery(IndexPredicate<V> p) throws QueryParseException {
if (p instanceof TimestampRangePredicate) {
TimestampRangePredicate<V> r = (TimestampRangePredicate<V>) p;
- return longRangeQuery.get(
+ return LongPoint.newRangeQuery(
r.getField().getName(),
r.getMinTimestamp().toEpochMilli(),
r.getMaxTimestamp().toEpochMilli());
@@ -240,7 +203,7 @@
private Query notTimestamp(TimestampRangePredicate<V> r) throws QueryParseException {
if (r.getMinTimestamp().toEpochMilli() == 0) {
- return longRangeQuery.get(
+ return LongPoint.newRangeQuery(
r.getField().getName(), r.getMaxTimestamp().toEpochMilli(), Long.MAX_VALUE);
}
throw new QueryParseException("cannot negate: " + r);
diff --git a/java/com/google/gerrit/server/change/AbandonUtil.java b/java/com/google/gerrit/server/change/AbandonUtil.java
index 29bd045..59b32c2 100644
--- a/java/com/google/gerrit/server/change/AbandonUtil.java
+++ b/java/com/google/gerrit/server/change/AbandonUtil.java
@@ -40,10 +40,7 @@
private final ChangeCleanupConfig cfg;
private final Provider<ChangeQueryProcessor> queryProvider;
- // Provider is needed, because AbandonUtil is singleton, but ChangeQueryBuilder accesses
- // index collection, that is only provided when multiversion index module is started.
- // TODO(davido); Remove provider again, when support for legacy numeric fields is removed.
- private final Provider<ChangeQueryBuilder> queryBuilderProvider;
+ private final ChangeQueryBuilder queryBuilder;
private final BatchAbandon batchAbandon;
private final InternalUser internalUser;
@@ -52,11 +49,11 @@
ChangeCleanupConfig cfg,
InternalUser.Factory internalUserFactory,
Provider<ChangeQueryProcessor> queryProvider,
- Provider<ChangeQueryBuilder> queryBuilderProvider,
+ ChangeQueryBuilder queryBuilder,
BatchAbandon batchAbandon) {
this.cfg = cfg;
this.queryProvider = queryProvider;
- this.queryBuilderProvider = queryBuilderProvider;
+ this.queryBuilder = queryBuilder;
this.batchAbandon = batchAbandon;
internalUser = internalUserFactory.create();
}
@@ -74,11 +71,7 @@
}
List<ChangeData> changesToAbandon =
- queryProvider
- .get()
- .enforceVisibility(false)
- .query(queryBuilderProvider.get().parse(query))
- .entities();
+ queryProvider.get().enforceVisibility(false).query(queryBuilder.parse(query)).entities();
ImmutableListMultimap.Builder<Project.NameKey, ChangeData> builder =
ImmutableListMultimap.builder();
for (ChangeData cd : changesToAbandon) {
@@ -118,7 +111,7 @@
queryProvider
.get()
.enforceVisibility(false)
- .query(queryBuilderProvider.get().parse(newQuery))
+ .query(queryBuilder.parse(newQuery))
.entities();
if (!changesToAbandon.isEmpty()) {
validChanges.add(cd);
diff --git a/java/com/google/gerrit/server/change/AddToAttentionSetOp.java b/java/com/google/gerrit/server/change/AddToAttentionSetOp.java
index a980c32..1cf31c1 100644
--- a/java/com/google/gerrit/server/change/AddToAttentionSetOp.java
+++ b/java/com/google/gerrit/server/change/AddToAttentionSetOp.java
@@ -16,13 +16,11 @@
import static java.util.Objects.requireNonNull;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.mail.send.AddToAttentionSetSender;
-import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -31,18 +29,14 @@
import com.google.gerrit.server.util.AttentionSetEmail;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
-import java.io.IOException;
/** Add a specified user to the attention set. */
public class AddToAttentionSetOp implements BatchUpdateOp {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
public interface Factory {
AddToAttentionSetOp create(Account.Id attentionUserId, String reason, boolean notify);
}
private final ChangeData.Factory changeDataFactory;
- private final MessageIdGenerator messageIdGenerator;
private final AddToAttentionSetSender.Factory addToAttentionSetSender;
private final AttentionSetEmail.Factory attentionSetEmailFactory;
@@ -63,14 +57,12 @@
AddToAttentionSetOp(
ChangeData.Factory changeDataFactory,
AddToAttentionSetSender.Factory addToAttentionSetSender,
- MessageIdGenerator messageIdGenerator,
AttentionSetEmail.Factory attentionSetEmailFactory,
@Assisted Account.Id attentionUserId,
@Assisted String reason,
@Assisted boolean notify) {
this.changeDataFactory = changeDataFactory;
this.addToAttentionSetSender = addToAttentionSetSender;
- this.messageIdGenerator = messageIdGenerator;
this.attentionSetEmailFactory = attentionSetEmailFactory;
this.attentionUserId = requireNonNull(attentionUserId, "user");
@@ -105,18 +97,13 @@
if (!notify) {
return;
}
- try {
- attentionSetEmailFactory
- .create(
- addToAttentionSetSender.create(ctx.getProject(), change.getId()),
- ctx,
- change,
- reason,
- messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), change.currentPatchSetId()),
- attentionUserId)
- .sendAsync();
- } catch (IOException e) {
- logger.atSevere().withCause(e).log(e.getMessage(), change.getId());
- }
+ attentionSetEmailFactory
+ .create(
+ addToAttentionSetSender.create(ctx.getProject(), change.getId()),
+ ctx,
+ change,
+ reason,
+ attentionUserId)
+ .sendAsync();
}
}
diff --git a/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java b/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java
index 50ee9d4..9fb4fc4 100644
--- a/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java
+++ b/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java
@@ -16,13 +16,11 @@
import static java.util.Objects.requireNonNull;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.RemoveFromAttentionSetSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.query.change.ChangeData;
@@ -32,19 +30,15 @@
import com.google.gerrit.server.util.AttentionSetEmail;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
-import java.io.IOException;
import java.util.Optional;
/** Remove a specified user from the attention set. */
public class RemoveFromAttentionSetOp implements BatchUpdateOp {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
public interface Factory {
RemoveFromAttentionSetOp create(Account.Id attentionUserId, String reason, boolean notify);
}
private final ChangeData.Factory changeDataFactory;
- private final MessageIdGenerator messageIdGenerator;
private final RemoveFromAttentionSetSender.Factory removeFromAttentionSetSender;
private final AttentionSetEmail.Factory attentionSetEmailFactory;
@@ -64,14 +58,12 @@
@Inject
RemoveFromAttentionSetOp(
ChangeData.Factory changeDataFactory,
- MessageIdGenerator messageIdGenerator,
RemoveFromAttentionSetSender.Factory removeFromAttentionSetSenderFactory,
AttentionSetEmail.Factory attentionSetEmailFactory,
@Assisted Account.Id attentionUserId,
@Assisted String reason,
@Assisted boolean notify) {
this.changeDataFactory = changeDataFactory;
- this.messageIdGenerator = messageIdGenerator;
this.removeFromAttentionSetSender = removeFromAttentionSetSenderFactory;
this.attentionSetEmailFactory = attentionSetEmailFactory;
this.attentionUserId = requireNonNull(attentionUserId, "user");
@@ -105,18 +97,13 @@
if (!notify) {
return;
}
- try {
- attentionSetEmailFactory
- .create(
- removeFromAttentionSetSender.create(ctx.getProject(), change.getId()),
- ctx,
- change,
- reason,
- messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), change.currentPatchSetId()),
- attentionUserId)
- .sendAsync();
- } catch (IOException e) {
- logger.atSevere().withCause(e).log(e.getMessage(), change.getId());
- }
+ attentionSetEmailFactory
+ .create(
+ removeFromAttentionSetSender.create(ctx.getProject(), change.getId()),
+ ctx,
+ change,
+ reason,
+ attentionUserId)
+ .sendAsync();
}
}
diff --git a/java/com/google/gerrit/server/index/IndexUtils.java b/java/com/google/gerrit/server/index/IndexUtils.java
index ee8dfc8..fced578 100644
--- a/java/com/google/gerrit/server/index/IndexUtils.java
+++ b/java/com/google/gerrit/server/index/IndexUtils.java
@@ -15,21 +15,18 @@
package com.google.gerrit.server.index;
import static com.google.gerrit.server.index.change.ChangeField.CHANGE;
-import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID_STR;
import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.project.ProjectField;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.index.account.AccountField;
import com.google.gerrit.server.index.group.GroupField;
-import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.GroupBackedUser;
import java.io.IOException;
import java.util.Set;
@@ -71,11 +68,9 @@
/**
* Returns a sanitized set of fields for change index queries by removing fields that the current
- * index version doesn't support and accounting for numeric vs. string primary keys. The primary
- * key situation is temporary and should be removed after the migration is done.
+ * index version doesn't support.
*/
- public static Set<String> changeFields(QueryOptions opts, boolean useLegacyNumericFields) {
- FieldDef<ChangeData, ?> idField = useLegacyNumericFields ? LEGACY_ID : LEGACY_ID_STR;
+ public static Set<String> changeFields(QueryOptions opts) {
// Ensure we request enough fields to construct a ChangeData. We need both
// change ID and project, which can either come via the Change field or
// separate fields.
@@ -84,10 +79,10 @@
// A Change is always sufficient.
return fs;
}
- if (fs.contains(PROJECT.getName()) && fs.contains(idField.getName())) {
+ if (fs.contains(PROJECT.getName()) && fs.contains(LEGACY_ID_STR.getName())) {
return fs;
}
- return Sets.union(fs, ImmutableSet.of(idField.getName(), PROJECT.getName()));
+ return Sets.union(fs, ImmutableSet.of(LEGACY_ID_STR.getName(), PROJECT.getName()));
}
/**
diff --git a/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java b/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
index 5de3ba4..7029d10 100644
--- a/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
@@ -22,30 +22,25 @@
/** Definition of account index versions (schemata). See {@link SchemaDefinitions}. */
public class AccountSchemaDefinitions extends SchemaDefinitions<AccountState> {
+
@Deprecated
- static final Schema<AccountState> V4 =
+ static final Schema<AccountState> V8 =
schema(
AccountField.ACTIVE,
AccountField.EMAIL,
AccountField.EXTERNAL_ID,
+ AccountField.EXTERNAL_ID_STATE,
AccountField.FULL_NAME,
AccountField.ID,
AccountField.NAME_PART,
+ AccountField.NAME_PART_NO_SECONDARY_EMAIL,
+ AccountField.PREFERRED_EMAIL,
+ AccountField.PREFERRED_EMAIL_EXACT,
+ AccountField.REF_STATE,
AccountField.REGISTERED,
AccountField.USERNAME,
AccountField.WATCHED_PROJECT);
- @Deprecated static final Schema<AccountState> V5 = schema(V4, AccountField.PREFERRED_EMAIL);
-
- @Deprecated
- static final Schema<AccountState> V6 =
- schema(V5, AccountField.REF_STATE, AccountField.EXTERNAL_ID_STATE);
-
- @Deprecated static final Schema<AccountState> V7 = schema(V6, AccountField.PREFERRED_EMAIL_EXACT);
-
- @Deprecated
- static final Schema<AccountState> V8 = schema(V7, AccountField.NAME_PART_NO_SECONDARY_EMAIL);
-
// Bump Lucene version requires reindexing
@Deprecated static final Schema<AccountState> V9 = schema(V8);
@@ -55,14 +50,17 @@
// New numeric types: use dimensional points using the k-d tree geo-spatial data structure
// to offer fast single- and multi-dimensional numeric range. As the consequense, integer
// document id type is replaced with string document id type.
+ @Deprecated
static final Schema<AccountState> V11 =
new Schema.Builder<AccountState>()
.add(V10)
.remove(AccountField.ID)
.add(AccountField.ID_STR)
- .legacyNumericFields(false)
.build();
+ // Bump Lucene version requires reindexing
+ static final Schema<AccountState> V12 = schema(V11);
+
/**
* Name of the account index to be used when contacting index backends or loading configurations.
*/
diff --git a/java/com/google/gerrit/server/index/account/StalenessChecker.java b/java/com/google/gerrit/server/index/account/StalenessChecker.java
index 50fdcde..81a4d1e 100644
--- a/java/com/google/gerrit/server/index/account/StalenessChecker.java
+++ b/java/com/google/gerrit/server/index/account/StalenessChecker.java
@@ -100,7 +100,7 @@
return StalenessCheckResult.notStale();
}
- boolean useLegacyNumericFields = i.getSchema().useLegacyNumericFields();
+ boolean useLegacyNumericFields = i.getSchema().hasField(AccountField.ID);
ImmutableSet<String> fields = useLegacyNumericFields ? FIELDS : FIELDS2;
Optional<FieldBundle> result =
i.getRaw(
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index c06347e..281bcb4 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -116,9 +116,6 @@
// TODO: Rename LEGACY_ID to NUMERIC_ID
/** Legacy change ID. */
- public static final FieldDef<ChangeData, Integer> LEGACY_ID =
- integer("legacy_id").stored().build(cd -> cd.getId().get());
-
public static final FieldDef<ChangeData, String> LEGACY_ID_STR =
exact("legacy_id_str").stored().build(cd -> String.valueOf(cd.getId().get()));
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndex.java b/java/com/google/gerrit/server/index/change/ChangeIndex.java
index 05c5c77..6fc2665 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndex.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndex.java
@@ -30,8 +30,6 @@
@Override
default Predicate<ChangeData> keyPredicate(Change.Id id) {
- return getSchema().useLegacyNumericFields()
- ? ChangePredicates.id(id)
- : ChangePredicates.idStr(id);
+ return ChangePredicates.idStr(id);
}
}
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 0a06735..aa08069 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -23,18 +23,25 @@
/** Definition of change index versions (schemata). See {@link SchemaDefinitions}. */
public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
@Deprecated
- static final Schema<ChangeData> V55 =
+ /** Added new field {@link ChangeField#IS_SUBMITTABLE} based on submit requirements. */
+ static final Schema<ChangeData> V74 =
schema(
ChangeField.ADDED,
ChangeField.APPROVAL,
ChangeField.ASSIGNEE,
+ ChangeField.ATTENTION_SET_FULL,
+ ChangeField.ATTENTION_SET_USERS,
+ ChangeField.ATTENTION_SET_USERS_COUNT,
ChangeField.AUTHOR,
ChangeField.CHANGE,
+ ChangeField.CHERRY_PICK,
+ ChangeField.CHERRY_PICK_OF_CHANGE,
+ ChangeField.CHERRY_PICK_OF_PATCHSET,
ChangeField.COMMENT,
ChangeField.COMMENTBY,
ChangeField.COMMIT,
- ChangeField.COMMITTER,
ChangeField.COMMIT_MESSAGE,
+ ChangeField.COMMITTER,
ChangeField.DELETED,
ChangeField.DELTA,
ChangeField.DIRECTORY,
@@ -47,14 +54,19 @@
ChangeField.EXTENSION,
ChangeField.FILE_PART,
ChangeField.FOOTER,
+ ChangeField.FUZZY_HASHTAG,
ChangeField.FUZZY_TOPIC,
ChangeField.GROUP,
ChangeField.HASHTAG,
ChangeField.HASHTAG_CASE_AWARE,
ChangeField.ID,
+ ChangeField.IS_PURE_REVERT,
+ ChangeField.IS_SUBMITTABLE,
ChangeField.LABEL,
- ChangeField.LEGACY_ID,
+ ChangeField.LEGACY_ID_STR,
+ ChangeField.MERGE,
ChangeField.MERGEABLE,
+ ChangeField.MERGED_ON,
ChangeField.ONLY_EXTENSIONS,
ChangeField.OWNER,
ChangeField.PATCH_SET,
@@ -77,131 +89,18 @@
ChangeField.STATUS,
ChangeField.STORED_SUBMIT_RECORD_LENIENT,
ChangeField.STORED_SUBMIT_RECORD_STRICT,
+ ChangeField.STORED_SUBMIT_REQUIREMENTS,
ChangeField.SUBMISSIONID,
ChangeField.SUBMIT_RECORD,
+ ChangeField.SUBMIT_RULE_RESULT,
ChangeField.TOTAL_COMMENT_COUNT,
ChangeField.TR,
ChangeField.UNRESOLVED_COMMENT_COUNT,
ChangeField.UPDATED,
+ ChangeField.UPLOADER,
ChangeField.WIP);
/**
- * The computation of the {@link ChangeField#EXTENSION} field is changed, hence reindexing is
- * required.
- */
- @Deprecated static final Schema<ChangeData> V56 = schema(V55);
-
- /**
- * New numeric types: use dimensional points using the k-d tree geo-spatial data structure to
- * offer fast single- and multi-dimensional numeric range. As the consequense, {@link
- * ChangeField#LEGACY_ID} is replaced with {@link ChangeField#LEGACY_ID_STR}.
- */
- @Deprecated
- static final Schema<ChangeData> V57 =
- new Schema.Builder<ChangeData>()
- .add(V56)
- .remove(ChangeField.LEGACY_ID)
- .add(ChangeField.LEGACY_ID_STR)
- .legacyNumericFields(false)
- .build();
-
- /**
- * Added new fields {@link ChangeField#CHERRY_PICK_OF_CHANGE} and {@link
- * ChangeField#CHERRY_PICK_OF_PATCHSET}.
- */
- @Deprecated
- static final Schema<ChangeData> V58 =
- new Schema.Builder<ChangeData>()
- .add(V57)
- .add(ChangeField.CHERRY_PICK_OF_CHANGE)
- .add(ChangeField.CHERRY_PICK_OF_PATCHSET)
- .build();
-
- /**
- * Added new fields {@link ChangeField#ATTENTION_SET_USERS} and {@link
- * ChangeField#ATTENTION_SET_FULL}.
- */
- @Deprecated
- static final Schema<ChangeData> V59 =
- new Schema.Builder<ChangeData>()
- .add(V58)
- .add(ChangeField.ATTENTION_SET_USERS)
- .add(ChangeField.ATTENTION_SET_FULL)
- .build();
-
- /** Added new fields {@link ChangeField#MERGE} */
- @Deprecated
- static final Schema<ChangeData> V60 =
- new Schema.Builder<ChangeData>().add(V59).add(ChangeField.MERGE).build();
-
- /** Added new field {@link ChangeField#MERGED_ON} */
- @Deprecated
- static final Schema<ChangeData> V61 =
- new Schema.Builder<ChangeData>().add(V60).add(ChangeField.MERGED_ON).build();
-
- /** Added new field {@link ChangeField#FUZZY_HASHTAG} */
- @Deprecated
- static final Schema<ChangeData> V62 =
- new Schema.Builder<ChangeData>().add(V61).add(ChangeField.FUZZY_HASHTAG).build();
-
- /**
- * The computation of the {@link ChangeField#DIRECTORY} field is changed, hence reindexing is
- * required.
- */
- @Deprecated static final Schema<ChangeData> V63 = schema(V62, false);
-
- /** Added support for MIN/MAX/ANY for {@link ChangeField#LABEL} */
- @Deprecated static final Schema<ChangeData> V64 = schema(V63, false);
-
- /** Added new field for submit requirements. */
- @Deprecated
- static final Schema<ChangeData> V65 =
- new Schema.Builder<ChangeData>().add(V64).add(ChangeField.STORED_SUBMIT_REQUIREMENTS).build();
-
- /**
- * The computation of {@link ChangeField#LABEL} has changed: We added the non_uploader arg to the
- * label field.
- */
- @Deprecated static final Schema<ChangeData> V66 = schema(V65, false);
-
- /** Updated submit records: store the rule name that created the submit record. */
- @Deprecated static final Schema<ChangeData> V67 = schema(V66, false);
-
- /** Added new field {@link ChangeField#SUBMIT_RULE_RESULT}. */
- @Deprecated
- static final Schema<ChangeData> V68 =
- new Schema.Builder<ChangeData>().add(V67).add(ChangeField.SUBMIT_RULE_RESULT).build();
-
- /** Added new field {@link ChangeField#CHERRY_PICK}. */
- @Deprecated
- static final Schema<ChangeData> V69 =
- new Schema.Builder<ChangeData>().add(V68).add(ChangeField.CHERRY_PICK).build();
-
- /** Added new field {@link ChangeField#ATTENTION_SET_USERS_COUNT}. */
- @Deprecated
- static final Schema<ChangeData> V70 =
- new Schema.Builder<ChangeData>().add(V69).add(ChangeField.ATTENTION_SET_USERS_COUNT).build();
-
- /** Added new field {@link ChangeField#UPLOADER}. */
- @Deprecated
- static final Schema<ChangeData> V71 =
- new Schema.Builder<ChangeData>().add(V70).add(ChangeField.UPLOADER).build();
-
- /** Added new field {@link ChangeField#IS_PURE_REVERT}. */
- @Deprecated
- static final Schema<ChangeData> V72 =
- new Schema.Builder<ChangeData>().add(V71).add(ChangeField.IS_PURE_REVERT).build();
-
- @Deprecated
- /** Added new "count=$count" argument to the {@link ChangeField#LABEL} operator. */
- static final Schema<ChangeData> V73 = schema(V72, false);
-
- @Deprecated
- /** Added new field {@link ChangeField#IS_SUBMITTABLE} based on submit requirements. */
- static final Schema<ChangeData> V74 =
- new Schema.Builder<ChangeData>().add(V73).add(ChangeField.IS_SUBMITTABLE).build();
-
- /**
* Added new field {@link ChangeField#PREFIX_HASHTAG} and {@link ChangeField#PREFIX_TOPIC} to
* allow easier search for topics.
*/
diff --git a/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java b/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
index c4d8952..773aa9a 100644
--- a/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
@@ -23,23 +23,20 @@
/** Definition of group index versions (schemata). See {@link SchemaDefinitions}. */
public class GroupSchemaDefinitions extends SchemaDefinitions<InternalGroup> {
@Deprecated
- static final Schema<InternalGroup> V2 =
+ static final Schema<InternalGroup> V5 =
schema(
+ GroupField.CREATED_ON,
GroupField.DESCRIPTION,
GroupField.ID,
GroupField.IS_VISIBLE_TO_ALL,
+ GroupField.MEMBER,
GroupField.NAME,
GroupField.NAME_PART,
GroupField.OWNER_UUID,
+ GroupField.REF_STATE,
+ GroupField.SUBGROUP,
GroupField.UUID);
- @Deprecated static final Schema<InternalGroup> V3 = schema(V2, GroupField.CREATED_ON);
-
- @Deprecated
- static final Schema<InternalGroup> V4 = schema(V3, GroupField.MEMBER, GroupField.SUBGROUP);
-
- @Deprecated static final Schema<InternalGroup> V5 = schema(V4, GroupField.REF_STATE);
-
// Bump Lucene version requires reindexing
@Deprecated static final Schema<InternalGroup> V6 = schema(V5);
@@ -48,7 +45,7 @@
// New numeric types: use dimensional points using the k-d tree geo-spatial data structure
// to offer fast single- and multi-dimensional numeric range.
- static final Schema<InternalGroup> V8 = schema(V7, false);
+ static final Schema<InternalGroup> V8 = schema(V7);
/** Singleton instance of the schema definitions. This is one per JVM. */
public static final GroupSchemaDefinitions INSTANCE = new GroupSchemaDefinitions();
diff --git a/java/com/google/gerrit/server/mail/send/AddToAttentionSetSender.java b/java/com/google/gerrit/server/mail/send/AddToAttentionSetSender.java
index b13bcf6..f9ef199 100644
--- a/java/com/google/gerrit/server/mail/send/AddToAttentionSetSender.java
+++ b/java/com/google/gerrit/server/mail/send/AddToAttentionSetSender.java
@@ -30,7 +30,7 @@
@Inject
public AddToAttentionSetSender(
EmailArguments args, @Assisted Project.NameKey project, @Assisted Change.Id changeId) {
- super(args, project, changeId);
+ super(args, "addToAttentionSet", project, changeId);
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/AttentionSetSender.java b/java/com/google/gerrit/server/mail/send/AttentionSetSender.java
index 8f898a8..f5af783 100644
--- a/java/com/google/gerrit/server/mail/send/AttentionSetSender.java
+++ b/java/com/google/gerrit/server/mail/send/AttentionSetSender.java
@@ -11,6 +11,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
+
package com.google.gerrit.server.mail.send;
import com.google.gerrit.entities.Account;
@@ -23,8 +24,9 @@
private Account.Id attentionSetUser;
private String reason;
- public AttentionSetSender(EmailArguments args, Project.NameKey project, Change.Id changeId) {
- super(args, "addToAttentionSet", ChangeEmail.newChangeData(args, project, changeId));
+ public AttentionSetSender(
+ EmailArguments args, String messageClass, Project.NameKey project, Change.Id changeId) {
+ super(args, messageClass, ChangeEmail.newChangeData(args, project, changeId));
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/RemoveFromAttentionSetSender.java b/java/com/google/gerrit/server/mail/send/RemoveFromAttentionSetSender.java
index 6762b7d..5242bfb 100644
--- a/java/com/google/gerrit/server/mail/send/RemoveFromAttentionSetSender.java
+++ b/java/com/google/gerrit/server/mail/send/RemoveFromAttentionSetSender.java
@@ -30,7 +30,7 @@
@Inject
public RemoveFromAttentionSetSender(
EmailArguments args, @Assisted Project.NameKey project, @Assisted Change.Id changeId) {
- super(args, project, changeId);
+ super(args, "removeFromAttentionSet", project, changeId);
}
@Override
diff --git a/java/com/google/gerrit/server/query/account/AccountPredicates.java b/java/com/google/gerrit/server/query/account/AccountPredicates.java
index 8f94089..dd8d685 100644
--- a/java/com/google/gerrit/server/query/account/AccountPredicates.java
+++ b/java/com/google/gerrit/server/query/account/AccountPredicates.java
@@ -66,7 +66,7 @@
public static Predicate<AccountState> id(Schema<AccountState> schema, Account.Id accountId) {
return new AccountPredicate(
- schema.useLegacyNumericFields() ? AccountField.ID : AccountField.ID_STR,
+ schema.hasField(AccountField.ID) ? AccountField.ID : AccountField.ID_STR,
AccountQueryBuilder.FIELD_ACCOUNT,
accountId.toString());
}
diff --git a/java/com/google/gerrit/server/query/change/ChangePredicates.java b/java/com/google/gerrit/server/query/change/ChangePredicates.java
index ce17b31..c874db7 100644
--- a/java/com/google/gerrit/server/query/change/ChangePredicates.java
+++ b/java/com/google/gerrit/server/query/change/ChangePredicates.java
@@ -136,15 +136,6 @@
* Returns a predicate that matches the change with the provided {@link
* com.google.gerrit.entities.Change.Id}.
*/
- public static Predicate<ChangeData> id(Change.Id id) {
- return new ChangeIndexPredicate(
- ChangeField.LEGACY_ID, ChangeQueryBuilder.FIELD_CHANGE, id.toString());
- }
-
- /**
- * Returns a predicate that matches the change with the provided {@link
- * com.google.gerrit.entities.Change.Id}.
- */
public static Predicate<ChangeData> idStr(Change.Id id) {
return new ChangeIndexPredicate(
ChangeField.LEGACY_ID_STR, ChangeQueryBuilder.FIELD_CHANGE, id.toString());
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 28ffef7..9e9a960 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -559,9 +559,7 @@
if (PAT_LEGACY_ID.matcher(query).matches()) {
Integer id = Ints.tryParse(query);
if (id != null) {
- return args.getSchema().useLegacyNumericFields()
- ? ChangePredicates.id(Change.id(id))
- : ChangePredicates.idStr(Change.id(id));
+ return ChangePredicates.idStr(Change.id(id));
}
} else if (PAT_CHANGE_ID.matcher(query).matches()) {
return ChangePredicates.idPrefix(parseChangeId(query));
diff --git a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
index f95dbb0..fc4c1d0 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -89,11 +89,7 @@
List<Predicate<ChangeData>> and = new ArrayList<>(5);
and.add(ChangePredicates.project(c.getProject()));
and.add(ChangePredicates.ref(c.getDest().branch()));
- and.add(
- Predicate.not(
- args.getSchema().useLegacyNumericFields()
- ? ChangePredicates.id(c.getId())
- : ChangePredicates.idStr(c.getId())));
+ and.add(Predicate.not(ChangePredicates.idStr(c.getId())));
and.add(Predicate.or(filePredicates));
ChangeDataCache changeDataCache = new ChangeDataCache(cd, args.projectCache);
diff --git a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index e7b25fb..99c1ca1 100644
--- a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -56,11 +56,6 @@
* holding on to a single instance.
*/
public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChangeQuery> {
- @FunctionalInterface
- static interface ChangeIdPredicateFactory {
- Predicate<ChangeData> create(Change.Id id);
- }
-
private static Predicate<ChangeData> ref(BranchNameKey branch) {
return ChangePredicates.ref(branch.branch());
}
@@ -84,9 +79,6 @@
private final ChangeData.Factory changeDataFactory;
private final ChangeNotes.Factory notesFactory;
- // TODO(davido): Remove the below fields when support for legacy numeric fields is removed.
- private final ChangeIdPredicateFactory predicateFactory;
-
@Inject
InternalChangeQuery(
ChangeQueryProcessor queryProcessor,
@@ -97,11 +89,6 @@
super(queryProcessor, indexes, indexConfig);
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
- predicateFactory =
- (id) ->
- schema().useLegacyNumericFields()
- ? ChangePredicates.id(id)
- : ChangePredicates.idStr(id);
}
public List<ChangeData> byKey(Change.Key key) {
@@ -113,13 +100,13 @@
}
public List<ChangeData> byLegacyChangeId(Change.Id id) {
- return query(predicateFactory.create(id));
+ return query(ChangePredicates.idStr(id));
}
public List<ChangeData> byLegacyChangeIds(Collection<Change.Id> ids) {
List<Predicate<ChangeData>> preds = new ArrayList<>(ids.size());
for (Change.Id id : ids) {
- preds.add(predicateFactory.create(id));
+ preds.add(ChangePredicates.idStr(id));
}
return query(or(preds));
}
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
index fbd99eb..fbb4fbd 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
@@ -70,7 +70,7 @@
private final Provider<CurrentUser> userProvider;
private final BatchUpdate.Factory batchUpdateFactory;
- private final Provider<ChangeQueryBuilder> queryBuilderProvider;
+ private final ChangeQueryBuilder queryBuilder;
private final Provider<InternalChangeQuery> queryProvider;
private final ChangeData.Factory changeDataFactory;
private final ChangeJson.Factory changeJsonFactory;
@@ -83,7 +83,7 @@
DeleteDraftComments(
Provider<CurrentUser> userProvider,
BatchUpdate.Factory batchUpdateFactory,
- Provider<ChangeQueryBuilder> queryBuilderProvider,
+ ChangeQueryBuilder queryBuilder,
Provider<InternalChangeQuery> queryProvider,
ChangeData.Factory changeDataFactory,
ChangeJson.Factory changeJsonFactory,
@@ -93,7 +93,7 @@
ExperimentFeatures experimentFeatures) {
this.userProvider = userProvider;
this.batchUpdateFactory = batchUpdateFactory;
- this.queryBuilderProvider = queryBuilderProvider;
+ this.queryBuilder = queryBuilder;
this.queryProvider = queryProvider;
this.changeDataFactory = changeDataFactory;
this.changeJsonFactory = changeJsonFactory;
@@ -161,7 +161,7 @@
return hasDraft;
}
try {
- return Predicate.and(hasDraft, queryBuilderProvider.get().parse(input.query));
+ return Predicate.and(hasDraft, queryBuilder.parse(input.query));
} catch (QueryParseException e) {
throw new BadRequestException("Invalid query: " + e.getMessage(), e);
}
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
index e3cf4db..842f4b9 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
@@ -32,7 +32,6 @@
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Url;
-import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.query.FieldBundle;
@@ -236,10 +235,7 @@
return suggestedReviewers;
}
- private static Account.Id fromIdField(FieldBundle f, boolean useLegacyNumericFields) {
- if (useLegacyNumericFields) {
- return Account.id(f.getValue(AccountField.ID).intValue());
- }
+ private static Account.Id fromIdField(FieldBundle f) {
return Account.id(Integer.valueOf(f.getValue(AccountField.ID_STR)));
}
@@ -255,10 +251,6 @@
accountQueryBuilder.defaultQuery(suggestReviewers.getQuery()));
logger.atFine().log("accounts index query: %s", pred);
accountIndexRewriter.validateMaxTermsInQuery(pred);
- boolean useLegacyNumericFields =
- accountIndexes.getSearchIndex().getSchema().useLegacyNumericFields();
- FieldDef<AccountState, ?> idField =
- useLegacyNumericFields ? AccountField.ID : AccountField.ID_STR;
ResultSet<FieldBundle> result =
accountIndexes
.getSearchIndex()
@@ -268,12 +260,10 @@
indexConfig,
0,
suggestReviewers.getLimit(),
- ImmutableSet.of(idField.getName())))
+ ImmutableSet.of(AccountField.ID_STR.getName())))
.readRaw();
List<Account.Id> matches =
- result.toList().stream()
- .map(f -> fromIdField(f, useLegacyNumericFields))
- .collect(toList());
+ result.toList().stream().map(f -> fromIdField(f)).collect(toList());
logger.atFine().log("Matches: %s", matches);
return matches;
} catch (TooManyTermsInQueryException e) {
diff --git a/java/com/google/gerrit/server/util/AttentionSetEmail.java b/java/com/google/gerrit/server/util/AttentionSetEmail.java
index 48ddd31..1b36139 100644
--- a/java/com/google/gerrit/server/util/AttentionSetEmail.java
+++ b/java/com/google/gerrit/server/util/AttentionSetEmail.java
@@ -18,19 +18,24 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.mail.send.AddToAttentionSetSender;
import com.google.gerrit.server.mail.send.AttentionSetSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
+import com.google.gerrit.server.mail.send.MessageIdGenerator.MessageId;
import com.google.gerrit.server.mail.send.RemoveFromAttentionSetSender;
import com.google.gerrit.server.update.Context;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.Optional;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
-public class AttentionSetEmail implements Runnable, RequestContext {
+public class AttentionSetEmail {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -43,7 +48,6 @@
* @param ctx context for sending the email.
* @param change the change that the user was added/removed in.
* @param reason reason for adding/removing the user.
- * @param messageId messageId for tracking the email.
* @param attentionUserId the user added/removed.
*/
AttentionSetEmail create(
@@ -51,70 +55,117 @@
Context ctx,
Change change,
String reason,
- MessageIdGenerator.MessageId messageId,
Account.Id attentionUserId);
}
- private ExecutorService sendEmailsExecutor;
- private AccountTemplateUtil accountTemplateUtil;
- private AttentionSetSender sender;
- private Context ctx;
- private Change change;
- private String reason;
-
- private MessageIdGenerator.MessageId messageId;
- private Account.Id attentionUserId;
+ private final ExecutorService sendEmailsExecutor;
+ private final AsyncSender asyncSender;
@Inject
AttentionSetEmail(
@SendEmailExecutor ExecutorService executor,
+ ThreadLocalRequestContext requestContext,
+ MessageIdGenerator messageIdGenerator,
AccountTemplateUtil accountTemplateUtil,
@Assisted AttentionSetSender sender,
@Assisted Context ctx,
@Assisted Change change,
@Assisted String reason,
- @Assisted MessageIdGenerator.MessageId messageId,
@Assisted Account.Id attentionUserId) {
this.sendEmailsExecutor = executor;
- this.accountTemplateUtil = accountTemplateUtil;
- this.sender = sender;
- this.ctx = ctx;
- this.change = change;
- this.reason = reason;
- this.messageId = messageId;
- this.attentionUserId = attentionUserId;
+
+ MessageId messageId;
+ try {
+ messageId =
+ messageIdGenerator.fromChangeUpdateAndReason(
+ ctx.getRepoView(), change.currentPatchSetId(), "AttentionSetEmail");
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+
+ this.asyncSender =
+ new AsyncSender(
+ requestContext,
+ ctx.getIdentifiedUser(),
+ sender,
+ messageId,
+ ctx.getNotify(change.getId()),
+ attentionUserId,
+ accountTemplateUtil.replaceTemplates(reason),
+ change.getId());
}
public void sendAsync() {
@SuppressWarnings("unused")
- Future<?> possiblyIgnoredError = sendEmailsExecutor.submit(this);
+ Future<?> possiblyIgnoredError = sendEmailsExecutor.submit(asyncSender);
}
- @Override
- public void run() {
- try {
- AccountState accountState =
- ctx.getUser().isIdentifiedUser() ? ctx.getUser().asIdentifiedUser().state() : null;
- if (accountState != null) {
- sender.setFrom(accountState.account().id());
- }
- sender.setNotify(ctx.getNotify(change.getId()));
- sender.setAttentionSetUser(attentionUserId);
- sender.setReason(accountTemplateUtil.replaceTemplates(reason));
- sender.setMessageId(messageId);
- sender.send();
- } catch (Exception e) {
- logger.atSevere().withCause(e).log("Cannot email update for change %s", change.getId());
+ /**
+ * {@link Runnable} that sends the email asynchonously.
+ *
+ * <p>Only pass objects into this class that are thread-safe (e.g. immutable) so that they can be
+ * safely accessed from the background thread.
+ */
+ private static class AsyncSender implements Runnable, RequestContext {
+ private final ThreadLocalRequestContext requestContext;
+ private final IdentifiedUser user;
+ private final AttentionSetSender sender;
+ private final MessageIdGenerator.MessageId messageId;
+ private final NotifyResolver.Result notify;
+ private final Account.Id attentionUserId;
+ private final String reason;
+ private final Change.Id changeId;
+
+ AsyncSender(
+ ThreadLocalRequestContext requestContext,
+ IdentifiedUser user,
+ AttentionSetSender sender,
+ MessageIdGenerator.MessageId messageId,
+ NotifyResolver.Result notify,
+ Account.Id attentionUserId,
+ String reason,
+ Change.Id changeId) {
+ this.requestContext = requestContext;
+ this.user = user;
+ this.sender = sender;
+ this.messageId = messageId;
+ this.notify = notify;
+ this.attentionUserId = attentionUserId;
+ this.reason = reason;
+ this.changeId = changeId;
}
- }
- @Override
- public String toString() {
- return "send-email comments";
- }
+ @Override
+ public void run() {
+ RequestContext old = requestContext.setContext(this);
+ try {
+ Optional<Account.Id> accountId =
+ user.isIdentifiedUser()
+ ? Optional.of(user.asIdentifiedUser().getAccountId())
+ : Optional.empty();
+ if (accountId.isPresent()) {
+ sender.setFrom(accountId.get());
+ }
+ sender.setNotify(notify);
+ sender.setAttentionSetUser(attentionUserId);
+ sender.setReason(reason);
+ sender.setMessageId(messageId);
+ sender.send();
+ } catch (Exception e) {
+ logger.atSevere().withCause(e).log("Cannot email update for change %s", changeId);
+ } finally {
+ requestContext.setContext(old);
+ }
+ }
- @Override
- public CurrentUser getUser() {
- return ctx.getUser();
+ @Override
+ public String toString() {
+ return "send-email attention-set-update";
+ }
+
+ @Override
+ public CurrentUser getUser() {
+ return user;
+ }
}
}
diff --git a/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java b/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
index cac376f..7386a03 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
@@ -32,10 +32,12 @@
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.index.IndexDefinition;
+import com.google.gerrit.index.Schema;
import com.google.gerrit.launcher.GerritLauncher;
import com.google.gerrit.server.index.GerritIndexStatus;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.inject.Injector;
import com.google.inject.Key;
@@ -48,6 +50,7 @@
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
+import org.junit.Assume;
import org.junit.Test;
@NoHttpd
@@ -175,7 +178,9 @@
@Test
public void onlineUpgradeChanges() throws Exception {
- int prevVersion = ChangeSchemaDefinitions.INSTANCE.getPrevious().getVersion();
+ Schema<ChangeData> previous = ChangeSchemaDefinitions.INSTANCE.getPrevious();
+ Assume.assumeNotNull(previous);
+ int prevVersion = previous.getVersion();
int currVersion = ChangeSchemaDefinitions.INSTANCE.getLatest().getVersion();
// Before storing any changes, switch back to the previous version.
diff --git a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
index fc56a3c..07abae9 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
@@ -28,11 +28,10 @@
@Ignore
public class FakeChangeIndex implements ChangeIndex {
- static final Schema<ChangeData> V1 = new Schema<>(1, false, ImmutableList.of(ChangeField.STATUS));
+ static final Schema<ChangeData> V1 = new Schema<>(1, ImmutableList.of(ChangeField.STATUS));
static final Schema<ChangeData> V2 =
- new Schema<>(
- 2, false, ImmutableList.of(ChangeField.STATUS, ChangeField.PATH, ChangeField.UPDATED));
+ new Schema<>(2, ImmutableList.of(ChangeField.STATUS, ChangeField.PATH, ChangeField.UPDATED));
private static class Source implements ChangeDataSource {
private final Predicate<ChangeData> p;
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index 16f7199..9a2499a 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -45,7 +45,6 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
@@ -269,19 +268,7 @@
AccountInfo user2 = newAccount("user");
requestContext.setContext(newRequestContext(Account.id(user2._accountId)));
- if (getSchemaVersion() < 5) {
- assertMissingField(AccountField.PREFERRED_EMAIL);
- assertFailingQuery("email:foo", "'email' operator is not supported by account index version");
- return;
- }
-
- // This at least needs the PREFERRED_EMAIL field which is available from schema version 5.
- if (getSchemaVersion() >= 5) {
- assertQuery(preferredEmail, user1);
- } else {
- assertQuery(preferredEmail);
- }
-
+ assertQuery(preferredEmail, user1);
assertQuery(secondaryEmail);
assertQuery("email:" + preferredEmail, user1);
@@ -369,14 +356,6 @@
assertQuery("self", user3);
assertQuery("me", user3);
- if (getSchemaVersion() < 8) {
- assertMissingField(AccountField.NAME_PART_NO_SECONDARY_EMAIL);
-
- // prefix queries only work if the NAME_PART_NO_SECONDARY_EMAIL field is available
- assertQuery("john");
- return;
- }
-
assertQuery("John", user1);
assertQuery("john", user1);
assertQuery("Doe", user1);
@@ -649,18 +628,13 @@
IndexConfig.createDefault(), 0, 1, schema.getStoredFields().keySet()));
assertThat(rawFields).isPresent();
- if (schema.useLegacyNumericFields()) {
+ if (schema.hasField(AccountField.ID)) {
assertThat(rawFields.get().getValue(AccountField.ID)).isEqualTo(userInfo._accountId);
} else {
assertThat(Integer.valueOf(rawFields.get().getValue(AccountField.ID_STR)))
.isEqualTo(userInfo._accountId);
}
- // The field EXTERNAL_ID_STATE is only supported from schema version 6.
- if (getSchemaVersion() < 6) {
- return;
- }
-
List<AccountExternalIdInfo> externalIdInfos = gApi.accounts().self().getExternalIds();
List<ByteArrayWrapper> blobs = new ArrayList<>();
for (AccountExternalIdInfo info : externalIdInfos) {
@@ -876,12 +850,6 @@
return accounts.stream().map(a -> a._accountId).collect(toList());
}
- protected void assertMissingField(FieldDef<AccountState, ?> field) {
- assertWithMessage("schema %s has field %s", getSchemaVersion(), field.getName())
- .that(getSchema().hasField(field))
- .isFalse();
- }
-
protected void assertFailingQuery(String query, String expectedMessage) throws Exception {
try {
assertQuery(query);
@@ -891,14 +859,6 @@
}
}
- protected int getSchemaVersion() {
- return getSchema().getVersion();
- }
-
- protected Schema<AccountState> getSchema() {
- return indexes.getSearchIndex().getSchema();
- }
-
/** Boiler plate code to check two byte arrays for equality */
private static class ByteArrayWrapper {
private byte[] arr;
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 6d9f916..baa4802 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -91,7 +91,6 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.httpd.raw.IndexPreloadingUtil;
-import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.IndexPredicate;
@@ -176,7 +175,7 @@
@Inject protected AllUsersName allUsersName;
@Inject protected BatchUpdate.Factory updateFactory;
@Inject protected ChangeInserter.Factory changeFactory;
- @Inject protected Provider<ChangeQueryBuilder> queryBuilderProvider;
+ @Inject protected ChangeQueryBuilder queryBuilder;
@Inject protected GerritApi gApi;
@Inject protected IdentifiedUser.GenericFactory userFactory;
@Inject protected ChangeIndexCollection indexes;
@@ -650,7 +649,6 @@
@Test
public void byAuthorExact() throws Exception {
- assume().that(getSchema().hasField(ChangeField.EXACT_AUTHOR)).isTrue();
byAuthorOrCommitterExact("author:");
}
@@ -661,7 +659,6 @@
@Test
public void byCommitterExact() throws Exception {
- assume().that(getSchema().hasField(ChangeField.EXACT_COMMITTER)).isTrue();
byAuthorOrCommitterExact("committer:");
}
@@ -1607,11 +1604,9 @@
assertQuery("ext:.jAvA", change4);
assertQuery("ext:cc", change3, change2, change1);
- if (getSchemaVersion() >= 56) {
- // matching changes with files that have no extension is possible
- assertQuery("ext:\"\"", change5, change4);
- assertFailingQuery("ext:");
- }
+ // matching changes with files that have no extension is possible
+ assertQuery("ext:\"\"", change5, change4);
+ assertFailingQuery("ext:");
}
@Test
@@ -1975,21 +1970,6 @@
}
@Test
- public void mergedOperatorSupportedByIndexVersion() throws Exception {
- if (getSchemaVersion() < 61) {
- assertMissingField(ChangeField.MERGED_ON);
- assertFailingQuery(
- "mergedbefore:2009-10-01",
- "'mergedbefore' operator is not supported by change index version");
- assertFailingQuery(
- "mergedafter:2009-10-01",
- "'mergedafter' operator is not supported by change index version");
- } else {
- assertThat(getSchema().hasField(ChangeField.MERGED_ON)).isTrue();
- }
- }
-
- @Test
public void byMergedBefore() throws Exception {
assume().that(getSchema().hasField(ChangeField.MERGED_ON)).isTrue();
long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS);
@@ -2473,10 +2453,6 @@
@Test
public void bySubmitRuleResult() throws Exception {
- if (getSchemaVersion() < 68) {
- assertMissingField(ChangeField.SUBMIT_RULE_RESULT);
- return;
- }
try (Registration registration =
extensionRegistry.newRegistration().add(new FakeSubmitRule())) {
TestRepository<Repo> repo = createProject("repo");
@@ -2497,13 +2473,6 @@
@Test
public void byNonExistingSubmitRule_returnsEmpty() throws Exception {
- // Some submit rules could be removed from the gerrit.config but there can be records for
- // merged changes in NoteDb for these rules. We allow querying for non-existent rules to handle
- // this case.
- if (getSchemaVersion() < 68) {
- assertMissingField(ChangeField.SUBMIT_RULE_RESULT);
- return;
- }
try (Registration registration =
extensionRegistry.newRegistration().add(new FakeSubmitRule())) {
TestRepository<Repo> repo = createProject("repo");
@@ -4112,7 +4081,6 @@
assertQuery(ChangeIndexPredicate.none());
- ChangeQueryBuilder queryBuilder = queryBuilderProvider.get();
for (Predicate<ChangeData> matchingOneChange :
ImmutableList.of(
// One index query, one post-filtering query.
@@ -4482,12 +4450,6 @@
}
}
- protected void assertMissingField(FieldDef<ChangeData, ?> field) {
- assertWithMessage("schema %s has field %s", getSchemaVersion(), field.getName())
- .that(getSchema().hasField(field))
- .isFalse();
- }
-
protected void assertFailingQuery(String query) throws Exception {
assertFailingQuery(query, null);
}
@@ -4504,10 +4466,6 @@
}
}
- protected int getSchemaVersion() {
- return getSchema().getVersion();
- }
-
protected Schema<ChangeData> getSchema() {
return indexes.getSearchIndex().getSchema();
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
index eb2a71d..77c9382 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
@@ -24,6 +24,7 @@
import {getAppContext} from '../../../services/app-context';
import {fontStyles} from '../../../styles/gr-font-styles';
import {queryAndAssert} from '../../../utils/common-util';
+import '@polymer/iron-icon/iron-icon';
import {
LabelNameToValuesMap,
ReviewInput,
@@ -36,6 +37,7 @@
import '../../change/gr-label-score-row/gr-label-score-row';
import {getOverallStatus} from '../../../utils/bulk-flow-util';
import {allSettled} from '../../../utils/async-util';
+import {pluralize} from '../../../utils/string-util';
@customElement('gr-change-list-bulk-vote-flow')
export class GrChangeListBulkVoteFlow extends LitElement {
@@ -85,6 +87,20 @@
margin-bottom: var(--spacing-m);
font-weight: var(--font-weight-h2);
}
+ .error-container {
+ background-color: var(--red-50);
+ margin-top: var(--spacing-l);
+ }
+ .error-container iron-icon {
+ padding: 10px var(--spacing-xl);
+ color: var(--red-700);
+ --iron-icon-height: 20px;
+ --iron-icon-width: 20px;
+ }
+ .error-container span {
+ position: relative;
+ top: 1px;
+ }
`,
];
}
@@ -145,13 +161,33 @@
'Trigger Votes',
permittedLabels
)}
+ ${this.renderErrors()}
</div>
- <!-- TODO: Add error handling status if something fails -->
</gr-dialog>
</gr-overlay>
`;
}
+ private renderErrors() {
+ if (getOverallStatus(this.progressByChange) !== ProgressStatus.FAILED) {
+ return nothing;
+ }
+ return html`
+ <div class="error-container">
+ <iron-icon icon="gr-icons:error"></iron-icon>
+ <span>
+ <!-- prettier-ignore -->
+ Failed to vote on ${pluralize(
+ Array.from(this.progressByChange.values()).filter(
+ status => status === ProgressStatus.FAILED
+ ).length,
+ 'change'
+ )}
+ </span>
+ </div>
+ `;
+ }
+
private renderLabels(
labels: Label[],
heading: string,
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts
index 1372eb8..e2cbaf0 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts
@@ -174,6 +174,75 @@
</gr-overlay> `);
});
+ test('renders with errors', async () => {
+ const changes: ChangeInfo[] = [change1];
+ getChangesStub.returns(Promise.resolve(changes));
+ model.sync(changes);
+ await waitUntilObserved(
+ model.loadingState$,
+ state => state === LoadingState.LOADED
+ );
+ stubRestApi('saveChangeReview').callsFake(
+ (_changeNum, _patchNum, _review, errFn) =>
+ Promise.resolve(new Response()).then(res => {
+ errFn && errFn();
+ return res;
+ })
+ );
+ await selectChange(change1);
+ await element.updateComplete;
+
+ queryAndAssert<GrButton>(query(element, 'gr-dialog'), '#confirm').click();
+
+ await waitUntil(
+ () =>
+ element.progressByChange.get(1 as NumericChangeId) ===
+ ProgressStatus.FAILED
+ );
+
+ expect(element).shadowDom.to.equal(/* HTML */ `<gr-button
+ aria-disabled="false"
+ flatten=""
+ id="voteFlowButton"
+ role="button"
+ tabindex="0"
+ >
+ Vote
+ </gr-button>
+ <gr-overlay
+ aria-hidden="true"
+ id="actionOverlay"
+ style="outline: none; display: none;"
+ tabindex="-1"
+ with-backdrop=""
+ >
+ <gr-dialog role="dialog">
+ <div slot="header">
+ <span class="main-heading"> Vote on selected changes </span>
+ </div>
+ <div slot="main">
+ <div class="newSubmitRequirements scoresTable">
+ <h3 class="heading-4 vote-type">Submit requirements votes</h3>
+ <gr-label-score-row name="A"> </gr-label-score-row>
+ <gr-label-score-row name="B"> </gr-label-score-row>
+ <gr-label-score-row name="C"> </gr-label-score-row>
+ <gr-label-score-row name="change1OnlyLabelD">
+ </gr-label-score-row>
+ </div>
+ <div class="newSubmitRequirements scoresTable">
+ <h3 class="heading-4 vote-type">Trigger Votes</h3>
+ <gr-label-score-row name="change1OnlyTriggerLabelE">
+ </gr-label-score-row>
+ </div>
+ <div class="error-container">
+ <iron-icon icon="gr-icons:error"> </iron-icon>
+ <span> Failed to vote on 1 change </span>
+ </div>
+ </div>
+ </gr-dialog>
+ </gr-overlay> `);
+ });
+
test('button state updates as changes are updated', async () => {
const changes: ChangeInfo[] = [change1];
getChangesStub.returns(Promise.resolve(changes));
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 7e4cf2c..105c0d8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -944,6 +944,8 @@
assertIsDefined(this._change, '_change');
if (!this._changeNum)
throw new Error('missing required changeNum property');
+ // to prevent 2 requests at the same time
+ if (this.$.commitMessageEditor.disabled) return;
// Trim trailing whitespace from each line.
const message = e.detail.content.replace(TRAILING_WHITESPACE_REGEX, '');
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 80cf8a4..45a6530 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -1529,7 +1529,7 @@
);
});
- test('_handleCommitMessageSave trims trailing whitespace', () => {
+ test('_handleCommitMessageSave trims trailing whitespace', async () => {
element._change = createChangeViewChange();
// Response code is 500, because we want to avoid window reloading
const putStub = stubRestApi('putChangeCommitMessage').returns(
@@ -1541,10 +1541,10 @@
element._handleCommitMessageSave(mockEvent('test \n test '));
assert.equal(putStub.lastCall.args[1], 'test\n test');
-
+ element.$.commitMessageEditor.disabled = false;
element._handleCommitMessageSave(mockEvent(' test\ntest'));
assert.equal(putStub.lastCall.args[1], ' test\ntest');
-
+ element.$.commitMessageEditor.disabled = false;
element._handleCommitMessageSave(mockEvent('\n\n\n\n\n\n\n\n'));
assert.equal(putStub.lastCall.args[1], '\n\n\n\n\n\n\n\n');
});
