Merge "Add a plugin API for adding css styles"
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/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/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/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
index 591ae26..5838ff1 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;
@@ -269,7 +270,8 @@
Project.nameKey((String) doc.get(ChangeField.PROJECT_SPEC.getName())),
Change.id(Integer.valueOf((String) doc.get(ChangeField.LEGACY_ID_STR.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/server/StarredChangesUtil.java b/java/com/google/gerrit/server/StarredChangesUtil.java
index 9ac85e0..0c8bccd 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.ID, ChangeField.STAR_SPEC)
.byLegacyChangeId(changeId);
if (changeData.size() != 1) {
throw new NoSuchChangeException(changeId);
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 7aefa63..5b8d784 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -19,7 +19,6 @@
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;
@@ -346,8 +345,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 +555,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 +567,7 @@
*/
public static final IndexedField<ChangeData, String> IS_SUBMITTABLE_FIELD =
IndexedField.<ChangeData>stringBuilder("IsSubmittable")
+ .size(1)
.build(
cd ->
// All submit requirements should be fulfilled
@@ -1211,10 +1211,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,17 +1226,26 @@
.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")
@@ -1245,14 +1254,20 @@
(cd, field) -> cd.setPatchSets(decodeProtos(field, PatchSetProtoConverter.INSTANCE)));
/** 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 +1281,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 +1298,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();
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 2af5d60..6c92f111 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -37,17 +37,11 @@
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,
@@ -72,11 +66,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,
@@ -96,7 +93,9 @@
ChangeField.REVERT_OF_FIELD,
ChangeField.REVIEWER_BY_EMAIL_FIELD,
ChangeField.REVIEWER_FIELD,
+ ChangeField.STARBY_FIELD,
ChangeField.STARTED_FIELD,
+ ChangeField.STAR_FIELD,
ChangeField.STATUS_FIELD,
ChangeField.SUBMISSIONID_FIELD,
ChangeField.TOPIC_FIELD,
@@ -104,7 +103,8 @@
ChangeField.TR_FIELD,
ChangeField.UNRESOLVED_COMMENT_COUNT_FIELD,
ChangeField.UPLOADER_FIELD,
- ChangeField.WIP_FIELD),
+ ChangeField.WIP_FIELD,
+ ChangeField.REVIEWEDBY_FIELD),
ImmutableList.<IndexedField<ChangeData, ?>.SearchSpec>of(
ChangeField.ADDED_LINES_SPEC,
ChangeField.ASSIGNEE_SPEC,
@@ -123,6 +123,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 +134,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,
@@ -152,14 +155,17 @@
ChangeField.REVERT_OF,
ChangeField.REVIEWER_BY_EMAIL,
ChangeField.REVIEWER_SPEC,
+ ChangeField.STARBY_SPEC,
ChangeField.STARTED_SPEC,
+ ChangeField.STAR_SPEC,
ChangeField.STATUS_SPEC,
ChangeField.SUBMISSIONID_SPEC,
ChangeField.TOTAL_COMMENT_COUNT_SPEC,
ChangeField.TR_SPEC,
ChangeField.UNRESOLVED_COMMENT_COUNT_SPEC,
ChangeField.UPLOADER_SPEC,
- ChangeField.WIP_SPEC));
+ ChangeField.WIP_SPEC,
+ ChangeField.REVIEWEDBY_SPEC));
/**
* Added new field {@link ChangeField#PREFIX_HASHTAG} and {@link ChangeField#PREFIX_TOPIC} to
@@ -198,7 +204,8 @@
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();
/**
* Name of the change index to be used when contacting index backends or loading configurations.
diff --git a/java/com/google/gerrit/server/query/change/ChangePredicates.java b/java/com/google/gerrit/server/query/change/ChangePredicates.java
index 6b8cdd8..c2672a9 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()));
}
/**
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/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/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/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-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/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/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-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..14b8280 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
@@ -348,7 +348,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,11 +369,21 @@
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) {
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/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/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,
),
