Merge "Bump gson version to 2.9.0"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index ace6995..1a535ef 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -5273,6 +5273,18 @@
+
By default, true.
+[[tracing.exportPerformanceMetrics]]tracing.exportPerformanceMetrics::
++
+Whether to export performance metrics.
++
+Performace logged when link:#tracing.performanceLogging[`performanceLogging`] is
+enabled, can be exported as metrics.
++
+NOTE: Since the payload returned could be of tens of thousands metrics,
+assess the latency of the metrics endpoint before enabling this option.
++
+By default, false.
+
[[tracing.traceid]]
==== Subsection tracing.<trace-id>
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index 55aa9bd..c62b626 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -304,25 +304,17 @@
Matches votes that are equal to the minimal or maximal voting range. Or any votes.
-==== approverin:link:rest-api-groups.html#group-id[\{group-id\}]
+[[approverin]]
+==== approverin:link:#group-id[\{group-id\}]
Matches votes granted by a user who is a member of
-link:rest-api-groups.html#group-id[\{group-id\}].
+link:#group-id[\{group-id\}].
-Avoid using a group name with spaces (if it has spaces, use the group uuid).
-Although supported for convenience, it's better to use group uuid than group
-name since using names only works as long as the names are unique (and future
-groups with the same name will break the query).
-
-==== uploaderin:link:rest-api-groups.html#group-id[\{group-id\}]
+[[uploaderin]]
+==== uploaderin:link:#group-id[\{group-id\}]
Matches votes where the new patch set was uploaded by a member of
-link:rest-api-groups.html#group-id[\{group-id\}].
-
-Avoid using a group name with spaces (if it has spaces, use the group uuid).
-Although supported for convenience, it's better to use group uuid than group
-name since using names only works as long as the names are unique (and future
-groups with the same name will break the query).
+link:#group-id[\{group-id\}].
==== has:unchanged-files
@@ -330,6 +322,29 @@
Only 'unchanged-files' is supported for 'has'.
+[[group-id]]
+==== Group ID
+
+Some predicates (link:#approverin[approverin], link:#uploaderin[uploaderin])
+expect a group ID as value. This group ID can be any of the
+link:rest-api-groups.html#group-id[group identifiers] that are supported in the
+REST API: group UUID, group ID (for Gerrit internal groups only) and group name
+
+It's preferred to reference groups by UUID, rather than name. Referencing
+groups by name is not recommended because:
+
+* Groups may be renamed and then the group reference can no longer be resolved.
+ If this happens another group with different members can take over the group
+ name, so that exemptions which have been granted by this predicate apply to
+ the other group. This is a security concern.
+* Group names that contain spaces are not supported.
+* Ambiguous group names cannot be resolved. This means if another group with
+ the same name gets created at a later point in time, the group name can no
+ longer be resolved and the predicate breaks.
+
+Using the group UUID has a small drawback though, since it makes the condition
+less human-readable.
+
==== Example
----
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index bf4b2e4..59f9c83 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -327,7 +327,6 @@
* git-protocol-v2
* git-upload-archive
* notedb
-* no_rbe
* pgm
* rest
* server
@@ -609,6 +608,40 @@
--disk-size=200
```
+Due to outdated Git version in official RBE docker images, a custom RBE docker
+image must be used. To build custom docker imager, change to the directory
+`tools/platforms` and build and publish custom RBE docker image.
+
+To build the custom RBE docker image, run:
+
+```
+docker build -t gcr.io/api-project-164060093628/rbe-ubuntu18-04 .
+```
+
+To publish the custom RBE docker image, run:
+
+```
+docker push gcr.io/api-project-164060093628/rbe-ubuntu18-04
+[...]
+latest: digest: sha256:de5186d4313630a6111f9a2449b72563d0bc59ec9fb60956f063b69a38a76834 size: 1584
+```
+
+Re-build rbe_autoconfig project conduct a new release and switch to using it
+in `WORKSPACE` file.
+
+Note, to authenticate to the gcr.io registry, the following command must be
+used:
+
+```
+gcloud auth configure-docker
+```
+
+To see the documentation, developer must be added to this group:
+https://groups.google.com/forum/#!forum/rbe-alpha-customers.
+
+Documentation can be found at:
+https://cloud.google.com/remote-build-execution/docs.
+
To use RBE, execute
```
diff --git a/Documentation/dev-processes.txt b/Documentation/dev-processes.txt
index e43e021..f0fe1e5 100644
--- a/Documentation/dev-processes.txt
+++ b/Documentation/dev-processes.txt
@@ -378,6 +378,12 @@
that they are vetted long enough before they go into a release and we can be sure
that the update doesn't introduce a regression.
+[[escalation-channel-to-google]]
+== Escalation channel to Google
+
+If anything urgent is blocking that requires the attention of a Googler you may
+escalate this by writing an email to Han-Wen Nienhuys: hanwen@google.com
+
[[deprecating-features]]
== Deprecating features
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index d8b6250..9df4b04 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -93,6 +93,9 @@
** `plugin`:
The name of the plugin that performed the operation.
+Performance metrics can be enabled via the
+link:config.gerrit.html#tracing.exportPerformanceMetrics[`tracing.exportPerformanceMetrics`]
+setting.
=== Pushes
diff --git a/Documentation/note-db.txt b/Documentation/note-db.txt
index b376d6e..0e1dfd0 100644
--- a/Documentation/note-db.txt
+++ b/Documentation/note-db.txt
@@ -253,5 +253,5 @@
In case of rollback from NoteDB to ReviewDB, all the meta refs and the
sequence ref need to be removed.
-The [remove-notedb-refs.sh,role=external,window=_blank](https://gerrit.googlesource.com/gerrit/+/refs/heads/master/contrib/remove-notedb-refs.sh)
+The link:https://gerrit.googlesource.com/gerrit/+/refs/heads/master/contrib/remove-notedb-refs.sh[remove-notedb-refs.sh,role=external,window=_blank]
script has been written to automate this process.
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 82a9638..a9b31b0 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -7114,7 +7114,11 @@
|==========================
|Field Name ||Description
|`name` ||The name of the file.
-|`content_type`||The content type of the file.
+|`content_type`||The content type of the file. For the commit message and merge
+list the value is `text/x-gerrit-commit-message` and `text/x-gerrit-merge-list`
+respectively. For git links the value is `x-git/gitlink`. For symlinks the value
+is `x-git/symlink`. For regular files the value is the file mime type (e.g.
+`text/x-java`, `text/x-c++src`, etc...).
|`lines` ||The total number of lines in the file.
|`web_links` |optional|
Links to the file in external sites as a list of
@@ -8286,13 +8290,16 @@
submit requirement did not define an applicability expression.
Note that fields `expression`, `passing_atoms` and `failing_atoms` are always
omitted for the `applicability_expression_result`.
-|`submittability_expression_result`||
+|`submittability_expression_result`|optional|
A link:#submit-requirement-expression-info[SubmitRequirementExpressionInfo]
-containing the result of evaluating the submittability expression.
+containing the result of evaluating the submittability expression. +
+If the submit requirement does not apply, the expression is not evaluated and
+the field is not set.
|`override_expression_result`|optional|
A link:#submit-requirement-expression-info[SubmitRequirementExpressionInfo]
-containing the result of evaluating the override expression. Not set if the
-submit requirement did not define an override expression.
+containing the result of evaluating the override expression. +
+Not set if the submit requirement did not define an override expression or
+if it does not apply.
|===========================
[[submitted-together-info]]
diff --git a/WORKSPACE b/WORKSPACE
index 67bc7094..84ae4f8 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -35,11 +35,11 @@
http_archive(
name = "rbe_jdk11",
- sha256 = "766796de71916118e528b9f4334c29c9c9b4e926227bf3264dee555e6a4306c8",
- strip_prefix = "rbe_autoconfig-2.0.0",
+ sha256 = "5939e2a4e56d1fc53b6c44c6db97ee068c9f4bd18e86c762f6ab8b4fff5e294b",
+ strip_prefix = "rbe_autoconfig-3.0.0",
urls = [
- "https://gerrit-bazel.storage.googleapis.com/rbe_autoconfig/v2.0.0.tar.gz",
- "https://github.com/davido/rbe_autoconfig/archive/v2.0.0.tar.gz",
+ "https://gerrit-bazel.storage.googleapis.com/rbe_autoconfig/v3.0.0.tar.gz",
+ "https://github.com/davido/rbe_autoconfig/archive/v3.0.0.tar.gz",
],
)
diff --git a/java/com/google/gerrit/acceptance/TestOnStoreSubmitRequirementResultModifier.java b/java/com/google/gerrit/acceptance/TestOnStoreSubmitRequirementResultModifier.java
new file mode 100644
index 0000000..63e130a
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/TestOnStoreSubmitRequirementResultModifier.java
@@ -0,0 +1,81 @@
+// 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.
+
+package com.google.gerrit.acceptance;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.server.project.OnStoreSubmitRequirementResultModifier;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.update.ChangeContext;
+import java.util.Optional;
+
+/** Implementation of {@link OnStoreSubmitRequirementResultModifier} that is used in tests. */
+public class TestOnStoreSubmitRequirementResultModifier
+ implements OnStoreSubmitRequirementResultModifier {
+
+ private ModificationStrategy modificationStrategy = ModificationStrategy.KEEP;
+
+ /**
+ * The strategy, used by this modifier to transform {@link SubmitRequirementResult} on {@link
+ * OnStoreSubmitRequirementResultModifier#modifyResultOnStore} invocations.
+ */
+ public enum ModificationStrategy {
+ KEEP,
+ FAIL,
+ PASS,
+ OVERRIDE
+ }
+
+ public void setModificationStrategy(ModificationStrategy modificationStrategy) {
+ this.modificationStrategy = modificationStrategy;
+ }
+
+ @Override
+ public SubmitRequirementResult modifyResultOnStore(
+ SubmitRequirement submitRequirement,
+ SubmitRequirementResult result,
+ ChangeData cd,
+ ChangeContext ctx) {
+ if (modificationStrategy.equals(ModificationStrategy.KEEP)) {
+ return result;
+ }
+ if (modificationStrategy.equals(ModificationStrategy.OVERRIDE)) {
+ return result
+ .toBuilder()
+ .overrideExpressionResult(
+ Optional.of(
+ SubmitRequirementExpressionResult.create(
+ submitRequirement.submittabilityExpression(),
+ SubmitRequirementExpressionResult.Status.PASS,
+ ImmutableList.of(),
+ ImmutableList.of(
+ submitRequirement.submittabilityExpression().expressionString()))))
+ .build();
+ }
+ return result
+ .toBuilder()
+ .submittabilityExpressionResult(
+ SubmitRequirementExpressionResult.create(
+ submitRequirement.submittabilityExpression(),
+ modificationStrategy.equals(ModificationStrategy.FAIL)
+ ? SubmitRequirementExpressionResult.Status.FAIL
+ : SubmitRequirementExpressionResult.Status.PASS,
+ ImmutableList.of(),
+ ImmutableList.of(submitRequirement.submittabilityExpression().expressionString())))
+ .build();
+ }
+}
diff --git a/java/com/google/gerrit/common/UsedAt.java b/java/com/google/gerrit/common/UsedAt.java
index 73b1d40..3e103c8 100644
--- a/java/com/google/gerrit/common/UsedAt.java
+++ b/java/com/google/gerrit/common/UsedAt.java
@@ -42,6 +42,7 @@
PLUGIN_SERVICEUSER,
PLUGIN_HIGH_AVAILABILITY,
PLUGIN_MULTI_SITE,
+ PLUGIN_WEBSESSION_FLATFILE,
PLUGINS_ALL, // Use this project if a method/type is generally made available to all plugins.
}
diff --git a/java/com/google/gerrit/common/data/FilenameComparator.java b/java/com/google/gerrit/common/data/FilenameComparator.java
index ebf423c..0b188df 100644
--- a/java/com/google/gerrit/common/data/FilenameComparator.java
+++ b/java/com/google/gerrit/common/data/FilenameComparator.java
@@ -24,7 +24,7 @@
public static final FilenameComparator INSTANCE = new FilenameComparator();
private static final Set<String> cppHeaderSuffixes =
- new HashSet<>(Arrays.asList(".h", ".hxx", ".hpp"));
+ new HashSet<>(Arrays.asList(".h", ".hh", ".hxx", ".hpp"));
private FilenameComparator() {}
diff --git a/java/com/google/gerrit/entities/SubmitRequirement.java b/java/com/google/gerrit/entities/SubmitRequirement.java
index 3f91cc7..673665d 100644
--- a/java/com/google/gerrit/entities/SubmitRequirement.java
+++ b/java/com/google/gerrit/entities/SubmitRequirement.java
@@ -44,7 +44,7 @@
* Expression of the condition that makes the requirement applicable. The expression should be
* evaluated for a specific {@link Change} and if it returns false, the requirement becomes
* irrelevant for the change (i.e. {@link #submittabilityExpression()} and {@link
- * #overrideExpression()} become irrelevant).
+ * #overrideExpression()} are not evaluated).
*
* <p>An empty {@link Optional} indicates that the requirement is applicable for any change.
*/
diff --git a/java/com/google/gerrit/entities/SubmitRequirementResult.java b/java/com/google/gerrit/entities/SubmitRequirementResult.java
index a97560b..18c27a1 100644
--- a/java/com/google/gerrit/entities/SubmitRequirementResult.java
+++ b/java/com/google/gerrit/entities/SubmitRequirementResult.java
@@ -32,12 +32,18 @@
public abstract Optional<SubmitRequirementExpressionResult> applicabilityExpressionResult();
/**
- * Result of evaluating a {@link SubmitRequirement#submittabilityExpression()} ()} on a change.
+ * Result of evaluating a {@link SubmitRequirement#submittabilityExpression()} on a change.
+ *
+ * <p>Empty if submit requirement does not apply.
*/
- @Nullable
- public abstract SubmitRequirementExpressionResult submittabilityExpressionResult();
+ public abstract Optional<SubmitRequirementExpressionResult> submittabilityExpressionResult();
- /** Result of evaluating a {@link SubmitRequirement#overrideExpression()} ()} on a change. */
+ /**
+ * Result of evaluating a {@link SubmitRequirement#overrideExpression()} on a change.
+ *
+ * <p>Empty if submit requirement does not apply, or if the submit requirement did not define an
+ * override expression.
+ */
public abstract Optional<SubmitRequirementExpressionResult> overrideExpressionResult();
/** SHA-1 of the patchset commit ID for which the submit requirement was evaluated. */
@@ -71,11 +77,11 @@
"Applicability expression result has an error: "
+ applicabilityExpressionResult().get().errorMessage().get());
}
- if (submittabilityExpressionResult() != null
- && submittabilityExpressionResult().errorMessage().isPresent()) {
+ if (submittabilityExpressionResult().isPresent()
+ && submittabilityExpressionResult().get().errorMessage().isPresent()) {
return Optional.of(
"Submittability expression result has an error: "
- + submittabilityExpressionResult().errorMessage().get());
+ + submittabilityExpressionResult().get().errorMessage().get());
}
if (overrideExpressionResult().isPresent()
&& overrideExpressionResult().get().errorMessage().isPresent()) {
@@ -168,6 +174,9 @@
Optional<SubmitRequirementExpressionResult> value);
public abstract Builder submittabilityExpressionResult(
+ Optional<SubmitRequirementExpressionResult> value);
+
+ public abstract Builder submittabilityExpressionResult(
@Nullable SubmitRequirementExpressionResult value);
public abstract Builder overrideExpressionResult(
@@ -182,33 +191,25 @@
public abstract SubmitRequirementResult build();
}
- private boolean assertPass(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ public static boolean assertPass(Optional<SubmitRequirementExpressionResult> expressionResult) {
return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.PASS);
}
- private boolean assertPass(@Nullable SubmitRequirementExpressionResult expressionResult) {
- return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.PASS);
- }
-
- private boolean assertFail(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ public static boolean assertFail(Optional<SubmitRequirementExpressionResult> expressionResult) {
return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.FAIL);
}
- private boolean assertError(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ public static boolean assertError(Optional<SubmitRequirementExpressionResult> expressionResult) {
return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.ERROR);
}
- private boolean assertError(@Nullable SubmitRequirementExpressionResult expressionResult) {
- return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.ERROR);
- }
-
- private boolean assertStatus(
- @Nullable SubmitRequirementExpressionResult expressionResult,
+ private static boolean assertStatus(
+ SubmitRequirementExpressionResult expressionResult,
SubmitRequirementExpressionResult.Status status) {
- return expressionResult != null && expressionResult.status() == status;
+ return expressionResult.status() == status;
}
- private boolean assertStatus(
+ private static boolean assertStatus(
Optional<SubmitRequirementExpressionResult> expressionResult,
SubmitRequirementExpressionResult.Status status) {
return expressionResult.isPresent() && assertStatus(expressionResult.get(), status);
diff --git a/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java b/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
index 7b87be8..4778038 100644
--- a/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
+++ b/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
@@ -14,6 +14,8 @@
package com.google.gerrit.extensions.common;
+import com.google.gerrit.common.Nullable;
+
/** Result of evaluating a submit requirement on a change. */
public class SubmitRequirementResultInfo {
public enum Status {
@@ -63,11 +65,11 @@
public boolean isLegacy;
/** Result of evaluating the applicability expression. */
- public SubmitRequirementExpressionInfo applicabilityExpressionResult;
+ @Nullable public SubmitRequirementExpressionInfo applicabilityExpressionResult;
/** Result of evaluating the submittability expression. */
- public SubmitRequirementExpressionInfo submittabilityExpressionResult;
+ @Nullable public SubmitRequirementExpressionInfo submittabilityExpressionResult;
/** Result of evaluating the override expression. */
- public SubmitRequirementExpressionInfo overrideExpressionResult;
+ @Nullable public SubmitRequirementExpressionInfo overrideExpressionResult;
}
diff --git a/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
index 92e16ce..5b62f96 100644
--- a/java/com/google/gerrit/httpd/CacheBasedWebSession.java
+++ b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
@@ -19,6 +19,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.httpd.restapi.ParameterParser;
@@ -40,7 +41,9 @@
public abstract class CacheBasedWebSession extends WebSession {
@VisibleForTesting public static final String ACCOUNT_COOKIE = "GerritAccount";
- protected static final long MAX_AGE_MINUTES = HOURS.toMinutes(12);
+
+ @UsedAt(UsedAt.Project.PLUGIN_WEBSESSION_FLATFILE)
+ public static final long MAX_AGE_MINUTES = HOURS.toMinutes(12);
private final HttpServletRequest request;
private final HttpServletResponse response;
diff --git a/java/com/google/gerrit/server/change/BatchAbandon.java b/java/com/google/gerrit/server/change/BatchAbandon.java
index ad6d7fb..2efa027 100644
--- a/java/com/google/gerrit/server/change/BatchAbandon.java
+++ b/java/com/google/gerrit/server/change/BatchAbandon.java
@@ -80,7 +80,7 @@
u.addOp(change.getId(), abandonOpFactory.create(accountState, msgTxt));
u.addOp(
change.getId(),
- storeSubmitRequirementsOpFactory.create(change.submitRequirements().values()));
+ storeSubmitRequirementsOpFactory.create(change.submitRequirements().values(), change));
}
u.execute();
diff --git a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
index 7793b76..96c863e 100644
--- a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
+++ b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
@@ -47,11 +47,11 @@
result.overrideExpressionResult().get(),
/* hide= */ false);
}
- if (result.submittabilityExpressionResult() != null) {
+ if (result.submittabilityExpressionResult().isPresent()) {
info.submittabilityExpressionResult =
submitRequirementExpressionToInfo(
req.submittabilityExpression(),
- result.submittabilityExpressionResult(),
+ result.submittabilityExpressionResult().get(),
/* hide= */ false);
}
info.status = SubmitRequirementResultInfo.Status.valueOf(result.status().toString());
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 7d3ff12..7c0b85a 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -438,7 +438,9 @@
DynamicSet.setOf(binder(), SubmitRequirement.class);
DynamicSet.setOf(binder(), QuotaEnforcer.class);
DynamicSet.setOf(binder(), PerformanceLogger.class);
- DynamicSet.bind(binder(), PerformanceLogger.class).to(PerformanceMetrics.class);
+ if (cfg.getBoolean("tracing", "exportPerformanceMetrics", false)) {
+ DynamicSet.bind(binder(), PerformanceLogger.class).to(PerformanceMetrics.class);
+ }
DynamicSet.setOf(binder(), RequestListener.class);
DynamicSet.bind(binder(), RequestListener.class).to(TraceRequestListener.class);
DynamicSet.setOf(binder(), ChangeETagComputation.class);
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index 197183f..e7e0e8f 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -437,8 +437,8 @@
case TRIVIAL_REBASE:
return ": Patch Set " + priorPatchSetId.get() + " was rebased.";
case NO_CHANGE:
- return ": New patch set was added with same tree, parent"
- + (commit.getParentCount() != 1 ? "s" : "")
+ return ": New patch set was added with same tree, parent "
+ + (commit.getParentCount() != 1 ? "trees" : "tree")
+ ", and commit message as Patch Set "
+ priorPatchSetId.get()
+ ".";
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
index 8b3ab5a..37a38fe 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
@@ -35,6 +35,17 @@
import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
+/**
+ * Provides {@link Gson} to parse {@link ChangeRevisionNote}, attached to the change update.
+ *
+ * <p>Apart from the adapters for the custom JSON format, this class also registers adapters that
+ * support forward/backward compatibility when modifying {@link ChangeNotes} storage format.
+ *
+ * <p>NOTE: All changes to the storage format must be both forward and backward compatible, see
+ * comment on {@link ChangeNotesParser}.
+ *
+ * <p>For JSON, such changes include e.g. modifications to the serialized {@code AutoValue} classes.
+ */
@Singleton
public class ChangeNoteJson {
private final Gson gson = newGson();
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 0d59542..1d8ec82 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -110,6 +110,31 @@
import org.eclipse.jgit.revwalk.FooterKey;
import org.eclipse.jgit.util.RawParseUtils;
+/**
+ * Parses {@link ChangeNotesState} out of the change meta ref.
+ *
+ * <p>NOTE: all changes to the change notes storage format must be both forward and backward
+ * compatible, i.e.:
+ *
+ * <ul>
+ * <li>The server, running the new binary version must be able to parse the data, written by the
+ * previous binary version.
+ * <li>The server, running the old binary version must be able to parse the data, written by the
+ * new binary version.
+ * </ul>
+ *
+ * <p>Thus, when introducing storage format update, the following procedure must be used:
+ *
+ * <ol>
+ * <li>The read path ({@link ChangeNotesParser}) needs to be updated to handle both the old and
+ * the new data format.
+ * <li>In a separate change, the write path (e.g. {@link ChangeUpdate}, {@link ChangeNoteJson}) is
+ * updated to write the new format, guarded by {@link
+ * com.google.gerrit.server.experiments.ExperimentFeatures} flag, if possible.
+ * <li>Once the 'read' change is roll out and is roll back safe, the 'write' change can be
+ * submitted/the experiment flag can be flipped.
+ * </ol>
+ */
class ChangeNotesParser {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 3d7ae10..a4623d6 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -118,6 +118,13 @@
* there is a single author and timestamp for each update.
*
* <p>This class is not thread-safe.
+ *
+ * <p>NOTE: This class also serializes the change in a custom storage format, used in NoteDB. All
+ * changes to the storage format must be both forward and backward compatible, see comment on {@link
+ * ChangeNotesParser}.
+ *
+ * <p>Such changes include e.g. introducing/removing footers, modifying footer formats, mutations of
+ * the attached {@link ChangeRevisionNote}.
*/
public class ChangeUpdate extends AbstractChangeUpdate {
public interface Factory {
diff --git a/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
index 6d5f4be..8957407 100644
--- a/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
+++ b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
@@ -1,4 +1,4 @@
-// Copyright (C) 2021 The Android Open Source Project
+// 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.
@@ -17,6 +17,8 @@
import com.google.gerrit.entities.SubmitRequirementResult;
import com.google.gerrit.server.experiments.ExperimentFeatures;
import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
+import com.google.gerrit.server.project.OnStoreSubmitRequirementResultModifier;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.inject.Inject;
@@ -29,16 +31,32 @@
public class StoreSubmitRequirementsOp implements BatchUpdateOp {
private final boolean storeRequirementsInNoteDb;
private final Collection<SubmitRequirementResult> submitRequirementResults;
+ private final ChangeData changeData;
+ private final OnStoreSubmitRequirementResultModifier onStoreSubmitRequirementResultModifier;
public interface Factory {
- StoreSubmitRequirementsOp create(Collection<SubmitRequirementResult> submitRequirements);
+
+ /**
+ * {@code submitRequirements} are explicitly passed to the operation so that they are evaluated
+ * before the {@link #updateChange} is called.
+ *
+ * <p>This is because the return results of {@link ChangeData#submitRequirements()} depend on
+ * the status of the change, which can be modified by other {@link BatchUpdateOp}, sharing the
+ * same {@link ChangeContext}.
+ */
+ StoreSubmitRequirementsOp create(
+ Collection<SubmitRequirementResult> submitRequirements, ChangeData changeData);
}
@Inject
public StoreSubmitRequirementsOp(
ExperimentFeatures experimentFeatures,
- @Assisted Collection<SubmitRequirementResult> submitRequirementResults) {
+ OnStoreSubmitRequirementResultModifier onStoreSubmitRequirementResultModifier,
+ @Assisted Collection<SubmitRequirementResult> submitRequirementResults,
+ @Assisted ChangeData changeData) {
+ this.onStoreSubmitRequirementResultModifier = onStoreSubmitRequirementResultModifier;
this.submitRequirementResults = submitRequirementResults;
+ this.changeData = changeData;
this.storeRequirementsInNoteDb =
experimentFeatures.isFeatureEnabled(
ExperimentFeaturesConstants
@@ -59,8 +77,13 @@
// from NoteDb and convert them to submit requirement results. See
// ChangeData#submitRequirements().
.filter(srResult -> !srResult.isLegacy())
+ .map(
+ // Pass to OnStoreSubmitRequirementResultModifier for override
+ srResult ->
+ onStoreSubmitRequirementResultModifier.modifyResultOnStore(
+ srResult.submitRequirement(), srResult, changeData, ctx))
.collect(Collectors.toList());
update.putSubmitRequirementResults(nonLegacySubmitRequirements);
- return !submitRequirementResults.isEmpty();
+ return !nonLegacySubmitRequirements.isEmpty();
}
}
diff --git a/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java b/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
index e655d92..a815f57 100644
--- a/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
+++ b/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
@@ -58,10 +58,10 @@
SubmitRequirementExpressionResultSerializer.serialize(
r.applicabilityExpressionResult().get()));
}
- if (r.submittabilityExpressionResult() != null) {
+ if (r.submittabilityExpressionResult().isPresent()) {
builder.setSubmittabilityExpressionResult(
SubmitRequirementExpressionResultSerializer.serialize(
- r.submittabilityExpressionResult()));
+ r.submittabilityExpressionResult().get()));
}
if (r.overrideExpressionResult().isPresent()) {
builder.setOverrideExpressionResult(
diff --git a/java/com/google/gerrit/server/project/CreateRefControl.java b/java/com/google/gerrit/server/project/CreateRefControl.java
index 69ac93e..2e76949 100644
--- a/java/com/google/gerrit/server/project/CreateRefControl.java
+++ b/java/com/google/gerrit/server/project/CreateRefControl.java
@@ -105,7 +105,7 @@
// If the tag has a PGP signature, allow a lower level of permission
// than if it doesn't have a PGP signature.
PermissionBackend.ForRef forRef = permissionBackend.user(user.get()).ref(branch);
- if (tag.getFullMessage().contains("-----BEGIN PGP SIGNATURE-----\n")) {
+ if (tag.getRawGpgSignature() != null) {
forRef.check(RefPermission.CREATE_SIGNED_TAG);
} else {
forRef.check(RefPermission.CREATE_TAG);
diff --git a/java/com/google/gerrit/server/project/OnStoreSubmitRequirementResultModifier.java b/java/com/google/gerrit/server/project/OnStoreSubmitRequirementResultModifier.java
new file mode 100644
index 0000000..7e6d93e
--- /dev/null
+++ b/java/com/google/gerrit/server/project/OnStoreSubmitRequirementResultModifier.java
@@ -0,0 +1,53 @@
+// 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.
+
+package com.google.gerrit.server.project;
+
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.update.ChangeContext;
+import com.google.inject.ImplementedBy;
+
+/**
+ * Extension point that allows to modify the {@link SubmitRequirementResult} when it is stored
+ * NoteDB.
+ *
+ * <p>The submit requirements are only stored for the closed (merged and abandoned) changes and the
+ * modifier only affects what {@link SubmitRequirementResult} will be stored in NoteDB.
+ *
+ * <p>It has no impact on open changes or evaluations on merge, i.e. does not affect the
+ * submittability of the change (never blocks the ready change from submission or allows bypassing
+ * unsatisfied submit requirement).
+ *
+ * <p>The extension point only applies to non-legacy submit requirements (including non-applicable,
+ * since they are stored too) and does not affect submit rule results.
+ */
+@ExtensionPoint
+@ImplementedBy(OnStoreSubmitRequirementResultModifierImpl.class)
+public interface OnStoreSubmitRequirementResultModifier {
+
+ /**
+ * Evaluate a single {@link SubmitRequirement} using the change data, modifying the original
+ * {@code SubmitRequirementResult}, if needed.
+ *
+ * <p>Only affects how the submit requirement is stored in NoteDb for closed changes.
+ */
+ SubmitRequirementResult modifyResultOnStore(
+ SubmitRequirement submitRequirement,
+ SubmitRequirementResult submitRequirementResult,
+ ChangeData changeData,
+ ChangeContext ctx);
+}
diff --git a/java/com/google/gerrit/server/project/OnStoreSubmitRequirementResultModifierImpl.java b/java/com/google/gerrit/server/project/OnStoreSubmitRequirementResultModifierImpl.java
new file mode 100644
index 0000000..39a717f
--- /dev/null
+++ b/java/com/google/gerrit/server/project/OnStoreSubmitRequirementResultModifierImpl.java
@@ -0,0 +1,35 @@
+// 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.
+package com.google.gerrit.server.project;
+
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.update.ChangeContext;
+
+/**
+ * Default implementation of {@link OnStoreSubmitRequirementResultModifier} that does not
+ * re-evaluate {@link SubmitRequirementResult}.
+ */
+public class OnStoreSubmitRequirementResultModifierImpl
+ implements OnStoreSubmitRequirementResultModifier {
+ @Override
+ public SubmitRequirementResult modifyResultOnStore(
+ SubmitRequirement submitRequirement,
+ SubmitRequirementResult result,
+ ChangeData cd,
+ ChangeContext ctx) {
+ return result;
+ }
+}
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 00f6876..3887604a 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -100,24 +100,27 @@
// Use a request context to execute predicates as an internal user with expanded visibility.
// This is so that the evaluation does not depend on who is running the current request (e.g.
// a "ownerin" predicate with group that is not visible to the person making this request).
- SubmitRequirementExpressionResult blockingResult =
- evaluateExpression(sr.submittabilityExpression(), cd);
Optional<SubmitRequirementExpressionResult> applicabilityResult =
sr.applicabilityExpression().isPresent()
? Optional.of(evaluateExpression(sr.applicabilityExpression().get(), cd))
: Optional.empty();
-
- Optional<SubmitRequirementExpressionResult> overrideResult =
- sr.overrideExpression().isPresent()
- ? Optional.of(evaluateExpression(sr.overrideExpression().get(), cd))
- : Optional.empty();
+ Optional<SubmitRequirementExpressionResult> submittabilityResult = Optional.empty();
+ Optional<SubmitRequirementExpressionResult> overrideResult = Optional.empty();
+ if (!sr.applicabilityExpression().isPresent()
+ || SubmitRequirementResult.assertPass(applicabilityResult)) {
+ submittabilityResult = Optional.of(evaluateExpression(sr.submittabilityExpression(), cd));
+ overrideResult =
+ sr.overrideExpression().isPresent()
+ ? Optional.of(evaluateExpression(sr.overrideExpression().get(), cd))
+ : Optional.empty();
+ }
return SubmitRequirementResult.builder()
.legacy(Optional.of(false))
.submitRequirement(sr)
.patchSetCommitId(cd.currentPatchSet().commitId())
- .submittabilityExpressionResult(blockingResult)
+ .submittabilityExpressionResult(submittabilityResult)
.applicabilityExpressionResult(applicabilityResult)
.overrideExpressionResult(overrideResult)
.build();
diff --git a/java/com/google/gerrit/server/restapi/change/Abandon.java b/java/com/google/gerrit/server/restapi/change/Abandon.java
index 98514b2..8dd0e78 100644
--- a/java/com/google/gerrit/server/restapi/change/Abandon.java
+++ b/java/com/google/gerrit/server/restapi/change/Abandon.java
@@ -131,7 +131,8 @@
u.addOp(notes.getChangeId(), op);
u.addOp(
notes.getChangeId(),
- storeSubmitRequirementsOpFactory.create(changeData.submitRequirements().values()));
+ storeSubmitRequirementsOpFactory.create(
+ changeData.submitRequirements().values(), changeData));
u.execute();
}
return op.getChange();
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index 0160fc9..a243228 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -647,7 +647,7 @@
.get(project)
.addOp(
changeId,
- storeSubmitRequirementsOpFactory.create(cd.submitRequirements().values()));
+ storeSubmitRequirementsOpFactory.create(cd.submitRequirements().values(), cd));
}
try {
submissionExecutor.setAdditionalBatchUpdateListeners(
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
index cb1b7b1..ff76546 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
@@ -22,6 +22,7 @@
import static com.google.gerrit.server.project.testing.TestLabels.label;
import static com.google.gerrit.server.project.testing.TestLabels.value;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -1081,9 +1082,9 @@
SubmitRequirementResult result =
notes.getSubmitRequirementsResult().stream().collect(MoreCollectors.onlyElement());
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
- assertThat(result.submittabilityExpressionResult().status())
+ assertThat(result.submittabilityExpressionResult().get().status())
.isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- assertThat(result.submittabilityExpressionResult().expression().expressionString())
+ assertThat(result.submittabilityExpressionResult().get().expression().expressionString())
.isEqualTo("label:Code-Review=MAX");
}
}
@@ -1121,9 +1122,9 @@
SubmitRequirementResult result =
notes.getSubmitRequirementsResult().stream().collect(MoreCollectors.onlyElement());
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
- assertThat(result.submittabilityExpressionResult().status())
+ assertThat(result.submittabilityExpressionResult().get().status())
.isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- assertThat(result.submittabilityExpressionResult().expression().expressionString())
+ assertThat(result.submittabilityExpressionResult().get().expression().expressionString())
.isEqualTo("label:Code-Review=MAX");
}
}
@@ -1815,6 +1816,120 @@
/* passingAtoms= */ null,
/* failingAtoms= */ null,
/* fulfilled= */ true);
+ assertThat(requirement.submittabilityExpressionResult).isNotNull();
+ }
+
+ @Test
+ public void submitRequirement_nonApplicable_submittabilityAndOverrideNotEvaluated()
+ throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setApplicabilityExpression(
+ SubmitRequirementExpression.of("branch:refs/heads/non-existent"))
+ .setSubmittabilityExpression(SubmitRequirementExpression.maxCodeReview())
+ .setOverrideExpression(SubmitRequirementExpression.of("project:" + project.get()))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "Code-Review", 2);
+
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements, "Code-Review", Status.NOT_APPLICABLE, /* isLegacy= */ false);
+ SubmitRequirementResultInfo requirement =
+ changeInfo.submitRequirements.stream().collect(MoreCollectors.onlyElement());
+ assertSubmitRequirementExpression(
+ requirement.applicabilityExpressionResult,
+ /* expression= */ null,
+ /* passingAtoms= */ null,
+ /* failingAtoms= */ null,
+ /* fulfilled= */ false);
+ assertThat(requirement.submittabilityExpressionResult).isNull();
+ assertThat(requirement.overrideExpressionResult).isNull();
+ }
+
+ @Test
+ public void submitRequirement_emptyApplicable_submittabilityAndOverrideEvaluated()
+ throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setApplicabilityExpression(Optional.empty())
+ .setSubmittabilityExpression(SubmitRequirementExpression.maxCodeReview())
+ .setOverrideExpression(SubmitRequirementExpression.of("project:non-existent"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "Code-Review", 2);
+
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
+ SubmitRequirementResultInfo requirement =
+ changeInfo.submitRequirements.stream().collect(MoreCollectors.onlyElement());
+ assertThat(requirement.applicabilityExpressionResult).isNull();
+ assertSubmitRequirementExpression(
+ requirement.submittabilityExpressionResult,
+ /* expression= */ SubmitRequirementExpression.maxCodeReview().expressionString(),
+ /* passingAtoms= */ ImmutableList.of(
+ SubmitRequirementExpression.maxCodeReview().expressionString()),
+ /* failingAtoms= */ ImmutableList.of(),
+ /* fulfilled= */ true);
+ assertSubmitRequirementExpression(
+ requirement.overrideExpressionResult,
+ /* expression= */ "project:non-existent",
+ /* passingAtoms= */ ImmutableList.of(),
+ /* failingAtoms= */ ImmutableList.of("project:non-existent"),
+ /* fulfilled= */ false);
+ }
+
+ @Test
+ public void submitRequirement_overriden_submittabilityEvaluated() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setApplicabilityExpression(Optional.empty())
+ .setSubmittabilityExpression(SubmitRequirementExpression.maxCodeReview())
+ .setOverrideExpression(SubmitRequirementExpression.of("project:" + project.get()))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "Code-Review", 1);
+
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements, "Code-Review", Status.OVERRIDDEN, /* isLegacy= */ false);
+ SubmitRequirementResultInfo requirement =
+ changeInfo.submitRequirements.stream()
+ .filter(sr -> !sr.isLegacy)
+ .collect(MoreCollectors.onlyElement());
+ assertThat(requirement.applicabilityExpressionResult).isNull();
+ assertSubmitRequirementExpression(
+ requirement.submittabilityExpressionResult,
+ /* expression= */ SubmitRequirementExpression.maxCodeReview().expressionString(),
+ /* passingAtoms= */ ImmutableList.of(),
+ /* failingAtoms= */ ImmutableList.of(
+ SubmitRequirementExpression.maxCodeReview().expressionString()),
+ /* fulfilled= */ false);
+ assertSubmitRequirementExpression(
+ requirement.overrideExpressionResult,
+ /* expression= */ "project:" + project.get(),
+ /* passingAtoms= */ ImmutableList.of("project:" + project.get()),
+ /* failingAtoms= */ ImmutableList.of(),
+ /* fulfilled= */ true);
}
@Test
@@ -2132,9 +2247,9 @@
SubmitRequirementResult result =
notes.getSubmitRequirementsResult().stream().collect(MoreCollectors.onlyElement());
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
- assertThat(result.submittabilityExpressionResult().status())
+ assertThat(result.submittabilityExpressionResult().get().status())
.isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- assertThat(result.submittabilityExpressionResult().expression().expressionString())
+ assertThat(result.submittabilityExpressionResult().get().expression().expressionString())
.isEqualTo("topic:test");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/server/project/OnStoreSubmitRequirementResultModifierIT.java b/javatests/com/google/gerrit/acceptance/server/project/OnStoreSubmitRequirementResultModifierIT.java
new file mode 100644
index 0000000..ae70832
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/project/OnStoreSubmitRequirementResultModifierIT.java
@@ -0,0 +1,258 @@
+// 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.
+
+package com.google.gerrit.acceptance.server.project;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.TestOnStoreSubmitRequirementResultModifier;
+import com.google.gerrit.acceptance.TestOnStoreSubmitRequirementResultModifier.ModificationStrategy;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementResultInfo.Status;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.server.change.TestSubmitInput;
+import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
+import com.google.gerrit.server.project.OnStoreSubmitRequirementResultModifier;
+import com.google.inject.AbstractModule;
+import com.google.inject.Module;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Tests for {@link OnStoreSubmitRequirementResultModifier} on the closed changes. */
+@NoHttpd
+public class OnStoreSubmitRequirementResultModifierIT extends AbstractDaemonTest {
+
+ private TestOnStoreSubmitRequirementResultModifier testOnStoreSrModifier;
+
+ @Override
+ public Module createModule() {
+ testOnStoreSrModifier = new TestOnStoreSubmitRequirementResultModifier();
+ return new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(OnStoreSubmitRequirementResultModifier.class).toInstance(testOnStoreSrModifier);
+ }
+ };
+ }
+
+ @Before
+ public void setUp() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.maxCodeReview())
+ .setAllowOverrideInChildProjects(false)
+ .build());
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.disabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_STORE_SUBMIT_REQUIREMENTS_ON_MERGE)
+ public void submitRequirementsNotStored_overrideNoOp() throws Exception {
+
+ testOnStoreSrModifier.setModificationStrategy(ModificationStrategy.OVERRIDE);
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ approve(changeId);
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
+
+ gApi.changes().id(changeId).current().submit();
+
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ true);
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_STORE_SUBMIT_REQUIREMENTS_ON_MERGE)
+ public void submitRequirementStored_canBeOverriddenForMergedChanges() throws Exception {
+ testOnStoreSrModifier.setModificationStrategy(ModificationStrategy.OVERRIDE);
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ approve(changeId);
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
+
+ gApi.changes().id(changeId).current().submit();
+
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.OVERRIDDEN, /* isLegacy= */ false);
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_STORE_SUBMIT_REQUIREMENTS_ON_MERGE)
+ public void submitRequirementStored_canBeOverriddenForAbandonedChanges() throws Exception {
+ testOnStoreSrModifier.setModificationStrategy(ModificationStrategy.OVERRIDE);
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ approve(changeId);
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
+
+ gApi.changes().id(changeId).abandon();
+
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.OVERRIDDEN, /* isLegacy= */ false);
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_STORE_SUBMIT_REQUIREMENTS_ON_MERGE)
+ public void overrideToUnsatisfied_unsatisfied_doesNotBlockSubmission() throws Exception {
+ testOnStoreSrModifier.setModificationStrategy(ModificationStrategy.FAIL);
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ approve(changeId);
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
+
+ gApi.changes().id(changeId).current().submit();
+
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(2);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ true);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_STORE_SUBMIT_REQUIREMENTS_ON_MERGE)
+ public void overrideToUnsatisfied_doesNotBlockSubmissionWithRetries() throws Exception {
+ testOnStoreSrModifier.setModificationStrategy(ModificationStrategy.FAIL);
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ approve(changeId);
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
+
+ TestSubmitInput input = new TestSubmitInput();
+ input.generateLockFailures = new ArrayDeque<>(ImmutableList.of(true));
+ gApi.changes().id(changeId).current().submit(input);
+
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(2);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ true);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_STORE_SUBMIT_REQUIREMENTS_ON_MERGE)
+ public void overrideToSatisfied_doesNotBypassSubmitRequirement() throws Exception {
+ testOnStoreSrModifier.setModificationStrategy(ModificationStrategy.PASS);
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
+ assertThrows(
+ ResourceConflictException.class, () -> gApi.changes().id(changeId).current().submit());
+ }
+
+ private void assertSubmitRequirementStatus(
+ Collection<SubmitRequirementResultInfo> results,
+ String requirementName,
+ SubmitRequirementResultInfo.Status status,
+ boolean isLegacy) {
+ assertWithMessage(
+ "Could not find submit requirement %s with status %s, legacy=%s, (results = %s)",
+ requirementName,
+ status,
+ isLegacy,
+ results.stream()
+ .map(r -> String.format("%s=%s, legacy=%s", r.name, r.status, r.isLegacy))
+ .collect(toImmutableList()))
+ .that(
+ results.stream()
+ .filter(
+ result ->
+ result.name.equals(requirementName)
+ && result.status == status
+ && result.isLegacy == isLegacy)
+ .count())
+ .isEqualTo(1);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
index 61e204f..43c7b6b 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
@@ -238,6 +238,68 @@
}
@Test
+ public void submittabilityAndOverrideNotEvaluated_whenApplicabilityIsFalse() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:non-existent-project",
+ /* submittabilityExpr= */ "message:\"Fix bug\"",
+ /* overrideExpr= */ "");
+
+ SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.NOT_APPLICABLE);
+ assertThat(result.applicabilityExpressionResult().get().status()).isEqualTo(Status.FAIL);
+ assertThat(result.submittabilityExpressionResult().isPresent()).isFalse();
+ assertThat(result.overrideExpressionResult().isPresent()).isFalse();
+ }
+
+ @Test
+ public void submittabilityAndOverrideEvaluated_whenApplicabilityIsEmpty() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ null,
+ /* submittabilityExpr= */ "message:\"Fix bug\"",
+ /* overrideExpr= */ "label:\"build-cop-override=-1\"");
+
+ SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
+ assertThat(result.applicabilityExpressionResult().isPresent()).isFalse();
+ assertThat(result.submittabilityExpressionResult().get().status()).isEqualTo(Status.PASS);
+ assertThat(result.overrideExpressionResult().get().status()).isEqualTo(Status.FAIL);
+ }
+
+ @Test
+ public void submittabilityAndOverrideEvaluated_whenApplicabilityIsTrue() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* submittabilityExpr= */ "message:\"Fix bug\"",
+ /* overrideExpr= */ "label:\"build-cop-override=-1\"");
+
+ SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
+
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
+ assertThat(result.applicabilityExpressionResult().get().status()).isEqualTo(Status.PASS);
+ assertThat(result.submittabilityExpressionResult().get().status()).isEqualTo(Status.PASS);
+ assertThat(result.overrideExpressionResult().get().status()).isEqualTo(Status.FAIL);
+ }
+
+ @Test
+ public void submittabilityIsEvaluated_whenOverrideApplies() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ null,
+ /* submittabilityExpr= */ "message:\"Fix bug\"",
+ /* overrideExpr= */ "project:" + project.get());
+
+ SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.OVERRIDDEN);
+
+ assertThat(result.applicabilityExpressionResult().isPresent()).isFalse();
+ assertThat(result.submittabilityExpressionResult().get().status()).isEqualTo(Status.PASS);
+ assertThat(result.overrideExpressionResult().get().status()).isEqualTo(Status.PASS);
+ }
+
+ @Test
public void submitRequirementIsSatisfied_whenSubmittabilityExpressionIsTrue() throws Exception {
SubmitRequirement sr =
createSubmitRequirement(
@@ -260,7 +322,7 @@
SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.UNSATISFIED);
- assertThat(result.submittabilityExpressionResult().failingAtoms())
+ assertThat(result.submittabilityExpressionResult().get().failingAtoms())
.containsExactly("label:\"Code-Review=+2\"");
}
@@ -314,7 +376,7 @@
SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.ERROR);
- assertThat(result.submittabilityExpressionResult().errorMessage().get())
+ assertThat(result.submittabilityExpressionResult().get().errorMessage().get())
.isEqualTo("Unsupported operator invalid_field:invalid_value");
}
diff --git a/javatests/com/google/gerrit/common/data/FilenameComparatorTest.java b/javatests/com/google/gerrit/common/data/FilenameComparatorTest.java
index ec71e05..d959c2a 100644
--- a/javatests/com/google/gerrit/common/data/FilenameComparatorTest.java
+++ b/javatests/com/google/gerrit/common/data/FilenameComparatorTest.java
@@ -45,6 +45,7 @@
@Test
public void cppExtensions() {
+ assertThat(comparator.compare("abc/file.hh", "abc/file.cc")).isLessThan(0);
assertThat(comparator.compare("abc/file.h", "abc/file.cc")).isLessThan(0);
assertThat(comparator.compare("abc/file.c", "abc/file.hpp")).isGreaterThan(0);
assertThat(comparator.compare("abc..xyz.file.h", "abc.xyz.file.cc")).isLessThan(0);
diff --git a/javatests/com/google/gerrit/git/BUILD b/javatests/com/google/gerrit/git/BUILD
index b12924a..c2c9cce 100644
--- a/javatests/com/google/gerrit/git/BUILD
+++ b/javatests/com/google/gerrit/git/BUILD
@@ -7,10 +7,7 @@
size = "medium",
timeout = "short",
srcs = MEDIUM_TESTS,
- tags = [
- "no_rbe",
- "no_windows",
- ],
+ tags = ["no_windows"],
deps = [
"//java/com/google/gerrit/git",
"//lib:guava",
diff --git a/javatests/com/google/gerrit/integration/git/BUILD b/javatests/com/google/gerrit/integration/git/BUILD
index bcb16f4..28755af 100644
--- a/javatests/com/google/gerrit/integration/git/BUILD
+++ b/javatests/com/google/gerrit/integration/git/BUILD
@@ -3,10 +3,7 @@
acceptance_tests(
srcs = ["GitProtocolV2IT.java"],
group = "protocol-v2",
- labels = [
- "git-protocol-v2",
- "no_rbe",
- ],
+ labels = ["git-protocol-v2"],
)
acceptance_tests(
diff --git a/javatests/com/google/gerrit/pgm/BUILD b/javatests/com/google/gerrit/pgm/BUILD
index 0655bb2..0fe4fad 100644
--- a/javatests/com/google/gerrit/pgm/BUILD
+++ b/javatests/com/google/gerrit/pgm/BUILD
@@ -4,7 +4,6 @@
junit_tests(
name = "pgm_tests",
srcs = glob(["**/*.java"]),
- tags = ["no_rbe"],
deps = [
"//java/com/google/gerrit/pgm/http/jetty",
"//java/com/google/gerrit/pgm/init/api",
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 87d1729..c694a87 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -27,10 +27,7 @@
),
resource_strip_prefix = "resources",
resources = ["//resources/com/google/gerrit/server"],
- tags = [
- "no_rbe",
- "no_windows",
- ],
+ tags = ["no_windows"],
visibility = ["//visibility:public"],
runtime_deps = [
"//java/com/google/gerrit/lucene",
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java
index 2418d1c..6abe4d1 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java
@@ -118,11 +118,11 @@
+ "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ "\"passingAtoms\":[\"refs/heads/master\"],"
+ "\"failingAtoms\":[]}},"
- + "\"submittabilityExpressionResult\":{"
+ + "\"submittabilityExpressionResult\":{\"value\":{"
+ "\"expression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
+ "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ "\"passingAtoms\":[\"label:\\\"Code-Review=+2\\\"\"],"
- + "\"failingAtoms\":[]},"
+ + "\"failingAtoms\":[]}},"
+ "\"overrideExpressionResult\":{\"value\":{"
+ "\"expression\":{\"expressionString\":\"label:Override=+1\"},"
+ "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
@@ -196,44 +196,29 @@
}
@Test
- public void submitRequirementResult_deserialize_nonOptionalSubmittabilityExpressionResultField()
+ public void submitRequirementResult_deserialize_optionalSubmittabilityExpressionResultField()
throws Exception {
assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(srReqResultSerial))
.isEqualTo(srReqResult);
}
@Test
- public void submitRequirementResult_deserialize_optionalSubmittabilityExpressionResultField()
+ public void submitRequirementResult_deserialize_nonOptionalSubmittabilityExpressionResultField()
throws Exception {
- String newFormatSrReqResultSerial =
+ String oldFormatSrReqResultSerial =
srReqResultSerial.replace(
- "\"submittabilityExpressionResult\":{"
- + "\"expression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
- + "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
- + "\"passingAtoms\":[\"label:\\\"Code-Review=+2\\\"\"],"
- + "\"failingAtoms\":[]},",
"\"submittabilityExpressionResult\":{\"value\":{"
+ "\"expression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
+ "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ "\"passingAtoms\":[\"label:\\\"Code-Review=+2\\\"\"],"
- + "\"failingAtoms\":[]}},");
- assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(newFormatSrReqResultSerial))
- .isEqualTo(srReqResult);
- }
-
- @Test
- public void submitRequirementResult_deserialize_emptyOptionalSubmittabilityExpressionResultField()
- throws Exception {
- String newFormatSrReqResultSerial =
- srReqResultSerial.replace(
+ + "\"failingAtoms\":[]}},",
"\"submittabilityExpressionResult\":{"
+ "\"expression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
+ "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ "\"passingAtoms\":[\"label:\\\"Code-Review=+2\\\"\"],"
- + "\"failingAtoms\":[]},",
- "\"submittabilityExpressionResult\":{\"value\":null},");
- assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(newFormatSrReqResultSerial))
- .isEqualTo(srReqResult.toBuilder().submittabilityExpressionResult(null).build());
+ + "\"failingAtoms\":[]},");
+ assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(oldFormatSrReqResultSerial))
+ .isEqualTo(srReqResult);
}
@Test
@@ -243,6 +228,20 @@
}
@Test
+ public void submitRequirementResult_emptySubmittabilityExpressionResultField_roundTrip()
+ throws Exception {
+ SubmitRequirementResult srResult =
+ srReqResult
+ .toBuilder()
+ .submittabilityExpressionResult(Optional.empty())
+ .applicabilityExpressionResult(Optional.empty())
+ .overrideExpressionResult(Optional.empty())
+ .build();
+ TypeAdapter<SubmitRequirementResult> adapter = SubmitRequirementResult.typeAdapter(gson);
+ assertThat(adapter.fromJson(adapter.toJson(srResult))).isEqualTo(srResult);
+ }
+
+ @Test
public void deserializeSubmitRequirementResult_withJGitPatchsetIdFormat() throws Exception {
String srResultSerialJgitFormat =
srReqResultSerial.replace(
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index e557277..c33a87f 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -32,6 +32,11 @@
import org.junit.Before;
import org.junit.Test;
+/**
+ * Tests for {@link ChangeNotesParser}.
+ *
+ * <p>When modifying storage format, please, add tests that both old and new data can be parsed.
+ */
public class ChangeNotesParserTest extends AbstractChangeNotesTest {
private TestRepository<InMemoryRepository> testRepo;
private ChangeNotesRevWalk walk;
diff --git a/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java b/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java
index 034e7ef..22207cc 100644
--- a/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java
+++ b/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java
@@ -371,7 +371,7 @@
assertThat(r.submitRequirement().submittabilityExpression().expressionString())
.isEqualTo(submitExpression);
assertThat(r.status()).isEqualTo(status);
- assertThat(r.submittabilityExpressionResult().status()).isEqualTo(expressionStatus);
+ assertThat(r.submittabilityExpressionResult().get().status()).isEqualTo(expressionStatus);
}
private SubmitRecord createSubmitRecord(
diff --git a/lib/nongoogle_test.sh b/lib/nongoogle_test.sh
index 957c33d..9e9d750 100755
--- a/lib/nongoogle_test.sh
+++ b/lib/nongoogle_test.sh
@@ -28,6 +28,7 @@
j2objc
jimfs
jruby
+log4j
lucene-analyzers-common
lucene-core
lucene-misc
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index d66c18e..c05fef4 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -22,6 +22,7 @@
"styles",
"types",
"utils",
+ "workers",
]
ts_config(
@@ -285,5 +286,6 @@
"--rules.no-property-visibility-mismatch off",
"--rules.no-incompatible-property-type off",
"--rules.no-incompatible-type-binding off",
+ "--rules.no-unknown-attribute error",
],
)
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirement/gr-change-list-column-requirement_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirement/gr-change-list-column-requirement_test.ts
index 1bf4d88..c2820e2 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirement/gr-change-list-column-requirement_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirement/gr-change-list-column-requirement_test.ts
@@ -118,8 +118,12 @@
<gr-vote-chip></gr-vote-chip>
</div>`);
const voteChip = queryAndAssert(element, 'gr-vote-chip');
- expect(voteChip).shadowDom.to.equal(`<span class="container">
+ expect(voteChip).shadowDom.to.equal(`<gr-tooltip-content
+ class="container"
+ has-tooltip=""
+ title="bad"
+ >
<div class="negative vote-chip">-1</div>
- </span>`);
+ </gr-tooltip-content>`);
});
});
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index f8b1a2b..11724096 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -73,7 +73,7 @@
export interface ChangeListSection {
countLabel?: string;
- isOutgoing?: boolean;
+ emptyStateSlotName?: string;
name?: string;
query?: string;
results: ChangeInfo[];
@@ -112,7 +112,7 @@
@state() private dynamicHeaderEndpoints?: string[];
- @property({type: Number})
+ @property({type: Number, attribute: 'selected-index'})
selectedIndex?: number;
@property({type: Boolean})
@@ -306,10 +306,8 @@
class="cell"
colspan="${this.computeColspan(changeSection, labelNames)}"
>
- ${this.getSpecialEmptySlot(changeSection)
- ? html`<slot
- name="${this.getSpecialEmptySlot(changeSection)}"
- ></slot>`
+ ${changeSection.emptyStateSlotName
+ ? html`<slot name="${changeSection.emptyStateSlotName}"></slot>`
: 'No changes'}
</td>
</tr>
@@ -672,15 +670,7 @@
if (this.selectedIndex) this.cursor.setCursorAtIndex(this.selectedIndex);
}
- // private but used in test
- getSpecialEmptySlot(section: ChangeListSection) {
- if (section.isOutgoing) return 'empty-outgoing';
- if (section.name === YOUR_TURN.name) return 'empty-your-turn';
- return '';
- }
-
- // private but used in test
- isEmpty(section: ChangeListSection) {
+ private isEmpty(section: ChangeListSection) {
return !section.results?.length;
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
index 29204b6..88b1357 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
@@ -25,7 +25,6 @@
queryAndAssert,
stubFlags,
} from '../../../test/test-utils';
-import {YOUR_TURN} from '../../core/gr-navigation/gr-navigation';
import {Key} from '../../../utils/dom-util';
import {TimeFormat} from '../../../constants/constants';
import {AccountId, NumericChangeId} from '../../../types/common';
@@ -44,6 +43,79 @@
element = basicFixture.instantiate();
});
+ test('renders', async () => {
+ element.preferences = {
+ legacycid_in_change_table: true,
+ time_format: TimeFormat.HHMM_12,
+ change_table: [],
+ };
+ element.account = {_account_id: 1001 as AccountId};
+ element.config = createServerInfo();
+ element.sections = [{results: new Array(1)}, {results: new Array(2)}];
+ element.selectedIndex = 0;
+ element.changes = [
+ {...createChange(), _number: 0 as NumericChangeId},
+ {...createChange(), _number: 1 as NumericChangeId},
+ {...createChange(), _number: 2 as NumericChangeId},
+ ];
+ await element.updateComplete;
+ expect(element).shadowDom.to.equal(`
+ <gr-change-list-item
+ aria-label="Test subject"
+ selected=""
+ >
+ </gr-change-list-item>
+ <gr-change-list-item aria-label="Test subject">
+ </gr-change-list-item>
+ <gr-change-list-item aria-label="Test subject">
+ </gr-change-list-item>
+ <table id="changeList">
+ <tbody class="groupContent">
+ <tr class="groupTitle">
+ <td
+ aria-hidden="true"
+ class="leftPadding"
+ >
+ </td>
+ <td
+ aria-label="Star status column"
+ class="star"
+ hidden=""
+ >
+ </td>
+ <td class="number">
+ #
+ </td>
+ <td class="subject">
+ Subject
+ </td>
+ <td class="status">
+ Status
+ </td>
+ <td class="owner">
+ Owner
+ </td>
+ <td class="reviewers">
+ Reviewers
+ </td>
+ <td class="repo">
+ Repo
+ </td>
+ <td class="branch">
+ Branch
+ </td>
+ <td class="updated">
+ Updated
+ </td>
+ <td class="size">
+ Size
+ </td>
+ </tr>
+ </tbody>
+ </table>
+ `);
+ });
+
suite('test show change number not logged in', () => {
setup(async () => {
element = basicFixture.instantiate();
@@ -251,35 +323,35 @@
assert.equal(noChangesMsg.length, 2);
});
- suite('empty section', () => {
- test('not shown on empty non-outgoing sections', () => {
- const section = {name: 'test', query: 'test', results: []};
- assert.isTrue(element.isEmpty(section));
- assert.equal(element.getSpecialEmptySlot(section), '');
- });
-
- test('shown on empty outgoing sections', () => {
+ suite('empty section slots', () => {
+ test('are shown on empty sections with slot name', async () => {
const section = {
name: 'test',
query: 'test',
results: [],
- isOutgoing: true,
+ emptyStateSlotName: 'test',
};
- assert.isTrue(element.isEmpty(section));
- assert.equal(element.getSpecialEmptySlot(section), 'empty-outgoing');
+ element.sections = [section];
+ await element.updateComplete;
+
+ assert.isEmpty(queryAll(element, 'gr-change-list-item'));
+ queryAndAssert(element, 'slot[name="test"]');
});
- test('shown on empty outgoing sections', () => {
- const section = {name: YOUR_TURN.name, query: 'test', results: []};
- assert.isTrue(element.isEmpty(section));
- assert.equal(element.getSpecialEmptySlot(section), 'empty-your-turn');
+ test('are not shown on empty sections without slot name', async () => {
+ const section = {name: 'test', query: 'test', results: []};
+ element.sections = [section];
+ await element.updateComplete;
+
+ assert.isEmpty(queryAll(element, 'gr-change-list-item'));
+ assert.notExists(query(element, 'slot[name="test"]'));
});
- test('not shown on non-empty outgoing sections', () => {
+ test('are not shown on non-empty sections with slot name', async () => {
const section = {
name: 'test',
query: 'test',
- isOutgoing: true,
+ emptyStateSlotName: 'test',
results: [
{
...createChange(),
@@ -288,7 +360,11 @@
},
],
};
- assert.isFalse(element.isEmpty(section));
+ element.sections = [section];
+ await element.updateComplete;
+
+ assert.isNotEmpty(queryAll(element, 'gr-change-list-item'));
+ assert.notExists(query(element, 'slot[name="test"]'));
});
});
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
index 1b04268..f61518c 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
@@ -28,6 +28,7 @@
import {htmlTemplate} from './gr-dashboard-view_html';
import {
GerritNav,
+ OUTGOING,
UserDashboard,
YOUR_TURN,
} from '../../core/gr-navigation/gr-navigation';
@@ -56,6 +57,7 @@
import {firePageError, fireTitleChange} from '../../../utils/event-util';
import {GerritView} from '../../../services/router/router-model';
import {RELOAD_DASHBOARD_INTERVAL_MS} from '../../../constants/constants';
+import {ChangeListSection} from '../gr-change-list/gr-change-list';
const PROJECT_PLACEHOLDER_PATTERN = /\${project}/g;
@@ -68,13 +70,10 @@
};
}
-interface DashboardChange {
- name: string;
- countLabel: string;
- query: string;
- results: ChangeInfo[];
- isOutgoing?: boolean;
-}
+const slotNameBySectionName = new Map<string, string>([
+ [YOUR_TURN.name, 'your-turn-slot'],
+ [OUTGOING.name, 'outgoing-slot'],
+]);
@customElement('gr-dashboard-view')
export class GrDashboardView extends PolymerElement {
@@ -101,7 +100,7 @@
params?: AppElementParams;
@property({type: Array})
- _results?: DashboardChange[];
+ _results?: ChangeListSection[];
@property({type: Boolean})
_loading = true;
@@ -301,7 +300,7 @@
countLabel: this._computeSectionCountLabel(results),
query: res.sections[i].query,
results: this._maybeSortResults(res.sections[i].name, results),
- isOutgoing: res.sections[i].isOutgoing,
+ emptyStateSlotName: slotNameBySectionName.get(res.sections[i].name),
};
})
.filter(
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_html.ts b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_html.ts
index b4fa7cc..95829ec 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_html.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_html.ts
@@ -78,12 +78,12 @@
showstar=""
account="[[account]]"
preferences="[[preferences]]"
- selectedindex="[[_selectedChangeIndex]]"
+ selected-index="[[_selectedChangeIndex]]"
sections="[[_results]]"
on-selected-index-changed="_handleSelectedIndexChanged"
on-toggle-star="_handleToggleStar"
>
- <div id="emptyOutgoing" slot="empty-outgoing">
+ <div id="emptyOutgoing" slot="outgoing-slot">
<template is="dom-if" if="[[_showNewUserHelp]]">
<gr-create-change-help
on-create-tap="_handleCreateChangeTap"
@@ -91,7 +91,7 @@
</template>
<template is="dom-if" if="[[!_showNewUserHelp]]"> No changes </template>
</div>
- <div id="emptyYourTurn" slot="empty-your-turn">
+ <div id="emptyYourTurn" slot="your-turn-slot">
<span>No changes need your attention 🎉</span>
</div>
</gr-change-list>
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.js b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.js
index aa76347..82fffff 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.js
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.js
@@ -315,9 +315,9 @@
});
});
- test('preserve isOutgoing sections', () => {
+ test('sets slot name to section name if custom state is requested', () => {
const sections = [
- {name: 'test1', query: 'test1', isOutgoing: true},
+ {name: 'Outgoing reviews', query: 'test1'},
{name: 'test2', query: 'test2'},
];
getChangesStub.restore();
@@ -326,8 +326,8 @@
return element._fetchDashboardChanges({sections}, false).then(() => {
assert.equal(element._results.length, 2);
- assert.isTrue(element._results[0].isOutgoing);
- assert.isNotOk(element._results[1].isOutgoing);
+ assert.equal(element._results[0].emptyStateSlotName, 'outgoing-slot');
+ assert.isNotOk(element._results[1].emptyStateSlotName);
});
});
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 d52281c..231b9d0 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
@@ -1343,8 +1343,7 @@
* for each path in order, awaiting the previous render to complete before
* continuing.
*
- * @param initialCount The total number of paths in the pass. This
- * is used to generate log messages.
+ * @param initialCount The total number of paths in the pass.
*/
private async _renderInOrder(
files: PatchSetFile[],
@@ -1376,11 +1375,19 @@
throw new Error('diffPrefs must be set');
}
- const promises: Array<Promise<unknown>> = [diffElem.reload()];
- if (this._loggedIn && !this.diffPrefs.manual_review) {
- promises.push(this._reviewFile(path, true));
+ // When one file is expanded individually then automatically mark as
+ // reviewed, if the user's diff prefs request it. Doing this for
+ // "Expand All" would not be what the user wants, because there is no
+ // control over which diffs were actually seen. And for lots of diffs
+ // that would even be a problem for write QPS quota.
+ if (
+ this._loggedIn &&
+ !this.diffPrefs.manual_review &&
+ initialCount === 1
+ ) {
+ this._reviewFile(path, true);
}
- return Promise.all(promises);
+ return diffElem.reload();
});
this._cancelForEachDiff = undefined;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index bd12eae..991c995 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -1009,24 +1009,6 @@
const reviewStub = sinon.stub(element, '_reviewFile');
let callCount = 0;
const diffs = [{
- path: 'p0',
- style: {},
- prefetchDiff() {},
- reload() {
- assert.equal(reviewStub.callCount, 2);
- assert.equal(callCount++, 2);
- return Promise.resolve();
- },
- }, {
- path: 'p1',
- style: {},
- prefetchDiff() {},
- reload() {
- assert.equal(reviewStub.callCount, 1);
- assert.equal(callCount++, 1);
- return Promise.resolve();
- },
- }, {
path: 'p2',
style: {},
prefetchDiff() {},
@@ -1036,11 +1018,9 @@
return Promise.resolve();
},
}];
- element._renderInOrder([
- {path: 'p2'}, {path: 'p1'}, {path: 'p0'},
- ], diffs, 3);
+ element._renderInOrder([{path: 'p2'}], diffs, 1);
await flush();
- assert.equal(reviewStub.callCount, 3);
+ assert.equal(reviewStub.callCount, 1);
});
test('_renderInOrder respects diffPrefs.manual_review', async () => {
diff --git a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts
index a6c67d7..77b9d38 100644
--- a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts
@@ -119,7 +119,6 @@
</span>
<iron-input
id="filterInput"
- placeholder="Filter"
.bindValue=${this.filterText}
@bind-value-changed=${(e: BindValueChangeEvent) => {
this.filterText = e.detail.value;
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.ts b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.ts
index 5da6fa8..748729c 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.ts
@@ -125,19 +125,19 @@
gr-button {
--button-background-color: var(--vote-chip-unselected-color);
}
- gr-button[vote='max'].iron-selected {
+ gr-button[data-vote='max'].iron-selected {
--button-background-color: var(--vote-chip-selected-positive-color);
}
- gr-button[vote='positive'].iron-selected {
+ gr-button[data-vote='positive'].iron-selected {
--button-background-color: var(--vote-chip-selected-positive-color);
}
- gr-button[vote='neutral'].iron-selected {
+ gr-button[data-vote='neutral'].iron-selected {
--button-background-color: var(--vote-chip-selected-neutral-color);
}
- gr-button[vote='negative'].iron-selected {
+ gr-button[data-vote='negative'].iron-selected {
--button-background-color: var(--vote-chip-selected-negative-color);
}
- gr-button[vote='min'].iron-selected {
+ gr-button[data-vote='min'].iron-selected {
--button-background-color: var(--vote-chip-selected-negative-color);
}
gr-button > gr-tooltip-content {
@@ -205,7 +205,7 @@
return html`
<iron-selector
id="labelSelector"
- attr-for-selected="data-value"
+ .attrForSelected=${'data-value'}
?hidden="${!this._computeAnyPermittedLabelValues()}"
selected="${ifDefined(this._computeLabelValue())}"
@selected-item-changed=${this.setSelectedValueText}
@@ -223,12 +223,12 @@
(value, index) => html`
<gr-button
role="radio"
- vote="${this._computeVoteAttribute(
+ title="${ifDefined(this.computeLabelValueTitle(value))}"
+ data-vote="${this._computeVoteAttribute(
Number(value),
index,
items.length
)}"
- title="${ifDefined(this.computeLabelValueTitle(value))}"
data-name="${ifDefined(this.label?.name)}"
data-value="${value}"
aria-label="${value}"
@@ -322,7 +322,7 @@
/**
* Private but used in tests.
* Maps the label value to exactly one of: min, max, positive, negative,
- * neutral. Used for the 'vote' attribute, because we don't want to
+ * neutral. Used for the 'data-vote' attribute, because we don't want to
* interfere with <iron-selector> using the 'class' attribute for setting
* 'iron-selected'.
*/
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.ts b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.ts
index 958b11a..e2d05c1 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.ts
@@ -343,7 +343,6 @@
<span class="placeholder" data-label="Verified"></span>
<iron-selector
aria-labelledby="labelName"
- attr-for-selected="data-value"
id="labelSelector"
role="radiogroup"
selected="+1"
@@ -356,7 +355,7 @@
role="radio"
tabindex="0"
title="bad"
- vote="min"
+ data-vote="min"
votechip=""
flatten=""
>
@@ -371,7 +370,7 @@
data-value=" 0"
role="radio"
tabindex="0"
- vote="neutral"
+ data-vote="neutral"
votechip=""
flatten=""
>
@@ -389,7 +388,7 @@
role="radio"
tabindex="0"
title="good"
- vote="max"
+ data-vote="max"
votechip=""
flatten=""
>
diff --git a/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores.ts b/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores.ts
index f3abe13..50492d2 100644
--- a/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores.ts
+++ b/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores.ts
@@ -120,6 +120,7 @@
const labels = this.change?.labels ?? {};
return html`<gr-trigger-vote
.label="${score.label}"
+ .displayValue=${score.value}
.labelInfo="${labels[score.label]}"
.change="${this.change}"
.mutable="${false}"
diff --git a/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores_test.ts b/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores_test.ts
index d9c1db1..74d1114 100644
--- a/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-message-scores/gr-message-scores_test.ts
@@ -162,9 +162,14 @@
const triggerChips =
element.shadowRoot?.querySelectorAll('gr-trigger-vote');
assert.equal(triggerChips?.length, 1);
- expect(triggerChips?.[0]).shadowDom.equal(`<div class="container">
+ const triggerChip = triggerChips?.[0];
+ expect(triggerChip).shadowDom.equal(`<div class="container">
<span class="label">Auto-Submit</span>
+ <gr-vote-chip></gr-vote-chip>
</div>`);
+ const voteChips = triggerChip?.shadowRoot?.querySelectorAll('gr-vote-chip');
+ assert.equal(voteChips?.length, 1);
+ expect(voteChips?.[0]).shadowDom.equal('');
const scoreChips = element.shadowRoot?.querySelectorAll('.score');
assert.equal(scoreChips?.length, 1);
expect(scoreChips?.[0]).dom.equal(`<span class="removed score">
diff --git a/polygerrit-ui/app/elements/change/gr-trigger-vote/gr-trigger-vote.ts b/polygerrit-ui/app/elements/change/gr-trigger-vote/gr-trigger-vote.ts
index a00de22..41964c2 100644
--- a/polygerrit-ui/app/elements/change/gr-trigger-vote/gr-trigger-vote.ts
+++ b/polygerrit-ui/app/elements/change/gr-trigger-vote/gr-trigger-vote.ts
@@ -45,6 +45,13 @@
@property({type: Object})
account?: AccountInfo;
+ /**
+ * If defined, trigger-vote is shown with this value instead of the latest
+ * vote. This is useful for change log.
+ */
+ @property()
+ displayValue?: string;
+
@property({type: Boolean})
mutable?: boolean;
@@ -115,6 +122,11 @@
private renderVotes() {
const {labelInfo} = this;
if (!labelInfo) return;
+ if (this.displayValue)
+ return html`<gr-vote-chip
+ .displayValue=${this.displayValue}
+ .label=${labelInfo}
+ ></gr-vote-chip>`;
if (isDetailedLabelInfo(labelInfo)) {
const approvals = getAllUniqueApprovals(labelInfo).filter(
approval => !hasNeutralStatus(labelInfo, approval)
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
index 8541d5d..386591b 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
@@ -74,7 +74,6 @@
suffixForDashboard?: string;
selfOnly?: boolean;
hideIfEmpty?: boolean;
- isOutgoing?: boolean;
results?: ChangeInfo[];
}
@@ -115,12 +114,11 @@
hideIfEmpty: true,
suffixForDashboard: 'limit:25',
};
-const OUTGOING: DashboardSection = {
+export const OUTGOING: DashboardSection = {
// Non-WIP open changes owned by viewed user. Filter out changes ignored
// by the viewing user.
name: 'Outgoing reviews',
query: 'is:open owner:${user} -is:wip -is:ignored',
- isOutgoing: true,
suffixForDashboard: 'limit:25',
};
const INCOMING: DashboardSection = {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index c5b509c..8e377b8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -100,6 +100,7 @@
import {distinctUntilChanged, map} from 'rxjs/operators';
import {deepEqual} from '../../../utils/deep-util';
import {Category} from '../../../api/checks';
+import {GrSyntaxLayerWorker} from '../../../embed/diff/gr-syntax-layer/gr-syntax-layer-worker';
const EMPTY_BLAME = 'No blame information for this diff.';
@@ -293,7 +294,7 @@
private readonly jsAPI = getAppContext().jsApiService;
- private readonly syntaxLayer = new GrSyntaxLayer();
+ private readonly syntaxLayer: GrSyntaxLayer | GrSyntaxLayerWorker;
private checksSubscription?: Subscription;
@@ -301,6 +302,12 @@
constructor() {
super();
+ const useWorker = this.flags.isEnabled(
+ KnownExperimentId.SYNTAX_LAYER_WORKER
+ );
+ this.syntaxLayer = useWorker
+ ? new GrSyntaxLayerWorker()
+ : new GrSyntaxLayer();
this._renderPrefs = {
...this._renderPrefs,
use_lit_components: this.flags.isEnabled(
@@ -701,7 +708,12 @@
/** Cancel any remaining diff builder rendering work. */
cancel() {
this.$.diff.cancel();
- this.syntaxLayer.cancel();
+ // TODO: Remove calling cancel() when switching to GrSyntaxLayerWorker.
+ // The plan is to only start syntax highlighting when a diff is visible on
+ // screen, and HighlightJS itself does not allow cancelling. So there is
+ // unclear benefit from the ability to cancel, which would be non-trivial
+ // to implement correctly and efficiently.
+ if (this.syntaxLayer instanceof GrSyntaxLayer) this.syntaxLayer.cancel();
}
getCursorStops() {
diff --git a/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-custom-plugin-header.ts b/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-custom-plugin-header.ts
deleted file mode 100644
index 6580ad6..0000000
--- a/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-custom-plugin-header.ts
+++ /dev/null
@@ -1,54 +0,0 @@
-/**
- * @license
- * Copyright (C) 2017 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.
- */
-/* eslint-disable lit/no-legacy-template-syntax,lit/prefer-static-styles */
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-import {customElement, property} from '@polymer/decorators';
-
-declare global {
- interface HTMLElementTagNameMap {
- 'gr-custom-plugin-header': GrCustomPluginHeader;
- }
-}
-
-@customElement('gr-custom-plugin-header')
-export class GrCustomPluginHeader extends PolymerElement {
- @property({type: String})
- logoUrl = '';
-
- @property({type: String})
- override title = '';
-
- static get template() {
- return html`
- <style>
- img {
- width: 1em;
- height: 1em;
- vertical-align: middle;
- }
- .title {
- margin-left: var(--spacing-xs);
- }
- </style>
- <span>
- <img src="[[logoUrl]]" hidden$="[[!logoUrl]]" />
- <span class="title">[[title]]</span>
- </span>
- `;
- }
-}
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts
index 64f97ca..cfbb942 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts
@@ -58,7 +58,7 @@
@property({type: String})
selectedScheme?: string;
- @property({type: Boolean})
+ @property({type: Boolean, attribute: 'show-keyboard-shortcut-tooltips'})
showKeyboardShortcutTooltips = false;
private readonly restApiService = getAppContext().restApiService;
diff --git a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
index b011d23..487145f 100644
--- a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
+++ b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
@@ -14,6 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+import '../gr-tooltip-content/gr-tooltip-content';
import {LitElement, css, html} from 'lit';
import {customElement, property} from 'lit/decorators';
import {
@@ -50,6 +51,13 @@
@property({type: Boolean})
more = false;
+ /**
+ * If defined, vote-chip is shown with this value instead of the latest vote.
+ * This is useful for change log.
+ */
+ @property()
+ displayValue?: string;
+
private readonly flagsService = getAppContext().flagsService;
static override get styles() {
@@ -58,6 +66,7 @@
:host([circle-shape]) .vote-chip {
border-radius: 50%;
border: none;
+ padding: 2px;
}
.vote-chip.max {
background-color: var(--vote-color-approved);
@@ -125,17 +134,24 @@
const renderValue = this.renderValue();
if (!renderValue) return;
- return html`<span class="container ${this.more ? 'more' : ''}">
+ return html`<gr-tooltip-content
+ class="container ${this.more ? 'more' : ''}"
+ title="${this.computeTooltip()}"
+ has-tooltip
+ >
<div class="vote-chip ${this.computeClass()}">${renderValue}</div>
${this.more
? html`<div class="chip-angle ${this.computeClass()}">
${renderValue}
</div>`
: ''}
- </span>`;
+ </gr-tooltip-content>`;
}
private renderValue() {
+ if (this.displayValue) {
+ return this.displayValue;
+ }
if (!this.label) {
return '';
} else if (isDetailedLabelInfo(this.label)) {
@@ -157,9 +173,19 @@
private computeClass() {
if (!this.label) {
return '';
+ } else if (this.displayValue) {
+ const status = getLabelStatus(this.label, Number(this.displayValue));
+ return classForLabelStatus(status);
} else {
const status = getLabelStatus(this.label, this.vote?.value);
return classForLabelStatus(status);
}
}
+
+ private computeTooltip() {
+ if (!this.label || !isDetailedLabelInfo(this.label)) {
+ return '';
+ }
+ return this.label.values?.[valueString(this.vote?.value)] ?? '';
+ }
}
diff --git a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip_test.ts b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip_test.ts
index d5c5df8..de8f5ae 100644
--- a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip_test.ts
@@ -48,9 +48,13 @@
});
test('renders', () => {
- expect(element).shadowDom.to.equal(`<span class="container">
+ expect(element).shadowDom.to.equal(`<gr-tooltip-content
+ class="container"
+ has-tooltip=""
+ title=""
+ >
<div class="max vote-chip">👍</div>
- </span>`);
+ </gr-tooltip-content>`);
});
});
@@ -69,11 +73,15 @@
});
test('renders', () => {
- expect(element).shadowDom.to.equal(`<span class="container">
+ expect(element).shadowDom.to.equal(`<gr-tooltip-content
+ class="container"
+ has-tooltip=""
+ title=""
+ >
<div class="positive vote-chip">
+2
</div>
- </span>`);
+ </gr-tooltip-content>`);
});
test('renders negative vote', async () => {
@@ -84,11 +92,15 @@
element = await fixture<GrVoteChip>(
html`<gr-vote-chip .label=${labelInfo} .vote=${vote}></gr-vote-chip>`
);
- expect(element).shadowDom.to.equal(`<span class="container">
+ expect(element).shadowDom.to.equal(`<gr-tooltip-content
+ class="container"
+ has-tooltip=""
+ title="Wrong Style or Formatting"
+ >
<div class="min vote-chip">
-1
</div>
- </span>`);
+ </gr-tooltip-content>`);
});
test('renders for more than 1 vote', async () => {
@@ -99,10 +111,14 @@
more
></gr-vote-chip>`
);
- expect(element).shadowDom.to.equal(`<span class="container more">
+ expect(element).shadowDom.to.equal(`<gr-tooltip-content
+ class="container more"
+ has-tooltip=""
+ title=""
+ >
<div class="positive vote-chip">+2</div>
<div class="chip-angle positive">+2</div>
- </span>`);
+ </gr-tooltip-content>`);
});
});
});
diff --git a/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts b/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts
new file mode 100644
index 0000000..ba64f27
--- /dev/null
+++ b/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts
@@ -0,0 +1,198 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
+import {FILE, GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
+import {DiffFileMetaInfo, DiffInfo} from '../../../types/diff';
+import {DiffLayer, DiffLayerListener} from '../../../types/types';
+import {Side} from '../../../constants/constants';
+import {LANGUAGE_MAP} from './gr-syntax-layer';
+import {getAppContext} from '../../../services/app-context';
+import {SyntaxLayerLine} from '../../../types/syntax-worker-api';
+
+const CLASS_PREFIX = 'gr-diff gr-syntax gr-syntax-';
+
+const CLASS_SAFELIST = new Set<string>([
+ 'attr',
+ 'attribute',
+ 'built_in',
+ 'comment',
+ 'doctag',
+ 'function',
+ 'keyword',
+ 'link',
+ 'literal',
+ 'meta',
+ 'meta-keyword',
+ 'name',
+ 'number',
+ 'params',
+ 'property',
+ 'regexp',
+ 'selector-attr',
+ 'selector-class',
+ 'selector-id',
+ 'selector-pseudo',
+ 'selector-tag',
+ 'string',
+ 'tag',
+ 'template-tag',
+ 'template-variable',
+ 'title',
+ 'type',
+ 'variable',
+]);
+
+export class GrSyntaxLayerWorker implements DiffLayer {
+ diff?: DiffInfo;
+
+ enabled = true;
+
+ private leftRanges: SyntaxLayerLine[] = [];
+
+ private rightRanges: SyntaxLayerLine[] = [];
+
+ private listeners: DiffLayerListener[] = [];
+
+ private readonly highlightService = getAppContext().highlightService;
+
+ init(diff?: DiffInfo) {
+ this.leftRanges = [];
+ this.rightRanges = [];
+ this.diff = diff;
+ }
+
+ setEnabled(enabled: boolean) {
+ this.enabled = enabled;
+ }
+
+ addListener(listener: DiffLayerListener) {
+ this.listeners.push(listener);
+ }
+
+ removeListener(listener: DiffLayerListener) {
+ this.listeners = this.listeners.filter(f => f !== listener);
+ }
+
+ annotate(el: HTMLElement, _: HTMLElement, line: GrDiffLine) {
+ if (!this.enabled) return;
+ if (line.beforeNumber === FILE || line.afterNumber === FILE) return;
+ if (line.beforeNumber === 'LOST' || line.afterNumber === 'LOST') return;
+
+ let side: Side | undefined;
+ if (
+ line.type === GrDiffLineType.REMOVE ||
+ (line.type === GrDiffLineType.BOTH &&
+ el.getAttribute('data-side') !== Side.RIGHT)
+ ) {
+ side = Side.LEFT;
+ } else if (
+ line.type === GrDiffLineType.ADD ||
+ el.getAttribute('data-side') !== Side.LEFT
+ ) {
+ side = Side.RIGHT;
+ }
+ if (!side) return;
+
+ const isLeft = side === Side.LEFT;
+ const lineNumber = isLeft ? line.beforeNumber : line.afterNumber;
+ const rangesPerLine = isLeft ? this.leftRanges : this.rightRanges;
+ const ranges = rangesPerLine[lineNumber - 1]?.ranges ?? [];
+
+ for (const range of ranges) {
+ if (!CLASS_SAFELIST.has(range.className)) continue;
+ GrAnnotation.annotateElement(
+ el,
+ range.start,
+ range.length,
+ CLASS_PREFIX + range.className
+ );
+ }
+ }
+
+ _getLanguage(metaInfo?: DiffFileMetaInfo) {
+ if (!metaInfo) return undefined;
+ // The Gerrit API provides only content-type, but for other users of
+ // gr-diff it may be more convenient to specify the language directly.
+ return metaInfo.language ?? LANGUAGE_MAP.get(metaInfo.content_type);
+ }
+
+ /**
+ * Computes SyntaxLayerLines asynchronously, which can then later be applied,
+ * when the annotate() method of the layer API is called.
+ *
+ * For larger files this is an expensive operation, but is offloaded to a web
+ * worker. We are using the HighlightJS lib for doing the actual highlighting.
+ *
+ * `init()` must have been called before. There is actually no good reason for
+ * init() and process() to be separated. The diff host typically allows the
+ * diff builder to render first and only then calls process(), but as soon as
+ * the diff is known the highlighting process can be kicked off.
+ * TODO(brohlfs): Call process() directly after init().
+ *
+ * annotate() will only be able to apply highlighting after process() has
+ * completed, but that will likely happen later. That is why layer can have
+ * listeners. When process() completes, the listeners will be notified, which
+ * tells the diff renderer that another call to annotate() is needed.
+ */
+ async process() {
+ this.leftRanges = [];
+ this.rightRanges = [];
+ if (!this.enabled || !this.diff) return;
+
+ const leftLanguage = this._getLanguage(this.diff.meta_a);
+ const rightLanguage = this._getLanguage(this.diff.meta_b);
+
+ let leftContent = '';
+ let rightContent = '';
+ for (const chunk of this.diff.content) {
+ const a = [...(chunk.a ?? []), ...(chunk.ab ?? [])];
+ const b = [...(chunk.b ?? []), ...(chunk.ab ?? [])];
+ for (const line of a) {
+ leftContent += line + '\n';
+ }
+ for (const line of b) {
+ rightContent += line + '\n';
+ }
+ }
+
+ const leftPromise = this.highlight(leftLanguage, leftContent);
+ const rightPromise = this.highlight(rightLanguage, rightContent);
+ this.leftRanges = await leftPromise;
+ this.rightRanges = await rightPromise;
+ this.notify();
+ }
+
+ async highlight(language?: string, code?: string) {
+ if (!language || !code) return [];
+ return this.highlightService.highlight(language, code);
+ }
+
+ notify() {
+ // We don't want to notify for lines that don't have any SyntaxLayerRange.
+ // So for both sides we are looking for the first and the last occurrence
+ // of a line with at least one SyntaxLayerRange.
+ const leftRangesReversed = [...this.leftRanges].reverse();
+ const leftStart = this.leftRanges.findIndex(line => line.ranges.length > 0);
+ const leftEnd =
+ this.leftRanges.length -
+ 1 -
+ leftRangesReversed.findIndex(line => line.ranges.length > 0);
+
+ const rightRangesReversed = [...this.rightRanges].reverse();
+ const rightStart = this.rightRanges.findIndex(
+ line => line.ranges.length > 0
+ );
+ const rightEnd =
+ this.rightRanges.length -
+ 1 -
+ rightRangesReversed.findIndex(line => line.ranges.length > 0);
+
+ for (const listener of this.listeners) {
+ if (leftStart > -1) listener(leftStart + 1, leftEnd + 1, Side.LEFT);
+ if (rightStart > -1) listener(rightStart + 1, rightEnd + 1, Side.RIGHT);
+ }
+ }
+}
diff --git a/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker_test.ts b/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker_test.ts
new file mode 100644
index 0000000..9c2cc9d
--- /dev/null
+++ b/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker_test.ts
@@ -0,0 +1,115 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {DiffInfo, GrDiffLineType, Side} from '../../../api/diff';
+import '../../../test/common-test-setup-karma';
+import {SyntaxLayerLine} from '../../../types/syntax-worker-api';
+import {GrDiffLine} from '../gr-diff/gr-diff-line';
+import {GrSyntaxLayerWorker} from './gr-syntax-layer-worker';
+
+const diff: DiffInfo = {
+ meta_a: {
+ name: 'somepath/somefile.js',
+ content_type: 'text/javascript',
+ lines: 3,
+ language: 'lang-left',
+ },
+ meta_b: {
+ name: 'somepath/somefile.js',
+ content_type: 'text/javascript',
+ lines: 4,
+ language: 'lang-right',
+ },
+ change_type: 'MODIFIED',
+ intraline_status: 'OK',
+ content: [
+ {
+ ab: ['import it;'],
+ },
+ {
+ b: ['b only'],
+ },
+ {
+ ab: [' public static final {', 'ab3'],
+ },
+ ],
+};
+
+const leftRanges: SyntaxLayerLine[] = [
+ {ranges: [{start: 0, length: 6, className: 'literal'}]},
+ {ranges: []},
+ {ranges: []},
+];
+
+const rightRanges: SyntaxLayerLine[] = [
+ {ranges: []},
+ {ranges: []},
+ {
+ ranges: [
+ {start: 0, length: 2, className: 'not-safe'},
+ {start: 2, length: 6, className: 'literal'},
+ {start: 9, length: 6, className: 'keyword'},
+ {start: 16, length: 5, className: 'name'},
+ ],
+ },
+ {ranges: []},
+];
+
+suite('gr-syntax-layer-worker tests', () => {
+ let layer: GrSyntaxLayerWorker;
+ let listener: sinon.SinonStub;
+
+ const annotate = (side: Side, lineNumber: number, text: string) => {
+ const el = document.createElement('div');
+ const lineNumberEl = document.createElement('td');
+ el.setAttribute('data-side', side);
+ el.innerText = text;
+ const line = new GrDiffLine(GrDiffLineType.BOTH);
+ if (side === Side.LEFT) line.beforeNumber = lineNumber;
+ if (side === Side.RIGHT) line.afterNumber = lineNumber;
+ layer.annotate(el, lineNumberEl, line);
+ return el;
+ };
+
+ setup(() => {
+ layer = new GrSyntaxLayerWorker();
+ listener = sinon.stub();
+ layer.addListener(listener);
+ sinon.stub(layer, 'highlight').callsFake((lang?: string) => {
+ if (lang === 'lang-left') return Promise.resolve(leftRanges);
+ if (lang === 'lang-right') return Promise.resolve(rightRanges);
+ return Promise.resolve([]);
+ });
+ layer.init(diff);
+ });
+
+ test('process and annotate line 2 LEFT', async () => {
+ await layer.process();
+ const el = annotate(Side.LEFT, 1, 'import it;');
+ assert.equal(
+ el.innerHTML,
+ '<hl class="gr-diff gr-syntax gr-syntax-literal">import</hl> it;'
+ );
+ assert.equal(listener.callCount, 2);
+ assert.equal(listener.getCall(0).args[0], 1);
+ assert.equal(listener.getCall(0).args[1], 1);
+ assert.equal(listener.getCall(0).args[2], Side.LEFT);
+ assert.equal(listener.getCall(1).args[0], 3);
+ assert.equal(listener.getCall(1).args[1], 3);
+ assert.equal(listener.getCall(1).args[2], Side.RIGHT);
+ });
+
+ test('process and annotate line 3 RIGHT', async () => {
+ await layer.process();
+ const el = annotate(Side.RIGHT, 3, ' public static final {');
+ assert.equal(
+ el.innerHTML,
+ ' <hl class="gr-diff gr-syntax gr-syntax-literal">public</hl> ' +
+ '<hl class="gr-diff gr-syntax gr-syntax-keyword">static</hl> ' +
+ '<hl class="gr-diff gr-syntax gr-syntax-name">final</hl> {'
+ );
+ assert.equal(listener.callCount, 2);
+ });
+});
diff --git a/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer.ts b/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer.ts
index b3d6605..bd34034 100644
--- a/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer.ts
+++ b/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer.ts
@@ -23,7 +23,7 @@
import {HLJS_LIBRARY_CONFIG} from '../../../elements/shared/gr-lib-loader/highlightjs_config';
import {Side} from '../../../constants/constants';
-const LANGUAGE_MAP = new Map<string, string>([
+export const LANGUAGE_MAP = new Map<string, string>([
['application/dart', 'dart'],
['application/json', 'json'],
['application/x-powershell', 'powershell'],
diff --git a/polygerrit-ui/app/embed/gr-diff-app-context-init.ts b/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
index 53ed193..7d2d742 100644
--- a/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
+++ b/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
@@ -93,6 +93,9 @@
pluginsModel: (_ctx: Partial<AppContext>) => {
throw new Error('pluginsModel is not implemented');
},
+ highlightService: (_ctx: Partial<AppContext>) => {
+ throw new Error('highlightService is not implemented');
+ },
};
return create<AppContext>(appRegistry);
}
diff --git a/polygerrit-ui/app/rules.bzl b/polygerrit-ui/app/rules.bzl
index 401c0c3..01bb31f 100644
--- a/polygerrit-ui/app/rules.bzl
+++ b/polygerrit-ui/app/rules.bzl
@@ -32,6 +32,18 @@
],
)
+ rollup_bundle(
+ name = "syntax-worker",
+ srcs = [app_name + "-full-src"],
+ config_file = ":rollup.config.js",
+ entry_point = "_pg_ts_out/workers/syntax-worker.js",
+ rollup_bin = "//tools/node_tools:rollup-bin",
+ sourcemap = "hidden",
+ deps = [
+ "@tools_npm//rollup-plugin-node-resolve",
+ ],
+ )
+
native.filegroup(
name = name + "_app_sources",
srcs = [
@@ -45,6 +57,13 @@
)
native.filegroup(
+ name = name + "_worker_sources",
+ srcs = [
+ "syntax-worker.js",
+ ],
+ )
+
+ native.filegroup(
name = name + "_top_sources",
srcs = [
"favicon.ico",
@@ -59,6 +78,7 @@
name + "_app_sources",
name + "_css_sources",
name + "_top_sources",
+ name + "_worker_sources",
"//lib/fonts:robotofonts",
"//lib/js:highlightjs__files",
"@ui_npm//:node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js",
@@ -70,11 +90,12 @@
outs = outs,
cmd = " && ".join([
"FONT_DIR=$$(dirname $(location @ui_npm//:node_modules/@polymer/font-roboto-local/package.json))/fonts",
- "mkdir -p $$TMP/polygerrit_ui/{styles/themes,fonts/{roboto,robotomono},bower_components/{highlightjs,webcomponentsjs,resemblejs},elements}",
+ "mkdir -p $$TMP/polygerrit_ui/{workers,styles/themes,fonts/{roboto,robotomono},bower_components/{highlightjs,webcomponentsjs,resemblejs},elements}",
"for f in $(locations " + name + "_app_sources); do ext=$${f##*.}; cp -p $$f $$TMP/polygerrit_ui/elements/" + app_name + ".$$ext; done",
"cp $(locations //lib/fonts:robotofonts) $$TMP/polygerrit_ui/fonts/",
"for f in $(locations " + name + "_top_sources); do cp $$f $$TMP/polygerrit_ui/; done",
"for f in $(locations " + name + "_css_sources); do cp $$f $$TMP/polygerrit_ui/styles; done",
+ "for f in $(locations " + name + "_worker_sources); do cp $$f $$TMP/polygerrit_ui/workers; done",
"for f in $(locations //lib/js:highlightjs__files); do cp $$f $$TMP/polygerrit_ui/bower_components/highlightjs/ ; done",
"cp $(location @ui_npm//:node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js) $$TMP/polygerrit_ui/bower_components/webcomponentsjs/webcomponents-lite.js",
"cp $(location @ui_npm//:node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js.map) $$TMP/polygerrit_ui/bower_components/webcomponentsjs/webcomponents-lite.js.map",
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 2d2400b..10aa266 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -37,6 +37,7 @@
import {ConfigModel, configModelToken} from '../models/config/config-model';
import {BrowserModel, browserModelToken} from '../models/browser/browser-model';
import {PluginsModel} from '../models/plugins/plugins-model';
+import {HighlightService} from './highlight/highlight-service';
/**
* The AppContext lazy initializator for all services
@@ -75,6 +76,7 @@
return new ShortcutsService(ctx.userModel, ctx.reportingService!);
},
pluginsModel: (_ctx: Partial<AppContext>) => new PluginsModel(),
+ highlightService: (_ctx: Partial<AppContext>) => new HighlightService(),
};
return create<AppContext>(appRegistry);
}
diff --git a/polygerrit-ui/app/services/app-context-init_test.ts b/polygerrit-ui/app/services/app-context-init_test.ts
index efe6aac..626c571 100644
--- a/polygerrit-ui/app/services/app-context-init_test.ts
+++ b/polygerrit-ui/app/services/app-context-init_test.ts
@@ -17,13 +17,13 @@
import '../test/common-test-setup-karma.js';
import {AppContext} from './app-context.js';
-import {createAppContext} from './app-context-init.js';
import {Finalizable} from './registry';
+import {createTestAppContext} from '../test/test-app-context-init.js';
suite('app context initializer tests', () => {
let appContext: AppContext & Finalizable;
setup(() => {
- appContext = createAppContext();
+ appContext = createTestAppContext();
});
teardown(() => {
diff --git a/polygerrit-ui/app/services/app-context.ts b/polygerrit-ui/app/services/app-context.ts
index 965146a..ba21734 100644
--- a/polygerrit-ui/app/services/app-context.ts
+++ b/polygerrit-ui/app/services/app-context.ts
@@ -26,6 +26,7 @@
import {RouterModel} from './router/router-model';
import {ShortcutsService} from './shortcuts/shortcuts-service';
import {PluginsModel} from '../models/plugins/plugins-model';
+import {HighlightService} from './highlight/highlight-service';
export interface AppContext {
routerModel: RouterModel;
@@ -39,6 +40,7 @@
userModel: UserModel;
shortcutsService: ShortcutsService;
pluginsModel: PluginsModel;
+ highlightService: HighlightService;
}
/**
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 515dab8..0edc13d 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -32,4 +32,5 @@
BULK_ACTIONS = 'UiFeature__bulk_actions_dashboard',
CHECK_RESULTS_IN_DIFFS = 'UiFeature__check_results_in_diffs',
DIFF_RENDERING_LIT = 'UiFeature__diff_rendering_lit',
+ SYNTAX_LAYER_WORKER = 'UiFeature__syntax_layer_worker',
}
diff --git a/polygerrit-ui/app/services/highlight/highlight-service.ts b/polygerrit-ui/app/services/highlight/highlight-service.ts
new file mode 100644
index 0000000..2711ba5
--- /dev/null
+++ b/polygerrit-ui/app/services/highlight/highlight-service.ts
@@ -0,0 +1,136 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {
+ SyntaxWorkerRequest,
+ SyntaxWorkerInit,
+ SyntaxWorkerResult,
+ SyntaxWorkerMessageType,
+ SyntaxLayerLine,
+} from '../../types/syntax-worker-api';
+import {createWorker} from '../../utils/worker-util';
+import {Finalizable} from '../registry';
+
+const hljsLibUrl = `${window.STATIC_RESOURCE_PATH}/bower_components/highlightjs/highlight.min.js`;
+
+const syntaxWorkerUrl = `${window.STATIC_RESOURCE_PATH}/workers/syntax-worker.js`;
+
+/**
+ * It is unlikely that a pool size greater than 3 will gain anything, because
+ * the app also needs the resources to process the results.
+ */
+const WORKER_POOL_SIZE = 3;
+
+/**
+ * Service for syntax highlighting. Maintains some HighlightJS workers doing
+ * their job in the background.
+ */
+export class HighlightService implements Finalizable {
+ // visible for testing
+ poolIdle: Set<Worker> = new Set();
+
+ // visible for testing
+ poolBusy: Set<Worker> = new Set();
+
+ // visible for testing
+ /** Queue for waiting that a worker becomes available. */
+ queueForWorker: Array<() => void> = [];
+
+ // visible for testing
+ /** Queue for waiting on the results of a worker. */
+ queueForResult: Map<Worker, (r: SyntaxLayerLine[]) => void> = new Map();
+
+ constructor() {
+ for (let i = 0; i < WORKER_POOL_SIZE; i++) {
+ this.addWorker();
+ }
+ }
+
+ /** Allows tests to produce fake workers. */
+ protected createWorker() {
+ return createWorker(syntaxWorkerUrl);
+ }
+
+ /** Creates, initializes and then moves a worker to the idle pool. */
+ private addWorker() {
+ const worker = this.createWorker();
+ // Will move to the idle pool after being initialized.
+ this.poolBusy.add(worker);
+ worker.onmessage = (e: MessageEvent<SyntaxWorkerResult>) => {
+ this.handleResult(worker, e.data);
+ };
+ const initMsg: SyntaxWorkerInit = {
+ type: SyntaxWorkerMessageType.INIT,
+ url: hljsLibUrl,
+ };
+ worker.postMessage(initMsg);
+ }
+
+ private moveIdleToBusy() {
+ const worker = this.poolIdle.values().next().value;
+ this.poolIdle.delete(worker);
+ this.poolBusy.add(worker);
+ return worker;
+ }
+
+ private moveBusyToIdle(worker: Worker) {
+ this.poolBusy.delete(worker);
+ this.poolIdle.add(worker);
+ const resolver = this.queueForWorker.shift();
+ if (resolver) resolver();
+ }
+
+ /**
+ * If there is worker in the idle pool, then return it. Otherwise wait for a
+ * worker to become a available.
+ */
+ private async requestWorker(): Promise<Worker> {
+ if (this.poolIdle.size > 0) {
+ const worker = this.moveIdleToBusy();
+ return Promise.resolve(worker);
+ }
+ await new Promise<void>(r => this.queueForWorker.push(r));
+ return this.requestWorker();
+ }
+
+ /**
+ * A worker is done with its job. Move it back to the idle pool and notify the
+ * resolver that is waiting for the results.
+ */
+ private handleResult(worker: Worker, result: SyntaxWorkerResult) {
+ this.moveBusyToIdle(worker);
+ if (result.error) console.error(`syntax worker failed: ${result.error}`);
+ const resolver = this.queueForResult.get(worker);
+ this.queueForResult.delete(worker);
+ if (resolver) resolver(result.ranges ?? []);
+ }
+
+ async highlight(language: string, code: string): Promise<SyntaxLayerLine[]> {
+ const worker = await this.requestWorker();
+ const message: SyntaxWorkerRequest = {
+ type: SyntaxWorkerMessageType.REQUEST,
+ language,
+ code,
+ };
+ const promise = new Promise<SyntaxLayerLine[]>(r => {
+ this.queueForResult.set(worker, r);
+ });
+ worker.postMessage(message);
+ return await promise;
+ }
+
+ finalize() {
+ for (const worker of this.poolIdle) {
+ worker.terminate();
+ }
+ this.poolIdle.clear();
+ for (const worker of this.poolBusy) {
+ worker.terminate();
+ }
+ this.poolBusy.clear();
+ this.queueForResult.clear();
+ this.queueForWorker.length = 0;
+ }
+}
diff --git a/polygerrit-ui/app/services/highlight/highlight-service_test.ts b/polygerrit-ui/app/services/highlight/highlight-service_test.ts
new file mode 100644
index 0000000..dc0c2f7
--- /dev/null
+++ b/polygerrit-ui/app/services/highlight/highlight-service_test.ts
@@ -0,0 +1,165 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../test/common-test-setup-karma';
+import {waitUntil} from '../../test/test-utils';
+import {
+ SyntaxWorkerMessage,
+ SyntaxWorkerResult,
+} from '../../types/syntax-worker-api';
+import {HighlightService} from './highlight-service';
+
+class FakeWorker implements Worker {
+ messages: SyntaxWorkerMessage[] = [];
+
+ constructor(private readonly autoRespond = true) {}
+
+ postMessage(message: SyntaxWorkerMessage) {
+ this.messages.push(message);
+ if (this.autoRespond) this.sendResult({ranges: []});
+ }
+
+ sendResult(result: SyntaxWorkerResult) {
+ if (this.onmessage)
+ this.onmessage({data: result} as MessageEvent<SyntaxWorkerResult>);
+ }
+
+ onmessage: ((e: MessageEvent<SyntaxWorkerResult>) => any) | null = null;
+
+ onmessageerror = null;
+
+ onerror = null;
+
+ terminate(): void {}
+
+ addEventListener(): void {}
+
+ removeEventListener(): void {}
+
+ dispatchEvent(): boolean {
+ return true;
+ }
+}
+
+export class MockHighlightService extends HighlightService {
+ idle = this.poolIdle as Set<FakeWorker>;
+
+ busy = this.poolBusy as Set<FakeWorker>;
+
+ override createWorker(): Worker {
+ return new FakeWorker();
+ }
+
+ countAllMessages() {
+ let count = 0;
+ for (const worker of [...this.idle, ...this.busy]) {
+ count += worker.messages.length;
+ }
+ return count;
+ }
+
+ sendToAll(result: SyntaxWorkerResult) {
+ for (const worker of this.busy) {
+ worker.sendResult(result);
+ }
+ }
+}
+
+export class MockHighlightServiceManual extends MockHighlightService {
+ override createWorker(): Worker {
+ return new FakeWorker(false);
+ }
+}
+
+suite('highlight-service tests', () => {
+ let service: MockHighlightServiceManual;
+
+ setup(() => {
+ service = new MockHighlightServiceManual();
+ });
+
+ test('initial state', () => {
+ assert.equal(service.poolBusy.size, 3);
+ assert.equal(service.poolIdle.size, 0);
+ assert.equal(service.queueForResult.size, 0);
+ assert.equal(service.queueForWorker.length, 0);
+ });
+
+ test('initialized workers move to idle pool', () => {
+ service.sendToAll({});
+ assert.equal(service.countAllMessages(), 3);
+
+ assert.equal(service.poolBusy.size, 0);
+ assert.equal(service.poolIdle.size, 3);
+ });
+
+ test('highlight 1', async () => {
+ service.sendToAll({});
+ assert.equal(service.countAllMessages(), 3);
+
+ const p = service.highlight('asdf', 'qwer');
+ assert.equal(service.poolBusy.size, 1);
+ assert.equal(service.poolIdle.size, 2);
+ await waitUntil(() => service.queueForResult.size > 0);
+ assert.equal(service.queueForResult.size, 1);
+ assert.equal(service.queueForWorker.length, 0);
+
+ service.sendToAll({ranges: []});
+ assert.equal(service.countAllMessages(), 4);
+ const ranges = await p;
+ assert.equal(ranges.length, 0);
+
+ await waitUntil(() => service.queueForResult.size === 0);
+ assert.equal(service.poolBusy.size, 0);
+ assert.equal(service.poolIdle.size, 3);
+ assert.equal(service.queueForResult.size, 0);
+ assert.equal(service.queueForWorker.length, 0);
+ });
+
+ test('highlight 5', async () => {
+ service.sendToAll({});
+ assert.equal(service.countAllMessages(), 3);
+
+ const p1 = service.highlight('asdf1', 'qwer1');
+ const p2 = service.highlight('asdf2', 'qwer2');
+ const p3 = service.highlight('asdf3', 'qwer3');
+ const p4 = service.highlight('asdf4', 'qwer4');
+ const p5 = service.highlight('asdf5', 'qwer5');
+
+ assert.equal(service.poolBusy.size, 3);
+ assert.equal(service.poolIdle.size, 0);
+ await waitUntil(() => service.queueForResult.size > 0);
+ assert.equal(service.queueForResult.size, 3);
+ assert.equal(service.queueForWorker.length, 2);
+
+ service.sendToAll({ranges: []});
+ assert.equal(service.countAllMessages(), 6);
+ const ranges1 = await p1;
+ const ranges2 = await p2;
+ const ranges3 = await p3;
+ assert.equal(ranges1.length, 0);
+ assert.equal(ranges2.length, 0);
+ assert.equal(ranges3.length, 0);
+
+ await waitUntil(() => service.queueForResult.size === 2);
+ assert.equal(service.poolBusy.size, 2);
+ assert.equal(service.poolIdle.size, 1);
+ assert.equal(service.queueForResult.size, 2);
+ assert.equal(service.queueForWorker.length, 0);
+
+ service.sendToAll({ranges: []});
+ assert.equal(service.countAllMessages(), 8);
+ const ranges4 = await p4;
+ const ranges5 = await p5;
+ assert.equal(ranges4.length, 0);
+ assert.equal(ranges5.length, 0);
+
+ await waitUntil(() => service.queueForResult.size === 0);
+ assert.equal(service.poolBusy.size, 0);
+ assert.equal(service.poolIdle.size, 3);
+ assert.equal(service.queueForResult.size, 0);
+ assert.equal(service.queueForWorker.length, 0);
+ });
+});
diff --git a/polygerrit-ui/app/test/test-app-context-init.ts b/polygerrit-ui/app/test/test-app-context-init.ts
index 9608239..7a9eee9 100644
--- a/polygerrit-ui/app/test/test-app-context-init.ts
+++ b/polygerrit-ui/app/test/test-app-context-init.ts
@@ -39,6 +39,7 @@
import {ConfigModel, configModelToken} from '../models/config/config-model';
import {BrowserModel, browserModelToken} from '../models/browser/browser-model';
import {PluginsModel} from '../models/plugins/plugins-model';
+import {MockHighlightService} from '../services/highlight/highlight-service_test';
export function createTestAppContext(): AppContext & Finalizable {
const appRegistry: Registry<AppContext> = {
@@ -67,6 +68,7 @@
return new ShortcutsService(ctx.userModel!, ctx.reportingService!);
},
pluginsModel: (_ctx: Partial<AppContext>) => new PluginsModel(),
+ highlightService: (_ctx: Partial<AppContext>) => new MockHighlightService(),
};
return create<AppContext>(appRegistry);
}
diff --git a/polygerrit-ui/app/tsconfig.json b/polygerrit-ui/app/tsconfig.json
index a335926..ac26c6d 100644
--- a/polygerrit-ui/app/tsconfig.json
+++ b/polygerrit-ui/app/tsconfig.json
@@ -95,6 +95,7 @@
"types/**/*",
"utils/**/*",
"test/**/*",
+ "workers/**/*",
"tmpl_out/**/*" //Created by template checker in dev-mode
]
}
diff --git a/polygerrit-ui/app/tsconfig_bazel.json b/polygerrit-ui/app/tsconfig_bazel.json
index cd83fc0..dba3060 100644
--- a/polygerrit-ui/app/tsconfig_bazel.json
+++ b/polygerrit-ui/app/tsconfig_bazel.json
@@ -22,10 +22,11 @@
"services/**/*",
"styles/**/*",
"types/**/*",
- "utils/**/*"
+ "utils/**/*",
+ "workers/**/*"
],
"exclude": [
"**/*_test.ts",
"**/*_test.js"
]
-}
+}
\ No newline at end of file
diff --git a/polygerrit-ui/app/tsconfig_bazel_test.json b/polygerrit-ui/app/tsconfig_bazel_test.json
index be3c934..78bb350 100644
--- a/polygerrit-ui/app/tsconfig_bazel_test.json
+++ b/polygerrit-ui/app/tsconfig_bazel_test.json
@@ -7,7 +7,9 @@
"../../external/ui_dev_npm/node_modules/@types"
],
"paths": {
- "@polymer/iron-test-helpers/*": ["../../ui_dev_npm/node_modules/@polymer/iron-test-helpers/*"]
+ "@polymer/iron-test-helpers/*": [
+ "../../ui_dev_npm/node_modules/@polymer/iron-test-helpers/*"
+ ]
}
},
"include": [
@@ -25,9 +27,10 @@
"scripts/**/*",
"services/**/*",
"styles/**/*",
+ "test/**/*",
"types/**/*",
"utils/**/*",
- "test/**/*"
+ "workers/**/*"
],
"exclude": []
-}
+}
\ No newline at end of file
diff --git a/polygerrit-ui/app/types/syntax-worker-api.ts b/polygerrit-ui/app/types/syntax-worker-api.ts
new file mode 100644
index 0000000..5dd8a36
--- /dev/null
+++ b/polygerrit-ui/app/types/syntax-worker-api.ts
@@ -0,0 +1,90 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/**
+ * This file defines the API of syntax-worker, which is a web worker for syntax
+ * highlighting based on the HighlightJS library.
+ *
+ * Workers communicate via `postMessage(e)` and `onMessage(e)` where `e` is a
+ * MessageEvent.
+ *
+ * SyntaxWorker expects incoming messages to be of type
+ * `MessageEvent<SyntaxWorkerMessage>`. And outgoing messages will be of type
+ * `MessageEvent<SyntaxWorkerResult>`.
+ */
+
+/** Type of incoming messages for SyntaxWorker. */
+export enum SyntaxWorkerMessageType {
+ INIT,
+ REQUEST,
+}
+
+/** Incoming message for SyntaxWorker. */
+export interface SyntaxWorkerMessage {
+ type: SyntaxWorkerMessageType;
+}
+
+/**
+ * Requests the worker to import the HighlightJS lib from the given URL and
+ * initializes and configures it. Has to be called once before you can send
+ * a SyntaxWorkerRequest message.
+ */
+export interface SyntaxWorkerInit extends SyntaxWorkerMessage {
+ type: SyntaxWorkerMessageType.INIT;
+ url: string;
+}
+
+export function isInit(
+ x: SyntaxWorkerMessage | undefined
+): x is SyntaxWorkerInit {
+ return !!x && x.type === SyntaxWorkerMessageType.INIT;
+}
+
+/**
+ * Requests the worker to highlight the given code. The worker must have been
+ * initialized before.
+ */
+export interface SyntaxWorkerRequest extends SyntaxWorkerMessage {
+ type: SyntaxWorkerMessageType.REQUEST;
+ language: string;
+ code: string;
+}
+
+export function isRequest(
+ x: SyntaxWorkerMessage | undefined
+): x is SyntaxWorkerRequest {
+ return !!x && x.type === SyntaxWorkerMessageType.REQUEST;
+}
+
+/** Type of outgoing messages of SyntaxWorker. */
+export interface SyntaxWorkerResult {
+ /** Unset or undefined means "success". */
+ error?: string;
+ /**
+ * Returned by SyntaxWorkerRequest calls. Every line gets its own array of
+ * ranges. `ranges[0]` are the ranges for line 1. Every line has an array,
+ * which may be empty. All ranges are guaranteed to be closed.
+ */
+ ranges?: SyntaxLayerLine[];
+}
+
+/** Ranges for one line. */
+export interface SyntaxLayerLine {
+ ranges: SyntaxLayerRange[];
+}
+
+/** Can be used for `length` in SyntaxLayerRange. */
+export const UNCLOSED = -1;
+
+/** Range of characters in a line to be syntax highlighted. */
+export interface SyntaxLayerRange {
+ /** 0-based inclusive. */
+ start: number;
+ /** Can only be UNCLOSED during processing. */
+ length: number;
+ /** HighlightJS specific names, e.g. 'literal'. */
+ className: string;
+}
diff --git a/polygerrit-ui/app/types/types.ts b/polygerrit-ui/app/types/types.ts
index c6137eb..ab78015 100644
--- a/polygerrit-ui/app/types/types.ts
+++ b/polygerrit-ui/app/types/types.ts
@@ -166,7 +166,7 @@
languageName: string,
code: string,
ignore_illegals: boolean,
- continuation: unknown
+ continuation?: unknown
): HighlightJSResult;
}
diff --git a/polygerrit-ui/app/utils/path-list-util.ts b/polygerrit-ui/app/utils/path-list-util.ts
index 411421e..605241a 100644
--- a/polygerrit-ui/app/utils/path-list-util.ts
+++ b/polygerrit-ui/app/utils/path-list-util.ts
@@ -45,7 +45,7 @@
const bFile = b.substr(0, bLastDotIndex) || b;
// Sort header files above others with the same base name.
- const headerExts = ['h', 'hxx', 'hpp'];
+ const headerExts = ['h', 'hh', 'hxx', 'hpp'];
if (aFile.length > 0 && aFile === bFile) {
if (headerExts.includes(aExt) && headerExts.includes(bExt)) {
return a.localeCompare(b);
diff --git a/polygerrit-ui/app/utils/path-list-util_test.ts b/polygerrit-ui/app/utils/path-list-util_test.ts
index 79b5f09..eb071a5 100644
--- a/polygerrit-ui/app/utils/path-list-util_test.ts
+++ b/polygerrit-ui/app/utils/path-list-util_test.ts
@@ -79,6 +79,12 @@
['foo/bar.h', 'foo/bar.hpp', 'foo/bar.hxx']
);
+ // Regression test for Issue 15635
+ assert.deepEqual(
+ ['manager.cc', 'manager.hh'].sort(specialFilePathCompare),
+ ['manager.hh', 'manager.cc']
+ );
+
// Regression test for Issue 4448.
assert.deepEqual(
[
diff --git a/polygerrit-ui/app/utils/hljs-util.ts b/polygerrit-ui/app/utils/syntax-util.ts
similarity index 88%
rename from polygerrit-ui/app/utils/hljs-util.ts
rename to polygerrit-ui/app/utils/syntax-util.ts
index 1bd2072..318d46a 100644
--- a/polygerrit-ui/app/utils/hljs-util.ts
+++ b/polygerrit-ui/app/utils/syntax-util.ts
@@ -3,11 +3,16 @@
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
+import {
+ SyntaxLayerLine,
+ SyntaxLayerRange,
+ UNCLOSED,
+} from '../types/syntax-worker-api';
/**
* Utilities related to working with the HighlightJS syntax highlighting lib.
*
- * Note that this utility is mostly used by the hljs-worker, which is a Web
+ * Note that this utility is mostly used by the syntax-worker, which is a Web
* Worker and can thus not depend on document, the DOM or any related
* functionality.
*/
@@ -20,19 +25,6 @@
const openingSpan = new RegExp('<span class="(.*?)">');
const closingSpan = new RegExp('</span>');
-/** Can be used for `length` in SyntaxLayerRange. */
-const UNCLOSED = -1;
-
-/** Range of characters in a line to be syntax highlighted. */
-export interface SyntaxLayerRange {
- /** 1-based inclusive. */
- start: number;
- /** Can only be UNCLOSED during processing. */
- length: number;
- /** HighlightJS specific names, e.g. 'literal'. */
- className: string;
-}
-
/**
* HighlightJS produces one long HTML string with HTML elements spanning
* multiple lines. <gr-diff> is line based, needs all elements closed at the end
@@ -44,16 +36,16 @@
*/
export function highlightedStringToRanges(
highlightedCode: string
-): SyntaxLayerRange[][] {
+): SyntaxLayerLine[] {
// What the function eventually returns.
- const rangesPerLine: SyntaxLayerRange[][] = [];
+ const rangesPerLine: SyntaxLayerLine[] = [];
// The unclosed ranges that are carried over from one line to the next.
let carryOverRanges: SyntaxLayerRange[] = [];
for (let line of highlightedCode.split('\n')) {
const ranges: SyntaxLayerRange[] = [...carryOverRanges];
carryOverRanges = [];
- rangesPerLine.push(ranges);
+ rangesPerLine.push({ranges});
// Remove all span tags one after another from left to right.
// For each opening <span ...> push a new (unclosed) range.
diff --git a/polygerrit-ui/app/utils/hljs-util_test.ts b/polygerrit-ui/app/utils/syntax-util_test.ts
similarity index 62%
rename from polygerrit-ui/app/utils/hljs-util_test.ts
rename to polygerrit-ui/app/utils/syntax-util_test.ts
index 3c577ca..fef908a 100644
--- a/polygerrit-ui/app/utils/hljs-util_test.ts
+++ b/polygerrit-ui/app/utils/syntax-util_test.ts
@@ -4,14 +4,14 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../test/common-test-setup-karma';
-import './hljs-util';
+import './syntax-util';
import {
highlightedStringToRanges,
removeFirstSpan,
SpanType,
-} from './hljs-util';
+} from './syntax-util';
-suite('file hljs-util', () => {
+suite('file syntax-util', () => {
suite('function removeFirstSpan()', () => {
test('no matches', async () => {
assert.isUndefined(removeFirstSpan(''));
@@ -44,23 +44,26 @@
suite('function highlightedStringToRanges()', () => {
test('no ranges', async () => {
- assert.deepEqual(highlightedStringToRanges(''), [[]]);
- assert.deepEqual(highlightedStringToRanges('\n'), [[], []]);
+ assert.deepEqual(highlightedStringToRanges(''), [{ranges: []}]);
+ assert.deepEqual(highlightedStringToRanges('\n'), [
+ {ranges: []},
+ {ranges: []},
+ ]);
assert.deepEqual(highlightedStringToRanges('asdf\nasdf\nasdf'), [
- [],
- [],
- [],
+ {ranges: []},
+ {ranges: []},
+ {ranges: []},
]);
});
test('one line, one span', async () => {
assert.deepEqual(
highlightedStringToRanges('asdf<span class="c">qwer</span>asdf'),
- [[{start: 4, length: 4, className: 'c'}]]
+ [{ranges: [{start: 4, length: 4, className: 'c'}]}]
);
assert.deepEqual(
highlightedStringToRanges('<span class="d">asdfqwer</span>'),
- [[{start: 0, length: 8, className: 'd'}]]
+ [{ranges: [{start: 0, length: 8, className: 'd'}]}]
);
});
@@ -70,10 +73,12 @@
'asdf<span class="c">qwer</span>zxcv<span class="d">qwer</span>asdf'
),
[
- [
- {start: 4, length: 4, className: 'c'},
- {start: 12, length: 4, className: 'd'},
- ],
+ {
+ ranges: [
+ {start: 4, length: 4, className: 'c'},
+ {start: 12, length: 4, className: 'd'},
+ ],
+ },
]
);
});
@@ -84,10 +89,12 @@
'asdf<span class="c">qwer<span class="d">zxcv</span>qwer</span>asdf'
),
[
- [
- {start: 4, length: 12, className: 'c'},
- {start: 8, length: 4, className: 'd'},
- ],
+ {
+ ranges: [
+ {start: 4, length: 12, className: 'c'},
+ {start: 8, length: 4, className: 'd'},
+ ],
+ },
]
);
});
@@ -99,8 +106,8 @@
'asd<span class="d">qwe</span>asd'
),
[
- [{start: 4, length: 4, className: 'c'}],
- [{start: 3, length: 3, className: 'd'}],
+ {ranges: [{start: 4, length: 4, className: 'c'}]},
+ {ranges: [{start: 3, length: 3, className: 'd'}]},
]
);
});
@@ -111,8 +118,8 @@
'asdf<span class="c">qwer\n' + 'asdf</span>qwer'
),
[
- [{start: 4, length: 4, className: 'c'}],
- [{start: 0, length: 4, className: 'c'}],
+ {ranges: [{start: 4, length: 4, className: 'c'}]},
+ {ranges: [{start: 0, length: 4, className: 'c'}]},
]
);
});
@@ -124,14 +131,18 @@
'asdf</span>qwer</span>zxcv'
),
[
- [
- {start: 4, length: 8, className: 'c'},
- {start: 8, length: 4, className: 'd'},
- ],
- [
- {start: 0, length: 8, className: 'c'},
- {start: 0, length: 4, className: 'd'},
- ],
+ {
+ ranges: [
+ {start: 4, length: 8, className: 'c'},
+ {start: 8, length: 4, className: 'd'},
+ ],
+ },
+ {
+ ranges: [
+ {start: 0, length: 8, className: 'c'},
+ {start: 0, length: 4, className: 'd'},
+ ],
+ },
]
);
});
@@ -145,16 +156,20 @@
'asdf</span>qwer'
),
[
- [{start: 4, length: 4, className: 'c'}],
- [
- {start: 0, length: 8, className: 'c'},
- {start: 4, length: 4, className: 'd'},
- ],
- [
- {start: 0, length: 8, className: 'c'},
- {start: 0, length: 4, className: 'd'},
- ],
- [{start: 0, length: 4, className: 'c'}],
+ {ranges: [{start: 4, length: 4, className: 'c'}]},
+ {
+ ranges: [
+ {start: 0, length: 8, className: 'c'},
+ {start: 4, length: 4, className: 'd'},
+ ],
+ },
+ {
+ ranges: [
+ {start: 0, length: 8, className: 'c'},
+ {start: 0, length: 4, className: 'd'},
+ ],
+ },
+ {ranges: [{start: 0, length: 4, className: 'c'}]},
]
);
});
diff --git a/polygerrit-ui/app/utils/worker-util.ts b/polygerrit-ui/app/utils/worker-util.ts
new file mode 100644
index 0000000..e4f5d2e
--- /dev/null
+++ b/polygerrit-ui/app/utils/worker-util.ts
@@ -0,0 +1,20 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/**
+ * We cannot import the worker script from cdn directly, because that is
+ * creating cross-origin issues. Instead we have to create a worker script on
+ * the fly and pull the actual worker via `importScripts()`. Apparently that
+ * is a well established pattern.
+ */
+function wrapUrl(url: string) {
+ const content = `importScripts("${url}");`;
+ return URL.createObjectURL(new Blob([content], {type: 'text/javascript'}));
+}
+
+export function createWorker(workerUrl: string): Worker {
+ return new Worker(wrapUrl(workerUrl));
+}
diff --git a/polygerrit-ui/app/workers/syntax-worker.ts b/polygerrit-ui/app/workers/syntax-worker.ts
new file mode 100644
index 0000000..85a0fb4
--- /dev/null
+++ b/polygerrit-ui/app/workers/syntax-worker.ts
@@ -0,0 +1,84 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {HighlightJS} from '../types/types';
+import {
+ SyntaxWorkerMessage,
+ SyntaxWorkerResult,
+ isRequest,
+ isInit,
+} from '../types/syntax-worker-api';
+import {highlightedStringToRanges} from '../utils/syntax-util';
+
+// This is an entry point file of a bundle. Keep free of exports!
+
+/**
+ * This is a web worker for calling the HighlightJS library for syntax
+ * highlighting. Files can be large and highlighting does not require
+ * the `document` or the `DOM`, so it is a perfect fit for a web worker.
+ *
+ * This file is a just a hub hooking into the web worker API. The message
+ * events for communicating with the main app are defined in the file
+ * `types/worker-api.ts`. And the `meat` of the computation is done in the
+ * file `syntax-util.ts`.
+ */
+
+/**
+ * `self` is for a worker what `window` is for the web app. It is called
+ * the `DedicatedWorkerGlobalScope`, see
+ * https://developer.mozilla.org/en-US/docs/Web/API/DedicatedWorkerGlobalScope
+ *
+ * Once imported the HighlightJS lib exposes its functionality via the global
+ * `hljs` variable.
+ */
+type Context = Worker & {hljs?: HighlightJS};
+const ctx: Context = self as unknown as Context;
+
+/**
+ * We are encapsulating the web worker API here, so this is the only place
+ * where you need to know about it and the MessageEvents in this file.
+ */
+ctx.onmessage = function (e: MessageEvent<SyntaxWorkerMessage>) {
+ try {
+ const message = e.data;
+ if (isInit(message)) {
+ worker.init(message.url);
+ const result: SyntaxWorkerResult = {ranges: []};
+ ctx.postMessage(result);
+ }
+ if (isRequest(message)) {
+ const ranges = worker.highlight(message.language, message.code);
+ const result: SyntaxWorkerResult = {ranges};
+ ctx.postMessage(result);
+ }
+ } catch (err) {
+ let error = 'syntax worker error';
+ if (err instanceof Error) error = err.message;
+ const result: SyntaxWorkerResult = {error, ranges: []};
+ ctx.postMessage(result);
+ }
+};
+
+class SyntaxWorker {
+ private highlightJsLib?: HighlightJS;
+
+ init(highlightJsLibUrl: string) {
+ importScripts(highlightJsLibUrl);
+ if (!ctx.hljs) {
+ throw new Error('HighlightJS lib not available after import');
+ }
+ this.highlightJsLib = ctx.hljs;
+ this.highlightJsLib.configure({classPrefix: ''});
+ }
+
+ highlight(language: string, code: string) {
+ if (!this.highlightJsLib) throw new Error('worker not initialized');
+ const highlight = this.highlightJsLib.highlight(language, code, true);
+ return highlightedStringToRanges(highlight.value);
+ }
+}
+
+/** Singleton instance being referenced in `onmessage` function above. */
+const worker = new SyntaxWorker();
diff --git a/tools/deps.bzl b/tools/deps.bzl
index abb90b3..62049e7 100644
--- a/tools/deps.bzl
+++ b/tools/deps.bzl
@@ -3,13 +3,13 @@
CAFFEINE_VERS = "2.9.2"
ANTLR_VERS = "3.5.2"
-SLF4J_VERS = "1.7.26"
+SLF4J_VERS = "1.7.33"
COMMONMARK_VERS = "0.10.0"
-FLEXMARK_VERS = "0.50.42"
+FLEXMARK_VERS = "0.50.50"
GREENMAIL_VERS = "1.5.5"
MAIL_VERS = "1.6.0"
MIME4J_VERS = "0.8.1"
-OW2_VERS = "9.0"
+OW2_VERS = "9.2"
AUTO_VALUE_VERSION = "1.7.4"
AUTO_VALUE_GSON_VERSION = "1.3.1"
PROLOG_VERS = "1.4.4"
@@ -114,25 +114,19 @@
maven_jar(
name = "log-api",
artifact = "org.slf4j:slf4j-api:" + SLF4J_VERS,
- sha1 = "77100a62c2e6f04b53977b9f541044d7d722693d",
+ sha1 = "d375aa1b98d34d5ddf73a3f19eaad66e98975b12",
)
maven_jar(
name = "log-ext",
artifact = "org.slf4j:slf4j-ext:" + SLF4J_VERS,
- sha1 = "31cdf122e000322e9efcb38913e9ab07825b17ef",
+ sha1 = "00da03640ae1ad57f964dcaa542fb5d804dce8a6",
)
maven_jar(
name = "jcl-over-slf4j",
artifact = "org.slf4j:jcl-over-slf4j:" + SLF4J_VERS,
- sha1 = "33fbc2d93de829fa5e263c5ce97f5eab8f57d53e",
- )
-
- maven_jar(
- name = "log4j",
- artifact = "log4j:log4j:1.2.17",
- sha1 = "5af35056b4d257e4b64b9e8069c0746e8b08629f",
+ sha1 = "28c441128bc81b6d95cc2857ae5bb46ae5bf658b",
)
maven_jar(
@@ -238,151 +232,151 @@
maven_jar(
name = "flexmark",
artifact = "com.vladsch.flexmark:flexmark:" + FLEXMARK_VERS,
- sha1 = "ed537d7bc31883b008cc17d243a691c7efd12a72",
+ sha1 = "7f61e190cff7e1bea64906408ccfa583b5ac9fc2",
)
maven_jar(
name = "flexmark-ext-abbreviation",
artifact = "com.vladsch.flexmark:flexmark-ext-abbreviation:" + FLEXMARK_VERS,
- sha1 = "dc27c3e7abbc8d2cfb154f41c68645c365bb9d22",
+ sha1 = "8f62f49cfaf8d33391e48a3b79e941d94e444e50",
)
maven_jar(
name = "flexmark-ext-anchorlink",
artifact = "com.vladsch.flexmark:flexmark-ext-anchorlink:" + FLEXMARK_VERS,
- sha1 = "6a8edb0165f695c9c19b7143a7fbd78c25c3b99c",
+ sha1 = "1d530e44b4439abf53ce9dcc784ffa5b9fd6ce89",
)
maven_jar(
name = "flexmark-ext-autolink",
artifact = "com.vladsch.flexmark:flexmark-ext-autolink:" + FLEXMARK_VERS,
- sha1 = "5da7a4d009ea08ef2d8714cc73e54a992c6d2d9a",
+ sha1 = "4026aa60fd6e146c2d4931acb19b2409c6a79b8a",
)
maven_jar(
name = "flexmark-ext-definition",
artifact = "com.vladsch.flexmark:flexmark-ext-definition:" + FLEXMARK_VERS,
- sha1 = "862d17812654624ed81ce8fc89c5ef819ff45f87",
+ sha1 = "bb74d36459e8e34653761aaa0095220b8b7b6cb6",
)
maven_jar(
name = "flexmark-ext-emoji",
artifact = "com.vladsch.flexmark:flexmark-ext-emoji:" + FLEXMARK_VERS,
- sha1 = "f0d7db64cb546798742b1ffc6db316a33f6acd76",
+ sha1 = "36d36cb227f831b81b636d3f901f07db06b8d84d",
)
maven_jar(
name = "flexmark-ext-escaped-character",
artifact = "com.vladsch.flexmark:flexmark-ext-escaped-character:" + FLEXMARK_VERS,
- sha1 = "6fd9ab77619df417df949721cb29c45914b326f8",
+ sha1 = "09f0cef3260b6f6371d6066cf32ce6a7dc2a7922",
)
maven_jar(
name = "flexmark-ext-footnotes",
artifact = "com.vladsch.flexmark:flexmark-ext-footnotes:" + FLEXMARK_VERS,
- sha1 = "e36bd69e43147cc6e19c3f55e4b27c0fc5a3d88c",
+ sha1 = "cdf0b79f026b09c6b87d91e61eb29ada1577aa5c",
)
maven_jar(
name = "flexmark-ext-gfm-issues",
artifact = "com.vladsch.flexmark:flexmark-ext-gfm-issues:" + FLEXMARK_VERS,
- sha1 = "5c825dd4e4fa4f7ccbe30dc92d7e35cdcb8a8c24",
+ sha1 = "e40d347e242e35d4344553120cb9e69c1c975f41",
)
maven_jar(
name = "flexmark-ext-gfm-strikethrough",
artifact = "com.vladsch.flexmark:flexmark-ext-gfm-strikethrough:" + FLEXMARK_VERS,
- sha1 = "3256735fd77e7228bf40f7888b4d3dc56787add4",
+ sha1 = "a6948f6e4fc2ec1059d6b53e73d4a30c24f6e05d",
)
maven_jar(
name = "flexmark-ext-gfm-tables",
artifact = "com.vladsch.flexmark:flexmark-ext-gfm-tables:" + FLEXMARK_VERS,
- sha1 = "62f0efcfb974756940ebe749fd4eb01323babc29",
+ sha1 = "92d3eb0c5dc79924448c186d89717f1df853aaa0",
)
maven_jar(
name = "flexmark-ext-gfm-tasklist",
artifact = "com.vladsch.flexmark:flexmark-ext-gfm-tasklist:" + FLEXMARK_VERS,
- sha1 = "76d4971ad9ce02f0e70351ab6bd06ad8e405e40d",
+ sha1 = "e6fb4d9e46c96e61a07fbb40cf653db58ed95a95",
)
maven_jar(
name = "flexmark-ext-gfm-users",
artifact = "com.vladsch.flexmark:flexmark-ext-gfm-users:" + FLEXMARK_VERS,
- sha1 = "7b0fc7e42e4da508da167fcf8e1cbf9ba7e21147",
+ sha1 = "44c83bdccfd41a399f3f7b11ba72728382dd3f5a",
)
maven_jar(
name = "flexmark-ext-ins",
artifact = "com.vladsch.flexmark:flexmark-ext-ins:" + FLEXMARK_VERS,
- sha1 = "9e51809867b9c4db0fb1c29599b4574e3d2a78e9",
+ sha1 = "b0d174d86ac2348420993dc2c997fad5f02c68fa",
)
maven_jar(
name = "flexmark-ext-jekyll-front-matter",
artifact = "com.vladsch.flexmark:flexmark-ext-jekyll-front-matter:" + FLEXMARK_VERS,
- sha1 = "44eb6dbb33b3831d3b40af938ddcd99c9c16a654",
+ sha1 = "2bfb31c67fd10af058b90f1b364f881f6276de80",
)
maven_jar(
name = "flexmark-ext-superscript",
artifact = "com.vladsch.flexmark:flexmark-ext-superscript:" + FLEXMARK_VERS,
- sha1 = "35815b8cb91000344d1fe5df21cacde8553d2994",
+ sha1 = "d210b007b46339082f79b6caf632b19ac8efc341",
)
maven_jar(
name = "flexmark-ext-tables",
artifact = "com.vladsch.flexmark:flexmark-ext-tables:" + FLEXMARK_VERS,
- sha1 = "f6768e98c7210b79d5e8bab76fff27eec6db51e6",
+ sha1 = "be77790470aa9bd90011067221504162a1d7b083",
)
maven_jar(
name = "flexmark-ext-toc",
artifact = "com.vladsch.flexmark:flexmark-ext-toc:" + FLEXMARK_VERS,
- sha1 = "1968d038fc6c8156f244f5a7eecb34e7e2f33705",
+ sha1 = "91d6d63ff5b70c3ebae98867bd7a346d71cb0ce1",
)
maven_jar(
name = "flexmark-ext-typographic",
artifact = "com.vladsch.flexmark:flexmark-ext-typographic:" + FLEXMARK_VERS,
- sha1 = "6549b9862b61c4434a855a733237103df9162849",
+ sha1 = "f51e247df80628df509a3af92a1efa1efb83d746",
)
maven_jar(
name = "flexmark-ext-wikilink",
artifact = "com.vladsch.flexmark:flexmark-ext-wikilink:" + FLEXMARK_VERS,
- sha1 = "e105b09dd35aab6e6f5c54dfe062ee59bd6f786a",
+ sha1 = "d14aeb078dbaf3e166621ae9499595a4a57b22ab",
)
maven_jar(
name = "flexmark-ext-yaml-front-matter",
artifact = "com.vladsch.flexmark:flexmark-ext-yaml-front-matter:" + FLEXMARK_VERS,
- sha1 = "b2d3a1e7f3985841062e8d3203617e29c6c21b52",
+ sha1 = "aba328b70e15f2553aac0a74e391edab37bf7b30",
)
maven_jar(
name = "flexmark-formatter",
artifact = "com.vladsch.flexmark:flexmark-formatter:" + FLEXMARK_VERS,
- sha1 = "a50c6cb10f6d623fc4354a572c583de1372d217f",
+ sha1 = "3e4ad3b1be29d41e8c35cadb70761768505f2164",
)
maven_jar(
name = "flexmark-html-parser",
artifact = "com.vladsch.flexmark:flexmark-html-parser:" + FLEXMARK_VERS,
- sha1 = "46c075f30017e131c1ada8538f1d8eacf652b044",
+ sha1 = "3c59b0061c50c9a8a48c46d4f4498d8eba249921",
)
maven_jar(
name = "flexmark-profile-pegdown",
artifact = "com.vladsch.flexmark:flexmark-profile-pegdown:" + FLEXMARK_VERS,
- sha1 = "d9aafd47629959cbeddd731f327ae090fc92b60f",
+ sha1 = "c01a2c2eebe06230858956d45847cee233790d7c",
)
maven_jar(
name = "flexmark-util",
artifact = "com.vladsch.flexmark:flexmark-util:" + FLEXMARK_VERS,
- sha1 = "417a9821d5d80ddacbfecadc6843ae7b259d5112",
+ sha1 = "fdfce72f5eb9ec53085804fd0c8d15f3680ae359",
)
# Transitive dependency of flexmark and gitiles
@@ -425,31 +419,31 @@
maven_jar(
name = "ow2-asm",
artifact = "org.ow2.asm:asm:" + OW2_VERS,
- sha1 = "af582ff60bc567c42d931500c3fdc20e0141ddf9",
+ sha1 = "81a03f76019c67362299c40e0ba13405f5467bff",
)
maven_jar(
name = "ow2-asm-analysis",
artifact = "org.ow2.asm:asm-analysis:" + OW2_VERS,
- sha1 = "4630afefbb43939c739445dde0af1a5729a0fb4e",
+ sha1 = "7487dd756daf96cab9986e44b9d7bcb796a61c10",
)
maven_jar(
name = "ow2-asm-commons",
artifact = "org.ow2.asm:asm-commons:" + OW2_VERS,
- sha1 = "5a34a3a9ac44f362f35d1b27932380b0031a3334",
+ sha1 = "f4d7f0fc9054386f2893b602454d48e07d4fbead",
)
maven_jar(
name = "ow2-asm-tree",
artifact = "org.ow2.asm:asm-tree:" + OW2_VERS,
- sha1 = "9df939f25c556b0c7efe00701d47e77a49837f24",
+ sha1 = "d96c99a30f5e1a19b0e609dbb19a44d8518ac01e",
)
maven_jar(
name = "ow2-asm-util",
artifact = "org.ow2.asm:asm-util:" + OW2_VERS,
- sha1 = "7c059a94ab5eed3347bf954e27fab58e52968848",
+ sha1 = "fbc178fc5ba3dab50fd7e8a5317b8b647c8e8946",
)
maven_jar(
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 272f822..612d897 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -17,6 +17,12 @@
"""
maven_jar(
+ name = "log4j",
+ artifact = "ch.qos.reload4j:reload4j:1.2.18.1",
+ sha1 = "7075022a11e18c1ad230de5be074e0c691fed17b",
+ )
+
+ maven_jar(
name = "j2objc",
artifact = "com.google.j2objc:j2objc-annotations:1.1",
sha1 = "ed28ded51a8b1c6b112568def5f4b455e6809019",
diff --git a/tools/platforms/Dockerfile b/tools/platforms/Dockerfile
new file mode 100644
index 0000000..157529c
--- /dev/null
+++ b/tools/platforms/Dockerfile
@@ -0,0 +1,21 @@
+# Copyright (C) 2021 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.
+
+FROM gcr.io/cloud-marketplace/google/rbe-ubuntu18-04:latest
+
+# Install Git >=2.18.0
+RUN add-apt-repository ppa:git-core/ppa && \
+ apt-get -y update && \
+ apt-get -y install git && \
+ apt-get clean
