Merge "Revert "Update rxjs to 7.8.0""
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 33c5bbd..cdc4833 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2209,7 +2209,6 @@
----
import com.google.gerrit.extensions.annotations.Listen;
import com.google.gerrit.extensions.webui.PatchSetWebLink;;
-import com.google.gerrit.extensions.webui.WebLinkTarget;
@Listen
public class MyWeblinkPlugin implements PatchSetWebLink {
@@ -2222,8 +2221,7 @@
String commitMessage, String branchName) {
return new WebLinkInfo(name,
imageUrl,
- String.format(placeHolderUrlProjectCommit, project, commit),
- WebLinkTarget.BLANK);
+ String.format(placeHolderUrlProjectCommit, project, commit));
}
}
----
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index fe901e7..a35f508 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -1385,6 +1385,33 @@
The change could not be rebased due to a path conflict during merge.
----
+Rebasing a change is allowed for the change owner, users with the
+link:access-control.html#category_rebase[Rebase] permission and users
+with the link:access-control.html#category_submit[Submit] permission.
+
+In addition, the rebaser or the original uploader, if rebasing is done
+on behalf of the uploader (see `rebase_on_behalf_of_uploader` option in
+link:#rebase-input[RebaseInput]), needs to have all permissions that
+are required to create the new patch set:
+
+* the link:access-control.html#category_push[Push] permission
+* the link:access-control.html#category_add_patch_set[Add Patch Set]
+ permission (only if the user is not the change owner)
+* the link:access-control.html#category_forge_author[Forge Author]
+ permission (only if the commit author is forged)
+* the link:access-control.html#category_forge_server[Forge Server]
+ permission (only if the commit author is the server identity)
+
+The same permissions were required for the upload of the original patch
+set. This means if the rebase is done on behalf of the uploader these
+permission checks should just pass, unless the uploader lost
+permissions after the upload of the original patch set. In this case
+rebasing on behalf of the uploader is not possible and a normal rebase
+(on behalf of the rebaser) must be done, which means that the rebaser
+becomes the uploader and takes over the change. If self approvals are
+disallowed, this means that the rebaser can no longer approve the
+change (as approvals of the uploader are ignored).
+
[[rebase-chain]]
=== Rebase Chain
--
@@ -7633,18 +7660,19 @@
[[diff-web-link-info]]
=== DiffWebLinkInfo
-The `DiffWebLinkInfo` entity describes a link on a diff screen to an
-external site.
+The `DiffWebLinkInfo` entity extends link:#web-link-info[WebLinkInfo] and
+describes a link on a diff screen to an external site.
-[options="header",cols="1,6"]
+[options="header",cols="1,^1,5"]
|=======================
-|Field Name|Description
-|`name` |The link name.
-|`url` |The link URL.
-|`image_url`|URL to the icon of the link.
-|show_on_side_by_side_diff_view|
+|Field Name||Description
+|`name` ||See link:#web-link-info[WebLinkInfo]
+|`tooltip` |optional|See link:#web-link-info[WebLinkInfo]
+|`url` ||See link:#web-link-info[WebLinkInfo]
+|`image_url`|optional|See link:#web-link-info[WebLinkInfo]
+|show_on_side_by_side_diff_view||
Whether the web link should be shown on the side-by-side diff screen.
-|show_on_unified_diff_view|
+|show_on_unified_diff_view||
Whether the web link should be shown on the unified diff screen.
|=======================
@@ -8925,15 +8953,23 @@
[[web-link-info]]
=== WebLinkInfo
-The `WebLinkInfo` entity describes a link to an external site.
+The `WebLinkInfo` entity describes a link to an external site. Depending on the
+context and the provided data the UI may decide to show the link as a text link,
+a linkified icon, or both.
+
+If the `tooltip` is not provided, then the UI may fall back to showing something
+like "Open in External Tool".
+
+Weblinks will always be opened in a new tab.
[options="header",cols="1,^1,5"]
|========================
|Field Name ||Description
-|`name` ||The link name.
+|`name` ||The text to be linkified.
+|`tooltip` |optional|Tooltip to show when hovering over the link. Using "Open
+in $NAME_OF_EXTERNAL_TOOL" is a good option here.
|`url` ||The link URL.
|`image_url`|optional|URL to the icon of the link.
-|`target` |optional|The target window in which the web link should be opened.
|========================
[[work-in-progress-input]]
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 44921ad..4293fd6 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -377,6 +377,10 @@
The link:http://www.brics.dk/automaton/[dk.brics.automaton library,role=external,window=_blank]
is used for the evaluation of such patterns.
+
+Note that the Gerrit host may not support regular expression search.
+You will then see an error dialog when using expressions starting with
+`^`.
++
The `^` required at the beginning of the regular expression not only
denotes a regular expression, but it also has the usual meaning of
anchoring the match to the start of the string. To match all Java
diff --git a/java/com/google/gerrit/acceptance/PushOneCommit.java b/java/com/google/gerrit/acceptance/PushOneCommit.java
index 5b1fa9b..9f38fcb 100644
--- a/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -41,6 +41,7 @@
import com.google.inject.assistedinject.AssistedInject;
import java.util.Arrays;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jgit.api.TagCommand;
@@ -490,12 +491,14 @@
public void assertMessage(String expectedMessage) {
RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref);
assertThat(refUpdate).isNotNull();
- assertThat(message(refUpdate).toLowerCase()).contains(expectedMessage.toLowerCase());
+ assertThat(message(refUpdate).toLowerCase(Locale.US))
+ .contains(expectedMessage.toLowerCase(Locale.US));
}
public void assertNotMessage(String message) {
RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref);
- assertThat(message(refUpdate).toLowerCase()).doesNotContain(message.toLowerCase());
+ assertThat(message(refUpdate).toLowerCase(Locale.US))
+ .doesNotContain(message.toLowerCase(Locale.US));
}
public String getMessage() {
diff --git a/java/com/google/gerrit/common/data/GlobalCapability.java b/java/com/google/gerrit/common/data/GlobalCapability.java
index 0a42d09..c7a9a63 100644
--- a/java/com/google/gerrit/common/data/GlobalCapability.java
+++ b/java/com/google/gerrit/common/data/GlobalCapability.java
@@ -22,6 +22,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Locale;
/**
* Server wide capabilities. Represented as {@link Permission} objects.
@@ -162,7 +163,7 @@
NAMES_LC = new ArrayList<>(NAMES_ALL.size());
for (String name : NAMES_ALL) {
- NAMES_LC.add(name.toLowerCase());
+ NAMES_LC.add(name.toLowerCase(Locale.US));
}
}
@@ -173,7 +174,7 @@
/** Returns true if the name is recognized as a capability name. */
public static boolean isGlobalCapability(String varName) {
- return NAMES_LC.contains(varName.toLowerCase());
+ return NAMES_LC.contains(varName.toLowerCase(Locale.US));
}
/** Returns true if the capability should have a range attached. */
diff --git a/java/com/google/gerrit/common/data/ParameterizedString.java b/java/com/google/gerrit/common/data/ParameterizedString.java
index 84bb535..c8c2b2b 100644
--- a/java/com/google/gerrit/common/data/ParameterizedString.java
+++ b/java/com/google/gerrit/common/data/ParameterizedString.java
@@ -19,6 +19,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
/** Performs replacements on strings such as <code>Hello ${user}</code>. */
@@ -213,7 +214,7 @@
new Function() {
@Override
String apply(String a) {
- return a.toLowerCase();
+ return a.toLowerCase(Locale.US);
}
});
m.put(
@@ -221,7 +222,7 @@
new Function() {
@Override
String apply(String a) {
- return a.toUpperCase();
+ return a.toUpperCase(Locale.US);
}
});
m.put(
diff --git a/java/com/google/gerrit/entities/EmailHeader.java b/java/com/google/gerrit/entities/EmailHeader.java
index e43b6a3..d3710c4 100644
--- a/java/com/google/gerrit/entities/EmailHeader.java
+++ b/java/com/google/gerrit/entities/EmailHeader.java
@@ -115,8 +115,8 @@
byte[] buf = new String(Character.toChars(cp)).getBytes(UTF_8);
for (byte b : buf) {
r.append('=');
- r.append(Integer.toHexString((b >>> 4) & 0x0f).toUpperCase());
- r.append(Integer.toHexString(b & 0x0f).toUpperCase());
+ r.append(Integer.toHexString((b >>> 4) & 0x0f).toUpperCase(Locale.US));
+ r.append(Integer.toHexString(b & 0x0f).toUpperCase(Locale.US));
}
} else {
diff --git a/java/com/google/gerrit/entities/LabelTypes.java b/java/com/google/gerrit/entities/LabelTypes.java
index a2f2e0b..fa7b741 100644
--- a/java/com/google/gerrit/entities/LabelTypes.java
+++ b/java/com/google/gerrit/entities/LabelTypes.java
@@ -19,6 +19,7 @@
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Optional;
@@ -38,11 +39,11 @@
}
public Optional<LabelType> byLabel(LabelId labelId) {
- return Optional.ofNullable(byLabel().get(labelId.get().toLowerCase()));
+ return Optional.ofNullable(byLabel().get(labelId.get().toLowerCase(Locale.US)));
}
public Optional<LabelType> byLabel(String labelName) {
- return Optional.ofNullable(byLabel().get(labelName.toLowerCase()));
+ return Optional.ofNullable(byLabel().get(labelName.toLowerCase(Locale.US)));
}
private Map<String, LabelType> byLabel() {
@@ -52,7 +53,7 @@
Map<String, LabelType> l = new HashMap<>();
if (labelTypes != null) {
for (LabelType t : labelTypes) {
- l.put(t.getName().toLowerCase(), t);
+ l.put(t.getName().toLowerCase(Locale.US), t);
}
}
byLabel = l;
diff --git a/java/com/google/gerrit/entities/Permission.java b/java/com/google/gerrit/entities/Permission.java
index 6a50711..2a34579 100644
--- a/java/com/google/gerrit/entities/Permission.java
+++ b/java/com/google/gerrit/entities/Permission.java
@@ -22,6 +22,7 @@
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
+import java.util.Locale;
import java.util.function.Consumer;
/** A single permission within an {@link AccessSection} of a project. */
@@ -64,37 +65,37 @@
static {
NAMES_LC = new ArrayList<>();
- NAMES_LC.add(ABANDON.toLowerCase());
- NAMES_LC.add(ADD_PATCH_SET.toLowerCase());
- NAMES_LC.add(CREATE.toLowerCase());
- NAMES_LC.add(CREATE_SIGNED_TAG.toLowerCase());
- NAMES_LC.add(CREATE_TAG.toLowerCase());
- NAMES_LC.add(DELETE.toLowerCase());
- NAMES_LC.add(DELETE_CHANGES.toLowerCase());
- NAMES_LC.add(DELETE_OWN_CHANGES.toLowerCase());
- NAMES_LC.add(EDIT_HASHTAGS.toLowerCase());
- NAMES_LC.add(EDIT_TOPIC_NAME.toLowerCase());
- NAMES_LC.add(FORGE_AUTHOR.toLowerCase());
- NAMES_LC.add(FORGE_COMMITTER.toLowerCase());
- NAMES_LC.add(FORGE_SERVER.toLowerCase());
- NAMES_LC.add(LABEL.toLowerCase());
- NAMES_LC.add(LABEL_AS.toLowerCase());
- NAMES_LC.add(REMOVE_LABEL.toLowerCase());
- NAMES_LC.add(OWNER.toLowerCase());
- NAMES_LC.add(PUSH.toLowerCase());
- NAMES_LC.add(PUSH_MERGE.toLowerCase());
- NAMES_LC.add(READ.toLowerCase());
- NAMES_LC.add(REBASE.toLowerCase());
- NAMES_LC.add(REMOVE_REVIEWER.toLowerCase());
- NAMES_LC.add(REVERT.toLowerCase());
- NAMES_LC.add(SUBMIT.toLowerCase());
- NAMES_LC.add(SUBMIT_AS.toLowerCase());
- NAMES_LC.add(TOGGLE_WORK_IN_PROGRESS_STATE.toLowerCase());
- NAMES_LC.add(VIEW_PRIVATE_CHANGES.toLowerCase());
+ NAMES_LC.add(ABANDON.toLowerCase(Locale.US));
+ NAMES_LC.add(ADD_PATCH_SET.toLowerCase(Locale.US));
+ NAMES_LC.add(CREATE.toLowerCase(Locale.US));
+ NAMES_LC.add(CREATE_SIGNED_TAG.toLowerCase(Locale.US));
+ NAMES_LC.add(CREATE_TAG.toLowerCase(Locale.US));
+ NAMES_LC.add(DELETE.toLowerCase(Locale.US));
+ NAMES_LC.add(DELETE_CHANGES.toLowerCase(Locale.US));
+ NAMES_LC.add(DELETE_OWN_CHANGES.toLowerCase(Locale.US));
+ NAMES_LC.add(EDIT_HASHTAGS.toLowerCase(Locale.US));
+ NAMES_LC.add(EDIT_TOPIC_NAME.toLowerCase(Locale.US));
+ NAMES_LC.add(FORGE_AUTHOR.toLowerCase(Locale.US));
+ NAMES_LC.add(FORGE_COMMITTER.toLowerCase(Locale.US));
+ NAMES_LC.add(FORGE_SERVER.toLowerCase(Locale.US));
+ NAMES_LC.add(LABEL.toLowerCase(Locale.US));
+ NAMES_LC.add(LABEL_AS.toLowerCase(Locale.US));
+ NAMES_LC.add(REMOVE_LABEL.toLowerCase(Locale.US));
+ NAMES_LC.add(OWNER.toLowerCase(Locale.US));
+ NAMES_LC.add(PUSH.toLowerCase(Locale.US));
+ NAMES_LC.add(PUSH_MERGE.toLowerCase(Locale.US));
+ NAMES_LC.add(READ.toLowerCase(Locale.US));
+ NAMES_LC.add(REBASE.toLowerCase(Locale.US));
+ NAMES_LC.add(REMOVE_REVIEWER.toLowerCase(Locale.US));
+ NAMES_LC.add(REVERT.toLowerCase(Locale.US));
+ NAMES_LC.add(SUBMIT.toLowerCase(Locale.US));
+ NAMES_LC.add(SUBMIT_AS.toLowerCase(Locale.US));
+ NAMES_LC.add(TOGGLE_WORK_IN_PROGRESS_STATE.toLowerCase(Locale.US));
+ NAMES_LC.add(VIEW_PRIVATE_CHANGES.toLowerCase(Locale.US));
LABEL_INDEX = NAMES_LC.indexOf(Permission.LABEL);
- LABEL_AS_INDEX = NAMES_LC.indexOf(Permission.LABEL_AS.toLowerCase());
- REMOVE_LABEL_INDEX = NAMES_LC.indexOf(Permission.REMOVE_LABEL.toLowerCase());
+ LABEL_AS_INDEX = NAMES_LC.indexOf(Permission.LABEL_AS.toLowerCase(Locale.US));
+ REMOVE_LABEL_INDEX = NAMES_LC.indexOf(Permission.REMOVE_LABEL.toLowerCase(Locale.US));
}
/** Returns true if the name is recognized as a permission name. */
@@ -102,7 +103,7 @@
return isLabel(varName)
|| isLabelAs(varName)
|| isRemoveLabel(varName)
- || NAMES_LC.contains(varName.toLowerCase());
+ || NAMES_LC.contains(varName.toLowerCase(Locale.US));
}
public static boolean hasRange(String varName) {
@@ -226,7 +227,7 @@
return REMOVE_LABEL_INDEX;
}
- int index = NAMES_LC.indexOf(a.getName().toLowerCase());
+ int index = NAMES_LC.indexOf(a.getName().toLowerCase(Locale.US));
return 0 <= index ? index : NAMES_LC.size();
}
diff --git a/java/com/google/gerrit/extensions/client/GerritTopMenu.java b/java/com/google/gerrit/extensions/client/GerritTopMenu.java
index b7e1a5a..b9a395e 100644
--- a/java/com/google/gerrit/extensions/client/GerritTopMenu.java
+++ b/java/com/google/gerrit/extensions/client/GerritTopMenu.java
@@ -14,6 +14,8 @@
package com.google.gerrit.extensions.client;
+import java.util.Locale;
+
public enum GerritTopMenu {
ALL,
MY,
@@ -25,6 +27,6 @@
public final String menuName;
GerritTopMenu() {
- menuName = name().substring(0, 1) + name().substring(1).toLowerCase();
+ menuName = name().substring(0, 1) + name().substring(1).toLowerCase(Locale.US);
}
}
diff --git a/java/com/google/gerrit/extensions/common/DiffWebLinkInfo.java b/java/com/google/gerrit/extensions/common/DiffWebLinkInfo.java
index 9bcf2cf..05029be 100644
--- a/java/com/google/gerrit/extensions/common/DiffWebLinkInfo.java
+++ b/java/com/google/gerrit/extensions/common/DiffWebLinkInfo.java
@@ -18,29 +18,26 @@
public Boolean showOnSideBySideDiffView;
public Boolean showOnUnifiedDiffView;
- public static DiffWebLinkInfo forSideBySideDiffView(
- String name, String imageUrl, String url, String target) {
- return new DiffWebLinkInfo(name, imageUrl, url, target, true, false);
+ public static DiffWebLinkInfo forSideBySideDiffView(String name, String imageUrl, String url) {
+ return new DiffWebLinkInfo(name, imageUrl, url, true, false);
}
- public static DiffWebLinkInfo forUnifiedDiffView(
- String name, String imageUrl, String url, String target) {
- return new DiffWebLinkInfo(name, imageUrl, url, target, false, true);
+ public static DiffWebLinkInfo forUnifiedDiffView(String name, String imageUrl, String url) {
+ return new DiffWebLinkInfo(name, imageUrl, url, false, true);
}
public static DiffWebLinkInfo forSideBySideAndUnifiedDiffView(
- String name, String imageUrl, String url, String target) {
- return new DiffWebLinkInfo(name, imageUrl, url, target, true, true);
+ String name, String imageUrl, String url) {
+ return new DiffWebLinkInfo(name, imageUrl, url, true, true);
}
private DiffWebLinkInfo(
String name,
String imageUrl,
String url,
- String target,
boolean showOnSideBySideDiffView,
boolean showOnUnifiedDiffView) {
- super(name, imageUrl, url, target);
+ super(name, imageUrl, url);
this.showOnSideBySideDiffView = showOnSideBySideDiffView ? true : null;
this.showOnUnifiedDiffView = showOnUnifiedDiffView ? true : null;
}
diff --git a/java/com/google/gerrit/extensions/common/WebLinkInfo.java b/java/com/google/gerrit/extensions/common/WebLinkInfo.java
index ba12be0..fbd8d2f 100644
--- a/java/com/google/gerrit/extensions/common/WebLinkInfo.java
+++ b/java/com/google/gerrit/extensions/common/WebLinkInfo.java
@@ -14,24 +14,18 @@
package com.google.gerrit.extensions.common;
-import com.google.gerrit.extensions.webui.WebLink.Target;
import java.util.Objects;
public class WebLinkInfo {
public String name;
+ public String tooltip;
public String imageUrl;
public String url;
- public String target;
- public WebLinkInfo(String name, String imageUrl, String url, String target) {
+ public WebLinkInfo(String name, String imageUrl, String url) {
this.name = name;
this.imageUrl = imageUrl;
this.url = url;
- this.target = target;
- }
-
- public WebLinkInfo(String name, String imageUrl, String url) {
- this(name, imageUrl, url, Target.SELF);
}
@Override
@@ -41,14 +35,14 @@
}
WebLinkInfo i = (WebLinkInfo) o;
return Objects.equals(name, i.name)
+ && Objects.equals(tooltip, i.tooltip)
&& Objects.equals(imageUrl, i.imageUrl)
- && Objects.equals(url, i.url)
- && Objects.equals(target, i.target);
+ && Objects.equals(url, i.url);
}
@Override
public int hashCode() {
- return Objects.hash(name, imageUrl, url, target);
+ return Objects.hash(name, tooltip, imageUrl, url);
}
@Override
@@ -56,12 +50,12 @@
return getClass().getSimpleName()
+ "{name="
+ name
+ + ", tooltip="
+ + tooltip
+ ", imageUrl="
+ imageUrl
+ ", url="
+ url
- + ", target"
- + target
+ "}";
}
diff --git a/java/com/google/gerrit/extensions/webui/WebLink.java b/java/com/google/gerrit/extensions/webui/WebLink.java
index 7cbeff2..36ae50c 100644
--- a/java/com/google/gerrit/extensions/webui/WebLink.java
+++ b/java/com/google/gerrit/extensions/webui/WebLink.java
@@ -15,16 +15,4 @@
package com.google.gerrit.extensions.webui;
/** Marks that the implementor has a method that provides a weblinkInfo */
-public interface WebLink {
- /** Class that holds target defaults for WebLink anchors. */
- class Target {
- /** Opens the link in a new window or tab */
- public static final String BLANK = "_blank";
- /** Opens the link in the frame it was clicked. */
- public static final String SELF = "_self";
- /** Opens link in parent frame. */
- public static final String PARENT = "_parent";
- /** Opens link in the full body of the window. */
- public static final String TOP = "_top";
- }
-}
+public interface WebLink {}
diff --git a/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java b/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
index 71dff97..fff4045 100644
--- a/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
+++ b/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
@@ -37,6 +37,7 @@
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@@ -82,7 +83,7 @@
if (strs.length != 0) {
Map<Long, Fingerprint> fps = Maps.newHashMapWithExpectedSize(strs.length);
for (String str : strs) {
- str = CharMatcher.whitespace().removeFrom(str).toUpperCase();
+ str = CharMatcher.whitespace().removeFrom(str).toUpperCase(Locale.US);
Fingerprint fp = new Fingerprint(BaseEncoding.base16().decode(str));
fps.put(fp.getId(), fp);
}
diff --git a/java/com/google/gerrit/gpg/server/GpgKeys.java b/java/com/google/gerrit/gpg/server/GpgKeys.java
index b3a2f53..00a0f57 100644
--- a/java/com/google/gerrit/gpg/server/GpgKeys.java
+++ b/java/com/google/gerrit/gpg/server/GpgKeys.java
@@ -48,6 +48,7 @@
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
+import java.util.Locale;
import java.util.Map;
import org.bouncycastle.bcpg.ArmoredOutputStream;
import org.bouncycastle.openpgp.PGPException;
@@ -106,7 +107,7 @@
static ExternalId findGpgKey(String str, Iterable<ExternalId> existingExtIds)
throws ResourceNotFoundException {
- str = CharMatcher.whitespace().removeFrom(str).toUpperCase();
+ str = CharMatcher.whitespace().removeFrom(str).toUpperCase(Locale.US);
if ((str.length() != 8 && str.length() != 40)
|| !CharMatcher.anyOf("0123456789ABCDEF").matchesAllOf(str)) {
throw new ResourceNotFoundException(str);
diff --git a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
index e909701..d6718ca 100644
--- a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
+++ b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
@@ -79,6 +79,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@@ -159,7 +160,7 @@
if (!_env.envMap.containsKey("SystemRoot")) {
String os = System.getProperty("os.name");
- if (os != null && os.toLowerCase().contains("windows")) {
+ if (os != null && os.toLowerCase(Locale.US).contains("windows")) {
String sysroot = System.getenv("SystemRoot");
if (sysroot == null || sysroot.isEmpty()) {
sysroot = "C:\\WINDOWS";
@@ -576,7 +577,7 @@
for (String name : getHeaderNames(req)) {
final String value = req.getHeader(name);
- env.set("HTTP_" + name.toUpperCase().replace('-', '_'), value);
+ env.set("HTTP_" + name.toUpperCase(Locale.US).replace('-', '_'), value);
}
Project.NameKey nameKey = projectState.getNameKey();
diff --git a/java/com/google/gerrit/index/IndexType.java b/java/com/google/gerrit/index/IndexType.java
index 75f8351..d8c8f6a 100644
--- a/java/com/google/gerrit/index/IndexType.java
+++ b/java/com/google/gerrit/index/IndexType.java
@@ -19,6 +19,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
+import java.util.Locale;
import java.util.Optional;
/**
@@ -51,7 +52,7 @@
if (Strings.isNullOrEmpty(value)) {
return Optional.empty();
}
- value = value.toUpperCase().replace("-", "_");
+ value = value.toUpperCase(Locale.US).replace("-", "_");
IndexType type = new IndexType(value);
if (!Strings.isNullOrEmpty(System.getenv(ENV_VAR))) {
checkArgument(
@@ -67,7 +68,7 @@
}
public IndexType(@Nullable String type) {
- this.type = type == null ? getDefault() : type.toLowerCase();
+ this.type = type == null ? getDefault() : type.toLowerCase(Locale.US);
}
public static String getDefault() {
diff --git a/java/com/google/gerrit/index/IndexedField.java b/java/com/google/gerrit/index/IndexedField.java
index 99004bb..94943d6 100644
--- a/java/com/google/gerrit/index/IndexedField.java
+++ b/java/com/google/gerrit/index/IndexedField.java
@@ -36,6 +36,7 @@
import java.lang.reflect.Type;
import java.sql.Timestamp;
import java.util.HashMap;
+import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.stream.StreamSupport;
@@ -351,7 +352,8 @@
private static String checkName(String name) {
String allowedCharacters = "abcdefghijklmnopqrstuvwxyz0123456789_";
- CharMatcher m = CharMatcher.anyOf(allowedCharacters + allowedCharacters.toUpperCase());
+ CharMatcher m =
+ CharMatcher.anyOf(allowedCharacters + allowedCharacters.toUpperCase(Locale.US));
checkArgument(name != null && m.matchesAllOf(name), "illegal field name: %s", name);
return name;
}
diff --git a/java/com/google/gerrit/index/query/IndexPredicate.java b/java/com/google/gerrit/index/query/IndexPredicate.java
index de81c47..0bde640 100644
--- a/java/com/google/gerrit/index/query/IndexPredicate.java
+++ b/java/com/google/gerrit/index/query/IndexPredicate.java
@@ -23,6 +23,7 @@
import com.google.common.primitives.Longs;
import com.google.gerrit.index.FieldType;
import com.google.gerrit.index.SchemaFieldDefs.SchemaField;
+import java.util.Locale;
import java.util.Objects;
import java.util.Set;
import java.util.stream.StreamSupport;
@@ -117,7 +118,8 @@
}
private static ImmutableSet<String> tokenizeString(String value) {
- return StreamSupport.stream(FULL_TEXT_SPLITTER.split(value.toLowerCase()).spliterator(), false)
+ return StreamSupport.stream(
+ FULL_TEXT_SPLITTER.split(value.toLowerCase(Locale.US)).spliterator(), false)
.filter(s -> !s.trim().isEmpty())
.collect(toImmutableSet());
}
diff --git a/java/com/google/gerrit/launcher/GerritLauncher.java b/java/com/google/gerrit/launcher/GerritLauncher.java
index 6be78d9..8783593 100644
--- a/java/com/google/gerrit/launcher/GerritLauncher.java
+++ b/java/com/google/gerrit/launcher/GerritLauncher.java
@@ -44,6 +44,7 @@
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.NavigableMap;
import java.util.Properties;
@@ -195,7 +196,7 @@
String cn = programClassName(name);
clazz = Class.forName(PKG + "." + cn, true, loader);
} catch (ClassNotFoundException cnfe) {
- if (name.equals(name.toLowerCase())) {
+ if (name.equals(name.toLowerCase(Locale.US))) {
clazz = Class.forName(PKG + "." + name, true, loader);
} else {
throw cnfe;
@@ -239,7 +240,7 @@
}
private static String programClassName(String cn) {
- if (cn.equals(cn.toLowerCase())) {
+ if (cn.equals(cn.toLowerCase(Locale.US))) {
StringBuilder buf = new StringBuilder();
buf.append(Character.toUpperCase(cn.charAt(0)));
for (int i = 1; i < cn.length(); i++) {
diff --git a/java/com/google/gerrit/mail/RawMailParser.java b/java/com/google/gerrit/mail/RawMailParser.java
index 929e9f9..79d1cb8f 100644
--- a/java/com/google/gerrit/mail/RawMailParser.java
+++ b/java/com/google/gerrit/mail/RawMailParser.java
@@ -26,6 +26,7 @@
import java.io.IOException;
import java.io.InputStreamReader;
import java.time.Instant;
+import java.util.Locale;
import org.apache.james.mime4j.MimeException;
import org.apache.james.mime4j.dom.Entity;
import org.apache.james.mime4j.dom.Message;
@@ -90,7 +91,7 @@
// Add additional headers
mimeMessage.getHeader().getFields().stream()
- .filter(f -> !MAIN_HEADERS.contains(f.getName().toLowerCase()))
+ .filter(f -> !MAIN_HEADERS.contains(f.getName().toLowerCase(Locale.US)))
.forEach(f -> messageBuilder.addAdditionalHeader(f.getName() + ": " + f.getBody()));
// Add text and html body parts
diff --git a/java/com/google/gerrit/pgm/init/api/ConsoleUI.java b/java/com/google/gerrit/pgm/init/api/ConsoleUI.java
index 7666076..865f7d7 100644
--- a/java/com/google/gerrit/pgm/init/api/ConsoleUI.java
+++ b/java/com/google/gerrit/pgm/init/api/ConsoleUI.java
@@ -20,6 +20,7 @@
import com.google.gerrit.common.Nullable;
import java.io.Console;
import java.util.EnumSet;
+import java.util.Locale;
import java.util.Set;
/** Console based interaction with the invoking user. */
@@ -165,15 +166,15 @@
String def, Set<String> allowedValues, @FormatString String fmt, Object... args) {
for (; ; ) {
String r = readString(def, fmt, args);
- if (allowedValues.contains(r.toLowerCase())) {
- return r.toLowerCase();
+ if (allowedValues.contains(r.toLowerCase(Locale.US))) {
+ return r.toLowerCase(Locale.US);
}
if (!"?".equals(r)) {
console.printf("error: '%s' is not a valid choice\n", r);
}
console.printf(" Supported options are:\n");
for (String v : allowedValues) {
- console.printf(" %s\n", v.toLowerCase());
+ console.printf(" %s\n", v.toLowerCase(Locale.US));
}
}
}
@@ -210,7 +211,8 @@
T def, A options, String fmt, Object... args) {
final String prompt = String.format(fmt, args);
for (; ; ) {
- String r = console.readLine("%-30s [%s/?]: ", prompt, def.toString().toLowerCase());
+ String r =
+ console.readLine("%-30s [%s/?]: ", prompt, def.toString().toLowerCase(Locale.US));
if (r == null) {
throw abort();
}
@@ -228,7 +230,7 @@
}
console.printf(" Supported options are:\n");
for (T e : options) {
- console.printf(" %s\n", e.toString().toLowerCase());
+ console.printf(" %s\n", e.toString().toLowerCase(Locale.US));
}
}
}
diff --git a/java/com/google/gerrit/pgm/util/AbstractProgram.java b/java/com/google/gerrit/pgm/util/AbstractProgram.java
index 96b042a..00cba31 100644
--- a/java/com/google/gerrit/pgm/util/AbstractProgram.java
+++ b/java/com/google/gerrit/pgm/util/AbstractProgram.java
@@ -18,6 +18,7 @@
import com.google.gerrit.util.cli.CmdLineParser;
import com.google.gerrit.util.cli.OptionHandlers;
import java.io.StringWriter;
+import java.util.Locale;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.Option;
@@ -35,7 +36,7 @@
if (0 < dot) {
n = n.substring(dot + 1);
}
- return n.toLowerCase();
+ return n.toLowerCase(Locale.US);
}
public final int main(String[] argv) throws Exception {
diff --git a/java/com/google/gerrit/server/ChangeUtil.java b/java/com/google/gerrit/server/ChangeUtil.java
index 2c5b539..2265055 100644
--- a/java/com/google/gerrit/server/ChangeUtil.java
+++ b/java/com/google/gerrit/server/ChangeUtil.java
@@ -31,6 +31,7 @@
import java.security.SecureRandom;
import java.util.Collection;
import java.util.List;
+import java.util.Locale;
import java.util.Optional;
import java.util.Random;
import java.util.Set;
@@ -149,7 +150,7 @@
}
public static String status(Change c) {
- return c != null ? c.getStatus().name().toLowerCase() : "deleted";
+ return c != null ? c.getStatus().name().toLowerCase(Locale.US) : "deleted";
}
private static final Pattern LINK_CHANGE_ID_PATTERN = Pattern.compile("I[0-9a-f]{40}");
diff --git a/java/com/google/gerrit/server/approval/testing/TestPatchSetApprovalUuidGenerator.java b/java/com/google/gerrit/server/approval/testing/TestPatchSetApprovalUuidGenerator.java
index 89727c7..676640d 100644
--- a/java/com/google/gerrit/server/approval/testing/TestPatchSetApprovalUuidGenerator.java
+++ b/java/com/google/gerrit/server/approval/testing/TestPatchSetApprovalUuidGenerator.java
@@ -20,6 +20,7 @@
import com.google.gerrit.entities.PatchSetApproval.UUID;
import com.google.gerrit.server.approval.PatchSetApprovalUuidGenerator;
import java.time.Instant;
+import java.util.Locale;
import javax.inject.Singleton;
/**
@@ -44,6 +45,6 @@
value,
invocationCount)
.replace("-", "_")
- .toLowerCase());
+ .toLowerCase(Locale.US));
}
}
diff --git a/java/com/google/gerrit/server/cache/PerThreadProjectCache.java b/java/com/google/gerrit/server/cache/PerThreadProjectCache.java
index 86f1d2d..8394343 100644
--- a/java/com/google/gerrit/server/cache/PerThreadProjectCache.java
+++ b/java/com/google/gerrit/server/cache/PerThreadProjectCache.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.cache;
import com.google.common.collect.Maps;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.entities.Project;
import java.util.Map;
import java.util.function.Supplier;
@@ -39,6 +40,7 @@
private PerThreadProjectCache() {}
+ @CanIgnoreReturnValue
public static <T> T getOrCompute(PerThreadCache.Key<Project.NameKey> key, Supplier<T> loader) {
PerThreadCache perThreadCache = PerThreadCache.get();
if (perThreadCache != null) {
diff --git a/java/com/google/gerrit/server/change/ArchiveFormatInternal.java b/java/com/google/gerrit/server/change/ArchiveFormatInternal.java
index f6e9ff9..0ed1f11 100644
--- a/java/com/google/gerrit/server/change/ArchiveFormatInternal.java
+++ b/java/com/google/gerrit/server/change/ArchiveFormatInternal.java
@@ -17,6 +17,7 @@
import java.io.Closeable;
import java.io.IOException;
import java.io.OutputStream;
+import java.util.Locale;
import org.apache.commons.compress.archivers.ArchiveOutputStream;
import org.eclipse.jgit.api.ArchiveCommand;
import org.eclipse.jgit.api.ArchiveCommand.Format;
@@ -47,7 +48,7 @@
}
public String getShortName() {
- return name().toLowerCase();
+ return name().toLowerCase(Locale.US);
}
public String getMimeType() {
diff --git a/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java b/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java
index 7fd075e..5e6a520 100644
--- a/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java
+++ b/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java
@@ -18,6 +18,7 @@
import com.google.common.collect.Multimap;
import java.util.Collections;
import java.util.LinkedHashSet;
+import java.util.Locale;
import java.util.Objects;
import java.util.Set;
import org.apache.commons.lang3.StringUtils;
@@ -139,7 +140,7 @@
@Override
public String toString() {
- return StringUtils.capitalize(name().toLowerCase());
+ return StringUtils.capitalize(name().toLowerCase(Locale.US));
}
}
diff --git a/java/com/google/gerrit/server/config/DownloadConfig.java b/java/com/google/gerrit/server/config/DownloadConfig.java
index 8ac858c..4fdbd4a 100644
--- a/java/com/google/gerrit/server/config/DownloadConfig.java
+++ b/java/com/google/gerrit/server/config/DownloadConfig.java
@@ -29,6 +29,7 @@
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
+import java.util.Locale;
import java.util.Set;
import org.eclipse.jgit.lib.Config;
@@ -101,7 +102,7 @@
@Nullable
private static String toCoreScheme(String s) {
try {
- Field f = CoreDownloadSchemes.class.getField(s.toUpperCase());
+ Field f = CoreDownloadSchemes.class.getField(s.toUpperCase(Locale.US));
int m = Modifier.PUBLIC | Modifier.STATIC | Modifier.FINAL;
if ((f.getModifiers() & m) == m && f.getType() == String.class) {
return (String) f.get(null);
diff --git a/java/com/google/gerrit/server/config/GitwebConfig.java b/java/com/google/gerrit/server/config/GitwebConfig.java
index 0a213b4..f8c0592 100644
--- a/java/com/google/gerrit/server/config/GitwebConfig.java
+++ b/java/com/google/gerrit/server/config/GitwebConfig.java
@@ -383,7 +383,9 @@
}
private WebLinkInfo link(String rest) {
- return new WebLinkInfo(type.getLinkName(), null, url + rest, null);
+ WebLinkInfo webLink = new WebLinkInfo(type.getLinkName(), null, url + rest);
+ webLink.tooltip = "Open in GitWeb";
+ return webLink;
}
@Nullable
diff --git a/java/com/google/gerrit/server/git/MultiProgressMonitor.java b/java/com/google/gerrit/server/git/MultiProgressMonitor.java
index c76c78e..ab5c988 100644
--- a/java/com/google/gerrit/server/git/MultiProgressMonitor.java
+++ b/java/com/google/gerrit/server/git/MultiProgressMonitor.java
@@ -170,6 +170,11 @@
public String getTotalDisplay(int total) {
return String.valueOf(total);
}
+
+ @Override
+ public void showDuration(boolean enabled) {
+ // not implemented
+ }
}
/** Handle for a sub-task whose total work can be updated while the task is in progress. */
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 7b8c0d2..0e9f7ca 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -195,6 +195,7 @@
import com.google.gerrit.server.update.SuperprojectUpdateOnSubmission;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.update.context.RefUpdateContext;
+import com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType;
import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.server.util.MagicBranch;
import com.google.gerrit.server.util.RequestScopePropagator;
@@ -891,7 +892,10 @@
case UPDATE:
case UPDATE_NONFASTFORWARD:
Task closeProgress = progress.beginSubTask("closed", UNKNOWN);
- autoCloseChanges(c, closeProgress);
+ try (RefUpdateContext ctx =
+ RefUpdateContext.open(RefUpdateType.AUTO_CLOSE_CHANGES)) {
+ autoCloseChanges(c, closeProgress);
+ }
closeProgress.end();
break;
diff --git a/java/com/google/gerrit/server/index/account/AccountField.java b/java/com/google/gerrit/server/index/account/AccountField.java
index ed58a0b..cc86ffc 100644
--- a/java/com/google/gerrit/server/index/account/AccountField.java
+++ b/java/com/google/gerrit/server/index/account/AccountField.java
@@ -150,7 +150,7 @@
.build(
a -> {
String preferredEmail = a.account().preferredEmail();
- return preferredEmail != null ? preferredEmail.toLowerCase() : null;
+ return preferredEmail != null ? preferredEmail.toLowerCase(Locale.US) : null;
});
public static final IndexedField<AccountState, String>.SearchSpec
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 92e722d..1d5818a 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -1042,7 +1042,7 @@
public static String formatLabel(
String label, int value, @Nullable Account.Id accountId, @Nullable Integer count) {
- return label.toLowerCase()
+ return label.toLowerCase(Locale.US)
+ (value >= 0 ? "+" : "")
+ value
+ (accountId != null ? "," + formatAccount(accountId) : "")
@@ -1055,7 +1055,7 @@
public static String formatLabel(
String label, String value, @Nullable Account.Id accountId, @Nullable Integer count) {
- return label.toLowerCase()
+ return label.toLowerCase(Locale.US)
+ "="
+ value
+ (accountId != null ? "," + formatAccount(accountId) : "")
@@ -1567,7 +1567,7 @@
continue;
}
for (SubmitRecord.Label label : rec.labels) {
- String sl = label.status.toString() + ',' + label.label.toLowerCase();
+ String sl = label.status.toString() + ',' + label.label.toLowerCase(Locale.US);
result.add(sl);
String slc = sl + ',';
if (label.appliedBy != null) {
@@ -1596,28 +1596,28 @@
result.add(
SubmitRecord.Label.Status.OK.name()
+ ","
- + srResult.submitRequirement().name().toLowerCase());
+ + srResult.submitRequirement().name().toLowerCase(Locale.US));
result.add(
SubmitRecord.Label.Status.MAY.name()
+ ","
- + srResult.submitRequirement().name().toLowerCase());
+ + srResult.submitRequirement().name().toLowerCase(Locale.US));
break;
case UNSATISFIED:
result.add(
SubmitRecord.Label.Status.NEED.name()
+ ","
- + srResult.submitRequirement().name().toLowerCase());
+ + srResult.submitRequirement().name().toLowerCase(Locale.US));
result.add(
SubmitRecord.Label.Status.REJECT.name()
+ ","
- + srResult.submitRequirement().name().toLowerCase());
+ + srResult.submitRequirement().name().toLowerCase(Locale.US));
break;
case NOT_APPLICABLE:
case ERROR:
result.add(
SubmitRecord.Label.Status.IMPOSSIBLE.name()
+ ","
- + srResult.submitRequirement().name().toLowerCase());
+ + srResult.submitRequirement().name().toLowerCase(Locale.US));
}
}
return result;
diff --git a/java/com/google/gerrit/server/ioutil/HostPlatform.java b/java/com/google/gerrit/server/ioutil/HostPlatform.java
index e27d17c..b912c52 100644
--- a/java/com/google/gerrit/server/ioutil/HostPlatform.java
+++ b/java/com/google/gerrit/server/ioutil/HostPlatform.java
@@ -16,6 +16,7 @@
import java.security.AccessController;
import java.security.PrivilegedAction;
+import java.util.Locale;
public final class HostPlatform {
private static final boolean win32 = compute("windows");
@@ -34,7 +35,7 @@
final String osDotName =
AccessController.doPrivileged(
(PrivilegedAction<String>) () -> System.getProperty("os.name"));
- return osDotName != null && osDotName.toLowerCase().contains(platform);
+ return osDotName != null && osDotName.toLowerCase(Locale.US).contains(platform);
}
private HostPlatform() {}
diff --git a/java/com/google/gerrit/server/mail/send/AddKeySender.java b/java/com/google/gerrit/server/mail/send/AddKeySender.java
index 6fe3cbe..73a46a4 100644
--- a/java/com/google/gerrit/server/mail/send/AddKeySender.java
+++ b/java/com/google/gerrit/server/mail/send/AddKeySender.java
@@ -68,7 +68,7 @@
super.init();
setHeader("Subject", String.format("[Gerrit Code Review] New %s Keys Added", getKeyType()));
setMessageId(messageIdGenerator.fromAccountUpdate(user.getAccountId()));
- add(RecipientType.TO, user.getAccountId());
+ addByAccountId(RecipientType.TO, user.getAccountId());
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 996b8f0..ec81cf6 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -24,6 +24,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Address;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeSizeBucket;
@@ -214,11 +215,10 @@
if (notify.handling().equals(NotifyHandling.OWNER_REVIEWERS)
|| notify.handling().equals(NotifyHandling.ALL)) {
try {
- addByEmail(
- RecipientType.CC, changeData.reviewersByEmail().byState(ReviewerStateInternal.CC));
- addByEmail(
- RecipientType.CC,
- changeData.reviewersByEmail().byState(ReviewerStateInternal.REVIEWER));
+ changeData.reviewersByEmail().byState(ReviewerStateInternal.CC).stream()
+ .forEach(address -> addByEmail(RecipientType.CC, address));
+ changeData.reviewersByEmail().byState(ReviewerStateInternal.REVIEWER).stream()
+ .forEach(address -> addByEmail(RecipientType.CC, address));
} catch (StorageException e) {
throw new EmailException("Failed to add unregistered CCs " + change.getChangeId(), e);
}
@@ -376,9 +376,9 @@
}
/** TO or CC all vested parties (change owner, patch set uploader, author). */
- protected void rcptToAuthors(RecipientType rt) {
+ protected void addAuthors(RecipientType rt) {
for (Account.Id id : authors) {
- add(rt, id);
+ addByAccountId(rt, id);
}
}
@@ -390,7 +390,7 @@
for (Map.Entry<Account.Id, Collection<String>> e : stars.asMap().entrySet()) {
if (e.getValue().contains(StarredChangesUtil.DEFAULT_LABEL)) {
- super.add(RecipientType.BCC, e.getKey());
+ super.addByAccountId(RecipientType.BCC, e.getKey());
}
}
}
@@ -414,7 +414,7 @@
try {
for (Account.Id id : changeData.reviewers().all()) {
- add(RecipientType.CC, id);
+ addByAccountId(RecipientType.CC, id);
}
} catch (StorageException err) {
logger.atWarning().withCause(err).log("Cannot CC users that reviewed updated change");
@@ -430,7 +430,7 @@
try {
for (Account.Id id : changeData.reviewers().byState(ReviewerStateInternal.REVIEWER)) {
- add(RecipientType.CC, id);
+ addByAccountId(RecipientType.CC, id);
}
} catch (StorageException err) {
logger.atWarning().withCause(err).log("Cannot CC users that commented on updated change");
@@ -438,7 +438,7 @@
}
@Override
- protected void add(RecipientType rt, Account.Id to) {
+ protected void addByAccountId(RecipientType rt, Account.Id to) {
addRecipient(rt, to, /* isWatcher= */ false);
}
@@ -462,11 +462,22 @@
if (emailOnlyAuthors && !authors.contains(to)) {
return;
}
- super.add(rt, to);
+ super.addByAccountId(rt, to);
}
@Override
- protected boolean isVisibleTo(Account.Id to) throws PermissionBackendException {
+ protected boolean isRecipientAllowed(Address addr) throws PermissionBackendException {
+ if (!projectState.statePermitsRead()) {
+ return false;
+ }
+ return args.permissionBackend
+ .user(args.anonymousUser.get())
+ .change(changeData)
+ .test(ChangePermission.READ);
+ }
+
+ @Override
+ protected boolean isRecipientAllowed(Account.Id to) throws PermissionBackendException {
if (!projectState.statePermitsRead()) {
return false;
}
diff --git a/java/com/google/gerrit/server/mail/send/DeleteKeySender.java b/java/com/google/gerrit/server/mail/send/DeleteKeySender.java
index 64a01ff..22c26b1 100644
--- a/java/com/google/gerrit/server/mail/send/DeleteKeySender.java
+++ b/java/com/google/gerrit/server/mail/send/DeleteKeySender.java
@@ -71,7 +71,7 @@
super.init();
setHeader("Subject", String.format("[Gerrit Code Review] %s Keys Deleted", getKeyType()));
setMessageId(messageIdGenerator.fromAccountUpdate(user.getAccountId()));
- add(RecipientType.TO, user.getAccountId());
+ addByAccountId(RecipientType.TO, user.getAccountId());
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java b/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java
index bd79d3a..52a16ac 100644
--- a/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java
+++ b/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java
@@ -62,8 +62,8 @@
bccStarredBy();
ccExistingReviewers();
includeWatchers(NotifyType.ALL_COMMENTS);
- reviewers.stream().forEach(r -> add(RecipientType.TO, r));
- addByEmail(RecipientType.TO, reviewersByEmail);
+ reviewers.stream().forEach(r -> addByAccountId(RecipientType.TO, r));
+ reviewersByEmail.stream().forEach(address -> addByEmail(RecipientType.TO, address));
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/HttpPasswordUpdateSender.java b/java/com/google/gerrit/server/mail/send/HttpPasswordUpdateSender.java
index 045c6a4..5fb66bb 100644
--- a/java/com/google/gerrit/server/mail/send/HttpPasswordUpdateSender.java
+++ b/java/com/google/gerrit/server/mail/send/HttpPasswordUpdateSender.java
@@ -50,7 +50,7 @@
setMessageId(
messageIdGenerator.fromReasonAccountIdAndTimestamp(
"HTTP_password_change", user.getAccountId(), TimeUtil.now()));
- add(RecipientType.TO, user.getAccountId());
+ addByAccountId(RecipientType.TO, user.getAccountId());
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/InboundEmailRejectionSender.java b/java/com/google/gerrit/server/mail/send/InboundEmailRejectionSender.java
index 2e0eeb3..0ddb0ad 100644
--- a/java/com/google/gerrit/server/mail/send/InboundEmailRejectionSender.java
+++ b/java/com/google/gerrit/server/mail/send/InboundEmailRejectionSender.java
@@ -63,7 +63,7 @@
setListIdHeader();
setHeader(FieldName.SUBJECT, "[Gerrit Code Review] Unable to process your email");
- add(RecipientType.TO, to);
+ addByEmail(RecipientType.TO, to);
if (!threadId.isEmpty()) {
setHeader(MailHeader.REFERENCES.fieldName(), threadId);
diff --git a/java/com/google/gerrit/server/mail/send/NewChangeSender.java b/java/com/google/gerrit/server/mail/send/NewChangeSender.java
index 968bb1a..aabf7ca 100644
--- a/java/com/google/gerrit/server/mail/send/NewChangeSender.java
+++ b/java/com/google/gerrit/server/mail/send/NewChangeSender.java
@@ -75,18 +75,18 @@
break;
case ALL:
default:
- extraCC.stream().forEach(cc -> add(RecipientType.CC, cc));
- extraCCByEmail.stream().forEach(cc -> add(RecipientType.CC, cc));
+ extraCC.stream().forEach(cc -> addByAccountId(RecipientType.CC, cc));
+ extraCCByEmail.stream().forEach(cc -> addByEmail(RecipientType.CC, cc));
// $FALL-THROUGH$
case OWNER_REVIEWERS:
- reviewers.stream().forEach(r -> add(RecipientType.TO, r, true));
- addByEmail(RecipientType.TO, reviewersByEmail, true);
- removedReviewers.stream().forEach(r -> add(RecipientType.TO, r, true));
- addByEmail(RecipientType.TO, removedByEmailReviewers, true);
+ reviewers.stream().forEach(r -> addByAccountId(RecipientType.TO, r, true));
+ reviewersByEmail.stream().forEach(r -> addByEmail(RecipientType.TO, r, true));
+ removedReviewers.stream().forEach(r -> addByAccountId(RecipientType.TO, r, true));
+ removedByEmailReviewers.stream().forEach(r -> addByEmail(RecipientType.TO, r, true));
break;
}
- rcptToAuthors(RecipientType.CC);
+ addAuthors(RecipientType.CC);
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/NotificationEmail.java b/java/com/google/gerrit/server/mail/send/NotificationEmail.java
index f023075..c943724 100644
--- a/java/com/google/gerrit/server/mail/send/NotificationEmail.java
+++ b/java/com/google/gerrit/server/mail/send/NotificationEmail.java
@@ -67,9 +67,9 @@
protected void includeWatchers(NotifyType type, boolean includeWatchersFromNotifyConfig) {
try {
Watchers matching = getWatchers(type, includeWatchersFromNotifyConfig);
- add(RecipientType.TO, matching.to);
- add(RecipientType.CC, matching.cc);
- add(RecipientType.BCC, matching.bcc);
+ addWatchers(RecipientType.TO, matching.to);
+ addWatchers(RecipientType.CC, matching.cc);
+ addWatchers(RecipientType.BCC, matching.bcc);
} catch (StorageException err) {
// Just don't CC everyone. Better to send a partial message to those
// we already have queued up then to fail deliver entirely to people
@@ -82,12 +82,12 @@
protected abstract Watchers getWatchers(NotifyType type, boolean includeWatchersFromNotifyConfig);
/** Add users or email addresses to the TO, CC, or BCC list. */
- protected void add(RecipientType type, WatcherList watcherList) {
+ protected void addWatchers(RecipientType type, WatcherList watcherList) {
for (Account.Id user : watcherList.accounts) {
addWatcher(type, user);
}
for (Address addr : watcherList.emails) {
- add(type, addr);
+ addByEmail(type, addr);
}
}
diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index 55f82d4..d00c874 100644
--- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -44,7 +44,6 @@
import java.net.URL;
import java.time.Instant;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
@@ -166,7 +165,7 @@
logger.atFine().log(
"CC email sender %s because the email strategy of this user is %s",
fromUser.get().account().id(), CC_ON_OWN_COMMENTS);
- add(RecipientType.CC, fromId);
+ addByAccountId(RecipientType.CC, fromId);
} else if (isImpersonating) {
// If we are impersonating a user, make sure they receive a CC of
// this message regardless of email strategy, unless email notifications are explicitly
@@ -176,7 +175,7 @@
"CC email sender %s because the email is sent on behalf of and email notifications"
+ " are enabled for this user.",
fromUser.get().account().id());
- add(RecipientType.CC, fromId);
+ addByAccountId(RecipientType.CC, fromId);
} else if (!notify.accounts().containsValue(fromId) && rcptTo.remove(fromId)) {
// If they don't want a copy, but we queued one up anyway,
@@ -331,7 +330,7 @@
setHeader(MailHeader.AUTO_SUBMITTED.fieldName(), "auto-generated");
for (RecipientType recipientType : notify.accounts().keySet()) {
- notify.accounts().get(recipientType).stream().forEach(a -> add(recipientType, a));
+ notify.accounts().get(recipientType).stream().forEach(a -> addByAccountId(recipientType, a));
}
setHeader(MailHeader.MESSAGE_TYPE.fieldName(), messageClass);
@@ -525,51 +524,86 @@
return true;
}
- /** Schedule this message for delivery to the listed address. */
- protected final void addByEmail(RecipientType rt, Collection<Address> list) {
- addByEmail(rt, list, false);
+ /**
+ * Adds a recipient that the email will be sent to.
+ *
+ * @param rt category of recipient (TO, CC, BCC)
+ * @param addr Name and email of the recipient.
+ */
+ public final void addByEmail(RecipientType rt, Address addr) {
+ addByEmail(rt, addr, false);
}
- /** Schedule this message for delivery to the listed address. */
- protected final void addByEmail(RecipientType rt, Collection<Address> list, boolean override) {
- for (final Address id : list) {
- add(rt, id, override);
- }
- }
-
- /** Schedule delivery of this message to the given account. */
- protected void add(RecipientType rt, Account.Id to) {
- add(rt, to, false);
- }
-
- protected void add(RecipientType rt, Account.Id to, boolean override) {
+ /**
+ * Adds a recipient that the email will be sent to.
+ *
+ * @param rt category of recipient (TO, CC, BCC).
+ * @param addr Name and email of the recipient.
+ * @param override if the recipient was added previously and override is false no change is made
+ * regardless of {@code rt}.
+ */
+ public final void addByEmail(RecipientType rt, Address addr, boolean override) {
try {
- if (!rcptTo.contains(to) && isVisibleTo(to)) {
- rcptTo.add(to);
- add(rt, toAddress(to), override);
+ if (isRecipientAllowed(addr)) {
+ add(rt, addr, override);
}
} catch (PermissionBackendException e) {
- logger.atSevere().withCause(e).log("Error reading database for account: %s", to);
+ logger.atSevere().withCause(e).log("Error checking permissions for email address: %s", addr);
}
}
/**
- * Returns whether this email is visible to the given account
+ * Returns whether this email is allowed to be sent to the given address
+ *
+ * @param addr email address of recipient.
+ * @throws PermissionBackendException thrown if checking a permission fails due to an error in the
+ * permission backend
+ */
+ protected boolean isRecipientAllowed(Address addr) throws PermissionBackendException {
+ return true;
+ }
+
+ /**
+ * Adds a recipient that the email will be sent to.
+ *
+ * @param rt category of recipient (TO, CC, BCC)
+ * @param to Gerrit Account of the recipient.
+ */
+ protected void addByAccountId(RecipientType rt, Account.Id to) {
+ addByAccountId(rt, to, false);
+ }
+
+ /**
+ * Adds a recipient that the email will be sent to.
+ *
+ * @param rt category of recipient (TO, CC, BCC)
+ * @param to Gerrit Account of the recipient.
+ * @param override if the recipient was added previously and override is false no change is made
+ * regardless of {@code rt}.
+ */
+ protected void addByAccountId(RecipientType rt, Account.Id to, boolean override) {
+ try {
+ if (!rcptTo.contains(to) && isRecipientAllowed(to)) {
+ rcptTo.add(to);
+ add(rt, toAddress(to), override);
+ }
+ } catch (PermissionBackendException e) {
+ logger.atSevere().withCause(e).log("Error checking permissions for account: %s", to);
+ }
+ }
+
+ /**
+ * Returns whether this email is allowed to be sent to the given account
*
* @param to account.
* @throws PermissionBackendException thrown if checking a permission fails due to an error in the
* permission backend
*/
- protected boolean isVisibleTo(Account.Id to) throws PermissionBackendException {
+ protected boolean isRecipientAllowed(Account.Id to) throws PermissionBackendException {
return true;
}
- /** Schedule delivery of this message to the given account. */
- protected final void add(RecipientType rt, Address addr) {
- add(rt, addr, false);
- }
-
- protected final void add(RecipientType rt, Address addr, boolean override) {
+ private final void add(RecipientType rt, Address addr, boolean override) {
if (addr != null && addr.email() != null && addr.email().length() > 0) {
if (!args.validator.isValid(addr.email())) {
logger.atWarning().log("Not emailing %s (invalid email address)", addr.email());
diff --git a/java/com/google/gerrit/server/mail/send/RegisterNewEmailSender.java b/java/com/google/gerrit/server/mail/send/RegisterNewEmailSender.java
index a54a652..f7bc336 100644
--- a/java/com/google/gerrit/server/mail/send/RegisterNewEmailSender.java
+++ b/java/com/google/gerrit/server/mail/send/RegisterNewEmailSender.java
@@ -54,7 +54,7 @@
protected void init() throws EmailException {
super.init();
setHeader("Subject", "[Gerrit Code Review] Email Verification");
- add(RecipientType.TO, Address.create(addr));
+ addByEmail(RecipientType.TO, Address.create(addr));
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java b/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java
index 1e53288..188c5d8 100644
--- a/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java
+++ b/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java
@@ -125,10 +125,10 @@
if (args.settings.sendNewPatchsetEmails) {
if (notify.handling().equals(NotifyHandling.ALL)
|| notify.handling().equals(NotifyHandling.OWNER_REVIEWERS)) {
- reviewers.stream().forEach(r -> add(RecipientType.TO, r));
- extraCC.stream().forEach(cc -> add(RecipientType.CC, cc));
+ reviewers.stream().forEach(r -> addByAccountId(RecipientType.TO, r));
+ extraCC.stream().forEach(cc -> addByAccountId(RecipientType.CC, cc));
}
- rcptToAuthors(RecipientType.CC);
+ addAuthors(RecipientType.CC);
}
bccStarredBy();
includeWatchers(NotifyType.NEW_PATCHSETS, !change.isWorkInProgress() && !change.isPrivate());
diff --git a/java/com/google/gerrit/server/mail/send/ReplyToChangeSender.java b/java/com/google/gerrit/server/mail/send/ReplyToChangeSender.java
index c765430..696cd17 100644
--- a/java/com/google/gerrit/server/mail/send/ReplyToChangeSender.java
+++ b/java/com/google/gerrit/server/mail/send/ReplyToChangeSender.java
@@ -38,6 +38,6 @@
setHeader("In-Reply-To", threadId);
setHeader("References", threadId);
- rcptToAuthors(RecipientType.TO);
+ addAuthors(RecipientType.TO);
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java b/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java
index 38ab8e9..84de569 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java
@@ -25,6 +25,7 @@
import com.google.gerrit.server.git.InsertedObject;
import java.io.IOException;
import java.util.List;
+import java.util.Locale;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.AnyObjectId;
@@ -125,10 +126,10 @@
List<FooterLine> src = getFooterLines();
footerLines = MultimapBuilder.hashKeys(src.size()).arrayListValues(1).build();
for (FooterLine fl : src) {
- footerLines.put(fl.getKey().toLowerCase(), fl.getValue());
+ footerLines.put(fl.getKey().toLowerCase(Locale.US), fl.getValue());
}
}
- return footerLines.get(key.getName().toLowerCase());
+ return footerLines.get(key.getName().toLowerCase(Locale.US));
}
public boolean isAttentionSetCommitOnly(boolean hasChangeMessage) {
@@ -137,7 +138,7 @@
.keySet()
.equals(
Sets.newHashSet(
- FOOTER_PATCH_SET.getName().toLowerCase(),
- FOOTER_ATTENTION.getName().toLowerCase()));
+ FOOTER_PATCH_SET.getName().toLowerCase(Locale.US),
+ FOOTER_ATTENTION.getName().toLowerCase(Locale.US)));
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 0ee0689..951a478 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -93,6 +93,7 @@
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@@ -760,7 +761,7 @@
throw expectedOneFooter(FOOTER_STATUS, statusLines);
}
Change.Status status =
- Enums.getIfPresent(Change.Status.class, statusLines.get(0).toUpperCase()).orNull();
+ Enums.getIfPresent(Change.Status.class, statusLines.get(0).toUpperCase(Locale.US)).orNull();
if (status == null) {
throw invalidFooter(FOOTER_STATUS, statusLines.get(0));
}
@@ -802,7 +803,7 @@
PatchSetState state =
Enums.getIfPresent(
PatchSetState.class,
- withParens.substring(1, withParens.length() - 1).toUpperCase())
+ withParens.substring(1, withParens.length() - 1).toUpperCase(Locale.US))
.orNull();
if (state != null) {
return state;
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index f8c7426..0a895fb 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -95,6 +95,7 @@
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@@ -741,7 +742,7 @@
}
if (status != null) {
- addFooter(msg, FOOTER_STATUS, status.name().toLowerCase());
+ addFooter(msg, FOOTER_STATUS, status.name().toLowerCase(Locale.US));
if (status.equals(Change.Status.ABANDONED)) {
clearAttentionSet("Change was abandoned");
}
@@ -1129,7 +1130,7 @@
private void addPatchSetFooter(StringBuilder sb, PatchSet.Id ps) {
addFooter(sb, FOOTER_PATCH_SET).append(ps.get());
if (psState != null) {
- sb.append(" (").append(psState.name().toLowerCase()).append(')');
+ sb.append(" (").append(psState.name().toLowerCase(Locale.US)).append(')');
}
sb.append('\n');
}
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index f9eaa906..6c8087e0 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -720,7 +720,7 @@
Map<String, String> lowerNames = Maps.newHashMapWithExpectedSize(2);
extensionPanelSections = new LinkedHashMap<>();
for (String name : rc.getSubsections(EXTENSION_PANELS)) {
- String lower = name.toLowerCase();
+ String lower = name.toLowerCase(Locale.US);
if (lowerNames.containsKey(lower)) {
error(
String.format(
@@ -970,7 +970,7 @@
submitRequirementSections = new LinkedHashMap<>();
for (String name : rc.getSubsections(SUBMIT_REQUIREMENT)) {
checkDuplicateSrDefinition(rc, name);
- String lower = name.toLowerCase();
+ String lower = name.toLowerCase(Locale.US);
if (lowerNames.containsKey(lower)) {
error(
String.format(
@@ -1102,7 +1102,7 @@
Map<String, String> lowerNames = Maps.newHashMapWithExpectedSize(2);
labelSections = new LinkedHashMap<>();
for (String name : rc.getSubsections(LABEL)) {
- String lower = name.toLowerCase();
+ String lower = name.toLowerCase(Locale.US);
if (lowerNames.containsKey(lower)) {
error(String.format("Label \"%s\" conflicts with \"%s\"", name, lowerNames.get(lower)));
}
@@ -1530,7 +1530,7 @@
if (capability != null) {
Set<String> have = new HashSet<>();
for (Permission permission : sort(capability.getPermissions())) {
- have.add(permission.getName().toLowerCase());
+ have.add(permission.getName().toLowerCase(Locale.US));
boolean needRange = GlobalCapability.hasRange(permission.getName());
List<String> rules = new ArrayList<>();
@@ -1544,7 +1544,7 @@
rc.setStringList(CAPABILITY, null, permission.getName(), rules);
}
for (String varName : rc.getNames(CAPABILITY)) {
- if (!have.contains(varName.toLowerCase())) {
+ if (!have.contains(varName.toLowerCase(Locale.US))) {
rc.unset(CAPABILITY, null, varName);
}
}
@@ -1575,7 +1575,7 @@
Set<String> have = new HashSet<>();
for (Permission permission : sort(as.getPermissions())) {
- have.add(permission.getName().toLowerCase());
+ have.add(permission.getName().toLowerCase(Locale.US));
boolean needRange = Permission.hasRange(permission.getName());
List<String> rules = new ArrayList<>();
@@ -1591,7 +1591,7 @@
for (String varName : rc.getNames(ACCESS, refName)) {
if (isCoreOrPluginPermission(convertLegacyPermission(varName))
- && !have.contains(varName.toLowerCase())) {
+ && !have.contains(varName.toLowerCase(Locale.US))) {
rc.unset(ACCESS, refName, varName);
}
}
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index 6352f66..9899a6d 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -56,6 +56,7 @@
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@@ -380,7 +381,7 @@
Map<String, SubmitRequirement> requirements = new LinkedHashMap<>();
for (ProjectState s : treeInOrder()) {
for (SubmitRequirement requirement : s.getConfig().getSubmitRequirementSections().values()) {
- String lowerName = requirement.name().toLowerCase();
+ String lowerName = requirement.name().toLowerCase(Locale.US);
SubmitRequirement old = requirements.get(lowerName);
if (old == null || old.allowOverrideInChildProjects()) {
requirements.put(lowerName, requirement);
@@ -395,7 +396,7 @@
Map<String, LabelType> types = new LinkedHashMap<>();
for (ProjectState s : treeInOrder()) {
for (LabelType type : s.getConfig().getLabelSections().values()) {
- String lower = type.getName().toLowerCase();
+ String lower = type.getName().toLowerCase(Locale.US);
LabelType old = types.get(lower);
if (old == null || old.isCanOverride()) {
types.put(lower, type);
@@ -449,11 +450,11 @@
public List<CommentLinkInfo> getCommentLinks() {
Map<String, CommentLinkInfo> cls = new LinkedHashMap<>();
for (CommentLinkInfo cl : commentLinks) {
- cls.put(cl.name.toLowerCase(), cl);
+ cls.put(cl.name.toLowerCase(Locale.US), cl);
}
for (ProjectState s : treeInOrder()) {
for (StoredCommentLinkInfo cl : s.getConfig().getCommentLinkSections().values()) {
- String name = cl.getName().toLowerCase();
+ String name = cl.getName().toLowerCase(Locale.US);
if (cl.getOverrideOnly()) {
CommentLinkInfo parent = cls.get(name);
if (parent == null) {
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 870fc3e..0991f20 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -36,6 +36,7 @@
import com.google.inject.Module;
import com.google.inject.Provider;
import com.google.inject.Scopes;
+import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.function.Function;
@@ -207,7 +208,8 @@
return globalSubmitRequirements.stream()
.collect(
toImmutableMap(
- globalRequirement -> globalRequirement.name().toLowerCase(), Function.identity()));
+ globalRequirement -> globalRequirement.name().toLowerCase(Locale.US),
+ Function.identity()));
}
/** Evaluate the predicate recursively using change data. */
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java b/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
index e54e5af..403e526 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
@@ -28,6 +28,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.HashMap;
+import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
@@ -142,10 +143,12 @@
// (projectConfigRequirements should not contain legacy entries)
// TODO(ghareeb): remove the filter statement
.filter(entry -> !entry.getValue().isLegacy())
- .collect(Collectors.toMap(sr -> sr.getKey().name().toLowerCase(), sr -> sr.getValue()));
+ .collect(
+ Collectors.toMap(
+ sr -> sr.getKey().name().toLowerCase(Locale.US), sr -> sr.getValue()));
for (Map.Entry<SubmitRequirement, SubmitRequirementResult> legacy :
legacyRequirements.entrySet()) {
- String srName = legacy.getKey().name().toLowerCase();
+ String srName = legacy.getKey().name().toLowerCase(Locale.US);
SubmitRequirementResult projectConfigResult = requirementsByName.get(srName);
SubmitRequirementResult legacyResult = legacy.getValue();
// If there's no project config requirement with the same name as the legacy requirement
diff --git a/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java b/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
index 11749cc..ed876c1 100644
--- a/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
@@ -28,6 +28,7 @@
import com.google.gerrit.server.group.GroupResolver;
import com.google.inject.Inject;
import java.util.Arrays;
+import java.util.Locale;
import java.util.Optional;
public class ApprovalQueryBuilder extends QueryBuilder<ApprovalContext, ApprovalQueryBuilder> {
@@ -113,7 +114,7 @@
private static <T extends Enum<T>> Optional<T> parseEnumValue(Class<T> clazz, String value) {
return Optional.ofNullable(
- Enums.getIfPresent(clazz, value.toUpperCase().replace('-', '_')).orNull());
+ Enums.getIfPresent(clazz, value.toUpperCase(Locale.US).replace('-', '_')).orNull());
}
private <T extends Enum<T>> String formatEnumValues(Class<T> clazz) {
diff --git a/java/com/google/gerrit/server/query/change/BranchSetIndexPredicate.java b/java/com/google/gerrit/server/query/change/BranchSetIndexPredicate.java
new file mode 100644
index 0000000..eb35b14
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/BranchSetIndexPredicate.java
@@ -0,0 +1,61 @@
+// Copyright (C) 2023 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.query.change;
+
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.query.OrPredicate;
+import com.google.gerrit.index.query.Predicate;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/** A Predicate to match any number of BranchNameKeys with O(1) efficiency */
+public class BranchSetIndexPredicate extends OrPredicate<ChangeData> {
+ private final String name;
+ private final Set<BranchNameKey> branches;
+
+ public BranchSetIndexPredicate(String name, Set<BranchNameKey> branches) throws StorageException {
+ super(getPredicates(branches));
+ this.name = name;
+ this.branches = branches;
+ }
+
+ @Override
+ public boolean match(ChangeData changeData) {
+ Change change = changeData.change();
+ if (change == null) {
+ return false;
+ }
+
+ return branches.contains(change.getDest());
+ }
+
+ @Override
+ public String toString() {
+ return "BranchSetIndexPredicate[" + name + "]" + super.toString();
+ }
+
+ private static List<Predicate<ChangeData>> getPredicates(Set<BranchNameKey> branches) {
+ return branches.stream()
+ .map(
+ branchNameKey ->
+ Predicate.and(
+ ChangePredicates.project(branchNameKey.project()),
+ ChangePredicates.ref(branchNameKey.branch())))
+ .collect(Collectors.toList());
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/ChangePredicates.java b/java/com/google/gerrit/server/query/change/ChangePredicates.java
index e9bf3c2..528d0ce 100644
--- a/java/com/google/gerrit/server/query/change/ChangePredicates.java
+++ b/java/com/google/gerrit/server/query/change/ChangePredicates.java
@@ -202,14 +202,14 @@
public static Predicate<ChangeData> hashtag(String hashtag) {
// Use toLowerCase without locale to match behavior in ChangeField.
return new ChangeIndexPredicate(
- ChangeField.HASHTAG_SPEC, HashtagsUtil.cleanupHashtag(hashtag).toLowerCase());
+ ChangeField.HASHTAG_SPEC, HashtagsUtil.cleanupHashtag(hashtag).toLowerCase(Locale.US));
}
/** Returns a predicate that matches changes tagged with the provided {@code hashtag}. */
public static Predicate<ChangeData> fuzzyHashtag(String hashtag) {
// Use toLowerCase without locale to match behavior in ChangeField.
return new ChangeIndexPredicate(
- ChangeField.FUZZY_HASHTAG, HashtagsUtil.cleanupHashtag(hashtag).toLowerCase());
+ ChangeField.FUZZY_HASHTAG, HashtagsUtil.cleanupHashtag(hashtag).toLowerCase(Locale.US));
}
/**
@@ -218,7 +218,7 @@
public static Predicate<ChangeData> prefixHashtag(String hashtag) {
// Use toLowerCase without locale to match behavior in ChangeField.
return new ChangeIndexPredicate(
- ChangeField.PREFIX_HASHTAG, HashtagsUtil.cleanupHashtag(hashtag).toLowerCase());
+ ChangeField.PREFIX_HASHTAG, HashtagsUtil.cleanupHashtag(hashtag).toLowerCase(Locale.US));
}
/** Returns a predicate that matches changes that modified the provided {@code file}. */
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 14b1d11..ef067a1 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -65,6 +65,7 @@
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupBackends;
import com.google.gerrit.server.account.GroupMembers;
+import com.google.gerrit.server.account.QueryList;
import com.google.gerrit.server.account.VersionedAccountDestinations;
import com.google.gerrit.server.account.VersionedAccountQueries;
import com.google.gerrit.server.change.ChangeTriplet;
@@ -101,6 +102,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@@ -490,7 +492,8 @@
protected final Arguments args;
protected Map<String, String> hasOperandAliases = Collections.emptyMap();
- private Map<Account.Id, DestinationList> destinationListByAccount = new HashMap<>();
+ private final Map<Account.Id, DestinationList> destinationListByAccount = new HashMap<>();
+ private final Map<Account.Id, QueryList> queryListByAccount = new HashMap<>();
private static final Splitter RULE_SPLITTER = Splitter.on("=");
private static final Splitter PLUGIN_SPLITTER = Splitter.on("_");
@@ -1102,7 +1105,7 @@
// submit record status, interpret as a submit record query.
int eq = name.indexOf('=');
if (eq > 0) {
- String statusName = name.substring(eq + 1).toUpperCase();
+ String statusName = name.substring(eq + 1).toUpperCase(Locale.US);
if (!isInt(statusName) && !MagicLabelValue.tryParse(statusName).isPresent()) {
SubmitRecord.Label.Status status =
Enums.getIfPresent(SubmitRecord.Label.Status.class, statusName).orNull();
@@ -1414,16 +1417,16 @@
String name = null;
Account.Id account = null;
- try (Repository git = args.repoManager.openRepository(args.allUsersName)) {
- // [name=]<name>
- if (inputArgs.keyValue.containsKey(ARG_ID_NAME)) {
- name = inputArgs.keyValue.get(ARG_ID_NAME).value();
- } else if (inputArgs.positional.size() == 1) {
- name = Iterables.getOnlyElement(inputArgs.positional);
- } else if (inputArgs.positional.size() > 1) {
- throw new QueryParseException("Error parsing named query: " + value);
- }
+ // [name=]<name>
+ if (inputArgs.keyValue.containsKey(ARG_ID_NAME)) {
+ name = inputArgs.keyValue.get(ARG_ID_NAME).value();
+ } else if (inputArgs.positional.size() == 1) {
+ name = Iterables.getOnlyElement(inputArgs.positional);
+ } else if (inputArgs.positional.size() > 1) {
+ throw new QueryParseException("Error parsing named query: " + value);
+ }
+ try {
// [,user=<user>]
if (inputArgs.keyValue.containsKey(ARG_ID_USER)) {
Set<Account.Id> accounts = parseAccount(inputArgs.keyValue.get(ARG_ID_USER).value());
@@ -1437,9 +1440,7 @@
account = self();
}
- VersionedAccountQueries q = VersionedAccountQueries.forUser(account);
- q.load(args.allUsersName, git);
- String query = q.getQueryList().getQuery(name);
+ String query = getQueryList(account).getQuery(name);
if (query != null) {
return parse(query);
}
@@ -1452,6 +1453,23 @@
throw new QueryParseException("Unknown named query: " + name);
}
+ protected QueryList getQueryList(Account.Id account) throws ConfigInvalidException, IOException {
+ QueryList ql = queryListByAccount.get(account);
+ if (ql == null) {
+ ql = loadQueryList(account);
+ queryListByAccount.put(account, ql);
+ }
+ return ql;
+ }
+
+ protected QueryList loadQueryList(Account.Id account) throws ConfigInvalidException, IOException {
+ VersionedAccountQueries q = VersionedAccountQueries.forUser(account);
+ try (Repository git = args.repoManager.openRepository(args.allUsersName)) {
+ q.load(args.allUsersName, git);
+ }
+ return q.getQueryList();
+ }
+
@Operator
public Predicate<ChangeData> reviewedby(String who)
throws QueryParseException, IOException, ConfigInvalidException {
@@ -1465,16 +1483,16 @@
String name = null;
Account.Id account = null;
- try (Repository git = args.repoManager.openRepository(args.allUsersName)) {
- // [name=]<name>
- if (inputArgs.keyValue.containsKey(ARG_ID_NAME)) {
- name = inputArgs.keyValue.get(ARG_ID_NAME).value();
- } else if (inputArgs.positional.size() == 1) {
- name = Iterables.getOnlyElement(inputArgs.positional);
- } else if (inputArgs.positional.size() > 1) {
- throw new QueryParseException("Error parsing named destination: " + value);
- }
+ // [name=]<name>
+ if (inputArgs.keyValue.containsKey(ARG_ID_NAME)) {
+ name = inputArgs.keyValue.get(ARG_ID_NAME).value();
+ } else if (inputArgs.positional.size() == 1) {
+ name = Iterables.getOnlyElement(inputArgs.positional);
+ } else if (inputArgs.positional.size() > 1) {
+ throw new QueryParseException("Error parsing named destination: " + value);
+ }
+ try {
// [,user=<user>]
if (inputArgs.keyValue.containsKey(ARG_ID_USER)) {
Set<Account.Id> accounts = parseAccount(inputArgs.keyValue.get(ARG_ID_USER).value());
@@ -1488,9 +1506,9 @@
account = self();
}
- Set<BranchNameKey> destinations = getDestinationList(git, account).getDestinations(name);
+ Set<BranchNameKey> destinations = getDestinationList(account).getDestinations(name);
if (destinations != null && !destinations.isEmpty()) {
- return new DestinationPredicate(destinations, value);
+ return new BranchSetIndexPredicate(FIELD_DESTINATION + ":" + value, destinations);
}
} catch (RepositoryNotFoundException e) {
throw new QueryParseException(
@@ -1501,20 +1519,22 @@
throw new QueryParseException("Unknown named destination: " + name);
}
- protected DestinationList getDestinationList(Repository git, Account.Id account)
+ protected DestinationList getDestinationList(Account.Id account)
throws ConfigInvalidException, RepositoryNotFoundException, IOException {
DestinationList dl = destinationListByAccount.get(account);
if (dl == null) {
- dl = loadDestinationList(git, account);
+ dl = loadDestinationList(account);
destinationListByAccount.put(account, dl);
}
return dl;
}
- protected DestinationList loadDestinationList(Repository git, Account.Id account)
+ protected DestinationList loadDestinationList(Account.Id account)
throws ConfigInvalidException, RepositoryNotFoundException, IOException {
VersionedAccountDestinations d = VersionedAccountDestinations.forUser(account);
- d.load(args.allUsersName, git);
+ try (Repository git = args.repoManager.openRepository(args.allUsersName)) {
+ d.load(args.allUsersName, git);
+ }
return d.getDestinationList();
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java b/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
index fa48511..d949ea8 100644
--- a/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.index.change.ChangeField;
import java.util.ArrayList;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.NavigableMap;
import java.util.Objects;
@@ -74,11 +75,11 @@
}
public static String canonicalize(Change.Status status) {
- return status.name().toLowerCase();
+ return status.name().toLowerCase(Locale.US);
}
public static Predicate<ChangeData> parse(String value) throws QueryParseException {
- String lower = value.toLowerCase();
+ String lower = value.toLowerCase(Locale.US);
NavigableMap<String, Predicate<ChangeData>> head = PREDICATES.tailMap(lower, true);
if (!head.isEmpty()) {
// Assume no statuses share a common prefix so we can only walk one entry.
diff --git a/java/com/google/gerrit/server/query/change/DestinationPredicate.java b/java/com/google/gerrit/server/query/change/DestinationPredicate.java
deleted file mode 100644
index 3c3d70f..0000000
--- a/java/com/google/gerrit/server/query/change/DestinationPredicate.java
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright (C) 2015 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.query.change;
-
-import com.google.gerrit.entities.BranchNameKey;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.index.query.PostFilterPredicate;
-import java.util.Set;
-
-public class DestinationPredicate extends PostFilterPredicate<ChangeData> {
- protected Set<BranchNameKey> destinations;
-
- public DestinationPredicate(Set<BranchNameKey> destinations, String value) {
- super(ChangeQueryBuilder.FIELD_DESTINATION, value);
- this.destinations = destinations;
- }
-
- @Override
- public boolean match(ChangeData object) {
- Change change = object.change();
- if (change == null) {
- return false;
- }
- return destinations.contains(change.getDest());
- }
-
- @Override
- public int getCost() {
- return 1;
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelPredicates.java b/java/com/google/gerrit/server/query/change/MagicLabelPredicates.java
index 82d8717..9ee4852 100644
--- a/java/com/google/gerrit/server/query/change/MagicLabelPredicates.java
+++ b/java/com/google/gerrit/server/query/change/MagicLabelPredicates.java
@@ -16,6 +16,7 @@
import static com.google.gerrit.server.query.change.EqualsLabelPredicates.type;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
@@ -30,6 +31,8 @@
import java.util.Optional;
public class MagicLabelPredicates {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
public static class PostFilterMagicLabelPredicate extends PostFilterPredicate<ChangeData> {
private static class PostFilterMatcher extends Matcher {
public PostFilterMatcher(
@@ -177,6 +180,7 @@
}
public boolean ignoresUploaderApprovals() {
+ logger.atFine().log("account = %d", account.get());
return account.equals(ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID)
|| account.equals(ChangeQueryBuilder.NON_CONTRIBUTOR_ACCOUNT_ID);
}
diff --git a/java/com/google/gerrit/server/query/change/SubmitRecordPredicate.java b/java/com/google/gerrit/server/query/change/SubmitRecordPredicate.java
index 1ea6c41..243712d 100644
--- a/java/com/google/gerrit/server/query/change/SubmitRecordPredicate.java
+++ b/java/com/google/gerrit/server/query/change/SubmitRecordPredicate.java
@@ -20,12 +20,13 @@
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.server.index.change.ChangeField;
+import java.util.Locale;
import java.util.Set;
public class SubmitRecordPredicate extends ChangeIndexPredicate {
public static Predicate<ChangeData> create(
String label, SubmitRecord.Label.Status status, Set<Account.Id> accounts) {
- String lowerLabel = label.toLowerCase();
+ String lowerLabel = label.toLowerCase(Locale.US);
if (accounts == null || accounts.isEmpty()) {
return new SubmitRecordPredicate(status.name() + ',' + lowerLabel);
}
diff --git a/java/com/google/gerrit/server/query/project/ProjectQueryBuilderImpl.java b/java/com/google/gerrit/server/query/project/ProjectQueryBuilderImpl.java
index f7135982..599683e 100644
--- a/java/com/google/gerrit/server/query/project/ProjectQueryBuilderImpl.java
+++ b/java/com/google/gerrit/server/query/project/ProjectQueryBuilderImpl.java
@@ -26,6 +26,7 @@
import com.google.gerrit.index.query.QueryParseException;
import com.google.inject.Inject;
import java.util.List;
+import java.util.Locale;
/** Parses a query string meant to be applied to project objects. */
public class ProjectQueryBuilderImpl extends QueryBuilder<ProjectData, ProjectQueryBuilderImpl>
@@ -72,7 +73,7 @@
}
ProjectState parsedState;
try {
- parsedState = ProjectState.valueOf(state.replace('-', '_').toUpperCase());
+ parsedState = ProjectState.valueOf(state.replace('-', '_').toUpperCase(Locale.US));
} catch (IllegalArgumentException e) {
throw error("state operator must be either 'active' or 'read-only'", e);
}
diff --git a/java/com/google/gerrit/server/restapi/account/GetCapabilities.java b/java/com/google/gerrit/server/restapi/account/GetCapabilities.java
index 6ab2c44..c45694e 100644
--- a/java/com/google/gerrit/server/restapi/account/GetCapabilities.java
+++ b/java/com/google/gerrit/server/restapi/account/GetCapabilities.java
@@ -45,6 +45,7 @@
import com.google.inject.Singleton;
import java.util.HashSet;
import java.util.LinkedHashMap;
+import java.util.Locale;
import java.util.Map;
import java.util.Set;
import org.kohsuke.args4j.Option;
@@ -124,7 +125,7 @@
}
private boolean want(String name) {
- return query == null || query.contains(name.toLowerCase());
+ return query == null || query.contains(name.toLowerCase(Locale.US));
}
private void addRanges(Map<String, Object> have, AccountLimits limits) {
diff --git a/java/com/google/gerrit/server/restapi/change/AllowedFormats.java b/java/com/google/gerrit/server/restapi/change/AllowedFormats.java
index e3ab135..763212d 100644
--- a/java/com/google/gerrit/server/restapi/change/AllowedFormats.java
+++ b/java/com/google/gerrit/server/restapi/change/AllowedFormats.java
@@ -22,6 +22,7 @@
import com.google.gerrit.server.config.DownloadConfig;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import java.util.Locale;
import java.util.Set;
@Singleton
@@ -36,7 +37,7 @@
for (String ext : format.getSuffixes()) {
exts.put(ext, format);
}
- exts.put(format.name().toLowerCase(), format);
+ exts.put(format.name().toLowerCase(Locale.US), format);
}
extensions = exts.build();
diff --git a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
index 84cf209..9715a5d 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
@@ -83,7 +83,7 @@
}
/**
- * Parses {@link ChangeResource} from {@link Change.Id}
+ * Parses {@link ChangeResource} from {@link com.google.gerrit.entities.Change.Id}
*
* <p>Reads the change from index, since project is unknown.
*/
@@ -106,7 +106,8 @@
}
/**
- * Parses {@link ChangeResource} from {@link Change.Id} in {@code project} at {@code metaRevId}
+ * Parses {@link ChangeResource} from {@link com.google.gerrit.entities.Change.Id} in {@code
+ * project} at {@code metaRevId}
*
* <p>Read change from ChangeNotesCache, so the method can be used upon creation, when the change
* might not be yet available in the index.
@@ -123,7 +124,7 @@
}
/**
- * Parses {@link ChangeResource} from {@link Change.Id}
+ * Parses {@link ChangeResource} from {@link com.google.gerrit.entities.Change.Id}
*
* <p>Reads the change from index, since project is unknown.
*/
diff --git a/java/com/google/gerrit/server/restapi/change/OnPostReview.java b/java/com/google/gerrit/server/restapi/change/OnPostReview.java
index b179d02..4999f18 100644
--- a/java/com/google/gerrit/server/restapi/change/OnPostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/OnPostReview.java
@@ -18,6 +18,7 @@
import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.notedb.ChangeNotes;
+import java.time.Instant;
import java.util.Map;
import java.util.Optional;
@@ -28,6 +29,7 @@
* Allows implementors to return a message that should be included into the change message that is
* posted on post review.
*
+ * @param when the timestamp at which the review is posted
* @param user the user that posts the review
* @param changeNotes the change on which post review is performed
* @param patchSet the patch set on which post review is performed
@@ -37,6 +39,7 @@
* {@link Optional#empty()} if the change message should not be extended
*/
default Optional<String> getChangeMessageAddOn(
+ Instant when,
IdentifiedUser user,
ChangeNotes changeNotes,
PatchSet patchSet,
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewOp.java b/java/com/google/gerrit/server/restapi/change/PostReviewOp.java
index 0036d87..a8f8adf 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewOp.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewOp.java
@@ -1035,7 +1035,8 @@
onPostReviews.runEach(
onPostReview ->
onPostReview
- .getChangeMessageAddOn(user, ctx.getNotes(), ps, oldApprovals, approvals)
+ .getChangeMessageAddOn(
+ ctx.getWhen(), user, ctx.getNotes(), ps, oldApprovals, approvals)
.ifPresent(
pluginMessage ->
pluginMessages.add(
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 4c7c352..691fc75 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -85,6 +85,7 @@
import java.util.Comparator;
import java.util.Iterator;
import java.util.List;
+import java.util.Locale;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -196,7 +197,8 @@
}
if (topic == null) {
return String.format(
- "revert-%s-%s", submissionId, RandomStringUtils.randomAlphabetic(10).toUpperCase());
+ "revert-%s-%s",
+ submissionId, RandomStringUtils.randomAlphabetic(10).toUpperCase(Locale.US));
}
return topic;
}
diff --git a/java/com/google/gerrit/server/restapi/config/ReloadConfig.java b/java/com/google/gerrit/server/restapi/config/ReloadConfig.java
index 9ce7ffd..c8f2ed6 100644
--- a/java/com/google/gerrit/server/restapi/config/ReloadConfig.java
+++ b/java/com/google/gerrit/server/restapi/config/ReloadConfig.java
@@ -33,6 +33,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.stream.Collectors;
@@ -59,7 +60,8 @@
updates.asMap().entrySet().stream()
.collect(
Collectors.toMap(
- e -> e.getKey().name().toLowerCase(), e -> toEntryInfos(e.getValue()))));
+ e -> e.getKey().name().toLowerCase(Locale.US),
+ e -> toEntryInfos(e.getValue()))));
}
private static List<ConfigUpdateEntryInfo> toEntryInfos(
diff --git a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
index cab5b45..bfcbffc 100644
--- a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
+++ b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
@@ -51,6 +51,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.Locale;
/**
* Evaluates a submit-like Prolog rule found in the rules.pl file of the current project and filters
@@ -381,7 +382,7 @@
String typeName = typeTerm.name();
try {
- return SubmitTypeRecord.OK(SubmitType.valueOf(typeName.toUpperCase()));
+ return SubmitTypeRecord.OK(SubmitType.valueOf(typeName.toUpperCase(Locale.US)));
} catch (IllegalArgumentException e) {
return typeError(
"Submit type rule "
diff --git a/java/com/google/gerrit/server/submit/MergeMetrics.java b/java/com/google/gerrit/server/submit/MergeMetrics.java
index e9832fd..4c7a197 100644
--- a/java/com/google/gerrit/server/submit/MergeMetrics.java
+++ b/java/com/google/gerrit/server/submit/MergeMetrics.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.submit;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
@@ -28,6 +29,8 @@
/** Metrics are recorded when a change is merged (aka submitted). */
public class MergeMetrics {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private final Provider<SubmitRequirementChangeQueryBuilder> submitRequirementChangequeryBuilder;
// TODO: This metric is for measuring the impact of allowing users to rebase changes on behalf of
@@ -81,12 +84,19 @@
// impersonated. Impersonating the uploader is only allowed on rebase by rebasing on behalf of
// the uploader. Hence if the current patch set has different accounts as uploader and real
// uploader we can assume that it was created by rebase on behalf of the uploader.
- return !cd.currentPatchSet().uploader().equals(cd.currentPatchSet().realUploader());
+ boolean isRebaseOnBehalfOfUploader =
+ !cd.currentPatchSet().uploader().equals(cd.currentPatchSet().realUploader());
+ logger.atFine().log("isRebaseOnBehalfOfUploader = %s", isRebaseOnBehalfOfUploader);
+ return isRebaseOnBehalfOfUploader;
}
private boolean hasCodeReviewApprovalOfRealUploader(ChangeData cd) {
- return cd.currentApprovals().stream()
- .anyMatch(psa -> psa.accountId().equals(cd.currentPatchSet().realUploader()));
+ boolean hasCodeReviewApprovalOfRealUploader =
+ cd.currentApprovals().stream()
+ .anyMatch(psa -> psa.accountId().equals(cd.currentPatchSet().realUploader()));
+ logger.atFine().log(
+ "hasCodeReviewApprovalOfRealUploader = %s", hasCodeReviewApprovalOfRealUploader);
+ return hasCodeReviewApprovalOfRealUploader;
}
private boolean ignoresCodeReviewApprovalsOfUploader(ChangeData cd) {
@@ -105,6 +115,9 @@
}
private boolean ignoresCodeReviewApprovalsOfUploader(Predicate<ChangeData> predicate) {
+ logger.atFine().log(
+ "predicate = (%s) %s (child count = %d)",
+ predicate.getClass().getName(), predicate, predicate.getChildCount());
if (predicate.getChildCount() == 0) {
// Submit requirements may require a Code-Review approval but ignore approvals by the
// uploader. This is done by using a label predicate with 'user=non_uploader' or
diff --git a/java/com/google/gerrit/server/update/context/RefUpdateContext.java b/java/com/google/gerrit/server/update/context/RefUpdateContext.java
index 1144e4f..56d536a 100644
--- a/java/com/google/gerrit/server/update/context/RefUpdateContext.java
+++ b/java/com/google/gerrit/server/update/context/RefUpdateContext.java
@@ -113,6 +113,8 @@
* <p>If a plugin updates one of a special refs - it must also open a nested context.
*/
PLUGIN,
+ /** A ref is updated as a part of auto-close-changes. */
+ AUTO_CLOSE_CHANGES
}
/** Opens a provided context. */
diff --git a/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/java/com/google/gerrit/sshd/commands/ReviewCommand.java
index e0805c0..143b060 100644
--- a/java/com/google/gerrit/sshd/commands/ReviewCommand.java
+++ b/java/com/google/gerrit/sshd/commands/ReviewCommand.java
@@ -56,6 +56,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
+import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@@ -358,7 +359,7 @@
}
private static String asOptionName(LabelType type) {
- return "--" + type.getName().toLowerCase();
+ return "--" + type.getName().toLowerCase(Locale.US);
}
private static Option newApproveOption(LabelType type, String usage) {
diff --git a/java/com/google/gerrit/testing/GerritTestName.java b/java/com/google/gerrit/testing/GerritTestName.java
index d287837..14493b6 100644
--- a/java/com/google/gerrit/testing/GerritTestName.java
+++ b/java/com/google/gerrit/testing/GerritTestName.java
@@ -15,6 +15,7 @@
package com.google.gerrit.testing;
import com.google.common.base.CharMatcher;
+import java.util.Locale;
import org.junit.BeforeClass;
import org.junit.rules.TestName;
import org.junit.rules.TestRule;
@@ -30,7 +31,7 @@
}
public String getSanitizedMethodName() {
- String name = delegate.getMethodName().toLowerCase();
+ String name = delegate.getMethodName().toLowerCase(Locale.US);
name =
CharMatcher.inRange('a', 'z')
.or(CharMatcher.inRange('A', 'Z'))
diff --git a/java/com/google/gerrit/testing/InMemoryRepositoryManager.java b/java/com/google/gerrit/testing/InMemoryRepositoryManager.java
index 8d1130c..8c87405 100644
--- a/java/com/google/gerrit/testing/InMemoryRepositoryManager.java
+++ b/java/com/google/gerrit/testing/InMemoryRepositoryManager.java
@@ -49,6 +49,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.NavigableSet;
@@ -135,6 +136,11 @@
private RefUpdateContextValidator() {}
public void validateRefUpdateContext(ReceiveCommand cmd) {
+ String refName = cmd.getRefName();
+
+ if (RefUpdateContextCollector.enabled()) {
+ RefUpdateContextCollector.register(refName, RefUpdateContext.getOpenedContexts());
+ }
if (TestActionRefUpdateContext.isOpen()
|| RefUpdateContext.hasOpen(OFFLINE_OPERATION)
|| RefUpdateContext.hasOpen(INIT_REPO)
@@ -143,8 +149,6 @@
return;
}
- String refName = cmd.getRefName();
-
Optional<ImmutableList<RefUpdateType>> allowedRefUpdateTypes =
RefUpdateContextValidator.INSTANCE.getAllowedRefUpdateTypes(refName);
@@ -300,6 +304,6 @@
}
private static String normalize(Project.NameKey name) {
- return name.get().toLowerCase();
+ return name.get().toLowerCase(Locale.US);
}
}
diff --git a/java/com/google/gerrit/testing/IndexVersions.java b/java/com/google/gerrit/testing/IndexVersions.java
index 3810707..2e843fe 100644
--- a/java/com/google/gerrit/testing/IndexVersions.java
+++ b/java/com/google/gerrit/testing/IndexVersions.java
@@ -26,6 +26,7 @@
import com.google.gerrit.index.SchemaDefinitions;
import java.util.ArrayList;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.NavigableMap;
import org.eclipse.jgit.lib.Config;
@@ -73,13 +74,14 @@
* if any of the specified schema versions doesn't exist
*/
public static <V> ImmutableList<Integer> get(SchemaDefinitions<V> schemaDef) {
- String envVar = schemaDef.getName().toUpperCase() + "_INDEX_VERSIONS";
+ String envVar = schemaDef.getName().toUpperCase(Locale.US) + "_INDEX_VERSIONS";
String value = System.getenv(envVar);
if (!Strings.isNullOrEmpty(value)) {
return get(schemaDef, "env variable " + envVar, value);
}
- String systemProperty = "gerrit.index." + schemaDef.getName().toLowerCase() + ".versions";
+ String systemProperty =
+ "gerrit.index." + schemaDef.getName().toLowerCase(Locale.US) + ".versions";
value = System.getProperty(systemProperty);
return get(schemaDef, "system property " + systemProperty, value);
}
@@ -138,7 +140,10 @@
i -> {
Config cfg = baseConfig;
cfg.setInt(
- "index", "lucene", schemaDef.getName().toLowerCase() + "TestVersion", i);
+ "index",
+ "lucene",
+ schemaDef.getName().toLowerCase(Locale.US) + "TestVersion",
+ i);
return cfg;
}));
}
diff --git a/java/com/google/gerrit/testing/RefUpdateContextCollector.java b/java/com/google/gerrit/testing/RefUpdateContextCollector.java
new file mode 100644
index 0000000..88232d2
--- /dev/null
+++ b/java/com/google/gerrit/testing/RefUpdateContextCollector.java
@@ -0,0 +1,92 @@
+// Copyright (C) 2023 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.testing;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.server.update.context.RefUpdateContext;
+import com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType;
+import java.util.AbstractMap.SimpleImmutableEntry;
+import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
+
+/**
+ * Stores information about each updated ref in tests, together with associated RefUpdateContext(s).
+ *
+ * <p>This is a {@link TestRule}, which clears the stored data after each test.
+ *
+ * <p>Usage:
+ *
+ * <pre>{@code
+ * class ...Test {
+ * \@Rule
+ * public RefUpdateContextCollector refContextCollector = new RefUpdateContextCollector();
+ * ...
+ * public void test() {
+ * // some actions
+ * assertThat(refContextCollector.getContextsByRef("refs/heads/main")).contains(...)
+ * }
+ * }
+ * }</pre>
+ */
+public class RefUpdateContextCollector implements TestRule {
+ private static ConcurrentLinkedQueue<Entry<String, ImmutableList<RefUpdateContext>>>
+ touchedRefsWithContexts = null;
+
+ @Override
+ public Statement apply(Statement statement, Description description) {
+ return new Statement() {
+ @Override
+ public void evaluate() throws Throwable {
+ try {
+ touchedRefsWithContexts = new ConcurrentLinkedQueue<>();
+ statement.evaluate();
+ } finally {
+ touchedRefsWithContexts = null;
+ }
+ }
+ };
+ }
+
+ public static boolean enabled() {
+ return touchedRefsWithContexts != null;
+ }
+
+ public static void register(String refName, ImmutableList<RefUpdateContext> openedContexts) {
+ if (touchedRefsWithContexts == null) {
+ return;
+ }
+ touchedRefsWithContexts.add(new SimpleImmutableEntry<>(refName, openedContexts));
+ }
+
+ public ImmutableList<String> getRefsByUpdateType(RefUpdateType refUpdateType) {
+ return touchedRefsWithContexts.stream()
+ .filter(
+ entry ->
+ entry.getValue().stream()
+ .map(RefUpdateContext::getUpdateType)
+ .anyMatch(refUpdateType::equals))
+ .map(Entry::getKey)
+ .collect(toImmutableList());
+ }
+
+ public void clear() {
+ touchedRefsWithContexts.clear();
+ }
+}
diff --git a/java/com/google/gerrit/testing/SshMode.java b/java/com/google/gerrit/testing/SshMode.java
index 41633bd..60bd5187 100644
--- a/java/com/google/gerrit/testing/SshMode.java
+++ b/java/com/google/gerrit/testing/SshMode.java
@@ -18,6 +18,7 @@
import com.google.common.base.Enums;
import com.google.common.base.Strings;
+import java.util.Locale;
/**
* Whether to enable/disable tests using SSH by inspecting the global environment.
@@ -43,7 +44,7 @@
if (Strings.isNullOrEmpty(value)) {
return YES;
}
- value = value.toUpperCase();
+ value = value.toUpperCase(Locale.US);
SshMode mode = Enums.getIfPresent(SshMode.class, value).orNull();
if (!Strings.isNullOrEmpty(System.getenv(ENV_VAR))) {
checkArgument(
diff --git a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
index 80431ee..b80ff9b 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
@@ -46,6 +46,7 @@
import com.google.gerrit.testing.TestTimeUtil;
import com.google.inject.Inject;
import java.util.List;
+import java.util.Locale;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
@@ -65,7 +66,8 @@
gApi.changes().id(changeId).abandon();
ChangeInfo info = get(changeId, MESSAGES);
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
- assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("abandoned");
+ assertThat(Iterables.getLast(info.messages).message.toLowerCase(Locale.US))
+ .contains("abandoned");
ResourceConflictException thrown =
assertThrows(ResourceConflictException.class, () -> gApi.changes().id(changeId).abandon());
@@ -82,13 +84,17 @@
ChangeInfo info = get(a.getChangeId(), MESSAGES);
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
- assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("abandoned");
- assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("deadbeef");
+ assertThat(Iterables.getLast(info.messages).message.toLowerCase(Locale.US))
+ .contains("abandoned");
+ assertThat(Iterables.getLast(info.messages).message.toLowerCase(Locale.US))
+ .contains("deadbeef");
info = get(b.getChangeId(), MESSAGES);
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
- assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("abandoned");
- assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("deadbeef");
+ assertThat(Iterables.getLast(info.messages).message.toLowerCase(Locale.US))
+ .contains("abandoned");
+ assertThat(Iterables.getLast(info.messages).message.toLowerCase(Locale.US))
+ .contains("deadbeef");
}
@Test
@@ -292,7 +298,8 @@
gApi.changes().id(changeId).restore();
ChangeInfo info = get(changeId, MESSAGES);
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
- assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("restored");
+ assertThat(Iterables.getLast(info.messages).message.toLowerCase(Locale.US))
+ .contains("restored");
ResourceConflictException thrown =
assertThrows(ResourceConflictException.class, () -> gApi.changes().id(changeId).restore());
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
index bb8f3f3..a0f0fe6 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
@@ -86,6 +86,7 @@
import com.google.inject.Inject;
import com.google.inject.Module;
import java.sql.Timestamp;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -1064,6 +1065,7 @@
@Override
public Optional<String> getChangeMessageAddOn(
+ Instant when,
IdentifiedUser user,
ChangeNotes changeNotes,
PatchSet patchSet,
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index 142a45c..9456a31 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -114,6 +114,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.stream.Stream;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
@@ -463,7 +464,7 @@
@Test
public void createDuplicateInternalGroupCaseInsensitiveName() throws Exception {
String dupGroupName = name("dupGroupA");
- String dupGroupNameLowerCase = name("dupGroupA").toLowerCase();
+ String dupGroupNameLowerCase = name("dupGroupA").toLowerCase(Locale.US);
gApi.groups().create(dupGroupName);
gApi.groups().create(dupGroupNameLowerCase);
assertThat(gApi.groups().list().getAsMap().keySet()).contains(dupGroupName);
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 0291f33..d0d6fb4 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -1771,7 +1771,6 @@
assertThat(patchSetLinkInfo.name).isEqualTo(expectedPatchSetLinkInfo.name);
assertThat(patchSetLinkInfo.imageUrl).isEqualTo(expectedPatchSetLinkInfo.imageUrl);
assertThat(patchSetLinkInfo.url).isEqualTo(expectedPatchSetLinkInfo.url);
- assertThat(patchSetLinkInfo.target).isEqualTo(expectedPatchSetLinkInfo.target);
assertThat(commitInfo.resolveConflictsWebLinks).hasSize(1);
WebLinkInfo resolveCommentsLinkInfo =
@@ -1780,7 +1779,6 @@
assertThat(resolveCommentsLinkInfo.imageUrl)
.isEqualTo(expectedResolveConflictsLinkInfo.imageUrl);
assertThat(resolveCommentsLinkInfo.url).isEqualTo(expectedResolveConflictsLinkInfo.url);
- assertThat(resolveCommentsLinkInfo.target).isEqualTo(expectedResolveConflictsLinkInfo.target);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java
index 0cdac5a..8295550 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java
@@ -41,7 +41,9 @@
import com.google.gerrit.server.events.ChangeMergedEvent;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType;
import com.google.gerrit.testing.FakeEmailSender.Message;
+import com.google.gerrit.testing.RefUpdateContextCollector;
import com.google.inject.Inject;
import java.util.List;
import org.eclipse.jgit.api.errors.GitAPIException;
@@ -54,12 +56,16 @@
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
public abstract class AbstractSubmitOnPush extends AbstractDaemonTest {
@Inject private ApprovalsUtil approvalsUtil;
@Inject private ProjectOperations projectOperations;
+ @Rule
+ public RefUpdateContextCollector refUpdateContextCollector = new RefUpdateContextCollector();
+
@Before
public void blockAnonymous() throws Exception {
blockAnonymousRead();
@@ -229,6 +235,25 @@
}
@Test
+ public void pushAutoclosesChanges_changeMetaInAutoClosesChangesContext() throws Exception {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref("refs/heads/master").group(adminGroupUuid()))
+ .update();
+ PushOneCommit.Result r = push("refs/for/master", PushOneCommit.SUBJECT, "one.txt", "One");
+ String refPrefix = r.getChange().getId().toRefPrefix();
+ assertThat(refUpdateContextCollector.getRefsByUpdateType(RefUpdateType.DIRECT_PUSH)).isEmpty();
+ assertThat(refUpdateContextCollector.getRefsByUpdateType(RefUpdateType.AUTO_CLOSE_CHANGES))
+ .isEmpty();
+ git().push().setRefSpecs(new RefSpec(r.getCommit().name() + ":refs/heads/master")).call();
+ assertThat(refUpdateContextCollector.getRefsByUpdateType(RefUpdateType.DIRECT_PUSH))
+ .containsExactly("refs/heads/master", refPrefix + "meta");
+ assertThat(refUpdateContextCollector.getRefsByUpdateType(RefUpdateType.AUTO_CLOSE_CHANGES))
+ .containsExactly(refPrefix + "meta");
+ }
+
+ @Test
public void mergeOnPushToBranchWithChangeMergedInOther() throws Exception {
enableCreateNewChangeForAllNotInTarget();
String master = "refs/heads/master";
diff --git a/javatests/com/google/gerrit/acceptance/git/ImplicitMergeCheckIT.java b/javatests/com/google/gerrit/acceptance/git/ImplicitMergeCheckIT.java
index 80cc508..e352e2d 100644
--- a/javatests/com/google/gerrit/acceptance/git/ImplicitMergeCheckIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/ImplicitMergeCheckIT.java
@@ -22,6 +22,7 @@
import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.git.ObjectIds;
+import java.util.Locale;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
@@ -61,7 +62,8 @@
PushOneCommit.Result m = push("refs/heads/master", "1", "f", "1");
PushOneCommit.Result c = push("refs/for/stable", "2", "f", "2");
- assertThat(c.getMessage().toLowerCase()).doesNotContain(implicitMergeOf(m.getCommit()));
+ assertThat(c.getMessage().toLowerCase(Locale.US))
+ .doesNotContain(implicitMergeOf(m.getCommit()));
}
@Test
@@ -74,7 +76,8 @@
PushOneCommit.Result m = push("refs/heads/master", "1", "f", "1");
PushOneCommit.Result c = push("refs/for/master", "2", "f", "2");
- assertThat(c.getMessage().toLowerCase()).doesNotContain(implicitMergeOf(m.getCommit()));
+ assertThat(c.getMessage().toLowerCase(Locale.US))
+ .doesNotContain(implicitMergeOf(m.getCommit()));
}
private String implicitMergeOf(ObjectId commit) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
index 62ef118..f40910a 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
@@ -52,6 +52,7 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.util.List;
+import java.util.Locale;
import java.util.Optional;
import java.util.Set;
import org.junit.Test;
@@ -179,7 +180,7 @@
assertThat(gApi.accounts().self().get().email).isNotEqualTo(email);
requestScopeOperations.resetCurrentApiUser();
- String emailOtherCase = email.toUpperCase();
+ String emailOtherCase = email.toUpperCase(Locale.US);
gApi.accounts().self().email(emailOtherCase).setPreferred();
assertThat(gApi.accounts().self().get().email).isEqualTo(email);
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
index 39a32af..61164f7 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
@@ -949,7 +949,7 @@
extIdNotes.insert(extId);
extIdNotes.commit(md);
assertThat(getAccountId(extIdNotes, scheme, id)).isEqualTo(accountId.get());
- assertThat(getExternalId(extIdNotes, scheme, id.toLowerCase()).isPresent()).isFalse();
+ assertThat(getExternalId(extIdNotes, scheme, id.toLowerCase(Locale.US)).isPresent()).isFalse();
}
private void testCaseInsensitiveExternalIdKey(
@@ -959,7 +959,8 @@
extIdNotes.insert(extId);
extIdNotes.commit(md);
assertThat(getAccountId(extIdNotes, scheme, id)).isEqualTo(accountId.get());
- assertThat(getAccountId(extIdNotes, scheme, id.toLowerCase())).isEqualTo(accountId.get());
+ assertThat(getAccountId(extIdNotes, scheme, id.toLowerCase(Locale.US)))
+ .isEqualTo(accountId.get());
}
/**
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/PutUsernameIT.java b/javatests/com/google/gerrit/acceptance/rest/account/PutUsernameIT.java
index f46cf0c..f4e9457 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/PutUsernameIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/PutUsernameIT.java
@@ -20,6 +20,7 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.extensions.api.accounts.UsernameInput;
+import java.util.Locale;
import org.junit.Test;
public class PutUsernameIT extends AbstractDaemonTest {
@@ -46,7 +47,7 @@
@GerritConfig(name = "auth.userNameCaseInsensitive", value = "true")
public void setExistingCaseInsensitive_Conflict() throws Exception {
UsernameInput in = new UsernameInput();
- in.username = admin.username().toUpperCase();
+ in.username = admin.username().toUpperCase(Locale.US);
adminRestSession
.put("/accounts/" + accountCreator.create().id().get() + "/username", in)
.assertConflict();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
index 80bedcd..6dfa82b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
@@ -47,6 +47,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.inject.Inject;
import java.util.List;
+import java.util.Locale;
import java.util.Set;
import java.util.stream.IntStream;
import org.junit.Before;
@@ -336,7 +337,7 @@
reviewers = suggestReviewers(changeId, user1.username() + " example");
assertThat(reviewers).hasSize(1);
- reviewers = suggestReviewers(changeId, user4.email().toLowerCase());
+ reviewers = suggestReviewers(changeId, user4.email().toLowerCase(Locale.US));
assertThat(reviewers).hasSize(1);
assertThat(reviewers.get(0).account.email).isEqualTo(user4.email());
}
diff --git a/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java b/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java
index 6f3aa15..35ecceb 100644
--- a/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java
@@ -43,6 +43,7 @@
import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import java.util.Locale;
import java.util.Optional;
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
@@ -176,7 +177,7 @@
assertThat(resolve(existingUsername)).containsExactly(idWithUsername);
assertThat(resolve(existingMixedCaseUsername)).containsExactly(idWithMixedCaseUsername);
- assertThat(resolve(existingMixedCaseUsername.toLowerCase()))
+ assertThat(resolve(existingMixedCaseUsername.toLowerCase(Locale.US)))
.containsExactly(idWithMixedCaseUsername);
}
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
index cced47f..28c795d 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.entities.NotifyConfig.NotifyType.ABANDONED_CHANGES;
import static com.google.gerrit.entities.NotifyConfig.NotifyType.ALL_COMMENTS;
import static com.google.gerrit.entities.NotifyConfig.NotifyType.NEW_CHANGES;
@@ -28,6 +29,7 @@
import static com.google.gerrit.extensions.api.changes.NotifyHandling.OWNER_REVIEWERS;
import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.CC_ON_OWN_COMMENTS;
import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.ENABLED;
+import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.project.testing.TestLabels.labelBuilder;
import static com.google.gerrit.server.project.testing.TestLabels.value;
@@ -306,6 +308,25 @@
addReviewerToReviewableChange(batch());
}
+ @Test
+ public void addReviewerToChangeNoAnonymousUsersNotified() throws Exception {
+ StagedChange sc = stageReviewableChange();
+ // Remove read permission for anonymous users.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/*").group(ANONYMOUS_USERS))
+ .add(allow(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
+ addReviewer(singly(), sc.changeId, sc.owner, reviewer.email());
+
+ // No BY_EMAIL cc's.
+ assertThat(sender).sent("newchange", sc).to(reviewer).cc(sc.reviewer).noOneElse();
+ assertThat(sender).didNotSend();
+ }
+
private void addReviewerToReviewableChangeByOwnerCcingSelf(Adder adder) throws Exception {
StagedChange sc = stageReviewableChange();
TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
@@ -672,6 +693,30 @@
}
@Test
+ public void commentOnChangeNotVisibleToAnonymousByReviewer() throws Exception {
+ StagedChange sc = stageReviewableChange();
+
+ // Remove read permission for anonymous users.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/*").group(ANONYMOUS_USERS))
+ .add(allow(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+
+ review(sc.reviewer, sc.changeId, ENABLED);
+ // Not cc'ed to BY_EMAIL added addresses.
+ assertThat(sender)
+ .sent("comment", sc)
+ .to(sc.owner)
+ .cc(sc.ccer)
+ .bcc(sc.starrer)
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
+ assertThat(sender).didNotSend();
+ }
+
+ @Test
public void commentOnReviewableChangeByOwnerCcingSelf() throws Exception {
StagedChange sc = stageReviewableChange();
review(sc.owner, sc.changeId, CC_ON_OWN_COMMENTS);
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/EmailValidatorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/EmailValidatorIT.java
index cc61dfb..3145234 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/EmailValidatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/EmailValidatorIT.java
@@ -26,6 +26,7 @@
import java.io.InputStream;
import java.io.InputStreamReader;
import java.lang.reflect.Field;
+import java.util.Locale;
import org.junit.After;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -83,12 +84,13 @@
continue;
}
if (tld.startsWith(UNSUPPORTED_PREFIX)) {
- String test = "test@example." + tld.toLowerCase().substring(UNSUPPORTED_PREFIX.length());
+ String test =
+ "test@example." + tld.toLowerCase(Locale.US).substring(UNSUPPORTED_PREFIX.length());
assertWithMessage("expected invalid TLD \"" + test + "\"")
.that(validator.isValid(test))
.isFalse();
} else {
- String test = "test@example." + tld.toLowerCase();
+ String test = "test@example." + tld.toLowerCase(Locale.US);
assertWithMessage("failed to validate TLD \"" + test + "\"")
.that(validator.isValid(test))
.isTrue();
diff --git a/javatests/com/google/gerrit/gpg/PublicKeyStoreTest.java b/javatests/com/google/gerrit/gpg/PublicKeyStoreTest.java
index 3727d38..a01807a 100644
--- a/javatests/com/google/gerrit/gpg/PublicKeyStoreTest.java
+++ b/javatests/com/google/gerrit/gpg/PublicKeyStoreTest.java
@@ -33,6 +33,7 @@
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
+import java.util.Locale;
import java.util.Set;
import java.util.TreeSet;
import org.bouncycastle.openpgp.PGPPublicKey;
@@ -80,7 +81,7 @@
PGPPublicKey key = validKeyWithoutExpiration().getPublicKey();
String objId = keyObjectId(key.getKeyID()).name();
assertEquals("ed0625dc46328a8c000000000000000000000000", objId);
- assertEquals(keyIdToString(key.getKeyID()).toLowerCase(), objId.substring(8, 16));
+ assertEquals(keyIdToString(key.getKeyID()).toLowerCase(Locale.US), objId.substring(8, 16));
}
@Test
diff --git a/javatests/com/google/gerrit/httpd/raw/ResourceServletTest.java b/javatests/com/google/gerrit/httpd/raw/ResourceServletTest.java
index 36641fe..8a2de7d 100644
--- a/javatests/com/google/gerrit/httpd/raw/ResourceServletTest.java
+++ b/javatests/com/google/gerrit/httpd/raw/ResourceServletTest.java
@@ -40,6 +40,7 @@
import java.time.LocalDateTime;
import java.time.Month;
import java.time.ZoneOffset;
+import java.util.Locale;
import java.util.concurrent.atomic.AtomicLong;
import java.util.zip.GZIPInputStream;
import org.junit.Before;
@@ -331,7 +332,7 @@
}
private static void assertCacheable(FakeHttpServletResponse res, boolean revalidate) {
- String header = res.getHeader("Cache-Control").toLowerCase();
+ String header = res.getHeader("Cache-Control").toLowerCase(Locale.US);
assertThat(header).contains("public");
if (revalidate) {
assertThat(header).contains("must-revalidate");
diff --git a/javatests/com/google/gerrit/server/IdentifiedUserTest.java b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
index 30ae4aa..ce045f7 100644
--- a/javatests/com/google/gerrit/server/IdentifiedUserTest.java
+++ b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
@@ -37,6 +37,7 @@
import com.google.inject.Injector;
import java.util.Arrays;
import java.util.HashSet;
+import java.util.Locale;
import java.util.Set;
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
@@ -114,14 +115,14 @@
@Test
public void emailsExistence() {
assertThat(identifiedUser.hasEmailAddress(TEST_CASES[0])).isTrue();
- assertThat(identifiedUser.hasEmailAddress(TEST_CASES[1].toLowerCase())).isTrue();
+ assertThat(identifiedUser.hasEmailAddress(TEST_CASES[1].toLowerCase(Locale.US))).isTrue();
assertThat(identifiedUser.hasEmailAddress(TEST_CASES[1])).isTrue();
- assertThat(identifiedUser.hasEmailAddress(TEST_CASES[1].toUpperCase())).isTrue();
+ assertThat(identifiedUser.hasEmailAddress(TEST_CASES[1].toUpperCase(Locale.US))).isTrue();
/* assert again to test cached email address by IdentifiedUser.validEmails */
assertThat(identifiedUser.hasEmailAddress(TEST_CASES[1])).isTrue();
assertThat(identifiedUser.hasEmailAddress(TEST_CASES[2])).isTrue();
- assertThat(identifiedUser.hasEmailAddress(TEST_CASES[2].toLowerCase())).isTrue();
+ assertThat(identifiedUser.hasEmailAddress(TEST_CASES[2].toLowerCase(Locale.US))).isTrue();
assertThat(identifiedUser.hasEmailAddress("non-exist@email.com")).isFalse();
/* assert again to test cached email address by IdentifiedUser.invalidEmails */
diff --git a/javatests/com/google/gerrit/server/cache/PerThreadProjectCacheTest.java b/javatests/com/google/gerrit/server/cache/PerThreadProjectCacheTest.java
index b9c1897..055b95d 100644
--- a/javatests/com/google/gerrit/server/cache/PerThreadProjectCacheTest.java
+++ b/javatests/com/google/gerrit/server/cache/PerThreadProjectCacheTest.java
@@ -25,7 +25,7 @@
try (PerThreadCache cache = PerThreadCache.create()) {
PerThreadCache.Key<Project.NameKey> key =
PerThreadCache.Key.create(Project.NameKey.class, Project.nameKey("test-project"));
- String unused = PerThreadProjectCache.getOrCompute(key, () -> "cached");
+ PerThreadProjectCache.getOrCompute(key, () -> "cached");
String value = PerThreadProjectCache.getOrCompute(key, () -> "directly served");
assertThat(value).isEqualTo("cached");
}
@@ -38,12 +38,12 @@
for (int i = 0; i < 50; i++) {
PerThreadCache.Key<Project.NameKey> key =
PerThreadCache.Key.create(Project.NameKey.class, Project.nameKey("test-project" + i));
- String unused = PerThreadProjectCache.getOrCompute(key, () -> "cached");
+ PerThreadProjectCache.getOrCompute(key, () -> "cached");
}
// Assert that the value was not persisted
PerThreadCache.Key<Project.NameKey> key =
PerThreadCache.Key.create(Project.NameKey.class, "Project" + 1000);
- String unused = PerThreadProjectCache.getOrCompute(key, () -> "new value");
+ PerThreadProjectCache.getOrCompute(key, () -> "new value");
String value = PerThreadProjectCache.getOrCompute(key, () -> "directly served");
assertThat(value).isEqualTo("directly served");
}
diff --git a/javatests/com/google/gerrit/server/index/IndexedFieldTest.java b/javatests/com/google/gerrit/server/index/IndexedFieldTest.java
index dacc37d..5029334 100644
--- a/javatests/com/google/gerrit/server/index/IndexedFieldTest.java
+++ b/javatests/com/google/gerrit/server/index/IndexedFieldTest.java
@@ -61,7 +61,6 @@
import org.junit.runner.RunWith;
/** Tests for {@link com.google.gerrit.index.IndexedField} */
-@SuppressWarnings("serial")
@RunWith(Theories.class)
public class IndexedFieldTest {
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index 1fede32..aea4b95 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -94,6 +94,7 @@
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
+import java.util.Locale;
import java.util.Optional;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
@@ -242,7 +243,7 @@
assertQuery(user5.email, user5);
assertQuery("email:" + user5.email, user5);
- assertQuery("email:" + user5.email.toUpperCase(), user5);
+ assertQuery("email:" + user5.email.toUpperCase(Locale.US), user5);
}
@Test
@@ -289,7 +290,7 @@
assertQuery(user1.username, user1);
assertQuery("username:" + user1.username, user1);
- assertQuery("username:" + user1.username.toUpperCase(), user1);
+ assertQuery("username:" + user1.username.toUpperCase(Locale.US), user1);
}
@Test
diff --git a/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java b/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java
index f39b875..18714ac 100644
--- a/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java
+++ b/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java
@@ -171,7 +171,7 @@
@Override
public void addHeader(String name, String value) {
- headers.put(name.toLowerCase(), value);
+ headers.put(name.toLowerCase(Locale.US), value);
}
@Override
@@ -181,7 +181,7 @@
@Override
public boolean containsHeader(String name) {
- return headers.containsKey(name.toLowerCase());
+ return headers.containsKey(name.toLowerCase(Locale.US));
}
@Override
@@ -232,13 +232,13 @@
@Override
public void setHeader(String name, String value) {
- headers.removeAll(name.toLowerCase());
+ headers.removeAll(name.toLowerCase(Locale.US));
addHeader(name, value);
}
@Override
public void setIntHeader(String name, int value) {
- headers.removeAll(name.toLowerCase());
+ headers.removeAll(name.toLowerCase(Locale.US));
addIntHeader(name, value);
}
@@ -262,7 +262,7 @@
@Override
public String getHeader(String name) {
- return Iterables.getFirst(headers.get(requireNonNull(name.toLowerCase())), null);
+ return Iterables.getFirst(headers.get(requireNonNull(name.toLowerCase(Locale.US))), null);
}
@Override
@@ -272,7 +272,7 @@
@Override
public Collection<String> getHeaders(String name) {
- return headers.get(requireNonNull(name.toLowerCase()));
+ return headers.get(requireNonNull(name.toLowerCase(Locale.US)));
}
public byte[] getActualBody() {
diff --git a/modules/jgit b/modules/jgit
index 176f17d..0687c73 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit 176f17d05ec154ce455ab2bde7429017d43d67fb
+Subproject commit 0687c73a12b5157c350de430f62ea8243d813e19
diff --git a/plugins/download-commands b/plugins/download-commands
index b4209a5..b90e523 160000
--- a/plugins/download-commands
+++ b/plugins/download-commands
@@ -1 +1 @@
-Subproject commit b4209a5a4c334077b255002cbadc2ef659adee3c
+Subproject commit b90e523f589a0e2902823233010163f453243926
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index 10db2cf..158435d 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit 10db2cf772989d031c6f3558010c51fe07cf9722
+Subproject commit 158435d2aa9f8729e4e78835969e54701af9203b
diff --git a/plugins/webhooks b/plugins/webhooks
index 16110f3..1dc0a71 160000
--- a/plugins/webhooks
+++ b/plugins/webhooks
@@ -1 +1 @@
-Subproject commit 16110f320dd5b6a40af87eaba4bf3af60cb0efd1
+Subproject commit 1dc0a718839f8872a59c189da7243ee77a4fe782
diff --git a/polygerrit-ui/app/api/gerrit.ts b/polygerrit-ui/app/api/gerrit.ts
index a5f7731..404907a 100644
--- a/polygerrit-ui/app/api/gerrit.ts
+++ b/polygerrit-ui/app/api/gerrit.ts
@@ -17,7 +17,7 @@
export declare interface Gerrit {
install(
callback: (plugin: PluginApi) => void,
- opt_version?: string,
+ version?: string,
src?: string
): void;
styles: Styles;
diff --git a/polygerrit-ui/app/api/rest-api.ts b/polygerrit-ui/app/api/rest-api.ts
index 6c00838..9a483f0 100644
--- a/polygerrit-ui/app/api/rest-api.ts
+++ b/polygerrit-ui/app/api/rest-api.ts
@@ -1095,14 +1095,14 @@
* https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#web-link-info
*/
export declare interface WebLinkInfo {
- /** The link name. */
+ /** The text to be linkified. */
name: string;
+ /** Tooltip to show when hovering over the link. */
+ tooltip?: string;
/** The link URL. */
url: string;
/** URL to the icon of the link. */
image_url?: string;
- /* Value of the "target" attribute for anchor elements. */
- target?: string;
}
/**
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-file-edit-dialog_test.ts b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-file-edit-dialog_test.ts
index 7621fac..e2da5d7 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-file-edit-dialog_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-file-edit-dialog_test.ts
@@ -10,14 +10,18 @@
import {GrCreateFileEditDialog} from './gr-create-file-edit-dialog';
import {stubRestApi, waitUntilCalled} from '../../../test/test-utils';
import {BranchName, RepoName} from '../../../api/rest-api';
-import {SinonStub} from 'sinon';
+import {SinonStubbedMember} from 'sinon';
import {testResolver} from '../../../test/common-test-setup';
-import {navigationToken} from '../../core/gr-navigation/gr-navigation';
+import {
+ NavigationService,
+ navigationToken,
+} from '../../core/gr-navigation/gr-navigation';
+import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
suite('gr-create-file-edit-dialog', () => {
let element: GrCreateFileEditDialog;
- let createChangeStub: SinonStub;
- let setUrlStub: SinonStub;
+ let createChangeStub: SinonStubbedMember<RestApiService['createChange']>;
+ let setUrlStub: SinonStubbedMember<NavigationService['setUrl']>;
setup(async () => {
createChangeStub = stubRestApi('createChange').resolves(createChange());
diff --git a/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list_test.ts b/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list_test.ts
index 34c88be..fa1a6a8 100644
--- a/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list_test.ts
@@ -390,7 +390,7 @@
test('fires page-error', async () => {
const response = {status: 404} as Response;
stubRestApi('getPlugins').callsFake(
- (_filter, _pluginsPerPage, _opt_offset, errFn) => {
+ (_filter, _pluginsPerPage, _offset, errFn) => {
if (errFn !== undefined) {
errFn(response);
}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
index 36b8507..048933b 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
@@ -43,7 +43,6 @@
AutocompleteCommitEvent,
ValueChangedEvent,
} from '../../../types/events';
-import {ifDefined} from 'lit/directives/if-defined.js';
import {resolve} from '../../../models/dependency';
import {createChangeUrl} from '../../../models/views/change';
import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
@@ -256,12 +255,7 @@
private renderWebLinks(webLink: WebLinkInfo) {
return html`
- <a
- class="weblink"
- href=${webLink.url}
- rel="noopener"
- target=${ifDefined(webLink.target)}
- >
+ <a class="weblink" href=${webLink.url} rel="noopener" target="_blank">
${webLink.name}
</a>
`;
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts
index 9a67fda..a2b23b4 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts
@@ -2303,7 +2303,7 @@
test('fires page-error', async () => {
const response = {status: 404} as Response;
stubRestApi('getRepoBranches').callsFake(
- (_filter, _repo, _reposBranchesPerPage, _opt_offset, errFn) => {
+ (_filter, _repo, _reposBranchesPerPage, _offset, errFn) => {
if (errFn !== undefined) {
errFn(response);
}
@@ -2501,7 +2501,7 @@
test('fires page-error', async () => {
const response = {status: 404} as Response;
stubRestApi('getRepoTags').callsFake(
- (_filter, _repo, _reposTagsPerPage, _opt_offset, errFn) => {
+ (_filter, _repo, _reposTagsPerPage, _offset, errFn) => {
if (errFn !== undefined) {
errFn(response);
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-action-bar/gr-change-list-action-bar_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-action-bar/gr-change-list-action-bar_test.ts
index 7a4f97c..4b7f0a3 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-action-bar/gr-change-list-action-bar_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-action-bar/gr-change-list-action-bar_test.ts
@@ -19,7 +19,7 @@
} from '../../../test/test-utils';
import {ChangeInfo, NumericChangeId} from '../../../types/common';
import './gr-change-list-action-bar';
-import type {GrChangeListActionBar} from './gr-change-list-action-bar';
+import {GrChangeListActionBar} from './gr-change-list-action-bar';
const change1 = {...createChange(), _number: 1 as NumericChangeId, actions: {}};
const change2 = {...createChange(), _number: 2 as NumericChangeId, actions: {}};
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow_test.ts
index 0cc3e18..af997a9 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow_test.ts
@@ -32,7 +32,7 @@
import {GrAutocomplete} from '../../shared/gr-autocomplete/gr-autocomplete';
import {GrButton} from '../../shared/gr-button/gr-button';
import './gr-change-list-hashtag-flow';
-import type {GrChangeListHashtagFlow} from './gr-change-list-hashtag-flow';
+import {GrChangeListHashtagFlow} from './gr-change-list-hashtag-flow';
suite('gr-change-list-hashtag-flow tests', () => {
let element: GrChangeListHashtagFlow;
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts
index 85e6212..d2f5fa2 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts
@@ -41,7 +41,7 @@
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
import './gr-change-list-reviewer-flow';
-import type {GrChangeListReviewerFlow} from './gr-change-list-reviewer-flow';
+import {GrChangeListReviewerFlow} from './gr-change-list-reviewer-flow';
const accounts: AccountInfo[] = [
createAccountWithIdNameAndEmail(0),
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow_test.ts
index d2dced2..489d8ee 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow_test.ts
@@ -32,7 +32,7 @@
import {GrAutocomplete} from '../../shared/gr-autocomplete/gr-autocomplete';
import {GrButton} from '../../shared/gr-button/gr-button';
import './gr-change-list-topic-flow';
-import type {GrChangeListTopicFlow} from './gr-change-list-topic-flow';
+import {GrChangeListTopicFlow} from './gr-change-list-topic-flow';
suite('gr-change-list-topic-flow tests', () => {
let element: GrChangeListTopicFlow;
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 6c7e661..117abd6 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
@@ -229,7 +229,7 @@
}
private calculateStartIndices(sections: ChangeListSection[]): number[] {
- const startIndices: number[] = new Array(sections.length).fill(0);
+ const startIndices = Array.from<number>({length: sections.length}).fill(0);
for (let i = 1; i < sections.length; ++i) {
startIndices[i] = startIndices[i - 1] + sections[i - 1].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 7e9735f..e201ab4 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
@@ -141,7 +141,10 @@
});
test('computeRelativeIndex', () => {
- element.sections = [{results: new Array(1)}, {results: new Array(2)}];
+ element.sections = [
+ {results: Array.from({length: 1})},
+ {results: Array.from({length: 2})},
+ ];
let selectedChangeIndex = 0;
assert.equal(
@@ -228,7 +231,10 @@
test('keyboard shortcuts', async () => {
sinon.stub(element, 'computeLabelNames');
- element.sections = [{results: new Array(1)}, {results: new Array(2)}];
+ element.sections = [
+ {results: Array.from({length: 1})},
+ {results: Array.from({length: 2})},
+ ];
element.selectedIndex = 0;
element.preferences = createDefaultPreferences();
element.config = createServerInfo();
@@ -300,7 +306,10 @@
queryAndAssert<HTMLInputElement>(query(item, '.selection'), 'input');
sinon.stub(element, 'computeLabelNames');
- element.sections = [{results: new Array(1)}, {results: new Array(2)}];
+ element.sections = [
+ {results: Array.from({length: 1})},
+ {results: Array.from({length: 2})},
+ ];
element.selectedIndex = 0;
element.preferences = createDefaultPreferences();
element.config = createServerInfo();
diff --git a/polygerrit-ui/app/elements/change/gr-comments-summary/gr-comments-summary.ts b/polygerrit-ui/app/elements/change/gr-comments-summary/gr-comments-summary.ts
index d0352f6..3d5533c 100644
--- a/polygerrit-ui/app/elements/change/gr-comments-summary/gr-comments-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-comments-summary/gr-comments-summary.ts
@@ -44,6 +44,9 @@
@property({type: Boolean})
emptyWhenNoComments = false;
+ @property({type: Boolean})
+ showAvatarForResolved = false;
+
@state()
selfAccount?: AccountInfo;
@@ -84,6 +87,9 @@
const unresolvedThreads = commentThreads.filter(isUnresolved);
const countUnresolvedComments = unresolvedThreads.length;
const unresolvedAuthors = this.getAccounts(unresolvedThreads);
+ const resolveAuthors = this.showAvatarForResolved
+ ? this.getAccounts(commentThreads.filter(isResolved))
+ : undefined;
return html`
${this.renderZeroState(countResolvedComments, countUnresolvedComments)}
${this.renderDraftChip()} ${this.renderMentionChip()}
@@ -91,7 +97,7 @@
countUnresolvedComments,
unresolvedAuthors
)}
- ${this.renderResolvedCommentsChip(countResolvedComments)}
+ ${this.renderResolvedCommentsChip(countResolvedComments, resolveAuthors)}
`;
}
@@ -163,8 +169,29 @@
>`;
}
- private renderResolvedCommentsChip(countResolvedComments: number) {
+ private renderResolvedCommentsChip(
+ countResolvedComments: number,
+ resolvedAuthors?: AccountInfo[]
+ ) {
if (!countResolvedComments) return nothing;
+ if (resolvedAuthors) {
+ return html` <gr-summary-chip
+ styleType=${SummaryChipStyles.CHECK}
+ category=${CommentTabState.SHOW_ALL}
+ .clickable=${this.clickableChips}
+ ><gr-avatar-stack .accounts=${resolvedAuthors} imageSize="32">
+ <gr-icon
+ slot="fallback"
+ icon="chat_bubble"
+ filled
+ class="unresolvedIcon"
+ >
+ </gr-icon> </gr-avatar-stack
+ >${this.showCommentCategoryName
+ ? `${countResolvedComments} resolved`
+ : `${countResolvedComments}`}</gr-summary-chip
+ >`;
+ }
return html` <gr-summary-chip
styleType=${SummaryChipStyles.CHECK}
category=${CommentTabState.SHOW_ALL}
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 13ca05c..c206d57 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
@@ -1872,14 +1872,14 @@
return '';
}
const commentThreadCount =
- this.changeComments.computeCommentThreadCount({
+ this.changeComments.computeCommentThreads({
patchNum: this.patchRange.basePatchNum,
path: file.__path,
- }) +
- this.changeComments.computeCommentThreadCount({
+ }).length +
+ this.changeComments.computeCommentThreads({
patchNum: this.patchRange.patchNum,
path: file.__path,
- });
+ }).length;
return commentThreadCount === 0 ? '' : `${commentThreadCount}c`;
}
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 c669209..13662982 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
@@ -168,7 +168,7 @@
// Render blank cells so that all same value votes are aligned
private renderBlankItems(position: string) {
const blankItemCount = this.computeBlankItemsCount(position);
- return new Array(blankItemCount)
+ return Array.from({length: blankItemCount})
.fill('')
.map(
() => html`
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
index 34292d6..4142d9f 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
@@ -5,7 +5,10 @@
*/
import '../../../test/common-test-setup';
import './gr-message';
-import {navigationToken} from '../../core/gr-navigation/gr-navigation';
+import {
+ NavigationService,
+ navigationToken,
+} from '../../core/gr-navigation/gr-navigation';
import {
createAccountWithIdNameAndEmail,
createChange,
@@ -39,7 +42,7 @@
} from '../../../types/events';
import {GrButton} from '../../shared/gr-button/gr-button';
import {CommentSide} from '../../../constants/constants';
-import {SinonStub} from 'sinon';
+import {SinonStubbedMember} from 'sinon';
import {html} from 'lit';
import {fixture, assert} from '@open-wc/testing';
import {testResolver} from '../../../test/common-test-setup';
@@ -423,7 +426,7 @@
});
suite('uploaded patchset X message navigates to X - 1 vs X', () => {
- let setUrlStub: SinonStub;
+ let setUrlStub: SinonStubbedMember<NavigationService['setUrl']>;
setup(() => {
element.change = {...createChange(), revisions: createRevisions(4)};
setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts
index e323ab1..ec43aea 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts
@@ -53,8 +53,8 @@
};
};
-const randomMessage = function (opt_params?: ChangeMessageInfo) {
- const params = opt_params || ({} as ChangeMessageInfo);
+const randomMessage = function (params?: ChangeMessageInfo) {
+ params = params || ({} as ChangeMessageInfo);
const author1 = {
_account_id: 1115495 as AccountId,
name: 'Andrew Bonventre',
@@ -71,7 +71,7 @@
};
function generateRandomMessages(count: number) {
- return new Array(count)
+ return Array.from({length: count})
.fill(undefined)
.map(() => randomMessage()) as ChangeMessageInfo[];
}
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index a1adfb8..05acb29 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -687,9 +687,7 @@
while (pos >= 0) {
const commit: CommitId = commits[pos].commit;
connected.push(commit);
- // TODO(TS): Ensure that both (commit and changeRevision) are string and use === instead
- // eslint-disable-next-line eqeqeq
- if (commit == changeRevision) {
+ if (commit === changeRevision) {
break;
}
pos--;
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index 38acf80..7c85bb6 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -89,7 +89,6 @@
fireError,
fire,
fireNoBubble,
- fireNoBubbleNoCompose,
fireIronAnnounce,
fireReload,
fireServerError,
@@ -121,7 +120,7 @@
import {customElement, property, state, query} from 'lit/decorators.js';
import {subscribe} from '../../lit/subscription-controller';
import {configModelToken} from '../../../models/config/config-model';
-import {hasHumanReviewer, isOwner} from '../../../utils/change-util';
+import {hasHumanReviewer} from '../../../utils/change-util';
import {commentsModelToken} from '../../../models/comments/comments-model';
import {
CommentEditingChangedDetail,
@@ -301,9 +300,6 @@
reviewerPendingConfirmation: SuggestedReviewerGroupInfo | null = null;
@state()
- sendButtonLabel?: string;
-
- @state()
savingComments = false;
@state()
@@ -336,14 +332,14 @@
newAttentionSet: Set<UserId> = new Set();
@state()
- sendDisabled?: boolean;
-
- @state()
patchsetLevelDraftIsResolved = true;
@state()
patchsetLevelComment?: UnsavedInfo | DraftInfo;
+ @state()
+ isOwner = false;
+
private readonly restApiService: RestApiService =
getAppContext().restApiService;
@@ -619,6 +615,11 @@
);
subscribe(
this,
+ () => this.getChangeModel().isOwner$,
+ x => (this.isOwner = x)
+ );
+ subscribe(
+ this,
() => this.getCommentsModel().mentionedUsersInDrafts$,
x => {
this.mentionedUsers = x;
@@ -712,10 +713,6 @@
}
if (changedProperties.has('canBeStarted')) {
this.computeMessagePlaceholder();
- this.computeSendButtonLabel();
- }
- if (changedProperties.has('sendDisabled')) {
- this.sendDisabledChanged();
}
if (changedProperties.has('attentionExpanded')) {
this.onAttentionExpandedChange();
@@ -743,7 +740,6 @@
override render() {
if (!this.change) return;
- this.sendDisabled = this.computeSendButtonDisabled();
return html`
<div tabindex="-1">
<section class="peopleContainer">
@@ -1015,7 +1011,7 @@
<gr-button
class="edit-attention-button"
@click=${this.handleAttentionModify}
- ?disabled=${this.sendDisabled}
+ ?disabled=${this.isSendDisabled()}
link
position-below
data-label="Edit"
@@ -1214,10 +1210,12 @@
<gr-button
id="sendButton"
primary
- ?disabled=${this.sendDisabled}
+ ?disabled=${this.isSendDisabled()}
class="action send"
- @click=${this.sendTapHandler}
- >${this.sendButtonLabel}
+ @click=${this.sendClickHandler}
+ >${this.canBeStarted
+ ? ButtonLabels.SEND + ' and ' + ButtonLabels.START_REVIEW
+ : ButtonLabels.SEND}
</gr-button>
</gr-tooltip-content>
</div>
@@ -1352,6 +1350,7 @@
);
}
+ // visible for testing
async send(includeComments: boolean, startReview: boolean) {
this.reporting.time(Timing.SEND_REPLY);
const labels = this.getLabelScores().getLabelValues();
@@ -1473,16 +1472,11 @@
}
chooseFocusTarget() {
- if (!isOwner(this.change, this.account)) return FocusTarget.BODY;
+ if (!this.isOwner) return FocusTarget.BODY;
if (hasHumanReviewer(this.change)) return FocusTarget.BODY;
return FocusTarget.REVIEWERS;
}
- isOwner(account?: AccountInfo, change?: ParsedChangeInfo | ChangeInfo) {
- if (!account || !change || !change.owner) return false;
- return account._account_id === change.owner._account_id;
- }
-
handle400Error(r?: Response | null) {
if (!r) throw new Error('Response is empty.');
let response: Response = r;
@@ -1557,8 +1551,8 @@
fire(this, 'iron-resize', {});
}
- computeAttentionButtonTitle(sendDisabled?: boolean) {
- return sendDisabled
+ computeAttentionButtonTitle() {
+ return this.isSendDisabled()
? 'Modify the attention set by adding a comment or use the account ' +
'hovercard in the change page.'
: 'Edit attention set changes';
@@ -1606,7 +1600,6 @@
? this.draftCommentThreads
: [];
const hasVote = !!this.labelsChanged;
- const isOwner = this.isOwner(this.account, this.change);
const isUploader = this.uploader?._account_id === this.account._account_id;
this.attentionCcsCount = removeServiceUsers(this.ccs).length;
@@ -1640,7 +1633,7 @@
.filter(
r =>
isAccountNewlyAdded(r, ReviewerState.REVIEWER, this.change) ||
- (this.canBeStarted && isOwner)
+ (this.canBeStarted && this.isOwner)
)
.filter(notIsReviewerAndHasDraftOrLabel)
.forEach(r => newAttention.add((r as AccountInfo)._account_id!));
@@ -1649,7 +1642,7 @@
if (this.uploader?._account_id && !isUploader) {
newAttention.add(this.uploader._account_id);
}
- if (this.change.owner?._account_id && !isOwner) {
+ if (this.change.owner?._account_id && !this.isOwner) {
newAttention.add(this.change.owner._account_id);
}
}
@@ -1677,18 +1670,11 @@
}
computeShowAttentionTip() {
- if (
- !this.account ||
- !this.change?.owner ||
- !this.currentAttentionSet ||
- !this.newAttentionSet
- )
- return false;
- const isOwner = this.account._account_id === this.change.owner._account_id;
+ if (!this.currentAttentionSet || !this.newAttentionSet) return false;
const addedIds = [...this.newAttentionSet].filter(
id => !this.currentAttentionSet.has(id)
);
- return isOwner && addedIds.length > 2;
+ return this.isOwner && addedIds.length > 2;
}
computeCommentAccounts(threads: CommentThread[]) {
@@ -1710,13 +1696,15 @@
}
computeShowNoAttentionUpdate() {
- return this.sendDisabled || this.computeNewAttentionAccounts().length === 0;
+ return (
+ this.isSendDisabled() || this.computeNewAttentionAccounts().length === 0
+ );
}
computeDoNotUpdateMessage() {
if (!this.currentAttentionSet || !this.newAttentionSet) return '';
if (
- this.sendDisabled ||
+ this.isSendDisabled() ||
areSetsEqual(this.currentAttentionSet, this.newAttentionSet)
) {
return 'No changes to the attention set.';
@@ -1828,32 +1816,30 @@
this.rebuildReviewerArrays();
}
- saveClickHandler(e: Event) {
+ private saveClickHandler(e: Event) {
e.preventDefault();
- if (!this.ccsList?.submitEntryText()) {
- // Do not proceed with the save if there is an invalid email entry in
- // the text field of the CC entry.
- return;
+ this.submit(false);
+ }
+
+ private sendClickHandler(e: Event) {
+ e.preventDefault();
+ this.submit(this.canBeStarted);
+ }
+
+ private submit(startReview?: boolean) {
+ if (startReview === undefined) {
+ startReview = this.isOwner && this.canBeStarted;
}
- this.send(this.includeComments, false);
- }
-
- sendTapHandler(e: Event) {
- e.preventDefault();
- this.submit();
- }
-
- submit() {
if (!this.ccsList?.submitEntryText()) {
// Do not proceed with the send if there is an invalid email entry in
// the text field of the CC entry.
return;
}
- if (this.sendDisabled) {
+ if (this.isSendDisabled()) {
fireAlert(this, EMPTY_REPLY_MESSAGE);
return;
}
- return this.send(this.includeComments, this.canBeStarted).catch(err => {
+ return this.send(this.includeComments, startReview).catch(err => {
fireError(this, `Error submitting review ${err}`);
});
}
@@ -1965,12 +1951,6 @@
this.cancel();
}
- computeSendButtonLabel() {
- this.sendButtonLabel = this.canBeStarted
- ? ButtonLabels.SEND + ' and ' + ButtonLabels.START_REVIEW
- : ButtonLabels.SEND;
- }
-
computeSendButtonTooltip(canBeStarted?: boolean, commentEditing?: boolean) {
if (commentEditing) {
return ButtonTooltips.DISABLED_COMMENT_EDITING;
@@ -1982,7 +1962,8 @@
return savingComments ? 'saving' : '';
}
- computeSendButtonDisabled() {
+ // visible for testing
+ isSendDisabled() {
if (
this.canBeStarted === undefined ||
this.patchsetLevelDraftMessage === undefined ||
@@ -2030,10 +2011,6 @@
this.pluginMessage = message;
}
- sendDisabledChanged() {
- fireNoBubbleNoCompose(this, 'send-disabled-changed', {});
- }
-
getReviewerSuggestionsProvider(change?: ChangeInfo | ParsedChangeInfo) {
if (!change) return;
const provider = new GrReviewerSuggestionsProvider(
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
index aebe82e..126343e 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
@@ -16,7 +16,11 @@
stubRestApi,
waitUntilVisible,
} from '../../../test/test-utils';
-import {ChangeStatus, ReviewerState} from '../../../constants/constants';
+import {
+ ChangeStatus,
+ DraftsAction,
+ ReviewerState,
+} from '../../../constants/constants';
import {JSON_PREFIX} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
import {StandardLabels} from '../../../utils/label-util';
import {
@@ -65,6 +69,7 @@
CommentsModel,
commentsModelToken,
} from '../../../models/comments/comments-model';
+import {isOwner} from '../../../utils/change-util';
function cloneableResponse(status: number, text: string) {
return {
@@ -177,9 +182,9 @@
);
}
- function interceptSaveReview() {
+ function interceptSaveReview(): Promise<ReviewInput> {
let resolver: (review: ReviewInput) => void;
- const promise = new Promise(resolve => {
+ const promise = new Promise<ReviewInput>(resolve => {
resolver = resolve;
});
stubSaveReview((review: ReviewInput) => {
@@ -257,7 +262,7 @@
<span> No changes to the attention set. </span>
<gr-tooltip-content
has-tooltip=""
- title="Edit attention set changes"
+ title="Modify the attention set by adding a comment or use the account hovercard in the change page."
>
<gr-button
aria-disabled="true"
@@ -459,14 +464,17 @@
const review = await saveReviewPromise;
assert.deepEqual(review, {
- drafts: 'PUBLISH_ALL_REVISIONS',
+ drafts: DraftsAction.PUBLISH_ALL_REVISIONS,
labels: {
'Code-Review': 0,
Verified: 0,
},
reviewers: [],
add_to_attention_set: [
- {reason: '<GERRIT_ACCOUNT_1> replied on the change', user: 999},
+ {
+ reason: '<GERRIT_ACCOUNT_1> replied on the change',
+ user: 999 as UserId,
+ },
],
remove_from_attention_set: [],
ignore_automatic_attention_set_rules: true,
@@ -492,13 +500,16 @@
const review = await saveReviewPromise;
assert.deepEqual(review, {
- drafts: 'PUBLISH_ALL_REVISIONS',
+ drafts: DraftsAction.PUBLISH_ALL_REVISIONS,
labels: {
'Code-Review': 0,
Verified: 0,
},
add_to_attention_set: [
- {reason: '<GERRIT_ACCOUNT_123> replied on the change', user: 314},
+ {
+ reason: '<GERRIT_ACCOUNT_123> replied on the change',
+ user: 314 as UserId,
+ },
],
reviewers: [],
ready: true,
@@ -523,14 +534,17 @@
const review = await saveReviewPromise;
assert.deepEqual(review, {
- drafts: 'PUBLISH_ALL_REVISIONS',
+ drafts: DraftsAction.PUBLISH_ALL_REVISIONS,
labels: {
'Code-Review': 0,
Verified: 0,
},
add_to_attention_set: [
// Name coming from createUserConfig in test-data-generator
- {reason: 'Name of user not set replied on the change', user: 314},
+ {
+ reason: 'Name of user not set replied on the change',
+ user: 314 as UserId,
+ },
],
reviewers: [],
ready: true,
@@ -598,6 +612,7 @@
element._ccs = [];
element.draftCommentThreads = draftThreads;
element.includeComments = includeComments;
+ element.isOwner = isOwner(change, element.account);
await element.updateComplete;
@@ -1067,6 +1082,7 @@
// If the change is "work in progress" and the owner sends a reply, then
// add all reviewers.
element.canBeStarted = true;
+ element.isOwner = isOwner(element.change, element.account);
element.computeNewAttention();
await element.updateComplete;
assert.sameMembers(
@@ -1076,6 +1092,7 @@
// ... but not when someone else replies.
element.account = {_account_id: 4 as AccountId};
+ element.isOwner = isOwner(element.change, element.account);
element.computeNewAttention();
assert.sameMembers([...element.newAttentionSet], []);
});
@@ -1195,14 +1212,17 @@
await waitUntil(() => element.disabled === false);
assert.equal(element.patchsetLevelDraftMessage.length, 0);
assert.deepEqual(review, {
- drafts: 'PUBLISH_ALL_REVISIONS',
+ drafts: DraftsAction.PUBLISH_ALL_REVISIONS,
labels: {
'Code-Review': -1,
Verified: -1,
},
reviewers: [],
add_to_attention_set: [
- {user: 999, reason: '<GERRIT_ACCOUNT_1> replied on the change'},
+ {
+ user: 999 as UserId,
+ reason: '<GERRIT_ACCOUNT_1> replied on the change',
+ },
],
remove_from_attention_set: [],
ignore_automatic_attention_set_rules: true,
@@ -1231,14 +1251,17 @@
const review = await saveReviewPromise;
await element.updateComplete;
assert.deepEqual(review, {
- drafts: 'KEEP',
+ drafts: DraftsAction.KEEP,
labels: {
'Code-Review': 0,
Verified: 0,
},
reviewers: [],
add_to_attention_set: [
- {reason: '<GERRIT_ACCOUNT_1> replied on the change', user: 999},
+ {
+ reason: '<GERRIT_ACCOUNT_1> replied on the change',
+ user: 999 as UserId,
+ },
],
remove_from_attention_set: [],
ignore_automatic_attention_set_rules: true,
@@ -1636,10 +1659,10 @@
});
test('chooseFocusTarget', () => {
- element.account = undefined;
+ element.isOwner = false;
assert.equal(element.chooseFocusTarget(), element.FocusTarget.BODY);
- element.account = element.change!.owner;
+ element.isOwner = true;
assert.equal(element.chooseFocusTarget(), element.FocusTarget.REVIEWERS);
element.change!.reviewers.REVIEWER = [createAccountWithId(314)];
@@ -1974,13 +1997,13 @@
const mapReviewer = function (
reviewer: AccountInfo,
- opt_state?: ReviewerState
+ state?: ReviewerState
) {
const result: ReviewerInput = {
reviewer: reviewer._account_id as AccountId,
};
- if (opt_state) {
- result.state = opt_state;
+ if (state) {
+ result.state = state;
}
return result;
};
@@ -2087,16 +2110,26 @@
pressKey(element, Key.ENTER);
});
- test('emit send on ctrl+enter key', async () => {
- // required so that "Send" button is enabled
+ test('send and start review on ctrl+enter for owner', async () => {
element.canBeStarted = true;
+ element.isOwner = true;
await element.updateComplete;
- stubSaveReview(() => undefined);
- const promise = mockPromise();
- element.addEventListener('send', () => promise.resolve());
+ const savePromise = interceptSaveReview();
pressKey(element, Key.ENTER, Modifier.CTRL_KEY);
- await promise;
+ const reviewInput = await savePromise;
+ assert.isTrue(reviewInput.ready);
+ });
+
+ test('save on ctrl+enter for reviewer', async () => {
+ element.canBeStarted = true;
+ element.isOwner = false;
+ await element.updateComplete;
+
+ const savePromise = interceptSaveReview();
+ pressKey(element, Key.ENTER, Modifier.CTRL_KEY);
+ const reviewInput = await savePromise;
+ assert.isUndefined(reviewInput.ready);
});
test('computeMessagePlaceholder', async () => {
@@ -2112,14 +2145,14 @@
);
});
- test('computeSendButtonLabel', async () => {
+ test('sendButton text', async () => {
element.canBeStarted = false;
await element.updateComplete;
- assert.equal(element.sendButtonLabel, 'Send');
+ assert.equal(element.sendButton?.innerText, 'SEND');
element.canBeStarted = true;
await element.updateComplete;
- assert.equal(element.sendButtonLabel, 'Send and Start review');
+ assert.equal(element.sendButton?.innerText, 'SEND AND START REVIEW');
});
test('handle400Error reviewers and CCs', async () => {
@@ -2239,7 +2272,7 @@
});
});
- test('computeSendButtonDisabled_canBeStarted', () => {
+ test('isSendDisabled_canBeStarted', () => {
// Mock canBeStarted
element.canBeStarted = true;
element.draftCommentThreads = [];
@@ -2250,10 +2283,10 @@
element.disabled = false;
element.commentEditing = false;
element.account = makeAccount();
- assert.isFalse(element.computeSendButtonDisabled());
+ assert.isFalse(element.isSendDisabled());
});
- test('computeSendButtonDisabled_allFalse', () => {
+ test('isSendDisabled_allFalse', () => {
// Mock everything false
element.canBeStarted = false;
element.draftCommentThreads = [];
@@ -2264,10 +2297,10 @@
element.disabled = false;
element.commentEditing = false;
element.account = makeAccount();
- assert.isTrue(element.computeSendButtonDisabled());
+ assert.isTrue(element.isSendDisabled());
});
- test('computeSendButtonDisabled_draftCommentsSend', () => {
+ test('isSendDisabled_draftCommentsSend', () => {
// Mock nonempty comment draft array; with sending comments.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
@@ -2278,10 +2311,10 @@
element.disabled = false;
element.commentEditing = false;
element.account = makeAccount();
- assert.isFalse(element.computeSendButtonDisabled());
+ assert.isFalse(element.isSendDisabled());
});
- test('computeSendButtonDisabled_draftCommentsDoNotSend', () => {
+ test('isSendDisabled_draftCommentsDoNotSend', () => {
// Mock nonempty comment draft array; without sending comments.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
@@ -2293,10 +2326,10 @@
element.commentEditing = false;
element.account = makeAccount();
- assert.isTrue(element.computeSendButtonDisabled());
+ assert.isTrue(element.isSendDisabled());
});
- test('computeSendButtonDisabled_changeMessage', () => {
+ test('isSendDisabled_changeMessage', () => {
// Mock nonempty change message.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
@@ -2308,10 +2341,10 @@
element.commentEditing = false;
element.account = makeAccount();
- assert.isFalse(element.computeSendButtonDisabled());
+ assert.isFalse(element.isSendDisabled());
});
- test('computeSendButtonDisabledreviewersChanged', () => {
+ test('isSendDisabledreviewersChanged', () => {
// Mock reviewers mutated.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
@@ -2323,10 +2356,10 @@
element.commentEditing = false;
element.account = makeAccount();
- assert.isFalse(element.computeSendButtonDisabled());
+ assert.isFalse(element.isSendDisabled());
});
- test('computeSendButtonDisabled_labelsChanged', () => {
+ test('isSendDisabled_labelsChanged', () => {
// Mock labels changed.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
@@ -2338,10 +2371,10 @@
element.commentEditing = false;
element.account = makeAccount();
- assert.isFalse(element.computeSendButtonDisabled());
+ assert.isFalse(element.isSendDisabled());
});
- test('computeSendButtonDisabled_dialogDisabled', () => {
+ test('isSendDisabled_dialogDisabled', () => {
// Whole dialog is disabled.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
@@ -2353,10 +2386,10 @@
element.commentEditing = false;
element.account = makeAccount();
- assert.isTrue(element.computeSendButtonDisabled());
+ assert.isTrue(element.isSendDisabled());
});
- test('computeSendButtonDisabled_existingVote', async () => {
+ test('isSendDisabled_existingVote', async () => {
const account = createAccountWithId();
(
element.change!.labels![StandardLabels.CODE_REVIEW]! as DetailedLabelInfo
@@ -2372,7 +2405,7 @@
element.account = account;
// User has already voted.
- assert.isFalse(element.computeSendButtonDisabled());
+ assert.isFalse(element.isSendDisabled());
});
test('_submit blocked when no mutations exist', async () => {
@@ -2428,19 +2461,19 @@
});
test('send button updates state as text is typed in patchset comment', async () => {
- assert.isTrue(element.computeSendButtonDisabled());
+ assert.isTrue(element.isSendDisabled());
queryAndAssert<GrComment>(element, '#patchsetLevelComment').messageText =
'hello';
await waitUntil(() => element.patchsetLevelDraftMessage === 'hello');
- assert.isFalse(element.computeSendButtonDisabled());
+ assert.isFalse(element.isSendDisabled());
queryAndAssert<GrComment>(element, '#patchsetLevelComment').messageText =
'';
await waitUntil(() => element.patchsetLevelDraftMessage === '');
- assert.isTrue(element.computeSendButtonDisabled());
+ assert.isTrue(element.isSendDisabled());
});
test('sending patchset level comment', async () => {
@@ -2468,14 +2501,17 @@
assert.deepEqual(autoSaveStub.callCount, 1);
assert.deepEqual(review, {
- drafts: 'PUBLISH_ALL_REVISIONS',
+ drafts: DraftsAction.PUBLISH_ALL_REVISIONS,
labels: {
'Code-Review': 0,
Verified: 0,
},
reviewers: [],
add_to_attention_set: [
- {reason: '<GERRIT_ACCOUNT_1> replied on the change', user: 999},
+ {
+ reason: '<GERRIT_ACCOUNT_1> replied on the change',
+ user: 999 as UserId,
+ },
],
remove_from_attention_set: [],
ignore_automatic_attention_set_rules: true,
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
index b317b97..36fadff 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
@@ -5,7 +5,10 @@
*/
import '../../../test/common-test-setup';
import './gr-apply-fix-dialog';
-import {navigationToken} from '../../core/gr-navigation/gr-navigation';
+import {
+ NavigationService,
+ navigationToken,
+} from '../../core/gr-navigation/gr-navigation';
import {queryAndAssert, stubRestApi} from '../../../test/test-utils';
import {GrApplyFixDialog} from './gr-apply-fix-dialog';
import {PatchSetNum} from '../../../types/common';
@@ -20,12 +23,12 @@
import {OpenFixPreviewEventDetail} from '../../../types/events';
import {GrButton} from '../../shared/gr-button/gr-button';
import {fixture, html, assert} from '@open-wc/testing';
-import {SinonStub} from 'sinon';
+import {SinonStubbedMember} from 'sinon';
import {testResolver} from '../../../test/common-test-setup';
suite('gr-apply-fix-dialog tests', () => {
let element: GrApplyFixDialog;
- let setUrlStub: SinonStub;
+ let setUrlStub: SinonStubbedMember<NavigationService['setUrl']>;
const TWO_FIXES: OpenFixPreviewEventDetail = {
patchNum: 2 as PatchSetNum,
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 1f16e01..6567e33 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -366,9 +366,9 @@
}
/**
- * Computes the number of comment threads in a given file or patch.
+ * Computes the comment threads in a given file or patch.
*/
- computeCommentThreadCount(
+ computeCommentThreads(
file: PatchSetFile | PatchNumOnly,
ignorePatchsetLevelComments?: boolean
) {
@@ -383,7 +383,7 @@
let threads = createCommentThreads(comments);
if (ignorePatchsetLevelComments)
threads = threads.filter(thread => !isPatchsetLevel(thread));
- return threads.length;
+ return threads;
}
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.ts
index 9263cd5..d337521 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.ts
@@ -911,30 +911,47 @@
);
});
- test('computeCommentThreadCount', () => {
+ test('computeCommentThreads - check length', () => {
assert.equal(
- changeComments.computeCommentThreadCount({
+ changeComments.computeCommentThreads({
patchNum: 2 as PatchSetNum,
path: 'file/one',
- }),
+ }).length,
3
);
- assert.equal(
- changeComments.computeCommentThreadCount({
+ assert.deepEqual(
+ changeComments.computeCommentThreads({
patchNum: 1 as PatchSetNum,
path: 'file/one',
}),
- 0
+ []
);
assert.equal(
- changeComments.computeCommentThreadCount({
+ changeComments.computeCommentThreads({
patchNum: 2 as PatchSetNum,
path: 'file/three',
- }),
+ }).length,
1
);
});
+ test('computeCommentThreads - check content', () => {
+ const expectedThreads: CommentThread[] = [
+ {
+ ...createCommentThread([{...comments[9], path: 'file/four'}]),
+ },
+ {
+ ...createCommentThread([{...comments[10], path: 'file/four'}]),
+ },
+ ];
+ assert.deepEqual(
+ changeComments.computeCommentThreads({
+ path: 'file/four',
+ }),
+ expectedThreads
+ );
+ });
+
test('computeDraftCount', () => {
assert.equal(
changeComments.computeDraftCount({
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
index dd0eecd..42b3344 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
@@ -46,7 +46,7 @@
import {GrDiffHost, LineInfo} from './gr-diff-host';
import {DiffInfo, DiffViewMode, IgnoreWhitespaceType} from '../../../api/diff';
import {ErrorCallback} from '../../../api/rest';
-import {SinonStub} from 'sinon';
+import {SinonStub, SinonStubbedMember} from 'sinon';
import {RunResult} from '../../../models/checks/checks-model';
import {GrCommentThread} from '../../shared/gr-comment-thread/gr-comment-thread';
import {assertIsDefined} from '../../../utils/common-util';
@@ -54,11 +54,13 @@
import {testResolver} from '../../../test/common-test-setup';
import {userModelToken, UserModel} from '../../../models/user/user-model';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
+import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
suite('gr-diff-host tests', () => {
let element: GrDiffHost;
let account = createAccountDetailWithId(1);
- let getDiffRestApiStub: SinonStub;
+ let getDiffRestApiStub: SinonStubbedMember<RestApiService['getDiff']>;
let userModel: UserModel;
setup(async () => {
@@ -816,7 +818,7 @@
});
suite('reportDiff', () => {
- let reportStub: SinonStub;
+ let reportStub: SinonStubbedMember<ReportingService['reportInteraction']>;
setup(async () => {
element = await fixture(html`<gr-diff-host></gr-diff-host>`);
@@ -1486,7 +1488,7 @@
...createDiff(),
content: [
{
- a: [new Array(501).join('*')],
+ a: ['*'.repeat(501)],
},
],
})
@@ -1542,7 +1544,7 @@
...createDiff(),
content: [
{
- a: [new Array(501).join('*')],
+ a: ['*'.repeat(501)],
},
],
})
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 17b0c67..4207c2b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -1334,7 +1334,7 @@
}
idx += direction;
- // Redirect to the change view if opt_noUp isn’t truthy and idx falls
+ // Redirect to the change view if noUp isn’t truthy and idx falls
// outside the bounds of [0, fileList.length).
if (idx < 0 || idx > fileList.length - 1) {
return {up: true};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index e7e1bc8..ed7ce1b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -51,7 +51,7 @@
import {Side} from '../../../api/diff';
import {Files, GrDiffView} from './gr-diff-view';
import {DropdownItem} from '../../shared/gr-dropdown-list/gr-dropdown-list';
-import {SinonFakeTimers, SinonStub} from 'sinon';
+import {SinonFakeTimers, SinonStub, SinonStubbedMember} from 'sinon';
import {
changeModelToken,
ChangeModel,
@@ -76,6 +76,8 @@
changeViewModelToken,
} from '../../../models/views/change';
import {FileNameToNormalizedFileInfoMap} from '../../../models/change/files-model';
+import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
+import {GrDiffCursor} from '../../../embed/diff/gr-diff-cursor/gr-diff-cursor';
function createComment(
id: string,
@@ -97,10 +99,10 @@
let element: GrDiffView;
let clock: SinonFakeTimers;
let diffCommentsStub;
- let getDiffRestApiStub: SinonStub;
- let navToChangeStub: SinonStub;
- let navToDiffStub: SinonStub;
- let navToEditStub: SinonStub;
+ let getDiffRestApiStub: SinonStubbedMember<RestApiService['getDiff']>;
+ let navToChangeStub: SinonStubbedMember<ChangeModel['navigateToChange']>;
+ let navToDiffStub: SinonStubbedMember<ChangeModel['navigateToDiff']>;
+ let navToEditStub: SinonStubbedMember<ChangeModel['navigateToEdit']>;
let changeModel: ChangeModel;
let viewModel: ChangeViewModel;
let commentsModel: CommentsModel;
@@ -536,8 +538,11 @@
element.basePatchNum = 5 as BasePatchSetNum;
await element.updateComplete;
element.handleDiffAgainstBase();
- const expected = [{path: 'some/path.txt'}, 10, PARENT];
- assert.deepEqual(navToDiffStub.lastCall.args, expected);
+ assert.deepEqual(navToDiffStub.lastCall.args, [
+ {path: 'some/path.txt'},
+ 10 as RevisionPatchSetNum,
+ PARENT,
+ ]);
});
test('diff against latest', async () => {
@@ -547,8 +552,11 @@
element.basePatchNum = 5 as BasePatchSetNum;
await element.updateComplete;
element.handleDiffAgainstLatest();
- const expected = [{path: 'foo'}, 12, 5];
- assert.deepEqual(navToDiffStub.lastCall.args, expected);
+ assert.deepEqual(navToDiffStub.lastCall.args, [
+ {path: 'foo'},
+ 12 as RevisionPatchSetNum,
+ 5 as BasePatchSetNum,
+ ]);
});
test('handleDiffBaseAgainstLeft', async () => {
@@ -564,8 +572,11 @@
});
await element.updateComplete;
element.handleDiffBaseAgainstLeft();
- const expected = [{path: 'foo'}, 1, PARENT];
- assert.deepEqual(navToDiffStub.lastCall.args, expected);
+ assert.deepEqual(navToDiffStub.lastCall.args, [
+ {path: 'foo'},
+ 1 as RevisionPatchSetNum,
+ PARENT,
+ ]);
});
test('handleDiffRightAgainstLatest', async () => {
@@ -575,8 +586,11 @@
element.basePatchNum = 1 as BasePatchSetNum;
await element.updateComplete;
element.handleDiffRightAgainstLatest();
- const expected = [{path: 'foo'}, 10, 3];
- assert.deepEqual(navToDiffStub.lastCall.args, expected);
+ assert.deepEqual(navToDiffStub.lastCall.args, [
+ {path: 'foo'},
+ 10 as RevisionPatchSetNum,
+ 3 as BasePatchSetNum,
+ ]);
});
test('handleDiffBaseAgainstLatest', async () => {
@@ -585,8 +599,11 @@
element.basePatchNum = 1 as BasePatchSetNum;
await element.updateComplete;
element.handleDiffBaseAgainstLatest();
- const expected = [{path: 'some/path.txt'}, 10, PARENT];
- assert.deepEqual(navToDiffStub.lastCall.args, expected);
+ assert.deepEqual(navToDiffStub.lastCall.args, [
+ {path: 'some/path.txt'},
+ 10 as RevisionPatchSetNum,
+ PARENT,
+ ]);
});
test('A fires an error event when not logged in', async () => {
@@ -1668,12 +1685,16 @@
});
suite('switching files', () => {
- let dispatchEventStub: SinonStub;
- let navToFileStub: SinonStub;
- let moveToPreviousChunkStub: SinonStub;
- let moveToNextChunkStub: SinonStub;
- let isAtStartStub: SinonStub;
- let isAtEndStub: SinonStub;
+ let dispatchEventStub: SinonStubbedMember<Element['dispatchEvent']>;
+ let navToFileStub: SinonStubbedMember<GrDiffView['navToFile']>;
+ let moveToPreviousChunkStub: SinonStubbedMember<
+ GrDiffCursor['moveToPreviousChunk']
+ >;
+ let moveToNextChunkStub: SinonStubbedMember<
+ GrDiffCursor['moveToNextChunk']
+ >;
+ let isAtStartStub: SinonStubbedMember<GrDiffCursor['isAtStart']>;
+ let isAtEndStub: SinonStubbedMember<GrDiffCursor['isAtEnd']>;
let nowStub: SinonStub;
setup(() => {
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index 01c007b..e33c48a 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -48,6 +48,7 @@
import {changeModelToken} from '../../../models/change/change-model';
import {changeViewModelToken} from '../../../models/views/change';
import {fireNoBubbleNoCompose} from '../../../utils/event-util';
+import {KnownExperimentId} from '../../../services/flags/flags';
// Maximum length for patch set descriptions.
const PATCH_DESC_MAX_LENGTH = 500;
@@ -126,6 +127,8 @@
private readonly getViewModel = resolve(this, changeViewModelToken);
+ private readonly flagsService = getAppContext().flagsService;
+
constructor() {
super();
subscribe(
@@ -325,6 +328,11 @@
}
private computeText(patchNum: PatchSetNum, prefix: string, sha: string) {
+ if (
+ this.flagsService.isEnabled(KnownExperimentId.COMMENTS_CHIPS_IN_FILE_LIST)
+ ) {
+ return `${prefix}${patchNum} | ${sha}`;
+ }
return (
`${prefix}${patchNum}` +
`${this.computePatchSetCommentsString(patchNum)}` +
@@ -344,6 +352,16 @@
bottomText: `${this.computePatchSetDescription(patchNum)}`,
value: patchNum,
};
+ if (
+ this.flagsService.isEnabled(KnownExperimentId.COMMENTS_CHIPS_IN_FILE_LIST)
+ ) {
+ entry.commentThreads = this.changeComments?.computeCommentThreads(
+ {
+ patchNum,
+ },
+ true
+ );
+ }
const date = this.computePatchSetDate(patchNum);
if (date) {
entry.date = date;
@@ -417,12 +435,12 @@
computePatchSetCommentsString(patchNum: PatchSetNum): string {
if (!this.changeComments) return '';
- const commentThreadCount = this.changeComments.computeCommentThreadCount(
+ const commentThreadCount = this.changeComments.computeCommentThreads(
{
patchNum,
},
true
- );
+ ).length;
const commentThreadString = pluralize(commentThreadCount, 'comment');
const unresolvedCount = this.changeComments.computeUnresolvedNum(
@@ -480,9 +498,9 @@
previous: detail.patchNum,
current: patchSetValue,
latest: latestPatchNum,
- commentCount: this.changeComments?.computeCommentThreadCount({
+ commentCount: this.changeComments?.computeCommentThreads({
patchNum: patchSetValue,
- }),
+ }).length,
});
detail.patchNum = patchSetValue;
} else {
@@ -490,9 +508,9 @@
this.reporting.reportInteraction('left-patchset-changed', {
previous: detail.basePatchNum,
current: patchSetValue,
- commentCount: this.changeComments?.computeCommentThreadCount({
+ commentCount: this.changeComments?.computeCommentThreads({
patchNum: patchSetValue,
- }),
+ }).length,
});
detail.basePatchNum = patchSetValue as BasePatchSetNum;
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts
index 0699e49..e0b792f 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts
@@ -7,18 +7,21 @@
import './gr-plugin-host';
import {GrPluginHost} from './gr-plugin-host';
import {fixture, html, assert} from '@open-wc/testing';
-import {SinonStub} from 'sinon';
+import {SinonStubbedMember} from 'sinon';
import {createServerInfo} from '../../../test/test-data-generators';
import {
ConfigModel,
configModelToken,
} from '../../../models/config/config-model';
import {testResolver} from '../../../test/common-test-setup';
-import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+import {
+ PluginLoader,
+ pluginLoaderToken,
+} from '../../shared/gr-js-api-interface/gr-plugin-loader';
suite('gr-plugin-host tests', () => {
let element: GrPluginHost;
- let loadPluginsStub: SinonStub;
+ let loadPluginsStub: SinonStubbedMember<PluginLoader['loadPlugins']>;
let configModel: ConfigModel;
setup(async () => {
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
index 7f0f85d..83ca149 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
@@ -90,10 +90,10 @@
return promise;
}
- function close(opt_action?: Function) {
+ function close(action?: Function) {
const promise = listen('close');
- if (opt_action) {
- opt_action();
+ if (action) {
+ action();
} else {
queryAndAssert<GrButton>(element, '#closeButton').click();
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index 9126905..104bb1e 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -26,12 +26,15 @@
createAccountDetailWithId,
createThread,
} from '../../../test/test-data-generators';
-import {SinonStub} from 'sinon';
+import {SinonStubbedMember} from 'sinon';
import {fixture, html, waitUntil, assert} from '@open-wc/testing';
import {GrButton} from '../gr-button/gr-button';
import {SpecialFilePath} from '../../../constants/constants';
import {GrIcon} from '../gr-icon/gr-icon';
-import {commentsModelToken} from '../../../models/comments/comments-model';
+import {
+ CommentsModel,
+ commentsModelToken,
+} from '../../../models/comments/comments-model';
import {testResolver} from '../../../test/common-test-setup';
const c1 = {
@@ -310,7 +313,7 @@
suite('action button clicks', () => {
let savePromise: MockPromise<DraftInfo>;
- let stub: SinonStub;
+ let stub: SinonStubbedMember<CommentsModel['saveDraft']>;
setup(async () => {
savePromise = mockPromise<DraftInfo>();
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index bfa27a2..de6a39c 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -494,6 +494,15 @@
margin-left: var(--spacing-m);
cursor: pointer;
}
+ .suggestEdit {
+ /** same height as header */
+ --margin: calc(0px - var(--spacing-s));
+ margin-right: var(--spacing-s);
+ }
+ .suggestEdit gr-icon {
+ color: inherit;
+ margin-right: var(--spacing-s);
+ }
`,
];
}
@@ -800,8 +809,9 @@
return html`<gr-button
link
class="action suggestEdit"
+ title="This button copies the text to make a suggestion"
@click=${this.createSuggestEdit}
- >Suggest Fix</gr-button
+ ><gr-icon icon="edit" id="icon" filled></gr-icon> Suggest edit</gr-button
>`;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index f0a5fea..3094d37 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -37,7 +37,7 @@
import {GrConfirmDeleteCommentDialog} from '../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog';
import {assertIsDefined} from '../../../utils/common-util';
import {Modifier} from '../../../utils/dom-util';
-import {SinonStub} from 'sinon';
+import {SinonStubbedMember} from 'sinon';
import {fixture, html, assert} from '@open-wc/testing';
import {GrButton} from '../gr-button/gr-button';
import {testResolver} from '../../../test/common-test-setup';
@@ -745,7 +745,7 @@
suite('auto saving', () => {
let clock: sinon.SinonFakeTimers;
let savePromise: MockPromise<DraftInfo>;
- let saveStub: SinonStub;
+ let saveStub: SinonStubbedMember<CommentsModel['saveDraft']>;
setup(async () => {
clock = sinon.useFakeTimers();
@@ -832,7 +832,7 @@
);
element.editing = true;
});
- test('renders suggest fix button', () => {
+ test('renders suggest edit button', () => {
assert.dom.equal(
queryAndAssert(element, 'gr-button.suggestEdit'),
/* HTML */ `<gr-button
@@ -840,8 +840,9 @@
link=""
role="button"
tabindex="0"
+ title="This button copies the text to make a suggestion"
>
- Suggest Fix
+ <gr-icon icon="edit" id="icon" filled></gr-icon> Suggest edit
</gr-button> `
);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
index c935e72..a083526 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
@@ -14,7 +14,7 @@
import {css, html, LitElement, PropertyValues} from 'lit';
import {customElement, property, query} from 'lit/decorators.js';
import {IronDropdownElement} from '@polymer/iron-dropdown/iron-dropdown';
-import {Timestamp} from '../../../types/common';
+import {CommentThread, Timestamp} from '../../../types/common';
import {NormalizedFileInfo} from '../../change/gr-file-list/gr-file-list';
import {GrButton} from '../gr-button/gr-button';
import {assertIsDefined} from '../../../utils/common-util';
@@ -43,6 +43,7 @@
date?: Timestamp;
disabled?: boolean;
file?: NormalizedFileInfo;
+ commentThreads?: CommentThread[];
}
declare global {
@@ -253,6 +254,14 @@
<div class="topContent">
<div>${item.text}</div>
${when(
+ item.commentThreads,
+ () => html`<gr-comments-summary
+ .commentThreads=${item.commentThreads}
+ emptyWhenNoComments
+ showAvatarForResolved
+ ></gr-comments-summary>`
+ )}
+ ${when(
item.date,
() => html`
<gr-date-formatter .dateStr=${item.date}></gr-date-formatter>
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts
index c8574cd..b82c023 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts
@@ -120,6 +120,7 @@
.inputContainer {
background-color: var(--dialog-background-color);
padding: var(--spacing-m);
+ white-space: nowrap;
}
/* This makes inputContainer on one line. */
.inputContainer gr-autocomplete,
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.ts
index 0bd491c..7b99660 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.ts
@@ -60,11 +60,11 @@
restApiService: RestApiService,
method: HttpMethod,
url: string,
- opt_callback?: (response: unknown) => void,
- opt_payload?: RequestPayload
+ callback?: (response: unknown) => void,
+ payload?: RequestPayload
) {
return restApiService
- .send(method, url, opt_payload)
+ .send(method, url, payload)
.then(response => {
if (response.status < 200 || response.status >= 300) {
return response.text().then((text: string | undefined) => {
@@ -79,8 +79,8 @@
}
})
.then(response => {
- if (opt_callback) {
- opt_callback(response);
+ if (callback) {
+ callback(response);
}
return response;
});
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts
index cce2840..38f4f17 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts
@@ -105,7 +105,7 @@
test('next button', async () => {
element.itemsPerPage = 25;
- element.items = new Array(26);
+ element.items = Array.from({length: 26});
element.loading = false;
await element.updateComplete;
@@ -116,7 +116,7 @@
assert.isFalse(element.hideNextArrow());
element.items = [];
assert.isTrue(element.hideNextArrow());
- element.items = new Array(4);
+ element.items = Array.from({length: 4});
assert.isTrue(element.hideNextArrow());
});
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-etag-decorator_test.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-etag-decorator_test.ts
index 4f22f25..fd69a33 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-etag-decorator_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-etag-decorator_test.ts
@@ -10,12 +10,12 @@
suite('gr-etag-decorator', () => {
let etag: GrEtagDecorator;
- const fakeRequest = (opt_etag?: string, opt_status?: number) => {
+ const fakeRequest = (etag?: string, status?: number) => {
const headers = new Headers();
- if (opt_etag) {
- headers.set('etag', opt_etag);
+ if (etag) {
+ headers.set('etag', etag);
}
- const status = opt_status || 200;
+ status = status || 200;
return {...new Response(), ok: true, status, headers};
};
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
index 21faf53..1084512 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
@@ -414,9 +414,7 @@
const params: Array<string | number | boolean> = [];
for (const [p, paramValue] of Object.entries(fetchParams)) {
- // TODO(TS): Replace == null with === and check for null and undefined
- // eslint-disable-next-line eqeqeq
- if (paramValue == null) {
+ if (paramValue === null || paramValue === undefined) {
params.push(this.encodeRFC5987(p));
continue;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser_test.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser_test.js
index 9c87910..4225173 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser_test.js
@@ -97,11 +97,11 @@
const date2 = '2017-01-26 12:11:55.000000000'; // Within threshold.
const date3 = '2017-01-26 12:33:50.000000000';
const date4 = '2017-01-26 12:44:50.000000000';
- const makeItem = function(state, reviewer, opt_date, opt_author) {
+ const makeItem = function(state, reviewer, date, author) {
return {
reviewer,
- updated: opt_date || date1,
- updated_by: opt_author || reviewer1,
+ updated: date || date1,
+ updated_by: author || reviewer1,
state,
};
};
@@ -173,9 +173,9 @@
test('format reviewer updates', () => {
const reviewer1 = {_account_id: 1};
const reviewer2 = {_account_id: 2};
- const makeItem = function(prev, state, opt_reviewer) {
+ const makeItem = function(prev, state, reviewer) {
return {
- reviewer: opt_reviewer || reviewer1,
+ reviewer: reviewer || reviewer1,
prev_state: prev,
state,
};
diff --git a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
index 338b2d7..ff068a7 100644
--- a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
+++ b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
@@ -28,11 +28,9 @@
static override styles = [
css`
.header {
- background-color: var(--user-suggestion-header-background);
- color: var(--user-suggestion-header-color);
+ background-color: var(--background-color-primary);
border: 1px solid var(--border-color);
- border-bottom: 0;
- padding: var(--spacing-xs) var(--spacing-s);
+ padding: var(--spacing-xs) var(--spacing-xl);
display: flex;
align-items: center;
border-top-left-radius: var(--border-radius);
@@ -41,8 +39,8 @@
.header .title {
flex: 1;
}
- gr-copy-clipboard {
- --gr-copy-clipboard-icon-color: var(--user-suggestion-header-color);
+ .copyButton {
+ margin-right: var(--spacing-l);
}
code {
max-width: var(--gr-formatted-text-prose-max-width, none);
@@ -71,21 +69,29 @@
if (!this.textContent) return nothing;
const code = this.textContent;
return html`<div class="header">
- <div class="title">Suggested fix</div>
- <div>
+ <div class="title">
+ <span>Suggested edit</span>
+ <a
+ href="https://gerrit-review.googlesource.com/Documentation/user-suggest-edits.html"
+ target="_blank"
+ ><gr-icon icon="help" title="read documentation"></gr-icon
+ ></a>
+ </div>
+ <div class="copyButton">
<gr-copy-clipboard
hideInput=""
text=${code}
- copyTargetName="Suggested fix"
+ copyTargetName="Suggested edit"
></gr-copy-clipboard>
</div>
<div>
<gr-button
secondary
+ flatten
class="action show-fix"
@click=${this.handleShowFix}
>
- Preview Fix
+ Show edit
</gr-button>
</div>
</div>
diff --git a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
index f49ea98..aecd93b 100644
--- a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
@@ -27,17 +27,24 @@
assert.shadowDom.equal(
element,
/* HTML */ `<div class="header">
- <div class="title">Suggested fix</div>
- <div>
+ <div class="title">
+ <span>Suggested edit</span>
+ <a
+ href="https://gerrit-review.googlesource.com/Documentation/user-suggest-edits.html"
+ target="_blank"
+ ><gr-icon icon="help" title="read documentation"></gr-icon
+ ></a>
+ </div>
+ <div class="copyButton">
<gr-copy-clipboard
hideinput=""
text="Hello World"
- copytargetname="Suggested fix"
+ copytargetname="Suggested edit"
></gr-copy-clipboard>
</div>
<div>
- <gr-button class="action show-fix" secondary=""
- >Preview Fix</gr-button
+ <gr-button class="action show-fix" secondary="" flatten=""
+ >Show edit</gr-button
>
</div>
</div>
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
index f6f7052..5b23ee7 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
@@ -135,7 +135,7 @@
element.context = 10;
const content = [
{
- ab: new Array(100).fill(
+ ab: Array.from<string>({length: 100}).fill(
'all work and no play make jack a dull boy'
),
},
@@ -165,9 +165,13 @@
test('at the beginning with skip chunks', async () => {
element.context = 10;
const content = [
- {ab: new Array(20).fill('all work and no play make jack a dull boy')},
+ {
+ ab: Array.from<string>({length: 20}).fill(
+ 'all work and no play make jack a dull boy'
+ ),
+ },
{skip: 43900},
- {ab: new Array(30).fill('some other content')},
+ {ab: Array.from<string>({length: 30}).fill('some other content')},
{a: ['some other content']},
];
@@ -213,7 +217,11 @@
test('at the beginning, smaller than context', () => {
element.context = 10;
const content = [
- {ab: new Array(5).fill('all work and no play make jack a dull boy')},
+ {
+ ab: Array.from<string>({length: 5}).fill(
+ 'all work and no play make jack a dull boy'
+ ),
+ },
{a: ['all work and no play make andybons a dull boy']},
];
@@ -235,7 +243,7 @@
const content = [
{a: ['all work and no play make andybons a dull boy']},
{
- ab: new Array(100).fill(
+ ab: Array.from<string>({length: 100}).fill(
'all work and no play make jill a dull girl'
),
},
@@ -266,7 +274,11 @@
element.context = 10;
const content = [
{a: ['all work and no play make andybons a dull boy']},
- {ab: new Array(5).fill('all work and no play make jill a dull girl')},
+ {
+ ab: Array.from<string>({length: 5}).fill(
+ 'all work and no play make jill a dull girl'
+ ),
+ },
];
return element.process(content, false).then(() => {
@@ -287,23 +299,39 @@
element.context = 10;
const content = [
{a: ['all work and no play make andybons a dull boy']},
- {ab: new Array(3).fill('all work and no play make jill a dull girl')},
{
- a: new Array(3).fill('all work and no play make jill a dull girl'),
- b: new Array(3).fill(
+ ab: Array.from<string>({length: 3}).fill(
+ 'all work and no play make jill a dull girl'
+ ),
+ },
+ {
+ a: Array.from<string>({length: 3}).fill(
+ 'all work and no play make jill a dull girl'
+ ),
+ b: Array.from<string>({length: 3}).fill(
' all work and no play make jill a dull girl'
),
common: true,
},
- {ab: new Array(3).fill('all work and no play make jill a dull girl')},
{
- a: new Array(3).fill('all work and no play make jill a dull girl'),
- b: new Array(3).fill(
+ ab: Array.from<string>({length: 3}).fill(
+ 'all work and no play make jill a dull girl'
+ ),
+ },
+ {
+ a: Array.from<string>({length: 3}).fill(
+ 'all work and no play make jill a dull girl'
+ ),
+ b: Array.from<string>({length: 3}).fill(
' all work and no play make jill a dull girl'
),
common: true,
},
- {ab: new Array(3).fill('all work and no play make jill a dull girl')},
+ {
+ ab: Array.from<string>({length: 3}).fill(
+ 'all work and no play make jill a dull girl'
+ ),
+ },
];
return element.process(content, false).then(() => {
@@ -387,7 +415,7 @@
const content = [
{a: ['all work and no play make andybons a dull boy']},
{
- ab: new Array(100).fill(
+ ab: Array.from<string>({length: 100}).fill(
'all work and no play make jill a dull girl'
),
},
@@ -425,7 +453,11 @@
element.context = 10;
const content = [
{a: ['all work and no play make andybons a dull boy']},
- {ab: new Array(5).fill('all work and no play make jill a dull girl')},
+ {
+ ab: Array.from<string>({length: 5}).fill(
+ 'all work and no play make jill a dull girl'
+ ),
+ },
{a: ['all work and no play make andybons a dull boy']},
];
@@ -448,9 +480,17 @@
element.context = 10;
const content = [
{a: ['all work and no play make andybons a dull boy']},
- {ab: new Array(20).fill('all work and no play make jill a dull girl')},
+ {
+ ab: Array.from<string>({length: 20}).fill(
+ 'all work and no play make jill a dull girl'
+ ),
+ },
{skip: 60},
- {ab: new Array(20).fill('all work and no play make jill a dull girl')},
+ {
+ ab: Array.from<string>({length: 20}).fill(
+ 'all work and no play make jill a dull girl'
+ ),
+ },
{a: ['all work and no play make andybons a dull boy']},
];
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index 34e9d5a..1c5d594 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -316,7 +316,6 @@
}
.diffContainer {
max-width: var(--diff-max-width, none);
- display: flex;
font-family: var(--monospace-font-family);
}
table {
diff --git a/polygerrit-ui/app/models/views/search_test.ts b/polygerrit-ui/app/models/views/search_test.ts
index 6ca8dc8..ed6de419 100644
--- a/polygerrit-ui/app/models/views/search_test.ts
+++ b/polygerrit-ui/app/models/views/search_test.ts
@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {assert} from '@open-wc/testing';
-import {SinonStub} from 'sinon';
+import {SinonStubbedMember} from 'sinon';
import {
AccountId,
BranchName,
@@ -13,7 +13,10 @@
RepoName,
TopicName,
} from '../../api/rest-api';
-import {navigationToken} from '../../elements/core/gr-navigation/gr-navigation';
+import {
+ NavigationService,
+ navigationToken,
+} from '../../elements/core/gr-navigation/gr-navigation';
import '../../test/common-test-setup';
import {testResolver} from '../../test/common-test-setup';
import {createChange} from '../../test/test-data-generators';
@@ -78,7 +81,7 @@
});
suite('query based navigation', () => {
- let replaceUrlStub: SinonStub;
+ let replaceUrlStub: SinonStubbedMember<NavigationService['replaceUrl']>;
let model: SearchViewModel;
setup(() => {
diff --git a/polygerrit-ui/app/services/gr-auth/gr-auth.ts b/polygerrit-ui/app/services/gr-auth/gr-auth.ts
index f41c83d..945d6f9 100644
--- a/polygerrit-ui/app/services/gr-auth/gr-auth.ts
+++ b/polygerrit-ui/app/services/gr-auth/gr-auth.ts
@@ -50,5 +50,5 @@
/**
* Perform network fetch with authentication.
*/
- fetch(url: string, opt_options?: AuthRequestInit): Promise<Response>;
+ fetch(url: string, options?: AuthRequestInit): Promise<Response>;
}
diff --git a/polygerrit-ui/app/services/gr-auth/gr-auth_impl.ts b/polygerrit-ui/app/services/gr-auth/gr-auth_impl.ts
index 51b8bfd..2312fc9 100644
--- a/polygerrit-ui/app/services/gr-auth/gr-auth_impl.ts
+++ b/polygerrit-ui/app/services/gr-auth/gr-auth_impl.ts
@@ -162,18 +162,18 @@
/**
* Perform network fetch with authentication.
*/
- fetch(url: string, opt_options?: AuthRequestInit): Promise<Response> {
- const options: AuthRequestInitWithHeaders = {
+ fetch(url: string, options?: AuthRequestInit): Promise<Response> {
+ const optionsWithHeaders: AuthRequestInitWithHeaders = {
headers: new Headers(),
...this.defaultOptions,
- ...opt_options,
+ ...options,
};
if (this.type === AuthType.ACCESS_TOKEN) {
return this._getAccessToken().then(accessToken =>
- this._fetchWithAccessToken(url, options, accessToken)
+ this._fetchWithAccessToken(url, optionsWithHeaders, accessToken)
);
} else {
- return this._fetchWithXsrfToken(url, options);
+ return this._fetchWithXsrfToken(url, optionsWithHeaders);
}
}
diff --git a/polygerrit-ui/app/services/gr-auth/gr-auth_mock.ts b/polygerrit-ui/app/services/gr-auth/gr-auth_mock.ts
index 3e14aa7..9cdd37e 100644
--- a/polygerrit-ui/app/services/gr-auth/gr-auth_mock.ts
+++ b/polygerrit-ui/app/services/gr-auth/gr-auth_mock.ts
@@ -57,7 +57,7 @@
setup(_getToken: GetTokenCallback, _defaultOptions: DefaultAuthOptions) {}
- fetch(_url: string, _opt_options?: AuthRequestInit): Promise<Response> {
+ fetch(_url: string, _options?: AuthRequestInit): Promise<Response> {
return Promise.resolve(new Response());
}
}
diff --git a/polygerrit-ui/app/services/gr-auth/gr-auth_test.ts b/polygerrit-ui/app/services/gr-auth/gr-auth_test.ts
index e64cec1..b9cef86 100644
--- a/polygerrit-ui/app/services/gr-auth/gr-auth_test.ts
+++ b/polygerrit-ui/app/services/gr-auth/gr-auth_test.ts
@@ -208,9 +208,9 @@
let getToken: sinon.SinonStub;
- const makeToken = (opt_accessToken?: string) => {
+ const makeToken = (accessToken?: string) => {
return {
- access_token: opt_accessToken || 'zbaz',
+ access_token: accessToken || 'zbaz',
expires_at: new Date(Date.now() + 10e8).getTime(),
};
};
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
index f552762..49259b4 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
@@ -29,7 +29,7 @@
eventName: string,
eventValue?: EventValue,
eventDetails?: EventDetails,
- opt_noLog?: boolean
+ noLog?: boolean
): void;
appStarted(): void;
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
index 5b6e852..55bb64c 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
@@ -131,8 +131,8 @@
};
// TODO(dmfilippov): TS-fix-any unclear what is context
// eslint-disable-next-line @typescript-eslint/no-explicit-any
- const catchErrors = function (opt_context?: any) {
- const context = opt_context || window;
+ const catchErrors = function (context?: any) {
+ context = context || window;
const oldOnError = context.onerror;
context.onerror = (
event: Event | string,
@@ -387,18 +387,18 @@
// We cache until metrics plugin is loaded
this.pending.push([eventInfo, noLog]);
if (this._isMetricsPluginLoaded()) {
- this.pending.forEach(([eventInfo, opt_noLog]) => {
- this._reportEvent(eventInfo, opt_noLog);
+ this.pending.forEach(([eventInfo, noLog]) => {
+ this._reportEvent(eventInfo, noLog);
});
this.pending = [];
}
}
}
- private _reportEvent(eventInfo: EventInfo, opt_noLog?: boolean) {
+ private _reportEvent(eventInfo: EventInfo, noLog?: boolean) {
const {type, value, name, eventDetails} = eventInfo;
document.dispatchEvent(new CustomEvent(type, {detail: eventInfo}));
- if (opt_noLog) {
+ if (noLog) {
return;
}
if (type !== ERROR.TYPE) {
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
index 83f1017..d570163 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
@@ -1191,7 +1191,7 @@
],
[{_number: 3, project: 'test/test'}],
] as unknown as ParsedJSON);
- // When opt_query instanceof Array, fetchJSON returns
+ // When query instanceof Array, fetchJSON returns
// Array<Array<Object>>.
await element.getChangesForMultipleQueries(undefined, []);
assert.equal(Object.keys(element._projectLookup).length, 3);
@@ -1210,7 +1210,7 @@
{_number: 3, project: 'test/test'},
] as unknown as ParsedJSON);
- // When opt_query !instanceof Array, fetchJSON returns Array<Object>.
+ // When query !instanceof Array, fetchJSON returns Array<Object>.
await element.getChanges();
assert.equal(Object.keys(element._projectLookup).length, 3);
const project1 = await element.getFromProjectLookup(1 as NumericChangeId);
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
index 86276d1..b1f1884 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
@@ -199,8 +199,8 @@
getChangeDetail(
changeNum?: number | string,
- opt_errFn?: ErrorCallback,
- opt_cancelCondition?: Function
+ errFn?: ErrorCallback,
+ cancelCondition?: Function
): Promise<ParsedChangeInfo | undefined>;
/**
diff --git a/polygerrit-ui/app/services/storage/gr-storage_test.ts b/polygerrit-ui/app/services/storage/gr-storage_test.ts
index b79a8d7..72878f8 100644
--- a/polygerrit-ui/app/services/storage/gr-storage_test.ts
+++ b/polygerrit-ui/app/services/storage/gr-storage_test.ts
@@ -11,7 +11,7 @@
suite('gr-storage tests', () => {
let grStorage: GrStorageService;
- function mockStorage(opt_quotaExceeded: boolean): Storage {
+ function mockStorage(quotaExceeded: boolean): Storage {
return {
getItem(key: string) {
return (this as any)[key];
@@ -20,7 +20,7 @@
delete (this as any)[key];
},
setItem(key: string, value: string) {
- if (opt_quotaExceeded) {
+ if (quotaExceeded) {
throw new DOMException('error', 'QuotaExceededError');
}
(this as any)[key] = value;
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index 1b0a71d..5dda8d6 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -154,7 +154,7 @@
export function changeStatuses(
change: ChangeInfo,
- opt_options?: ChangeStatusesOptions
+ options?: ChangeStatusesOptions
): ChangeStates[] {
const states = [];
if (change.status === ChangeStatus.MERGED) {
@@ -163,10 +163,7 @@
if (change.status === ChangeStatus.ABANDONED) {
return [ChangeStates.ABANDONED];
}
- if (
- change.mergeable === false ||
- (opt_options && opt_options.mergeable === false)
- ) {
+ if (change.mergeable === false || (options && options.mergeable === false)) {
// 'mergeable' prop may not always exist (@see Issue 6819)
states.push(ChangeStates.MERGE_CONFLICT);
} else if (change.contains_git_conflicts) {
@@ -181,12 +178,12 @@
// If there are any pre-defined statuses, only return those. Otherwise,
// will determine the derived status.
- if (states.length || !opt_options) {
+ if (states.length || !options) {
return states;
}
// If no missing requirements, either active or ready to submit.
- if (change.submittable && opt_options.submitEnabled) {
+ if (change.submittable && options.submitEnabled) {
states.push(ChangeStates.READY_TO_SUBMIT);
} else {
// Otherwise it is active.
diff --git a/polygerrit-ui/app/utils/path-list-util_test.ts b/polygerrit-ui/app/utils/path-list-util_test.ts
index 3c9e0d3..cdd8182 100644
--- a/polygerrit-ui/app/utils/path-list-util_test.ts
+++ b/polygerrit-ui/app/utils/path-list-util_test.ts
@@ -144,7 +144,7 @@
assert.equal(shortenedPath, expectedPath);
});
- test('truncatePath with opt_threshold', () => {
+ test('truncatePath with threshold', () => {
let path = 'level1/level2/level3/level4/file.js';
let shortenedPath = truncatePath(path, 2);
// The expected path is truncated with an ellipsis.