Merge "Custom git gc-preserve command which can preserve packs for JGit"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 3e82254..8d30db2 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -561,7 +561,8 @@
+
To enable the actual usage of contributor agreement the project
specific config option in the `project.config` must be set:
-link:config-project-config.html[receive.requireContributorAgreement].
+link:config-project-config.html#receive.requireContributorAgreement[
+receive.requireContributorAgreement].
[[auth.trustContainerAuth]]auth.trustContainerAuth::
+
diff --git a/Documentation/user-attention-set.txt b/Documentation/user-attention-set.txt
index c4b8aa7..5e5d3f8 100644
--- a/Documentation/user-attention-set.txt
+++ b/Documentation/user-attention-set.txt
@@ -146,8 +146,14 @@
image::images/browser-notification-preference.png["user preference for browser notifications", align="center"]
-Current implementation works only when gerrit is open in one of the tabs. We
-poll every 5 minutes for changes in attention set.
+The notifications work only when Gerrit is open in one of the browser tabs.
+The latency to get the notification is up to 5 minutes.
+
+If you are not getting notifications:
+ - Check your user preferences - Allow browser notification setting
+ - Make sure notifications are turned on for the Gerrit site in the browser
+ - Make sure browser notifications are turned on in your operating system
+ - Your host can have browser notifications disabled for some user groups
=== Bold Changes / Mark Reviewed
diff --git a/java/com/google/gerrit/index/SchemaUtil.java b/java/com/google/gerrit/index/SchemaUtil.java
index 8f47cf5..079f8be 100644
--- a/java/com/google/gerrit/index/SchemaUtil.java
+++ b/java/com/google/gerrit/index/SchemaUtil.java
@@ -103,6 +103,17 @@
}
public static <V> Schema<V> schema(
+ int version,
+ ImmutableList<IndexedField<V, ?>> indexedFields,
+ ImmutableList<IndexedField<V, ?>.SearchSpec> searchSpecs) {
+ return new Schema.Builder<V>()
+ .version(version)
+ .addIndexedFields(indexedFields)
+ .addSearchSpecs(searchSpecs)
+ .build();
+ }
+
+ public static <V> Schema<V> schema(
Schema<V> schema,
ImmutableList<FieldDef<V, ?>> fieldDefs,
ImmutableList<IndexedField<V, ?>> indexedFields,
@@ -116,6 +127,17 @@
}
public static <V> Schema<V> schema(
+ Schema<V> schema,
+ ImmutableList<IndexedField<V, ?>> indexedFields,
+ ImmutableList<IndexedField<V, ?>.SearchSpec> searchSpecs) {
+ return new Schema.Builder<V>()
+ .add(schema)
+ .addIndexedFields(indexedFields)
+ .addSearchSpecs(searchSpecs)
+ .build();
+ }
+
+ public static <V> Schema<V> schema(
ImmutableList<FieldDef<V, ?>> fieldDefs,
ImmutableList<IndexedField<V, ?>> indexFields,
ImmutableList<IndexedField<V, ?>.SearchSpec> searchSpecs) {
diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
index 5838ff1..d60be14 100644
--- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
+++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
@@ -268,7 +268,8 @@
ChangeData cd =
changeDataFactory.create(
Project.nameKey((String) doc.get(ChangeField.PROJECT_SPEC.getName())),
- Change.id(Integer.valueOf((String) doc.get(ChangeField.LEGACY_ID_STR.getName()))));
+ Change.id(
+ Integer.valueOf((String) doc.get(ChangeField.NUMERIC_ID_STR_SPEC.getName()))));
for (SchemaField<ChangeData, ?> field : getSchema().getSchemaFields().values()) {
boolean isProtoField = SchemaFieldDefs.isProtoField(field);
field.setIfPossible(cd, new FakeStoredValue(doc.get(field.getName()), isProtoField));
diff --git a/java/com/google/gerrit/lucene/ChangeSubIndex.java b/java/com/google/gerrit/lucene/ChangeSubIndex.java
index f7b1f2c..84cc83a 100644
--- a/java/com/google/gerrit/lucene/ChangeSubIndex.java
+++ b/java/com/google/gerrit/lucene/ChangeSubIndex.java
@@ -119,10 +119,10 @@
void add(Document doc, Values<ChangeData> values) {
// Add separate DocValues fields for those fields needed for sorting.
SchemaField<ChangeData, ?> f = values.getField();
- if (f == ChangeField.LEGACY_ID_STR) {
+ if (f == ChangeField.NUMERIC_ID_STR_SPEC) {
String v = (String) getOnlyElement(values.getValues());
doc.add(new NumericDocValuesField(ID_STR_SORT_FIELD, Integer.valueOf(v)));
- } else if (f == ChangeField.UPDATED) {
+ } else if (f == ChangeField.UPDATED_SPEC) {
long t = ((Timestamp) getOnlyElement(values.getValues())).getTime();
doc.add(new NumericDocValuesField(UPDATED_SORT_FIELD, t));
} else if (f == ChangeField.MERGED_ON_SPEC) {
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index de4b26f..e16adf5 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -17,7 +17,7 @@
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_STR;
+import static com.google.gerrit.server.index.change.ChangeField.NUMERIC_ID_STR_SPEC;
import static com.google.gerrit.server.index.change.ChangeField.PROJECT_SPEC;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.OPEN_STATUSES;
@@ -103,21 +103,21 @@
public class LuceneChangeIndex implements ChangeIndex {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- static final String UPDATED_SORT_FIELD = sortFieldName(ChangeField.UPDATED);
+ static final String UPDATED_SORT_FIELD = sortFieldName(ChangeField.UPDATED_SPEC);
static final String MERGED_ON_SORT_FIELD = sortFieldName(ChangeField.MERGED_ON_SPEC);
- static final String ID_STR_SORT_FIELD = sortFieldName(ChangeField.LEGACY_ID_STR);
+ static final String ID_STR_SORT_FIELD = sortFieldName(ChangeField.NUMERIC_ID_STR_SPEC);
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();
+ private static final String CHANGE_FIELD = ChangeField.CHANGE_SPEC.getName();
static Term idTerm(ChangeData cd) {
return idTerm(cd.getId());
}
static Term idTerm(Change.Id id) {
- return QueryBuilder.stringTerm(LEGACY_ID_STR.getName(), Integer.toString(id.get()));
+ return QueryBuilder.stringTerm(NUMERIC_ID_STR_SPEC.getName(), Integer.toString(id.get()));
}
private final ListeningExecutorService executor;
@@ -501,7 +501,7 @@
ImmutableList.Builder<ChangeData> result =
ImmutableList.builderWithExpectedSize(docs.size());
for (Document doc : docs) {
- result.add(toChangeData(fields(doc, fields), fields, LEGACY_ID_STR.getName()));
+ result.add(toChangeData(fields(doc, fields), fields, NUMERIC_ID_STR_SPEC.getName()));
}
return result.build();
} catch (InterruptedException e) {
diff --git a/java/com/google/gerrit/server/StarredChangesUtil.java b/java/com/google/gerrit/server/StarredChangesUtil.java
index 0c8bccd..fd08fa8 100644
--- a/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -312,7 +312,7 @@
List<ChangeData> changeData =
queryProvider
.get()
- .setRequestedFields(ChangeField.ID, ChangeField.STAR_SPEC)
+ .setRequestedFields(ChangeField.CHANGE_ID_SPEC, ChangeField.STAR_SPEC)
.byLegacyChangeId(changeId);
if (changeData.size() != 1) {
throw new NoSuchChangeException(changeId);
diff --git a/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java b/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java
index 727aab3..cfeec70 100644
--- a/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java
+++ b/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java
@@ -162,7 +162,7 @@
List<ChangeData> cds =
queryProvider
.get()
- .setRequestedFields(ChangeField.CHANGE, ChangeField.REVIEWER_SPEC)
+ .setRequestedFields(ChangeField.CHANGE_SPEC, ChangeField.REVIEWER_SPEC)
.byProject(key);
Map<Change.Id, CachedChange> result = new HashMap<>(cds.size());
for (ChangeData cd : cds) {
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHook.java b/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHook.java
index c06622b..7c22bd8 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHook.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHook.java
@@ -111,10 +111,10 @@
.get()
.setRequestedFields(
// Required for ChangeIsVisibleToPrdicate.
- ChangeField.CHANGE,
+ ChangeField.CHANGE_SPEC,
ChangeField.REVIEWER_SPEC,
// Required during advertiseOpenChanges.
- ChangeField.PATCH_SET)
+ ChangeField.PATCH_SET_SPEC)
.enforceVisibility(true)
.setLimit(limit)
.query(
diff --git a/java/com/google/gerrit/server/index/IndexUtils.java b/java/com/google/gerrit/server/index/IndexUtils.java
index 7d40c00..213094e 100644
--- a/java/com/google/gerrit/server/index/IndexUtils.java
+++ b/java/com/google/gerrit/server/index/IndexUtils.java
@@ -14,8 +14,8 @@
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_STR;
+import static com.google.gerrit.server.index.change.ChangeField.CHANGE_SPEC;
+import static com.google.gerrit.server.index.change.ChangeField.NUMERIC_ID_STR_SPEC;
import static com.google.gerrit.server.index.change.ChangeField.PROJECT_SPEC;
import com.google.common.collect.ImmutableSet;
@@ -77,14 +77,14 @@
// change ID and project, which can either come via the Change field or
// separate fields.
Set<String> fs = opts.fields();
- if (fs.contains(CHANGE.getName())) {
+ if (fs.contains(CHANGE_SPEC.getName())) {
// A Change is always sufficient.
return fs;
}
- if (fs.contains(PROJECT_SPEC.getName()) && fs.contains(LEGACY_ID_STR.getName())) {
+ if (fs.contains(PROJECT_SPEC.getName()) && fs.contains(NUMERIC_ID_STR_SPEC.getName())) {
return fs;
}
- return Sets.union(fs, ImmutableSet.of(LEGACY_ID_STR.getName(), PROJECT_SPEC.getName()));
+ return Sets.union(fs, ImmutableSet.of(NUMERIC_ID_STR_SPEC.getName(), PROJECT_SPEC.getName()));
}
/**
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index eb4f6bf..254a7e5 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -18,10 +18,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static com.google.gerrit.index.FieldDef.exact;
-import static com.google.gerrit.index.FieldDef.prefix;
-import static com.google.gerrit.index.FieldDef.storedOnly;
-import static com.google.gerrit.index.FieldDef.timestamp;
import static com.google.gerrit.server.util.AttentionSetUtil.additionsOnly;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
@@ -41,6 +37,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.common.io.Files;
import com.google.common.primitives.Longs;
+import com.google.common.reflect.TypeToken;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Address;
@@ -58,16 +55,16 @@
import com.google.gerrit.entities.converter.PatchSetApprovalProtoConverter;
import com.google.gerrit.entities.converter.PatchSetProtoConverter;
import com.google.gerrit.entities.converter.ProtoConverter;
-import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexedField;
import com.google.gerrit.index.RefState;
import com.google.gerrit.index.SchemaFieldDefs;
import com.google.gerrit.index.SchemaUtil;
import com.google.gerrit.json.OutputFormat;
-import com.google.gerrit.proto.Protos;
+import com.google.gerrit.proto.Entities;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.StarredChangesUtil;
+import com.google.gerrit.server.cache.proto.Cache;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -125,12 +122,27 @@
// TODO: Rename LEGACY_ID to NUMERIC_ID
/** Legacy change ID. */
- public static final FieldDef<ChangeData, String> LEGACY_ID_STR =
- exact("legacy_id_str").stored().build(cd -> String.valueOf(cd.getId().get()));
+ public static final IndexedField<ChangeData, String> NUMERIC_ID_STR_FIELD =
+ IndexedField.<ChangeData>stringBuilder("NumericIdStr")
+ .stored()
+ .required()
+ // The numeric change id is integer in string form
+ .size(10)
+ .build(cd -> String.valueOf(cd.getId().get()));
+
+ public static final IndexedField<ChangeData, String>.SearchSpec NUMERIC_ID_STR_SPEC =
+ NUMERIC_ID_STR_FIELD.exact("legacy_id_str");
/** Newer style Change-Id key. */
- public static final FieldDef<ChangeData, String> ID =
- prefix(ChangeQueryBuilder.FIELD_CHANGE_ID).build(changeGetter(c -> c.getKey().get()));
+ public static final IndexedField<ChangeData, String> CHANGE_ID_FIELD =
+ IndexedField.<ChangeData>stringBuilder("ChangeId")
+ .required()
+ // The new style key is in form Isha1
+ .size(41)
+ .build(changeGetter(c -> c.getKey().get()));
+
+ public static final IndexedField<ChangeData, String>.SearchSpec CHANGE_ID_SPEC =
+ CHANGE_ID_FIELD.prefix(ChangeQueryBuilder.FIELD_CHANGE_ID);
/** Change status string, in the same format as {@code status:}. */
public static final IndexedField<ChangeData, String> STATUS_FIELD =
@@ -193,11 +205,14 @@
/** Last update time since January 1, 1970. */
// TODO(issue-15518): Migrate type for timestamp index fields from Timestamp to Instant
- public static final FieldDef<ChangeData, Timestamp> UPDATED =
- timestamp("updated2")
+ public static final IndexedField<ChangeData, Timestamp> UPDATED_FIELD =
+ IndexedField.<ChangeData>timestampBuilder("LastUpdated")
.stored()
.build(changeGetter(change -> Timestamp.from(change.getLastUpdatedOn())));
+ public static final IndexedField<ChangeData, Timestamp>.SearchSpec UPDATED_SPEC =
+ UPDATED_FIELD.timestamp("updated2");
+
/** When this change was merged, time since January 1, 1970. */
// TODO(issue-15518): Migrate type for timestamp index fields from Timestamp to Instant
public static final IndexedField<ChangeData, Timestamp> MERGED_ON_FIELD =
@@ -972,20 +987,45 @@
EXACT_COMMITTER_FIELD.exact(ChangeQueryBuilder.FIELD_EXACTCOMMITTER);
/** Serialized change object, used for pre-populating results. */
- public static final FieldDef<ChangeData, byte[]> CHANGE =
- storedOnly("_change")
+ private static final TypeToken<Entities.Change> CHANGE_TYPE_TOKEN =
+ new TypeToken<Entities.Change>() {
+ private static final long serialVersionUID = 1L;
+ };
+
+ public static final IndexedField<ChangeData, Entities.Change> CHANGE_FIELD =
+ IndexedField.<ChangeData, Entities.Change>builder("Change", CHANGE_TYPE_TOKEN)
+ .stored()
+ .required()
+ .protoConverter(Optional.of(ChangeProtoConverter.INSTANCE))
.build(
- changeGetter(change -> toProto(ChangeProtoConverter.INSTANCE, change)),
- (cd, field) -> cd.setChange(parseProtoFrom(field, ChangeProtoConverter.INSTANCE)));
+ changeGetter(change -> entityToProto(ChangeProtoConverter.INSTANCE, change)),
+ (cd, value) ->
+ cd.setChange(decodeProtoToEntity(value, ChangeProtoConverter.INSTANCE)));
+
+ public static final IndexedField<ChangeData, Entities.Change>.SearchSpec CHANGE_SPEC =
+ CHANGE_FIELD.storedOnly("_change");
/** Serialized approvals for the current patch set, used for pre-populating results. */
- public static final FieldDef<ChangeData, Iterable<byte[]>> APPROVAL =
- storedOnly("_approval")
- .buildRepeatable(
- cd -> toProtos(PatchSetApprovalProtoConverter.INSTANCE, cd.currentApprovals()),
+ private static final TypeToken<Iterable<Entities.PatchSetApproval>> APPROVAL_TYPE_TOKEN =
+ new TypeToken<Iterable<Entities.PatchSetApproval>>() {
+ private static final long serialVersionUID = 1L;
+ };
+
+ public static final IndexedField<ChangeData, Iterable<Entities.PatchSetApproval>> APPROVAL_FIELD =
+ IndexedField.<ChangeData, Iterable<Entities.PatchSetApproval>>builder(
+ "Approval", APPROVAL_TYPE_TOKEN)
+ .stored()
+ .required()
+ .protoConverter(Optional.of(PatchSetApprovalProtoConverter.INSTANCE))
+ .build(
+ cd ->
+ entitiesToProtos(PatchSetApprovalProtoConverter.INSTANCE, cd.currentApprovals()),
(cd, field) ->
cd.setCurrentApprovals(
- decodeProtos(field, PatchSetApprovalProtoConverter.INSTANCE)));
+ decodeProtosToEntities(field, PatchSetApprovalProtoConverter.INSTANCE)));
+
+ public static final IndexedField<ChangeData, Iterable<Entities.PatchSetApproval>>.SearchSpec
+ APPROVAL_SPEC = APPROVAL_FIELD.storedOnly("_approval");
public static String formatLabel(String label, int value) {
return formatLabel(label, value, /* accountId= */ null, /* count= */ null);
@@ -1256,11 +1296,24 @@
GROUP_FIELD.exact(ChangeQueryBuilder.FIELD_GROUP);
/** Serialized patch set object, used for pre-populating results. */
- public static final FieldDef<ChangeData, Iterable<byte[]>> PATCH_SET =
- storedOnly("_patch_set")
- .buildRepeatable(
- cd -> toProtos(PatchSetProtoConverter.INSTANCE, cd.patchSets()),
- (cd, field) -> cd.setPatchSets(decodeProtos(field, PatchSetProtoConverter.INSTANCE)));
+ private static final TypeToken<Iterable<Entities.PatchSet>> PATCH_SET_TYPE_TOKEN =
+ new TypeToken<Iterable<Entities.PatchSet>>() {
+ private static final long serialVersionUID = 1L;
+ };
+
+ public static final IndexedField<ChangeData, Iterable<Entities.PatchSet>> PATCH_SET_FIELD =
+ IndexedField.<ChangeData, Iterable<Entities.PatchSet>>builder(
+ "PatchSet", PATCH_SET_TYPE_TOKEN)
+ .stored()
+ .required()
+ .protoConverter(Optional.of(PatchSetProtoConverter.INSTANCE))
+ .build(
+ cd -> entitiesToProtos(PatchSetProtoConverter.INSTANCE, cd.patchSets()),
+ (cd, value) ->
+ cd.setPatchSets(decodeProtosToEntities(value, PatchSetProtoConverter.INSTANCE)));
+
+ public static final IndexedField<ChangeData, Iterable<Entities.PatchSet>>.SearchSpec
+ PATCH_SET_SPEC = PATCH_SET_FIELD.storedOnly("_patch_set");
/** Users who have edits on this change. */
public static final IndexedField<ChangeData, Iterable<Integer>> EDITBY_FIELD =
@@ -1317,9 +1370,9 @@
SubmitRuleOptions.builder().build();
/** All submit rules results in the form of "$ruleName,$status". */
- public static final FieldDef<ChangeData, Iterable<String>> SUBMIT_RULE_RESULT =
- exact("submit_rule_result")
- .buildRepeatable(
+ public static final IndexedField<ChangeData, Iterable<String>> SUBMIT_RULE_RESULT_FIELD =
+ IndexedField.<ChangeData>iterableStringBuilder("SubmitRuleResult")
+ .build(
cd -> {
List<String> result = new ArrayList<>();
List<SubmitRecord> submitRecords = cd.submitRecords(SUBMIT_RULE_OPTIONS_STRICT);
@@ -1329,6 +1382,9 @@
return result;
});
+ public static final IndexedField<ChangeData, Iterable<String>>.SearchSpec
+ SUBMIT_RULE_RESULT_SPEC = SUBMIT_RULE_RESULT_FIELD.exact("submit_rule_result");
+
/**
* JSON type for storing SubmitRecords.
*
@@ -1415,12 +1471,17 @@
}
}
- public static final FieldDef<ChangeData, Iterable<String>> SUBMIT_RECORD =
- exact("submit_record").buildRepeatable(ChangeField::formatSubmitRecordValues);
+ public static final IndexedField<ChangeData, Iterable<String>> SUBMIT_RECORD_FIELD =
+ IndexedField.<ChangeData>iterableStringBuilder("SubmitRecord")
+ .build(ChangeField::formatSubmitRecordValues);
- public static final FieldDef<ChangeData, Iterable<byte[]>> STORED_SUBMIT_RECORD_STRICT =
- storedOnly("full_submit_record_strict")
- .buildRepeatable(
+ public static final IndexedField<ChangeData, Iterable<String>>.SearchSpec SUBMIT_RECORD_SPEC =
+ SUBMIT_RECORD_FIELD.exact("submit_record");
+
+ public static final IndexedField<ChangeData, Iterable<byte[]>> STORED_SUBMIT_RECORD_STRICT_FIELD =
+ IndexedField.<ChangeData>iterableByteArrayBuilder("FullSubmitRecordStrict")
+ .stored()
+ .build(
cd -> storedSubmitRecords(cd, SUBMIT_RULE_OPTIONS_STRICT),
(cd, field) ->
parseSubmitRecords(
@@ -1430,17 +1491,27 @@
SUBMIT_RULE_OPTIONS_STRICT,
cd));
- public static final FieldDef<ChangeData, Iterable<byte[]>> STORED_SUBMIT_RECORD_LENIENT =
- storedOnly("full_submit_record_lenient")
- .buildRepeatable(
- cd -> storedSubmitRecords(cd, SUBMIT_RULE_OPTIONS_LENIENT),
- (cd, field) ->
- parseSubmitRecords(
- StreamSupport.stream(field.spliterator(), false)
- .map(f -> new String(f, UTF_8))
- .collect(toSet()),
- SUBMIT_RULE_OPTIONS_LENIENT,
- cd));
+ public static final IndexedField<ChangeData, Iterable<byte[]>>.SearchSpec
+ STORED_SUBMIT_RECORD_STRICT_SPEC =
+ STORED_SUBMIT_RECORD_STRICT_FIELD.storedOnly("full_submit_record_strict");
+
+ public static final IndexedField<ChangeData, Iterable<byte[]>>
+ STORED_SUBMIT_RECORD_LENIENT_FIELD =
+ IndexedField.<ChangeData>iterableByteArrayBuilder("FullSubmitRecordLenient")
+ .stored()
+ .build(
+ cd -> storedSubmitRecords(cd, SUBMIT_RULE_OPTIONS_LENIENT),
+ (cd, field) ->
+ parseSubmitRecords(
+ StreamSupport.stream(field.spliterator(), false)
+ .map(f -> new String(f, UTF_8))
+ .collect(toSet()),
+ SUBMIT_RULE_OPTIONS_LENIENT,
+ cd));
+
+ public static final IndexedField<ChangeData, Iterable<byte[]>>.SearchSpec
+ STORED_SUBMIT_RECORD_LENIENT_SPEC =
+ STORED_SUBMIT_RECORD_LENIENT_FIELD.storedOnly("full_submit_record_lenient");
public static void parseSubmitRecords(
Collection<String> values, SubmitRuleOptions opts, ChangeData out) {
@@ -1545,22 +1616,35 @@
}
/** Serialized submit requirements, used for pre-populating results. */
- public static final FieldDef<ChangeData, Iterable<byte[]>> STORED_SUBMIT_REQUIREMENTS =
- storedOnly("full_submit_requirements")
- .buildRepeatable(
- cd ->
- toProtos(
- SubmitRequirementProtoConverter.INSTANCE, cd.submitRequirements().values()),
- (cd, field) -> parseSubmitRequirements(field, cd));
+ private static final TypeToken<Iterable<Cache.SubmitRequirementResultProto>>
+ STORED_SUBMIT_REQUIREMENTS_TYPE_TOKEN =
+ new TypeToken<Iterable<Cache.SubmitRequirementResultProto>>() {
+ private static final long serialVersionUID = 1L;
+ };
- private static void parseSubmitRequirements(Iterable<byte[]> values, ChangeData out) {
+ public static final IndexedField<ChangeData, Iterable<Cache.SubmitRequirementResultProto>>
+ STORED_SUBMIT_REQUIREMENTS_FIELD =
+ IndexedField.<ChangeData, Iterable<Cache.SubmitRequirementResultProto>>builder(
+ "StoredSubmitRequirements", STORED_SUBMIT_REQUIREMENTS_TYPE_TOKEN)
+ .stored()
+ .required()
+ .protoConverter(Optional.of(SubmitRequirementProtoConverter.INSTANCE))
+ .build(
+ cd ->
+ entitiesToProtos(
+ SubmitRequirementProtoConverter.INSTANCE,
+ cd.submitRequirements().values()),
+ (cd, value) -> parseSubmitRequirements(value, cd));
+
+ public static final IndexedField<ChangeData, Iterable<Cache.SubmitRequirementResultProto>>
+ .SearchSpec
+ STORED_SUBMIT_REQUIREMENTS_SPEC =
+ STORED_SUBMIT_REQUIREMENTS_FIELD.storedOnly("full_submit_requirements");
+
+ private static void parseSubmitRequirements(
+ Iterable<Cache.SubmitRequirementResultProto> values, ChangeData out) {
out.setSubmitRequirements(
- StreamSupport.stream(values.spliterator(), false)
- .map(
- f ->
- SubmitRequirementProtoConverter.INSTANCE.fromProto(
- Protos.parseUnchecked(
- SubmitRequirementProtoConverter.INSTANCE.getParser(), f)))
+ decodeProtosToEntities(values, SubmitRequirementProtoConverter.INSTANCE).stream()
.filter(sr -> !sr.isLegacy())
.collect(
ImmutableMap.toImmutableMap(sr -> sr.submitRequirement(), Function.identity())));
@@ -1571,9 +1655,10 @@
*
* <p>Emitted as UTF-8 encoded strings of the form {@code project:ref/name:[hex sha]}.
*/
- public static final FieldDef<ChangeData, Iterable<byte[]>> REF_STATE =
- storedOnly("ref_state")
- .buildRepeatable(
+ public static final IndexedField<ChangeData, Iterable<byte[]>> REF_STATE_FIELD =
+ IndexedField.<ChangeData>iterableByteArrayBuilder("RefState")
+ .stored()
+ .build(
cd -> {
List<byte[]> result = new ArrayList<>();
cd.getRefStates()
@@ -1583,15 +1668,19 @@
},
(cd, field) -> cd.setRefStates(RefState.parseStates(field)));
+ public static final IndexedField<ChangeData, Iterable<byte[]>>.SearchSpec REF_STATE_SPEC =
+ REF_STATE_FIELD.storedOnly("ref_state");
+
/**
* All ref wildcard patterns that were used in the course of indexing this document.
*
* <p>Emitted as UTF-8 encoded strings of the form {@code project:ref/name/*}. See {@link
* RefStatePattern} for the pattern format.
*/
- public static final FieldDef<ChangeData, Iterable<byte[]>> REF_STATE_PATTERN =
- storedOnly("ref_state_pattern")
- .buildRepeatable(
+ public static final IndexedField<ChangeData, Iterable<byte[]>> REF_STATE_PATTERN_FIELD =
+ IndexedField.<ChangeData>iterableByteArrayBuilder("RefStatePattern")
+ .stored()
+ .build(
cd -> {
Change.Id id = cd.getId();
Project.NameKey project = cd.change().getProject();
@@ -1610,6 +1699,9 @@
},
(cd, field) -> cd.setRefStatePatterns(field));
+ public static final IndexedField<ChangeData, Iterable<byte[]>>.SearchSpec REF_STATE_PATTERN_SPEC =
+ REF_STATE_PATTERN_FIELD.storedOnly("ref_state_pattern");
+
@Nullable
private static String getTopic(ChangeData cd) {
Change c = cd.change();
@@ -1619,24 +1711,28 @@
return firstNonNull(c.getTopic(), "");
}
- private static <T> List<byte[]> toProtos(ProtoConverter<?, T> converter, Collection<T> objects) {
- return objects.stream().map(object -> toProto(converter, object)).collect(toImmutableList());
+ private static <V extends MessageLite, T> V entityToProto(
+ ProtoConverter<V, T> converter, T object) {
+ return converter.toProto(object);
}
- private static <T> byte[] toProto(ProtoConverter<?, T> converter, T object) {
- return Protos.toByteArray(converter.toProto(object));
- }
-
- private static <T> List<T> decodeProtos(Iterable<byte[]> raw, ProtoConverter<?, T> converter) {
- return StreamSupport.stream(raw.spliterator(), false)
- .map(bytes -> parseProtoFrom(bytes, converter))
+ private static <V extends MessageLite, T> List<V> entitiesToProtos(
+ ProtoConverter<V, T> converter, Collection<T> objects) {
+ return objects.stream()
+ .map(object -> entityToProto(converter, object))
.collect(toImmutableList());
}
- private static <P extends MessageLite, T> T parseProtoFrom(
- byte[] bytes, ProtoConverter<P, T> converter) {
- P message = Protos.parseUnchecked(converter.getParser(), bytes, 0, bytes.length);
- return converter.fromProto(message);
+ private static <V extends MessageLite, T> List<T> decodeProtosToEntities(
+ Iterable<V> raw, ProtoConverter<V, T> converter) {
+ return StreamSupport.stream(raw.spliterator(), false)
+ .map(proto -> decodeProtoToEntity(proto, converter))
+ .collect(toImmutableList());
+ }
+
+ private static <V extends MessageLite, T> T decodeProtoToEntity(
+ V proto, ProtoConverter<V, T> converter) {
+ return converter.fromProto(proto);
}
private static <T> SchemaFieldDefs.Getter<ChangeData, T> changeGetter(Function<Change, T> func) {
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 5677b40..5dc1500 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -34,27 +34,17 @@
static final Schema<ChangeData> V74 =
schema(
/* version= */ 74,
- ImmutableList.of(
- ChangeField.APPROVAL,
- ChangeField.CHANGE,
- ChangeField.ID,
- ChangeField.LEGACY_ID_STR,
- ChangeField.PATCH_SET,
- ChangeField.REF_STATE,
- ChangeField.REF_STATE_PATTERN,
- ChangeField.STORED_SUBMIT_RECORD_LENIENT,
- ChangeField.STORED_SUBMIT_RECORD_STRICT,
- ChangeField.STORED_SUBMIT_REQUIREMENTS,
- ChangeField.SUBMIT_RECORD,
- ChangeField.SUBMIT_RULE_RESULT,
- ChangeField.UPDATED),
+ ImmutableList.of(),
ImmutableList.<IndexedField<ChangeData, ?>>of(
ChangeField.ADDED_LINES_FIELD,
+ ChangeField.APPROVAL_FIELD,
ChangeField.ASSIGNEE_FIELD,
ChangeField.ATTENTION_SET_FULL_FIELD,
ChangeField.ATTENTION_SET_USERS_COUNT_FIELD,
ChangeField.ATTENTION_SET_USERS_FIELD,
ChangeField.AUTHOR_PARTS_FIELD,
+ ChangeField.CHANGE_FIELD,
+ ChangeField.CHANGE_ID_FIELD,
ChangeField.CHERRY_PICK_FIELD,
ChangeField.CHERRY_PICK_OF_CHANGE_FIELD,
ChangeField.CHERRY_PICK_OF_PATCHSET_FIELD,
@@ -82,36 +72,49 @@
ChangeField.MERGEABLE_FIELD,
ChangeField.MERGED_ON_FIELD,
ChangeField.MERGE_FIELD,
+ ChangeField.NUMERIC_ID_STR_FIELD,
ChangeField.ONLY_EXTENSIONS_FIELD,
ChangeField.OWNER_FIELD,
+ ChangeField.PATCH_SET_FIELD,
ChangeField.PATH_FIELD,
ChangeField.PENDING_REVIEWER_BY_EMAIL_FIELD,
ChangeField.PENDING_REVIEWER_FIELD,
ChangeField.PRIVATE_FIELD,
ChangeField.PROJECT_FIELD,
ChangeField.REF_FIELD,
+ ChangeField.REF_STATE_FIELD,
+ ChangeField.REF_STATE_PATTERN_FIELD,
ChangeField.REVERT_OF_FIELD,
+ ChangeField.REVIEWEDBY_FIELD,
ChangeField.REVIEWER_BY_EMAIL_FIELD,
ChangeField.REVIEWER_FIELD,
ChangeField.STARBY_FIELD,
ChangeField.STARTED_FIELD,
ChangeField.STAR_FIELD,
ChangeField.STATUS_FIELD,
+ ChangeField.STORED_SUBMIT_RECORD_LENIENT_FIELD,
+ ChangeField.STORED_SUBMIT_RECORD_STRICT_FIELD,
+ ChangeField.STORED_SUBMIT_REQUIREMENTS_FIELD,
ChangeField.SUBMISSIONID_FIELD,
+ ChangeField.SUBMIT_RECORD_FIELD,
+ ChangeField.SUBMIT_RULE_RESULT_FIELD,
ChangeField.TOPIC_FIELD,
ChangeField.TOTAL_COMMENT_COUNT_FIELD,
ChangeField.TR_FIELD,
ChangeField.UNRESOLVED_COMMENT_COUNT_FIELD,
+ ChangeField.UPDATED_FIELD,
ChangeField.UPLOADER_FIELD,
- ChangeField.WIP_FIELD,
- ChangeField.REVIEWEDBY_FIELD),
+ ChangeField.WIP_FIELD),
ImmutableList.<IndexedField<ChangeData, ?>.SearchSpec>of(
ChangeField.ADDED_LINES_SPEC,
+ ChangeField.APPROVAL_SPEC,
ChangeField.ASSIGNEE_SPEC,
ChangeField.ATTENTION_SET_FULL_SPEC,
ChangeField.ATTENTION_SET_USERS,
ChangeField.ATTENTION_SET_USERS_COUNT,
ChangeField.AUTHOR_PARTS_SPEC,
+ ChangeField.CHANGE_ID_SPEC,
+ ChangeField.CHANGE_SPEC,
ChangeField.CHERRY_PICK_OF_CHANGE,
ChangeField.CHERRY_PICK_OF_PATCHSET,
ChangeField.CHERRY_PICK_SPEC,
@@ -143,8 +146,10 @@
ChangeField.MERGEABLE_SPEC,
ChangeField.MERGED_ON_SPEC,
ChangeField.MERGE_SPEC,
+ ChangeField.NUMERIC_ID_STR_SPEC,
ChangeField.ONLY_EXTENSIONS_SPEC,
ChangeField.OWNER_SPEC,
+ ChangeField.PATCH_SET_SPEC,
ChangeField.PATH_SPEC,
ChangeField.PENDING_REVIEWER_BY_EMAIL,
ChangeField.PENDING_REVIEWER_SPEC,
@@ -152,20 +157,28 @@
ChangeField.PROJECTS_SPEC,
ChangeField.PROJECT_SPEC,
ChangeField.REF_SPEC,
+ ChangeField.REF_STATE_PATTERN_SPEC,
+ ChangeField.REF_STATE_SPEC,
ChangeField.REVERT_OF,
+ ChangeField.REVIEWEDBY_SPEC,
ChangeField.REVIEWER_BY_EMAIL,
ChangeField.REVIEWER_SPEC,
ChangeField.STARBY_SPEC,
ChangeField.STARTED_SPEC,
ChangeField.STAR_SPEC,
ChangeField.STATUS_SPEC,
+ ChangeField.STORED_SUBMIT_RECORD_LENIENT_SPEC,
+ ChangeField.STORED_SUBMIT_RECORD_STRICT_SPEC,
+ ChangeField.STORED_SUBMIT_REQUIREMENTS_SPEC,
ChangeField.SUBMISSIONID_SPEC,
+ ChangeField.SUBMIT_RECORD_SPEC,
+ ChangeField.SUBMIT_RULE_RESULT_SPEC,
ChangeField.TOTAL_COMMENT_COUNT_SPEC,
ChangeField.TR_SPEC,
ChangeField.UNRESOLVED_COMMENT_COUNT_SPEC,
+ ChangeField.UPDATED_SPEC,
ChangeField.UPLOADER_SPEC,
- ChangeField.WIP_SPEC,
- ChangeField.REVIEWEDBY_SPEC));
+ ChangeField.WIP_SPEC));
/**
* Added new field {@link ChangeField#PREFIX_HASHTAG} and {@link ChangeField#PREFIX_TOPIC} to
diff --git a/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java b/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
index 76610f3..26477a4 100644
--- a/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
+++ b/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
@@ -15,7 +15,7 @@
package com.google.gerrit.server.index.change;
import static com.google.common.base.Preconditions.checkState;
-import static com.google.gerrit.server.index.change.ChangeField.CHANGE;
+import static com.google.gerrit.server.index.change.ChangeField.CHANGE_SPEC;
import static com.google.gerrit.server.index.change.ChangeField.PROJECT_SPEC;
import com.google.common.annotations.VisibleForTesting;
@@ -69,7 +69,7 @@
int limit,
Set<String> fields) {
// Always include project since it is needed to load the change from NoteDb.
- if (!fields.contains(CHANGE.getName()) && !fields.contains(PROJECT_SPEC.getName())) {
+ if (!fields.contains(CHANGE_SPEC.getName()) && !fields.contains(PROJECT_SPEC.getName())) {
fields = new HashSet<>(fields);
fields.add(PROJECT_SPEC.getName());
}
@@ -176,6 +176,6 @@
@Override
public boolean hasChange() {
- return index.getSchema().hasField(ChangeField.CHANGE);
+ return index.getSchema().hasField(ChangeField.CHANGE_SPEC);
}
}
diff --git a/java/com/google/gerrit/server/index/change/StalenessChecker.java b/java/com/google/gerrit/server/index/change/StalenessChecker.java
index ad5cc2b..eb4af01 100644
--- a/java/com/google/gerrit/server/index/change/StalenessChecker.java
+++ b/java/com/google/gerrit/server/index/change/StalenessChecker.java
@@ -56,9 +56,9 @@
public static final ImmutableSet<String> FIELDS =
ImmutableSet.of(
- ChangeField.CHANGE.getName(),
- ChangeField.REF_STATE.getName(),
- ChangeField.REF_STATE_PATTERN.getName());
+ ChangeField.CHANGE_SPEC.getName(),
+ ChangeField.REF_STATE_SPEC.getName(),
+ ChangeField.REF_STATE_PATTERN_SPEC.getName());
private final ChangeIndexCollection indexes;
private final GitRepositoryManager repoManager;
@@ -82,8 +82,8 @@
return StalenessCheckResult
.notStale(); // No index; caller couldn't do anything if it is stale.
}
- if (!i.getSchema().hasField(ChangeField.REF_STATE)
- || !i.getSchema().hasField(ChangeField.REF_STATE_PATTERN)) {
+ if (!i.getSchema().hasField(ChangeField.REF_STATE_SPEC)
+ || !i.getSchema().hasField(ChangeField.REF_STATE_PATTERN_SPEC)) {
return StalenessCheckResult.notStale(); // Index version not new enough for this check.
}
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index fc1256e..6d35012 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -1297,7 +1297,8 @@
parsedConfig.fromText(cfg);
projectLevelConfigs.put(pathInfo.path, parsedConfig);
} catch (ConfigInvalidException e) {
- logger.atWarning().withCause(e).log("Unable to parse config");
+ logger.atWarning().withCause(e).log(
+ "Unable to parse config for project %s", projectName.get());
}
}
}
diff --git a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
index ab4bb70..9463b39 100644
--- a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
+++ b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
@@ -264,7 +264,7 @@
.changeIndexQuery(
"projectsConsistencyCheckerQueryChanges",
q ->
- q.setRequestedFields(ChangeField.CHANGE, ChangeField.PATCH_SET)
+ q.setRequestedFields(ChangeField.CHANGE_SPEC, ChangeField.PATCH_SET_SPEC)
.query(and(basePredicate, or(predicates))))
.call();
diff --git a/java/com/google/gerrit/server/query/change/AgePredicate.java b/java/com/google/gerrit/server/query/change/AgePredicate.java
index c1138bd..8a9ba2e 100644
--- a/java/com/google/gerrit/server/query/change/AgePredicate.java
+++ b/java/com/google/gerrit/server/query/change/AgePredicate.java
@@ -27,7 +27,7 @@
protected final Instant cut;
public AgePredicate(String value) {
- super(ChangeField.UPDATED, ChangeQueryBuilder.FIELD_AGE, value);
+ super(ChangeField.UPDATED_SPEC, ChangeQueryBuilder.FIELD_AGE, value);
long s = ConfigUtil.getTimeUnit(getValue(), 0, SECONDS);
long ms = MILLISECONDS.convert(s, SECONDS);
diff --git a/java/com/google/gerrit/server/query/change/ChangePredicates.java b/java/com/google/gerrit/server/query/change/ChangePredicates.java
index 4637191..84ceb3d 100644
--- a/java/com/google/gerrit/server/query/change/ChangePredicates.java
+++ b/java/com/google/gerrit/server/query/change/ChangePredicates.java
@@ -128,7 +128,7 @@
*/
public static Predicate<ChangeData> idStr(Change.Id id) {
return new ChangeIndexCardinalPredicate(
- ChangeField.LEGACY_ID_STR, ChangeQueryBuilder.FIELD_CHANGE, id.toString(), 1);
+ ChangeField.NUMERIC_ID_STR_SPEC, ChangeQueryBuilder.FIELD_CHANGE, id.toString(), 1);
}
/**
@@ -302,7 +302,7 @@
/** Returns a predicate that matches changes whose ID starts with the provided {@code id}. */
public static Predicate<ChangeData> idPrefix(String id) {
- return new ChangeIndexCardinalPredicate(ChangeField.ID, id, 5);
+ return new ChangeIndexCardinalPredicate(ChangeField.CHANGE_ID_SPEC, id, 5);
}
/**
@@ -352,7 +352,7 @@
* in the form of 'gerrit~$rule_name'.
*/
public static Predicate<ChangeData> submitRuleStatus(String value) {
- return new ChangeIndexPredicate(ChangeField.SUBMIT_RULE_RESULT, value);
+ return new ChangeIndexPredicate(ChangeField.SUBMIT_RULE_RESULT_SPEC, value);
}
/**
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 8262e58..83c348c 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -520,7 +520,7 @@
@Operator
public Predicate<ChangeData> before(String value) throws QueryParseException {
- return new BeforePredicate(ChangeField.UPDATED, ChangeQueryBuilder.OPERATOR_BEFORE, value);
+ return new BeforePredicate(ChangeField.UPDATED_SPEC, ChangeQueryBuilder.OPERATOR_BEFORE, value);
}
@Operator
@@ -530,7 +530,7 @@
@Operator
public Predicate<ChangeData> after(String value) throws QueryParseException {
- return new AfterPredicate(ChangeField.UPDATED, ChangeQueryBuilder.OPERATOR_AFTER, value);
+ return new AfterPredicate(ChangeField.UPDATED_SPEC, ChangeQueryBuilder.OPERATOR_AFTER, value);
}
@Operator
diff --git a/java/com/google/gerrit/server/query/change/SubmitRecordPredicate.java b/java/com/google/gerrit/server/query/change/SubmitRecordPredicate.java
index ecddbb6..1ea6c41 100644
--- a/java/com/google/gerrit/server/query/change/SubmitRecordPredicate.java
+++ b/java/com/google/gerrit/server/query/change/SubmitRecordPredicate.java
@@ -36,7 +36,7 @@
}
private SubmitRecordPredicate(String value) {
- super(ChangeField.SUBMIT_RECORD, value);
+ super(ChangeField.SUBMIT_RECORD_SPEC, value);
}
@Override
diff --git a/java/com/google/gerrit/server/query/change/SubmittablePredicate.java b/java/com/google/gerrit/server/query/change/SubmittablePredicate.java
index 060a92e..e543ac3 100644
--- a/java/com/google/gerrit/server/query/change/SubmittablePredicate.java
+++ b/java/com/google/gerrit/server/query/change/SubmittablePredicate.java
@@ -21,7 +21,7 @@
protected final SubmitRecord.Status status;
public SubmittablePredicate(SubmitRecord.Status status) {
- super(ChangeField.SUBMIT_RECORD, status.name());
+ super(ChangeField.SUBMIT_RECORD_SPEC, status.name());
this.status = status;
}
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 947ca6a..62fdcbb 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -236,6 +236,7 @@
cherryPickInput = createCherryPickInput(revertInput);
Instant timestamp = TimeUtil.now();
+ String initialMessage = revertInput.message;
for (BranchNameKey projectAndBranch : changesPerProjectAndBranch.keySet()) {
cherryPickInput.base = null;
Project.NameKey project = projectAndBranch.project();
@@ -253,6 +254,7 @@
.collect(Collectors.toSet());
revertAllChangesInProjectAndBranch(
+ initialMessage,
revertInput,
project,
sortedChangesInProjectAndBranch,
@@ -265,7 +267,9 @@
return revertSubmissionInfo;
}
+ // Warning: reuses and modifies revertInput.message.
private void revertAllChangesInProjectAndBranch(
+ String initialMessage,
RevertInput revertInput,
Project.NameKey project,
Iterator<PatchSetData> sortedChangesInProjectAndBranch,
@@ -273,8 +277,6 @@
Instant timestamp)
throws IOException, RestApiException, UpdateException, ConfigInvalidException,
PermissionBackendException {
-
- String initialMessage = revertInput.message;
while (sortedChangesInProjectAndBranch.hasNext()) {
ChangeNotes changeNotes = sortedChangesInProjectAndBranch.next().data().notes();
if (cherryPickInput.base == null) {
@@ -282,6 +284,7 @@
cherryPickInput.base = getBase(changeNotes, commitIdsInProjectAndBranch).name();
}
+ // Set revert message for the current revert change.
revertInput.message = getMessage(initialMessage, changeNotes);
if (cherryPickInput.base.equals(changeNotes.getCurrentPatchSet().commitId().getName())) {
// This is the code in case this is the first revert of this project + branch, and the
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
index 527253c..e0e980e 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
@@ -63,6 +63,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
+import java.util.regex.Pattern;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Repository;
@@ -1278,10 +1279,38 @@
.distinct()
.count())
.isEqualTo(1);
+
+ // Size
List<ChangeApi> revertChanges = getChangeApis(revertSubmissionInfo);
+ assertThat(revertChanges).hasSize(3);
+ assertThat(gApi.changes().id(revertChanges.get(1).id()).current().related().changes).hasSize(2);
+
+ // Contents
assertThat(revertChanges.get(0).current().files().get("c.txt").linesDeleted).isEqualTo(1);
assertThat(revertChanges.get(1).current().files().get("a.txt").linesDeleted).isEqualTo(1);
assertThat(revertChanges.get(2).current().files().get("b.txt").linesDeleted).isEqualTo(1);
+
+ // Commit message
+ assertThat(revertChanges.get(0).current().commit(false).message)
+ .matches(
+ Pattern.compile(
+ "Revert \"first change\"\n\n"
+ + "This reverts commit [a-f0-9]+\\.\n\n"
+ + "Change-Id: I[a-f0-9]+\n"));
+ assertThat(revertChanges.get(1).current().commit(false).message)
+ .matches(
+ Pattern.compile(
+ "Revert \"second change\"\n\n"
+ + "This reverts commit [a-f0-9]+\\.\n\n"
+ + "Change-Id: I[a-f0-9]+\n"));
+ assertThat(revertChanges.get(2).current().commit(false).message)
+ .matches(
+ Pattern.compile(
+ "Revert \"third change\"\n\n"
+ + "This reverts commit [a-f0-9]+\\.\n\n"
+ + "Change-Id: I[a-f0-9]+\n"));
+
+ // Relationships
String sha1FirstChange = resultCommits.get(0).getCommit().getName();
String sha1ThirdChange = resultCommits.get(2).getCommit().getName();
String sha1SecondRevert = revertChanges.get(2).current().commit(false).commit;
@@ -1291,9 +1320,6 @@
.isEqualTo(sha1ThirdChange);
assertThat(revertChanges.get(1).current().commit(false).parents.get(0).commit)
.isEqualTo(sha1SecondRevert);
-
- assertThat(revertChanges).hasSize(3);
- assertThat(gApi.changes().id(revertChanges.get(1).id()).current().related().changes).hasSize(2);
}
@Test
diff --git a/javatests/com/google/gerrit/index/IndexUpgradeValidatorTest.java b/javatests/com/google/gerrit/index/IndexUpgradeValidatorTest.java
index aa2605e..7f98a9d 100644
--- a/javatests/com/google/gerrit/index/IndexUpgradeValidatorTest.java
+++ b/javatests/com/google/gerrit/index/IndexUpgradeValidatorTest.java
@@ -22,7 +22,6 @@
import com.google.gerrit.index.SchemaFieldDefs.Getter;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -35,23 +34,42 @@
// SchemaFields.
@Test
public void valid() {
- IndexUpgradeValidator.assertValid(schema(1, ChangeField.ID), schema(2, ChangeField.ID));
IndexUpgradeValidator.assertValid(
- schema(1, ChangeField.ID),
+ schema(
+ 1,
+ ImmutableList.of(ChangeField.CHANGE_ID_FIELD),
+ ImmutableList.of(ChangeField.CHANGE_ID_SPEC)),
schema(
2,
- ImmutableList.<FieldDef<ChangeData, ?>>of(ChangeField.ID),
- ImmutableList.<IndexedField<ChangeData, ?>>of(ChangeField.OWNER_FIELD),
- ImmutableList.<IndexedField<ChangeData, ?>.SearchSpec>of(ChangeField.OWNER_SPEC)));
+ ImmutableList.of(ChangeField.CHANGE_ID_FIELD),
+ ImmutableList.of(ChangeField.CHANGE_ID_SPEC)));
IndexUpgradeValidator.assertValid(
- schema(1, ChangeField.ID),
+ schema(
+ 1,
+ ImmutableList.of(ChangeField.CHANGE_ID_FIELD),
+ ImmutableList.of(ChangeField.CHANGE_ID_SPEC)),
schema(
2,
- ImmutableList.<FieldDef<ChangeData, ?>>of(ChangeField.ID),
+ ImmutableList.<FieldDef<ChangeData, ?>>of(),
ImmutableList.<IndexedField<ChangeData, ?>>of(
- ChangeField.OWNER_FIELD, ChangeField.COMMITTER_PARTS_FIELD),
+ ChangeField.OWNER_FIELD, ChangeField.CHANGE_ID_FIELD),
ImmutableList.<IndexedField<ChangeData, ?>.SearchSpec>of(
- ChangeField.OWNER_SPEC, ChangeField.COMMITTER_PARTS_SPEC)));
+ ChangeField.OWNER_SPEC, ChangeField.CHANGE_ID_SPEC)));
+ IndexUpgradeValidator.assertValid(
+ schema(
+ 1,
+ ImmutableList.of(ChangeField.CHANGE_ID_FIELD),
+ ImmutableList.of(ChangeField.CHANGE_ID_SPEC)),
+ schema(
+ 2,
+ ImmutableList.<IndexedField<ChangeData, ?>>of(
+ ChangeField.CHANGE_ID_FIELD,
+ ChangeField.OWNER_FIELD,
+ ChangeField.COMMITTER_PARTS_FIELD),
+ ImmutableList.<IndexedField<ChangeData, ?>.SearchSpec>of(
+ ChangeField.CHANGE_ID_SPEC,
+ ChangeField.OWNER_SPEC,
+ ChangeField.COMMITTER_PARTS_SPEC)));
}
@Test
@@ -61,10 +79,12 @@
AssertionError.class,
() ->
IndexUpgradeValidator.assertValid(
- schema(1, ChangeField.ID),
+ schema(
+ 1,
+ ImmutableList.of(ChangeField.CHANGE_ID_FIELD),
+ ImmutableList.of(ChangeField.CHANGE_ID_SPEC)),
schema(
2,
- ImmutableList.<FieldDef<ChangeData, ?>>of(),
ImmutableList.<IndexedField<ChangeData, ?>>of(ChangeField.OWNER_FIELD),
ImmutableList.<IndexedField<ChangeData, ?>.SearchSpec>of(
ChangeField.OWNER_SPEC))));
@@ -76,32 +96,38 @@
@Test
public void invalid_modify() {
// Change value type from String to Integer.
- FieldDef<ChangeData, Integer> ID_MODIFIED =
- new FieldDef.Builder<>(FieldType.INTEGER, ChangeQueryBuilder.FIELD_CHANGE_ID)
- .build(cd -> 42);
+ IndexedField<ChangeData, Integer> ID_MODIFIED =
+ IndexedField.<ChangeData>integerBuilder(ChangeField.CHANGE_ID_FIELD.name()).build(cd -> 42);
AssertionError e =
assertThrows(
AssertionError.class,
() ->
IndexUpgradeValidator.assertValid(
- schema(1, ChangeField.ID), schema(2, ID_MODIFIED)));
+ schema(
+ 1,
+ ImmutableList.of(ChangeField.CHANGE_ID_FIELD),
+ ImmutableList.of(ChangeField.CHANGE_ID_SPEC)),
+ schema(2, ImmutableList.of(ID_MODIFIED), ImmutableList.of())));
assertThat(e).hasMessageThat().contains("Fields may not be modified");
- assertThat(e).hasMessageThat().contains(ChangeQueryBuilder.FIELD_CHANGE_ID);
+ assertThat(e).hasMessageThat().contains(ChangeField.CHANGE_ID_FIELD.name());
}
@Test
public void invalid_modify_referenceEquality() {
// Comparison uses Object.equals(), i.e. reference equality.
Getter<ChangeData, String> getter = cd -> cd.change().getKey().get();
- FieldDef<ChangeData, String> ID_1 =
- new FieldDef.Builder<>(FieldType.PREFIX, ChangeQueryBuilder.FIELD_CHANGE_ID).build(getter);
- FieldDef<ChangeData, String> ID_2 =
- new FieldDef.Builder<>(FieldType.PREFIX, ChangeQueryBuilder.FIELD_CHANGE_ID).build(getter);
+ IndexedField<ChangeData, String> ID_1 =
+ IndexedField.<ChangeData>stringBuilder(ChangeField.CHANGE_ID_FIELD.name()).build(getter);
+ IndexedField<ChangeData, String> ID_2 =
+ IndexedField.<ChangeData>stringBuilder(ChangeField.CHANGE_ID_FIELD.name()).build(getter);
AssertionError e =
assertThrows(
AssertionError.class,
- () -> IndexUpgradeValidator.assertValid(schema(1, ID_1), schema(2, ID_2)));
+ () ->
+ IndexUpgradeValidator.assertValid(
+ schema(1, ImmutableList.of(ID_1), ImmutableList.of()),
+ schema(2, ImmutableList.of(ID_2), ImmutableList.of())));
assertThat(e).hasMessageThat().contains("Fields may not be modified");
- assertThat(e).hasMessageThat().contains(ChangeQueryBuilder.FIELD_CHANGE_ID);
+ assertThat(e).hasMessageThat().contains(ChangeField.CHANGE_ID_FIELD.name());
}
}
diff --git a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
index 2aa9ca4..95e8aa5 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
@@ -42,11 +42,11 @@
static final Schema<ChangeData> V2 =
schema(
2,
- ImmutableList.<FieldDef<ChangeData, ?>>of(ChangeField.UPDATED),
+ ImmutableList.of(),
ImmutableList.<IndexedField<ChangeData, ?>>of(
- ChangeField.PATH_FIELD, ChangeField.STATUS_FIELD),
+ ChangeField.PATH_FIELD, ChangeField.STATUS_FIELD, ChangeField.UPDATED_FIELD),
ImmutableList.<IndexedField<ChangeData, ?>.SearchSpec>of(
- ChangeField.PATH_SPEC, ChangeField.STATUS_SPEC));
+ ChangeField.PATH_SPEC, ChangeField.STATUS_SPEC, ChangeField.UPDATED_SPEC));
private static class Source implements ChangeDataSource {
private final Predicate<ChangeData> p;
diff --git a/modules/jgit b/modules/jgit
index e81c513..e74f385 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit e81c5135fefb33103b12c8132a4fd5101f0c7b26
+Subproject commit e74f3855ad9d54c986d60b0b2ea4c223d52b2cd1
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index d5e1049..a78ba1c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -1364,7 +1364,11 @@
return;
}
assertIsDefined(this.confirmRevertDialog, 'confirmRevertDialog');
- this.confirmRevertDialog.populate(change, this.commitMessage, changes);
+ this.confirmRevertDialog.populate(
+ change,
+ this.commitMessage,
+ changes.length
+ );
this.showActionDialog(this.confirmRevertDialog);
});
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index a1021a0..bdcdc31 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -1582,13 +1582,8 @@
'Revert submission 199 0' +
'\n\n' +
'Reason for revert: <INSERT REASONING HERE>' +
- '\n' +
- 'Reverted Changes:' +
- '\n' +
- '1234567890:random' +
- '\n' +
- '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
- '\n';
+ '\n\n' +
+ 'Reverted changes: /q/submissionid:199+0\n';
assert.equal(confirmRevertDialog.message, expectedMsg);
const radioInputs = queryAll<HTMLInputElement>(
confirmRevertDialog,
@@ -1648,13 +1643,8 @@
'Revert submission 199 0' +
'\n\n' +
'Reason for revert: <INSERT REASONING HERE>' +
- '\n' +
- 'Reverted Changes:' +
- '\n' +
- '1234567890:random' +
- '\n' +
- '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
- '\n';
+ '\n\n' +
+ 'Reverted changes: /q/submissionid:199+0\n';
const singleChangeMsg =
'Revert "random commit message"\n\nThis reverts ' +
'commit 2000.\n\nReason' +
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
index b238d77..6b37284 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
@@ -14,9 +14,9 @@
import {BindValueChangeEvent} from '../../../types/events';
import {resolve} from '../../../models/dependency';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+import {createSearchUrl} from '../../../models/views/search';
const ERR_COMMIT_NOT_FOUND = 'Unable to find the commit hash of this change.';
-const CHANGE_SUBJECT_LIMIT = 50;
const INSERT_REASON_STRING = '<INSERT REASONING HERE>';
// TODO(dhruvsri): clean up repeated definitions after moving to js modules
@@ -58,8 +58,9 @@
@state()
private showRevertSubmission = false;
+ // Value supplied by populate(). Non-private for access in tests.
@state()
- private changesCount?: number;
+ changesCount?: number;
@state()
showErrorMessage = false;
@@ -189,15 +190,15 @@
);
}
- populate(change: ChangeInfo, commitMessage: string, changes: ChangeInfo[]) {
- this.changesCount = changes.length;
+ populate(change: ChangeInfo, commitMessage: string, changesCount: number) {
+ this.changesCount = changesCount;
// The option to revert a single change is always available
this.populateRevertSingleChangeMessage(
change,
commitMessage,
change.current_revision
);
- this.populateRevertSubmissionMessage(change, changes, commitMessage);
+ this.populateRevertSubmissionMessage(change, commitMessage);
}
populateRevertSingleChangeMessage(
@@ -225,12 +226,6 @@
this.originalRevertMessages[this.revertType] = this.message;
}
- private getTrimmedChangeSubject(subject: string) {
- if (!subject) return '';
- if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
- return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
- }
-
private modifyRevertSubmissionMsg(
change: ChangeInfo,
msg: string,
@@ -243,30 +238,22 @@
);
}
- populateRevertSubmissionMessage(
- change: ChangeInfo,
- changes: ChangeInfo[],
- commitMessage: string
- ) {
+ populateRevertSubmissionMessage(change: ChangeInfo, commitMessage: string) {
// Follow the same convention of the revert
const commitHash = change.current_revision;
if (!commitHash) {
fireAlert(this, ERR_COMMIT_NOT_FOUND);
return;
}
- if (!changes || changes.length <= 1) return;
- const revertTitle = `Revert submission ${change.submission_id}`;
- let message =
- revertTitle +
+ if (this.changesCount! <= 1) return;
+ const message =
+ `Revert submission ${change.submission_id}` +
'\n\n' +
'Reason for revert: <INSERT ' +
- 'REASONING HERE>\n';
- message += 'Reverted Changes:\n';
- changes.forEach(change => {
- message +=
- `${change.change_id.substring(0, 10)}:` +
- `${this.getTrimmedChangeSubject(change.subject)}\n`;
- });
+ 'REASONING HERE>\n\n' +
+ 'Reverted changes: ' +
+ createSearchUrl({query: `submissionid:${change.submission_id}`}) +
+ '\n';
this.message = this.modifyRevertSubmissionMsg(
change,
message,
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
index 59416da..38309f2 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
@@ -6,7 +6,7 @@
import {fixture, html, assert} from '@open-wc/testing';
import '../../../test/common-test-setup';
import {createChange} from '../../../test/test-data-generators';
-import {CommitId} from '../../../types/common';
+import {ChangeSubmissionId, CommitId} from '../../../types/common';
import {EventType} from '../../../types/events';
import './gr-confirm-revert-dialog';
import {GrConfirmRevertDialog} from './gr-confirm-revert-dialog';
@@ -111,4 +111,22 @@
'Reason for revert: <INSERT REASONING HERE>\n';
assert.equal(element.message, expected);
});
+
+ test('revert submission', () => {
+ element.changesCount = 3;
+ element.populateRevertSubmissionMessage(
+ {
+ ...createChange(),
+ submission_id: '5545' as ChangeSubmissionId,
+ current_revision: 'abcd123' as CommitId,
+ },
+ 'one line commit\n\nChange-Id: abcdefg\n'
+ );
+
+ const expected =
+ 'Revert submission 5545\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>\n\n' +
+ 'Reverted changes: /q/submissionid:5545\n';
+ assert.equal(element.message, expected);
+ });
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts
index abe3c10..2a61ef2 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts
@@ -103,8 +103,8 @@
private findSection(group?: GrDiffGroup): GrDiffSection | undefined {
if (!group) return undefined;
- const leftClass = `left-${group.lineRange.left.start_line}`;
- const rightClass = `right-${group.lineRange.right.start_line}`;
+ const leftClass = `left-${group.startLine(Side.LEFT)}`;
+ const rightClass = `right-${group.startLine(Side.RIGHT)}`;
return (
this.outputEl.querySelector<GrDiffSection>(
`gr-diff-section.${leftClass}.${rightClass}`
@@ -137,8 +137,8 @@
}
protected override buildSectionElement(group: GrDiffGroup) {
- const leftCl = `left-${group.lineRange.left.start_line}`;
- const rightCl = `right-${group.lineRange.right.start_line}`;
+ const leftCl = `left-${group.startLine(Side.LEFT)}`;
+ const rightCl = `right-${group.startLine(Side.RIGHT)}`;
const section = html`
<gr-diff-section
class="${leftCl} ${rightCl}"
@@ -149,25 +149,39 @@
.renderPrefs=${this.renderPrefs}
></gr-diff-section>
`;
- // TODO: Refactor GrDiffBuilder.emitGroup() and buildSectionElement()
- // such that we can render directly into the correct container.
- const tempContainer = document.createElement('div');
- render(section, tempContainer);
- return tempContainer.firstElementChild as GrDiffSection;
+ // When using Lit's `render()` method it wants to be in full control of the
+ // element that it renders into, so we let it render into a temp element.
+ // Rendering into the diff table directly would interfere with
+ // `clearDiffContent()`for example.
+ // TODO: Remove legacy diff builder, then convert <gr-diff> to be fully lit
+ // controlled, then this code will become part of the standard `render()` of
+ // <gr-diff> as a LitElement.
+ const tempEl = document.createElement('div');
+ render(section, tempEl);
+ const sectionEl = tempEl.firstElementChild as GrDiffSection;
+ return sectionEl;
}
override addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
- render(
- html`
- <colgroup>
- <col class=${diffClasses('blame')}></col>
- ${this.renderUnifiedColumns(lineNumberWidth)}
- ${this.renderSideBySideColumns(Side.LEFT, lineNumberWidth)}
- ${this.renderSideBySideColumns(Side.RIGHT, lineNumberWidth)}
- </colgroup>
- `,
- outputEl
- );
+ const colgroup = html`
+ <colgroup>
+ <col class=${diffClasses('blame')}></col>
+ ${this.renderUnifiedColumns(lineNumberWidth)}
+ ${this.renderSideBySideColumns(Side.LEFT, lineNumberWidth)}
+ ${this.renderSideBySideColumns(Side.RIGHT, lineNumberWidth)}
+ </colgroup>
+ `;
+ // When using Lit's `render()` method it wants to be in full control of the
+ // element that it renders into, so we let it render into a temp element.
+ // Rendering into the diff table directly would interfere with
+ // `clearDiffContent()`for example.
+ // TODO: Remove legacy diff builder, then convert <gr-diff> to be fully lit
+ // controlled, then this code will become part of the standard `render()` of
+ // <gr-diff> as a LitElement.
+ const tempEl = document.createElement('div');
+ render(colgroup, tempEl);
+ const colgroupEl = tempEl.firstElementChild as HTMLElement;
+ outputEl.appendChild(colgroupEl);
}
private renderUnifiedColumns(lineNumberWidth: number) {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
index 14b8280..13a6b00 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
@@ -27,6 +27,10 @@
contentRightRef: Ref<LitElement> = createRef();
+ contentCellLeftRef: Ref<HTMLTableCellElement> = createRef();
+
+ contentCellRightRef: Ref<HTMLTableCellElement> = createRef();
+
lineNumberLeftRef: Ref<HTMLTableCellElement> = createRef();
lineNumberRightRef: Ref<HTMLTableCellElement> = createRef();
@@ -201,9 +205,7 @@
}
getContentCell(side: Side) {
- const div = this.contentRef(side)?.value;
- if (!div) return undefined;
- return div.parentElement as HTMLTableCellElement;
+ return this.contentCellRef(side)?.value;
}
getBlameCell() {
@@ -339,6 +341,7 @@
// prettier-ignore
return html`
<td
+ ${ref(this.contentCellRef(side))}
class=${diffClasses(...extras)}
@mouseenter=${() => {
if (lineNumber)
@@ -390,6 +393,12 @@
return side === Side.LEFT ? this.contentLeftRef : this.contentRightRef;
}
+ private contentCellRef(side: Side) {
+ return side === Side.LEFT
+ ? this.contentCellLeftRef
+ : this.contentCellRightRef;
+ }
+
private lineNumberRef(side: Side) {
return side === Side.LEFT
? this.lineNumberLeftRef
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts
index e5fce12..14bb17e 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -30,6 +30,29 @@
number: number;
}
+/**
+ * From <tr> diff row go up to <tbody> diff chunk.
+ *
+ * In Lit based diff there is a <gr-diff-row> element in between the two.
+ */
+export function fromRowToChunk(
+ rowEl: HTMLElement
+): HTMLTableSectionElement | undefined {
+ const parent = rowEl.parentElement;
+ if (!parent) return undefined;
+ if (parent.tagName === 'TBODY') {
+ return parent as HTMLTableSectionElement;
+ }
+
+ const grandParent = parent.parentElement;
+ if (!grandParent) return undefined;
+ if (grandParent.tagName === 'TBODY') {
+ return grandParent as HTMLTableSectionElement;
+ }
+
+ return undefined;
+}
+
/** A subset of the GrDiff API that the cursor is using. */
export interface GrDiffCursorable extends HTMLElement {
isRangeSelected(): boolean;
@@ -179,8 +202,7 @@
moveToNextChunk(clipToTop?: boolean): CursorMoveResult {
const result = this.cursorManager.next({
filter: (row: HTMLElement) => this._isFirstRowOfChunk(row),
- getTargetHeight: target =>
- (target?.parentNode as HTMLElement)?.scrollHeight || 0,
+ getTargetHeight: target => fromRowToChunk(target)?.scrollHeight || 0,
clipToTop,
});
this._fixSide();
@@ -413,13 +435,14 @@
}
_isFirstRowOfChunk(row: HTMLElement) {
- const parentClassList = (row.parentNode as HTMLElement).classList;
- const isInChunk =
- parentClassList.contains('section') && parentClassList.contains('delta');
- const previousRow = row.previousSibling as HTMLElement;
- const firstContentRow =
- !previousRow || previousRow.classList.contains('moveControls');
- return isInChunk && firstContentRow;
+ const chunk = fromRowToChunk(row);
+ if (!chunk) return false;
+
+ const isInDeltaChunk = chunk.classList.contains('delta');
+ if (!isInDeltaChunk) return false;
+
+ const firstRow = chunk.querySelector('tr:not(.moveControls)');
+ return firstRow === row;
}
_rowHasThread(row: HTMLElement): boolean {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
index 1e554b7..b9db280 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
@@ -7,7 +7,12 @@
import '../gr-diff/gr-diff';
import './gr-diff-cursor';
import {fixture, html, assert} from '@open-wc/testing';
-import {mockPromise, queryAll, queryAndAssert} from '../../../test/test-utils';
+import {
+ mockPromise,
+ queryAll,
+ queryAndAssert,
+ waitUntil,
+} from '../../../test/test-utils';
import {createDiff} from '../../../test/test-data-generators';
import {createDefaultDiffPrefs} from '../../../constants/constants';
import {GrDiffCursor} from './gr-diff-cursor';
@@ -41,37 +46,29 @@
diff = createDiff();
diffElement.prefs = createDefaultDiffPrefs();
+ diffElement.renderPrefs = {use_lit_components: true};
diffElement.diff = diff;
await promise;
});
test('diff cursor functionality (side-by-side)', () => {
- // The cursor has been initialized to the first delta.
assert.isOk(cursor.diffRow);
- const firstDeltaRow = queryAndAssert<HTMLElement>(
+ const deltaRows = queryAll<HTMLTableRowElement>(
diffElement,
- '.section.delta .diff-row'
+ '.section.delta tr.diff-row'
);
- assert.equal(cursor.diffRow, firstDeltaRow);
+ assert.equal(cursor.diffRow, deltaRows[0]);
cursor.moveDown();
- assert.isOk(firstDeltaRow.nextElementSibling);
- assert.notEqual(cursor.diffRow, firstDeltaRow);
- assert.equal(
- cursor.diffRow,
- firstDeltaRow.nextElementSibling as HTMLElement
- );
+ assert.notEqual(cursor.diffRow, deltaRows[0]);
+ assert.equal(cursor.diffRow, deltaRows[1]);
cursor.moveUp();
- assert.isOk(firstDeltaRow.nextElementSibling);
- assert.notEqual(
- cursor.diffRow,
- firstDeltaRow.nextElementSibling as HTMLElement
- );
- assert.equal(cursor.diffRow, firstDeltaRow);
+ assert.notEqual(cursor.diffRow, deltaRows[1]);
+ assert.equal(cursor.diffRow, deltaRows[0]);
});
test('moveToFirstChunk', async () => {
@@ -115,20 +112,26 @@
] as HTMLElement[];
assert.equal(chunks.length, 2);
+ const rows = [
+ ...queryAll(diffElement, '.section.delta tr.diff-row'),
+ ] as HTMLTableRowElement[];
+ assert.equal(rows.length, 2);
+
// Verify it works on fresh diff.
cursor.moveToFirstChunk();
assert.ok(cursor.diffRow);
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 0);
+ assert.equal(cursor.diffRow, rows[0]);
assert.equal(cursor.side, Side.RIGHT);
// Verify it works from other cursor positions.
cursor.moveToNextChunk();
assert.ok(cursor.diffRow);
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 1);
+ assert.equal(cursor.diffRow, rows[1]);
assert.equal(cursor.side, Side.LEFT);
+
cursor.moveToFirstChunk();
assert.ok(cursor.diffRow);
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 0);
+ assert.equal(cursor.diffRow, rows[0]);
assert.equal(cursor.side, Side.RIGHT);
});
@@ -164,20 +167,31 @@
await waitForEventOnce(diffElement, 'render');
cursor._updateStops();
- const chunks = [...queryAll(diffElement, '.section.delta')];
+ const chunks = [
+ ...queryAll(diffElement, '.section.delta'),
+ ] as HTMLElement[];
assert.equal(chunks.length, 2);
+ const rows = [
+ ...queryAll(diffElement, '.section.delta tr.diff-row'),
+ ] as HTMLTableRowElement[];
+ assert.equal(rows.length, 2);
+
// Verify it works on fresh diff.
cursor.moveToLastChunk();
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 1);
+ assert.ok(cursor.diffRow);
+ assert.equal(cursor.diffRow, rows[1]);
assert.equal(cursor.side, Side.RIGHT);
// Verify it works from other cursor positions.
cursor.moveToPreviousChunk();
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 0);
+ assert.ok(cursor.diffRow);
+ assert.equal(cursor.diffRow, rows[0]);
assert.equal(cursor.side, Side.LEFT);
+
cursor.moveToLastChunk();
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 1);
+ assert.ok(cursor.diffRow);
+ assert.equal(cursor.diffRow, rows[1]);
assert.equal(cursor.side, Side.RIGHT);
});
@@ -221,30 +235,22 @@
});
test('diff cursor functionality (unified)', () => {
- // The cursor has been initialized to the first delta.
assert.isOk(cursor.diffRow);
- const firstDeltaRow = queryAndAssert<HTMLElement>(
- diffElement,
- '.section.delta .diff-row'
- );
- assert.equal(cursor.diffRow, firstDeltaRow);
+ const rows = [
+ ...queryAll(diffElement, '.section.delta tr.diff-row'),
+ ] as HTMLTableRowElement[];
+ assert.equal(cursor.diffRow, rows[0]);
cursor.moveDown();
- assert.notEqual(cursor.diffRow, firstDeltaRow);
- assert.equal(
- cursor.diffRow,
- firstDeltaRow.nextElementSibling as HTMLElement
- );
+ assert.notEqual(cursor.diffRow, rows[0]);
+ assert.equal(cursor.diffRow, rows[1]);
cursor.moveUp();
- assert.notEqual(
- cursor.diffRow,
- firstDeltaRow.nextElementSibling as HTMLElement
- );
- assert.equal(cursor.diffRow, firstDeltaRow);
+ assert.notEqual(cursor.diffRow, rows[1]);
+ assert.equal(cursor.diffRow, rows[0]);
});
});
@@ -253,19 +259,21 @@
// mode.
assert.equal(diffElement.viewMode, 'SIDE_BY_SIDE');
- const firstDeltaSection = queryAndAssert<HTMLElement>(
- diffElement,
- '.section.delta'
- );
- const firstDeltaRow = queryAndAssert<HTMLElement>(
- firstDeltaSection,
- '.diff-row'
- );
+ const rows = [
+ ...queryAll(diffElement, '.section tr.diff-row'),
+ ] as HTMLTableRowElement[];
+ assert.equal(rows.length, 50);
+ const deltaRows = [
+ ...queryAll(diffElement, '.section.delta tr.diff-row'),
+ ] as HTMLTableRowElement[];
+ assert.equal(deltaRows.length, 14);
+ const indexFirstDelta = rows.indexOf(deltaRows[0]);
+ const rowBeforeFirstDelta = rows[indexFirstDelta - 1];
// Because the first delta in this diff is on the right, it should be set
// to the right side.
assert.equal(cursor.side, Side.RIGHT);
- assert.equal(cursor.diffRow, firstDeltaRow);
+ assert.equal(cursor.diffRow, deltaRows[0]);
const firstIndex = cursor.cursorManager.index;
// Move the side to the left. Because this delta only has a right side, we
@@ -274,33 +282,26 @@
cursor.moveLeft();
assert.equal(cursor.side, Side.LEFT);
- assert.notEqual(cursor.diffRow, firstDeltaRow);
+ assert.notEqual(cursor.diffRow, rows[0]);
+ assert.equal(cursor.diffRow, rowBeforeFirstDelta);
assert.equal(cursor.cursorManager.index, firstIndex - 1);
- assert.equal(
- cursor.diffRow!.parentElement,
- firstDeltaSection.previousSibling
- );
// If we move down, we should skip everything in the first delta because
// we are on the left side and the first delta has no content on the left.
cursor.moveDown();
assert.equal(cursor.side, Side.LEFT);
- assert.notEqual(cursor.diffRow, firstDeltaRow);
+ assert.notEqual(cursor.diffRow, rowBeforeFirstDelta);
+ assert.notEqual(cursor.diffRow, rows[0]);
assert.isTrue(cursor.cursorManager.index > firstIndex);
- assert.equal(cursor.diffRow!.parentElement, firstDeltaSection.nextSibling);
});
test('chunk skip functionality', () => {
- const chunks = [...queryAll(diffElement, '.section.delta')];
- const indexOfChunk = function (chunk: HTMLElement) {
- return Array.prototype.indexOf.call(chunks, chunk);
- };
+ const deltaChunks = [...queryAll(diffElement, 'tbody.section.delta')];
// We should be initialized to the first chunk. Since this chunk only has
// content on the right side, our side should be right.
- let currentIndex = indexOfChunk(cursor.diffRow!.parentElement!);
- assert.equal(currentIndex, 0);
+ assert.equal(cursor.diffRow, deltaChunks[0].querySelector('tr'));
assert.equal(cursor.side, Side.RIGHT);
// Move to the next chunk.
@@ -308,9 +309,7 @@
// Since this chunk only has content on the left side. we should have been
// automatically moved over.
- const previousIndex = currentIndex;
- currentIndex = indexOfChunk(cursor.diffRow!.parentElement!);
- assert.equal(currentIndex, previousIndex + 1);
+ assert.equal(cursor.diffRow, deltaChunks[1].querySelector('tr'));
assert.equal(cursor.side, Side.LEFT);
});
@@ -358,10 +357,10 @@
test('renders moveControls with simple descriptions', () => {
const [movedIn, movedOut] = [
- ...queryAll(diffElement, '.dueToMove .moveControls'),
+ ...queryAll<HTMLElement>(diffElement, '.dueToMove tr.moveControls'),
];
- assert.equal(movedIn.textContent, 'Moved in');
- assert.equal(movedOut.textContent, 'Moved out');
+ assert.include(movedIn.innerText, 'Moved in');
+ assert.include(movedOut.innerText, 'Moved out');
});
});
@@ -409,10 +408,10 @@
test('renders moveControls with simple descriptions', () => {
const [movedIn, movedOut] = [
- ...queryAll(diffElement, '.dueToMove .moveControls'),
+ ...queryAll<HTMLElement>(diffElement, '.dueToMove tr.moveControls'),
];
- assert.equal(movedIn.textContent, 'Moved from lines 4 - 6');
- assert.equal(movedOut.textContent, 'Moved to lines 2 - 4');
+ assert.include(movedIn.innerText, 'Moved from lines 4 - 6');
+ assert.include(movedOut.innerText, 'Moved to lines 2 - 4');
});
test('startLineAnchor of movedIn chunk fires events', async () => {
@@ -609,6 +608,7 @@
const showContext = queryAndAssert<HTMLElement>(controls, '.showContext');
showContext.click();
await waitForEventOnce(diffElement, 'render');
+ await waitUntil(() => spy.called);
assert.isTrue(spy.called);
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection.ts b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection.ts
index db53eae..a790736 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection.ts
@@ -4,10 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../../../styles/shared-styles';
-import {
- normalize,
- NormalizedRange,
-} from '../gr-diff-highlight/gr-range-normalizer';
+import {normalize} from '../gr-diff-highlight/gr-range-normalizer';
import {
descendedFromClass,
parentWithClass,
@@ -21,7 +18,6 @@
getSideByLineEl,
isThreadEl,
} from '../gr-diff/gr-diff-utils';
-import {assertIsDefined} from '../../../utils/common-util';
/**
* Possible CSS classes indicating the state of selection. Dynamically added/
@@ -121,19 +117,17 @@
}
handleCopy = (e: ClipboardEvent) => {
- let commentSelected = false;
const target = e.composedPath()[0];
if (!(target instanceof Element)) return;
if (target instanceof HTMLTextAreaElement) return;
if (!descendedFromClass(target, 'diff-row', this.diffTable)) return;
if (!this.diffTable) return;
- if (this.diffTable.classList.contains(SelectionClass.COMMENT)) {
- commentSelected = true;
- }
+ if (this.diffTable.classList.contains(SelectionClass.COMMENT)) return;
+
const lineEl = getLineElByChild(target);
if (!lineEl) return;
const side = getSideByLineEl(lineEl);
- const text = this.getSelectedText(side, commentSelected);
+ const text = this.getSelectedText(side);
if (text && e.clipboardData) {
e.clipboardData.setData('Text', text);
e.preventDefault();
@@ -166,14 +160,11 @@
* @param commentSelected Whether or not a comment is selected.
* @return The selected text.
*/
- getSelectedText(side: Side, commentSelected: boolean) {
+ getSelectedText(side: Side) {
const sel = this.getSelection();
if (!sel || sel.rangeCount !== 1) {
return ''; // No multi-select support yet.
}
- if (commentSelected) {
- return this.getCommentLines(sel, side);
- }
const range = normalize(sel.getRangeAt(0));
const startLineEl = getLineElByChild(range.startContainer);
if (!startLineEl) return;
@@ -253,82 +244,4 @@
this.linesCache[side] = lines;
return lines;
}
-
- /**
- * Query the diffElement for comments and check whether they lie inside the
- * selection range.
- *
- * @param sel The selection of the window.
- * @param side The side that is currently selected.
- * @return The selected comment text.
- */
- getCommentLines(sel: Selection, side: Side) {
- const range = normalize(sel.getRangeAt(0));
- const content = [];
- assertIsDefined(this.diffTable, 'diffTable');
- const messages = this.diffTable.querySelectorAll(
- `.side-by-side [data-side="${side}"] .message *, .unified .message *`
- );
-
- for (let i = 0; i < messages.length; i++) {
- const el = messages[i];
- // Check if the comment element exists inside the selection.
- if (sel.containsNode(el, true)) {
- // Padded elements require newlines for accurate spacing.
- if (
- el.parentElement!.id === 'container' ||
- el.parentElement!.nodeName === 'BLOCKQUOTE'
- ) {
- if (content.length && content[content.length - 1] !== '') {
- content.push('');
- }
- }
-
- if (
- el.id === 'output' &&
- !descendedFromClass(el, 'collapsed', this.diffTable)
- ) {
- content.push(this.getTextContentForRange(el, sel, range));
- }
- }
- }
-
- return content.join('\n');
- }
-
- /**
- * Given a DOM node, a selection, and a selection range, recursively get all
- * of the text content within that selection.
- * Using a domNode that isn't in the selection returns an empty string.
- *
- * @param domNode The root DOM node.
- * @param sel The selection.
- * @param range The normalized selection range.
- * @return The text within the selection.
- */
- getTextContentForRange(
- domNode: Node,
- sel: Selection,
- range: NormalizedRange
- ) {
- if (!sel.containsNode(domNode, true)) {
- return '';
- }
-
- let text = '';
- if (domNode instanceof Text) {
- text = domNode.textContent || '';
- if (domNode === range.endContainer) {
- text = text.substring(0, range.endOffset);
- }
- if (domNode === range.startContainer) {
- text = text.substring(range.startOffset);
- }
- } else {
- for (const childNode of domNode.childNodes) {
- text += this.getTextContentForRange(childNode, sel, range);
- }
- }
- return text;
- }
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
index 8acaf04..9e3d288 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
@@ -5,96 +5,25 @@
*/
import '../../../test/common-test-setup';
import './gr-diff-selection';
+import '../gr-diff/gr-diff';
+import '../../../elements/shared/gr-comment-thread/gr-comment-thread';
import {GrDiffSelection} from './gr-diff-selection';
import {createDiff} from '../../../test/test-data-generators';
import {DiffInfo, Side} from '../../../api/diff';
-import {GrFormattedText} from '../../../elements/shared/gr-formatted-text/gr-formatted-text';
import {fixture, html, assert} from '@open-wc/testing';
import {mouseDown} from '../../../test/test-utils';
+import {GrDiff} from '../gr-diff/gr-diff';
+import {waitForEventOnce} from '../../../utils/event-util';
+import {createDefaultDiffPrefs} from '../../../constants/constants';
-const diffTableTemplate = html`
- <table id="diffTable" class="side-by-side">
- <tr class="diff-row">
- <td class="blame" data-line-number="1"></td>
- <td class="lineNum left" data-value="1">1</td>
- <td class="content">
- <div class="contentText" data-side="left">ba ba</div>
- <div data-side="left">
- <div class="comment-thread">
- <div class="gr-formatted-text message">
- <span id="output" class="gr-formatted-text"
- >This is a comment</span
- >
- </div>
- </div>
- </div>
- </td>
- <td class="lineNum right" data-value="1">1</td>
- <td class="content">
- <div class="contentText" data-side="right">some other text</div>
- </td>
- </tr>
- <tr class="diff-row">
- <td class="blame" data-line-number="2"></td>
- <td class="lineNum left" data-value="2">2</td>
- <td class="content">
- <div class="contentText" data-side="left">zin</div>
- </td>
- <td class="lineNum right" data-value="2">2</td>
- <td class="content">
- <div class="contentText" data-side="right">more more more</div>
- <div data-side="right">
- <div class="comment-thread">
- <div class="gr-formatted-text message">
- <span id="output" class="gr-formatted-text"
- >This is a comment on the right</span
- >
- </div>
- </div>
- </div>
- </td>
- </tr>
- <tr class="diff-row">
- <td class="blame" data-line-number="3"></td>
- <td class="lineNum left" data-value="3">3</td>
- <td class="content">
- <div class="contentText" data-side="left">ga ga</div>
- <div data-side="left">
- <div class="comment-thread">
- <div class="gr-formatted-text message">
- <span id="output" class="gr-formatted-text"
- >This is <a>a</a> different comment 💩 unicode is fun</span
- >
- </div>
- </div>
- </div>
- </td>
- <td class="lineNum right" data-value="3">3</td>
- </tr>
- <tr class="diff-row">
- <td class="blame" data-line-number="4"></td>
- <td class="lineNum left" data-value="4">4</td>
- <td class="content">
- <div class="contentText" data-side="left">ga ga</div>
- <div data-side="left">
- <div class="comment-thread">
- <textarea data-side="right">test for textarea copying</textarea>
- </div>
- </div>
- </td>
- <td class="lineNum right" data-value="4">4</td>
- </tr>
- <tr class="not-diff-row">
- <td class="other">
- <div class="contentText" data-side="right">some other text</div>
- </td>
- </tr>
- </table>
-`;
+function firstTextNode(el: HTMLElement) {
+ return [...el.childNodes].filter(node => node.nodeType === Node.TEXT_NODE)[0];
+}
suite('gr-diff-selection', () => {
let element: GrDiffSelection;
- let diffTable: HTMLTableElement;
+ let diffTable: HTMLElement;
+ let grDiff: GrDiff;
const emulateCopyOn = function (target: HTMLElement | null) {
const fakeEvent = {
@@ -112,8 +41,8 @@
};
setup(async () => {
- element = new GrDiffSelection();
- diffTable = await fixture<HTMLTableElement>(diffTableTemplate);
+ grDiff = await fixture<GrDiff>(html`<gr-diff></gr-diff>`);
+ element = grDiff.diffSelection;
const diff: DiffInfo = {
...createDiff(),
@@ -132,51 +61,56 @@
},
],
};
- element.init(diff, diffTable);
+ grDiff.prefs = createDefaultDiffPrefs();
+ grDiff.renderPrefs = {use_lit_components: true};
+ grDiff.diff = diff;
+ await waitForEventOnce(grDiff, 'render');
+ assert.isOk(element.diffTable);
+ diffTable = element.diffTable!;
});
test('applies selected-left on left side click', () => {
- element.diffTable!.classList.add('selected-right');
+ diffTable.classList.add('selected-right');
const lineNumberEl = diffTable.querySelector<HTMLElement>('.lineNum.left');
if (!lineNumberEl) assert.fail('line number element missing');
mouseDown(lineNumberEl);
assert.isTrue(
- element.diffTable!.classList.contains('selected-left'),
+ diffTable.classList.contains('selected-left'),
'adds selected-left'
);
assert.isFalse(
- element.diffTable!.classList.contains('selected-right'),
+ diffTable.classList.contains('selected-right'),
'removes selected-right'
);
});
test('applies selected-right on right side click', () => {
- element.diffTable!.classList.add('selected-left');
+ diffTable.classList.add('selected-left');
const lineNumberEl = diffTable.querySelector<HTMLElement>('.lineNum.right');
if (!lineNumberEl) assert.fail('line number element missing');
mouseDown(lineNumberEl);
assert.isTrue(
- element.diffTable!.classList.contains('selected-right'),
+ diffTable.classList.contains('selected-right'),
'adds selected-right'
);
assert.isFalse(
- element.diffTable!.classList.contains('selected-left'),
+ diffTable.classList.contains('selected-left'),
'removes selected-left'
);
});
test('applies selected-blame on blame click', () => {
- element.diffTable!.classList.add('selected-left');
+ diffTable.classList.add('selected-left');
const blameDiv = document.createElement('div');
blameDiv.classList.add('blame');
- element.diffTable!.appendChild(blameDiv);
+ diffTable.appendChild(blameDiv);
mouseDown(blameDiv);
assert.isTrue(
- element.diffTable!.classList.contains('selected-blame'),
+ diffTable.classList.contains('selected-blame'),
'adds selected-right'
);
assert.isFalse(
- element.diffTable!.classList.contains('selected-left'),
+ diffTable.classList.contains('selected-left'),
'removes selected-left'
);
});
@@ -190,7 +124,7 @@
test('asks for text for left side Elements', () => {
const getSelectedTextStub = sinon.stub(element, 'getSelectedText');
emulateCopyOn(diffTable.querySelector('div.contentText'));
- assert.deepEqual([Side.LEFT, false], getSelectedTextStub.lastCall.args);
+ assert.deepEqual([Side.LEFT], getSelectedTextStub.lastCall.args);
});
test('reacts to copy for content Elements', () => {
@@ -216,25 +150,25 @@
});
test('setClasses adds given SelectionClass values, removes others', () => {
- element.diffTable!.classList.add('selected-right');
+ diffTable.classList.add('selected-right');
element.setClasses(['selected-comment', 'selected-left']);
- assert.isTrue(element.diffTable!.classList.contains('selected-comment'));
- assert.isTrue(element.diffTable!.classList.contains('selected-left'));
- assert.isFalse(element.diffTable!.classList.contains('selected-right'));
- assert.isFalse(element.diffTable!.classList.contains('selected-blame'));
+ assert.isTrue(diffTable.classList.contains('selected-comment'));
+ assert.isTrue(diffTable.classList.contains('selected-left'));
+ assert.isFalse(diffTable.classList.contains('selected-right'));
+ assert.isFalse(diffTable.classList.contains('selected-blame'));
element.setClasses(['selected-blame']);
- assert.isFalse(element.diffTable!.classList.contains('selected-comment'));
- assert.isFalse(element.diffTable!.classList.contains('selected-left'));
- assert.isFalse(element.diffTable!.classList.contains('selected-right'));
- assert.isTrue(element.diffTable!.classList.contains('selected-blame'));
+ assert.isFalse(diffTable.classList.contains('selected-comment'));
+ assert.isFalse(diffTable.classList.contains('selected-left'));
+ assert.isFalse(diffTable.classList.contains('selected-right'));
+ assert.isTrue(diffTable.classList.contains('selected-blame'));
});
test('setClasses removes before it ads', () => {
- element.diffTable!.classList.add('selected-right');
- const addStub = sinon.stub(element.diffTable!.classList, 'add');
+ diffTable.classList.add('selected-right');
+ const addStub = sinon.stub(diffTable.classList, 'add');
const removeStub = sinon
- .stub(element.diffTable!.classList, 'remove')
+ .stub(diffTable.classList, 'remove')
.callsFake(() => {
assert.isFalse(addStub.called);
});
@@ -244,149 +178,43 @@
});
test('copies content correctly', () => {
- element.diffTable!.classList.add('selected-left');
- element.diffTable!.classList.remove('selected-right');
+ diffTable.classList.add('selected-left');
+ diffTable.classList.remove('selected-right');
const selection = document.getSelection();
if (selection === null) assert.fail('no selection');
selection.removeAllRanges();
const range = document.createRange();
- range.setStart(diffTable.querySelector('div.contentText')!.firstChild!, 3);
- range.setEnd(
- diffTable.querySelectorAll('div.contentText')[4]!.firstChild!,
- 2
- );
+ const texts = diffTable.querySelectorAll<HTMLElement>('gr-diff-text');
+ range.setStart(firstTextNode(texts[0]), 3);
+ range.setEnd(firstTextNode(texts[4]), 2);
selection.addRange(range);
- assert.equal(element.getSelectedText(Side.LEFT, false), 'ba\nzin\nga');
- });
- test('copies comments', () => {
- element.diffTable!.classList.add('selected-left');
- element.diffTable!.classList.add('selected-comment');
- element.diffTable!.classList.remove('selected-right');
- const selection = document.getSelection();
- if (selection === null) assert.fail('no selection');
- selection.removeAllRanges();
- const range = document.createRange();
- range.setStart(
- diffTable.querySelector('.gr-formatted-text *')!.firstChild!,
- 3
- );
- range.setEnd(
- diffTable.querySelectorAll('.gr-formatted-text *')[2].childNodes[2],
- 7
- );
- selection.addRange(range);
- assert.equal(
- 's is a comment\nThis is a differ',
- element.getSelectedText(Side.LEFT, true)
- );
- });
-
- test('respects astral chars in comments', () => {
- element.diffTable!.classList.add('selected-left');
- element.diffTable!.classList.add('selected-comment');
- element.diffTable!.classList.remove('selected-right');
- const selection = document.getSelection();
- if (selection === null) assert.fail('no selection');
- selection.removeAllRanges();
- const range = document.createRange();
- const nodes = diffTable.querySelectorAll('.gr-formatted-text *');
- range.setStart(nodes[2].childNodes[2], 13);
- range.setEnd(nodes[2].childNodes[2], 23);
- selection.addRange(range);
- assert.equal('mment 💩 u', element.getSelectedText(Side.LEFT, true));
+ assert.equal(element.getSelectedText(Side.LEFT), 'ba\nzin\nga');
});
test('defers to default behavior for textarea', () => {
- element.diffTable!.classList.add('selected-left');
- element.diffTable!.classList.remove('selected-right');
+ diffTable.classList.add('selected-left');
+ diffTable.classList.remove('selected-right');
const selectedTextSpy = sinon.spy(element, 'getSelectedText');
emulateCopyOn(diffTable.querySelector('textarea'));
+
assert.isFalse(selectedTextSpy.called);
});
test('regression test for 4794', () => {
- element.diffTable!.classList.add('selected-right');
- element.diffTable!.classList.remove('selected-left');
+ diffTable.classList.add('selected-right');
+ diffTable.classList.remove('selected-left');
const selection = document.getSelection();
if (!selection) assert.fail('no selection');
selection.removeAllRanges();
const range = document.createRange();
- range.setStart(
- diffTable.querySelectorAll('div.contentText')[1]!.firstChild!,
- 4
- );
- range.setEnd(
- diffTable.querySelectorAll('div.contentText')[1]!.firstChild!,
- 10
- );
+ const texts = diffTable.querySelectorAll<HTMLElement>('gr-diff-text');
+ range.setStart(firstTextNode(texts[1]), 4);
+ range.setEnd(firstTextNode(texts[1]), 10);
selection.addRange(range);
- assert.equal(element.getSelectedText(Side.RIGHT, false), ' other');
- });
- test('copies to end of side (issue 7895)', () => {
- element.diffTable!.classList.add('selected-left');
- element.diffTable!.classList.remove('selected-right');
- const selection = document.getSelection();
- if (selection === null) assert.fail('no selection');
- selection.removeAllRanges();
- const range = document.createRange();
- range.setStart(diffTable.querySelector('div.contentText')!.firstChild!, 3);
- range.setEnd(
- diffTable.querySelectorAll('div.contentText')[4]!.firstChild!,
- 2
- );
- selection.addRange(range);
- assert.equal(element.getSelectedText(Side.LEFT, false), 'ba\nzin\nga');
- });
-
- suite('getTextContentForRange', () => {
- let selection: Selection;
- let range: Range;
- let nodes: NodeListOf<GrFormattedText>;
-
- setup(() => {
- element.diffTable!.classList.add('selected-left');
- element.diffTable!.classList.add('selected-comment');
- element.diffTable!.classList.remove('selected-right');
- const s = document.getSelection();
- if (s === null) assert.fail('no selection');
- selection = s;
- selection.removeAllRanges();
- range = document.createRange();
- nodes = diffTable.querySelectorAll('.gr-formatted-text *');
- });
-
- test('multi level element contained in range', () => {
- range.setStart(nodes[2].childNodes[0], 1);
- range.setEnd(nodes[2].childNodes[2], 7);
- selection.addRange(range);
- assert.equal(
- element.getTextContentForRange(diffTable, selection, range),
- 'his is a differ'
- );
- });
-
- test('multi level element as startContainer of range', () => {
- range.setStart(nodes[2].childNodes[1], 0);
- range.setEnd(nodes[2].childNodes[2], 7);
- selection.addRange(range);
- assert.equal(
- element.getTextContentForRange(diffTable, selection, range),
- 'a differ'
- );
- });
-
- test('startContainer === endContainer', () => {
- range.setStart(nodes[0].firstChild!, 2);
- range.setEnd(nodes[0].firstChild!, 12);
- selection.addRange(range);
- assert.equal(
- element.getTextContentForRange(diffTable, selection, range),
- 'is is a co'
- );
- });
+ assert.equal(element.getSelectedText(Side.RIGHT), ' other');
});
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
index 7a724b9..59da20d 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
@@ -9,6 +9,7 @@
import {assertIsDefined, assert} from '../../../utils/common-util';
import {untilRendered} from '../../../utils/dom-util';
import {isDefined} from '../../../types/types';
+import {LitElement} from 'lit';
export enum GrDiffGroupType {
/** Unchanged context. */
@@ -433,6 +434,15 @@
return lineRange.start_line <= line && line <= lineRange.end_line;
}
+ startLine(side: Side): LineNumber {
+ if (this.type === GrDiffGroupType.CONTEXT_CONTROL) {
+ return side === Side.LEFT
+ ? this.lineRange.left.start_line
+ : this.lineRange.right.start_line;
+ }
+ return this.lines[0].lineNumber(side);
+ }
+
private _updateRangeWithNewLine(line: GrDiffLine) {
if (
line.beforeNumber === 'FILE' ||
@@ -487,12 +497,13 @@
// This is a temporary hack while migration to lit based diff rendering:
// Elements with 'display: contents;' do not have a height, so they
// won't work as intended with `untilRendered()`.
- const watchEl =
- this.element.tagName === 'GR-DIFF-SECTION'
- ? this.element.firstElementChild
- : this.element;
- assertIsDefined(watchEl);
- await untilRendered(watchEl as HTMLElement);
+ const isLitDiff = this.element.tagName === 'GR-DIFF-SECTION';
+ if (isLitDiff) {
+ await (this.element as LitElement).updateComplete;
+ await untilRendered(this.element.firstElementChild as HTMLElement);
+ } else {
+ await untilRendered(this.element);
+ }
}
/**
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group_test.ts
index 71e2e71d..4a19282 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group_test.ts
@@ -11,6 +11,7 @@
hideInContextControl,
} from './gr-diff-group';
import {assert} from '@open-wc/testing';
+import {Side} from '../../../api/diff';
suite('gr-diff-group tests', () => {
test('delta line pairs', () => {
@@ -252,4 +253,42 @@
assert.isFalse(group.isTotal());
});
});
+
+ suite('startLine', () => {
+ test('DELTA', () => {
+ const lines: GrDiffLine[] = [];
+ lines.push(new GrDiffLine(GrDiffLineType.BOTH, 3, 4));
+ const group = new GrDiffGroup({type: GrDiffGroupType.DELTA, lines});
+ assert.equal(group.startLine(Side.LEFT), 3);
+ assert.equal(group.startLine(Side.RIGHT), 4);
+ });
+
+ test('CONTEXT CONTROL', () => {
+ const lines: GrDiffLine[] = [];
+ lines.push(new GrDiffLine(GrDiffLineType.BOTH, 3, 4));
+ const delta = new GrDiffGroup({type: GrDiffGroupType.DELTA, lines});
+ const group = new GrDiffGroup({
+ type: GrDiffGroupType.CONTEXT_CONTROL,
+ contextGroups: [delta],
+ });
+ assert.equal(group.startLine(Side.LEFT), 3);
+ assert.equal(group.startLine(Side.RIGHT), 4);
+ });
+
+ test('FILE', () => {
+ const lines: GrDiffLine[] = [];
+ lines.push(new GrDiffLine(GrDiffLineType.BOTH, 'FILE', 'FILE'));
+ const group = new GrDiffGroup({type: GrDiffGroupType.DELTA, lines});
+ assert.equal(group.startLine(Side.LEFT), 'FILE');
+ assert.equal(group.startLine(Side.RIGHT), 'FILE');
+ });
+
+ test('LOST', () => {
+ const lines: GrDiffLine[] = [];
+ lines.push(new GrDiffLine(GrDiffLineType.BOTH, 'LOST', 'LOST'));
+ const group = new GrDiffGroup({type: GrDiffGroupType.DELTA, lines});
+ assert.equal(group.startLine(Side.LEFT), 'LOST');
+ assert.equal(group.startLine(Side.RIGHT), 'LOST');
+ });
+ });
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index 569de48..2097170 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -259,7 +259,8 @@
// Private but used in tests.
renderDiffTableTask?: DelayedPromise<void>;
- private diffSelection = new GrDiffSelection();
+ // Private but used in tests.
+ diffSelection = new GrDiffSelection();
// Private but used in tests.
highlights = new GrDiffHighlight();
diff --git a/polygerrit-ui/app/services/service-worker-installer.ts b/polygerrit-ui/app/services/service-worker-installer.ts
index e98f84a..b83713c 100644
--- a/polygerrit-ui/app/services/service-worker-installer.ts
+++ b/polygerrit-ui/app/services/service-worker-installer.ts
@@ -79,10 +79,12 @@
) {
this.allowBrowserNotificationsPreference =
prefs.allow_browser_notifications;
+ // flag can disable notifications similar to user setting
navigator.serviceWorker.controller?.postMessage({
type: ServiceWorkerMessageType.USER_PREFERENCE_CHANGE,
allowBrowserNotificationsPreference:
- this.allowBrowserNotificationsPreference,
+ this.allowBrowserNotificationsPreference &&
+ this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS),
});
}
});
