Merge "Fallback to markdown-less rendering for content that is too long"
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/pg-plugin-checks-api.txt b/Documentation/pg-plugin-checks-api.txt
index 8786cc4..e4cb5d0 100644
--- a/Documentation/pg-plugin-checks-api.txt
+++ b/Documentation/pg-plugin-checks-api.txt
@@ -10,6 +10,10 @@
when a change page is loaded. Such a call would return a list of `Runs` and each
run can contain a list of `Results`.
+`Results` messages will render as markdown. It follows the
+[CommonMark](https://commonmark.org/help/) spec except inline images and direct
+HTML are not rendered and kept as plaintext.
+
The details of the ChecksApi are documented in the
link:https://gerrit.googlesource.com/gerrit/+/master/polygerrit-ui/app/api/checks.ts[source code].
Note that this link points to the `master` branch and might thus reflect a
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/contrib/git-gc-preserve b/contrib/git-gc-preserve
new file mode 100644
index 0000000..54b8fca
--- /dev/null
+++ b/contrib/git-gc-preserve
@@ -0,0 +1,117 @@
+#!/bin/bash
+# Copyright (C) 2022 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+usage() { # error_message
+ cat <<-EOF
+NAME
+ git-gc-preserve - Run git gc and preserve old packs to avoid races for JGit
+
+ This command uses custom git config options to configure if preserved packs
+ from the last run of git gc should be pruned and if packs should be preserved.
+
+ This is similar to the implementation in JGit [1] which is used by
+ JGit to avoid errors [2] in such situations.
+
+ Don't run multiple instances of this command concurrently on the same
+ repository since it does not attempt to implement the file locking
+ which git gc --auto does [3].
+
+ [1] https://git.eclipse.org/r/c/jgit/jgit/+/87969
+ [2] https://git.eclipse.org/r/c/jgit/jgit/+/122288
+ [3] https://github.com/git/git/commit/64a99eb4760de2ce2f0c04e146c0a55c34f50f20
+
+SYNOPSIS
+ git gc-preserve
+
+DESCRIPTION
+ Runs git gc and can preserve old packs to avoid races with concurrently
+ executed commands in JGit.
+
+CONFIGURATION
+ "gc.prunepreserved": if set to "true" preserved packs from the last gc run
+ are pruned before current packs are preserved.
+
+ "gc.preserveoldpacks": if set to "true" current packs will be hard linked
+ to objects/pack/preserved before git gc is executed. JGit will
+ fallback to the preserved packs in this directory in case it comes
+ across missing objects which might be caused by a concurrent run of
+ git gc.
+EOF
+ exit
+}
+
+# prune preserved packs if gc.prunepreserved == true
+prune_preserved() { # repo
+ configured=$(git --git-dir="$1" config --get gc.prunepreserved)
+ if [ "$configured" != "true" ]; then
+ return 0
+ fi
+ local preserved=$1/objects/pack/preserved
+ if [ -d "$preserved" ]; then
+ printf "Pruning old preserved packs: "
+ count=$(find "$preserved" -name "*.old-pack" | wc -l)
+ rm -rf "$preserved"
+ echo "$count, done."
+ fi
+}
+
+# preserve packs if gc.preserveoldpacks == true
+preserve_packs() { # repo
+ configured=$(git --git-dir="$1" config --get gc.preserveoldpacks)
+ if [ "$configured" != "true" ]; then
+ return 0
+ fi
+ local packdir=$1/objects/pack
+ pushd "$packdir" >/dev/null || exit
+ mkdir -p preserved
+ printf "Preserving packs: "
+ count=0
+ for file in pack-*{.pack,.idx} ; do
+ ln -f "$file" preserved/"$(get_preserved_packfile_name "$file")"
+ if [[ "$file" == pack-*.pack ]]; then
+ ((count++))
+ fi
+ done
+ echo "$count, done."
+ popd >/dev/null || exit
+}
+
+# pack-0...2.pack to pack-0...2.old-pack
+# pack-0...2.idx to pack-0...2.old-idx
+get_preserved_packfile_name() { # packfile > preserved_packfile
+ local old=${1/%\.pack/.old-pack}
+ old=${old/%\.idx/.old-idx}
+ echo "$old"
+}
+
+# main
+
+while [ $# -gt 0 ] ; do
+ case "$1" in
+ -u|-h) usage ;;
+ esac
+ shift
+done
+args=$(git rev-parse --sq-quote "$@")
+
+repopath=$(git rev-parse --git-dir)
+if [ -z "$repopath" ]; then
+ usage
+ exit $?
+fi
+
+prune_preserved "$repopath"
+preserve_packs "$repopath"
+git gc ${args:+"$args"}
diff --git a/java/com/google/gerrit/entities/Permission.java b/java/com/google/gerrit/entities/Permission.java
index e3e3a57..6d2fa32 100644
--- a/java/com/google/gerrit/entities/Permission.java
+++ b/java/com/google/gerrit/entities/Permission.java
@@ -278,7 +278,10 @@
public Permission build() {
setRules(
- rulesBuilders.stream().map(PermissionRule.Builder::build).collect(toImmutableList()));
+ rulesBuilders.stream()
+ .map(PermissionRule.Builder::build)
+ .distinct()
+ .collect(toImmutableList()));
return autoBuild();
}
diff --git a/java/com/google/gerrit/entities/PermissionRule.java b/java/com/google/gerrit/entities/PermissionRule.java
index 9a2d31e..1665c1c 100644
--- a/java/com/google/gerrit/entities/PermissionRule.java
+++ b/java/com/google/gerrit/entities/PermissionRule.java
@@ -202,6 +202,9 @@
int dotdot = range.indexOf("..");
int min = parseInt(range.substring(0, dotdot));
int max = parseInt(range.substring(dotdot + 2));
+ if (min > max) {
+ throw new IllegalArgumentException("Invalid range in rule: " + orig);
+ }
rule.setRange(min, max);
} else {
throw new IllegalArgumentException("Invalid range in rule: " + orig);
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 396ba74..44e7854 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1386,22 +1386,21 @@
throw new BadRequestException("Expected JSON object");
}
- @SuppressWarnings("unchecked")
private static Object createInstance(Type type)
throws NoSuchMethodException, InstantiationException, IllegalAccessException,
InvocationTargetException {
if (type instanceof Class) {
- Class<Object> clazz = (Class<Object>) type;
- Constructor<Object> c = clazz.getDeclaredConstructor();
+ Class<?> clazz = (Class<?>) type;
+ Constructor<?> c = clazz.getDeclaredConstructor();
c.setAccessible(true);
return c.newInstance();
}
if (type instanceof ParameterizedType) {
Type rawType = ((ParameterizedType) type).getRawType();
- if (rawType instanceof Class && List.class.isAssignableFrom((Class<Object>) rawType)) {
+ if (rawType instanceof Class && List.class.isAssignableFrom((Class<?>) rawType)) {
return new ArrayList<>();
}
- if (rawType instanceof Class && Map.class.isAssignableFrom((Class<Object>) rawType)) {
+ if (rawType instanceof Class && Map.class.isAssignableFrom((Class<?>) rawType)) {
return new HashMap<>();
}
}
diff --git a/java/com/google/gerrit/index/SchemaFieldDefs.java b/java/com/google/gerrit/index/SchemaFieldDefs.java
index e0b5dd2..db45b8d 100644
--- a/java/com/google/gerrit/index/SchemaFieldDefs.java
+++ b/java/com/google/gerrit/index/SchemaFieldDefs.java
@@ -100,4 +100,12 @@
public interface Setter<I, T> {
void set(I object, T value);
}
+
+ public static boolean isProtoField(SchemaField<?, ?> schemaField) {
+ if (!(schemaField instanceof IndexedField<?, ?>.SearchSpec)) {
+ return false;
+ }
+ IndexedField<?, ?> indexedField = ((IndexedField<?, ?>.SearchSpec) schemaField).getField();
+ return indexedField.isProtoType() || indexedField.isProtoIterableType();
+ }
}
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 591ae26..d60be14 100644
--- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
+++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
@@ -28,6 +28,7 @@
import com.google.gerrit.index.Index;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
+import com.google.gerrit.index.SchemaFieldDefs;
import com.google.gerrit.index.SchemaFieldDefs.SchemaField;
import com.google.gerrit.index.project.ProjectData;
import com.google.gerrit.index.project.ProjectIndex;
@@ -267,9 +268,11 @@
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()) {
- field.setIfPossible(cd, new FakeStoredValue(doc.get(field.getName())));
+ boolean isProtoField = SchemaFieldDefs.isProtoField(field);
+ field.setIfPossible(cd, new FakeStoredValue(doc.get(field.getName()), isProtoField));
}
return cd;
}
diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
index 6eac6f7..c158c65 100644
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -38,16 +38,19 @@
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.Schema.Values;
+import com.google.gerrit.index.SchemaFieldDefs;
import com.google.gerrit.index.SchemaFieldDefs.SchemaField;
import com.google.gerrit.index.query.DataSource;
import com.google.gerrit.index.query.FieldBundle;
import com.google.gerrit.index.query.ListResultSet;
import com.google.gerrit.index.query.ResultSet;
+import com.google.gerrit.proto.Protos;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.index.IndexUtils;
import com.google.gerrit.server.index.options.AutoFlush;
import com.google.gerrit.server.logging.LoggingContextAwareExecutorService;
import com.google.gerrit.server.logging.LoggingContextAwareScheduledExecutorService;
+import com.google.protobuf.MessageLite;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.Set;
@@ -366,8 +369,12 @@
doc.add(new TextField(name, (String) value, store));
}
} else if (type == FieldType.STORED_ONLY) {
+ boolean isProtoField = SchemaFieldDefs.isProtoField(values.getField());
for (Object value : values.getValues()) {
- doc.add(new StoredField(name, (byte[]) value));
+ // Lucene stores protos as bytes
+ doc.add(
+ new StoredField(
+ name, isProtoField ? Protos.toByteArray((MessageLite) value) : (byte[]) value));
}
} else {
throw FieldType.badFieldType(type);
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 9ac85e0..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)
+ .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/approval/ApprovalCopier.java b/java/com/google/gerrit/server/approval/ApprovalCopier.java
index cd2d79e..a1889da 100644
--- a/java/com/google/gerrit/server/approval/ApprovalCopier.java
+++ b/java/com/google/gerrit/server/approval/ApprovalCopier.java
@@ -46,6 +46,7 @@
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.approval.ApprovalContext;
import com.google.gerrit.server.query.approval.ApprovalQueryBuilder;
+import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
@@ -353,10 +354,25 @@
Predicate<ApprovalContext> copyConditionPredicate =
approvalQueryBuilder.parse(labelType.getCopyCondition().get());
boolean canCopy = copyConditionPredicate.asMatchable().match(ctx);
- ImmutableSet.Builder<String> passingAtoms = ImmutableSet.builder();
- ImmutableSet.Builder<String> failingAtoms = ImmutableSet.builder();
- evaluateAtoms(copyConditionPredicate, ctx, passingAtoms, failingAtoms);
- return ApprovalCopyResult.create(canCopy, passingAtoms.build(), failingAtoms.build());
+ ImmutableSet.Builder<String> passingAtomsBuilder = ImmutableSet.builder();
+ ImmutableSet.Builder<String> failingAtomsBuilder = ImmutableSet.builder();
+ evaluateAtoms(copyConditionPredicate, ctx, passingAtomsBuilder, failingAtomsBuilder);
+ ImmutableSet<String> passingAtoms = passingAtomsBuilder.build();
+ ImmutableSet<String> failingAtoms = failingAtomsBuilder.build();
+ logger.atFine().log(
+ "%s copy %s of account %d on change %d from patch set %d to patch set %d"
+ + " (copyCondition = %s, passingAtoms = %s, failingAtoms = %s, changeKind = %s)",
+ canCopy ? "Can" : "Cannot",
+ LabelVote.create(labelType.getName(), approvalValue).format(),
+ approverId.get(),
+ changeNotes.getChangeId().get(),
+ sourcePatchSetId.get(),
+ targetPatchSet.id().get(),
+ labelType.getCopyCondition().get(),
+ passingAtoms,
+ failingAtoms,
+ changeKind.name());
+ return ApprovalCopyResult.create(canCopy, passingAtoms, failingAtoms);
}
} catch (QueryParseException e) {
logger.atWarning().withCause(e).log(
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 7aefa63..254a7e5 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -18,11 +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.integer;
-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;
@@ -42,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;
@@ -59,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;
@@ -126,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 =
@@ -194,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 =
@@ -346,8 +360,7 @@
/** Folders that are touched by the current patch set. */
public static final IndexedField<ChangeData, Iterable<String>> DIRECTORY_FIELD =
- IndexedField.<ChangeData>iterableStringBuilder("Directory")
- .build(ChangeField::getDirectories);
+ IndexedField.<ChangeData>iterableStringBuilder("DirField").build(ChangeField::getDirectories);
public static final IndexedField<ChangeData, Iterable<String>>.SearchSpec DIRECTORY_SPEC =
DIRECTORY_FIELD.exact(ChangeQueryBuilder.FIELD_DIRECTORY);
@@ -557,6 +570,7 @@
public static final IndexedField<ChangeData, String> IS_PURE_REVERT_FIELD =
IndexedField.<ChangeData>stringBuilder("IsPureRevert")
+ .size(1)
.build(cd -> Boolean.TRUE.equals(cd.isPureRevert()) ? "1" : "0");
public static final IndexedField<ChangeData, String>.SearchSpec IS_PURE_REVERT_SPEC =
@@ -568,6 +582,7 @@
*/
public static final IndexedField<ChangeData, String> IS_SUBMITTABLE_FIELD =
IndexedField.<ChangeData>stringBuilder("IsSubmittable")
+ .size(1)
.build(
cd ->
// All submit requirements should be fulfilled
@@ -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);
@@ -1046,6 +1086,15 @@
public static final IndexedField<ChangeData, String>.SearchSpec COMMIT_MESSAGE_EXACT =
COMMIT_MESSAGE_EXACT_FIELD.exact(ChangeQueryBuilder.FIELD_MESSAGE_EXACT);
+ /** Commit message of the current patch set. */
+ public static final IndexedField<ChangeData, String> SUBJECT_FIELD =
+ IndexedField.<ChangeData>stringBuilder("Subject")
+ .required()
+ .build(changeGetter(Change::getSubject));
+
+ public static final IndexedField<ChangeData, String>.SearchSpec SUBJECT_SPEC =
+ SUBJECT_FIELD.fullText(ChangeQueryBuilder.FIELD_SUBJECT);
+
/** Summary or inline comment. */
public static final IndexedField<ChangeData, Iterable<String>> COMMENT_FIELD =
IndexedField.<ChangeData>iterableStringBuilder("Comment")
@@ -1211,10 +1260,10 @@
COMMENTBY_FIELD.integer(ChangeQueryBuilder.FIELD_COMMENTBY);
/** Star labels on this change in the format: <account-id>:<label> */
- public static final FieldDef<ChangeData, Iterable<String>> STAR =
- exact(ChangeQueryBuilder.FIELD_STAR)
+ public static final IndexedField<ChangeData, Iterable<String>> STAR_FIELD =
+ IndexedField.<ChangeData>iterableStringBuilder("Star")
.stored()
- .buildRepeatable(
+ .build(
cd ->
Iterables.transform(
cd.stars().entries(),
@@ -1226,33 +1275,61 @@
.map(f -> StarredChangesUtil.StarField.parse(f))
.collect(toImmutableListMultimap(e -> e.accountId(), e -> e.label()))));
+ public static final IndexedField<ChangeData, Iterable<String>>.SearchSpec STAR_SPEC =
+ STAR_FIELD.exact(ChangeQueryBuilder.FIELD_STAR);
+
/** Users that have starred the change with any label. */
- public static final FieldDef<ChangeData, Iterable<Integer>> STARBY =
- integer(ChangeQueryBuilder.FIELD_STARBY)
- .buildRepeatable(cd -> Iterables.transform(cd.stars().keySet(), Account.Id::get));
+ public static final IndexedField<ChangeData, Iterable<Integer>> STARBY_FIELD =
+ IndexedField.<ChangeData>iterableIntegerBuilder("StarBy")
+ .build(cd -> Iterables.transform(cd.stars().keySet(), Account.Id::get));
+
+ public static final IndexedField<ChangeData, Iterable<Integer>>.SearchSpec STARBY_SPEC =
+ STARBY_FIELD.integer(ChangeQueryBuilder.FIELD_STARBY);
/** Opaque group identifiers for this change's patch sets. */
- public static final FieldDef<ChangeData, Iterable<String>> GROUP =
- exact(ChangeQueryBuilder.FIELD_GROUP)
- .buildRepeatable(
+ public static final IndexedField<ChangeData, Iterable<String>> GROUP_FIELD =
+ IndexedField.<ChangeData>iterableStringBuilder("Group")
+ .build(
cd -> cd.patchSets().stream().flatMap(ps -> ps.groups().stream()).collect(toSet()));
+ public static final IndexedField<ChangeData, Iterable<String>>.SearchSpec GROUP_SPEC =
+ 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 FieldDef<ChangeData, Iterable<Integer>> EDITBY =
- integer(ChangeQueryBuilder.FIELD_EDITBY)
- .buildRepeatable(cd -> cd.editsByUser().stream().map(Account.Id::get).collect(toSet()));
+ public static final IndexedField<ChangeData, Iterable<Integer>> EDITBY_FIELD =
+ IndexedField.<ChangeData>iterableIntegerBuilder("EditBy")
+ .build(cd -> cd.editsByUser().stream().map(Account.Id::get).collect(toSet()));
+
+ public static final IndexedField<ChangeData, Iterable<Integer>>.SearchSpec EDITBY_SPEC =
+ EDITBY_FIELD.integer(ChangeQueryBuilder.FIELD_EDITBY);
/** Users who have draft comments on this change. */
- public static final FieldDef<ChangeData, Iterable<Integer>> DRAFTBY =
- integer(ChangeQueryBuilder.FIELD_DRAFTBY)
- .buildRepeatable(cd -> cd.draftsByUser().stream().map(Account.Id::get).collect(toSet()));
+ public static final IndexedField<ChangeData, Iterable<Integer>> DRAFTBY_FIELD =
+ IndexedField.<ChangeData>iterableIntegerBuilder("DraftBy")
+ .build(cd -> cd.draftsByUser().stream().map(Account.Id::get).collect(toSet()));
+
+ public static final IndexedField<ChangeData, Iterable<Integer>>.SearchSpec DRAFTBY_SPEC =
+ DRAFTBY_FIELD.integer(ChangeQueryBuilder.FIELD_DRAFTBY);
public static final Integer NOT_REVIEWED = -1;
@@ -1266,10 +1343,10 @@
* <p>If the latest update is by the change owner, then the special value {@link #NOT_REVIEWED} is
* emitted.
*/
- public static final FieldDef<ChangeData, Iterable<Integer>> REVIEWEDBY =
- integer(ChangeQueryBuilder.FIELD_REVIEWEDBY)
+ public static final IndexedField<ChangeData, Iterable<Integer>> REVIEWEDBY_FIELD =
+ IndexedField.<ChangeData>iterableIntegerBuilder("ReviewedBy")
.stored()
- .buildRepeatable(
+ .build(
cd -> {
Set<Account.Id> reviewedBy = cd.reviewedBy();
if (reviewedBy.isEmpty()) {
@@ -1283,6 +1360,9 @@
.map(Account::id)
.collect(toImmutableSet())));
+ public static final IndexedField<ChangeData, Iterable<Integer>>.SearchSpec REVIEWEDBY_SPEC =
+ REVIEWEDBY_FIELD.integer(ChangeQueryBuilder.FIELD_REVIEWEDBY);
+
public static final SubmitRuleOptions SUBMIT_RULE_OPTIONS_LENIENT =
SubmitRuleOptions.builder().recomputeOnClosedChanges(true).build();
@@ -1290,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);
@@ -1302,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.
*
@@ -1388,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(
@@ -1403,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) {
@@ -1518,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())));
@@ -1544,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()
@@ -1556,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();
@@ -1583,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();
@@ -1592,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 2af5d60..5dc1500 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -34,33 +34,17 @@
static final Schema<ChangeData> V74 =
schema(
/* version= */ 74,
- ImmutableList.of(
- ChangeField.APPROVAL,
- ChangeField.CHANGE,
- ChangeField.DRAFTBY,
- ChangeField.EDITBY,
- ChangeField.GROUP,
- ChangeField.ID,
- ChangeField.LEGACY_ID_STR,
- ChangeField.PATCH_SET,
- ChangeField.REF_STATE,
- ChangeField.REF_STATE_PATTERN,
- ChangeField.REVIEWEDBY,
- ChangeField.STAR,
- ChangeField.STARBY,
- 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,
@@ -72,11 +56,14 @@
ChangeField.DELETED_LINES_FIELD,
ChangeField.DELTA_LINES_FIELD,
ChangeField.DIRECTORY_FIELD,
+ ChangeField.DRAFTBY_FIELD,
+ ChangeField.EDITBY_FIELD,
ChangeField.EXACT_AUTHOR_FIELD,
ChangeField.EXACT_COMMITTER_FIELD,
ChangeField.EXTENSION_FIELD,
ChangeField.FILE_PART_FIELD,
ChangeField.FOOTER_FIELD,
+ ChangeField.GROUP_FIELD,
ChangeField.HASHTAG_CASE_AWARE_FIELD,
ChangeField.HASHTAG_FIELD,
ChangeField.IS_PURE_REVERT_FIELD,
@@ -85,33 +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),
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,
@@ -123,6 +126,8 @@
ChangeField.DELETED_LINES_SPEC,
ChangeField.DELTA_LINES_SPEC,
ChangeField.DIRECTORY_SPEC,
+ ChangeField.DRAFTBY_SPEC,
+ ChangeField.EDITBY_SPEC,
ChangeField.EXACT_AUTHOR_SPEC,
ChangeField.EXACT_COMMITTER_SPEC,
ChangeField.EXACT_COMMIT_SPEC,
@@ -132,6 +137,7 @@
ChangeField.FOOTER_SPEC,
ChangeField.FUZZY_HASHTAG,
ChangeField.FUZZY_TOPIC,
+ ChangeField.GROUP_SPEC,
ChangeField.HASHTAG_CASE_AWARE_SPEC,
ChangeField.HASHTAG_SPEC,
ChangeField.IS_PURE_REVERT_SPEC,
@@ -140,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,
@@ -149,15 +157,26 @@
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));
@@ -198,8 +217,18 @@
static final Schema<ChangeData> V79 =
new Schema.Builder<ChangeData>()
.add(V78)
- .remove(ChangeField.DRAFTBY, ChangeField.STAR, ChangeField.STARBY)
+ .remove(ChangeField.STAR_SPEC, ChangeField.STARBY_SPEC, ChangeField.DRAFTBY_SPEC)
+ .remove(ChangeField.STAR_FIELD, ChangeField.STARBY_FIELD, ChangeField.DRAFTBY_FIELD)
.build();
+
+ /** Add subject field. */
+ static final Schema<ChangeData> V80 =
+ new Schema.Builder<ChangeData>()
+ .add(V79)
+ .addIndexedFields(ChangeField.SUBJECT_FIELD)
+ .addSearchSpecs(ChangeField.SUBJECT_SPEC)
+ .build();
+
/**
* Name of the change index to be used when contacting index backends or loading configurations.
*/
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 6b8cdd8..84ceb3d 100644
--- a/java/com/google/gerrit/server/query/change/ChangePredicates.java
+++ b/java/com/google/gerrit/server/query/change/ChangePredicates.java
@@ -74,7 +74,7 @@
* com.google.gerrit.entities.Account.Id} has a pending change edit.
*/
public static Predicate<ChangeData> editBy(Account.Id id) {
- return new ChangeIndexPredicate(ChangeField.EDITBY, id.toString());
+ return new ChangeIndexPredicate(ChangeField.EDITBY_SPEC, id.toString());
}
/**
@@ -111,7 +111,7 @@
public static Predicate<ChangeData> reviewedBy(Collection<Account.Id> ids) {
List<Predicate<ChangeData>> predicates = new ArrayList<>(ids.size());
for (Account.Id id : ids) {
- predicates.add(new ChangeIndexPredicate(ChangeField.REVIEWEDBY, id.toString()));
+ predicates.add(new ChangeIndexPredicate(ChangeField.REVIEWEDBY_SPEC, id.toString()));
}
return Predicate.or(predicates);
}
@@ -119,7 +119,7 @@
/** Returns a predicate that matches changes that were not yet reviewed. */
public static Predicate<ChangeData> unreviewed() {
return Predicate.not(
- new ChangeIndexPredicate(ChangeField.REVIEWEDBY, ChangeField.NOT_REVIEWED.toString()));
+ new ChangeIndexPredicate(ChangeField.REVIEWEDBY_SPEC, ChangeField.NOT_REVIEWED.toString()));
}
/**
@@ -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);
}
/**
@@ -332,6 +332,10 @@
return new ChangeIndexPredicate(ChangeField.COMMIT_MESSAGE, message);
}
+ public static Predicate<ChangeData> subject(String subject) {
+ return new ChangeIndexPredicate(ChangeField.SUBJECT_SPEC, subject);
+ }
+
/**
* Returns a predicate that matches changes where the provided {@code comment} appears in any
* comment on any patch set of the change. Uses full-text search semantics.
@@ -348,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 b433b25..83c348c 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -186,6 +186,7 @@
public static final String FIELD_MERGEABLE = "mergeable2";
public static final String FIELD_MERGED_ON = "mergedon";
public static final String FIELD_MESSAGE = "message";
+ public static final String FIELD_SUBJECT = "subject";
public static final String FIELD_MESSAGE_EXACT = "messageexact";
public static final String FIELD_OWNER = "owner";
public static final String FIELD_OWNERIN = "ownerin";
@@ -519,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
@@ -529,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
@@ -1140,6 +1141,12 @@
return ChangePredicates.message(text);
}
+ @Operator
+ public Predicate<ChangeData> subject(String value) throws QueryParseException {
+ checkFieldAvailable(ChangeField.SUBJECT_SPEC, ChangeQueryBuilder.FIELD_SUBJECT);
+ return ChangePredicates.subject(value);
+ }
+
private Predicate<ChangeData> starredBySelf() throws QueryParseException {
return ChangePredicates.starBy(
args.starredChangesUtil, self(), StarredChangesUtil.DEFAULT_LABEL);
diff --git a/java/com/google/gerrit/server/query/change/GroupPredicate.java b/java/com/google/gerrit/server/query/change/GroupPredicate.java
index f470cf9..c4aba0d 100644
--- a/java/com/google/gerrit/server/query/change/GroupPredicate.java
+++ b/java/com/google/gerrit/server/query/change/GroupPredicate.java
@@ -20,7 +20,7 @@
public class GroupPredicate extends ChangeIndexPredicate {
public GroupPredicate(String group) {
- super(ChangeField.GROUP, group);
+ super(ChangeField.GROUP_SPEC, group);
}
@Override
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/java/com/google/gerrit/server/restapi/project/SetAccess.java b/java/com/google/gerrit/server/restapi/project/SetAccess.java
index 23d60fe..6957275 100644
--- a/java/com/google/gerrit/server/restapi/project/SetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/SetAccess.java
@@ -19,6 +19,9 @@
import com.google.gerrit.entities.AccessSection;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.InvalidNameException;
+import com.google.gerrit.extensions.api.access.AccessSectionInfo;
+import com.google.gerrit.extensions.api.access.PermissionInfo;
+import com.google.gerrit.extensions.api.access.PermissionRuleInfo;
import com.google.gerrit.extensions.api.access.ProjectAccessInfo;
import com.google.gerrit.extensions.api.access.ProjectAccessInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -39,6 +42,7 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.List;
+import java.util.Map;
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
@@ -80,6 +84,8 @@
throws Exception {
MetaDataUpdate.User metaDataUpdateUser = metaDataUpdateFactory.get();
+ validateInput(input);
+
ProjectConfig config;
List<AccessSection> removals =
@@ -137,4 +143,65 @@
return Response.ok(getAccess.apply(rsrc.getNameKey()));
}
+
+ private static void validateInput(ProjectAccessInput input) throws BadRequestException {
+ if (input.add != null) {
+ for (Map.Entry<String, AccessSectionInfo> accessSectionEntry : input.add.entrySet()) {
+ validateAccessSection(accessSectionEntry.getKey(), accessSectionEntry.getValue());
+ }
+ }
+ }
+
+ private static void validateAccessSection(String ref, AccessSectionInfo accessSectionInfo)
+ throws BadRequestException {
+ if (accessSectionInfo != null) {
+ for (Map.Entry<String, PermissionInfo> permissionEntry :
+ accessSectionInfo.permissions.entrySet()) {
+ validatePermission(ref, permissionEntry.getKey(), permissionEntry.getValue());
+ }
+ }
+ }
+
+ private static void validatePermission(
+ String ref, String permission, PermissionInfo permissionInfo) throws BadRequestException {
+ if (permissionInfo != null) {
+ for (Map.Entry<String, PermissionRuleInfo> permissionRuleEntry :
+ permissionInfo.rules.entrySet()) {
+ validatePermissionRule(
+ ref, permission, permissionRuleEntry.getKey(), permissionRuleEntry.getValue());
+ }
+ }
+ }
+
+ private static void validatePermissionRule(
+ String ref, String permission, String groupId, PermissionRuleInfo permissionRuleInfo)
+ throws BadRequestException {
+ if (permissionRuleInfo != null) {
+ if (permissionRuleInfo.min != null || permissionRuleInfo.max != null) {
+ if (permissionRuleInfo.min == null) {
+ throw new BadRequestException(
+ String.format(
+ "Invalid range for permission rule that assigns %s to group %s on ref %s:"
+ + " ..%d (min is required if max is set)",
+ permission, groupId, ref, permissionRuleInfo.max));
+ }
+
+ if (permissionRuleInfo.max == null) {
+ throw new BadRequestException(
+ String.format(
+ "Invalid range for permission rule that assigns %s to group %s on ref %s:"
+ + " %d.. (max is required if min is set)",
+ permission, groupId, ref, permissionRuleInfo.min));
+ }
+
+ if (permissionRuleInfo.min > permissionRuleInfo.max) {
+ throw new BadRequestException(
+ String.format(
+ "Invalid range for permission rule that assigns %s to group %s on ref %s:"
+ + " %d..%d (min must be <= max)",
+ permission, groupId, ref, permissionRuleInfo.min, permissionRuleInfo.max));
+ }
+ }
+ }
+ }
}
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/acceptance/api/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
index 7c33ec2..6bd2b68 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
@@ -67,6 +67,7 @@
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.schema.GrantRevertPermission;
import com.google.inject.Inject;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -74,6 +75,7 @@
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository;
@@ -263,6 +265,38 @@
}
@Test
+ public void addDuplicatedAccessSection_doesNotAddDuplicateEntry() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ // Update project config. Record the file content and the refs_config object ID
+ accessInput.add.put(REFS_HEADS, accessSectionInfo);
+ pApi().access(accessInput);
+ ObjectId refsConfigId =
+ projectOperations.project(newProjectName).getHead(RefNames.REFS_CONFIG).getId();
+ List<String> projectConfigLines =
+ Arrays.asList(projectOperations.project(newProjectName).getConfig().toText().split("\n"));
+ assertThat(projectConfigLines)
+ .containsExactly(
+ "[submit]",
+ "\taction = inherit",
+ "[access \"refs/heads/*\"]",
+ "\tlabel-Code-Review = deny group Registered Users",
+ "\tlabel-Code-Review = -1..+1 group Project Owners",
+ "\tpush = group Registered Users");
+
+ // Apply the same update once more. Make sure that the file content and the ref did not change
+ pApi().access(accessInput);
+
+ List<String> newProjectConfigLines =
+ Arrays.asList(projectOperations.project(newProjectName).getConfig().toText().split("\n"));
+ ObjectId newRefsConfigId =
+ projectOperations.project(newProjectName).getHead(RefNames.REFS_CONFIG).getId();
+ assertThat(projectConfigLines).isEqualTo(newProjectConfigLines);
+ assertThat(refsConfigId).isEqualTo(newRefsConfigId);
+ }
+
+ @Test
public void addAccessSectionForPluginPermission() throws Exception {
try (Registration registration =
extensionRegistry
@@ -325,6 +359,79 @@
}
@Test
+ public void addAccessSectionWithInvalidLabelRange_minGreaterThanMax() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ PermissionInfo permissionInfo = newPermissionInfo();
+ PermissionRuleInfo permissionRuleInfo =
+ new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+ permissionInfo.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), permissionRuleInfo);
+ permissionRuleInfo.min = 1;
+ permissionRuleInfo.max = -1;
+ accessSectionInfo.permissions.put("label-Code-Review", permissionInfo);
+
+ accessInput.add.put(REFS_HEADS, accessSectionInfo);
+ BadRequestException ex =
+ assertThrows(BadRequestException.class, () -> pApi().access(accessInput));
+ assertThat(ex)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Invalid range for permission rule that assigns label-Code-Review to group %s"
+ + " on ref refs/heads/*: 1..-1 (min must be <= max)",
+ SystemGroupBackend.REGISTERED_USERS.get()));
+ }
+
+ @Test
+ public void addAccessSectionWithInvalidLabelRange_minSetMaxMissing() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ PermissionInfo permissionInfo = newPermissionInfo();
+ PermissionRuleInfo permissionRuleInfo =
+ new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+ permissionInfo.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), permissionRuleInfo);
+ permissionRuleInfo.min = -1;
+ accessSectionInfo.permissions.put("label-Code-Review", permissionInfo);
+
+ accessInput.add.put(REFS_HEADS, accessSectionInfo);
+ BadRequestException ex =
+ assertThrows(BadRequestException.class, () -> pApi().access(accessInput));
+ assertThat(ex)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Invalid range for permission rule that assigns label-Code-Review to group %s"
+ + " on ref refs/heads/*: -1.. (max is required if min is set)",
+ SystemGroupBackend.REGISTERED_USERS.get()));
+ }
+
+ @Test
+ public void addAccessSectionWithInvalidLabelRange_maxSetMinMissing() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ PermissionInfo permissionInfo = newPermissionInfo();
+ PermissionRuleInfo permissionRuleInfo =
+ new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+ permissionInfo.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), permissionRuleInfo);
+ permissionRuleInfo.max = 1;
+ accessSectionInfo.permissions.put("label-Code-Review", permissionInfo);
+
+ accessInput.add.put(REFS_HEADS, accessSectionInfo);
+ BadRequestException ex =
+ assertThrows(BadRequestException.class, () -> pApi().access(accessInput));
+ assertThat(ex)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Invalid range for permission rule that assigns label-Code-Review to group %s"
+ + " on ref refs/heads/*: ..1 (min is required if max is set)",
+ SystemGroupBackend.REGISTERED_USERS.get()));
+ }
+
+ @Test
public void createAccessChangeNop() throws Exception {
ProjectAccessInput accessInput = newProjectAccessInput();
assertThrows(BadRequestException.class, () -> pApi().accessChange(accessInput));
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
index 5fd2159..28a0196 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.acceptance.GitUtil.fetch;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -32,6 +33,8 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.git.ObjectIds;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.project.GroupList;
import com.google.gerrit.server.project.LabelConfigValidator;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.Inject;
@@ -155,6 +158,81 @@
}
@Test
+ public void rejectCreatingLabelPermissionWithInvalidRange_minGreaterThanMax() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ImmutableMap.of(
+ ProjectConfig.PROJECT_CONFIG,
+ "[access \"refs/heads/*\"]\n label-Code-Review = 1..-1 group Registered-Users",
+ GroupList.FILE_NAME,
+ String.format("%s\tRegistered-Users", SystemGroupBackend.REGISTERED_USERS.get())));
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: invalid project configuration:\n"
+ + "ERROR: commit %s: project.config: invalid rule in"
+ + " access.refs/heads/*.label-Code-Review:"
+ + " invalid range in rule: 1..-1 group Registered-Users",
+ abbreviateName(r.getCommit()), abbreviateName(r.getCommit())));
+ }
+
+ @Test
+ public void rejectCreatingLabelPermissionWithInvalidRange_minSetMaxMissing() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ImmutableMap.of(
+ ProjectConfig.PROJECT_CONFIG,
+ "[access \"refs/heads/*\"]\n label-Code-Review = -1.. group Registered-Users",
+ GroupList.FILE_NAME,
+ String.format("%s\tRegistered-Users", SystemGroupBackend.REGISTERED_USERS.get())));
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: invalid project configuration:\n"
+ + "ERROR: commit %s: project.config: invalid rule in"
+ + " access.refs/heads/*.label-Code-Review:"
+ + " invalid range in rule: -1.. group Registered-Users",
+ abbreviateName(r.getCommit()), abbreviateName(r.getCommit())));
+ }
+
+ @Test
+ public void rejectCreatingLabelPermissionWithInvalidRange_maxSetMinMissing() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ImmutableMap.of(
+ ProjectConfig.PROJECT_CONFIG,
+ "[access \"refs/heads/*\"]\n label-Code-Review = ..1 group Registered-Users",
+ GroupList.FILE_NAME,
+ String.format("%s\tRegistered-Users", SystemGroupBackend.REGISTERED_USERS.get())));
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: invalid project configuration:\n"
+ + "ERROR: commit %s: project.config: invalid rule in"
+ + " access.refs/heads/*.label-Code-Review:"
+ + " invalid range in rule: ..1 group Registered-Users",
+ abbreviateName(r.getCommit()), abbreviateName(r.getCommit())));
+ }
+
+ @Test
public void rejectSettingCopyMinScore() throws Exception {
testRejectSettingLabelFlag(
LabelConfigValidator.KEY_COPY_MIN_SCORE, /* value= */ true, "is:MIN");
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
index fed22f2..c1a7627 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
@@ -329,7 +329,7 @@
}
@Test
- public void addDuplicatePermissions() throws Exception {
+ public void addDuplicatePermissions_isIgnored() throws Exception {
TestPermission permission =
TestProjectUpdate.allow(Permission.ABANDON).ref("refs/foo").group(REGISTERED_USERS).build();
Project.NameKey key = projectOperations.newProject().create();
@@ -340,9 +340,8 @@
assertThat(config).subsections("access").containsExactly("refs/foo");
assertThat(config)
.subsectionValues("access", "refs/foo")
- .containsExactly(
- "abandon", "group global:Registered-Users",
- "abandon", "group global:Registered-Users");
+ // Duplicated permission was recorded only once
+ .containsExactly("abandon", "group global:Registered-Users");
projectOperations.project(key).forUpdate().add(permission).update();
config = projectOperations.project(key).getConfig();
@@ -350,10 +349,8 @@
assertThat(config).subsections("access").containsExactly("refs/foo");
assertThat(config)
.subsectionValues("access", "refs/foo")
- .containsExactly(
- "abandon", "group global:Registered-Users",
- "abandon", "group global:Registered-Users",
- "abandon", "group global:Registered-Users");
+ // Duplicated permission in request was dropped
+ .containsExactly("abandon", "group global:Registered-Users");
}
@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/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index d7ec030..23f33fc 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -148,6 +148,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -635,7 +636,7 @@
Change change1 = insert(repo, newChange(repo), userId);
assertQuery("is:uploader", change1);
assertQuery("uploader:" + userId.get(), change1);
- change1 = newPatchSet(repo, change1, user2CurrentUser);
+ change1 = newPatchSet(repo, change1, user2CurrentUser, /* message= */ Optional.empty());
// Uploader has changed
assertQuery("uploader:" + userId.get());
assertQuery("uploader:" + user2.get(), change1);
@@ -770,8 +771,9 @@
Account.Id user2 =
accountManager.authenticate(authRequestFactory.createForUser("anotheruser")).getAccountId();
CurrentUser user2CurrentUser = userFactory.create(user2);
- newPatchSet(repo, change1, user2CurrentUser);
+ change1 = newPatchSet(repo, change1, user2CurrentUser, /* message= */ Optional.empty());
assertQuery("uploaderin:Administrators");
+ assertQuery("uploaderin:\"Registered Users\"", change1);
}
@Test
@@ -1033,6 +1035,55 @@
}
@Test
+ public void bySubject() throws Exception {
+ assume().that(getSchema().hasField(ChangeField.SUBJECT_SPEC)).isTrue();
+ TestRepository<Repo> repo = createProject("repo");
+ RevCommit commit1 =
+ repo.parseBody(
+ repo.commit()
+ .message(
+ "First commit with test subject\n\n"
+ + "Message body\n\n"
+ + "Change-Id: I986c6a013dd5b3a2e8a0271c04deac2c9752b920")
+ .create());
+ Change change1 = insert(repo, newChangeForCommit(repo, commit1));
+ RevCommit commit2 =
+ repo.parseBody(
+ repo.commit()
+ .message(
+ "Second commit with test subject\n\n"
+ + "Message body for another commit\n\n"
+ + "Change-Id: I986c6a013dd5b3a2e8a0271c04deac2c9752b921")
+ .create());
+ Change change2 = insert(repo, newChangeForCommit(repo, commit2));
+ RevCommit commit3 =
+ repo.parseBody(
+ repo.commit()
+ .message(
+ "Third commit with test subject\n\n"
+ + "Last message body\n\n"
+ + "Change-Id: I986c6a013dd5b3a2e8a0271c04deac2c9752b921")
+ .create());
+ Change change3 = insert(repo, newChangeForCommit(repo, commit3));
+
+ assertQuery("subject:First", change1);
+ assertQuery("subject:Second", change2);
+ assertQuery("subject:Third", change3);
+ assertQuery("subject:\"commit with test subject\"", change3, change2, change1);
+ assertQuery("subject:\"Message body\"");
+ assertQuery("subject:body");
+ change1 =
+ newPatchSet(
+ repo,
+ change1,
+ user,
+ Optional.of("Rework of commit with test subject\n\n" + "Message body\n\n"));
+ assertQuery("subject:Rework", change1);
+ assertQuery("subject:First");
+ assertQuery("subject:\"commit with test subject\"", change1, change3, change2);
+ }
+
+ @Test
public void fullTextWithNumbers() throws Exception {
TestRepository<Repo> repo = createProject("repo");
RevCommit commit1 = repo.parseBody(repo.commit().message("12345 67890").create());
@@ -2807,7 +2858,7 @@
gApi.changes().id(change2.getId().get()).current().review(new ReviewInput().message("comment"));
PatchSet.Id ps3_1 = change3.currentPatchSetId();
- change3 = newPatchSet(repo, change3, user);
+ change3 = newPatchSet(repo, change3, user, /* message= */ Optional.empty());
assertThat(change3.currentPatchSetId()).isNotEqualTo(ps3_1);
// Response to previous patch set still counts as reviewing.
gApi.changes()
@@ -4040,13 +4091,18 @@
}
}
- protected Change newPatchSet(TestRepository<Repo> repo, Change c, CurrentUser user)
+ protected Change newPatchSet(
+ TestRepository<Repo> repo, Change c, CurrentUser user, Optional<String> message)
throws Exception {
// Add a new file so the patch set is not a trivial rebase, to avoid default
// Code-Review label copying.
int n = c.currentPatchSetId().get() + 1;
RevCommit commit =
- repo.parseBody(repo.commit().message("message").add("file" + n, "contents " + n).create());
+ repo.parseBody(
+ repo.commit()
+ .message(message.orElse("message"))
+ .add("file" + n, "contents " + n)
+ .create());
PatchSetInserter inserter =
patchSetFactory
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/package.json b/package.json
index af1354b..ae1bb2f 100644
--- a/package.json
+++ b/package.json
@@ -18,7 +18,7 @@
"eslint-config-google": "^0.14.0",
"eslint-plugin-html": "^6.2.0",
"eslint-plugin-import": "^2.26.0",
- "eslint-plugin-jsdoc": "^39.3.2",
+ "eslint-plugin-jsdoc": "^39.6.4",
"eslint-plugin-lit": "^1.6.1",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^4.0.0",
diff --git a/polygerrit-ui/app/api/plugin.ts b/polygerrit-ui/app/api/plugin.ts
index 733f112..3ed48e6 100644
--- a/polygerrit-ui/app/api/plugin.ts
+++ b/polygerrit-ui/app/api/plugin.ts
@@ -14,6 +14,7 @@
import {ChangeActionsPluginApi} from './change-actions';
import {RestPluginApi} from './rest';
import {HookApi, RegisterOptions} from './hook';
+import {StylePluginApi} from './styles';
export enum TargetElement {
CHANGE_ACTIONS = 'changeactions',
@@ -77,10 +78,11 @@
moduleName?: string,
options?: RegisterOptions
): HookApi<T>;
- // DEPRECATED: Just add <style> elements to `document.head`.
+ // DEPRECATED: Use styleApi() instead.
registerStyleModule(endpoint: string, moduleName: string): void;
reporting(): ReportingPluginApi;
restApi(): RestPluginApi;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
screen(screenName: string, moduleName?: string): any;
+ styleApi(): StylePluginApi;
}
diff --git a/polygerrit-ui/app/api/rest.ts b/polygerrit-ui/app/api/rest.ts
index 283e029..a09f711 100644
--- a/polygerrit-ui/app/api/rest.ts
+++ b/polygerrit-ui/app/api/rest.ts
@@ -15,7 +15,10 @@
PUT = 'PUT',
}
-export type ErrorCallback = (response?: Response | null, err?: Error) => void;
+export type ErrorCallback = (
+ response?: Response | null,
+ err?: Error
+) => Promise<void> | void;
export declare interface RestPluginApi {
getLoggedIn(): Promise<boolean>;
diff --git a/polygerrit-ui/app/api/styles.ts b/polygerrit-ui/app/api/styles.ts
index f5b22a1..6ca8496 100644
--- a/polygerrit-ui/app/api/styles.ts
+++ b/polygerrit-ui/app/api/styles.ts
@@ -22,6 +22,7 @@
toString(): string;
}
+/** Accessible via `window.Gerrit.styles`. */
export declare interface Styles {
font: Style;
form: Style;
@@ -32,3 +33,22 @@
table: Style;
modal: Style;
}
+
+/** Accessible via `window.Gerrit.install(plugin => {plugin.styleApi()})`. */
+export declare interface StylePluginApi {
+ /**
+ * Convenience method for adding a CSS rule to a <style> element in <head>.
+ *
+ * Note that you can only insert one rule per call. See `insertRule()`
+ * documentation of `CSSStyleSheet`:
+ * https://developer.mozilla.org/en-US/docs/Web/API/CSSStyleSheet/insertRule
+ *
+ * @param rule the css rule, e.g.:
+ * ```
+ * html.darkTheme {
+ * --header-background-color: blue;
+ * }
+ * ```
+ */
+ insertCSSRule(rule: string): void;
+}
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
index af4e87e..1be5fa3 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
@@ -129,6 +129,7 @@
constructor() {
super();
+ this.addEventListener('reload', () => window.location.reload());
subscribe(
this,
() => this.getAdminViewModel().state$,
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
index cee0fa4..3254b5c 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
@@ -29,6 +29,7 @@
import {configModelToken} from '../../../models/config/config-model';
import {resolve} from '../../../models/dependency';
import {createChangeUrl} from '../../../models/views/change';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
const SUGGESTIONS_LIMIT = 15;
const REF_PREFIX = 'refs/heads/';
@@ -241,7 +242,13 @@
input = input.substring(REF_PREFIX.length);
}
return this.restApiService
- .getRepoBranches(input, this.repoName, SUGGESTIONS_LIMIT)
+ .getRepoBranches(
+ input,
+ this.repoName,
+ SUGGESTIONS_LIMIT,
+ /* offset=*/ undefined,
+ throwingErrorCallback
+ )
.then(response => {
if (!response) return [];
const branches: Array<{name: BranchName}> = [];
diff --git a/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.ts
index 889a859..558d571 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.ts
@@ -6,8 +6,6 @@
import '@polymer/iron-input/iron-input';
import '../../shared/gr-button/gr-button';
import '../../shared/gr-select/gr-select';
-import {encodeURL, getBaseUrl} from '../../../utils/url-util';
-import {page} from '../../../utils/page-wrapper-utils';
import {BranchName, RepoName} from '../../../types/common';
import {getAppContext} from '../../../services/app-context';
import {formStyles} from '../../../styles/gr-form-styles';
@@ -15,7 +13,7 @@
import {LitElement, PropertyValues, css, html} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
import {BindValueChangeEvent} from '../../../types/events';
-import {fireEvent} from '../../../utils/event-util';
+import {fireAlert, fireEvent, fireReload} from '../../../utils/event-util';
import {RepoDetailView} from '../../../models/views/repo';
declare global {
@@ -126,13 +124,13 @@
throw new Error('itemName name is not set');
}
const USE_HEAD = this.itemRevision ? this.itemRevision : 'HEAD';
- const url = `${getBaseUrl()}/admin/repos/${encodeURL(this.repoName, true)}`;
if (this.itemDetail === RepoDetailView.BRANCHES) {
return this.restApiService
.createRepoBranch(this.repoName, this.itemName, {revision: USE_HEAD})
.then(itemRegistered => {
if (itemRegistered.status === 201) {
- page.show(`${url},branches`);
+ fireAlert(this, 'Branch created successfully. Reloading...');
+ fireReload(this);
}
});
} else if (this.itemDetail === RepoDetailView.TAGS) {
@@ -143,7 +141,8 @@
})
.then(itemRegistered => {
if (itemRegistered.status === 201) {
- page.show(`${url},tags`);
+ fireAlert(this, 'Tag created successfully. Reloading...');
+ fireReload(this);
}
});
}
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
index c88b1c5..a90abbd 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
@@ -23,6 +23,7 @@
import {LitElement, css, html} from 'lit';
import {customElement, query, property, state} from 'lit/decorators.js';
import {fireEvent} from '../../../utils/event-util';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
declare global {
interface HTMLElementTagNameMap {
@@ -204,7 +205,11 @@
}
private async getRepoSuggestions(input: string) {
- const response = await this.restApiService.getSuggestedRepos(input);
+ const response = await this.restApiService.getSuggestedRepos(
+ input,
+ /* n=*/ undefined,
+ throwingErrorCallback
+ );
const repos = [];
for (const [name, repo] of Object.entries(response ?? {})) {
@@ -214,7 +219,12 @@
}
private async getGroupSuggestions(input: string) {
- const response = await this.restApiService.getSuggestedGroups(input);
+ const response = await this.restApiService.getSuggestedGroups(
+ input,
+ /* project=*/ undefined,
+ /* n=*/ undefined,
+ throwingErrorCallback
+ );
const groups = [];
for (const [name, group] of Object.entries(response ?? {})) {
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
index 8824009..af9977b 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
@@ -42,6 +42,7 @@
import {configModelToken} from '../../../models/config/config-model';
import {resolve} from '../../../models/dependency';
import {modalStyles} from '../../../styles/gr-modal-styles';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
const SAVING_ERROR_TEXT =
'Group may not exist, or you may not have ' + 'permission to add it';
@@ -489,7 +490,7 @@
if (errResponse) {
if (errResponse.status === 404) {
fireAlert(this, SAVING_ERROR_TEXT);
- return errResponse;
+ return;
}
throw Error(errResponse.statusText);
}
@@ -529,13 +530,20 @@
/* private but used in test */
getGroupSuggestions(input: string) {
- return this.restApiService.getSuggestedGroups(input).then(response => {
- const groups: AutocompleteSuggestion[] = [];
- for (const [name, group] of Object.entries(response ?? {})) {
- groups.push({name, value: decodeURIComponent(group.id)});
- }
- return groups;
- });
+ return this.restApiService
+ .getSuggestedGroups(
+ input,
+ /* project=*/ undefined,
+ /* n=*/ undefined,
+ throwingErrorCallback
+ )
+ .then(response => {
+ const groups: AutocompleteSuggestion[] = [];
+ for (const [name, group] of Object.entries(response ?? {})) {
+ groups.push({name, value: decodeURIComponent(group.id)});
+ }
+ return groups;
+ });
}
private handleGroupMemberTextChanged(e: CustomEvent) {
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
index e65b16b..ba2b3fd 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
@@ -24,6 +24,7 @@
import {subpageStyles} from '../../../styles/gr-subpage-styles';
import {LitElement, PropertyValues, css, html} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
const INTERNAL_GROUP_REGEX = /^[\da-f]{40}$/;
@@ -427,13 +428,20 @@
}
private getGroupSuggestions(input: string) {
- return this.restApiService.getSuggestedGroups(input).then(response => {
- const groups: AutocompleteSuggestion[] = [];
- for (const [name, group] of Object.entries(response ?? {})) {
- groups.push({name, value: decodeURIComponent(group.id)});
- }
- return groups;
- });
+ return this.restApiService
+ .getSuggestedGroups(
+ input,
+ /* project=*/ undefined,
+ /* n=*/ undefined,
+ throwingErrorCallback
+ )
+ .then(response => {
+ const groups: AutocompleteSuggestion[] = [];
+ for (const [name, group] of Object.entries(response ?? {})) {
+ groups.push({name, value: decodeURIComponent(group.id)});
+ }
+ return groups;
+ });
}
// private but used in test
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts
index 49fa99f..07b7c87 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts
@@ -41,6 +41,7 @@
import {menuPageStyles} from '../../../styles/gr-menu-page-styles';
import {when} from 'lit/directives/when.js';
import {ValueChangedEvent} from '../../../types/events';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
const MAX_AUTOCOMPLETE_RESULTS = 20;
@@ -471,7 +472,8 @@
.getSuggestedGroups(
this.groupFilter || '',
this.repo,
- MAX_AUTOCOMPLETE_RESULTS
+ MAX_AUTOCOMPLETE_RESULTS,
+ throwingErrorCallback
)
.then(response => {
const groups: GroupSuggestion[] = [];
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
index 27deb82..389e7c4 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
@@ -43,6 +43,7 @@
import {ifDefined} from 'lit/directives/if-defined.js';
import {resolve} from '../../../models/dependency';
import {createChangeUrl} from '../../../models/views/change';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
const NOTHING_TO_SAVE = 'No changes to save.';
@@ -397,7 +398,12 @@
private getInheritFromSuggestions(): Promise<AutocompleteSuggestion[]> {
return this.restApiService
- .getRepos(this.inheritFromFilter, MAX_AUTOCOMPLETE_RESULTS)
+ .getRepos(
+ this.inheritFromFilter,
+ MAX_AUTOCOMPLETE_RESULTS,
+ /* offset=*/ undefined,
+ throwingErrorCallback
+ )
.then(response => {
const repos: AutocompleteSuggestion[] = [];
if (!response) {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow.ts
index ad1f718..f8fcad8 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow.ts
@@ -27,6 +27,7 @@
import {fireAlert} from '../../../utils/event-util';
import {pluralize} from '../../../utils/string-util';
import {Interaction} from '../../../constants/reporting';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
@customElement('gr-change-list-hashtag-flow')
export class GrChangeListHashtagFlow extends LitElement {
@@ -298,7 +299,8 @@
query: string
): Promise<AutocompleteSuggestion[]> {
const suggestions = await this.restApiService.getChangesWithSimilarHashtag(
- query
+ query,
+ throwingErrorCallback
);
this.existingHashtagSuggestions = (suggestions ?? [])
.flatMap(change => change.hashtags ?? [])
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow.ts
index 363b1ae..7752476 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow.ts
@@ -28,6 +28,7 @@
import {fireAlert} from '../../../utils/event-util';
import {pluralize} from '../../../utils/string-util';
import {Interaction} from '../../../constants/reporting';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
@customElement('gr-change-list-topic-flow')
export class GrChangeListTopicFlow extends LitElement {
@@ -343,7 +344,8 @@
query: string
): Promise<AutocompleteSuggestion[]> {
const suggestions = await this.restApiService.getChangesWithSimilarTopic(
- query
+ query,
+ throwingErrorCallback
);
this.existingTopicSuggestions = (suggestions ?? [])
.map(change => change.topic)
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 adc3bd7..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
@@ -1351,7 +1351,6 @@
showRevertDialog() {
const change = this.change;
if (!change) return;
- // The search is still broken if there is a " in the topic.
const query = `submissionid: "${change.submission_id}"`;
/* A chromium plugin expects that the modifyRevertMsg hook will only
be called after the revert button is pressed, hence we populate the
@@ -1365,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-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index f32bb8d..a06f8c9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -81,6 +81,7 @@
import {createSearchUrl} from '../../../models/views/search';
import {createChangeUrl} from '../../../models/views/change';
import {GeneratedWebLink, getChangeWeblinks} from '../../../utils/weblink-util';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
@@ -1141,7 +1142,7 @@
input: string
): Promise<AutocompleteSuggestion[]> {
return this.restApiService
- .getChangesWithSimilarTopic(input)
+ .getChangesWithSimilarTopic(input, throwingErrorCallback)
.then(response =>
(response ?? [])
.map(change => change.topic)
@@ -1157,7 +1158,7 @@
input: string
): Promise<AutocompleteSuggestion[]> {
return this.restApiService
- .getChangesWithSimilarHashtag(input)
+ .getChangesWithSimilarHashtag(input, throwingErrorCallback)
.then(response =>
(response ?? [])
.flatMap(change => change.hashtags ?? [])
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index fe2bb4d..91b82dc 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -2112,7 +2112,6 @@
this.viewState.basePatchNum = PARENT;
const patchChanged = this.hasPatchRangeChanged(this.viewState);
- let patchNumChanged = this.hasPatchNumChanged(this.viewState);
this.patchRange = {
patchNum: this.viewState.patchNum,
@@ -2136,14 +2135,13 @@
...this.patchRange,
patchNum: computeLatestPatchNum(this.allPatchSets),
};
- patchNumChanged = true;
}
if (patchChanged) {
// We need to collapse all diffs when viewState changes so that a non
// existing diff is not requested. See Issue 125270 for more details.
this.fileList?.resetFileState();
this.fileList?.collapseAllDiffs();
- this.reloadPatchNumDependentResources(patchNumChanged).then(() => {
+ this.reloadPatchNumDependentResources().then(() => {
this.sendShowChangeEvent();
});
}
@@ -2978,25 +2976,8 @@
* Kicks off requests for resources that rely on the patch range
* (`this.patchRange`) being defined.
*/
- reloadPatchNumDependentResources(patchNumChanged?: boolean) {
- assertIsDefined(this.changeNum, 'changeNum');
- if (!this.patchRange?.patchNum) throw new Error('missing patchNum');
- const promises = [this.loadAndSetCommitInfo()];
- if (patchNumChanged) {
- promises.push(
- this.getCommentsModel().reloadPortedComments(
- this.changeNum,
- this.patchRange?.patchNum
- )
- );
- promises.push(
- this.getCommentsModel().reloadPortedDrafts(
- this.changeNum,
- this.patchRange?.patchNum
- )
- );
- }
- return Promise.all(promises);
+ reloadPatchNumDependentResources() {
+ return this.loadAndSetCommitInfo();
}
// Private but used in tests
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 4fafcd9..9d89192 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -103,10 +103,6 @@
import {rootUrl} from '../../../utils/url-util';
import {testResolver} from '../../../test/common-test-setup';
import {UserModel, userModelToken} from '../../../models/user/user-model';
-import {
- CommentsModel,
- commentsModelToken,
-} from '../../../models/comments/comments-model';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
suite('gr-change-view tests', () => {
@@ -114,7 +110,6 @@
let setUrlStub: sinon.SinonStub;
let userModel: UserModel;
let changeModel: ChangeModel;
- let commentsModel: CommentsModel;
const ROBOT_COMMENTS_LIMIT = 10;
@@ -382,7 +377,6 @@
sinon.stub(element.actions, 'reload').returns(Promise.resolve());
});
userModel = testResolver(userModelToken);
- commentsModel = testResolver(commentsModelToken);
changeModel = testResolver(changeModelToken);
});
@@ -1490,7 +1484,7 @@
.callsFake(() => Promise.resolve());
const reloadPatchDependentStub = sinon
.stub(element, 'reloadPatchNumDependentResources')
- .callsFake(() => Promise.resolve([undefined, undefined, undefined]));
+ .callsFake(() => Promise.resolve());
assertIsDefined(element.fileList);
await element.fileList.updateComplete;
const collapseStub = sinon.stub(element.fileList, 'collapseAllDiffs');
@@ -1525,46 +1519,6 @@
assert.isTrue(collapseStub.calledTwice);
});
- test('reload ported comments when patchNum changes', async () => {
- assertIsDefined(element.fileList);
- sinon.stub(element, 'loadData').callsFake(() => Promise.resolve());
- sinon.stub(element, 'loadAndSetCommitInfo');
- await element.updateComplete;
- const reloadPortedCommentsStub = sinon.stub(
- commentsModel,
- 'reloadPortedComments'
- );
- const reloadPortedDraftsStub = sinon.stub(
- commentsModel,
- 'reloadPortedDrafts'
- );
- sinon.stub(element.fileList, 'collapseAllDiffs');
-
- const value: ChangeViewState = {
- ...createChangeViewState(),
- view: GerritView.CHANGE,
- patchNum: 1 as RevisionPatchSetNum,
- };
- element.viewState = value;
- await element.updateComplete;
-
- element.initialLoadComplete = true;
- element.change = {
- ...createChangeViewChange(),
- revisions: {
- rev1: createRevision(1),
- rev2: createRevision(2),
- },
- };
-
- value.basePatchNum = 1 as BasePatchSetNum;
- value.patchNum = 2 as RevisionPatchSetNum;
- element.viewState = {...value};
- await element.updateComplete;
- assert.isTrue(reloadPortedCommentsStub.calledOnce);
- assert.isTrue(reloadPortedDraftsStub.calledOnce);
- });
-
test('do not reload entire page when patchRange doesnt change', async () => {
assertIsDefined(element.fileList);
const reloadStub = sinon
@@ -2382,7 +2336,7 @@
sinon.stub(element, 'getLatestCommitMessage').returns(Promise.resolve());
sinon
.stub(element, 'reloadPatchNumDependentResources')
- .returns(Promise.resolve([undefined, undefined, undefined]));
+ .returns(Promise.resolve());
});
test("don't report changeDisplayed on reply", async () => {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
index ffb180d..5d7e55c 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
@@ -37,6 +37,7 @@
import {BindValueChangeEvent} from '../../../types/events';
import {resolve} from '../../../models/dependency';
import {createSearchUrl} from '../../../models/views/search';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
const SUGGESTIONS_LIMIT = 15;
const CHANGE_SUBJECT_LIMIT = 50;
@@ -631,7 +632,13 @@
input = input.substring('refs/heads/'.length);
}
return this.restApiService
- .getRepoBranches(input, this.project, SUGGESTIONS_LIMIT)
+ .getRepoBranches(
+ input,
+ this.project,
+ SUGGESTIONS_LIMIT,
+ /* offset=*/ undefined,
+ throwingErrorCallback
+ )
.then(response => {
if (!response) return [];
const branches: Array<{name: BranchName}> = [];
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-move-dialog/gr-confirm-move-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-move-dialog/gr-confirm-move-dialog.ts
index 7adc2ca..8f5c8dc 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-move-dialog/gr-confirm-move-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-move-dialog/gr-confirm-move-dialog.ts
@@ -14,6 +14,7 @@
import {Key, Modifier} from '../../../utils/dom-util';
import {ValueChangedEvent} from '../../../types/events';
import {ShortcutController} from '../../lit/shortcut-controller';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
const SUGGESTIONS_LIMIT = 15;
@@ -163,7 +164,13 @@
input = input.substring('refs/heads/'.length);
}
return this.restApiService
- .getRepoBranches(input, this.project, SUGGESTIONS_LIMIT)
+ .getRepoBranches(
+ input,
+ this.project,
+ SUGGESTIONS_LIMIT,
+ /* offest=*/ undefined,
+ throwingErrorCallback
+ )
.then(response => {
if (!response) return [];
const branches: Array<{name: BranchName}> = [];
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
index 14cd5e8..072ec73d 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
@@ -16,6 +16,7 @@
import {getAppContext} from '../../../services/app-context';
import {sharedStyles} from '../../../styles/shared-styles';
import {ValueChangedEvent} from '../../../types/events';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
export interface RebaseChange {
name: string;
@@ -222,7 +223,13 @@
// last time it was run.
fetchRecentChanges() {
return this.restApiService
- .getChanges(undefined, 'is:open -age:90d')
+ .getChanges(
+ undefined,
+ 'is:open -age:90d',
+ /* offset=*/ undefined,
+ /* options=*/ undefined,
+ throwingErrorCallback
+ )
.then(response => {
if (!response) return [];
const changes: RebaseChange[] = [];
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/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 00d5a8c..071c489 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -955,7 +955,10 @@
<div class="path" role="columnheader">File</div>
<div class="comments desktop" role="columnheader">Comments</div>
<div class="comments mobile" role="columnheader" title="Comments">C</div>
- <div class="sizeBars desktop" role="columnheader">Size</div>
+ ${when(
+ this.showSizeBars,
+ () => html`<div class="sizeBars desktop" role="columnheader">Size</div>`
+ )}
<div class="header-stats" role="columnheader">Delta</div>
<!-- endpoint: change-view-file-list-header -->
${when(showDynamicColumns, () => this.renderDynamicHeaderEndpoints())}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
index 66f1e57..1fbddc5 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
@@ -113,6 +113,7 @@
.stub(element, '_saveReviewedState')
.callsFake(() => Promise.resolve());
await element.updateComplete;
+ element.showSizeBars = true;
// Wait for expandedFilesChanged to complete.
await waitEventLoop();
});
@@ -205,7 +206,7 @@
</div>
</div>
<div class="desktop" role="gridcell">
- <div aria-hidden="true" class="hide sizeBars"></div>
+ <div aria-hidden="true" class="sizeBars"></div>
</div>
<div class="stats" role="gridcell">
<div>
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
index 64bfe91..2b7d1ff 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
@@ -528,6 +528,9 @@
this.reporting.reportErrorDialog(message);
this.errorDialog.text = message;
this.errorDialog.showSignInButton = !!options && !!options.showSignInButton;
+ if (this.errorModal.hasAttribute('open')) {
+ this.errorModal.close();
+ }
this.errorModal.showModal();
}
}
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts
index 25173b4..45ba33b 100644
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts
+++ b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts
@@ -61,7 +61,6 @@
display: block;
max-height: 100vh;
min-width: 60vw;
- overflow-y: auto;
}
main {
display: flex;
diff --git a/polygerrit-ui/app/elements/core/gr-notifications-prompt/gr-notifications-prompt.ts b/polygerrit-ui/app/elements/core/gr-notifications-prompt/gr-notifications-prompt.ts
index e01bb34..ab95711 100644
--- a/polygerrit-ui/app/elements/core/gr-notifications-prompt/gr-notifications-prompt.ts
+++ b/polygerrit-ui/app/elements/core/gr-notifications-prompt/gr-notifications-prompt.ts
@@ -51,7 +51,7 @@
#notificationsPrompt {
position: absolute;
right: 30px;
- top: 100px;
+ top: 50px;
z-index: 150; /* Less than gr-hovercard's, higher than rest */
display: flex;
background-color: var(--background-color-primary);
diff --git a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts
index 5bcef11..b9c920a 100644
--- a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts
+++ b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts
@@ -19,6 +19,7 @@
import {resolve} from '../../../models/dependency';
import {configModelToken} from '../../../models/config/config-model';
import {createSearchUrl} from '../../../models/views/search';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
const MAX_AUTOCOMPLETE_RESULTS = 10;
const SELF_EXPRESSION = 'self';
@@ -98,7 +99,11 @@
expression: string
): Promise<AutocompleteSuggestion[]> {
return this.restApiService
- .getSuggestedRepos(expression, MAX_AUTOCOMPLETE_RESULTS)
+ .getSuggestedRepos(
+ expression,
+ MAX_AUTOCOMPLETE_RESULTS,
+ throwingErrorCallback
+ )
.then(projects => {
if (!projects) {
return [];
@@ -128,7 +133,12 @@
return Promise.resolve([]);
}
return this.restApiService
- .getSuggestedGroups(expression, undefined, MAX_AUTOCOMPLETE_RESULTS)
+ .getSuggestedGroups(
+ expression,
+ undefined,
+ MAX_AUTOCOMPLETE_RESULTS,
+ throwingErrorCallback
+ )
.then(groups => {
if (!groups) {
return [];
@@ -158,7 +168,13 @@
return Promise.resolve([]);
}
return this.restApiService
- .getSuggestedAccounts(expression, MAX_AUTOCOMPLETE_RESULTS)
+ .getSuggestedAccounts(
+ expression,
+ MAX_AUTOCOMPLETE_RESULTS,
+ /* canSee=*/ undefined,
+ /* filterActive=*/ undefined,
+ throwingErrorCallback
+ )
.then(accounts => {
if (!accounts) {
return [];
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
index 6cfefe5..d1d05b7 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
@@ -33,6 +33,7 @@
import {resolve} from '../../../models/dependency';
import {modalStyles} from '../../../styles/gr-modal-styles';
import {whenVisible} from '../../../utils/dom-util';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
@customElement('gr-edit-controls')
export class GrEditControls extends LitElement {
@@ -514,7 +515,12 @@
assertIsDefined(this.change, 'this.change');
assertIsDefined(this.patchNum, 'this.patchNum');
return this.restApiService
- .queryChangeFiles(this.change._number, this.patchNum, input)
+ .queryChangeFiles(
+ this.change._number,
+ this.patchNum,
+ input,
+ throwingErrorCallback
+ )
.then(res => {
if (!res)
throw new Error('Failed to retrieve files. Response not set.');
diff --git a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts
index 15ec512..2996e50 100644
--- a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts
+++ b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts
@@ -21,6 +21,7 @@
import {when} from 'lit/directives/when.js';
import {fire} from '../../../utils/event-util';
import {PropertiesOfType} from '../../../utils/type-util';
+import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
type NotificationKey = PropertiesOfType<Required<ProjectWatchInfo>, boolean>;
@@ -193,13 +194,15 @@
// private but used in tests.
getProjectSuggestions(input: string) {
- return this.restApiService.getSuggestedRepos(input).then(response => {
- const repos: AutocompleteSuggestion[] = [];
- for (const [name, repo] of Object.entries(response ?? {})) {
- repos.push({name, value: repo.id});
- }
- return repos;
- });
+ return this.restApiService
+ .getSuggestedRepos(input, /* n=*/ undefined, throwingErrorCallback)
+ .then(response => {
+ const repos: AutocompleteSuggestion[] = [];
+ for (const [name, repo] of Object.entries(response ?? {})) {
+ repos.push({name, value: repo.id});
+ }
+ return repos;
+ });
}
private handleRemoveProject(project: ProjectWatchInfo) {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api.ts
new file mode 100644
index 0000000..13eefc8
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api.ts
@@ -0,0 +1,43 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {PluginApi} from '../../../api/plugin';
+import {StylePluginApi} from '../../../api/styles';
+import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
+
+function getOrCreatePluginStyleEl(): HTMLStyleElement {
+ const el =
+ document.head.querySelector<HTMLStyleElement>('style#plugin-style');
+ if (el) return el;
+
+ const styleEl = document.createElement('style');
+ styleEl.setAttribute('id', 'plugin-style');
+ // Append at the end so that they override the default light and dark theme
+ // styles.
+ document.head.appendChild(styleEl);
+ return styleEl;
+}
+
+export class GrPluginStyleApi implements StylePluginApi {
+ constructor(
+ private readonly reporting: ReportingService,
+ private readonly plugin: PluginApi
+ ) {
+ this.reporting.trackApi(this.plugin, 'style', 'constructor');
+ }
+
+ insertCSSRule(rule: string): void {
+ this.reporting.trackApi(this.plugin, 'style', 'insertCSSRule');
+
+ const styleEl = getOrCreatePluginStyleEl();
+ try {
+ styleEl.sheet?.insertRule(rule);
+ } catch (error) {
+ console.error(
+ `Failed to insert CSS rule for plugin ${this.plugin.getPluginName()}: ${error}`
+ );
+ }
+ }
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api_test.ts
new file mode 100644
index 0000000..469d667
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api_test.ts
@@ -0,0 +1,51 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+import './gr-js-api-interface';
+import {query, queryAndAssert} from '../../../utils/common-util';
+import {assert} from '@open-wc/testing';
+import {StylePluginApi} from '../../../api/styles';
+
+suite('gr-plugin-style-api tests', () => {
+ let styleApi: StylePluginApi;
+
+ setup(() => {
+ window.Gerrit.install(
+ p => (styleApi = p.styleApi()),
+ '0.1',
+ 'http://test.com/plugins/testplugin/static/test.js'
+ );
+ });
+
+ teardown(() => {
+ const styleEl = query<HTMLStyleElement>(
+ document.head,
+ 'style#plugin-style'
+ );
+ styleEl?.remove();
+ });
+
+ test('insertCSSRule adds a rule', async () => {
+ styleApi.insertCSSRule('html{color:green;}');
+ const styleEl = queryAndAssert<HTMLStyleElement>(
+ document.head,
+ 'style#plugin-style'
+ );
+ const styleSheet = styleEl.sheet;
+ assert.equal(styleSheet?.cssRules.length, 1);
+ });
+
+ test('insertCSSRule re-uses the <style> element', async () => {
+ styleApi.insertCSSRule('html{color:green;}');
+ styleApi.insertCSSRule('html{margin:0px;}');
+ const styleEl = queryAndAssert<HTMLStyleElement>(
+ document.head,
+ 'style#plugin-style'
+ );
+ const styleSheet = styleEl.sheet;
+ assert.equal(styleSheet?.cssRules.length, 2);
+ });
+});
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
index 2ed3794..7170051 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
@@ -35,6 +35,8 @@
import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
import {PluginsModel} from '../../../models/plugins/plugins-model';
+import {GrPluginStyleApi} from './gr-plugin-style-api';
+import {StylePluginApi} from '../../../api/styles';
/**
* Plugin-provided custom components can affect content in extension
@@ -244,6 +246,10 @@
return new GrReportingJsApi(this.report, this);
}
+ styleApi(): StylePluginApi {
+ return new GrPluginStyleApi(this.report, this);
+ }
+
admin(): AdminPluginApi {
return new GrAdminApi(this.report, this);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-repo-branch-picker/gr-repo-branch-picker.ts b/polygerrit-ui/app/elements/shared/gr-repo-branch-picker/gr-repo-branch-picker.ts
index 8fa351b..6d2fe20 100644
--- a/polygerrit-ui/app/elements/shared/gr-repo-branch-picker/gr-repo-branch-picker.ts
+++ b/polygerrit-ui/app/elements/shared/gr-repo-branch-picker/gr-repo-branch-picker.ts
@@ -21,6 +21,7 @@
import {assertIsDefined} from '../../../utils/common-util';
import {fire} from '../../../utils/event-util';
import {BindValueChangeEvent} from '../../../types/events';
+import {throwingErrorCallback} from '../gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
const SUGGESTIONS_LIMIT = 15;
const REF_PREFIX = 'refs/heads/';
@@ -121,7 +122,13 @@
input = input.substring(REF_PREFIX.length);
}
return this.restApiService
- .getRepoBranches(input, this.repo, SUGGESTIONS_LIMIT)
+ .getRepoBranches(
+ input,
+ this.repo,
+ SUGGESTIONS_LIMIT,
+ /* offset=*/ undefined,
+ throwingErrorCallback
+ )
.then(res => this.branchResponseToSuggestions(res));
}
@@ -143,7 +150,12 @@
// private but used in test
getRepoSuggestions(input: string) {
return this.restApiService
- .getRepos(input, SUGGESTIONS_LIMIT)
+ .getRepos(
+ input,
+ SUGGESTIONS_LIMIT,
+ /* offset=*/ undefined,
+ throwingErrorCallback
+ )
.then(res => this.repoResponseToSuggestions(res));
}
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
index 1fbe5b2..8b3e2a6 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
@@ -183,6 +183,35 @@
[name: string]: string[] | string | number | boolean | undefined | null;
};
+/**
+ * Error callback that throws an error.
+ *
+ * Pass into REST API methods as errFn to make the returned Promises reject on
+ * error.
+ *
+ * If error is provided, it's thrown.
+ * Otherwise if response with error is provided the promise that will throw an
+ * error is returned.
+ */
+export function throwingErrorCallback(
+ response?: Response | null,
+ err?: Error
+): void | Promise<void> {
+ if (err) throw err;
+ if (!response) return;
+
+ return response.text().then(errorText => {
+ let message = `Error ${response.status}`;
+ if (response.statusText) {
+ message += ` (${response.statusText})`;
+ }
+ if (errorText) {
+ message += `: ${errorText}`;
+ }
+ throw new Error(message);
+ });
+}
+
interface SendRequestBase {
method: HttpMethod | undefined;
body?: RequestPayload;
@@ -361,27 +390,26 @@
*
* @param noAcceptHeader - don't add default accept json header
*/
- fetchJSON(
+ async fetchJSON(
req: FetchJSONRequest,
noAcceptHeader?: boolean
): Promise<ParsedJSON | undefined> {
if (!noAcceptHeader) {
req = this.addAcceptJsonHeader(req);
}
- return this.fetchRawJSON(req).then(response => {
- if (!response) {
+ const response = await this.fetchRawJSON(req);
+ if (!response) {
+ return;
+ }
+ if (!response.ok) {
+ if (req.errFn) {
+ await req.errFn.call(undefined, response);
return;
}
- if (!response.ok) {
- if (req.errFn) {
- req.errFn.call(null, response);
- return;
- }
- fireServerError(response, req);
- return;
- }
- return this.getResponseObject(response);
- });
+ fireServerError(response, req);
+ return;
+ }
+ return this.getResponseObject(response);
}
urlWithParams(url: string, fetchParams?: FetchParams): string {
@@ -474,7 +502,7 @@
* (i.e. no exception and response.ok is true). If response fails then
* promise resolves either to void if errFn is set or rejects if errFn
* is not set */
- send(req: SendRequest): Promise<Response | ParsedJSON | undefined> {
+ async send(req: SendRequest): Promise<Response | ParsedJSON | undefined> {
const options: AuthRequestInit = {method: req.method};
if (req.body) {
options.headers = new Headers();
@@ -499,38 +527,30 @@
fetchOptions: options,
anonymizedUrl: req.reportUrlAsIs ? url : req.anonymizedUrl,
};
- const xhr = this.fetch(fetchReq)
- .catch(err => {
- fireNetworkError(err);
- if (req.errFn) {
- req.errFn.call(undefined, null, err);
- return;
- } else {
- throw err;
- }
- })
- .then(response => {
- if (response && !response.ok) {
- if (req.errFn) {
- req.errFn.call(undefined, response);
- return;
- }
- fireServerError(response, fetchReq);
- }
- return response;
- });
+ let xhr;
+ try {
+ xhr = await this.fetch(fetchReq);
+ } catch (err) {
+ fireNetworkError(err as Error);
+ if (req.errFn) {
+ await req.errFn.call(undefined, null, err as Error);
+ xhr = undefined;
+ } else {
+ throw err;
+ }
+ }
+ if (xhr && !xhr.ok) {
+ if (req.errFn) {
+ await req.errFn.call(undefined, xhr);
+ } else {
+ fireServerError(xhr, fetchReq);
+ }
+ }
if (req.parseResponse) {
- // TODO(TS): remove as Response and fix error.
- // Javascript code allows returning of a Response object from errFn.
- // This can be a mistake and we should add check here or it can be used
- // somewhere - in this case we should fix it carefully (define
- // different type of callback if parseResponse is true, etc...).
- return xhr.then(res => this.getResponseObject(res as Response));
+ xhr = xhr && this.getResponseObject(xhr);
}
- // The actual xhr type is Promise<Response|undefined|void> because of the
- // catch callback
- return xhr as Promise<Response | undefined>;
+ return xhr;
}
invalidateFetchPromisesPrefix(prefix: string) {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper_test.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper_test.ts
index 1234b59..9f0319e 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper_test.ts
@@ -9,7 +9,7 @@
FetchPromisesCache,
GrRestApiHelper,
} from './gr-rest-api-helper';
-import {waitEventLoop} from '../../../../test/test-utils';
+import {assertFails, waitEventLoop} from '../../../../test/test-utils';
import {FakeScheduler} from '../../../../services/scheduler/fake-scheduler';
import {RetryScheduler} from '../../../../services/scheduler/retry-scheduler';
import {ParsedJSON} from '../../../../types/common';
@@ -229,6 +229,79 @@
assert.isTrue(cancelCalled);
});
+ suite('throwing in errFn', () => {
+ function throwInPromise(response?: Response | null, _?: Error) {
+ return response?.text().then(text => {
+ throw new Error(text);
+ });
+ }
+
+ function throwImmediately(_1?: Response | null, _2?: Error) {
+ throw new Error('Error Callback error');
+ }
+
+ setup(() => {
+ authFetchStub.returns(
+ Promise.resolve({
+ ...new Response(),
+ status: 400,
+ ok: false,
+ text() {
+ return Promise.resolve('Nope');
+ },
+ })
+ );
+ });
+
+ test('errFn with Promise throw cause send to reject on error', async () => {
+ const promise = helper.send({
+ method: HttpMethod.GET,
+ url: '/dummy/url',
+ parseResponse: false,
+ errFn: throwInPromise,
+ });
+ await assertReadRequest();
+
+ const err = await assertFails(promise);
+ assert.equal((err as Error).message, 'Nope');
+ });
+
+ test('errFn with Promise throw cause fetchJSON to reject on error', async () => {
+ const promise = helper.fetchJSON({
+ url: '/dummy/url',
+ errFn: throwInPromise,
+ });
+ await assertReadRequest();
+
+ const err = await assertFails(promise);
+ assert.equal((err as Error).message, 'Nope');
+ });
+
+ test('errFn with immediate throw cause send to reject on error', async () => {
+ const promise = helper.send({
+ method: HttpMethod.GET,
+ url: '/dummy/url',
+ parseResponse: false,
+ errFn: throwImmediately,
+ });
+ await assertReadRequest();
+
+ const err = await assertFails(promise);
+ assert.equal((err as Error).message, 'Error Callback error');
+ });
+
+ test('errFn with immediate Promise cause fetchJSON to reject on error', async () => {
+ const promise = helper.fetchJSON({
+ url: '/dummy/url',
+ errFn: throwImmediately,
+ });
+ await assertReadRequest();
+
+ const err = await assertFails(promise);
+ assert.equal((err as Error).message, 'Error Callback error');
+ });
+ });
+
suite('429 errors', () => {
setup(() => {
authFetchStub.returns(
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
index 169cd59..9f52517 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
@@ -234,9 +234,7 @@
hiddenText in order to correctly position the dropdown. After being moved,
it is set as the positionTarget for the emojiSuggestions dropdown. -->
<span id="caratSpan"></span>
- ${this.renderEmojiDropdown()}
- ${this.renderMentionsDropdown()}
- </gr-autocomplete-dropdown>
+ ${this.renderEmojiDropdown()} ${this.renderMentionsDropdown()}
<iron-autogrow-textarea
id="textarea"
class=${classMap({noBorder: this.hideBorder})}
@@ -359,12 +357,8 @@
}
private handleTabKey(e: KeyboardEvent) {
- // Tab should have normal behavior if the picker is closed or if the user
- // has only typed ':'.
- if (
- !this.isDropdownVisible() ||
- (this.isEmojiDropdownActive() && this.currentSearchString === '')
- ) {
+ // Tab should have normal behavior if the picker is closed.
+ if (!this.isDropdownVisible()) {
return;
}
e.preventDefault();
@@ -374,12 +368,9 @@
// private but used in test
handleEnterByKey(e: KeyboardEvent) {
- // Enter should have newline behavior if the picker is closed or if the user
- // has only typed ':'. Also make sure that shortcuts aren't clobbered.
- if (
- !this.isDropdownVisible() ||
- (this.isEmojiDropdownActive() && this.currentSearchString === '')
- ) {
+ // Enter should have newline behavior if the picker is closed. Also make
+ // sure that shortcuts aren't clobbered.
+ if (!this.isDropdownVisible()) {
this.indent(e);
return;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
index 634c387..1460246 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
@@ -617,19 +617,6 @@
await element.updateComplete;
assert.equal(element.text, '💯');
});
-
- test('enter key - ignored on just colon without more information', async () => {
- const enterSpy = sinon.spy(element.emojiSuggestions!, 'getCursorTarget');
- pressKey(element.textarea! as HTMLElement, Key.ENTER);
- assert.isFalse(enterSpy.called);
- element.textarea!.focus();
- element.textarea!.selectionStart = 1;
- element.textarea!.selectionEnd = 1;
- element.text = ':';
- await element.updateComplete;
- pressKey(element.textarea! as HTMLElement, Key.ENTER);
- assert.isFalse(enterSpy.called);
- });
});
suite('gr-textarea monospace', () => {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts
index 090d125..5270603 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts
@@ -347,7 +347,8 @@
createTextEl(
lineNumberEl: HTMLElement | null,
line: GrDiffLine,
- side?: Side
+ side?: Side,
+ twoSlots?: boolean
) {
const td = createElementDiff('td');
if (line.type !== GrDiffLineType.BLANK) {
@@ -403,9 +404,19 @@
const threadGroupEl = document.createElement('div');
threadGroupEl.className = 'thread-group';
threadGroupEl.setAttribute('data-side', side);
+
const slot = document.createElement('slot');
slot.name = `${side}-${lineNumber}`;
threadGroupEl.appendChild(slot);
+
+ // For line.type === BOTH in unified diff we want two slots.
+ if (twoSlots) {
+ const slot = document.createElement('slot');
+ const otherSide = side === Side.LEFT ? Side.RIGHT : Side.LEFT;
+ slot.name = `${otherSide}-${line.lineNumber(otherSide)}`;
+ threadGroupEl.appendChild(slot);
+ }
+
td.appendChild(threadGroupEl);
}
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-builder-unified.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified.ts
index f760232..ffc2dcd 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified.ts
@@ -142,7 +142,8 @@
.join(' ')
.trim()
);
- row.appendChild(this.createTextEl(lineNumberEl, line, side));
+ const twoSlots = line.type === GrDiffLineType.BOTH;
+ row.appendChild(this.createTextEl(lineNumberEl, line, side, twoSlots));
return row;
}
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 e496cca..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)
@@ -348,7 +351,7 @@
if (lineNumber)
fire(this, 'line-mouse-leave', {lineNum: lineNumber, side});
}}
- >${this.renderText(side)}${this.renderThreadGroup(side, lineNumber)}</td>
+ >${this.renderText(side)}${this.renderThreadGroup(side)}</td>
`;
}
@@ -369,17 +372,33 @@
return html`<td class=${diffClasses(...extras)}>${sign}</td>`;
}
- private renderThreadGroup(side: Side, lineNumber?: LineNumber) {
- if (!lineNumber) return;
- // .content has `white-space: pre`, so prettier must not add spaces.
- // prettier-ignore
- return html`<div class="thread-group" data-side=${side}><slot name="${side}-${lineNumber}"></slot></div>`;
+ private renderThreadGroup(side: Side) {
+ const lineNumber = this.lineNumber(side);
+ if (!lineNumber) return nothing;
+ return html`<div class="thread-group" data-side=${side}>
+ <slot name="${side}-${lineNumber}"></slot>
+ ${this.renderSecondSlot()}
+ </div>`;
+ }
+
+ private renderSecondSlot() {
+ if (!this.unifiedDiff) return nothing;
+ if (this.line(Side.LEFT)?.type !== GrDiffLineType.BOTH) return nothing;
+ return html`<slot
+ name="${Side.LEFT}-${this.lineNumber(Side.LEFT)}"
+ ></slot>`;
}
private contentRef(side: Side) {
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-builder/gr-diff-row_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
index 27fc920..1c7b311 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
@@ -93,6 +93,66 @@
);
});
+ test('both unified', async () => {
+ const line = new GrDiffLine(GrDiffLineType.BOTH, 1, 1);
+ line.text = 'lorem ipsum';
+ element.left = line;
+ element.right = line;
+ element.unifiedDiff = true;
+ await element.updateComplete;
+ assert.lightDom.equal(
+ element,
+ /* HTML */ `
+ <table>
+ <tbody>
+ <tr
+ aria-labelledby="left-button-1 right-button-1 right-content-1"
+ class="both diff-row gr-diff unified"
+ tabindex="-1"
+ >
+ <td class="blame gr-diff" data-line-number="1"></td>
+ <td class="gr-diff left lineNum" data-value="1">
+ <button
+ aria-label="1 unmodified"
+ class="gr-diff left lineNumButton"
+ data-value="1"
+ id="left-button-1"
+ tabindex="-1"
+ >
+ 1
+ </button>
+ </td>
+ <td class="gr-diff lineNum right" data-value="1">
+ <button
+ aria-label="1 unmodified"
+ class="gr-diff lineNumButton right"
+ data-value="1"
+ id="right-button-1"
+ tabindex="-1"
+ >
+ 1
+ </button>
+ </td>
+ <td class="both content gr-diff no-intraline-info right">
+ <div
+ class="contentText gr-diff"
+ data-side="right"
+ id="right-content-1"
+ >
+ <gr-diff-text> lorem ipsum </gr-diff-text>
+ </div>
+ <div class="thread-group" data-side="right">
+ <slot name="right-1"> </slot>
+ <slot name="left-1"> </slot>
+ </div>
+ </td>
+ </tr>
+ </tbody>
+ </table>
+ `
+ );
+ });
+
test('add', async () => {
const line = new GrDiffLine(GrDiffLineType.ADD, 0, 1);
line.text = 'lorem ipsum';
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/token-highlight-layer.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/token-highlight-layer.ts
index 1e5dd65..e9076aa 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/token-highlight-layer.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/token-highlight-layer.ts
@@ -139,11 +139,17 @@
let atLeastOneTokenMatched = false;
while ((match = tokenMatcher.exec(text))) {
const token = match[0];
- const index = match.index;
- const length = token.length;
+
// Binary files encoded as text for example can have super long lines
// with super long tokens. Let's guard against this scenario.
- if (length > TOKEN_LENGTH_LIMIT) continue;
+ if (token.length > TOKEN_LENGTH_LIMIT) continue;
+
+ // This is to correctly count surrogate pairs in text and token.
+ // If the index calculation becomes a hotspot, we could precompute a code
+ // unit to code point index map for text before iterating over the results
+ const index = GrAnnotation.getStringLength(text.slice(0, match.index));
+ const length = GrAnnotation.getStringLength(token);
+
atLeastOneTokenMatched = true;
const highlightTypeClass =
token === this.currentHighlight ? CSS_HIGHLIGHT : '';
@@ -339,7 +345,7 @@
start_line: line,
start_column: index + 1, // 1-based inclusive
end_line: line,
- end_column: index + token.length, // 1-based inclusive
+ end_column: index + GrAnnotation.getStringLength(token), // 1-based inclusive
};
this.tokenHighlightListener({token, element, side, range});
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/token-highlight-layer_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/token-highlight-layer_test.ts
index 1beed46..8fd03bb 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/token-highlight-layer_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/token-highlight-layer_test.ts
@@ -105,15 +105,17 @@
suite('annotate', () => {
function assertAnnotation(
args: any[],
- el: HTMLElement,
- start: number,
- length: number,
- cssClass: string
+ expected: {
+ parent: HTMLElement;
+ offset: number;
+ length: number;
+ cssClass: string;
+ }
) {
- assert.equal(args[0], el);
- assert.equal(args[1], start);
- assert.equal(args[2], length);
- assert.equal(args[3], cssClass);
+ assert.equal(args[0], expected.parent);
+ assert.equal(args[1], expected.offset);
+ assert.equal(args[2], expected.length);
+ assert.equal(args[3], expected.cssClass);
}
test('annotate adds css token', () => {
@@ -121,27 +123,51 @@
const el = createLine('these are words');
annotate(el);
assert.isTrue(annotateElementStub.calledThrice);
- assertAnnotation(
- annotateElementStub.args[0],
- el,
- 0,
- 5,
- 'tk-text-these tk-index-0 token '
- );
- assertAnnotation(
- annotateElementStub.args[1],
- el,
- 6,
- 3,
- 'tk-text-are tk-index-6 token '
- );
- assertAnnotation(
- annotateElementStub.args[2],
- el,
- 10,
- 5,
- 'tk-text-words tk-index-10 token '
- );
+ assertAnnotation(annotateElementStub.args[0], {
+ parent: el,
+ offset: 0,
+ length: 5,
+ cssClass: 'tk-text-these tk-index-0 token ',
+ });
+ assertAnnotation(annotateElementStub.args[1], {
+ parent: el,
+ offset: 6,
+ length: 3,
+ cssClass: 'tk-text-are tk-index-6 token ',
+ });
+ assertAnnotation(annotateElementStub.args[2], {
+ parent: el,
+ offset: 10,
+ length: 5,
+ cssClass: 'tk-text-words tk-index-10 token ',
+ });
+ });
+
+ test('annotate adds css tokens w/ emojis', () => {
+ const annotateElementStub = sinon.stub(GrAnnotation, 'annotateElement');
+ const el = createLine('these 💩 are 👨👩👧👦 words');
+
+ annotate(el);
+
+ assert.isTrue(annotateElementStub.calledThrice);
+ assertAnnotation(annotateElementStub.args[0], {
+ parent: el,
+ offset: 0,
+ length: 5,
+ cssClass: 'tk-text-these tk-index-0 token ',
+ });
+ assertAnnotation(annotateElementStub.args[1], {
+ parent: el,
+ offset: 8,
+ length: 3,
+ cssClass: 'tk-text-are tk-index-8 token ',
+ });
+ assertAnnotation(annotateElementStub.args[2], {
+ parent: el,
+ offset: 20,
+ length: 5,
+ cssClass: 'tk-text-words tk-index-20 token ',
+ });
});
test('annotate adds mouse handlers', () => {
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-highlight/gr-annotation.ts b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts
index 92175eb..38bd707 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts
@@ -171,18 +171,20 @@
cssClass: string,
firstPart?: boolean
) {
- if (this.getLength(node) === offset || offset === 0) {
- return this.wrapInHighlight(node, cssClass);
- } else {
- if (firstPart) {
- this.splitNode(node, offset);
- // Node points to first part of the Text, second one is sibling.
- } else {
- // if node is Text then splitNode will return a Text
- node = this.splitNode(node, offset) as Text;
- }
+ if (
+ (this.getLength(node) === offset && firstPart) ||
+ (offset === 0 && !firstPart)
+ ) {
return this.wrapInHighlight(node, cssClass);
}
+ if (firstPart) {
+ this.splitNode(node, offset);
+ // Node points to first part of the Text, second one is sibling.
+ } else {
+ // if node is Text then splitNode will return a Text
+ node = this.splitNode(node, offset) as Text;
+ }
+ return this.wrapInHighlight(node, cssClass);
},
/**
@@ -225,7 +227,6 @@
*/
splitTextNode(node: Text, offset: number) {
if (node.textContent?.match(REGEX_ASTRAL_SYMBOL)) {
- // TODO (viktard): Polyfill Array.from for IE10.
const head = Array.from(node.textContent);
const tail = head.splice(offset);
const parent = node.parentNode;
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation_test.ts
index fdf1785..f319a3c 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation_test.ts
@@ -27,79 +27,77 @@
str = textNode.textContent!;
});
+ test('_annotateText length:0 offset:0', () => {
+ GrAnnotation._annotateText(textNode, 0, 0, 'foobar');
+
+ assert.equal(parent.textContent, str);
+ assert.equal(
+ parent.innerHTML,
+ '<hl class="foobar"></hl>Lorem ipsum dolor sit amet, suspendisse inceptos vehicula'
+ );
+ });
+
+ test('_annotateText length:0 offset:1', () => {
+ GrAnnotation._annotateText(textNode, 1, 0, 'foobar');
+
+ assert.equal(parent.textContent, str);
+ assert.equal(
+ parent.innerHTML,
+ 'L<hl class="foobar"></hl>orem ipsum dolor sit amet, suspendisse inceptos vehicula'
+ );
+ });
+
+ test('_annotateText length:0 offset:str.length', () => {
+ GrAnnotation._annotateText(textNode, str.length, 0, 'foobar');
+
+ assert.equal(parent.textContent, str);
+ assert.equal(
+ parent.innerHTML,
+ 'Lorem ipsum dolor sit amet, suspendisse inceptos vehicula<hl class="foobar"></hl>'
+ );
+ });
+
test('_annotateText Case 1', () => {
GrAnnotation._annotateText(textNode, 0, str.length, 'foobar');
- assert.equal(parent.childNodes.length, 1);
- assert.instanceOf(parent.childNodes[0], HTMLElement);
- const firstChild = parent.childNodes[0] as HTMLElement;
- assert.equal(firstChild.className, 'foobar');
- assert.instanceOf(firstChild.childNodes[0], Text);
- assert.equal(firstChild.childNodes[0].textContent, str);
+ assert.equal(parent.textContent, str);
+ assert.equal(
+ parent.innerHTML,
+ '<hl class="foobar">Lorem ipsum dolor sit amet, suspendisse inceptos vehicula</hl>'
+ );
});
test('_annotateText Case 2', () => {
- const length = 12;
- const substr = str.substr(0, length);
- const remainder = str.substr(length);
+ GrAnnotation._annotateText(textNode, 0, 12, 'foobar');
- GrAnnotation._annotateText(textNode, 0, length, 'foobar');
-
- assert.equal(parent.childNodes.length, 2);
-
- assert.instanceOf(parent.childNodes[0], HTMLElement);
- const firstChild = parent.childNodes[0] as HTMLElement;
- assert.equal(firstChild.className, 'foobar');
- assert.instanceOf(firstChild.childNodes[0], Text);
- assert.equal(firstChild.childNodes[0].textContent, substr);
-
- assert.instanceOf(parent.childNodes[1], Text);
- assert.equal(parent.childNodes[1].textContent, remainder);
+ assert.equal(parent.textContent, str);
+ assert.equal(
+ parent.innerHTML,
+ '<hl class="foobar">Lorem ipsum </hl>dolor sit amet, suspendisse inceptos vehicula'
+ );
});
test('_annotateText Case 3', () => {
- const index = 12;
- const length = str.length - index;
- const remainder = str.substr(0, index);
- const substr = str.substr(index);
+ GrAnnotation._annotateText(textNode, 12, str.length - 12, 'foobar');
- GrAnnotation._annotateText(textNode, index, length, 'foobar');
-
- assert.equal(parent.childNodes.length, 2);
-
- assert.instanceOf(parent.childNodes[0], Text);
- assert.equal(parent.childNodes[0].textContent, remainder);
-
- const secondChild = parent.childNodes[1] as HTMLElement;
- assert.instanceOf(secondChild, HTMLElement);
- assert.equal(secondChild.className, 'foobar');
- assert.instanceOf(secondChild.childNodes[0], Text);
- assert.equal(secondChild.childNodes[0].textContent, substr);
+ assert.equal(parent.textContent, str);
+ assert.equal(
+ parent.innerHTML,
+ 'Lorem ipsum <hl class="foobar">dolor sit amet, suspendisse inceptos vehicula</hl>'
+ );
});
test('_annotateText Case 4', () => {
const index = str.indexOf('dolor');
const length = 'dolor '.length;
- const remainderPre = str.substr(0, index);
- const substr = str.substr(index, length);
- const remainderPost = str.substr(index + length);
-
GrAnnotation._annotateText(textNode, index, length, 'foobar');
- assert.equal(parent.childNodes.length, 3);
-
- assert.instanceOf(parent.childNodes[0], Text);
- assert.equal(parent.childNodes[0].textContent, remainderPre);
-
- const secondChild = parent.childNodes[1] as HTMLElement;
- assert.instanceOf(secondChild, HTMLElement);
- assert.equal(secondChild.className, 'foobar');
- assert.instanceOf(secondChild.childNodes[0], Text);
- assert.equal(secondChild.childNodes[0].textContent, substr);
-
- assert.instanceOf(parent.childNodes[2], Text);
- assert.equal(parent.childNodes[2].textContent, remainderPost);
+ assert.equal(parent.textContent, str);
+ assert.equal(
+ parent.innerHTML,
+ 'Lorem ipsum <hl class="foobar">dolor </hl>sit amet, suspendisse inceptos vehicula'
+ );
});
test('_annotateElement design doc example', () => {
@@ -116,45 +114,9 @@
});
assert.equal(parent.textContent, str);
-
- // Layer 1:
- const layer1 = parent.querySelectorAll<HTMLElement>('.layer-1');
- assert.equal(layer1.length, 1);
- assert.equal(layer1[0].textContent, layers[0]);
- assert.equal(layer1[0].parentElement, parent);
-
- // Layer 2:
- const layer2 = parent.querySelectorAll<HTMLElement>('.layer-2');
- assert.equal(layer2.length, 1);
- assert.equal(layer2[0].textContent, layers[1]);
- assert.equal(layer2[0].parentElement, parent);
-
- // Layer 3:
- const layer3 = parent.querySelectorAll<HTMLElement>('.layer-3');
- assert.equal(layer3.length, 1);
- assert.equal(layer3[0].textContent, layers[2]);
- assert.equal(layer3[0].parentElement, layer1[0]);
-
- // Layer 4:
- const layer4 = parent.querySelectorAll<HTMLElement>('.layer-4');
- assert.equal(layer4.length, 3);
-
- assert.equal(layer4[0].textContent, 'et, ');
- assert.equal(layer4[0].parentElement, layer3[0]);
-
- assert.equal(layer4[1].textContent, 'suspendisse ');
- assert.equal(layer4[1].parentElement, parent);
-
- assert.equal(layer4[2].textContent, 'ince');
- assert.equal(layer4[2].parentElement, layer2[0]);
-
assert.equal(
- [
- layer4[0].textContent,
- layer4[1].textContent,
- layer4[2].textContent,
- ].join(''),
- layers[3]
+ parent.innerHTML,
+ 'Lorem ipsum dolor sit <hl class="layer-1"><hl class="layer-3">am<hl class="layer-4">et, </hl></hl></hl><hl class="layer-4">suspendisse </hl><hl class="layer-2"><hl class="layer-4">ince</hl>ptos </hl>vehicula'
);
});
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/embed/diff/gr-diff/gr-diff_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
index bbe091f..ce1393d 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
@@ -3218,6 +3218,7 @@
</div>
<div class="thread-group" data-side="right">
<slot name="right-FILE"> </slot>
+ <slot name="left-FILE"> </slot>
</div>
</td>
</tr>
diff --git a/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts b/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts
index 58f3f75..38eecfa 100644
--- a/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts
+++ b/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts
@@ -144,12 +144,13 @@
side,
range,
operation: (forLine, startChar, endChar) => {
- forLine.push({
- start: startChar,
- end: endChar,
- id: id(commentRange),
- longRange,
- });
+ if (startChar !== endChar)
+ forLine.push({
+ start: startChar,
+ end: endChar,
+ id: id(commentRange),
+ longRange,
+ });
},
});
}
@@ -202,7 +203,7 @@
// Normalize invalid ranges where the start is after the end but the
// start still makes sense. Set the end to the end of the line.
// @see Issue 5744
- if (range.start >= range.end && range.start < line.text.length) {
+ if (range.start > range.end && range.start < line.text.length) {
range.end = line.text.length;
}
diff --git a/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.ts b/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.ts
index 33515b25..7feda47 100644
--- a/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.ts
@@ -69,6 +69,16 @@
},
};
+const rangeF: CommentRangeLayer = {
+ side: Side.RIGHT,
+ range: {
+ end_character: 0,
+ end_line: 24,
+ start_character: 0,
+ start_line: 23,
+ },
+};
+
suite('gr-ranged-comment-layer', () => {
let element: GrRangedCommentLayer;
@@ -79,6 +89,7 @@
rangeC,
rangeD,
rangeE,
+ rangeF,
];
element = new GrRangedCommentLayer();
@@ -219,6 +230,16 @@
);
});
+ test('do not annotate lines with end_character 0', () => {
+ line = new GrDiffLine(GrDiffLineType.BOTH);
+ line.afterNumber = 24;
+ el.setAttribute('data-side', Side.RIGHT);
+
+ element.annotate(el, lineNumberEl, line);
+
+ assert.isFalse(annotateElementStub.called);
+ });
+
test('updateRanges remove all', () => {
assertHasRange(rangeA, true);
assertHasRange(rangeB, true);
diff --git a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
index c8f4fa6..b383fd7 100644
--- a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
+++ b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
@@ -219,6 +219,7 @@
);
}
this.addEventListener('request-dependency', this.resolveDep);
+ this.addEventListener('reload', this.reload);
}
private removeTargetEventListeners() {
@@ -231,6 +232,7 @@
}
this.targetCleanups = [];
this.removeEventListener('request-dependency', this.resolveDep);
+ this.removeEventListener('reload', this.reload);
}
/**
@@ -246,6 +248,10 @@
}
}
+ readonly reload = () => {
+ this.dispatchEventThroughTarget('reload');
+ };
+
readonly mouseDebounceHide = (e: MouseEvent) => {
this.debounceHide({mouseEvent: e});
};
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index 9b7986b..6ad03a3 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -1076,7 +1076,8 @@
changesPerPage?: number,
query?: string,
offset?: 'n,z' | number,
- options?: string
+ options?: string,
+ errFn?: ErrorCallback
): Promise<ChangeInfo[] | undefined> {
const request = this.getRequestForGetChanges(
changesPerPage,
@@ -1086,9 +1087,13 @@
);
return Promise.resolve(
- this._restApiHelper.fetchJSON(request, true) as Promise<
- ChangeInfo[] | undefined
- >
+ this._restApiHelper.fetchJSON(
+ {
+ ...request,
+ errFn,
+ },
+ true
+ ) as Promise<ChangeInfo[] | undefined>
).then(response => {
if (!response) {
return;
@@ -1313,13 +1318,15 @@
queryChangeFiles(
changeNum: NumericChangeId,
patchNum: PatchSetNum,
- query: string
+ query: string,
+ errFn?: ErrorCallback
) {
return this._getChangeURLAndFetch({
changeNum,
endpoint: `/files?q=${encodeURIComponent(query)}`,
revision: patchNum,
anonymizedEndpoint: '/files?q=*',
+ errFn,
}) as Promise<string[] | undefined>;
}
@@ -1350,22 +1357,37 @@
>;
}
- getChangeSuggestedReviewers(changeNum: NumericChangeId, inputVal: string) {
+ getChangeSuggestedReviewers(
+ changeNum: NumericChangeId,
+ inputVal: string,
+ errFn?: ErrorCallback
+ ) {
return this._getChangeSuggestedGroup(
ReviewerState.REVIEWER,
changeNum,
- inputVal
+ inputVal,
+ errFn
);
}
- getChangeSuggestedCCs(changeNum: NumericChangeId, inputVal: string) {
- return this._getChangeSuggestedGroup(ReviewerState.CC, changeNum, inputVal);
+ getChangeSuggestedCCs(
+ changeNum: NumericChangeId,
+ inputVal: string,
+ errFn?: ErrorCallback
+ ) {
+ return this._getChangeSuggestedGroup(
+ ReviewerState.CC,
+ changeNum,
+ inputVal,
+ errFn
+ );
}
_getChangeSuggestedGroup(
reviewerState: ReviewerState,
changeNum: NumericChangeId,
- inputVal: string
+ inputVal: string,
+ errFn?: ErrorCallback
): Promise<SuggestedReviewerInfo[] | undefined> {
// More suggestions may obscure content underneath in the reply dialog,
// see issue 10793.
@@ -1381,6 +1403,7 @@
endpoint: '/suggest_reviewers',
params,
reportEndpointAsIs: true,
+ errFn,
}) as Promise<SuggestedReviewerInfo[] | undefined>;
}
@@ -1468,7 +1491,8 @@
async getRepos(
filter: string | undefined,
reposPerPage: number,
- offset?: number
+ offset?: number,
+ errFn?: ErrorCallback
): Promise<ProjectInfoWithName[] | undefined> {
const [isQuery, url] = this._getReposUrl(filter, reposPerPage, offset);
@@ -1482,11 +1506,13 @@
return this._fetchSharedCacheURL({
url,
anonymizedUrl: '/projects/?*',
+ errFn,
}) as Promise<ProjectInfoWithName[] | undefined>;
} else {
const result = await (this._fetchSharedCacheURL({
url,
anonymizedUrl: '/projects/?*',
+ errFn,
}) as Promise<NameToProjectInfoMap | undefined>);
if (result === undefined) return [];
return Object.entries(result).map(([name, project]) => {
@@ -1612,7 +1638,8 @@
getSuggestedGroups(
inputVal: string,
project?: RepoName,
- n?: number
+ n?: number,
+ errFn?: ErrorCallback
): Promise<GroupNameToGroupInfoMap | undefined> {
const params: QueryGroupsParams = {s: inputVal};
if (n) {
@@ -1625,12 +1652,14 @@
url: '/groups/',
params,
reportUrlAsIs: true,
+ errFn,
}) as Promise<GroupNameToGroupInfoMap | undefined>;
}
getSuggestedRepos(
inputVal: string,
- n?: number
+ n?: number,
+ errFn?: ErrorCallback
): Promise<NameToProjectInfoMap | undefined> {
const params = {
m: inputVal,
@@ -1644,6 +1673,7 @@
url: '/projects/',
params,
reportUrlAsIs: true,
+ errFn,
});
}
@@ -1651,7 +1681,8 @@
inputVal: string,
n?: number,
canSee?: NumericChangeId,
- filterActive?: boolean
+ filterActive?: boolean,
+ errFn?: ErrorCallback
): Promise<AccountInfo[] | undefined> {
const params: QueryAccountsParams = {o: 'DETAILS', q: ''};
const queryParams = [];
@@ -1678,6 +1709,7 @@
url: '/accounts/',
params,
anonymizedUrl: '/accounts/?n=*',
+ errFn,
}) as Promise<AccountInfo[] | undefined>;
}
@@ -1830,23 +1862,29 @@
}) as Promise<ChangeInfo[] | undefined>;
}
- getChangesWithSimilarTopic(topic: string): Promise<ChangeInfo[] | undefined> {
+ getChangesWithSimilarTopic(
+ topic: string,
+ errFn?: ErrorCallback
+ ): Promise<ChangeInfo[] | undefined> {
const query = `intopic:${escapeAndWrapSearchOperatorValue(topic)}`;
return this._restApiHelper.fetchJSON({
url: '/changes/',
params: {q: query},
anonymizedUrl: '/changes/intopic:*',
+ errFn,
}) as Promise<ChangeInfo[] | undefined>;
}
getChangesWithSimilarHashtag(
- hashtag: string
+ hashtag: string,
+ errFn?: ErrorCallback
): Promise<ChangeInfo[] | undefined> {
const query = `inhashtag:${escapeAndWrapSearchOperatorValue(hashtag)}`;
return this._restApiHelper.fetchJSON({
url: '/changes/',
params: {q: query},
anonymizedUrl: '/changes/inhashtag:*',
+ errFn,
}) as Promise<ChangeInfo[] | undefined>;
}
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
index 984efa9..b4b1afb 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
@@ -127,7 +127,8 @@
getRepos(
filter: string | undefined,
reposPerPage: number,
- offset?: number
+ offset?: number,
+ errFn?: ErrorCallback
): Promise<ProjectInfoWithName[] | undefined>;
send(
@@ -152,11 +153,13 @@
getChangeSuggestedReviewers(
changeNum: NumericChangeId,
- input: string
+ input: string,
+ errFn?: ErrorCallback
): Promise<SuggestedReviewerInfo[] | undefined>;
getChangeSuggestedCCs(
changeNum: NumericChangeId,
- input: string
+ input: string,
+ errFn?: ErrorCallback
): Promise<SuggestedReviewerInfo[] | undefined>;
/**
* Request list of accounts via https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#query-account
@@ -166,12 +169,14 @@
input: string,
n?: number,
canSee?: NumericChangeId,
- filterActive?: boolean
+ filterActive?: boolean,
+ errFn?: ErrorCallback
): Promise<AccountInfo[] | undefined>;
getSuggestedGroups(
input: string,
project?: RepoName,
- n?: number
+ n?: number,
+ errFn?: ErrorCallback
): Promise<GroupNameToGroupInfoMap | undefined>;
/**
* Execute a change action or revision action on a change.
@@ -267,7 +272,8 @@
queryChangeFiles(
changeNum: NumericChangeId,
patchNum: PatchSetNum,
- query: string
+ query: string,
+ errFn?: ErrorCallback
): Promise<string[] | undefined>;
getRepoAccessRights(
@@ -472,7 +478,8 @@
changesPerPage?: number,
query?: string,
offset?: 'n,z' | number,
- options?: string
+ options?: string,
+ errFn?: ErrorCallback
): Promise<ChangeInfo[] | undefined>;
getChangesForMultipleQueries(
changesPerPage?: number,
@@ -515,7 +522,8 @@
getSuggestedRepos(
inputVal: string,
- n?: number
+ n?: number,
+ errFn?: ErrorCallback
): Promise<NameToProjectInfoMap | undefined>;
invalidateGroupsCache(): void;
@@ -652,9 +660,13 @@
changeToExclude?: NumericChangeId;
}
): Promise<ChangeInfo[] | undefined>;
- getChangesWithSimilarTopic(topic: string): Promise<ChangeInfo[] | undefined>;
+ getChangesWithSimilarTopic(
+ topic: string,
+ errFn?: ErrorCallback
+ ): Promise<ChangeInfo[] | undefined>;
getChangesWithSimilarHashtag(
- hashtag: string
+ hashtag: string,
+ errFn?: ErrorCallback
): Promise<ChangeInfo[] | undefined>;
/**
diff --git a/polygerrit-ui/app/services/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts b/polygerrit-ui/app/services/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
index 3b13dbc..842dace 100644
--- a/polygerrit-ui/app/services/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
+++ b/polygerrit-ui/app/services/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
@@ -29,6 +29,7 @@
GroupId,
ReviewerState,
} from '../../api/rest-api';
+import {throwingErrorCallback} from '../../elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
export interface ReviewerSuggestionsProvider {
getSuggestions(input: string): Promise<Suggestion[]>;
@@ -52,6 +53,11 @@
this.changes = changes;
}
+ /**
+ * Requests related suggestions.
+ *
+ * If the request fails the returned promise is rejected.
+ */
async getSuggestions(input: string): Promise<Suggestion[]> {
if (!this.loggedIn) return [];
@@ -121,8 +127,16 @@
input: string
): Promise<SuggestedReviewerInfo[] | undefined> {
return this.type === ReviewerState.REVIEWER
- ? this.restApi.getChangeSuggestedReviewers(changeNumber, input)
- : this.restApi.getChangeSuggestedCCs(changeNumber, input);
+ ? this.restApi.getChangeSuggestedReviewers(
+ changeNumber,
+ input,
+ throwingErrorCallback
+ )
+ : this.restApi.getChangeSuggestedCCs(
+ changeNumber,
+ input,
+ throwingErrorCallback
+ );
}
}
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),
});
}
});
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index 40ad59c..ecae845 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -294,7 +294,13 @@
const styleEl = document.createElement('style');
styleEl.setAttribute('id', 'dark-theme');
safeStyleEl.setTextContent(styleEl, darkThemeCss);
- document.head.appendChild(styleEl);
+
+ // We would like to insert the dark theme styles after the light theme such
+ // that the dark theme values override the defaults in the light theme. But
+ // OTOH we want to insert before any plugin provided styles, because we do NOT
+ // want to override those.
+ const pluginStyleEl = document.head.querySelector('style#plugin-style');
+ document.head.insertBefore(styleEl, pluginStyleEl);
}
export function removeTheme() {
diff --git a/polygerrit-ui/app/tsconfig.json b/polygerrit-ui/app/tsconfig.json
index 0ddf130..98aaf0f 100644
--- a/polygerrit-ui/app/tsconfig.json
+++ b/polygerrit-ui/app/tsconfig.json
@@ -47,7 +47,7 @@
"lib": [
"dom",
"dom.iterable",
- "es2020",
+ "es2021",
"webworker"
],
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index bfddbdc..ebf9e7a 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -24,6 +24,7 @@
import {getApprovalInfo} from './label-util';
import {RestApiService} from '../services/gr-rest-api/gr-rest-api';
import {ParsedChangeInfo} from '../types/types';
+import {throwingErrorCallback} from '../elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
export const ACCOUNT_TEMPLATE_REGEX = '<GERRIT_ACCOUNT_(\\d+)>';
const SUGGESTIONS_LIMIT = 15;
@@ -196,7 +197,13 @@
filterActive = false
) {
return restApiService
- .getSuggestedAccounts(input, SUGGESTIONS_LIMIT, canSee, filterActive)
+ .getSuggestedAccounts(
+ input,
+ SUGGESTIONS_LIMIT,
+ canSee,
+ filterActive,
+ throwingErrorCallback
+ )
.then(accounts => {
if (!accounts) return [];
const accountSuggestions = [];
diff --git a/polygerrit-ui/app/utils/string-util.ts b/polygerrit-ui/app/utils/string-util.ts
index 9955f68..81dcde1 100644
--- a/polygerrit-ui/app/utils/string-util.ts
+++ b/polygerrit-ui/app/utils/string-util.ts
@@ -40,7 +40,7 @@
* contain spaces and colons.
*/
export function escapeAndWrapSearchOperatorValue(value: string): string {
- return `"${value.replace('\\', '\\\\').replace('"', '\\"')}"`;
+ return `"${value.replaceAll('\\', '\\\\').replaceAll('"', '\\"')}"`;
}
/**
diff --git a/polygerrit-ui/app/utils/string-util_test.ts b/polygerrit-ui/app/utils/string-util_test.ts
index c6c65b1..d6c4187 100644
--- a/polygerrit-ui/app/utils/string-util_test.ts
+++ b/polygerrit-ui/app/utils/string-util_test.ts
@@ -10,6 +10,7 @@
ordinal,
listForSentence,
diffFilePaths,
+ escapeAndWrapSearchOperatorValue,
} from './string-util';
suite('string-util tests', () => {
@@ -84,4 +85,11 @@
fileName: 'COMMIT_MSG',
});
});
+
+ test('escapeAndWrapSearchOperatorValue', () => {
+ assert.equal(
+ escapeAndWrapSearchOperatorValue('"value of \\: \\"something"'),
+ '"\\"value of \\\\: \\\\\\"something\\""'
+ );
+ });
});
diff --git a/polygerrit-ui/app/workers/service-worker-class.ts b/polygerrit-ui/app/workers/service-worker-class.ts
index f9cc591..1c54158 100644
--- a/polygerrit-ui/app/workers/service-worker-class.ts
+++ b/polygerrit-ui/app/workers/service-worker-class.ts
@@ -131,9 +131,12 @@
// User can have different service workers for different origins/hosts.
// TODO(milutin): Check if this works properly with getBaseUrl()
const data = {url: `${self.location.origin}${changeUrl}`};
-
- // TODO(milutin): Add gerrit host icon
- this.ctx.registration.showNotification(change.subject, {body, data});
+ const icon = `${self.location.origin}/favicon.ico`;
+ this.ctx.registration.showNotification(change.subject, {
+ body,
+ data,
+ icon,
+ });
this.sendReport('notify about 1 change');
}
@@ -141,7 +144,8 @@
const title = `You are in the attention set for ${numOfChangesToNotifyAbout} changes.`;
const dashboardUrl = createDashboardUrl({});
const data = {url: `${self.location.origin}${dashboardUrl}`};
- this.ctx.registration.showNotification(title, {data});
+ const icon = `${self.location.origin}/favicon.ico`;
+ this.ctx.registration.showNotification(title, {data, icon});
this.sendReport(`notify about ${numOfChangesToNotifyAbout} changes`);
}
diff --git a/tools/bzl/asciidoc.bzl b/tools/bzl/asciidoc.bzl
index 7977cf0..0858d60 100644
--- a/tools/bzl/asciidoc.bzl
+++ b/tools/bzl/asciidoc.bzl
@@ -122,7 +122,7 @@
),
"_exe": attr.label(
default = Label("//java/com/google/gerrit/asciidoctor:asciidoc"),
- cfg = "host",
+ cfg = "exec",
allow_files = True,
executable = True,
),
diff --git a/yarn.lock b/yarn.lock
index ab1eb89..d66270b 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -90,10 +90,10 @@
dependencies:
google-protobuf "^3.6.1"
-"@es-joy/jsdoccomment@~0.31.0":
- version "0.31.0"
- resolved "https://registry.yarnpkg.com/@es-joy/jsdoccomment/-/jsdoccomment-0.31.0.tgz#dbc342cc38eb6878c12727985e693eaef34302bc"
- integrity sha512-tc1/iuQcnaiSIUVad72PBierDFpsxdUHtEF/OrfqvM1CBAsIoMP51j52jTMb3dXriwhieTo289InzZj72jL3EQ==
+"@es-joy/jsdoccomment@~0.36.1":
+ version "0.36.1"
+ resolved "https://registry.yarnpkg.com/@es-joy/jsdoccomment/-/jsdoccomment-0.36.1.tgz#c37db40da36e4b848da5fd427a74bae3b004a30f"
+ integrity sha512-922xqFsTpHs6D0BUiG4toiyPOMc8/jafnWKxz1KWgS4XzKPy2qXf1Pe6UFuNSCQqt6tOuhAWXBNuuyUhJmw9Vg==
dependencies:
comment-parser "1.3.1"
esquery "^1.4.0"
@@ -1704,17 +1704,17 @@
resolve "^1.22.0"
tsconfig-paths "^3.14.1"
-eslint-plugin-jsdoc@^39.3.2:
- version "39.3.2"
- resolved "https://registry.yarnpkg.com/eslint-plugin-jsdoc/-/eslint-plugin-jsdoc-39.3.2.tgz#b9c3becdbd860a75b8bd07bd04a0eaaad7c79403"
- integrity sha512-RSGN94RYzIJS/WfW3l6cXzRLfJWxvJgNQZ4w0WCaxJWDJMigtwTsILEAfKqmmPkT2rwMH/s3C7G5ChDE6cwPJg==
+eslint-plugin-jsdoc@^39.6.4:
+ version "39.6.4"
+ resolved "https://registry.yarnpkg.com/eslint-plugin-jsdoc/-/eslint-plugin-jsdoc-39.6.4.tgz#b940aebd3eea26884a0d341785d2dc3aba6a38a7"
+ integrity sha512-fskvdLCfwmPjHb6e+xNGDtGgbF8X7cDwMtVLAP2WwSf9Htrx68OAx31BESBM1FAwsN2HTQyYQq7m4aW4Q4Nlag==
dependencies:
- "@es-joy/jsdoccomment" "~0.31.0"
+ "@es-joy/jsdoccomment" "~0.36.1"
comment-parser "1.3.1"
debug "^4.3.4"
escape-string-regexp "^4.0.0"
esquery "^1.4.0"
- semver "^7.3.7"
+ semver "^7.3.8"
spdx-expression-parse "^3.0.1"
eslint-plugin-lit@^1.6.1:
@@ -4107,6 +4107,13 @@
dependencies:
lru-cache "^6.0.0"
+semver@^7.3.8:
+ version "7.3.8"
+ resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.8.tgz#07a78feafb3f7b32347d725e33de7e2a2df67798"
+ integrity sha512-NB1ctGL5rlHrPJtFDVIVzTyQylMLu9N9VICA6HSFJo8MCGVTMW6gfpicwKmmK/dAjTOrqu5l63JJOpDSrAis3A==
+ dependencies:
+ lru-cache "^6.0.0"
+
set-blocking@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/set-blocking/-/set-blocking-2.0.0.tgz#045f9782d011ae9a6803ddd382b24392b3d890f7"
