Merge "Add icons for checks table for expanding/collapsing message"
diff --git a/Documentation/config-accounts.txt b/Documentation/config-accounts.txt
index e642425..4f4685b 100644
--- a/Documentation/config-accounts.txt
+++ b/Documentation/config-accounts.txt
@@ -185,8 +185,6 @@
link:user-review-ui.html#diff-preferences[diff] and edit preferences:
----
-[general]
- showSiteHeader = false
[diff]
hideTopMenu = true
[edit]
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index 0b6a218..1ac0fd8 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -8,7 +8,8 @@
* A Linux or macOS system (Windows is not supported at this time)
* A JDK for Java 8|9|10|11|...
* Python 2 or 3
-* Node.js (including npm)
+* link:https://github.com/nodesource/distributions/blob/master/README.md[Node.js (including npm)]
+* Bower (`sudo npm install -g bower`)
* link:https://www.bazel.io/versions/master/docs/install.html[Bazel]
* Maven
* zip, unzip
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index e292549..77ef60d 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2600,10 +2600,8 @@
modifications, `NOT_READY`. Other statuses are available for particular cases.
A change can be submitted if all the plugins accept the change.
-Plugins may also decide not to vote on a given change by returning an empty
-Collection (ie: the plugin is not enabled for this repository), or to vote
-several times (ie: one SubmitRecord per project in the hierarchy).
-The results are handled as if multiple plugins voted for the change.
+Plugins may also decide not to vote on a given change by returning an
+`Optional.empty()` (ie: the plugin is not enabled for this repository).
If a plugin decides not to vote, it's name will not be displayed in the UI and
it will not be recoded in the database.
@@ -2638,20 +2636,20 @@
[source, java]
----
-import java.util.Collection;
+import java.util.Optional;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRecord.Status;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.SubmitRule;
public class MyPluginRules implements SubmitRule {
- public Collection<SubmitRecord> evaluate(ChangeData changeData) {
+ public Optional<SubmitRecord> evaluate(ChangeData changeData) {
// Implement your submitability logic here
// Assuming we want to prevent this change from being submitted:
SubmitRecord record = new SubmitRecord();
record.status = Status.NOT_READY;
- return record;
+ return Optional.of(record);
}
}
----
diff --git a/Documentation/dev-release.txt b/Documentation/dev-release.txt
index cda7a49..96e8cb0 100644
--- a/Documentation/dev-release.txt
+++ b/Documentation/dev-release.txt
@@ -373,7 +373,7 @@
feature-deprecations that we were holding off on to have a stable release where
the feature is still contained, but marked as deprecated.
-See link:dev-contributing.html#deprecating-features[Deprecating features] for
+See link:dev-processes.html#deprecating-features[Deprecating features] for
details.
GERRIT
diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt
index 17c9a61..e9f1861 100644
--- a/Documentation/intro-user.txt
+++ b/Documentation/intro-user.txt
@@ -694,36 +694,6 @@
The following preferences can be configured:
-- [[review-category]]`Display In Review Category`:
-+
-This setting controls how the values of the review labels in change
-lists and dashboards are visualized.
-+
-** `None`:
-+
-For each review label only the voting value is shown. Approvals are
-rendered as a green check mark icon, vetoes as a red X icon.
-+
-** `Show Name`:
-+
-For each review label the voting value is shown together with the full
-name of the voting user.
-+
-** `Show Email`:
-+
-For each review label the voting value is shown together with the email
-address of the voting user.
-+
-** `Show Username`:
-+
-For each review label the voting value is shown together with the
-username of the voting user.
-+
-** `Show Abbreviated Name`:
-+
-For each review label the voting value is shown together with the
-initials of the full name of the voting user.
-
- [[page-size]]`Maximum Page Size`:
+
The maximum number of entries that are shown on one page, e.g. used
diff --git a/Documentation/js-api.txt b/Documentation/js-api.txt
index 4ef2a6c..d909c00 100644
--- a/Documentation/js-api.txt
+++ b/Documentation/js-api.txt
@@ -713,10 +713,12 @@
[[Gerrit_css]]
=== Gerrit.css()
+[WARNING]
+This method is deprecated. It doesn't work with Shadow DOM and
+will be removed in the future. Please, use link:pg-plugin-dev.html#plugin-styles[plugin.styles] instead.
+
Creates a new unique CSS class and injects it into the document.
The name of the class is returned and can be used by the plugin.
-See link:#Gerrit_html[`Gerrit.html()`] for an easy way to use
-generated class names.
Classes created with this function should be created once at install
time and reused throughout the plugin. Repeatedly creating the same
@@ -814,112 +816,6 @@
the current browser window will navigate to the non-Gerrit URL.
The user can return to Gerrit with the back button.
-[[Gerrit_html]]
-=== Gerrit.html()
-Parses an HTML fragment after performing template replacements. If
-the HTML has a single root element or node that node is returned,
-otherwise it is wrapped inside a `<div>` and the div is returned.
-
-.Signature
-[source,javascript]
-----
-Gerrit.html(htmlText, options, wantElements);
-----
-
-* htmlText: string of HTML to be parsed. A new unattached `<div>` is
- created in the browser's document and the innerHTML property is
- assigned to the passed string, after performing replacements. If
- the div has exactly one child, that child will be returned instead
- of the div.
-
-* options: optional object reference supplying replacements for any
- `{name}` references in htmlText. Navigation through objects is
- supported permitting `{style.bar}` to be replaced with `"foo"` if
- options was `{style: {bar: "foo"}}`. Value replacements are HTML
- escaped before being inserted into the document fragment.
-
-* wantElements: if options is given and wantElements is also true
- an object consisting of `{root: parsedElement, elements: {...}}` is
- returned instead of the parsed element. The elements object contains
- a property for each element using `id={name}` in htmlText.
-
-.Example
-[source,javascript]
-----
-var style = {bar: Gerrit.css('background: yellow')};
-Gerrit.html(
- '<span class="{style.bar}">Hello {name}!</span>',
- {style: style, name: "World"});
-----
-
-Event handlers can be automatically attached to elements referenced
-through an attribute id. Object navigation is not supported for ids,
-and the parser strips the id attribute before returning the result.
-Handler functions must begin with `on` and be a function to be
-installed on the element. This approach is useful for onclick and
-other handlers that do not want to create circular references that
-will eventually leak browser memory.
-
-.Example
-[source,javascript]
-----
-var options = {
- link: {
- onclick: function(e) { window.close() },
- },
-};
-Gerrit.html('<a href="javascript:;" id="{link}">Close</a>', options);
-----
-
-When using options to install handlers care must be taken to not
-accidentally include the returned element into the event handler's
-closure. This is why options is built before calling `Gerrit.html()`
-and not inline as a shown above with "Hello World".
-
-DOM nodes can optionally be returned, allowing handlers to access the
-elements identified by `id={name}` at a later point in time.
-
-.Example
-[source,javascript]
-----
-var w = Gerrit.html(
- '<div>Name: <input type="text" id="{name}"></div>'
- + '<div>Age: <input type="text" id="{age}"></div>'
- + '<button id="{submit}"><div>Save</div></button>',
- {
- submit: {
- onclick: function(s) {
- var e = w.elements;
- window.alert(e.name.value + " is " + e.age.value);
- },
- },
- }, true);
-----
-
-To prevent memory leaks `w.root` and `w.elements` should be set to
-null when the elements are no longer necessary. Screens can use
-link:#screen_onUnload[screen.onUnload()] to define a callback function
-to perform this cleanup:
-
-[source,javascript]
-----
-var w = Gerrit.html(...);
-screen.body.appendElement(w.root);
-screen.onUnload(function() { w.clear() });
-----
-
-[[Gerrit_injectCss]]
-=== Gerrit.injectCss()
-Injects CSS rules into the document by appending onto the end of the
-existing rule list. CSS rules are global to the entire application
-and must be manually scoped by each plugin. For an automatic scoping
-alternative see link:#Gerrit_css[`css()`].
-
-[source,javascript]
-----
-Gerrit.injectCss('.myplugin_bg {background: #000}');
-----
-
[[Gerrit_install]]
=== Gerrit.install()
Registers a new plugin by invoking the supplied initialization
diff --git a/Documentation/pg-plugin-dev.txt b/Documentation/pg-plugin-dev.txt
index 8fb5655..d901851 100644
--- a/Documentation/pg-plugin-dev.txt
+++ b/Documentation/pg-plugin-dev.txt
@@ -360,6 +360,16 @@
Deprecated. Use link:#plugin-settings[`plugin.settings()`] instead.
+[[plugin-styles]]
+=== styles
+`plugin.styles()`
+
+.Params:
+- none
+
+.Returns:
+- Instance of link:pg-plugin-styles-api.html[GrStylesApi]
+
=== changeMetadata
`plugin.changeMetadata()`
@@ -372,6 +382,7 @@
=== theme
`plugin.theme()`
+
Note: TODO
=== url
diff --git a/Documentation/pg-plugin-style-object.txt b/Documentation/pg-plugin-style-object.txt
new file mode 100644
index 0000000..cdcfb55
--- /dev/null
+++ b/Documentation/pg-plugin-style-object.txt
@@ -0,0 +1,33 @@
+= Gerrit Code Review - GrStyleObject
+
+Store information about css style properties. You can't create this object
+directly. Instead you should use the link:pg-plugin-styles-api.html#css[css] method.
+This object allows to apply style correctly to elements within different shadow
+subtree.
+
+[[get-class-name]]
+== getClassName
+`styleObject.getClassName(element)`
+
+.Params
+- `element` - an HTMLElement.
+
+.Returns
+- `string` - class name. The class name is valid only within the shadow root of `element`.
+
+Creates a new unique CSS class and injects it into the appropriate place
+in DOM (it can be document or shadow root for element). This class can be later
+added to the element or to any other element in the same shadow root. It is guarantee,
+that method adds CSS class only once for each shadow root.
+
+== apply
+`styleObject.apply(element)`
+
+.Params
+- `element` - element to apply style.
+
+Create a new unique CSS class (see link:#get-class-name[getClassName]) and
+adds class to the element.
+
+
+
diff --git a/Documentation/pg-plugin-styles-api.txt b/Documentation/pg-plugin-styles-api.txt
new file mode 100644
index 0000000..a829325
--- /dev/null
+++ b/Documentation/pg-plugin-styles-api.txt
@@ -0,0 +1,29 @@
+= Gerrit Code Review - Plugin styles API
+
+This API is provided by link:pg-plugin-dev.html#plugin-styles[plugin.styles()]
+and provides a way to apply dynamically created styles to elements in a
+document.
+
+[[css]]
+== css
+`styles.css(rulesStr)`
+
+.Params
+- `*string* rulesStr` string with CSS styling declarations.
+
+Example:
+----
+const styleObject = plugin.styles().css('background: black; color: white;');
+...
+const className = styleObject.getClassName(element)
+...
+element.classList.add(className);
+...
+styleObject.apply(someOtherElement);
+----
+
+.Returns
+- Instance of link:pg-plugin-style-object.html[GrStyleObject].
+
+
+
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 5d22659..145af0e 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1250,14 +1250,11 @@
)]}'
{
"changes_per_page": 25,
- "show_site_header": true,
- "use_flash_clipboard": true,
"download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
"diff_view": "SIDE_BY_SIDE",
"size_bar_in_change_table": true,
- "review_category_strategy": "ABBREV",
"mute_common_path_prefixes": true,
"publish_comments_on_push": true,
"work_in_progress_by_default": true,
@@ -1306,14 +1303,11 @@
{
"changes_per_page": 50,
- "show_site_header": true,
- "use_flash_clipboard": true,
"expand_inline_diffs": true,
"download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
"size_bar_in_change_table": true,
- "review_category_strategy": "NAME",
"diff_view": "SIDE_BY_SIDE",
"mute_common_path_prefixes": true,
"my": [
@@ -1357,14 +1351,11 @@
)]}'
{
"changes_per_page": 50,
- "show_site_header": true,
- "use_flash_clipboard": true,
"expand_inline_diffs": true,
"download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
"size_bar_in_change_table": true,
- "review_category_strategy": "NAME",
"diff_view": "SIDE_BY_SIDE",
"publish_comments_on_push": true,
"work_in_progress_by_default": true,
@@ -2722,10 +2713,6 @@
|`changes_per_page` ||
The number of changes to show on each page.
Allowed values are `10`, `25`, `50`, `100`.
-|`show_site_header` |not set if `false`|
-Whether the site header should be shown.
-|`use_flash_clipboard` |not set if `false`|
-Whether to use the flash clipboard widget.
|`expand_inline_diffs` |not set if `false`|
Whether to expand diffs inline instead of opening as separate page
(PolyGerrit only).
@@ -2750,9 +2737,6 @@
Whether to show the change sizes as colored bars in the change table.
|`legacycid_in_change_table` |not set if `false`|
Whether to show change number in the change table.
-|`review_category_strategy` ||
-The strategy used to displayed info in the review category column.
-Allowed values are `NONE`, `NAME`, `EMAIL`, `USERNAME`, `ABBREV`.
|`mute_common_path_prefixes` |not set if `false`|
Whether to mute common path prefixes in file names in the file table.
|`signed_off_by` |not set if `false`|
@@ -2796,10 +2780,6 @@
|`changes_per_page` |optional|
The number of changes to show on each page.
Allowed values are `10`, `25`, `50`, `100`.
-|`show_site_header` |optional|
-Whether the site header should be shown.
-|`use_flash_clipboard` |optional|
-Whether to use the flash clipboard widget.
|`expand_inline_diffs` |not set if `false`|
Whether to expand diffs inline instead of opening as separate page
(PolyGerrit only).
@@ -2822,9 +2802,6 @@
Whether to show the change sizes as colored bars in the change table.
|`legacycid_in_change_table` |optional|
Whether to show change number in the change table.
-|`review_category_strategy` |optional|
-The strategy used to displayed info in the review category column.
-Allowed values are `NONE`, `NAME`, `EMAIL`, `USERNAME`, `ABBREV`.
|`mute_common_path_prefixes` |optional|
Whether to mute common path prefixes in file names in the file table.
|`signed_off_by` |optional|
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index ec269e4..0df6aa5 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -2987,7 +2987,20 @@
Suggest the reviewers for a given query `q` and result limit `n`. If result
limit is not passed, then the default 10 is used.
-Groups can be excluded from the results by specifying 'e=f'.
+This REST endpoint only suggests accounts that
+
+* are active
+* can see the change
+* are visible to the calling user
+* are not already reviewer on the change
+* don't own the change
+
+Groups can be excluded from the results by specifying the 'exclude-groups'
+request parameter:
+
+--
+'GET /changes/link:#change-id[\{change-id\}]/suggest_reviewers?q=J&n=5&exclude-groups'
+--
As result a list of link:#suggested-reviewer-info[SuggestedReviewerInfo] entries is returned.
@@ -4345,8 +4358,10 @@
R = label('Any-Label-Name', reject(_)).
----
-The response is a list of link:#submit-record[SubmitRecord] entries
-describing the permutations that satisfy the tested submit rule.
+The response is a link:#submit-record[SubmitRecord] describing the
+permutations that satisfy the tested submit rule.
+
+If the submit rule was a np-op, the response is "`204 No Content`".
.Response
----
@@ -4355,14 +4370,12 @@
Content-Type: application/json; charset=UTF-8
)]}'
- [
- {
- "status": "NOT_READY",
- "reject": {
- "Any-Label-Name": {}
- }
+ {
+ "status": "NOT_READY",
+ "reject": {
+ "Any-Label-Name": {}
}
- ]
+ }
----
When testing with the `curl` command line client the
diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt
index 96b376d..52c393f 100644
--- a/Documentation/rest-api-config.txt
+++ b/Documentation/rest-api-config.txt
@@ -1054,14 +1054,11 @@
)]}'
{
"changes_per_page": 25,
- "show_site_header": true,
- "use_flash_clipboard": true,
"download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
"diff_view": "SIDE_BY_SIDE",
"size_bar_in_change_table": true,
- "review_category_strategy": "NONE",
"mute_common_path_prefixes": true,
"publish_comments_on_push": true,
"my": [
@@ -1133,14 +1130,11 @@
)]}'
{
"changes_per_page": 50,
- "show_site_header": true,
- "use_flash_clipboard": true,
"download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
"diff_view": "SIDE_BY_SIDE",
"size_bar_in_change_table": true,
- "review_category_strategy": "NONE",
"mute_common_path_prefixes": true,
"publish_comments_on_push": true,
"my": [
@@ -1545,11 +1539,15 @@
|`update_delay` ||
link:config-gerrit.html#change.updateDelay[How often in seconds the web
interface should poll for updates to the currently open change].
-|`submit_whole_topic` ||
+|`submit_whole_topic` |not set if `false`|
link:config-gerrit.html#change.submitWholeTopic[A configuration if
the whole topic is submitted].
|`disable_private_changes` |not set if `false`|
Returns true if private changes are disabled.
+|`exclude_mergeable_in_change_info` |not set if `false`|
+Value of the link:config-gerrit.html#change.api.excludeMergeableInChangeInfo[
+configuration parameter] that controls whether the mergeability bit in
+link:rest-api-changes.html#change-info[ChangeInfo] will never be set.
|=============================
[[check-account-external-ids-input]]
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index 010ef6d..e3318ea 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -381,6 +381,7 @@
abstract class SuggestedReviewersRequest {
private String query;
private int limit;
+ private boolean excludeGroups;
public abstract List<SuggestedReviewerInfo> get() throws RestApiException;
@@ -394,6 +395,11 @@
return this;
}
+ public SuggestedReviewersRequest excludeGroups(boolean excludeGroups) {
+ this.excludeGroups = excludeGroups;
+ return this;
+ }
+
public String getQuery() {
return query;
}
@@ -401,6 +407,10 @@
public int getLimit() {
return limit;
}
+
+ public boolean getExcludeGroups() {
+ return excludeGroups;
+ }
}
/**
diff --git a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index 7d356bf..f8404ce 100644
--- a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -146,7 +146,7 @@
SubmitType testSubmitType(TestSubmitRuleInput in) throws RestApiException;
- List<TestSubmitRuleInfo> testSubmitRule(TestSubmitRuleInput in) throws RestApiException;
+ TestSubmitRuleInfo testSubmitRule(TestSubmitRuleInput in) throws RestApiException;
MergeListRequest getMergeList() throws RestApiException;
@@ -351,7 +351,7 @@
}
@Override
- public List<TestSubmitRuleInfo> testSubmitRule(TestSubmitRuleInput in) throws RestApiException {
+ public TestSubmitRuleInfo testSubmitRule(TestSubmitRuleInput in) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
index fa95b8f..f5a740e 100644
--- a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
+++ b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
@@ -68,14 +68,6 @@
}
}
- public enum ReviewCategoryStrategy {
- NONE,
- NAME,
- EMAIL,
- USERNAME,
- ABBREV
- }
-
public enum DiffView {
SIDE_BY_SIDE,
UNIFIED_DIFF
@@ -131,10 +123,6 @@
/** Number of changes to show in a screen. */
public Integer changesPerPage;
- /** Should the site header be displayed when logged in ? */
- public Boolean showSiteHeader;
- /** Should the Flash helper movie be used to copy text to the clipboard? */
- public Boolean useFlashClipboard;
/** Type of download URL the user prefers to use. */
public String downloadScheme;
/** Type of download command the user prefers to use. */
@@ -148,7 +136,6 @@
public DiffView diffView;
public Boolean sizeBarInChangeTable;
public Boolean legacycidInChangeTable;
- public ReviewCategoryStrategy reviewCategoryStrategy;
public Boolean muteCommonPathPrefixes;
public Boolean signedOffBy;
public EmailStrategy emailStrategy;
@@ -160,10 +147,6 @@
public List<String> changeTable;
public Map<String, String> urlAliases;
- public boolean isShowInfoInReviewCategory() {
- return getReviewCategoryStrategy() != ReviewCategoryStrategy.NONE;
- }
-
public DateFormat getDateFormat() {
if (dateFormat == null) {
return DateFormat.STD;
@@ -178,13 +161,6 @@
return timeFormat;
}
- public ReviewCategoryStrategy getReviewCategoryStrategy() {
- if (reviewCategoryStrategy == null) {
- return ReviewCategoryStrategy.NONE;
- }
- return reviewCategoryStrategy;
- }
-
public DiffView getDiffView() {
if (diffView == null) {
return DiffView.SIDE_BY_SIDE;
@@ -209,8 +185,6 @@
public static GeneralPreferencesInfo defaults() {
GeneralPreferencesInfo p = new GeneralPreferencesInfo();
p.changesPerPage = DEFAULT_PAGESIZE;
- p.showSiteHeader = true;
- p.useFlashClipboard = true;
p.downloadScheme = null;
p.downloadCommand = DownloadCommand.CHECKOUT;
p.dateFormat = DateFormat.STD;
@@ -221,7 +195,6 @@
p.diffView = DiffView.SIDE_BY_SIDE;
p.sizeBarInChangeTable = true;
p.legacycidInChangeTable = false;
- p.reviewCategoryStrategy = ReviewCategoryStrategy.NONE;
p.muteCommonPathPrefixes = true;
p.signedOffBy = false;
p.emailStrategy = EmailStrategy.ENABLED;
diff --git a/java/com/google/gerrit/extensions/common/ChangeConfigInfo.java b/java/com/google/gerrit/extensions/common/ChangeConfigInfo.java
index 1e822e3..e8aeb40 100644
--- a/java/com/google/gerrit/extensions/common/ChangeConfigInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeConfigInfo.java
@@ -23,4 +23,5 @@
public String replyTooltip;
public int updateDelay;
public Boolean submitWholeTopic;
+ public Boolean excludeMergeableInChangeInfo;
}
diff --git a/java/com/google/gerrit/extensions/restapi/UnprocessableEntityException.java b/java/com/google/gerrit/extensions/restapi/UnprocessableEntityException.java
index 9ed83b2..c1a9ad2 100644
--- a/java/com/google/gerrit/extensions/restapi/UnprocessableEntityException.java
+++ b/java/com/google/gerrit/extensions/restapi/UnprocessableEntityException.java
@@ -21,4 +21,8 @@
public UnprocessableEntityException(String msg) {
super(msg);
}
+
+ public UnprocessableEntityException(String msg, Throwable cause) {
+ super(msg, cause);
+ }
}
diff --git a/java/com/google/gerrit/server/account/Preferences.java b/java/com/google/gerrit/server/account/Preferences.java
index f33d8fe..bba5843 100644
--- a/java/com/google/gerrit/server/account/Preferences.java
+++ b/java/com/google/gerrit/server/account/Preferences.java
@@ -65,8 +65,6 @@
* <p>The config file has separate sections for general, diff and edit preferences:
*
* <pre>
- * [general]
- * showSiteHeader = false
* [diff]
* hideTopMenu = true
* [edit]
@@ -401,7 +399,7 @@
my.add(new MenuItem("Edits", "#/q/has:edit", null));
my.add(new MenuItem("Watched Changes", "#/q/is:watched+is:open", null));
my.add(new MenuItem("Starred Changes", "#/q/is:starred", null));
- my.add(new MenuItem("Groups", "#/groups/self", null));
+ my.add(new MenuItem("Groups", "#/settings/#Groups", null));
}
return my;
}
diff --git a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index 3cbcb73..04911eb 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -448,6 +448,7 @@
try {
suggestReviewers.setQuery(r.getQuery());
suggestReviewers.setLimit(r.getLimit());
+ suggestReviewers.setExcludeGroups(r.getExcludeGroups());
return suggestReviewers.apply(change).value();
} catch (Exception e) {
throw asRestApiException("Cannot retrieve suggested reviewers", e);
diff --git a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index cc0a2f1..f1bd690 100644
--- a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -547,7 +547,7 @@
}
@Override
- public List<TestSubmitRuleInfo> testSubmitRule(TestSubmitRuleInput in) throws RestApiException {
+ public TestSubmitRuleInfo testSubmitRule(TestSubmitRuleInput in) throws RestApiException {
try {
return testSubmitRule.get().apply(revision, in).value();
} catch (Exception e) {
diff --git a/java/com/google/gerrit/server/change/DeleteChangeOp.java b/java/com/google/gerrit/server/change/DeleteChangeOp.java
index 2449df2..37ac713 100644
--- a/java/com/google/gerrit/server/change/DeleteChangeOp.java
+++ b/java/com/google/gerrit/server/change/DeleteChangeOp.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.extensions.events.ChangeDeleted;
@@ -114,7 +115,23 @@
String prefix = PatchSet.id(id, 1).toRefName();
prefix = prefix.substring(0, prefix.length() - 1);
for (Map.Entry<String, ObjectId> e : ctx.getRepoView().getRefs(prefix).entrySet()) {
- ctx.addRefUpdate(e.getValue(), ObjectId.zeroId(), prefix + e.getKey());
+ removeRef(ctx, e, prefix);
}
+ removeUserEdits(ctx);
+ }
+
+ private void removeUserEdits(RepoContext ctx) throws IOException {
+ String prefix = RefNames.REFS_USERS;
+ String editRef = String.format("/edit-%s/", id);
+ for (Map.Entry<String, ObjectId> e : ctx.getRepoView().getRefs(prefix).entrySet()) {
+ if (e.getKey().contains(editRef)) {
+ removeRef(ctx, e, prefix);
+ }
+ }
+ }
+
+ private void removeRef(RepoContext ctx, Map.Entry<String, ObjectId> entry, String prefix)
+ throws IOException {
+ ctx.addRefUpdate(entry.getValue(), ObjectId.zeroId(), prefix + entry.getKey());
}
}
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
index 87ee27f..07bd963 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
@@ -190,6 +190,7 @@
"Replacing change in index",
Metadata.builder()
.changeId(cd.getId().get())
+ .patchSetId(cd.currentPatchSet().number())
.indexVersion(i.getSchema().getVersion())
.build())) {
i.replace(cd);
diff --git a/java/com/google/gerrit/server/logging/Metadata.java b/java/com/google/gerrit/server/logging/Metadata.java
index 7184a92..dae78c0 100644
--- a/java/com/google/gerrit/server/logging/Metadata.java
+++ b/java/com/google/gerrit/server/logging/Metadata.java
@@ -15,8 +15,14 @@
package com.google.gerrit.server.logging;
import com.google.auto.value.AutoValue;
+import com.google.common.base.MoreObjects;
+import com.google.common.base.MoreObjects.ToStringHelper;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.Arrays;
import java.util.Optional;
/** Metadata that is provided to {@link PerformanceLogger}s as context for performance records. */
@@ -134,6 +140,99 @@
// The username of an account.
public abstract Optional<String> username();
+ /**
+ * Returns a string representation of this instance that is suitable for logging.
+ *
+ * <p>{@link #toString()} formats the {@link Optional} fields as {@code key=Optional[value]} or
+ * {@code key=Optional.empty}. Since this class has many optional fields from which usually only a
+ * few are populated this leads to long string representations such as
+ *
+ * <pre>
+ * Metadata{accountId=Optional.empty, actionType=Optional.empty, authDomainName=Optional.empty,
+ * branchName=Optional.empty, cacheKey=Optional.empty, cacheName=Optional.empty,
+ * className=Optional.empty, changeId=Optional[9212550], changeIdType=Optional.empty,
+ * eventType=Optional.empty, exportValue=Optional.empty, filePath=Optional.empty,
+ * garbageCollectorName=Optional.empty, gitOperation=Optional.empty, groupId=Optional.empty,
+ * groupName=Optional.empty, groupUuid=Optional.empty, httpStatus=Optional.empty,
+ * indexName=Optional.empty, indexVersion=Optional[0], methodName=Optional.empty,
+ * multiple=Optional.empty, operationName=Optional.empty, partial=Optional.empty,
+ * noteDbFilePath=Optional.empty, noteDbRefName=Optional.empty,
+ * noteDbSequenceType=Optional.empty, noteDbTable=Optional.empty, patchSetId=Optional.empty,
+ * pluginMetadata=[], pluginName=Optional.empty, projectName=Optional.empty,
+ * pushType=Optional.empty, resourceCount=Optional.empty, restViewName=Optional.empty,
+ * revision=Optional.empty, username=Optional.empty}
+ * </pre>
+ *
+ * <p>That's hard to read in logs. This is why this method
+ *
+ * <ul>
+ * <li>drops fields which have {@code Optional.empty} as value and
+ * <li>reformats values that are {@code Optional[value]} to {@code value}.
+ * </ul>
+ *
+ * <p>For the example given above the formatted string would look like this:
+ *
+ * <pre>
+ * Metadata{changeId=9212550, indexVersion=0, pluginMetadata=[]}
+ * </pre>
+ *
+ * @return string representation of this instance that is suitable for logging
+ */
+ public String toStringForLogging() {
+ // Append class name.
+ String className = getClass().getSimpleName();
+ if (className.startsWith("AutoValue_")) {
+ className = className.substring(10);
+ }
+ ToStringHelper stringHelper = MoreObjects.toStringHelper(className);
+
+ // Append key-value pairs for field which are set.
+ Method[] methods = Metadata.class.getDeclaredMethods();
+ Arrays.<Method>sort(methods, (m1, m2) -> m1.getName().compareTo(m2.getName()));
+ for (Method method : methods) {
+ if (Modifier.isStatic(method.getModifiers())) {
+ // skip static method
+ continue;
+ }
+
+ if (method.getName().equals(Thread.currentThread().getStackTrace()[1].getMethodName())) {
+ // skip this method (toStringForLogging() method)
+ continue;
+ }
+
+ if (method.getReturnType().equals(Void.TYPE) || method.getParameterCount() > 0) {
+ // skip method since it's not a getter
+ continue;
+ }
+
+ method.setAccessible(true);
+
+ Object returnValue;
+ try {
+ returnValue = method.invoke(this);
+ } catch (IllegalArgumentException | IllegalAccessException | InvocationTargetException e) {
+ // should never happen
+ throw new IllegalStateException(e);
+ }
+
+ if (returnValue instanceof Optional) {
+ Optional<?> fieldValueOptional = (Optional<?>) returnValue;
+ if (!fieldValueOptional.isPresent()) {
+ // drop this key-value pair
+ continue;
+ }
+
+ // format as 'key=value' instead of 'key=Optional[value]'
+ stringHelper.add(method.getName(), fieldValueOptional.get());
+ } else {
+ // not an Optional value, keep as is
+ stringHelper.add(method.getName(), returnValue);
+ }
+ }
+
+ return stringHelper.toString();
+ }
+
public static Metadata.Builder builder() {
return new AutoValue_Metadata.Builder();
}
diff --git a/java/com/google/gerrit/server/logging/TraceContext.java b/java/com/google/gerrit/server/logging/TraceContext.java
index 06db7b4..b597a51 100644
--- a/java/com/google/gerrit/server/logging/TraceContext.java
+++ b/java/com/google/gerrit/server/logging/TraceContext.java
@@ -196,7 +196,8 @@
LoggingContext.getInstance()
.addPerformanceLogRecord(
() -> PerformanceLogRecord.create(operation, elapsedMs, metadata));
- logger.atFine().log("%s (%s) (%d ms)", operation, metadata, elapsedMs);
+ logger.atFine().log(
+ "%s (%s) (%d ms)", operation, metadata.toStringForLogging(), elapsedMs);
});
}
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index 32c8b06..5691a82 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -280,16 +280,6 @@
}
}
- public void flush() throws IOException {
- checkNotExecuted();
- if (changeRepo != null) {
- changeRepo.flush();
- }
- if (allUsersRepo != null) {
- allUsersRepo.flush();
- }
- }
-
@Nullable
public BatchRefUpdate execute() throws IOException {
return execute(false);
diff --git a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
index 36c6d36..7495957 100644
--- a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
@@ -31,9 +31,9 @@
import com.google.gerrit.server.rules.SubmitRule;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
-import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Optional;
import java.util.stream.Collectors;
/**
@@ -83,15 +83,15 @@
this.opts = options;
}
- public static List<SubmitRecord> defaultRuleError() {
+ public static SubmitRecord defaultRuleError() {
return createRuleError(DEFAULT_MSG);
}
- public static List<SubmitRecord> createRuleError(String err) {
+ public static SubmitRecord createRuleError(String err) {
SubmitRecord rec = new SubmitRecord();
rec.status = SubmitRecord.Status.RULE_ERROR;
rec.errorMessage = err;
- return Collections.singletonList(rec);
+ return rec;
}
public static SubmitTypeRecord defaultTypeError() {
@@ -120,7 +120,7 @@
throw new NoSuchProjectException(cd.project());
}
} catch (StorageException | NoSuchProjectException e) {
- return ruleError("Error looking up change " + cd.getId(), e);
+ return Collections.singletonList(ruleError("Error looking up change " + cd.getId(), e));
}
if ((!opts.allowClosed() || OnlineReindexMode.isActive()) && change.isClosed()) {
@@ -133,12 +133,13 @@
// and then we collect the results in one list.
return Streams.stream(submitRules)
.map(c -> c.call(s -> s.evaluate(cd)))
- .flatMap(Collection::stream)
+ .filter(Optional::isPresent)
+ .map(Optional::get)
.collect(Collectors.toList());
}
}
- private List<SubmitRecord> ruleError(String err, Exception e) {
+ private SubmitRecord ruleError(String err, Exception e) {
logger.atSevere().withCause(e).log(err);
return defaultRuleError();
}
diff --git a/java/com/google/gerrit/server/restapi/change/Rebase.java b/java/com/google/gerrit/server/restapi/change/Rebase.java
index 5f0a56b..bd3742b 100644
--- a/java/com/google/gerrit/server/restapi/change/Rebase.java
+++ b/java/com/google/gerrit/server/restapi/change/Rebase.java
@@ -165,7 +165,7 @@
}
} catch (NoSuchChangeException e) {
throw new UnprocessableEntityException(
- String.format("Base change not found: %s", input.base));
+ String.format("Base change not found: %s", input.base), e);
}
PatchSet.Id baseId = base.patchSet().id();
diff --git a/java/com/google/gerrit/server/restapi/change/SuggestChangeReviewers.java b/java/com/google/gerrit/server/restapi/change/SuggestChangeReviewers.java
index 213bae9..28296dd 100644
--- a/java/com/google/gerrit/server/restapi/change/SuggestChangeReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/SuggestChangeReviewers.java
@@ -39,15 +39,20 @@
public class SuggestChangeReviewers extends SuggestReviewers
implements RestReadView<ChangeResource> {
+ private final PermissionBackend permissionBackend;
+ private final Provider<CurrentUser> self;
+ private final ProjectCache projectCache;
+
+ private boolean excludeGroups;
+
@Option(
name = "--exclude-groups",
aliases = {"-e"},
usage = "exclude groups from query")
- boolean excludeGroups;
-
- private final PermissionBackend permissionBackend;
- private final Provider<CurrentUser> self;
- private final ProjectCache projectCache;
+ public SuggestChangeReviewers setExcludeGroups(boolean excludeGroups) {
+ this.excludeGroups = excludeGroups;
+ return this;
+ }
@Inject
SuggestChangeReviewers(
diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
index d5ed9a4..bae2e52 100644
--- a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
+++ b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
@@ -15,8 +15,6 @@
package com.google.gerrit.server.restapi.change;
import com.google.common.base.MoreObjects;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Lists;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.TestSubmitRuleInfo;
@@ -37,7 +35,6 @@
import com.google.gerrit.server.rules.RulesCache;
import com.google.inject.Inject;
import java.util.LinkedHashMap;
-import java.util.List;
import org.kohsuke.args4j.Option;
public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubmitRuleInput> {
@@ -65,7 +62,7 @@
}
@Override
- public Response<List<TestSubmitRuleInfo>> apply(RevisionResource rsrc, TestSubmitRuleInput input)
+ public Response<TestSubmitRuleInfo> apply(RevisionResource rsrc, TestSubmitRuleInput input)
throws AuthException, PermissionBackendException, BadRequestException {
if (input == null) {
input = new TestSubmitRuleInput();
@@ -83,15 +80,12 @@
throw new BadRequestException("project not found");
}
ChangeData cd = changeDataFactory.create(rsrc.getNotes());
- List<SubmitRecord> records =
- ImmutableList.copyOf(
- prologRule.evaluate(
- cd, PrologOptions.dryRunOptions(input.rule, input.filters == Filters.SKIP)));
- List<TestSubmitRuleInfo> out = Lists.newArrayListWithCapacity(records.size());
+ SubmitRecord record =
+ prologRule.evaluate(
+ cd, PrologOptions.dryRunOptions(input.rule, input.filters == Filters.SKIP));
+
AccountLoader accounts = accountInfoFactory.create(true);
- for (SubmitRecord r : records) {
- out.add(newSubmitRuleInfo(r, accounts));
- }
+ TestSubmitRuleInfo out = newSubmitRuleInfo(record, accounts);
accounts.fill();
return Response.ok(out);
}
diff --git a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
index 32d6f17..a36e75c 100644
--- a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
+++ b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
@@ -234,7 +234,9 @@
+ "\u2026";
info.updateDelay =
(int) ConfigUtil.getTimeUnit(config, "change", null, "updateDelay", 300, TimeUnit.SECONDS);
- info.submitWholeTopic = MergeSuperSet.wholeTopicEnabled(config);
+ info.submitWholeTopic = toBoolean(MergeSuperSet.wholeTopicEnabled(config));
+ info.excludeMergeableInChangeInfo =
+ toBoolean(this.config.getBoolean("change", "api", "excludeMergeableInChangeInfo", false));
info.disablePrivateChanges =
toBoolean(this.config.getBoolean("change", null, "disablePrivateChanges", false));
return info;
diff --git a/java/com/google/gerrit/server/rules/DefaultSubmitRule.java b/java/com/google/gerrit/server/rules/DefaultSubmitRule.java
index ee997b2..0016789 100644
--- a/java/com/google/gerrit/server/rules/DefaultSubmitRule.java
+++ b/java/com/google/gerrit/server/rules/DefaultSubmitRule.java
@@ -31,8 +31,8 @@
import com.google.inject.Singleton;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
import java.util.List;
+import java.util.Optional;
/**
* Java implementation of Gerrit's default pre-submit rules behavior: check if the labels have the
@@ -62,13 +62,13 @@
}
@Override
- public Collection<SubmitRecord> evaluate(ChangeData cd) {
+ public Optional<SubmitRecord> evaluate(ChangeData cd) {
ProjectState projectState = projectCache.get(cd.project());
// In case at least one project has a rules.pl file, we let Prolog handle it.
// The Prolog rules engine will also handle the labels for us.
if (projectState == null || projectState.hasPrologRules()) {
- return Collections.emptyList();
+ return Optional.empty();
}
SubmitRecord submitRecord = new SubmitRecord();
@@ -85,7 +85,7 @@
submitRecord.errorMessage = "Unable to fetch labels and approvals for the change";
submitRecord.status = SubmitRecord.Status.RULE_ERROR;
- return Collections.singletonList(submitRecord);
+ return Optional.of(submitRecord);
}
submitRecord.labels = new ArrayList<>(labelTypes.size());
@@ -98,7 +98,7 @@
submitRecord.errorMessage = "Unable to find the LabelFunction for label " + t.getName();
submitRecord.status = SubmitRecord.Status.RULE_ERROR;
- return Collections.singletonList(submitRecord);
+ return Optional.of(submitRecord);
}
Collection<PatchSetApproval> approvalsForLabel = getApprovalsForLabel(approvals, t);
@@ -118,7 +118,7 @@
}
}
- return Collections.singletonList(submitRecord);
+ return Optional.of(submitRecord);
}
private static List<PatchSetApproval> getApprovalsForLabel(
diff --git a/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java b/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
index ff5d99e..f64ab13 100644
--- a/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
+++ b/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
@@ -17,7 +17,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
@@ -34,6 +33,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.Optional;
/**
* Rule to require an approval from a user that did not upload the current patch set or block
@@ -59,7 +59,7 @@
IgnoreSelfApprovalRule() {}
@Override
- public Collection<SubmitRecord> evaluate(ChangeData cd) {
+ public Optional<SubmitRecord> evaluate(ChangeData cd) {
List<LabelType> labelTypes;
List<PatchSetApproval> approvals;
try {
@@ -67,13 +67,13 @@
approvals = cd.currentApprovals();
} catch (StorageException e) {
logger.atWarning().withCause(e).log(E_UNABLE_TO_FETCH_LABELS);
- return singletonRuleError(E_UNABLE_TO_FETCH_LABELS);
+ return ruleError(E_UNABLE_TO_FETCH_LABELS);
}
boolean shouldIgnoreSelfApproval = labelTypes.stream().anyMatch(LabelType::ignoreSelfApproval);
if (!shouldIgnoreSelfApproval) {
// Shortcut to avoid further processing if no label should ignore uploader approvals
- return ImmutableList.of();
+ return Optional.empty();
}
Account.Id uploader;
@@ -81,7 +81,7 @@
uploader = cd.currentPatchSet().uploader();
} catch (StorageException e) {
logger.atWarning().withCause(e).log(E_UNABLE_TO_FETCH_UPLOADER);
- return singletonRuleError(E_UNABLE_TO_FETCH_UPLOADER);
+ return ruleError(E_UNABLE_TO_FETCH_UPLOADER);
}
SubmitRecord submitRecord = new SubmitRecord();
@@ -123,10 +123,10 @@
}
if (submitRecord.labels.isEmpty()) {
- return ImmutableList.of();
+ return Optional.empty();
}
- return ImmutableList.of(submitRecord);
+ return Optional.of(submitRecord);
}
private static boolean labelCheckPassed(SubmitRecord.Label label) {
@@ -143,11 +143,11 @@
return false;
}
- private static Collection<SubmitRecord> singletonRuleError(String reason) {
+ private static Optional<SubmitRecord> ruleError(String reason) {
SubmitRecord submitRecord = new SubmitRecord();
submitRecord.errorMessage = reason;
submitRecord.status = SubmitRecord.Status.RULE_ERROR;
- return ImmutableList.of(submitRecord);
+ return Optional.of(submitRecord);
}
@VisibleForTesting
diff --git a/java/com/google/gerrit/server/rules/PrologRule.java b/java/com/google/gerrit/server/rules/PrologRule.java
index e15b4b5..bf1d545 100644
--- a/java/com/google/gerrit/server/rules/PrologRule.java
+++ b/java/com/google/gerrit/server/rules/PrologRule.java
@@ -21,8 +21,7 @@
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import java.util.Collection;
-import java.util.Collections;
+import java.util.Optional;
@Singleton
public class PrologRule implements SubmitRule {
@@ -36,17 +35,17 @@
}
@Override
- public Collection<SubmitRecord> evaluate(ChangeData cd) {
+ public Optional<SubmitRecord> evaluate(ChangeData cd) {
ProjectState projectState = projectCache.get(cd.project());
// We only want to run the Prolog engine if we have at least one rules.pl file to use.
if ((projectState == null || !projectState.hasPrologRules())) {
- return Collections.emptyList();
+ return Optional.empty();
}
- return evaluate(cd, PrologOptions.defaultOptions());
+ return Optional.of(evaluate(cd, PrologOptions.defaultOptions()));
}
- public Collection<SubmitRecord> evaluate(ChangeData cd, PrologOptions opts) {
+ public SubmitRecord evaluate(ChangeData cd, PrologOptions opts) {
return getEvaluator(cd, opts).evaluate();
}
diff --git a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
index 7f6450d..0b49ae5 100644
--- a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
+++ b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.rules;
+import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.project.SubmitRuleEvaluator.createRuleError;
import static com.google.gerrit.server.project.SubmitRuleEvaluator.defaultRuleError;
import static com.google.gerrit.server.project.SubmitRuleEvaluator.defaultTypeError;
@@ -50,7 +51,6 @@
import com.googlecode.prolog_cafe.lang.VariableTerm;
import java.io.StringReader;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.Collections;
import java.util.List;
@@ -140,10 +140,9 @@
/**
* Evaluate the submit rules.
*
- * @return List of {@link SubmitRecord} objects returned from the evaluated rules, including any
- * errors.
+ * @return {@link SubmitRecord} returned from the evaluated rules. Can include errors.
*/
- public Collection<SubmitRecord> evaluate() {
+ public SubmitRecord evaluate() {
Change change;
try {
change = cd.change();
@@ -193,28 +192,32 @@
* output. Later after the loop the out collection is reversed to restore it to the original
* ordering.
*/
- public List<SubmitRecord> resultsToSubmitRecord(Term submitRule, List<Term> results) {
- boolean foundOk = false;
- List<SubmitRecord> out = new ArrayList<>(results.size());
+ public SubmitRecord resultsToSubmitRecord(Term submitRule, List<Term> results) {
+ checkState(!results.isEmpty(), "the list of Prolog terms must not be empty");
+
+ SubmitRecord resultSubmitRecord = new SubmitRecord();
+ resultSubmitRecord.labels = new ArrayList<>();
for (int resultIdx = results.size() - 1; 0 <= resultIdx; resultIdx--) {
Term submitRecord = results.get(resultIdx);
- SubmitRecord rec = new SubmitRecord();
- out.add(rec);
if (!(submitRecord instanceof StructureTerm) || 1 != submitRecord.arity()) {
return invalidResult(submitRule, submitRecord);
}
- if ("ok".equals(submitRecord.name())) {
- rec.status = SubmitRecord.Status.OK;
-
- } else if ("not_ready".equals(submitRecord.name())) {
- rec.status = SubmitRecord.Status.NOT_READY;
-
- } else {
+ if (!"ok".equals(submitRecord.name()) && !"not_ready".equals(submitRecord.name())) {
return invalidResult(submitRule, submitRecord);
}
+ // This transformation is required to adapt Prolog's behavior to the way Gerrit handles
+ // SubmitRecords, as defined in the SubmitRecord#allRecordsOK method.
+ // When several rules are defined in Prolog, they are all matched to a SubmitRecord. We want
+ // the change to be submittable when at least one result is OK.
+ if ("ok".equals(submitRecord.name())) {
+ resultSubmitRecord.status = SubmitRecord.Status.OK;
+ } else if ("not_ready".equals(submitRecord.name()) && resultSubmitRecord.status == null) {
+ resultSubmitRecord.status = SubmitRecord.Status.NOT_READY;
+ }
+
// Unpack the one argument. This should also be a structure with one
// argument per label that needs to be reported on to the caller.
//
@@ -224,8 +227,6 @@
return invalidResult(submitRule, submitRecord);
}
- rec.labels = new ArrayList<>(submitRecord.arity());
-
for (Term state : ((StructureTerm) submitRecord).args()) {
if (!(state instanceof StructureTerm)
|| 2 != state.arity()
@@ -234,7 +235,7 @@
}
SubmitRecord.Label lbl = new SubmitRecord.Label();
- rec.labels.add(lbl);
+ resultSubmitRecord.labels.add(lbl);
lbl.label = checkLabelName(state.arg(0).name());
Term status = state.arg(1);
@@ -265,24 +266,12 @@
}
}
- if (rec.status == SubmitRecord.Status.OK) {
- foundOk = true;
+ if (resultSubmitRecord.status == SubmitRecord.Status.OK) {
break;
}
}
- Collections.reverse(out);
-
- // This transformation is required to adapt Prolog's behavior to the way Gerrit handles
- // SubmitRecords, as defined in the SubmitRecord#allRecordsOK method.
- // When several rules are defined in Prolog, they are all matched to a SubmitRecord. We want
- // the change to be submittable when at least one result is OK.
- if (foundOk) {
- for (SubmitRecord record : out) {
- record.status = SubmitRecord.Status.OK;
- }
- }
-
- return out;
+ Collections.reverse(resultSubmitRecord.labels);
+ return resultSubmitRecord;
}
@VisibleForTesting
@@ -299,7 +288,7 @@
return VALID_LABEL_MATCHER.retainFrom(name);
}
- private List<SubmitRecord> invalidResult(Term rule, Term record, String reason) {
+ private SubmitRecord invalidResult(Term rule, Term record, String reason) {
return ruleError(
String.format(
"Submit rule %s for change %s of %s output invalid result: %s%s",
@@ -310,15 +299,15 @@
(reason == null ? "" : ". Reason: " + reason)));
}
- private List<SubmitRecord> invalidResult(Term rule, Term record) {
+ private SubmitRecord invalidResult(Term rule, Term record) {
return invalidResult(rule, record, null);
}
- private List<SubmitRecord> ruleError(String err) {
+ private SubmitRecord ruleError(String err) {
return ruleError(err, null);
}
- private List<SubmitRecord> ruleError(String err, Exception e) {
+ private SubmitRecord ruleError(String err, Exception e) {
if (opts.logErrors()) {
logger.atSevere().withCause(e).log(err);
return defaultRuleError();
diff --git a/java/com/google/gerrit/server/rules/SubmitRule.java b/java/com/google/gerrit/server/rules/SubmitRule.java
index 20cb8fb..b221117 100644
--- a/java/com/google/gerrit/server/rules/SubmitRule.java
+++ b/java/com/google/gerrit/server/rules/SubmitRule.java
@@ -16,13 +16,13 @@
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.server.query.change.ChangeData;
-import java.util.Collection;
+import java.util.Optional;
/**
* Allows plugins to decide whether a change is ready to be submitted or not.
*
- * <p>For a given {@link ChangeData}, each plugin is called and returns a {@link Collection} of
- * {@link SubmitRecord}. This collection can be empty, or contain one or several values.
+ * <p>For a given {@link ChangeData}, each plugin is called and returns a {@link Optional} of {@link
+ * SubmitRecord}.
*
* <p>A Change can only be submitted if all the plugins give their consent.
*
@@ -38,6 +38,9 @@
*/
@ExtensionPoint
public interface SubmitRule {
- /** Returns a {@link Collection} of {@link SubmitRecord} status for the change. */
- Collection<SubmitRecord> evaluate(ChangeData changeData);
+ /**
+ * Returns a {@link Optional} of {@link SubmitRecord} status for the change. {@code
+ * Optional#empty()} if the SubmitRule was a no-op.
+ */
+ Optional<SubmitRecord> evaluate(ChangeData changeData);
}
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index c8b101c..d24832b 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -315,7 +315,7 @@
.addTag(RequestId.Type.TRACE_ID, "retry-on-failure-" + new RequestId())
.forceLogging();
logger.atFine().withCause(t).log(
- "%s failed, retry with tracing enabled", caller);
+ "AutoRetry: %s failed, retry with tracing enabled", caller);
metrics.autoRetryCount.increment(actionType, caller);
return true;
}
@@ -323,7 +323,8 @@
// A non-recoverable failure occurred. We retried the operation with tracing
// enabled and it failed again. Log the failure so that admin can see if it
// differs from the failure that triggered the retry.
- logger.atFine().withCause(t).log("auto-retry of %s has failed", caller);
+ logger.atFine().withCause(t).log(
+ "AutoRetry: auto-retry of %s has failed", caller);
metrics.failuresOnAutoRetryCount.increment(actionType, caller);
return false;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
index 12266c9..f253533 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
@@ -28,7 +28,6 @@
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DownloadCommand;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailFormat;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
-import com.google.gerrit.extensions.client.GeneralPreferencesInfo.ReviewCategoryStrategy;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.TimeFormat;
import com.google.gerrit.extensions.client.MenuItem;
import com.google.gerrit.extensions.config.DownloadScheme;
@@ -66,15 +65,13 @@
new MenuItem("Edits", "#/q/has:edit", null),
new MenuItem("Watched Changes", "#/q/is:watched+is:open", null),
new MenuItem("Starred Changes", "#/q/is:starred", null),
- new MenuItem("Groups", "#/groups/self", null));
+ new MenuItem("Groups", "#/settings/#Groups", null));
assertThat(o.changeTable).isEmpty();
GeneralPreferencesInfo i = GeneralPreferencesInfo.defaults();
// change all default values
i.changesPerPage *= -1;
- i.showSiteHeader ^= true;
- i.useFlashClipboard ^= true;
i.downloadCommand = DownloadCommand.REPO_DOWNLOAD;
i.dateFormat = DateFormat.US;
i.timeFormat = TimeFormat.HHMM_24;
@@ -88,7 +85,6 @@
i.legacycidInChangeTable ^= true;
i.muteCommonPathPrefixes ^= true;
i.signedOffBy ^= true;
- i.reviewCategoryStrategy = ReviewCategoryStrategy.ABBREV;
i.diffView = DiffView.UNIFIED_DIFF;
i.my = new ArrayList<>();
i.my.add(new MenuItem("name", "url"));
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 918b62d..359ed1d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -83,6 +83,7 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.FooterConstants;
+import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
@@ -116,6 +117,7 @@
import com.google.gerrit.extensions.client.Comment.Range;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.client.SubmitType;
@@ -864,6 +866,21 @@
assertThat(thrown).hasMessageThat().contains("Cannot revert initial commit");
}
+ @Test
+ public void cantCreateRevertWithoutProjectWritePermission() throws Exception {
+ PushOneCommit.Result r = createChange();
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit();
+ projectCache.checkedGet(project).getProject().setState(ProjectState.READ_ONLY);
+
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class, () -> gApi.changes().id(r.getChangeId()).revert());
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("project state " + ProjectState.READ_ONLY + " does not permit write");
+ }
+
@FunctionalInterface
private interface Rebase {
void call(String id) throws RestApiException;
@@ -1400,6 +1417,47 @@
}
@Test
+ public void deleteChangeRemovesItsChangeEdit() throws Exception {
+ PushOneCommit.Result result = createChange();
+
+ requestScopeOperations.setApiUser(user.id());
+ String changeId = result.getChangeId();
+ gApi.changes().id(changeId).edit().create();
+ gApi.changes()
+ .id(changeId)
+ .edit()
+ .modifyFile(FILE_NAME, RawInputUtil.create("foo".getBytes(UTF_8)));
+
+ requestScopeOperations.setApiUser(admin.id());
+ try (Repository repo = repoManager.openRepository(project)) {
+ String expected =
+ RefNames.refsUsers(user.id()) + "/edit-" + result.getChange().getId() + "/1";
+ assertThat(repo.getRefDatabase().getRefsByPrefix(expected)).isNotEmpty();
+ gApi.changes().id(changeId).delete();
+ assertThat(repo.getRefDatabase().getRefsByPrefix(expected)).isEmpty();
+ }
+ }
+
+ @Test
+ public void deleteChangeDoesntRemoveOtherChangeEdits() throws Exception {
+ PushOneCommit.Result result = createChange();
+ PushOneCommit.Result irrelevantChangeResult = createChange();
+ requestScopeOperations.setApiUser(admin.id());
+ String changeId = result.getChangeId();
+ String irrelevantChangeId = irrelevantChangeResult.getChangeId();
+
+ gApi.changes().id(irrelevantChangeId).edit().create();
+ gApi.changes()
+ .id(irrelevantChangeId)
+ .edit()
+ .modifyFile(FILE_NAME, RawInputUtil.create("foo".getBytes(UTF_8)));
+
+ gApi.changes().id(changeId).delete();
+
+ assertThat(gApi.changes().id(irrelevantChangeId).edit().get()).isPresent();
+ }
+
+ @Test
public void rebaseUpToDateChange() throws Exception {
PushOneCommit.Result r = createChange();
ResourceConflictException thrown =
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
index 1842a9ec..4d76074 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
@@ -30,7 +30,7 @@
import com.google.gerrit.server.rules.SubmitRule;
import com.google.inject.Module;
import java.util.ArrayList;
-import java.util.Collection;
+import java.util.Optional;
import org.junit.Test;
public class ChangeSubmitRequirementIT extends AbstractDaemonTest {
@@ -66,12 +66,12 @@
private static class CustomSubmitRule implements SubmitRule {
@Override
- public Collection<SubmitRecord> evaluate(ChangeData changeData) {
+ public Optional<SubmitRecord> evaluate(ChangeData changeData) {
SubmitRecord record = new SubmitRecord();
record.labels = new ArrayList<>();
record.status = SubmitRecord.Status.NOT_READY;
record.requirements = ImmutableList.of(req);
- return ImmutableList.of(record);
+ return Optional.of(record);
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java
index 62600b2..aa613fb 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java
@@ -262,8 +262,8 @@
TestSubmitRuleInput in = new TestSubmitRuleInput();
in.rule = "invalid prolog rule";
// We have no rules.pl by default. The fact that the default rules are showing up here is a bug.
- List<TestSubmitRuleInfo> response = gApi.changes().id(changeId).current().testSubmitRule(in);
- assertThat(response).containsExactly(invalidPrologRuleInfo());
+ TestSubmitRuleInfo response = gApi.changes().id(changeId).current().testSubmitRule(in);
+ assertThat(response).isEqualTo(invalidPrologRuleInfo());
}
@Test
@@ -274,8 +274,8 @@
TestSubmitRuleInput in = new TestSubmitRuleInput();
in.rule = "invalid prolog rule";
- List<TestSubmitRuleInfo> response = gApi.changes().id(changeId).current().testSubmitRule(in);
- assertThat(response).containsExactly(invalidPrologRuleInfo());
+ TestSubmitRuleInfo response = gApi.changes().id(changeId).current().testSubmitRule(in);
+ assertThat(response).isEqualTo(invalidPrologRuleInfo());
}
private static TestSubmitRuleInfo invalidPrologRuleInfo() {
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index 15b9a93..ac37530 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -52,8 +52,8 @@
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.List;
+import java.util.Optional;
import java.util.SortedMap;
import java.util.SortedSet;
import org.apache.http.message.BasicHeader;
@@ -679,7 +679,7 @@
boolean failOnce;
@Override
- public Collection<SubmitRecord> evaluate(ChangeData changeData) {
+ public Optional<SubmitRecord> evaluate(ChangeData changeData) {
if (failOnce) {
failOnce = false;
throw new IllegalStateException("forced failure from test");
@@ -691,7 +691,7 @@
SubmitRecord submitRecord = new SubmitRecord();
submitRecord.status = SubmitRecord.Status.OK;
- return ImmutableList.of(submitRecord);
+ return Optional.of(submitRecord);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
index 5401a2c..6cc2059 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
@@ -34,7 +34,9 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.api.accounts.EmailInput;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -134,6 +136,27 @@
}
@Test
+ public void suggestReviewersWithExcludeGroups() throws Exception {
+ String changeId = createChange().getChangeId();
+
+ // by default groups are included
+ List<SuggestedReviewerInfo> reviewers = suggestReviewers(changeId, name("user"));
+ assertReviewers(
+ reviewers, ImmutableList.of(user1, user2, user3), ImmutableList.of(group1, group2, group3));
+
+ // exclude groups
+ reviewers =
+ gApi.changes().id(changeId).suggestReviewers(name("user")).excludeGroups(true).get();
+ assertReviewers(reviewers, ImmutableList.of(user1, user2, user3), ImmutableList.of());
+
+ // explicitly include groups
+ reviewers =
+ gApi.changes().id(changeId).suggestReviewers(name("user")).excludeGroups(false).get();
+ assertReviewers(
+ reviewers, ImmutableList.of(user1, user2, user3), ImmutableList.of(group1, group2, group3));
+ }
+
+ @Test
@GerritConfig(name = "accounts.visibility", value = "SAME_GROUP")
public void suggestReviewersSameGroupVisibility() throws Exception {
String changeId = createChange().getChangeId();
@@ -493,6 +516,38 @@
}
@Test
+ public void suggestNoExistingReviewers() throws Exception {
+ String name = name("foo");
+ TestAccount foo1 = accountCreator.create(name + "-1");
+ TestAccount foo2 = accountCreator.create(name + "-2");
+
+ String changeId = createChange().getChangeId();
+ assertReviewers(
+ suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
+
+ gApi.changes().id(changeId).addReviewer(foo2.id().toString());
+ assertReviewers(suggestReviewers(changeId, name), ImmutableList.of(foo1), ImmutableList.of());
+ }
+
+ @Test
+ public void suggestCcAsReviewer() throws Exception {
+ String name = name("foo");
+ TestAccount foo1 = accountCreator.create(name + "-1");
+ TestAccount foo2 = accountCreator.create(name + "-2");
+
+ String changeId = createChange().getChangeId();
+ assertReviewers(
+ suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
+
+ AddReviewerInput reviewerInput = new AddReviewerInput();
+ reviewerInput.reviewer = foo2.id().toString();
+ reviewerInput.state = ReviewerState.CC;
+ gApi.changes().id(changeId).addReviewer(reviewerInput);
+ assertReviewers(
+ suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
+ }
+
+ @Test
public void suggestBySecondaryEmailWithModifyAccount() throws Exception {
String secondaryEmail = "foo.secondary@example.com";
TestAccount foo = createAccountWithSecondaryEmail("foo", secondaryEmail);
diff --git a/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java b/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
index 4a74018..1d87ca1 100644
--- a/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
@@ -168,6 +168,8 @@
assertThat(i.change.replyLabel).isEqualTo("Reply\u2026");
assertThat(i.change.updateDelay).isEqualTo(300);
assertThat(i.change.disablePrivateChanges).isNull();
+ assertThat(i.change.submitWholeTopic).isNull();
+ assertThat(i.change.excludeMergeableInChangeInfo).isNull();
// download
assertThat(i.download.archives).containsExactly("tar", "tbz2", "tgz", "txz");
@@ -190,4 +192,18 @@
// user
assertThat(i.user.anonymousCowardName).isEqualTo(AnonymousCowardNameProvider.DEFAULT);
}
+
+ @Test
+ @GerritConfig(name = "change.submitWholeTopic", value = "true")
+ public void serverConfigWithSubmitWholeTopic() throws Exception {
+ ServerInfo i = gApi.config().server().getInfo();
+ assertThat(i.change.submitWholeTopic).isTrue();
+ }
+
+ @Test
+ @GerritConfig(name = "change.api.excludeMergeableInChangeInfo", value = "true")
+ public void serverConfigWithExcludeMergeableInChangeInfo() throws Exception {
+ ServerInfo i = gApi.config().server().getInfo();
+ assertThat(i.change.excludeMergeableInChangeInfo).isTrue();
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java b/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
index 37237c6..1c820af 100644
--- a/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.server.rules;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
@@ -24,8 +25,8 @@
import com.google.gerrit.common.data.SubmitRequirement;
import com.google.gerrit.server.rules.IgnoreSelfApprovalRule;
import com.google.inject.Inject;
-import java.util.Collection;
import java.util.Map;
+import java.util.Optional;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.junit.Test;
@@ -41,10 +42,10 @@
PushOneCommit.Result r = createChange();
approve(r.getChangeId());
- Collection<SubmitRecord> submitRecords = rule.evaluate(r.getChange());
+ Optional<SubmitRecord> submitRecord = rule.evaluate(r.getChange());
- assertThat(submitRecords).hasSize(1);
- SubmitRecord result = submitRecords.iterator().next();
+ assertThat(submitRecord).isPresent();
+ SubmitRecord result = submitRecord.get();
assertThat(result.status).isEqualTo(SubmitRecord.Status.NOT_READY);
assertThat(result.labels).isNotEmpty();
assertThat(result.requirements)
@@ -67,8 +68,8 @@
// Approve as admin
approve(r.getChangeId());
- Collection<SubmitRecord> submitRecords = rule.evaluate(r.getChange());
- assertThat(submitRecords).isEmpty();
+ Optional<SubmitRecord> submitRecord = rule.evaluate(r.getChange());
+ assertThat(submitRecord).isEmpty();
}
@Test
@@ -78,8 +79,8 @@
PushOneCommit.Result r = createChange();
approve(r.getChangeId());
- Collection<SubmitRecord> submitRecords = rule.evaluate(r.getChange());
- assertThat(submitRecords).isEmpty();
+ Optional<SubmitRecord> submitRecord = rule.evaluate(r.getChange());
+ assertThat(submitRecord).isEmpty();
}
private void enableRule(String labelName, boolean newState) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java
index efc3b5b..de2b105 100644
--- a/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.server.rules;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -30,7 +31,6 @@
import com.googlecode.prolog_cafe.lang.StructureTerm;
import com.googlecode.prolog_cafe.lang.Term;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.List;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
@@ -46,9 +46,9 @@
StructureTerm labels = new StructureTerm("label", verifiedLabel);
List<Term> terms = ImmutableList.of(makeTerm("ok", labels));
- Collection<SubmitRecord> records = evaluator.resultsToSubmitRecord(null, terms);
+ SubmitRecord record = evaluator.resultsToSubmitRecord(null, terms);
- assertThat(records).hasSize(1);
+ assertThat(record.status).isEqualTo(SubmitRecord.Status.OK);
}
/**
@@ -113,23 +113,16 @@
terms.add(makeTerm("not_ready", makeLabels(label3)));
// When
- List<SubmitRecord> records = evaluator.resultsToSubmitRecord(null, terms);
+ SubmitRecord record = evaluator.resultsToSubmitRecord(null, terms);
// assert that
- SubmitRecord record1Expected = new SubmitRecord();
- record1Expected.status = SubmitRecord.Status.OK;
- record1Expected.labels = new ArrayList<>();
- record1Expected.labels.add(submitRecordLabel2);
+ SubmitRecord expectedRecord = new SubmitRecord();
+ expectedRecord.status = SubmitRecord.Status.OK;
+ expectedRecord.labels = new ArrayList<>();
+ expectedRecord.labels.add(submitRecordLabel2);
+ expectedRecord.labels.add(submitRecordLabel3);
- SubmitRecord record2Expected = new SubmitRecord();
- record2Expected.status = SubmitRecord.Status.OK;
- record2Expected.labels = new ArrayList<>();
- record2Expected.labels.add(submitRecordLabel3);
-
- assertThat(records).hasSize(2);
-
- assertThat(records.get(0)).isEqualTo(record1Expected);
- assertThat(records.get(1)).isEqualTo(record2Expected);
+ assertThat(record).isEqualTo(expectedRecord);
}
private static Term makeTerm(String status, StructureTerm labels) {
diff --git a/javatests/com/google/gerrit/server/account/PreferencesTest.java b/javatests/com/google/gerrit/server/account/PreferencesTest.java
new file mode 100644
index 0000000..b1d31bf
--- /dev/null
+++ b/javatests/com/google/gerrit/server/account/PreferencesTest.java
@@ -0,0 +1,66 @@
+// Copyright (C) 2019 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.account;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.git.ValidationError;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+/** Tests for parsing user preferences from Git. */
+public class PreferencesTest {
+
+ enum Unknown {
+ STATE
+ }
+
+ @Test
+ public void ignoreUnknownAccountPreferencesWhenParsing() {
+ ValidationError.Sink errorSink = Mockito.mock(ValidationError.Sink.class);
+ Preferences preferences =
+ new Preferences(Account.id(1), configWithUnknownEntries(), new Config(), errorSink);
+ GeneralPreferencesInfo parsedPreferences = preferences.getGeneralPreferences();
+
+ assertThat(parsedPreferences).isNotNull();
+ assertThat(parsedPreferences.expandInlineDiffs).isTrue();
+ verifyNoMoreInteractions(errorSink);
+ }
+
+ @Test
+ public void ignoreUnknownDefaultAccountPreferencesWhenParsing() {
+ ValidationError.Sink errorSink = Mockito.mock(ValidationError.Sink.class);
+ Preferences preferences =
+ new Preferences(Account.id(1), new Config(), configWithUnknownEntries(), errorSink);
+ GeneralPreferencesInfo parsedPreferences = preferences.getGeneralPreferences();
+
+ assertThat(parsedPreferences).isNotNull();
+ assertThat(parsedPreferences.expandInlineDiffs).isTrue();
+ verifyNoMoreInteractions(errorSink);
+ }
+
+ private static Config configWithUnknownEntries() {
+ Config cfg = new Config();
+ cfg.setBoolean("general", null, "expandInlineDiffs", true);
+ cfg.setBoolean("general", null, "unknown", true);
+ cfg.setEnum("general", null, "unknownenum", Unknown.STATE);
+ cfg.setString("general", null, "unknownstring", "bla");
+ return cfg;
+ }
+}
diff --git a/javatests/com/google/gerrit/server/logging/MetadataTest.java b/javatests/com/google/gerrit/server/logging/MetadataTest.java
new file mode 100644
index 0000000..89e5690
--- /dev/null
+++ b/javatests/com/google/gerrit/server/logging/MetadataTest.java
@@ -0,0 +1,36 @@
+// Copyright (C) 2019 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.logging;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+
+public class MetadataTest {
+
+ @Test
+ public void stringForLoggingOmitsEmptyOptionalValuesAndReformatsOptionalValuesThatArePresent() {
+ Metadata metadata = Metadata.builder().accountId(1000001).branchName("refs/heads/foo").build();
+ assertThat(metadata.toStringForLogging())
+ .isEqualTo("Metadata{accountId=1000001, branchName=refs/heads/foo, pluginMetadata=[]}");
+ }
+
+ @Test
+ public void
+ stringForLoggingOmitsEmptyOptionalValuesAndReformatsOptionalValuesThatArePresentNoFieldsSet() {
+ Metadata metadata = Metadata.builder().build();
+ assertThat(metadata.toStringForLogging()).isEqualTo("Metadata{pluginMetadata=[]}");
+ }
+}
diff --git a/plugins/replication b/plugins/replication
index 90253f8..9b10716 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 90253f88e42653a0c3af632beae507847f71adec
+Subproject commit 9b10716f0afaf58e29e591d22859120c4e35a702
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
index 8e57534..63f3eaf 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
@@ -329,8 +329,17 @@
name: 'ldap/tests tests'}});
assert.equal(element._rules.length, 3);
assert.equal(Object.keys(element._groupsWithRules).length, 3);
- assert.deepEqual(element.permission.value.rules['ldap:CN=test test'],
- {action: 'ALLOW', min: -2, max: 2, added: true});
+ if (Polymer.Element) {
+ // Under Polymer 2 gr-rule-editor.js#_handleValueChange get's
+ // fully loaded before this change, thus `modified: true` get's managed
+ // to be added. Under Polymer 1 it was a mix hence why it was not
+ // added in time for when this test ran.
+ assert.deepEqual(element.permission.value.rules['ldap:CN=test test'],
+ {action: 'ALLOW', min: -2, max: 2, modified: true, added: true});
+ } else {
+ assert.deepEqual(element.permission.value.rules['ldap:CN=test test'],
+ {action: 'ALLOW', min: -2, max: 2, added: true});
+ }
// New rule should be removed if cancel from editing.
element.editing = false;
assert.equal(element._rules.length, 2);
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
index 61362a8..5459749 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
@@ -344,6 +344,12 @@
} else if (obj[k].added) {
this._updateAddRemoveObj(addRemoveObj,
path.concat(ref), 'add', obj[k]);
+ /**
+ * As add / delete both can happen in the new section,
+ * so here to make sure it will remove the deleted ones.
+ * @see Issue 11339
+ */
+ this._recursivelyRemoveDeleted(addRemoveObj.add[k]);
continue;
}
this._recursivelyUpdateAddRemoveObj(obj[k], addRemoveObj,
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
index abd11d6..8b1de6e 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
@@ -416,6 +416,72 @@
assert.deepEqual(obj, expectedResult);
});
+ test('_recursivelyUpdateAddRemoveObj on new added section', () => {
+ const obj = {
+ 'refs/for/*': {
+ permissions: {
+ 'label-Code-Review': {
+ rules: {
+ e798fed07afbc9173a587f876ef8760c78d240c1: {
+ min: -2,
+ max: 2,
+ action: 'ALLOW',
+ added: true,
+ },
+ },
+ added: true,
+ label: 'Code-Review',
+ },
+ 'labelAs-Code-Review': {
+ rules: {
+ 'ldap:gerritcodereview-eng': {
+ min: -2,
+ max: 2,
+ action: 'ALLOW',
+ added: true,
+ deleted: true,
+ },
+ },
+ added: true,
+ label: 'Code-Review',
+ },
+ },
+ added: true,
+ },
+ };
+
+ const expectedResult = {
+ add: {
+ 'refs/for/*': {
+ permissions: {
+ 'label-Code-Review': {
+ rules: {
+ e798fed07afbc9173a587f876ef8760c78d240c1: {
+ min: -2,
+ max: 2,
+ action: 'ALLOW',
+ added: true,
+ },
+ },
+ added: true,
+ label: 'Code-Review',
+ },
+ 'labelAs-Code-Review': {
+ rules: {},
+ added: true,
+ label: 'Code-Review',
+ },
+ },
+ added: true,
+ },
+ },
+ remove: {},
+ };
+ const updateObj = {add: {}, remove: {}};
+ element._recursivelyUpdateAddRemoveObj(obj, updateObj);
+ assert.deepEqual(updateObj, expectedResult);
+ });
+
test('_handleSaveForReview with no changes', () => {
assert.deepEqual(element._computeAddAndRemove(), {add: {}, remove: {}});
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 8176d80..cbb653b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -330,6 +330,12 @@
-ms-user-select: text;
user-select: text;
}
+
+ .whitespace-change-only-message {
+ background-color: var(--diff-context-control-background-color);
+ border: 1px solid var(--diff-context-control-border-color);
+ text-align: center;
+ }
</style>
<style include="gr-syntax-theme"></style>
<style include="gr-ranged-comment-theme"></style>
@@ -365,6 +371,13 @@
id="diffTable"
class$="[[_diffTableClass]]"
role="presentation"></table>
+
+ <template is="dom-if" if="[[showNoChangeMessage(loading, prefs, _diffLength)]]">
+ <div class="whitespace-change-only-message">
+ This file only contains whitespace changes.
+ Modify the whitespace setting to see the changes.
+ </div>
+ </template>
</gr-diff-builder>
</gr-diff-highlight>
</gr-diff-selection>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index c69b313..113e28f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -296,6 +296,12 @@
this._unobserveNodes();
},
+ showNoChangeMessage(loading, prefs, diffLength) {
+ return !loading &&
+ prefs && prefs.ignore_whitespace !== 'IGNORE_NONE'
+ && diffLength === 0;
+ },
+
_enableSelectionObserver(loggedIn, isAttached) {
// Polymer 2: check for undefined
if ([loggedIn, isAttached].some(arg => arg === undefined)) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index a86f5c7..15deaff 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -1007,6 +1007,100 @@
});
});
});
+
+ suite('whitespace changes only message', () => {
+ const setupDiff = function(ignore_whitespace, diffContent) {
+ element = fixture('basic');
+ element.prefs = {
+ ignore_whitespace,
+ auto_hide_diff_table_header: true,
+ context: 10,
+ cursor_blink_rate: 0,
+ font_size: 12,
+ intraline_difference: true,
+ line_length: 100,
+ line_wrapping: false,
+ show_line_endings: true,
+ show_tabs: true,
+ show_whitespace_errors: true,
+ syntax_highlighting: true,
+ tab_size: 8,
+ theme: 'DEFAULT',
+ };
+
+ element.diff = {
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/carrot.js b/carrot.js',
+ 'index 2adc47d..f9c2f2c 100644',
+ '--- a/carrot.js',
+ '+++ b/carrot.jjs',
+ 'file differ',
+ ],
+ content: diffContent,
+ binary: true,
+ };
+
+ element._renderDiffTable();
+ flushAsynchronousOperations();
+ };
+
+ test('show the message if ignore_whitespace is criteria matches', () => {
+ setupDiff('IGNORE_ALL', [{skip: 100}]);
+ assert.isTrue(element.showNoChangeMessage(
+ /* loading= */ false,
+ element.prefs,
+ element._diffLength
+ ));
+ });
+
+ test('do not show the message if still loading', () => {
+ setupDiff('IGNORE_ALL', [{skip: 100}]);
+ assert.isFalse(element.showNoChangeMessage(
+ /* loading= */ true,
+ element.prefs,
+ element._diffLength
+ ));
+ });
+
+ test('do not show the message if contains valid changes', () => {
+ const content = [{
+ a: ['all work and no play make andybons a dull boy'],
+ b: ['elgoog elgoog elgoog'],
+ }, {
+ ab: [
+ 'Non eram nescius, Brute, cum, quae summis ingeniis ',
+ 'exquisitaque doctrina philosophi Graeco sermone tractavissent',
+ ],
+ }];
+ setupDiff('IGNORE_ALL', content);
+ assert.equal(element._diffLength, 3);
+ assert.isFalse(element.showNoChangeMessage(
+ /* loading= */ false,
+ element.prefs,
+ element._diffLength
+ ));
+ });
+
+ test('do not show message if ignore whitespace is disabled', () => {
+ const content = [{
+ a: ['all work and no play make andybons a dull boy'],
+ b: ['elgoog elgoog elgoog'],
+ }, {
+ ab: [
+ 'Non eram nescius, Brute, cum, quae summis ingeniis ',
+ 'exquisitaque doctrina philosophi Graeco sermone tractavissent',
+ ],
+ }];
+ setupDiff('IGNORE_NONE', content);
+ assert.isFalse(element.showNoChangeMessage(
+ /* loading= */ false,
+ element.prefs,
+ element._diffLength
+ ));
+ });
+ });
});
a11ySuite('basic');
diff --git a/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api.html b/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api.html
new file mode 100644
index 0000000..74b87c8
--- /dev/null
+++ b/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api.html
@@ -0,0 +1,18 @@
+<!--
+@license
+Copyright (C) 2019 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.
+-->
+
+<script src="gr-styles-api.js"></script>
diff --git a/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api.js b/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api.js
new file mode 100644
index 0000000..879b392
--- /dev/null
+++ b/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api.js
@@ -0,0 +1,77 @@
+/**
+ * @license
+ * Copyright (C) 2019 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.
+ */
+(function(window) {
+ 'use strict';
+
+ // Prevent redefinition.
+ if (window.GrStylesApi) { return; }
+
+ let styleObjectCount = 0;
+
+ function GrStyleObject(rulesStr) {
+ this._rulesStr = rulesStr;
+ this._className = `__pg_js_api_class_${styleObjectCount}`;
+ styleObjectCount++;
+ }
+
+ /**
+ * Creates a new unique CSS class and injects it in a root node of the element
+ * if it hasn't been added yet. A root node is an document or is the
+ * associated shadowRoot. This class can be added to any element with the same
+ * root node.
+ * @param {HTMLElement} element The element to get class name for.
+ * @return {string} Appropriate class name for the element is returned
+ */
+ GrStyleObject.prototype.getClassName = function(element) {
+ const rootNode = Polymer.Settings.useShadow
+ ? element.getRootNode() : document.body;
+ if (!rootNode.__pg_js_api_style_tags) {
+ rootNode.__pg_js_api_style_tags = {};
+ }
+ if (!rootNode.__pg_js_api_style_tags[this._className]) {
+ const styleTag = document.createElement('style');
+ styleTag.innerHTML = `.${this._className} { ${this._rulesStr} }`;
+ rootNode.appendChild(styleTag);
+ rootNode.__pg_js_api_style_tags[this._className] = true;
+ }
+ return this._className;
+ };
+
+ /**
+ * Apply shared style to the element.
+ * @param {HTMLElement} element The element to apply style for
+ */
+ GrStyleObject.prototype.apply = function(element) {
+ element.classList.add(this.getClassName(element));
+ };
+
+
+ function GrStylesApi() {
+ }
+
+ /**
+ * Creates a new GrStyleObject with specified style properties.
+ * @param {string} String with style properties.
+ * @return {GrStyleObject}
+ */
+ GrStylesApi.prototype.css = function(ruleStr) {
+ return new GrStyleObject(ruleStr);
+ };
+
+
+ window.GrStylesApi = GrStylesApi;
+})(window);
diff --git a/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api_test.html b/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api_test.html
new file mode 100644
index 0000000..d67a309
--- /dev/null
+++ b/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api_test.html
@@ -0,0 +1,182 @@
+<!DOCTYPE html>
+<!--
+@license
+Copyright (C) 2019 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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-admin-api</title>
+<script src="/test/common-test-setup.js"></script>
+<script src="/bower_components/webcomponentsjs/custom-elements-es5-adapter.js"></script>
+
+<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
+<script src="/bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../../test/common-test-setup.html"/>
+<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
+<link rel="import" href="gr-styles-api.html">
+
+<script>void(0);</script>
+
+<dom-module id="gr-style-test-element">
+ <template>
+ <div id="wrapper"></div>
+ </template>
+ <script>Polymer({is: 'gr-style-test-element'});</script>
+</dom-module>
+
+<script>
+ suite('gr-styles-api tests', () => {
+ let sandbox;
+ let stylesApi;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ let plugin;
+ Gerrit.install(p => { plugin = p; }, '0.1',
+ 'http://test.com/plugins/testplugin/static/test.js');
+ sandbox.stub(Gerrit, '_arePluginsLoaded').returns(true);
+ stylesApi = plugin.styles();
+ });
+
+ teardown(() => {
+ stylesApi = null;
+ sandbox.restore();
+ });
+
+ test('exists', () => {
+ assert.isOk(stylesApi);
+ });
+
+ test('css', () => {
+ const styleObject = stylesApi.css('background: red');
+ assert.isDefined(styleObject);
+ });
+ });
+
+ suite('GrStyleObject tests', () => {
+ let sandbox;
+ let stylesApi;
+ let displayInlineStyle;
+ let displayNoneStyle;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ let plugin;
+ Gerrit.install(p => { plugin = p; }, '0.1',
+ 'http://test.com/plugins/testplugin/static/test.js');
+ sandbox.stub(Gerrit, '_arePluginsLoaded').returns(true);
+ stylesApi = plugin.styles();
+ displayInlineStyle = stylesApi.css('display: inline');
+ displayNoneStyle = stylesApi.css('display: none');
+ });
+
+ teardown(() => {
+ displayInlineStyle = null;
+ displayNoneStyle = null;
+ stylesApi = null;
+ sandbox.restore();
+ });
+
+ function createNestedElements(parentElement) {
+ /* parentElement
+ * |--- element1
+ * |--- element2
+ * |--- element3
+ **/
+ const element1 = document.createElement('div');
+ const element2 = document.createElement('div');
+ const element3 = document.createElement('div');
+ Polymer.dom(parentElement).appendChild(element1);
+ Polymer.dom(parentElement).appendChild(element2);
+ Polymer.dom(element2).appendChild(element3);
+
+ return [element1, element2, element3];
+ }
+
+
+ test('getClassName - body level elements', () => {
+ const bodyLevelElements = createNestedElements(document.body);
+
+ testGetClassName(bodyLevelElements);
+ });
+
+ test('getClassName - elements inside polymer element', () => {
+ const polymerElement = document.createElement('gr-style-test-element');
+ Polymer.dom(document.body).appendChild(polymerElement);
+ const contentElements = createNestedElements(polymerElement.$.wrapper);
+
+ testGetClassName(contentElements);
+ });
+
+ function testGetClassName(elements) {
+ assertAllElementsHaveDefaultStyle(elements);
+
+ const className1 = displayInlineStyle.getClassName(elements[0]);
+ const className2 = displayNoneStyle.getClassName(elements[1]);
+ const className3 = displayInlineStyle.getClassName(elements[2]);
+
+ assert.notEqual(className2, className1);
+ assert.equal(className3, className1);
+
+ assertAllElementsHaveDefaultStyle(elements);
+
+ elements[0].classList.add(className1);
+ elements[1].classList.add(className2);
+ elements[2].classList.add(className1);
+
+ assertDisplayPropertyValues(elements, ['inline', 'none', 'inline']);
+ }
+
+ test('apply - body level elements', () => {
+ const bodyLevelElements = createNestedElements(document.body);
+
+ testApply(bodyLevelElements);
+ });
+
+ test('apply - elements inside polymer element', () => {
+ const polymerElement = document.createElement('gr-style-test-element');
+ Polymer.dom(document.body).appendChild(polymerElement);
+ const contentElements = createNestedElements(polymerElement.$.wrapper);
+
+ testApply(contentElements);
+ });
+
+ function testApply(elements) {
+ assertAllElementsHaveDefaultStyle(elements);
+ displayInlineStyle.apply(elements[0]);
+ displayNoneStyle.apply(elements[1]);
+ displayInlineStyle.apply(elements[2]);
+ assertDisplayPropertyValues(elements, ['inline', 'none', 'inline']);
+ }
+
+
+ function assertAllElementsHaveDefaultStyle(elements) {
+ for (const element of elements) {
+ assert.equal(getComputedStyle(element).getPropertyValue('display'),
+ 'block');
+ }
+ }
+
+ function assertDisplayPropertyValues(elements, expectedDisplayValues) {
+ for (const key in elements) {
+ if (elements.hasOwnProperty(key)) {
+ assert.equal(
+ getComputedStyle(elements[key]).getPropertyValue('display'),
+ expectedDisplayValues[key]);
+ }
+ }
+ }
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context.js
index 45f28d1..76cb51f 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context.js
@@ -43,25 +43,26 @@
* Method to add annotations to a content line.
* @param {number} offset The char offset where the update starts.
* @param {number} length The number of chars that the update covers.
- * @param {string} cssClass The name of a CSS class created using Gerrit.css.
+ * @param {GrStyleObject} styleObject The style object for the range.
* @param {string} side The side of the update. ('left' or 'right')
*/
GrAnnotationActionsContext.prototype.annotateRange = function(
- offset, length, cssClass, side) {
+ offset, length, styleObject, side) {
if (this._contentEl && this._contentEl.getAttribute('data-side') == side) {
- GrAnnotation.annotateElement(this._contentEl, offset, length, cssClass);
+ GrAnnotation.annotateElement(this._contentEl, offset, length,
+ styleObject.getClassName(this._contentEl));
}
};
/**
* Method to add a CSS class to the line number TD element.
- * @param {string} cssClass The name of a CSS class created using Gerrit.css.
+ * @param {GrStyleObject} styleObject The style object for the range.
* @param {string} side The side of the update. ('left' or 'right')
*/
GrAnnotationActionsContext.prototype.annotateLineNumber = function(
- cssClass, side) {
+ styleObject, side) {
if (this._lineNumberEl && this._lineNumberEl.classList.contains(side)) {
- this._lineNumberEl.classList.add(cssClass);
+ styleObject.apply(this._lineNumberEl);
}
};
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context_test.html
index 653999c..2a6487e 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context_test.html
@@ -42,9 +42,13 @@
let sandbox;
let el;
let lineNumberEl;
+ let plugin;
setup(() => {
sandbox = sinon.sandbox.create();
+ Gerrit.install(p => { plugin = p; }, '0.1',
+ 'http://test.com/plugins/testplugin/static/test.js');
+
const str = 'lorem ipsum blah blah';
const line = {text: str};
el = document.createElement('div');
@@ -64,32 +68,34 @@
annotateElementSpy = sandbox.spy(GrAnnotation, 'annotateElement');
const start = 0;
const end = 100;
- const cssClass = Gerrit.css('background-color: #000000');
+ const cssStyleObject = plugin.styles().css('background-color: #000000');
// Assert annotateElement is not called when side is different.
- instance.annotateRange(start, end, cssClass, 'left');
+ instance.annotateRange(start, end, cssStyleObject, 'left');
assert.equal(annotateElementSpy.callCount, 0);
// Assert annotateElement is called once when side is the same.
- instance.annotateRange(start, end, cssClass, 'right');
+ instance.annotateRange(start, end, cssStyleObject, 'right');
assert.equal(annotateElementSpy.callCount, 1);
const args = annotateElementSpy.getCalls()[0].args;
assert.equal(args[0], el);
assert.equal(args[1], start);
assert.equal(args[2], end);
- assert.equal(args[3], cssClass);
+ assert.equal(args[3], cssStyleObject.getClassName(el));
});
test('test annotateLineNumber', () => {
- const cssClass = Gerrit.css('background-color: #000000');
+ const cssStyleObject = plugin.styles().css('background-color: #000000');
+
+ const className = cssStyleObject.getClassName(lineNumberEl);
// Assert that css class is *not* applied when side is different.
- instance.annotateLineNumber(cssClass, 'left');
- assert.isFalse(lineNumberEl.classList.contains(cssClass));
+ instance.annotateLineNumber(cssStyleObject, 'left');
+ assert.isFalse(lineNumberEl.classList.contains(className));
// Assert that css class is applied when side is the same.
- instance.annotateLineNumber(cssClass, 'right');
- assert.isTrue(lineNumberEl.classList.contains(cssClass));
+ instance.annotateLineNumber(cssStyleObject, 'right');
+ assert.isTrue(lineNumberEl.classList.contains(className));
});
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.html
index e04867a..e460660 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.html
@@ -26,6 +26,7 @@
<link rel="import" href="../../plugins/gr-popup-interface/gr-popup-interface.html">
<link rel="import" href="../../plugins/gr-repo-api/gr-repo-api.html">
<link rel="import" href="../../plugins/gr-settings-api/gr-settings-api.html">
+<link rel="import" href="../../plugins/gr-styles-api/gr-styles-api.html">
<link rel="import" href="../../plugins/gr-theme-api/gr-theme-api.html">
<link rel="import" href="../gr-rest-api-interface/gr-rest-api-interface.html">
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
index fff2e33..2dd0e095 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
@@ -319,6 +319,10 @@
return new GrSettingsApi(this);
};
+ Plugin.prototype.styles = function() {
+ return new GrStylesApi();
+ };
+
/**
* To make REST requests for plugin-provided endpoints, use
* @example
@@ -511,7 +515,13 @@
'Please use plugin.getPluginName() instead.');
};
+ /**
+ * @deprecated Use plugin.styles().css(rulesStr) instead. Please, consult
+ * the documentation how to replace it accordingly.
+ */
Gerrit.css = function(rulesStr) {
+ console.warn('Gerrit.css(rulesStr) is deprecated!',
+ 'Use plugin.styles().css(rulesStr)');
if (!Gerrit._customStyleSheet) {
const styleEl = document.createElement('style');
document.head.appendChild(styleEl);
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.html b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.html
index 5697e77..61facc0 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.html
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.html
@@ -16,8 +16,9 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
-<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
+<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<script src="/bower_components/ba-linkify/ba-linkify.js"></script>
<script src="link-text-parser.js"></script>
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
index 73295b1..3483d0e 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
@@ -126,6 +126,24 @@
assert.equal(linkEl.textContent, changeID);
});
+ test('Change-Id pattern was parsed and linked with base url', () => {
+ window.CANONICAL_PATH = '/r';
+
+ // "Change-Id:" pattern.
+ const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
+ const prefix = 'Change-Id: ';
+ element.content = prefix + changeID;
+
+ const textNode = element.$.output.childNodes[0];
+ const linkEl = element.$.output.childNodes[1];
+ assert.equal(textNode.textContent, prefix);
+ const url = '/r/q/' + changeID;
+ assert.equal(linkEl.target, '_blank');
+ // Since url is a path, the host is added automatically.
+ assert.isTrue(linkEl.href.endsWith(url));
+ assert.equal(linkEl.textContent, changeID);
+ });
+
test('Multiple matches', () => {
element.content = 'Issue 3650\nIssue 3450';
const linkEl1 = element.$.output.childNodes[0];
@@ -185,6 +203,15 @@
assert.equal(linkEl.textContent, 'foo');
});
+ test('html with base url', () => {
+ window.CANONICAL_PATH = '/r';
+
+ element.content = 'hash:foo';
+ const linkEl = element.$.output.childNodes[0];
+ assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
+ assert.equal(linkEl.textContent, 'foo');
+ });
+
test('disabled config', () => {
element.content = 'foo:baz';
assert.equal(element.$.output.innerHTML, 'foo:baz');
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
index 8526c3e..c5ac00e 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
@@ -50,6 +50,7 @@
this.linkConfig = linkConfig;
this.callback = callback;
this.removeZeroWidthSpace = opt_removeZeroWidthSpace;
+ this.baseUrl = Gerrit.BaseUrlBehavior.getBaseUrl();
Object.preventExtensions(this);
}
@@ -172,6 +173,10 @@
GrLinkTextParser.prototype.addLink =
function(text, href, position, length, outputArray) {
if (!text || this.hasOverlap(position, length, outputArray)) { return; }
+ if (!!this.baseUrl && href.startsWith('/') &&
+ !href.startsWith(this.baseUrl)) {
+ href = this.baseUrl + href;
+ }
this.addItem(text, href, null, position, length, outputArray);
};
@@ -189,6 +194,10 @@
GrLinkTextParser.prototype.addHTML =
function(html, position, length, outputArray) {
if (this.hasOverlap(position, length, outputArray)) { return; }
+ if (!!this.baseUrl && html.match(/<a href=\"\//g) &&
+ !html.match(`/<a href=\"${this.baseUrl}/g`)) {
+ html = html.replace(/<a href=\"\//g, `<a href=\"${this.baseUrl}\/`);
+ }
this.addItem(null, null, html, position, length, outputArray);
};
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index e235741..9b63f75 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -349,11 +349,7 @@
* @param {Defs.FetchJSONRequest} req
*/
_fetchJSON(req) {
- if (!req.fetchOptions) req.fetchOptions = {};
- if (!req.fetchOptions.headers) req.fetchOptions.headers = new Headers();
- if (!req.fetchOptions.headers.has('Accept')) {
- req.fetchOptions.headers.append('Accept', 'application/json');
- }
+ req = this._addAcceptJsonHeader(req);
return this._fetchRawJSON(req).then(response => {
if (!response) {
return;
@@ -425,6 +421,19 @@
return JSON.parse(source.substring(JSON_PREFIX.length));
},
+ /**
+ * @param {Defs.FetchJSONRequest} req
+ * @return {Defs.FetchJSONRequest}
+ */
+ _addAcceptJsonHeader(req) {
+ if (!req.fetchOptions) req.fetchOptions = {};
+ if (!req.fetchOptions.headers) req.fetchOptions.headers = new Headers();
+ if (!req.fetchOptions.headers.has('Accept')) {
+ req.fetchOptions.headers.append('Accept', 'application/json');
+ }
+ return req;
+ },
+
getConfig(noCache) {
if (!noCache) {
return this._fetchSharedCacheURL({
@@ -1126,7 +1135,8 @@
return;
}
this._credentialCheck.checking = true;
- const req = {url: '/accounts/self/detail', reportUrlAsIs: true};
+ let req = {url: '/accounts/self/detail', reportUrlAsIs: true};
+ req = this._addAcceptJsonHeader(req);
// Skip the REST response cache.
return this._fetchRawJSON(req).then(res => {
if (!res) { return; }
@@ -1399,7 +1409,7 @@
return this.getChangeActionURL(changeNum, null, '/detail').then(url => {
const urlWithParams = this._urlWithParams(url, optionsHex);
const params = {O: optionsHex};
- const req = {
+ let req = {
url,
errFn: opt_errFn,
cancelCondition: opt_cancelCondition,
@@ -1407,6 +1417,7 @@
fetchOptions: this._etags.getOptions(urlWithParams),
anonymizedUrl: '/changes/*~*/detail?O=' + optionsHex,
};
+ req = this._addAcceptJsonHeader(req);
return this._fetchRawJSON(req).then(response => {
if (response && response.status === 304) {
return Promise.resolve(this._parsePrefixedJSON(
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index 2299d62..9d0d83a 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -575,6 +575,15 @@
});
});
+ test('checkCredentials accepts only json', () => {
+ const authFetchStub = sandbox.stub(element._auth, 'fetch')
+ .returns(Promise.resolve());
+ element.checkCredentials();
+ assert.isTrue(authFetchStub.called);
+ assert.equal(authFetchStub.lastCall.args[1].headers.get('Accept'),
+ 'application/json');
+ });
+
test('legacy n,z key in change url is replaced', () => {
const stub = sandbox.stub(element, '_fetchJSON')
.returns(Promise.resolve([]));
@@ -1227,6 +1236,16 @@
});
});
+ test('_getChangeDetail accepts only json', () => {
+ const authFetchStub = sandbox.stub(element._auth, 'fetch')
+ .returns(Promise.resolve());
+ const errFn = sinon.stub();
+ element._getChangeDetail(123, '516714', errFn);
+ assert.isTrue(authFetchStub.called);
+ assert.equal(authFetchStub.lastCall.args[1].headers.get('Accept'),
+ 'application/json');
+ });
+
test('_getChangeDetail populates _projectLookup', () => {
sandbox.stub(element, 'getChangeActionURL')
.returns(Promise.resolve(''));
diff --git a/polygerrit-ui/app/samples/coverage-plugin.html b/polygerrit-ui/app/samples/coverage-plugin.html
index f8b5560..f5f9c6e 100644
--- a/polygerrit-ui/app/samples/coverage-plugin.html
+++ b/polygerrit-ui/app/samples/coverage-plugin.html
@@ -32,6 +32,11 @@
const coverageData = {};
let displayCoverage = false;
const annotationApi = plugin.annotationApi();
+ const styleApi = plugin.styles();
+
+ const coverageStyle = styleApi.css('background-color: #EF9B9B !important');
+ const emptyStyle = styleApi.css('');
+
annotationApi.addLayer(context => {
if (Object.keys(coverageData).length === 0) {
// Coverage data is not ready yet.
@@ -41,16 +46,16 @@
const line = context.line;
// Highlight lines missing coverage with this background color if
// coverage should be displayed, else do nothing.
- const cssClass = displayCoverage
- ? Gerrit.css('background-color: #EF9B9B')
- : Gerrit.css('');
+ const annotationStyle = displayCoverage
+ ? coverageStyle
+ : emptyStyle;
if (coverageData[path] &&
coverageData[path].changeNum === context.changeNum &&
coverageData[path].patchNum === context.patchNum) {
const linesMissingCoverage = coverageData[path].linesMissingCoverage;
if (linesMissingCoverage.includes(line.afterNumber)) {
- context.annotateRange(0, line.text.length, cssClass, 'right');
- context.annotateLineNumber(cssClass, 'right');
+ context.annotateRange(0, line.text.length, annotationStyle, 'right');
+ context.annotateLineNumber(annotationStyle, 'right');
}
}
}).enableToggleCheckbox('Display Coverage', checkbox => {
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 2b052a0..0fbc8f1 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -125,6 +125,7 @@
'edit/gr-edit-file-controls/gr-edit-file-controls_test.html',
'edit/gr-editor-view/gr-editor-view_test.html',
'plugins/gr-admin-api/gr-admin-api_test.html',
+ 'plugins/gr-styles-api/gr-styles-api_test.html',
'plugins/gr-attribute-helper/gr-attribute-helper_test.html',
'plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.html',
'plugins/gr-event-helper/gr-event-helper_test.html',
diff --git a/tools/BUILD b/tools/BUILD
index 89ce558..f4718e1 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -55,7 +55,7 @@
"-Xep:FunctionalInterfaceClash:ERROR",
"-Xep:FutureReturnValueIgnored:ERROR",
"-Xep:GetClassOnEnum:ERROR",
- "-Xep:ImmutableAnnotationChecker:ERROR",
+ "-Xep:ImmutableAnnotationChecker:WARN",
"-Xep:ImmutableEnumChecker:ERROR",
"-Xep:IncompatibleModifiers:ERROR",
"-Xep:InjectOnConstructorOfAbstractClass:ERROR",
