Merge changes I4f2c3b57,Id5536fb9,I73268c35
* changes:
Allow clients to provide ID for tracing
Don't start a new trace if request tracing was already started
TraceContext: Add generic method to start request tracing
diff --git a/Documentation/dev-polygerrit.txt b/Documentation/dev-polygerrit.txt
index 79049fc..5621d32 100644
--- a/Documentation/dev-polygerrit.txt
+++ b/Documentation/dev-polygerrit.txt
@@ -1,12 +1,15 @@
= PolyGerrit - GUI
-[IMPORTANT]
-PolyGerrit is still a beta feature. Some features may be missing.
-
== Configuring
By default both GWT and PolyGerrit UI are available to users.
+To make PolyGerrit the default UI but keep GWT as a secondary UI:
+----
+[gerrit]
+ ui = POLYGERRIT
+----
+
To disable GWT but not PolyGerrit:
----
[gerrit]
diff --git a/Documentation/images/inline-edit-add-file-page.png b/Documentation/images/inline-edit-add-file-page.png
new file mode 100644
index 0000000..1a761b4
--- /dev/null
+++ b/Documentation/images/inline-edit-add-file-page.png
Binary files differ
diff --git a/Documentation/images/inline-edit-create-change-form.png b/Documentation/images/inline-edit-create-change-form.png
new file mode 100644
index 0000000..7a93460
--- /dev/null
+++ b/Documentation/images/inline-edit-create-change-form.png
Binary files differ
diff --git a/Documentation/images/inline-edit-create-change.png b/Documentation/images/inline-edit-create-change.png
new file mode 100644
index 0000000..1df0421
--- /dev/null
+++ b/Documentation/images/inline-edit-create-change.png
Binary files differ
diff --git a/Documentation/images/inline-edit-delete-file.png b/Documentation/images/inline-edit-delete-file.png
new file mode 100644
index 0000000..1634e0f
--- /dev/null
+++ b/Documentation/images/inline-edit-delete-file.png
Binary files differ
diff --git a/Documentation/images/inline-edit-diff-screen.png b/Documentation/images/inline-edit-diff-screen.png
new file mode 100644
index 0000000..228484a
--- /dev/null
+++ b/Documentation/images/inline-edit-diff-screen.png
Binary files differ
diff --git a/Documentation/images/inline-edit-home-page.png b/Documentation/images/inline-edit-home-page.png
new file mode 100644
index 0000000..a1b8eb4
--- /dev/null
+++ b/Documentation/images/inline-edit-home-page.png
Binary files differ
diff --git a/Documentation/images/inline-edit-new-change-page.png b/Documentation/images/inline-edit-new-change-page.png
new file mode 100644
index 0000000..8a33dd6
--- /dev/null
+++ b/Documentation/images/inline-edit-new-change-page.png
Binary files differ
diff --git a/Documentation/images/inline-edit-open-file.png b/Documentation/images/inline-edit-open-file.png
new file mode 100644
index 0000000..a5422f5
--- /dev/null
+++ b/Documentation/images/inline-edit-open-file.png
Binary files differ
diff --git a/Documentation/images/inline-edit-prefill-files.png b/Documentation/images/inline-edit-prefill-files.png
new file mode 100644
index 0000000..0b2b766
--- /dev/null
+++ b/Documentation/images/inline-edit-prefill-files.png
Binary files differ
diff --git a/Documentation/images/inline-edit-review-message.png b/Documentation/images/inline-edit-review-message.png
new file mode 100644
index 0000000..bd76fad
--- /dev/null
+++ b/Documentation/images/inline-edit-review-message.png
Binary files differ
diff --git a/Documentation/images/inline-edit-start-review-button.png b/Documentation/images/inline-edit-start-review-button.png
new file mode 100644
index 0000000..df6350b
--- /dev/null
+++ b/Documentation/images/inline-edit-start-review-button.png
Binary files differ
diff --git a/Documentation/user-inline-edit.txt b/Documentation/user-inline-edit.txt
index bce8183..ada2560 100644
--- a/Documentation/user-inline-edit.txt
+++ b/Documentation/user-inline-edit.txt
@@ -1,191 +1,236 @@
-= Inline Edit
+= Creating and Editing Changes in the Gerrit Web Interface
-This page explains the workflow for creating and amending changes in the
-browser.
+== Overview
+
+The following content explains how to use the Gerrit web interface to create
+and edit changes. Use the web interface to make minor changes to files. When
+you create a change in the Gerrit user interface, you don't clone a Gerrit
+repository or use the CLI to issue Git commands — you perform your work
+directly in the Gerrit web interface.
+
+To learn more, see the link:intro-user.html[Gerrit User's Guide].
[[create-change]]
-== Creating a New Change
+== Creating a Change
-A new change can be created directly in the browser, meaning it is not necessary
-to clone the whole repository to make trivial changes.
+To create a change in the Gerrit web interface:
-The new change is created as a public
-link:user-upload.html#wip[work-in-progress change].
+. From the link:http://gerrit-review.googlesource.com[Gerrit Code Review]
+ dashboard, select Browse > Repositories.
-There are two different ways to create a new change:
+. Under Repository Name, click the name of the repository you want to work
+ on. For example, Public-Projects. To find a specific repository, enter all
+ or part of its name next to Filter:
++
+image::images/inline-edit-home-page.png[width=600]
-By clicking on the 'Create Change' button in the project screen:
+. In the left navigation panel for the repository you selected, click
+ Commands:
++
+image::images/inline-edit-create-change.png[width=350]
-[[create-change-from-project-info-screen]]
+. Under Repository Commands, click Create Change.
-image::images/inline-edit-create-change-project-screen.png[width=800, link="images/inline-edit-create-change-project-screen.png"]
+. In the Create Change window, enter the following information:
-The user can select the branch on which the new change should be created:
+ * Select branch for new change: Specify the destination branch of the
+ change.
-image::images/inline-edit-create-change-project-screen-dialog.png[width=800, link="images/inline-edit-create-change-project-screen-dialog.png"]
+ * Provide base commit SHA1 for change: Leave this field blank.
-By clicking the 'Follow-Up' button on the change screen, to create a new change
-based on the selected change.
++
+IMPORTANT: Git uses a unique SHA1 value to identify each and every commit (in
+other words, each Git commit generates a new SHA1 hash). This value differs
+from a Gerrit Change-Id, which is used by Gerrit to uniquely identify a
+change. The Gerrit Change-Id remains static throughout the life of a Gerrit
+change.
-[[create-change-from-change-screen]]
+ - Description: Briefly describe the change. Be sure to use the
+ link:dev-contributing.html#commit-message[Commit Message] format.
+ The first line becomes the subject of the change and is included in
+ the Commit Message. Because the message also appears on its own in
+ dashboards and in the results of `git log --pretty=oneline output`,
+ make the message informative and brief.
-image::images/inline-edit-create-follow-up-change.png[width=800, link="images/inline-edit-create-follow-up-change.png"]
+ - Private change: Select this option to designate this change as private.
+ Only you (and any reviewers you add) can see your private changes.
+
+. On the Create Change window, click Create. Gerrit creates a public Work
+ In Progress (WIP) change. Until the change is sent for review, it remains a
+ WIP and appears in _your_ dashboard only. In addition, all email
+ notifications are turned off.
+
+. Add the files you want to be reviewed.
+
+
+[[add-files]]
+== Adding a File to a Change
+
+Files can only be added to changes that have not been merged into the code
+base.
+
+To add a file to the change:
+
+. In the top left corner of the change, click Edit.
+. Next to Files, click Open:
+
++
+image::images/inline-edit-open-file.png[width=600]
+
+. In the Open File window, do one of the following:
+
+* To add an existing file:
+
+ ** Enter all or part of the file name in the text box. Gerrit automatically
+ populates a list of possible matching files:
++
+image::images/inline-edit-prefill-files.png[width=500]
++
+ ** Select the file you want to add to the change.
+ ** Click Open.
++
+_or,_
+
+* To create a new file, enter the name of the new file you want to add to the
+change and then click Open.
+
[[editing-change]]
-== Editing Changes
+== Modifying a Change
-To switch to edit mode, press the 'Edit' button at the top of the file list:
+To work on a file you've added to a change:
-[[switch-to-edit-mode]]
-image::images/inline-edit-enter-edit-mode-from-file-list.png[width=800, link="images/inline-edit-enter-edit-mode-from-file-list.png"]
+. On the change page, click the file name. When you add a new file to a
+ change, a blank page is displayed. When you add an existing file to a
+ change, the entire file is displayed.
-While in edit mode, it is possible to add new files to the change by clicking
-the 'Add...' button at the top of the file list.
+. Update the file and then click Save. You _must_ click Save to add the
+ file to the change.
-File changes can be reverted or files can be removed from the change or
-deleted files can be restored, by clicking the icons to the left of the file
-name.
+. To close the text editor and display the change page, click Close.
++
+When you save your work and close the file, the file is added to the change
+and the file name is listed in the Files section. The letter displayed to the
+left of the file name denotes the action performed on the file. In this case,
+one file was modified:
-To switch from edit mode back to review mode, click the 'Done Editing' button.
+- M: Modified
+- A: Added
+- D: Deleted
++
+image::images/inline-edit-add-file-page.png[width=650]
-image::images/inline-edit-file-list-in-edit-mode.png[width=800, link="images/inline-edit-file-list-in-edit-mode.png"]
+. When you're done editing and adding files, click Stop Editing.
-[[open-full-screen-editor]]
-While in edit mode, clicking on a file name in the file list opens a full
-screen editor for that file.
+. Click Publish Edit. When you publish an edit, you promote it to a regular
+ patch set. The special ref that represents the change is deleted when the
+ change is published.
-To save edits, click the 'Save' button or press `CTRL-S`. To return to the
-change screen, click the 'Close' button.
+Not happy with your edits? Click Delete Edit.
-Note that when editing the commit message, trailing blank lines will be stripped.
-image::images/inline-edit-full-screen-editor.png[width=800, link="images/inline-edit-full-screen-editor.png"]
+[[submit-change]]
+== Starting the Review
-If there are unsaved edits when the 'Close' button is pressed, a dialog will
-pop up asking to confirm the edits.
+When you start a review, Gerrit removes the WIP designation and submits
+the change to code review. The change appears in other Gerrit dashboards and
+reviewers are notified when the change is updated.
-image::images/inline-edit-confirm-unsaved-edits.png[width=800, link="images/inline-edit-confirm-unsaved-edits.png"]
+To start a review:
-To discard the unsaved edits and return to the change screen, click the 'OK'
-button. To continue editing, click 'Cancel'.
+. Open the change and then click Start Review:
++
+image::images/inline-edit-start-review-button.png[width=400]
-[[switch-to-edit-mode-from-side-by-side]]
+. In the change notification form:
-While in review mode, it is possible to switch directly to edit mode and into an
-editor for a file under review by clicking on the edit icon in the patch set list
-on the side-by-side diff view.
+ ** Add the names of the reviewers and anyone else you want to copy.
+ ** Describe the change.
+ ** Click Start Review:
++
+image::images/inline-edit-review-message.png[width=550]
-image::images/inline-edit-enter-edit-mode-from-diff.png[width=800, link="images/inline-edit-enter-edit-mode-from-diff.png"]
+The change is now displayed in other Gerrit dashboards and reviewers are
+notified that the change is available for code review.
-[[reviewing-changes-made-in-change-edit]]
-== Reviewing Change Edits
-Change edits are reviewed in the same way as regular patch sets, using the
-side-by-side diff screen. Change edits are shown as 'edit' in the patch list
-on the diff screen:
+[[review-edits]]
+== Reviewing Changes
-image::images/inline-edit-edit-in-diff-screen-patch-list.png[width=800, link="images/inline-edit-edit-in-diff-screen-patch-list.png"]
+Use the side-by-side diff screen.
-and on the change screen:
+image::images/inline-edit-diff-screen.png[width=800]
-image::images/inline-edit-edit-in-patch-list.png[width=800, link="images/inline-edit-edit-in-patch-list.png"]
+It's possible that subsequent patch sets may exist. For example, this sequence
+means that the change was created on top of patch set 9 while a regular
+patchset was uploaded later:
-Note that patch sets may exist that were created after the change edit was created.
+1 2 3 4 5 6 7 8 9 edit 10
-For example this sequence:
-`1 2 3 4 5 6 7 8 9 edit 10`
+[[search-for-changes]]
+== Searching for Changes with Pending Edits
-means that the change edit was created on top of patch set number 9 and a regular
-patch set was uploaded later.
+To find changes with pending edits:
-[[change-edit-actions]]
-== Change Edit Actions
+* From the Gerrit dashboard, select Your > Changes. All your changes are
+listed, according to Work in progress, Outgoing reviews, Incoming reviews,
+CCed on, and Recently closed.
-Change edits can be deleted, published and rebased, and a patch set that
-represents a change edit can be downloaded like a regular patch set.
+For more information about Search operators, see
+link:user-search.html[Searching Changes]. For example, to find only
+those changes that contain edits, see link:user-search.html#has[has:edit].
-[[delete-change-edit]]
-There is a special ref for a change edit. When the change edit is deleted, this
-ref is deleted as well. To delete a change edit click on the "Delete Edit"
-button.
+[change-edit-actions]
+== Modifying Changes
-[[publish-change-edit]]
-
-When a change edit is based on the current patch set, it can be published. By
-publishing a change edit it is promoted to a regular patch set. The special ref
-that represents the change edit is deleted on publish. To publish a change edit
-click on the "Publish Edit" button. This button is only shown when the change
-edit is based on the current patch set. Otherwise the change edit must first be
-rebased onto the current patch set.
[[rebase-change-edit]]
+=== Rebasing a Change Edit
-Only change edits that are based on the current patch set can be published. If
-in the meantime a new patch set was uploaded, the change edit must be rebased on
-top of the current patch set before it can be published. Rebasing a change
-edit is done by clicking on the "Rebase Edit" button. If the rebase results in
-conflicts, these conflicts cannot be resolved in the browser. In this case the
-change edit must be downloaded (see below) and the conflicts must be resolved in
-the local environment. The commit that contains the conflict resolution can then
-be uploaded by setting `edit` as option on the target ref:
+Only when a change is based on the current patch set can the change be
+published. In the meantime, if a new patch set has been uploaded, the change
+must be rebased on top of the current patch set before the change can be
+published.
-----
- $ git push host HEAD:refs/for/master%edit
-----
+To rebase a change:
+
+- Open the change and then click Rebase Edit.
+
+If the rebase generates conflicts, the conflicts can't be resolved in the web
+interface. Instead, the change must be downloaded (see below) and the conflicts
+resolved in the local environment.
+
+When the conflicts are resolved in the local environment, the commit that
+contains the conflict resolution can be uploaded by setting `edit` as an
+option on the target ref. For example:
+
+....
+$ git push host HEAD:refs/for/master%edit
+....
+
[[download-change-edit-patch]]
+=== Downloading a Patch
-Like regular patch sets, change edits can be downloaded by the download
-commands (e.g. provided by the `download-commands` plugin). To download a
-change edit, select the desired scheme from the "Download" dropdown and copy the
-command to your terminal. Note: only change edit owners and users that were
-granted the link:access-control.html#capability_accessDatabase[accessDatabase]
-global capability are able to access change edit refs.
+As with regular patch sets, you can download changes. For example, as provided
+by the `download-commands` plugin. Only the owners of a change and those
+users granted the
+link:access-control.html#capability_accessDatabase[accessDatabase] global
+capability can access change refs.
-[[search-for-change-edits]]
+To download a change:
-To search change edits from the UI the link:user-search.html#has[has:edit]
-predicate can be used.
+. Open the change, click the More icon, and then select Download patch.
+. Copy the desired scheme from the Download drop-down.
+. Paste the command into a terminal window.
-Alternatively change edits can be accessed through "My => Edits" dashboard.
-
-[[not-implemented-features]]
-== Not Implemented Features
-
-* Support default configuration options for inline editor that an
-administrator has set in `refs/users/default:preferences.config` file.
-
-* Allow to rename files that are already contained in the change (from the file table).
-The same rename file dialog can be used with preselected and disabled original file
-name.
-
-* Changed files in change edit should be marked as changed in file table in edit mode.
-One option is to use dirty icon or "*" char in front of changed files, another option
-is to use different hyperlink color for changed files (red?), to avoid adding yet another
-column to the file table
-
-* Add navigation icons in header area of edit screen. When dozen files need to be changed
-in context of change edit, this is not the best workflow to open one file in edit screen,
-change it, save it, close edit screen and select next file from the file table to edit.
-"<-" | "->" icons in header of edit screen could be used to navigate to the next file to
-change from the file table. This would behave like the navigation icons in side by side
-with the following logic on click:
-
-** "save-when-file-was-changed" or
-** "close-when-no-changes"
-
-* Implement conflict resolution during rebase of change edit using inline edit
-feature by creating new edit on top of current patch set with auto merge content
-
-* Similarly, reuse inline edit feature for conflict resolution during rebase of regular
-patch sets
+image::images/inline-edit-actions-download.png[width=600]
GERRIT
-------
+
Part of link:index.html[Gerrit Code Review]
-SEARCHBOX
----------
+SEARCHBOX
\ No newline at end of file
diff --git a/WORKSPACE b/WORKSPACE
index a2ee457..0a6caa2 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -816,13 +816,6 @@
sha1 = "42a25dc3219429f0e5d060061f71acb49bf010a0",
)
-# Only needed when jgit is built from the development tree
-maven_jar(
- name = "hamcrest-library",
- artifact = "org.hamcrest:hamcrest-library:1.3",
- sha1 = "4785a3c21320980282f9f33d0d1264a69040538f",
-)
-
TRUTH_VERS = "0.42"
maven_jar(
diff --git a/contrib/abandon_stale.py b/contrib/abandon_stale.py
index 3501b8b..2e01131 100755
--- a/contrib/abandon_stale.py
+++ b/contrib/abandon_stale.py
@@ -103,6 +103,9 @@
default=None,
action='store',
help='only abandon changes owned by the given user')
+ parser.add_option('--exclude-wip', dest='exclude_wip',
+ action='store_true',
+ help='Exclude changes that are Work-in-Progress')
parser.add_option('-v', '--verbose', dest='verbose',
action='store_true',
help='enable verbose (debug) logging')
@@ -148,7 +151,9 @@
if options.testmode:
query_terms = ["status:new", "owner:self", "topic:test-abandon"]
else:
- query_terms = ["status:new", "-is:wip", "age:%s" % options.age]
+ query_terms = ["status:new", "age:%s" % options.age]
+ if options.exclude_wip:
+ query_terms += ["-is:wip"]
if options.branches:
query_terms += ["branch:%s" % b for b in options.branches]
elif options.exclude_branches:
diff --git a/java/com/google/gerrit/acceptance/EventRecorder.java b/java/com/google/gerrit/acceptance/EventRecorder.java
index 218ee18..5654c35 100644
--- a/java/com/google/gerrit/acceptance/EventRecorder.java
+++ b/java/com/google/gerrit/acceptance/EventRecorder.java
@@ -63,6 +63,7 @@
eventListenerRegistration =
eventListeners.add(
+ "gerrit",
new UserScopedEventListener() {
@Override
public void onEvent(Event e) {
diff --git a/java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java b/java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java
new file mode 100644
index 0000000..5efdc81
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java
@@ -0,0 +1,20 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.testsuite;
+
+@FunctionalInterface
+public interface ThrowingConsumer<T> {
+ void accept(T t) throws Exception;
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
index 3d741b0..94b511b 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
@@ -138,13 +138,12 @@
return TestAccountUpdate.builder(this::updateAccount);
}
- private TestAccount updateAccount(TestAccountUpdate accountUpdate)
+ private void updateAccount(TestAccountUpdate accountUpdate)
throws OrmException, IOException, ConfigInvalidException {
AccountsUpdate.AccountUpdater accountUpdater =
(account, updateBuilder) -> fillBuilder(updateBuilder, accountUpdate, accountId);
Optional<AccountState> updatedAccount = updateAccount(accountUpdater);
checkState(updatedAccount.isPresent(), "Tried to update non-existing test account");
- return toTestAccount(updatedAccount.get());
}
private Optional<AccountState> updateAccount(AccountsUpdate.AccountUpdater accountUpdater)
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
index 517e4b5..251f452 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
@@ -15,7 +15,7 @@
package com.google.gerrit.acceptance.testsuite.account;
import com.google.auto.value.AutoValue;
-import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
+import com.google.gerrit.acceptance.testsuite.ThrowingConsumer;
import java.util.Optional;
@AutoValue
@@ -32,9 +32,9 @@
public abstract Optional<Boolean> active();
- abstract ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater();
+ abstract ThrowingConsumer<TestAccountUpdate> accountUpdater();
- public static Builder builder(ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater) {
+ public static Builder builder(ThrowingConsumer<TestAccountUpdate> accountUpdater) {
return new AutoValue_TestAccountUpdate.Builder()
.accountUpdater(accountUpdater)
.httpPassword("http-pass");
@@ -82,14 +82,13 @@
return active(false);
}
- abstract Builder accountUpdater(
- ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater);
+ abstract Builder accountUpdater(ThrowingConsumer<TestAccountUpdate> accountUpdater);
abstract TestAccountUpdate autoBuild();
- public TestAccount update() throws Exception {
+ public void update() throws Exception {
TestAccountUpdate accountUpdate = autoBuild();
- return accountUpdate.accountUpdater().apply(accountUpdate);
+ accountUpdate.accountUpdater().accept(accountUpdate);
}
}
}
diff --git a/java/com/google/gerrit/common/data/LabelType.java b/java/com/google/gerrit/common/data/LabelType.java
index 7bfd22e..29fc9c7 100644
--- a/java/com/google/gerrit/common/data/LabelType.java
+++ b/java/com/google/gerrit/common/data/LabelType.java
@@ -14,16 +14,18 @@
package com.google.gerrit.common.data;
+import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.collectingAndThen;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toMap;
+
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
public class LabelType {
public static final boolean DEF_ALLOW_POST_SUBMIT = true;
@@ -70,22 +72,13 @@
private static List<LabelValue> sortValues(List<LabelValue> values) {
values = new ArrayList<>(values);
- if (values.size() <= 1) {
- return Collections.unmodifiableList(values);
+ if (values.isEmpty()) {
+ return Collections.emptyList();
}
- Collections.sort(
- values,
- new Comparator<LabelValue>() {
- @Override
- public int compare(LabelValue o1, LabelValue o2) {
- return o1.getValue() - o2.getValue();
- }
- });
- short min = values.get(0).getValue();
- short max = values.get(values.size() - 1).getValue();
- short v = min;
+ values = values.stream().sorted(comparing(LabelValue::getValue)).collect(toList());
+ short v = values.get(0).getValue();
short i = 0;
- List<LabelValue> result = new ArrayList<>(max - min + 1);
+ ArrayList<LabelValue> result = new ArrayList<>();
// Fill in any missing values with empty text.
while (i < values.size()) {
while (v < values.get(i).getValue()) {
@@ -94,6 +87,7 @@
v++;
result.add(values.get(i++));
}
+ result.trimToSize();
return Collections.unmodifiableList(result);
}
@@ -117,7 +111,6 @@
private transient boolean canOverride;
private transient List<String> refPatterns;
- private transient List<Integer> intList;
private transient Map<Short, LabelValue> byValue;
protected LabelType() {}
@@ -148,6 +141,13 @@
setCopyMaxScore(DEF_COPY_MAX_SCORE);
setCopyMinScore(DEF_COPY_MIN_SCORE);
setAllowPostSubmit(DEF_ALLOW_POST_SUBMIT);
+
+ byValue =
+ values
+ .stream()
+ .collect(
+ collectingAndThen(
+ toMap(LabelValue::getValue, v -> v), Collections::unmodifiableMap));
}
public String getName() {
@@ -162,11 +162,8 @@
if (functionName == null) {
return null;
}
- Optional<LabelFunction> f = LabelFunction.parse(functionName);
- if (!f.isPresent()) {
- throw new IllegalStateException("Unsupported functionName: " + functionName);
- }
- return f.get();
+ return LabelFunction.parse(functionName)
+ .orElseThrow(() -> new IllegalStateException("Unsupported functionName: " + functionName));
}
public void setFunction(@Nullable LabelFunction function) {
@@ -194,7 +191,12 @@
}
public void setRefPatterns(List<String> refPatterns) {
- this.refPatterns = refPatterns;
+ if (refPatterns != null) {
+ this.refPatterns =
+ refPatterns.stream().collect(collectingAndThen(toList(), Collections::unmodifiableList));
+ } else {
+ this.refPatterns = null;
+ }
}
public List<LabelValue> getValues() {
@@ -281,36 +283,13 @@
}
public LabelValue getValue(short value) {
- initByValue();
return byValue.get(value);
}
public LabelValue getValue(PatchSetApproval ca) {
- initByValue();
return byValue.get(ca.getValue());
}
- private void initByValue() {
- if (byValue == null) {
- byValue = new HashMap<>();
- for (LabelValue v : values) {
- byValue.put(v.getValue(), v);
- }
- }
- }
-
- public List<Integer> getValuesAsList() {
- if (intList == null) {
- intList = new ArrayList<>(values.size());
- for (LabelValue v : values) {
- intList.add(Integer.valueOf(v.getValue()));
- }
- Collections.sort(intList);
- Collections.reverse(intList);
- }
- return intList;
- }
-
public LabelId getLabelId() {
return new LabelId(name);
}
diff --git a/java/com/google/gerrit/common/data/LabelValue.java b/java/com/google/gerrit/common/data/LabelValue.java
index 811e751..c0ba781 100644
--- a/java/com/google/gerrit/common/data/LabelValue.java
+++ b/java/com/google/gerrit/common/data/LabelValue.java
@@ -14,6 +14,8 @@
package com.google.gerrit.common.data;
+import java.util.Objects;
+
public class LabelValue {
public static String formatValue(short value) {
if (value < 0) {
@@ -56,6 +58,20 @@
}
@Override
+ public boolean equals(Object o) {
+ if (!(o instanceof LabelValue)) {
+ return false;
+ }
+ LabelValue v = (LabelValue) o;
+ return value == v.value && Objects.equals(text, v.text);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(value, text);
+ }
+
+ @Override
public String toString() {
return format();
}
diff --git a/java/com/google/gerrit/extensions/common/SubmitRequirementInfo.java b/java/com/google/gerrit/extensions/common/SubmitRequirementInfo.java
index a940403..53f0375 100644
--- a/java/com/google/gerrit/extensions/common/SubmitRequirementInfo.java
+++ b/java/com/google/gerrit/extensions/common/SubmitRequirementInfo.java
@@ -14,6 +14,7 @@
package com.google.gerrit.extensions.common;
+import com.google.common.base.MoreObjects;
import java.util.Map;
import java.util.Objects;
@@ -50,4 +51,14 @@
public int hashCode() {
return Objects.hash(status, fallbackText, type, data);
}
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("status", status)
+ .add("fallbackText", fallbackText)
+ .add("type", type)
+ .add("data", data)
+ .toString();
+ }
}
diff --git a/java/com/google/gerrit/extensions/registration/DynamicItem.java b/java/com/google/gerrit/extensions/registration/DynamicItem.java
index b5eafe3..4f36ab4 100644
--- a/java/com/google/gerrit/extensions/registration/DynamicItem.java
+++ b/java/com/google/gerrit/extensions/registration/DynamicItem.java
@@ -14,6 +14,7 @@
package com.google.gerrit.extensions.registration;
+import com.google.gerrit.common.Nullable;
import com.google.inject.Binder;
import com.google.inject.Key;
import com.google.inject.Provider;
@@ -77,7 +78,8 @@
* @param item item to store.
*/
public static <T> DynamicItem<T> itemOf(Class<T> member, T item) {
- return new DynamicItem<>(keyFor(TypeLiteral.get(member)), Providers.of(item), "gerrit");
+ return new DynamicItem<>(
+ keyFor(TypeLiteral.get(member)), Providers.of(item), PluginName.GERRIT);
}
@SuppressWarnings("unchecked")
@@ -126,12 +128,26 @@
* @return the configured item instance; null if no implementation has been bound to the item.
* This is common if no plugin registered an implementation for the type.
*/
+ @Nullable
public T get() {
NamedProvider<T> item = ref.get();
return item != null ? item.impl.get() : null;
}
/**
+ * Get the name of the plugin that has bound the configured item, or null.
+ *
+ * @return the name of the plugin that has bound the configured item; null if no implementation
+ * has been bound to the item. This is common if no plugin registered an implementation for
+ * the type.
+ */
+ @Nullable
+ public String getPluginName() {
+ NamedProvider<T> item = ref.get();
+ return item != null ? item.pluginName : null;
+ }
+
+ /**
* Set the element to provide.
*
* @param item the item to use. Must not be null.
@@ -154,7 +170,7 @@
NamedProvider<T> old = null;
while (!ref.compareAndSet(old, item)) {
old = ref.get();
- if (old != null && !"gerrit".equals(old.pluginName)) {
+ if (old != null && !PluginName.GERRIT.equals(old.pluginName)) {
throw new ProvisionException(
String.format(
"%s already provided by %s, ignoring plugin %s",
@@ -186,7 +202,9 @@
NamedProvider<T> old = null;
while (!ref.compareAndSet(old, item)) {
old = ref.get();
- if (old != null && !"gerrit".equals(old.pluginName) && !pluginName.equals(old.pluginName)) {
+ if (old != null
+ && !PluginName.GERRIT.equals(old.pluginName)
+ && !pluginName.equals(old.pluginName)) {
// We allow to replace:
// 1. Gerrit core items, e.g. websession cache
// can be replaced by plugin implementation
@@ -222,6 +240,7 @@
}
@Override
+ @Nullable
public ReloadableHandle replace(Key<T> newKey, Provider<T> newItem) {
NamedProvider<T> n = new NamedProvider<>(newItem, item.pluginName);
if (ref.compareAndSet(item, n)) {
diff --git a/java/com/google/gerrit/extensions/registration/DynamicItemProvider.java b/java/com/google/gerrit/extensions/registration/DynamicItemProvider.java
index 5b76741..d8dd1f9 100644
--- a/java/com/google/gerrit/extensions/registration/DynamicItemProvider.java
+++ b/java/com/google/gerrit/extensions/registration/DynamicItemProvider.java
@@ -36,7 +36,7 @@
@Override
public DynamicItem<T> get() {
- return new DynamicItem<>(key, find(injector, type), "gerrit");
+ return new DynamicItem<>(key, find(injector, type), PluginName.GERRIT);
}
private static <T> Provider<T> find(Injector src, TypeLiteral<T> type) {
diff --git a/java/com/google/gerrit/extensions/registration/DynamicMap.java b/java/com/google/gerrit/extensions/registration/DynamicMap.java
index 0bf6edd..96d19b2 100644
--- a/java/com/google/gerrit/extensions/registration/DynamicMap.java
+++ b/java/com/google/gerrit/extensions/registration/DynamicMap.java
@@ -41,12 +41,26 @@
* singleton and non-singleton members.
*/
public abstract class DynamicMap<T> implements Iterable<DynamicMap.Entry<T>> {
- public interface Entry<T> {
- String getPluginName();
+ public static class Entry<T> {
+ private final NamePair namePair;
+ private final Provider<T> provider;
- String getExportName();
+ private Entry(NamePair namePair, Provider<T> provider) {
+ this.namePair = namePair;
+ this.provider = provider;
+ }
- Provider<T> getProvider();
+ public String getPluginName() {
+ return namePair.pluginName;
+ }
+
+ public String getExportName() {
+ return namePair.exportName;
+ }
+
+ public Provider<T> getProvider() {
+ return provider;
+ }
}
/**
@@ -162,23 +176,8 @@
@Override
public Entry<T> next() {
- final Map.Entry<NamePair, Provider<T>> e = i.next();
- return new Entry<T>() {
- @Override
- public String getPluginName() {
- return e.getKey().pluginName;
- }
-
- @Override
- public String getExportName() {
- return e.getKey().exportName;
- }
-
- @Override
- public Provider<T> getProvider() {
- return e.getValue();
- }
- };
+ Map.Entry<NamePair, Provider<T>> e = i.next();
+ return new Entry<>(e.getKey(), e.getValue());
}
@Override
diff --git a/java/com/google/gerrit/extensions/registration/DynamicMapProvider.java b/java/com/google/gerrit/extensions/registration/DynamicMapProvider.java
index 420a356..9d96131 100644
--- a/java/com/google/gerrit/extensions/registration/DynamicMapProvider.java
+++ b/java/com/google/gerrit/extensions/registration/DynamicMapProvider.java
@@ -37,7 +37,7 @@
if (bindings != null) {
for (Binding<T> b : bindings) {
if (b.getKey().getAnnotation() != null) {
- m.put("gerrit", b.getKey(), b.getProvider());
+ m.put(PluginName.GERRIT, b.getKey(), b.getProvider());
}
}
}
diff --git a/java/com/google/gerrit/extensions/registration/DynamicSet.java b/java/com/google/gerrit/extensions/registration/DynamicSet.java
index 7ffb86d..df3560a 100644
--- a/java/com/google/gerrit/extensions/registration/DynamicSet.java
+++ b/java/com/google/gerrit/extensions/registration/DynamicSet.java
@@ -14,6 +14,12 @@
package com.google.gerrit.extensions.registration;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
+import static java.util.Comparator.naturalOrder;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
import com.google.inject.Binder;
import com.google.inject.Key;
import com.google.inject.Provider;
@@ -39,6 +45,24 @@
* singleton and non-singleton members.
*/
public class DynamicSet<T> implements Iterable<T> {
+ public static class Entry<T> {
+ private final String pluginName;
+ private final Provider<T> provider;
+
+ private Entry(String pluginName, Provider<T> provider) {
+ this.pluginName = pluginName;
+ this.provider = provider;
+ }
+
+ public String getPluginName() {
+ return pluginName;
+ }
+
+ public Provider<T> getProvider() {
+ return provider;
+ }
+ }
+
/**
* Declare a singleton {@code DynamicSet<T>} with a binder.
*
@@ -129,12 +153,12 @@
}
public static <T> DynamicSet<T> emptySet() {
- return new DynamicSet<>(Collections.<AtomicReference<Provider<T>>>emptySet());
+ return new DynamicSet<>(Collections.<AtomicReference<NamedProvider<T>>>emptySet());
}
- private final CopyOnWriteArrayList<AtomicReference<Provider<T>>> items;
+ private final CopyOnWriteArrayList<AtomicReference<NamedProvider<T>>> items;
- DynamicSet(Collection<AtomicReference<Provider<T>>> base) {
+ DynamicSet(Collection<AtomicReference<NamedProvider<T>>> base) {
items = new CopyOnWriteArrayList<>(base);
}
@@ -144,17 +168,33 @@
@Override
public Iterator<T> iterator() {
- final Iterator<AtomicReference<Provider<T>>> itr = items.iterator();
+ Iterator<Entry<T>> entryIterator = entryIterator();
return new Iterator<T>() {
- private T next;
+ @Override
+ public boolean hasNext() {
+ return entryIterator.hasNext();
+ }
+
+ @Override
+ public T next() {
+ Entry<T> next = entryIterator.next();
+ return next != null ? next.getProvider().get() : null;
+ }
+ };
+ }
+
+ public Iterator<Entry<T>> entryIterator() {
+ final Iterator<AtomicReference<NamedProvider<T>>> itr = items.iterator();
+ return new Iterator<Entry<T>>() {
+ private Entry<T> next;
@Override
public boolean hasNext() {
while (next == null && itr.hasNext()) {
- Provider<T> p = itr.next().get();
+ NamedProvider<T> p = itr.next().get();
if (p != null) {
try {
- next = p.get();
+ next = new Entry<>(p.pluginName, p.impl);
} catch (RuntimeException e) {
// TODO Log failed member of DynamicSet.
}
@@ -164,9 +204,9 @@
}
@Override
- public T next() {
+ public Entry<T> next() {
if (hasNext()) {
- T result = next;
+ Entry<T> result = next;
next = null;
return result;
}
@@ -198,13 +238,29 @@
}
/**
- * Add one new element to the set.
+ * Get the names of all running plugins supplying this type.
*
- * @param item the item to add to the collection. Must not be null.
- * @return handle to remove the item at a later point in time.
+ * @return sorted set of active plugins that supply at least one item.
*/
- public RegistrationHandle add(T item) {
- return add(Providers.of(item));
+ public ImmutableSortedSet<String> plugins() {
+ return items
+ .stream()
+ .map(i -> i.get().pluginName)
+ .collect(toImmutableSortedSet(naturalOrder()));
+ }
+
+ /**
+ * Get the items exported by a single plugin.
+ *
+ * @param pluginName name of the plugin.
+ * @return items exported by a plugin.
+ */
+ public ImmutableSet<Provider<T>> byPlugin(String pluginName) {
+ return items
+ .stream()
+ .filter(i -> i.get().pluginName.equals(pluginName))
+ .map(i -> i.get().impl)
+ .collect(toImmutableSet());
}
/**
@@ -213,13 +269,24 @@
* @param item the item to add to the collection. Must not be null.
* @return handle to remove the item at a later point in time.
*/
- public RegistrationHandle add(Provider<T> item) {
- final AtomicReference<Provider<T>> ref = new AtomicReference<>(item);
+ public RegistrationHandle add(String pluginName, T item) {
+ return add(pluginName, Providers.of(item));
+ }
+
+ /**
+ * Add one new element to the set.
+ *
+ * @param item the item to add to the collection. Must not be null.
+ * @return handle to remove the item at a later point in time.
+ */
+ public RegistrationHandle add(String pluginName, Provider<T> item) {
+ final AtomicReference<NamedProvider<T>> ref =
+ new AtomicReference<>(new NamedProvider<>(item, pluginName));
items.add(ref);
return new RegistrationHandle() {
@Override
public void remove() {
- if (ref.compareAndSet(item, null)) {
+ if (ref.compareAndSet(ref.get(), null)) {
items.remove(ref);
}
}
@@ -229,6 +296,7 @@
/**
* Add one new element that may be hot-replaceable in the future.
*
+ * @param pluginName unique name of the plugin providing the item.
* @param key unique description from the item's Guice binding. This can be later obtained from
* the registration handle to facilitate matching with the new equivalent instance during a
* hot reload.
@@ -236,18 +304,19 @@
* @return a handle that can remove this item later, or hot-swap the item without it ever leaving
* the collection.
*/
- public ReloadableRegistrationHandle<T> add(Key<T> key, Provider<T> item) {
- AtomicReference<Provider<T>> ref = new AtomicReference<>(item);
+ public ReloadableRegistrationHandle<T> add(String pluginName, Key<T> key, Provider<T> item) {
+ AtomicReference<NamedProvider<T>> ref =
+ new AtomicReference<>(new NamedProvider<>(item, pluginName));
items.add(ref);
- return new ReloadableHandle(ref, key, item);
+ return new ReloadableHandle(ref, key, ref.get());
}
private class ReloadableHandle implements ReloadableRegistrationHandle<T> {
- private final AtomicReference<Provider<T>> ref;
+ private final AtomicReference<NamedProvider<T>> ref;
private final Key<T> key;
- private final Provider<T> item;
+ private final NamedProvider<T> item;
- ReloadableHandle(AtomicReference<Provider<T>> ref, Key<T> key, Provider<T> item) {
+ ReloadableHandle(AtomicReference<NamedProvider<T>> ref, Key<T> key, NamedProvider<T> item) {
this.ref = ref;
this.key = key;
this.item = item;
@@ -267,8 +336,9 @@
@Override
public ReloadableHandle replace(Key<T> newKey, Provider<T> newItem) {
- if (ref.compareAndSet(item, newItem)) {
- return new ReloadableHandle(ref, newKey, newItem);
+ NamedProvider<T> n = new NamedProvider<>(newItem, item.pluginName);
+ if (ref.compareAndSet(item, n)) {
+ return new ReloadableHandle(ref, newKey, n);
}
return null;
}
diff --git a/java/com/google/gerrit/extensions/registration/DynamicSetProvider.java b/java/com/google/gerrit/extensions/registration/DynamicSetProvider.java
index 707c76a..6d36f54 100644
--- a/java/com/google/gerrit/extensions/registration/DynamicSetProvider.java
+++ b/java/com/google/gerrit/extensions/registration/DynamicSetProvider.java
@@ -38,16 +38,17 @@
return new DynamicSet<>(find(injector, type));
}
- private static <T> List<AtomicReference<Provider<T>>> find(Injector src, TypeLiteral<T> type) {
+ private static <T> List<AtomicReference<NamedProvider<T>>> find(
+ Injector src, TypeLiteral<T> type) {
List<Binding<T>> bindings = src.findBindingsByType(type);
int cnt = bindings != null ? bindings.size() : 0;
if (cnt == 0) {
return Collections.emptyList();
}
- List<AtomicReference<Provider<T>>> r = new ArrayList<>(cnt);
+ List<AtomicReference<NamedProvider<T>>> r = new ArrayList<>(cnt);
for (Binding<T> b : bindings) {
if (b.getKey().getAnnotation() != null) {
- r.add(new AtomicReference<>(b.getProvider()));
+ r.add(new AtomicReference<>(new NamedProvider<>(b.getProvider(), PluginName.GERRIT)));
}
}
return r;
diff --git a/java/com/google/gerrit/extensions/registration/PluginName.java b/java/com/google/gerrit/extensions/registration/PluginName.java
new file mode 100644
index 0000000..c110d45
--- /dev/null
+++ b/java/com/google/gerrit/extensions/registration/PluginName.java
@@ -0,0 +1,22 @@
+// Copyright (C) 2018 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.extensions.registration;
+
+public class PluginName {
+ /** Name that is used as plugin name if Gerrit core implements a plugin extension point. */
+ public static final String GERRIT = "gerrit";
+
+ private PluginName() {}
+}
diff --git a/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicTypes.java b/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicTypes.java
index 9342e0f..fd31fcd 100644
--- a/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicTypes.java
+++ b/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicTypes.java
@@ -107,7 +107,7 @@
}
public static List<RegistrationHandle> attachSets(
- Injector src, Map<TypeLiteral<?>, DynamicSet<?>> sets) {
+ Injector src, String pluginName, Map<TypeLiteral<?>, DynamicSet<?>> sets) {
if (src == null || sets == null || sets.isEmpty()) {
return Collections.emptyList();
}
@@ -123,7 +123,7 @@
for (Binding<Object> b : bindings(src, type)) {
if (b.getKey().getAnnotation() != null) {
- handles.add(set.add(b.getKey(), b.getProvider()));
+ handles.add(set.add(pluginName, b.getKey(), b.getProvider()));
}
}
}
@@ -174,8 +174,8 @@
handles = new ArrayList<>(4);
Injector parent = self.getParent();
while (parent != null) {
- handles.addAll(attachSets(self, dynamicSetsOf(parent)));
- handles.addAll(attachMaps(self, "gerrit", dynamicMapsOf(parent)));
+ handles.addAll(attachSets(self, PluginName.GERRIT, dynamicSetsOf(parent)));
+ handles.addAll(attachMaps(self, PluginName.GERRIT, dynamicMapsOf(parent)));
parent = parent.getParent();
}
if (handles.isEmpty()) {
diff --git a/java/com/google/gerrit/httpd/AllRequestFilter.java b/java/com/google/gerrit/httpd/AllRequestFilter.java
index b8b0bc8..9d171d5 100644
--- a/java/com/google/gerrit/httpd/AllRequestFilter.java
+++ b/java/com/google/gerrit/httpd/AllRequestFilter.java
@@ -76,7 +76,7 @@
// synchronized.
if (!initializedFilters.contains(filter)) {
filter.init(filterConfig);
- initializedFilters.add(filter);
+ initializedFilters.add("gerrit", filter);
}
} else {
ret = false;
@@ -89,7 +89,7 @@
initializedFilters = new DynamicSet<>();
for (AllRequestFilter filter : filtersToCleanUp) {
if (filters.contains(filter)) {
- initializedFilters.add(filter);
+ initializedFilters.add("gerrit", filter);
} else {
filter.destroy();
}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java b/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java
index 4af03a3..562687b 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java
@@ -15,6 +15,7 @@
package com.google.gerrit.httpd.restapi;
import com.google.common.base.Strings;
+import com.google.gerrit.extensions.registration.PluginName;
import com.google.gerrit.httpd.restapi.RestApiServlet.ViewData;
import com.google.gerrit.metrics.Counter1;
import com.google.gerrit.metrics.Counter2;
@@ -79,7 +80,8 @@
break;
}
}
- if (!Strings.isNullOrEmpty(viewData.pluginName) && !"gerrit".equals(viewData.pluginName)) {
+ if (!Strings.isNullOrEmpty(viewData.pluginName)
+ && !PluginName.GERRIT.equals(viewData.pluginName)) {
impl = viewData.pluginName + '-' + impl;
}
return impl;
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index e897169..38fe0e2 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -70,6 +70,7 @@
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.registration.PluginName;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
@@ -323,7 +324,7 @@
viewData = new ViewData(null, rc.list());
} else if (isPost(req)) {
RestView<RestResource> restCollectionView =
- rc.views().get("gerrit", "POST_ON_COLLECTION./");
+ rc.views().get(PluginName.GERRIT, "POST_ON_COLLECTION./");
if (restCollectionView != null) {
viewData = new ViewData(null, restCollectionView);
} else {
@@ -346,7 +347,7 @@
}
if (isPost(req) || isPut(req)) {
- RestView<RestResource> createView = rc.views().get("gerrit", "CREATE./");
+ RestView<RestResource> createView = rc.views().get(PluginName.GERRIT, "CREATE./");
if (createView != null) {
viewData = new ViewData(null, createView);
status = SC_CREATED;
@@ -355,7 +356,8 @@
throw e;
}
} else if (isDelete(req)) {
- RestView<RestResource> deleteView = rc.views().get("gerrit", "DELETE_MISSING./");
+ RestView<RestResource> deleteView =
+ rc.views().get(PluginName.GERRIT, "DELETE_MISSING./");
if (deleteView != null) {
viewData = new ViewData(null, deleteView);
status = SC_NO_CONTENT;
@@ -413,7 +415,7 @@
}
if (isPost(req) || isPut(req)) {
- RestView<RestResource> createView = c.views().get("gerrit", "CREATE./");
+ RestView<RestResource> createView = c.views().get(PluginName.GERRIT, "CREATE./");
if (createView != null) {
viewData = new ViewData(null, createView);
status = SC_CREATED;
@@ -422,7 +424,8 @@
throw e;
}
} else if (isDelete(req)) {
- RestView<RestResource> deleteView = c.views().get("gerrit", "DELETE_MISSING./");
+ RestView<RestResource> deleteView =
+ c.views().get(PluginName.GERRIT, "DELETE_MISSING./");
if (deleteView != null) {
viewData = new ViewData(null, deleteView);
status = SC_NO_CONTENT;
@@ -1244,14 +1247,14 @@
}
String name = method + "." + p.get(0);
- RestView<RestResource> core = views.get("gerrit", name);
+ RestView<RestResource> core = views.get(PluginName.GERRIT, name);
if (core != null) {
- return new ViewData("gerrit", core);
+ return new ViewData(PluginName.GERRIT, core);
}
- core = views.get("gerrit", "GET." + p.get(0));
+ core = views.get(PluginName.GERRIT, "GET." + p.get(0));
if (core != null) {
- return new ViewData("gerrit", core);
+ return new ViewData(PluginName.GERRIT, core);
}
Map<String, RestView<RestResource>> r = new TreeMap<>();
diff --git a/java/com/google/gerrit/pgm/init/InitLogging.java b/java/com/google/gerrit/pgm/init/InitLogging.java
new file mode 100644
index 0000000..52d0d2f
--- /dev/null
+++ b/java/com/google/gerrit/pgm/init/InitLogging.java
@@ -0,0 +1,67 @@
+// Copyright (C) 2018 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.pgm.init;
+
+import com.google.gerrit.pgm.init.api.InitStep;
+import com.google.gerrit.pgm.init.api.Section;
+import com.google.inject.Inject;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+public class InitLogging implements InitStep {
+ private static final String CONTAINER = "container";
+ private static final String JAVA_OPTIONS = "javaOptions";
+ private static final String FLOGGER_BACKEND_PROPERTY = "flogger.backend_factory";
+ private static final String FLOGGER_LOGGING_CONTEXT = "flogger.logging_context";
+
+ private final Section container;
+
+ @Inject
+ public InitLogging(Section.Factory sections) {
+ this.container = sections.get(CONTAINER, null);
+ }
+
+ @Override
+ public void run() throws Exception {
+ List<String> javaOptions = new ArrayList<>(Arrays.asList(container.getList(JAVA_OPTIONS)));
+ if (!isSet(javaOptions, FLOGGER_BACKEND_PROPERTY)) {
+ javaOptions.add(
+ getJavaOption(
+ FLOGGER_BACKEND_PROPERTY,
+ "com.google.common.flogger.backend.log4j.Log4jBackendFactory#getInstance"));
+ }
+ if (!isSet(javaOptions, FLOGGER_LOGGING_CONTEXT)) {
+ javaOptions.add(
+ getJavaOption(
+ FLOGGER_LOGGING_CONTEXT,
+ "com.google.gerrit.server.logging.LoggingContext#getInstance"));
+ }
+ container.setList(JAVA_OPTIONS, javaOptions);
+ }
+
+ private static boolean isSet(List<String> javaOptions, String javaOptionName) {
+ return javaOptions
+ .stream()
+ .anyMatch(
+ o ->
+ o.startsWith("-D" + javaOptionName + "=")
+ || o.startsWith("\"-D" + javaOptionName + "="));
+ }
+
+ private static String getJavaOption(String javaOptionName, String value) {
+ return String.format("-D%s=%s", javaOptionName, value);
+ }
+}
diff --git a/java/com/google/gerrit/pgm/init/InitModule.java b/java/com/google/gerrit/pgm/init/InitModule.java
index 65cf355..f677ceb 100644
--- a/java/com/google/gerrit/pgm/init/InitModule.java
+++ b/java/com/google/gerrit/pgm/init/InitModule.java
@@ -49,6 +49,7 @@
if (initDb) {
step().to(InitDatabase.class);
}
+ step().to(InitLogging.class);
step().to(InitIndex.class);
step().to(InitAuth.class);
step().to(InitAdminUser.class);
diff --git a/java/com/google/gerrit/pgm/init/api/Section.java b/java/com/google/gerrit/pgm/init/api/Section.java
index 009e989..baf37b6 100644
--- a/java/com/google/gerrit/pgm/init/api/Section.java
+++ b/java/com/google/gerrit/pgm/init/api/Section.java
@@ -23,6 +23,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.EnumSet;
+import java.util.List;
import java.util.Objects;
import java.util.Set;
@@ -59,6 +60,10 @@
return flags.cfg.getString(section, subsection, name);
}
+ public String[] getList(String name) {
+ return flags.cfg.getStringList(section, subsection, name);
+ }
+
public void set(String name, String value) {
final ArrayList<String> all = new ArrayList<>();
all.addAll(Arrays.asList(flags.cfg.getStringList(section, subsection, name)));
@@ -79,6 +84,10 @@
}
}
+ public void setList(String name, List<String> values) {
+ flags.cfg.setStringList(section, subsection, name, values);
+ }
+
public <T extends Enum<?>> void set(String name, T value) {
if (value != null) {
set(name, value.name());
diff --git a/java/com/google/gerrit/server/ChangeUtil.java b/java/com/google/gerrit/server/ChangeUtil.java
index d90f5d0..571f322 100644
--- a/java/com/google/gerrit/server/ChangeUtil.java
+++ b/java/com/google/gerrit/server/ChangeUtil.java
@@ -37,17 +37,9 @@
private static final Random UUID_RANDOM = new SecureRandom();
private static final BaseEncoding UUID_ENCODING = BaseEncoding.base16().lowerCase();
- private static final int SUBJECT_MAX_LENGTH = 80;
- private static final String SUBJECT_CROP_APPENDIX = "...";
- private static final int SUBJECT_CROP_RANGE = 10;
-
public static final Ordering<PatchSet> PS_ID_ORDER =
Ordering.from(comparingInt(PatchSet::getPatchSetId));
- public static String formatChangeUrl(String canonicalWebUrl, Change change) {
- return canonicalWebUrl + "c/" + change.getProject().get() + "/+/" + change.getChangeId();
- }
-
/** @return a new unique identifier for change message entities. */
public static String messageUuid() {
byte[] buf = new byte[8];
@@ -123,21 +115,6 @@
id);
}
- public static String cropSubject(String subject) {
- if (subject.length() > SUBJECT_MAX_LENGTH) {
- int maxLength = SUBJECT_MAX_LENGTH - SUBJECT_CROP_APPENDIX.length();
- for (int cropPosition = maxLength;
- cropPosition > maxLength - SUBJECT_CROP_RANGE;
- cropPosition--) {
- if (Character.isWhitespace(subject.charAt(cropPosition - 1))) {
- return subject.substring(0, cropPosition) + SUBJECT_CROP_APPENDIX;
- }
- }
- return subject.substring(0, maxLength) + SUBJECT_CROP_APPENDIX;
- }
- return subject;
- }
-
public static String status(Change c) {
return c != null ? c.getStatus().name().toLowerCase() : "deleted";
}
diff --git a/java/com/google/gerrit/server/cache/CacheMetrics.java b/java/com/google/gerrit/server/cache/CacheMetrics.java
index 11f2034..3435652 100644
--- a/java/com/google/gerrit/server/cache/CacheMetrics.java
+++ b/java/com/google/gerrit/server/cache/CacheMetrics.java
@@ -18,6 +18,7 @@
import com.google.common.cache.CacheStats;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.registration.PluginName;
import com.google.gerrit.metrics.CallbackMetric;
import com.google.gerrit.metrics.CallbackMetric1;
import com.google.gerrit.metrics.Description;
@@ -95,7 +96,7 @@
}
private static String metricNameOf(DynamicMap.Entry<Cache<?, ?>> e) {
- if ("gerrit".equals(e.getPluginName())) {
+ if (PluginName.GERRIT.equals(e.getPluginName())) {
return e.getExportName();
}
return String.format("plugin/%s/%s", e.getPluginName(), e.getExportName());
diff --git a/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java b/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java
index be06601..a7fdbbd 100644
--- a/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java
+++ b/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java
@@ -18,6 +18,7 @@
import com.google.common.cache.RemovalListener;
import com.google.common.cache.RemovalNotification;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.PluginName;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -36,7 +37,7 @@
private final DynamicSet<CacheRemovalListener> listeners;
private final String cacheName;
- private String pluginName = "gerrit";
+ private String pluginName = PluginName.GERRIT;
@Inject
ForwardingRemovalListener(
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 04b649b..e02f666 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -1207,6 +1207,12 @@
Collection<LabelInfo> labels = out.labels.values();
Set<Account.Id> fixed = Sets.newHashSetWithExpectedSize(labels.size());
Set<Account.Id> removable = Sets.newHashSetWithExpectedSize(labels.size());
+
+ // Check if the user has the permission to remove a reviewer. This means we can bypass the
+ // testRemoveReviewer check for a specific reviewer in the loop saving potentially many
+ // permission checks.
+ boolean canRemoveAnyReviewer =
+ permissionBackendForChange(userProvider.get(), cd).test(ChangePermission.REMOVE_REVIEWER);
for (LabelInfo label : labels) {
if (label.all == null) {
continue;
@@ -1214,8 +1220,9 @@
for (ApprovalInfo ai : label.all) {
Account.Id id = new Account.Id(ai._accountId);
- if (removeReviewerControl.testRemoveReviewer(
- cd, userProvider.get(), id, MoreObjects.firstNonNull(ai.value, 0))) {
+ if (canRemoveAnyReviewer
+ || removeReviewerControl.testRemoveReviewer(
+ cd, userProvider.get(), id, MoreObjects.firstNonNull(ai.value, 0))) {
removable.add(id);
} else {
fixed.add(id);
@@ -1232,7 +1239,8 @@
for (AccountInfo ai : ccs) {
if (ai._accountId != null) {
Account.Id id = new Account.Id(ai._accountId);
- if (removeReviewerControl.testRemoveReviewer(cd, userProvider.get(), id, 0)) {
+ if (canRemoveAnyReviewer
+ || removeReviewerControl.testRemoveReviewer(cd, userProvider.get(), id, 0)) {
removable.add(id);
}
}
diff --git a/java/com/google/gerrit/server/config/CacheResource.java b/java/com/google/gerrit/server/config/CacheResource.java
index 16c7508..ffa7b5a 100644
--- a/java/com/google/gerrit/server/config/CacheResource.java
+++ b/java/com/google/gerrit/server/config/CacheResource.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.config;
import com.google.common.cache.Cache;
+import com.google.gerrit.extensions.registration.PluginName;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.inject.Provider;
import com.google.inject.TypeLiteral;
@@ -52,7 +53,7 @@
}
public static String cacheNameOf(String plugin, String name) {
- if ("gerrit".equals(plugin)) {
+ if (PluginName.GERRIT.equals(plugin)) {
return name;
}
return plugin + "-" + name;
diff --git a/java/com/google/gerrit/server/extensions/events/EventUtil.java b/java/com/google/gerrit/server/extensions/events/EventUtil.java
index 74fba9a..5116708 100644
--- a/java/com/google/gerrit/server/extensions/events/EventUtil.java
+++ b/java/com/google/gerrit/server/extensions/events/EventUtil.java
@@ -121,8 +121,11 @@
public void logEventListenerError(Object event, Object listener, Exception error) {
logger.atWarning().log(
- "Error in event listener %s for event %s: %s",
- listener.getClass().getName(), event.getClass().getName(), error.getMessage());
+ "Error in event listener %s for event %s: %s - %s",
+ listener.getClass().getName(),
+ event.getClass().getName(),
+ error.getClass().getName(),
+ error.getMessage());
logger.atFine().withCause(error).log(
"Cause of error in event listener %s:", listener.getClass().getName());
}
diff --git a/java/com/google/gerrit/server/extensions/webui/UiActions.java b/java/com/google/gerrit/server/extensions/webui/UiActions.java
index f8cb4ce..af28bed3 100644
--- a/java/com/google/gerrit/server/extensions/webui/UiActions.java
+++ b/java/com/google/gerrit/server/extensions/webui/UiActions.java
@@ -26,6 +26,7 @@
import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.registration.PluginName;
import com.google.gerrit.extensions.restapi.RestCollection;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
@@ -169,7 +170,7 @@
PrivateInternals_UiActionDescription.setMethod(dsc, e.getExportName().substring(0, d));
PrivateInternals_UiActionDescription.setId(
- dsc, "gerrit".equals(e.getPluginName()) ? name : e.getPluginName() + '~' + name);
+ dsc, PluginName.GERRIT.equals(e.getPluginName()) ? name : e.getPluginName() + '~' + name);
return dsc;
}
}
diff --git a/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java b/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
index 1c87a63..513d909 100644
--- a/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
+++ b/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
@@ -14,12 +14,16 @@
package com.google.gerrit.server.git;
-import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.inject.Inject;
/** Print a change description for use in git command-line progress. */
public class DefaultChangeReportFormatter implements ChangeReportFormatter {
+ private static final int SUBJECT_MAX_LENGTH = 80;
+ private static final String SUBJECT_CROP_APPENDIX = "...";
+ private static final int SUBJECT_CROP_RANGE = 10;
+
private final String canonicalWebUrl;
@Inject
@@ -37,19 +41,37 @@
return formatChangeUrl(canonicalWebUrl, input);
}
- @Override
- public String changeClosed(ChangeReportFormatter.Input input) {
- return String.format(
- "change %s closed", ChangeUtil.formatChangeUrl(canonicalWebUrl, input.change()));
+ public static String formatChangeUrl(String canonicalWebUrl, Change change) {
+ return canonicalWebUrl + "c/" + change.getProject().get() + "/+/" + change.getChangeId();
}
- private String formatChangeUrl(String url, Input input) {
+ @Override
+ public String changeClosed(ChangeReportFormatter.Input input) {
+ return String.format("change %s closed", formatChangeUrl(canonicalWebUrl, input.change()));
+ }
+
+ protected String cropSubject(String subject) {
+ if (subject.length() > SUBJECT_MAX_LENGTH) {
+ int maxLength = SUBJECT_MAX_LENGTH - SUBJECT_CROP_APPENDIX.length();
+ for (int cropPosition = maxLength;
+ cropPosition > maxLength - SUBJECT_CROP_RANGE;
+ cropPosition--) {
+ if (Character.isWhitespace(subject.charAt(cropPosition - 1))) {
+ return subject.substring(0, cropPosition) + SUBJECT_CROP_APPENDIX;
+ }
+ }
+ return subject.substring(0, maxLength) + SUBJECT_CROP_APPENDIX;
+ }
+ return subject;
+ }
+
+ protected String formatChangeUrl(String url, Input input) {
StringBuilder m =
new StringBuilder()
.append(" ")
- .append(ChangeUtil.formatChangeUrl(url, input.change()))
+ .append(formatChangeUrl(url, input.change()))
.append(" ")
- .append(ChangeUtil.cropSubject(input.subject()));
+ .append(cropSubject(input.subject()));
if (input.isEdit()) {
m.append(" [EDIT]");
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 8af1592..6107ecd 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -59,6 +59,7 @@
import com.google.common.collect.Sets;
import com.google.common.collect.SortedSetMultimap;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.LabelType;
@@ -650,6 +651,10 @@
if (magicBranch != null && magicBranch.cmd.getResult() == NOT_ATTEMPTED) {
newChanges = selectNewAndReplacedChangesFromMagicBranch(newProgress);
}
+
+ // Commit validation has already happened, so any changes without Change-Id are for the
+ // deprecated feature.
+ warnAboutMissingChangeId(newChanges);
preparePatchSetsForReplace(newChanges);
insertChangesAndPatchSets(newChanges, replaceProgress);
newProgress.end();
@@ -971,6 +976,7 @@
// The referenced change must exist and must still be open.
Change.Id changeId = Change.Id.parse(m.group(1));
parseReplaceCommand(cmd, changeId);
+ messages.add(new ValidationMessage("warning: pushes to refs/changes are deprecated", false));
} else {
reject(cmd, "upload to refs/changes not allowed");
}
@@ -1938,6 +1944,23 @@
return true;
}
+ private void warnAboutMissingChangeId(List<CreateRequest> newChanges) {
+ for (CreateRequest create : newChanges) {
+ try {
+ receivePack.getRevWalk().parseBody(create.commit);
+ } catch (IOException e) {
+ continue;
+ }
+ List<String> idList = create.commit.getFooterLines(FooterConstants.CHANGE_ID);
+
+ if (idList.isEmpty()) {
+ messages.add(
+ new ValidationMessage("warning: pushing without Change-Id is deprecated", false));
+ break;
+ }
+ }
+ }
+
private List<CreateRequest> selectNewAndReplacedChangesFromMagicBranch(Task newProgress) {
logger.atFine().log("Finding new and replaced changes");
List<CreateRequest> newChanges = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 84ea8d3..3e382fb 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -566,8 +566,7 @@
return Collections.emptyList();
} catch (AuthException e) {
throw new CommitValidationException(
- "invalid author",
- invalidEmail(receiveEvent.commit, "author", author, user, canonicalWebUrl));
+ "invalid author", invalidEmail("author", author, user, canonicalWebUrl));
} catch (PermissionBackendException e) {
logger.atSevere().withCause(e).log("cannot check FORGE_AUTHOR");
throw new CommitValidationException("internal auth error");
@@ -600,8 +599,7 @@
return Collections.emptyList();
} catch (AuthException e) {
throw new CommitValidationException(
- "invalid committer",
- invalidEmail(receiveEvent.commit, "committer", committer, user, canonicalWebUrl));
+ "invalid committer", invalidEmail("committer", committer, user, canonicalWebUrl));
} catch (PermissionBackendException e) {
logger.atSevere().withCause(e).log("cannot check FORGE_COMMITTER");
throw new CommitValidationException("internal auth error");
@@ -821,42 +819,30 @@
}
private static CommitValidationMessage invalidEmail(
- RevCommit c,
- String type,
- PersonIdent who,
- IdentifiedUser currentUser,
- String canonicalWebUrl) {
+ String type, PersonIdent who, IdentifiedUser currentUser, String canonicalWebUrl) {
StringBuilder sb = new StringBuilder();
- sb.append("\n");
- sb.append("ERROR: In commit ").append(c.name()).append("\n");
- sb.append("ERROR: ")
- .append(type)
- .append(" email address ")
+
+ sb.append("email address ")
.append(who.getEmailAddress())
- .append("\n");
- sb.append("ERROR: does not match your user account and you have no 'forge ")
+ .append(" is not registered in your account, and you lack 'forge ")
.append(type)
.append("' permission.\n");
- sb.append("ERROR:\n");
+
if (currentUser.getEmailAddresses().isEmpty()) {
- sb.append("ERROR: You have not registered any email addresses.\n");
+ sb.append("You have not registered any email addresses.\n");
} else {
- sb.append("ERROR: The following addresses are currently registered:\n");
+ sb.append("The following addresses are currently registered:\n");
for (String address : currentUser.getEmailAddresses()) {
- sb.append("ERROR: ").append(address).append("\n");
+ sb.append(" ").append(address).append("\n");
}
}
- sb.append("ERROR:\n");
+
if (canonicalWebUrl != null) {
- sb.append("ERROR: To register an email address, please visit:\n");
- sb.append("ERROR: ")
- .append(canonicalWebUrl)
- .append("#")
- .append(PageLinks.SETTINGS_CONTACT)
- .append("\n");
+ sb.append("To register an email address, visit:\n");
+ sb.append(canonicalWebUrl).append("#").append(PageLinks.SETTINGS_CONTACT).append("\n");
}
sb.append("\n");
- return new CommitValidationMessage(sb.toString(), false);
+ return new CommitValidationMessage(sb.toString(), true);
}
/**
diff --git a/java/com/google/gerrit/server/permissions/GlobalPermission.java b/java/com/google/gerrit/server/permissions/GlobalPermission.java
index 71718fb..01ef725 100644
--- a/java/com/google/gerrit/server/permissions/GlobalPermission.java
+++ b/java/com/google/gerrit/server/permissions/GlobalPermission.java
@@ -24,6 +24,7 @@
import com.google.gerrit.extensions.api.access.GerritPermission;
import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
import com.google.gerrit.extensions.api.access.PluginPermission;
+import com.google.gerrit.extensions.registration.PluginName;
import java.lang.annotation.Annotation;
import java.util.Collections;
import java.util.LinkedHashSet;
@@ -116,7 +117,7 @@
Class<?> annotationClass)
throws PermissionBackendException {
if (pluginName != null
- && !"gerrit".equals(pluginName)
+ && !PluginName.GERRIT.equals(pluginName)
&& (scope == CapabilityScope.PLUGIN || scope == CapabilityScope.CONTEXT)) {
return new PluginPermission(pluginName, capability, fallBackToAdmin);
}
diff --git a/java/com/google/gerrit/server/plugins/JarPluginProvider.java b/java/com/google/gerrit/server/plugins/JarPluginProvider.java
index 229f394..5b80059 100644
--- a/java/com/google/gerrit/server/plugins/JarPluginProvider.java
+++ b/java/com/google/gerrit/server/plugins/JarPluginProvider.java
@@ -16,6 +16,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.extensions.registration.PluginName;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.config.SitePaths;
@@ -90,7 +91,7 @@
@Override
public String getProviderPluginName() {
- return "gerrit";
+ return PluginName.GERRIT;
}
private static String getExtension(Path path) {
diff --git a/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java b/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java
index 300bec7..8bc04a3 100644
--- a/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java
+++ b/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java
@@ -281,7 +281,8 @@
private void attachSet(
Map<TypeLiteral<?>, DynamicSet<?>> sets, @Nullable Injector src, Plugin plugin) {
- for (RegistrationHandle h : PrivateInternals_DynamicTypes.attachSets(src, sets)) {
+ for (RegistrationHandle h :
+ PrivateInternals_DynamicTypes.attachSets(src, plugin.getName(), sets)) {
plugin.add(h);
}
}
@@ -434,7 +435,7 @@
oi.remove();
replace(newPlugin, h2, b);
} else {
- newPlugin.add(set.add(b.getKey(), b.getProvider()));
+ newPlugin.add(set.add(newPlugin.getName(), b.getKey(), b.getProvider()));
}
}
}
diff --git a/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java b/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java
index 0bef1e5..4d89482 100644
--- a/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java
+++ b/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java
@@ -16,6 +16,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.PluginName;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.nio.file.Path;
@@ -60,7 +61,7 @@
@Override
public String getProviderPluginName() {
- return "gerrit";
+ return PluginName.GERRIT;
}
private ServerPluginProvider providerOf(Path srcPath) {
diff --git a/java/com/google/gerrit/server/restapi/config/CachesCollection.java b/java/com/google/gerrit/server/restapi/config/CachesCollection.java
index 152fef9..a4b8802 100644
--- a/java/com/google/gerrit/server/restapi/config/CachesCollection.java
+++ b/java/com/google/gerrit/server/restapi/config/CachesCollection.java
@@ -20,6 +20,7 @@
import com.google.common.cache.Cache;
import com.google.gerrit.extensions.annotations.RequiresAnyCapability;
import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.registration.PluginName;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
@@ -66,7 +67,7 @@
permissionBackend.currentUser().check(GlobalPermission.VIEW_CACHES);
String cacheName = id.get();
- String pluginName = "gerrit";
+ String pluginName = PluginName.GERRIT;
int i = cacheName.lastIndexOf('-');
if (i != -1) {
pluginName = cacheName.substring(0, i);
diff --git a/java/com/google/gerrit/server/restapi/config/PostCaches.java b/java/com/google/gerrit/server/restapi/config/PostCaches.java
index 7f9b756..57ba097 100644
--- a/java/com/google/gerrit/server/restapi/config/PostCaches.java
+++ b/java/com/google/gerrit/server/restapi/config/PostCaches.java
@@ -20,6 +20,7 @@
import com.google.common.cache.Cache;
import com.google.gerrit.extensions.annotations.RequiresAnyCapability;
import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.registration.PluginName;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
@@ -110,7 +111,7 @@
List<CacheResource> cacheResources = new ArrayList<>(cacheNames.size());
for (String n : cacheNames) {
- String pluginName = "gerrit";
+ String pluginName = PluginName.GERRIT;
String cacheName = n;
int i = cacheName.lastIndexOf('-');
if (i != -1) {
diff --git a/java/com/google/gerrit/server/schema/AllProjectsCreator.java b/java/com/google/gerrit/server/schema/AllProjectsCreator.java
index d96e9b0..348f88c 100644
--- a/java/com/google/gerrit/server/schema/AllProjectsCreator.java
+++ b/java/com/google/gerrit/server/schema/AllProjectsCreator.java
@@ -81,6 +81,7 @@
@Nullable private GroupReference batch;
private String message;
private int firstChangeId = ReviewDb.FIRST_CHANGE_ID;
+ private LabelType codeReviewLabel;
private List<LabelType> additionalLabelType;
@Inject
@@ -98,6 +99,7 @@
this.anonymous = systemGroupBackend.getGroup(ANONYMOUS_USERS);
this.registered = systemGroupBackend.getGroup(REGISTERED_USERS);
this.owners = systemGroupBackend.getGroup(PROJECT_OWNERS);
+ this.codeReviewLabel = getDefaultCodeReviewLabel();
this.additionalLabelType = new ArrayList<>();
}
@@ -125,6 +127,15 @@
return this;
}
+ /** If called, the provided "Code-Review" label will be used rather than the default. */
+ @UsedAt(UsedAt.Project.GOOGLE)
+ public AllProjectsCreator setCodeReviewLabel(LabelType labelType) {
+ checkArgument(
+ labelType.getName().equals("Code-Review"), "label should have 'Code-Review' as its name");
+ this.codeReviewLabel = labelType;
+ return this;
+ }
+
@UsedAt(UsedAt.Project.GOOGLE)
public AllProjectsCreator addAdditionalLabel(LabelType labelType) {
additionalLabelType.add(labelType);
@@ -190,8 +201,7 @@
stream.add(rule(config, batch));
}
- LabelType codeReviewLabel = initCodeReviewLabel(config);
- initAdditionalLabels(config);
+ initLabels(config);
grant(config, heads, codeReviewLabel, -1, 1, registered);
grant(config, heads, codeReviewLabel, -2, 2, admin, owners);
@@ -222,12 +232,6 @@
}
}
- public static LabelType initCodeReviewLabel(ProjectConfig c) {
- LabelType type = getDefaultCodeReviewLabel();
- c.getLabelSections().put(type.getName(), type);
- return type;
- }
-
@UsedAt(UsedAt.Project.GOOGLE)
public static LabelType getDefaultCodeReviewLabel() {
LabelType type =
@@ -244,10 +248,8 @@
return type;
}
- private void initAdditionalLabels(ProjectConfig projectConfig) {
- if (additionalLabelType.isEmpty()) {
- return;
- }
+ private void initLabels(ProjectConfig projectConfig) {
+ projectConfig.getLabelSections().put(codeReviewLabel.getName(), codeReviewLabel);
additionalLabelType.forEach(t -> projectConfig.getLabelSections().put(t.getName(), t));
}
diff --git a/java/com/google/gerrit/server/schema/AllUsersCreator.java b/java/com/google/gerrit/server/schema/AllUsersCreator.java
index 90002f6..3779d0d 100644
--- a/java/com/google/gerrit/server/schema/AllUsersCreator.java
+++ b/java/com/google/gerrit/server/schema/AllUsersCreator.java
@@ -14,8 +14,10 @@
package com.google.gerrit.server.schema;
+import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.schema.AclUtil.grant;
+import static com.google.gerrit.server.schema.AllProjectsCreator.getDefaultCodeReviewLabel;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.Version;
@@ -26,6 +28,7 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.UsedAt;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -50,6 +53,7 @@
private final GroupReference registered;
@Nullable private GroupReference admin;
+ private LabelType codeReviewLabel;
@Inject
AllUsersCreator(
@@ -61,6 +65,7 @@
this.allUsersName = allUsersName;
this.serverUser = serverUser;
this.registered = systemGroupBackend.getGroup(REGISTERED_USERS);
+ this.codeReviewLabel = getDefaultCodeReviewLabel();
}
/**
@@ -72,6 +77,15 @@
return this;
}
+ /** If called, the provided "Code-Review" label will be used rather than the default. */
+ @UsedAt(UsedAt.Project.GOOGLE)
+ public AllUsersCreator setCodeReviewLabel(LabelType labelType) {
+ checkArgument(
+ labelType.getName().equals("Code-Review"), "label should have 'Code-Review' as its name");
+ this.codeReviewLabel = labelType;
+ return this;
+ }
+
public void create() throws IOException, ConfigInvalidException {
try (Repository git = mgr.openRepository(allUsersName)) {
initAllUsers(git);
@@ -100,11 +114,14 @@
AccessSection users =
config.getAccessSection(
RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}", true);
- LabelType cr = AllProjectsCreator.initCodeReviewLabel(config);
+
+ // Initialize "Code-Review" label.
+ config.getLabelSections().put(codeReviewLabel.getName(), codeReviewLabel);
+
grant(config, users, Permission.READ, false, true, registered);
grant(config, users, Permission.PUSH, false, true, registered);
grant(config, users, Permission.SUBMIT, false, true, registered);
- grant(config, users, cr, -2, 2, true, registered);
+ grant(config, users, codeReviewLabel, -2, 2, true, registered);
if (admin != null) {
AccessSection defaults = config.getAccessSection(RefNames.REFS_USERS_DEFAULT, true);
diff --git a/java/com/google/gerrit/sshd/commands/StreamEvents.java b/java/com/google/gerrit/sshd/commands/StreamEvents.java
index c97372c..ffd98d5 100644
--- a/java/com/google/gerrit/sshd/commands/StreamEvents.java
+++ b/java/com/google/gerrit/sshd/commands/StreamEvents.java
@@ -151,6 +151,7 @@
stdout = toPrintWriter(out);
eventListenerRegistration =
eventListeners.add(
+ "gerrit",
new UserScopedEventListener() {
@Override
public void onEvent(Event event) {
diff --git a/java/com/google/gerrit/util/cli/BUILD b/java/com/google/gerrit/util/cli/BUILD
index b3ee09d..c94fc1d 100644
--- a/java/com/google/gerrit/util/cli/BUILD
+++ b/java/com/google/gerrit/util/cli/BUILD
@@ -7,8 +7,8 @@
"//java/com/google/gerrit/common:server",
"//lib:args4j",
"//lib:guava",
+ "//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-assistedinject",
- "@flogger//jar",
],
)
diff --git a/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java b/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java
index 3c7b966..bf387fd 100644
--- a/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java
+++ b/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java
@@ -39,7 +39,7 @@
@Test
public void universalGroupBackendHandlesTestGroup() throws Exception {
- RegistrationHandle registrationHandle = groupBackends.add(testGroupBackend);
+ RegistrationHandle registrationHandle = groupBackends.add("gerrit", testGroupBackend);
try {
assertThat(universalGroupBackend.handles(testUUID)).isTrue();
} finally {
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 05642c9..b362b2b 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -230,7 +230,7 @@
@Before
public void addAccountIndexEventCounter() {
accountIndexedCounter = new AccountIndexedCounter();
- accountIndexEventCounterHandle = accountIndexedListeners.add(accountIndexedCounter);
+ accountIndexEventCounterHandle = accountIndexedListeners.add("gerrit", accountIndexedCounter);
}
@After
@@ -243,7 +243,7 @@
@Before
public void addRefUpdateCounter() {
refUpdateCounter = new RefUpdateCounter();
- refUpdateCounterHandle = refUpdateListeners.add(refUpdateCounter);
+ refUpdateCounterHandle = refUpdateListeners.add("gerrit", refUpdateCounter);
}
@After
@@ -532,6 +532,7 @@
accountOperations.newAccount().preferredEmail("foo@deactivatable.com").create();
RegistrationHandle registrationHandle =
accountActivationValidationListeners.add(
+ "gerrit",
new AccountActivationValidationListener() {
@Override
public void validateActivation(AccountState account) throws ValidationException {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 6fd9545..fe55115 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -203,7 +203,7 @@
@Before
public void addChangeIndexedCounter() {
changeIndexedCounter = new ChangeIndexedCounter();
- changeIndexedCounterHandle = changeIndexedListeners.add(changeIndexedCounter);
+ changeIndexedCounterHandle = changeIndexedListeners.add("gerrit", changeIndexedCounter);
}
@After
@@ -2599,6 +2599,7 @@
PushOneCommit.Result change = createChange();
RegistrationHandle handle =
changeMessageModifiers.add(
+ "gerrit",
new ChangeMessageModifier() {
@Override
public String onSubmit(
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index 4e3f048..8f1bdbc 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -1298,7 +1298,7 @@
GroupIndexedCounter groupIndexedCounter = new GroupIndexedCounter();
RegistrationHandle groupIndexEventCounterHandle =
- groupIndexedListeners.add(groupIndexedCounter);
+ groupIndexedListeners.add("gerrit", groupIndexedCounter);
try {
// Running the reindexer right after startup should not need to reindex any group since
// reindexing was already done on startup.
@@ -1355,7 +1355,7 @@
GroupIndexedCounter groupIndexedCounter = new GroupIndexedCounter();
RegistrationHandle groupIndexEventCounterHandle =
- groupIndexedListeners.add(groupIndexedCounter);
+ groupIndexedListeners.add("gerrit", groupIndexedCounter);
try {
// No group indexing happened on startup. All groups should be reindexed now.
slaveGroupIndexer.run();
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index 5be8dfd..f7d3867 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -68,7 +68,7 @@
@Before
public void addProjectIndexedCounter() {
projectIndexedCounter = new ProjectIndexedCounter();
- projectIndexedCounterHandle = projectIndexedListeners.add(projectIndexedCounter);
+ projectIndexedCounterHandle = projectIndexedListeners.add("gerrit", projectIndexedCounter);
}
@After
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 8a3d0f3..ca4304e 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -920,6 +920,7 @@
CountDownLatch reindexed = new CountDownLatch(1);
RegistrationHandle handle =
changeIndexedListeners.add(
+ "gerrit",
new ChangeIndexedListener() {
@Override
public void onChangeIndexed(String projectName, int id) {
@@ -1086,6 +1087,7 @@
WebLinkInfo expectedWebLinkInfo = new WebLinkInfo("foo", "imageUrl", "url");
RegistrationHandle handle =
patchSetLinks.add(
+ "gerrit",
new PatchSetWebLink() {
@Override
public WebLinkInfo getPatchSetWebLink(String projectName, String commit) {
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index eab76c3..7ad34a6 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -103,6 +103,7 @@
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository;
@@ -340,6 +341,20 @@
}
@Test
+ public void pushWithoutChangeIdDeprecated() throws Exception {
+ setRequireChangeId(InheritableBoolean.FALSE);
+ testRepo
+ .branch("HEAD")
+ .commit()
+ .message("A change")
+ .author(admin.getIdent())
+ .committer(new PersonIdent(admin.getIdent(), testRepo.getDate()))
+ .create();
+ PushResult result = pushHead(testRepo, "refs/for/master");
+ assertThat(result.getMessages()).contains("warning: pushing without Change-Id is deprecated");
+ }
+
+ @Test
public void autocloseByChangeId() throws Exception {
// Create a change
PushOneCommit.Result r = pushTo("refs/for/master");
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index 25a6763..b48d47e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -53,9 +53,10 @@
public void setup() {
projectCreationListener = new TraceValidatingProjectCreationValidationListener();
projectCreationListenerRegistrationHandle =
- projectCreationValidationListeners.add(projectCreationListener);
+ projectCreationValidationListeners.add("gerrit", projectCreationListener);
commitValidationListener = new TraceValidatingCommitValidationListener();
- commitValidationRegistrationHandle = commitValidationListeners.add(commitValidationListener);
+ commitValidationRegistrationHandle =
+ commitValidationListeners.add("gerrit", commitValidationListener);
}
@After
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index 5580279..af11149 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -1324,7 +1324,7 @@
protected void addOnSubmitValidationListener(OnSubmitValidationListener listener) {
assertThat(onSubmitValidatorHandle).isNull();
- onSubmitValidatorHandle = onSubmitValidationListeners.add(listener);
+ onSubmitValidatorHandle = onSubmitValidationListeners.add("gerrit", listener);
}
private String getLatestDiff(Repository repo) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index 171babd..f45f9dc 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -331,7 +331,7 @@
assertThat(origActions.get("abandon").label).isEqualTo("Abandon");
Visitor v = new Visitor();
- visitorHandle = actionVisitors.add(v);
+ visitorHandle = actionVisitors.add("gerrit", v);
Map<String, ActionInfo> newActions =
gApi.changes().id(id).get(EnumSet.of(ListChangesOption.CHANGE_ACTIONS)).actions;
@@ -380,7 +380,7 @@
assertThat(origActions.get("rebase").label).isEqualTo("Rebase");
Visitor v = new Visitor();
- visitorHandle = actionVisitors.add(v);
+ visitorHandle = actionVisitors.add("gerrit", v);
// Test different codepaths within ActionJson...
// ...via revision API.
@@ -443,7 +443,7 @@
assertThat(origActions.get("description").label).isEqualTo("Edit Description");
Visitor v = new Visitor();
- visitorHandle = actionVisitors.add(v);
+ visitorHandle = actionVisitors.add("gerrit", v);
// Unlike for the current revision, actions for old revisions are only available via the
// revision API.
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
index 8cd1770..2182b2f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
@@ -97,6 +97,7 @@
PushOneCommit.Result change = createChange();
RegistrationHandle handle =
changeMessageModifiers.add(
+ "gerrit",
new ChangeMessageModifier() {
@Override
public String onSubmit(
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
index e8b8fe8..3d8d06e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
@@ -87,6 +87,7 @@
RegistrationHandle handle =
changeMessageModifiers.add(
+ "gerrit",
new ChangeMessageModifier() {
@Override
public String onSubmit(
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index 3534959..a64305c 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -89,6 +89,7 @@
public void webLink() throws Exception {
RegistrationHandle handle =
fileHistoryWebLinkDynamicSet.add(
+ "gerrit",
new FileHistoryWebLink() {
@Override
public WebLinkInfo getFileHistoryWebLink(
@@ -111,6 +112,7 @@
public void webLinkNoRefsMetaConfig() throws Exception {
RegistrationHandle handle =
fileHistoryWebLinkDynamicSet.add(
+ "gerrit",
new FileHistoryWebLink() {
@Override
public WebLinkInfo getFileHistoryWebLink(
diff --git a/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java b/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
index 14b3858..f4a833f 100644
--- a/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
@@ -76,6 +76,7 @@
eventListenerRegistration =
source.add(
+ "gerrit",
new CommentAddedListener() {
@Override
public void onCommentAdded(Event event) {
diff --git a/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
index a5d78c6..87c5ace 100644
--- a/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
@@ -610,7 +610,7 @@
}
private void addListener(NotesMigrationStateListener listener) {
- addedListeners.add(listeners.add(listener));
+ addedListeners.add(listeners.add("gerrit", listener));
}
private ImmutableSortedSet<String> getObjectFiles(Project.NameKey project) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
index 45b7767..8b0c56b 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
@@ -82,6 +82,7 @@
eventListenerRegistration =
source.add(
+ "gerrit",
new CommentAddedListener() {
@Override
public void onCommentAdded(Event event) {
diff --git a/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java b/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
index 25bb7a6..ed3cdbc 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
@@ -50,7 +50,7 @@
@Before
public void addChangeIndexedCounter() {
changeIndexedCounter = new ChangeIndexedCounter();
- changeIndexedCounterHandle = changeIndexedListeners.add(changeIndexedCounter);
+ changeIndexedCounterHandle = changeIndexedListeners.add("gerrit", changeIndexedCounter);
}
@After
diff --git a/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java b/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
index 6b3c0e0..899b0cf 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
@@ -28,7 +28,7 @@
public void setup() {
projectCreationListener = new TraceValidatingProjectCreationValidationListener();
projectCreationListenerRegistrationHandle =
- projectCreationValidationListeners.add(projectCreationListener);
+ projectCreationValidationListeners.add("gerrit", projectCreationListener);
}
@After
diff --git a/javatests/com/google/gerrit/common/data/LabelTypeTest.java b/javatests/com/google/gerrit/common/data/LabelTypeTest.java
new file mode 100644
index 0000000..6c3befb
--- /dev/null
+++ b/javatests/com/google/gerrit/common/data/LabelTypeTest.java
@@ -0,0 +1,48 @@
+// Copyright (C) 2018 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.common.data;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import org.junit.Test;
+
+public class LabelTypeTest {
+ @Test
+ public void sortLabelValues() {
+ LabelValue v0 = new LabelValue((short) 0, "Zero");
+ LabelValue v1 = new LabelValue((short) 1, "One");
+ LabelValue v2 = new LabelValue((short) 2, "Two");
+ LabelType types = new LabelType("Label", ImmutableList.of(v2, v0, v1));
+ assertThat(types.getValues()).containsExactly(v0, v1, v2).inOrder();
+ }
+
+ @Test
+ public void insertMissingLabelValues() {
+ LabelValue v0 = new LabelValue((short) 0, "Zero");
+ LabelValue v2 = new LabelValue((short) 2, "Two");
+ LabelValue v5 = new LabelValue((short) 5, "Five");
+ LabelType types = new LabelType("Label", ImmutableList.of(v2, v5, v0));
+ assertThat(types.getValues())
+ .containsExactly(
+ v0,
+ new LabelValue((short) 1, ""),
+ v2,
+ new LabelValue((short) 3, ""),
+ new LabelValue((short) 4, ""),
+ v5)
+ .inOrder();
+ }
+}
diff --git a/javatests/com/google/gerrit/extensions/registration/DynamicSetTest.java b/javatests/com/google/gerrit/extensions/registration/DynamicSetTest.java
index 117e474..bbcee2d 100644
--- a/javatests/com/google/gerrit/extensions/registration/DynamicSetTest.java
+++ b/javatests/com/google/gerrit/extensions/registration/DynamicSetTest.java
@@ -15,9 +15,12 @@
package com.google.gerrit.extensions.registration;
import static com.google.common.truth.Truth.assertThat;
+import static java.util.stream.Collectors.toSet;
import com.google.inject.Key;
+import com.google.inject.Provider;
import com.google.inject.util.Providers;
+import java.util.Iterator;
import org.junit.Test;
public class DynamicSetTest {
@@ -40,7 +43,7 @@
@Test
public void containsTrueWithSingleElement() throws Exception {
DynamicSet<Integer> ds = new DynamicSet<>();
- ds.add(2);
+ ds.add("gerrit", 2);
assertThat(ds.contains(2)).isTrue(); // See above comment about ds.contains
}
@@ -48,7 +51,7 @@
@Test
public void containsFalseWithSingleElement() throws Exception {
DynamicSet<Integer> ds = new DynamicSet<>();
- ds.add(2);
+ ds.add("gerrit", 2);
assertThat(ds.contains(3)).isFalse(); // See above comment about ds.contains
}
@@ -56,8 +59,8 @@
@Test
public void containsTrueWithTwoElements() throws Exception {
DynamicSet<Integer> ds = new DynamicSet<>();
- ds.add(2);
- ds.add(4);
+ ds.add("gerrit", 2);
+ ds.add("gerrit", 4);
assertThat(ds.contains(4)).isTrue(); // See above comment about ds.contains
}
@@ -65,8 +68,8 @@
@Test
public void containsFalseWithTwoElements() throws Exception {
DynamicSet<Integer> ds = new DynamicSet<>();
- ds.add(2);
- ds.add(4);
+ ds.add("gerrit", 2);
+ ds.add("gerrit", 4);
assertThat(ds.contains(3)).isFalse(); // See above comment about ds.contains
}
@@ -74,12 +77,12 @@
@Test
public void containsDynamic() throws Exception {
DynamicSet<Integer> ds = new DynamicSet<>();
- ds.add(2);
+ ds.add("gerrit", 2);
Key<Integer> key = Key.get(Integer.class);
- ReloadableRegistrationHandle<Integer> handle = ds.add(key, Providers.of(4));
+ ReloadableRegistrationHandle<Integer> handle = ds.add("gerrit", key, Providers.of(4));
- ds.add(6);
+ ds.add("gerrit", 6);
// At first, 4 is contained.
assertThat(ds.contains(4)).isTrue(); // See above comment about ds.contains
@@ -90,4 +93,49 @@
// And now 4 should no longer be contained.
assertThat(ds.contains(4)).isFalse(); // See above comment about ds.contains
}
+
+ @Test
+ public void plugins() {
+ DynamicSet<Integer> ds = new DynamicSet<>();
+ ds.add("foo", 1);
+ ds.add("bar", 2);
+ ds.add("bar", 3);
+
+ assertThat(ds.plugins()).containsExactly("bar", "foo").inOrder();
+ }
+
+ @Test
+ public void byPlugin() {
+ DynamicSet<Integer> ds = new DynamicSet<>();
+ ds.add("foo", 1);
+ ds.add("bar", 2);
+ ds.add("bar", 3);
+
+ assertThat(ds.byPlugin("foo").stream().map(Provider::get).collect(toSet())).containsExactly(1);
+ assertThat(ds.byPlugin("bar").stream().map(Provider::get).collect(toSet()))
+ .containsExactly(2, 3);
+ }
+
+ @Test
+ public void entryIterator() {
+ DynamicSet<Integer> ds = new DynamicSet<>();
+ ds.add("foo", 1);
+ ds.add("bar", 2);
+ ds.add("bar", 3);
+
+ Iterator<DynamicSet.Entry<Integer>> entryIterator = ds.entryIterator();
+ DynamicSet.Entry<Integer> next = entryIterator.next();
+ assertThat(next.getPluginName()).isEqualTo("foo");
+ assertThat(next.getProvider().get()).isEqualTo(1);
+
+ next = entryIterator.next();
+ assertThat(next.getPluginName()).isEqualTo("bar");
+ assertThat(next.getProvider().get()).isEqualTo(2);
+
+ next = entryIterator.next();
+ assertThat(next.getPluginName()).isEqualTo("bar");
+ assertThat(next.getProvider().get()).isEqualTo(3);
+
+ assertThat(entryIterator.hasNext()).isFalse();
+ }
}
diff --git a/javatests/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java b/javatests/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java
index 086dcc2..1c6559b0 100644
--- a/javatests/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java
+++ b/javatests/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java
@@ -76,7 +76,7 @@
*/
private ReloadableRegistrationHandle<AllRequestFilter> addFilter(AllRequestFilter filter) {
Key<AllRequestFilter> key = Key.get(AllRequestFilter.class);
- return filters.add(key, Providers.of(filter));
+ return filters.add("gerrit", key, Providers.of(filter));
}
@Test
diff --git a/javatests/com/google/gerrit/server/account/UniversalGroupBackendTest.java b/javatests/com/google/gerrit/server/account/UniversalGroupBackendTest.java
index 91cc2b7..6dd0f3e 100644
--- a/javatests/com/google/gerrit/server/account/UniversalGroupBackendTest.java
+++ b/javatests/com/google/gerrit/server/account/UniversalGroupBackendTest.java
@@ -55,7 +55,7 @@
user = createNiceMock(IdentifiedUser.class);
replay(user);
backends = new DynamicSet<>();
- backends.add(new SystemGroupBackend(new Config()));
+ backends.add("gerrit", new SystemGroupBackend(new Config()));
backend = new UniversalGroupBackend(backends);
}
@@ -123,7 +123,7 @@
replay(member, notMember, backend);
backends = new DynamicSet<>();
- backends.add(backend);
+ backends.add("gerrit", backend);
backend = new UniversalGroupBackend(backends);
GroupMembership checker = backend.membershipsOf(member);
diff --git a/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java b/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java
index 7f8b6f3..d3f69982 100644
--- a/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java
+++ b/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java
@@ -15,8 +15,8 @@
package com.google.gerrit.server.schema;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
-import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
@@ -36,6 +36,7 @@
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import org.eclipse.jgit.lib.Repository;
import org.junit.After;
@@ -108,15 +109,22 @@
assertThat(codeReview.getDefaultValue()).isEqualTo(0);
assertThat(codeReview.getFunction()).isEqualTo(LabelFunction.MAX_WITH_BLOCK);
assertThat(codeReview.isCopyMinScore()).isTrue();
- assertValueRange(codeReview, 2, 1, 0, -1, -2);
+ assertValueRange(codeReview, -2, -1, 0, 1, 2);
}
private void assertValueRange(LabelType label, Integer... range) {
- assertThat(label.getValuesAsList()).containsExactlyElementsIn(Arrays.asList(range)).inOrder();
- assertThat(label.getMax().getValue()).isEqualTo(range[0]);
- assertThat(label.getMin().getValue()).isEqualTo(range[range.length - 1]);
+ List<Integer> rangeList = Arrays.asList(range);
+ assertThat(rangeList).isNotEmpty();
+ assertThat(rangeList).isStrictlyOrdered();
+
+ assertThat(label.getValues().stream().map(v -> (int) v.getValue()))
+ .containsExactlyElementsIn(rangeList)
+ .inOrder();
+ assertThat(label.getMax().getValue()).isEqualTo(Collections.max(rangeList));
+ assertThat(label.getMin().getValue()).isEqualTo(Collections.min(rangeList));
for (LabelValue v : label.getValues()) {
- assertThat(Strings.isNullOrEmpty(v.getText())).isFalse();
+ assertThat(v.getText()).isNotNull();
+ assertThat(v.getText()).isNotEmpty();
}
}
}
diff --git a/javatests/com/google/gerrit/server/schema/Schema_166_to_167_WithGroupsInReviewDbTest.java b/javatests/com/google/gerrit/server/schema/Schema_166_to_167_WithGroupsInReviewDbTest.java
index 9256ee4..75f9307 100644
--- a/javatests/com/google/gerrit/server/schema/Schema_166_to_167_WithGroupsInReviewDbTest.java
+++ b/javatests/com/google/gerrit/server/schema/Schema_166_to_167_WithGroupsInReviewDbTest.java
@@ -586,7 +586,7 @@
AccountGroup group = createInReviewDb("group");
TestGroupBackend testGroupBackend = new TestGroupBackend();
- backends.add(testGroupBackend);
+ backends.add("gerrit", testGroupBackend);
AccountGroup.UUID subgroupUuid = testGroupBackend.create("test").getGroupUUID();
assertThat(groupBackend.handles(subgroupUuid)).isTrue();
addSubgroupsInReviewDb(group.getId(), subgroupUuid);
diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl
index ba34750..6ada5bd 100644
--- a/lib/jgit/jgit.bzl
+++ b/lib/jgit/jgit.bzl
@@ -18,9 +18,23 @@
name = "jgit",
path = LOCAL_JGIT_REPO,
)
+ jgit_maven_repos_dev()
else:
jgit_maven_repos()
+def jgit_maven_repos_dev():
+ # Transitive dependencies from JGit's WORKSPACE.
+ maven_jar(
+ name = "hamcrest-library",
+ artifact = "org.hamcrest:hamcrest-library:1.3",
+ sha1 = "4785a3c21320980282f9f33d0d1264a69040538f",
+ )
+ maven_jar(
+ name = "jzlib",
+ artifact = "com.jcraft:jzlib:1.1.1",
+ sha1 = "a1551373315ffc2f96130a0e5704f74e151777ba",
+ )
+
def jgit_maven_repos():
maven_jar(
name = "jgit-lib",
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.html b/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
index 1e19107..349cadc 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
@@ -23,7 +23,6 @@
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-autocomplete/gr-autocomplete.html">
<link rel="import" href="../../shared/gr-copy-clipboard/gr-copy-clipboard.html">
-<link rel="import" href="../../shared/gr-download-commands/gr-download-commands.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../shared/gr-select/gr-select.html">
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 ce0392d..799b831 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
@@ -213,7 +213,7 @@
for (const key in response) {
if (!response.hasOwnProperty(key)) { continue; }
projects.push({
- name: key,
+ name: response[key].name,
value: response[key].id,
});
}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.html b/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.html
index f42652d..7db4e4c 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.html
@@ -27,7 +27,12 @@
}
</style>
<h3>[[title]]</h3>
- <gr-button on-tap="_onCommandTap">[[title]]</gr-button>
+ <gr-button
+ title$="[[tooltip]]"
+ disabled$="[[disabled]]"
+ on-tap="_onCommandTap">
+ [[title]]
+ </gr-button>
</template>
<script src="gr-repo-command.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.js b/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.js
index e49c4de..bcdb7f6 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.js
@@ -22,6 +22,8 @@
properties: {
title: String,
+ disabled: Boolean,
+ tooltip: String,
},
/**
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html
index 935c967..dba01aa 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html
@@ -50,7 +50,8 @@
on-command-tap="_handleEditRepoConfig">
</gr-repo-command>
<gr-repo-command
- title="Run GC"
+ title="[[_repoConfig.actions.gc.label]]"
+ tooltip="[[_repoConfig.actions.gc.title]]"
hidden$="[[!_repoConfig.actions.gc.enabled]]"
on-command-tap="_handleRunningGC">
</gr-repo-command>
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js
index 5560972..5a8b5b1 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js
@@ -120,12 +120,7 @@
.then(repos => {
// Late response.
if (filter !== this._filter || !repos) { return; }
- this._repos = Object.keys(repos)
- .map(key => {
- const repo = repos[key];
- repo.name = key;
- return repo;
- });
+ this._repos = repos;
this._loading = false;
});
},
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html
index 96a5454..b6c8fcc 100644
--- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html
@@ -39,7 +39,6 @@
has-tooltip
button-title="Copy full SHA to clipboard"
hide-input
- hide-label
text="[[commitInfo.commit]]">
</gr-copy-clipboard>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index b2857a5..8e32ea3 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -1351,7 +1351,6 @@
const setupDiff = function(diff) {
const mock = document.createElement('mock-diff-response');
- diff.$.diff._diff = mock.diffResponse;
diff.comments = {
left: diff.path === '/COMMIT_MSG' ? commitMsgComments : [],
right: [],
@@ -1379,7 +1378,7 @@
theme: 'DEFAULT',
ignore_whitespace: 'IGNORE_NONE',
};
- diff.$.diff._renderDiffTable();
+ diff._diff = mock.diffResponse;
};
const renderAndGetNewDiffs = function(index) {
diff --git a/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.html b/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.html
index 1e77d5d..a9843a3 100644
--- a/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.html
@@ -17,6 +17,7 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
+<link rel="import" href="../../shared/gr-shell-command/gr-shell-command.html">
<link rel="import" href="../../../styles/shared-styles.html">
<dom-module id="gr-upload-help-dialog">
@@ -33,33 +34,9 @@
margin-left: 1em;
list-style: decimal;
}
- p,
- .commandContainer {
+ p {
margin-bottom: .75em;
}
- .commandContainer {
- background: #f5f5f5;
- padding: .5em .5em .5em 2.5em;
- position: relative;
- width: 100%;
- }
- .commandContainer:before {
- background: #ebebeb;
- bottom: 0;
- box-sizing: border-box;
- content: '$';
- display: block;
- left: 0;
- padding: .8em;
- position: absolute;
- top: 0;
- width: 2em;
- }
- .commandContainer gr-copy-clipboard {
- --text-container-style: {
- border: none;
- }
- }
</style>
<gr-dialog
confirm-label="Done"
@@ -79,18 +56,14 @@
Update the local commit with your modifications using the following
command.
</p>
- <div class="commandContainer">
- <gr-copy-clipboard text="[[_commitCommand]]"></gr-copy-clipboard>
- </div>
+ <gr-shell-command command="[[_commitCommand]]"></gr-shell-command>
<p>
Leave the "Change-Id:" line of the commit message as is.
</p>
</li>
<li>
<p>Push the updated commit to Gerrit.</p>
- <div class="commandContainer">
- <gr-copy-clipboard text="[[_pushCommand]]"></gr-copy-clipboard>
- </div>
+ <gr-shell-command command="[[_pushCommand]]"></gr-shell-command>
</li>
<li>
<p>Refresh this page to view the the update.</p>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
index cb768ef..88ff79b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
@@ -35,7 +35,6 @@
GrDiffBuilderImage.prototype.constructor = GrDiffBuilderImage;
GrDiffBuilderImage.prototype.renderDiff = function() {
- this._outputEl.classList.add('image-diff');
const section = this._createElement('tbody', 'image-diff');
this._emitImagePair(section);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
index 2a00425..9668a54 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
@@ -35,6 +35,7 @@
<mock-diff-response></mock-diff-response>
<gr-diff></gr-diff>
<gr-diff-cursor></gr-diff-cursor>
+ <gr-rest-api-interface></gr-rest-api-interface>
</template>
</test-fixture>
@@ -48,27 +49,18 @@
setup(done => {
sandbox = sinon.sandbox.create();
- stub('gr-rest-api-interface', {
- getLoggedIn() { return Promise.resolve(false); },
- getDiff() {
- return Promise.resolve(mockDiffResponse.diffResponse);
- },
- });
-
const fixtureElems = fixture('basic');
mockDiffResponse = fixtureElems[0];
diffElement = fixtureElems[1];
cursorElement = fixtureElems[2];
+ const restAPI = fixtureElems[3];
// Register the diff with the cursor.
cursorElement.push('diffs', diffElement);
+ diffElement.loggedIn = false;
diffElement.patchRange = {basePatchNum: 1, patchNum: 2};
diffElement.comments = {left: [], right: []};
- diffElement.$.restAPI.getDiffPreferences().then(prefs => {
- diffElement.prefs = prefs;
- });
-
const setupDone = () => {
cursorElement._updateStops();
cursorElement.moveToFirstChunk();
@@ -77,7 +69,10 @@
};
diffElement.addEventListener('render', setupDone);
- diffElement.reload();
+ restAPI.getDiffPreferences().then(prefs => {
+ diffElement.prefs = prefs;
+ diffElement.diff = mockDiffResponse.diffResponse;
+ });
});
teardown(() => sandbox.restore());
@@ -219,7 +214,7 @@
done();
}
diffElement.addEventListener('render', renderHandler);
- diffElement.reload();
+ diffElement._diffChanged(mockDiffResponse.diffResponse);
});
test('initialLineNumber enabled', done => {
@@ -239,7 +234,7 @@
cursorElement.initialLineNumber = 10;
cursorElement.side = 'right';
- diffElement.reload();
+ diffElement._diffChanged(mockDiffResponse.diffResponse);
});
test('getAddress', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
index ff74fba..e3bf866 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
@@ -16,6 +16,9 @@
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
+<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
+
<link rel="import" href="../gr-diff/gr-diff.html">
<dom-module id="gr-diff-host">
@@ -30,17 +33,23 @@
project-config="[[projectConfig]]"
project-name="[[projectName]]"
display-line="[[displayLine]]"
- is-image-diff="{{isImageDiff}}"
+ is-image-diff="[[isImageDiff]]"
commit-range="[[commitRange]]"
- files-weblinks="{{filesWeblinks}}"
hidden$="[[hidden]]"
no-render-on-prefs-change="[[noRenderOnPrefsChange]]"
comments="[[comments]]"
line-wrapping="[[lineWrapping]]"
view-mode="[[viewMode]]"
line-of-interest="[[lineOfInterest]]"
- show-load-failure="[[showLoadFailure]]"
- is-blame-loaded="{{isBlameLoaded}}"></gr-diff>
+ logged-in="[[_loggedIn]]"
+ loading="[[_loading]]"
+ error-message="[[_errorMessage]]"
+ base-image="[[_baseImage]]"
+ revision-image=[[_revisionImage]]
+ blame="[[_blame]]"
+ diff="[[_diff]]"></gr-diff>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
+ <gr-reporting id="reporting" category="diff"></gr-reporting>
</template>
<script src="gr-diff-host.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 84b920a..3e9e796 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -17,12 +17,33 @@
(function() {
'use strict';
+ const MSG_EMPTY_BLAME = 'No blame information for this diff.';
+
+ const EVENT_AGAINST_PARENT = 'diff-against-parent';
+ const EVENT_ZERO_REBASE = 'rebase-percent-zero';
+ const EVENT_NONZERO_REBASE = 'rebase-percent-nonzero';
+
const DiffViewMode = {
SIDE_BY_SIDE: 'SIDE_BY_SIDE',
UNIFIED: 'UNIFIED_DIFF',
};
/**
+ * @param {Object} diff
+ * @return {boolean}
+ */
+ function isImageDiff(diff) {
+ if (!diff) { return false; }
+
+ const isA = diff.meta_a &&
+ diff.meta_a.content_type.startsWith('image/');
+ const isB = diff.meta_b &&
+ diff.meta_b.content_type.startsWith('image/');
+
+ return !!(diff.binary && (isA || isB));
+ }
+
+ /**
* Wrapper around gr-diff.
*
* Webcomponent fetching diffs and related data from restAPI and passing them
@@ -71,11 +92,13 @@
},
isImageDiff: {
type: Boolean,
+ computed: '_computeIsImageDiff(_diff)',
notify: true,
},
commitRange: Object,
filesWeblinks: {
type: Object,
+ value() { return {}; },
notify: true,
},
hidden: {
@@ -113,12 +136,109 @@
isBlameLoaded: {
type: Boolean,
notify: true,
+ computed: '_computeIsBlameLoaded(_blame)',
},
+
+ _loggedIn: {
+ type: Boolean,
+ value: false,
+ },
+
+ _loading: {
+ type: Boolean,
+ value: false,
+ },
+
+ /** @type {?string} */
+ _errorMessage: {
+ type: String,
+ value: null,
+ },
+
+ /** @type {?Object} */
+ _baseImage: Object,
+ /** @type {?Object} */
+ _revisionImage: Object,
+
+ _diff: Object,
+
+ /** @type {?Object} */
+ _blame: {
+ type: Object,
+ value: null,
+ },
+ },
+
+ listeners: {
+ 'draft-interaction': '_handleDraftInteraction',
+ },
+
+ ready() {
+ if (this._canReload()) {
+ this.reload();
+ }
+ },
+
+ attached() {
+ this._getLoggedIn().then(loggedIn => {
+ this._loggedIn = loggedIn;
+ });
},
/** @return {!Promise} */
reload() {
- return this.$.diff.reload();
+ this._loading = true;
+ this._errorMessage = null;
+
+ const diffRequest = this._getDiff()
+ .then(diff => {
+ this._reportDiff(diff);
+ return diff;
+ })
+ .catch(e => {
+ this._handleGetDiffError(e);
+ return null;
+ });
+
+ const assetRequest = diffRequest.then(diff => {
+ // If the diff is null, then it's failed to load.
+ if (!diff) { return null; }
+
+ return this._loadDiffAssets(diff);
+ });
+
+ return Promise.all([diffRequest, assetRequest])
+ .then(results => {
+ const diff = results[0];
+ if (!diff) {
+ return Promise.resolve();
+ }
+ this.filesWeblinks = this._getFilesWeblinks(diff);
+ return new Promise(resolve => {
+ const callback = () => {
+ resolve();
+ this.removeEventListener('render', callback);
+ };
+ this.addEventListener('render', callback);
+ this._diff = diff;
+ });
+ })
+ .catch(err => {
+ console.warn('Error encountered loading diff:', err);
+ })
+ .then(() => { this._loading = false; });
+ },
+
+ _getFilesWeblinks(diff) {
+ if (!this.commitRange) { return {}; }
+ return {
+ meta_a: Gerrit.Nav.getFileWebLinks(
+ this.projectName, this.commitRange.baseCommit, this.path,
+ {weblinks: diff && diff.meta_a && diff.meta_a.web_links}),
+ meta_b: Gerrit.Nav.getFileWebLinks(
+ this.projectName, this.commitRange.commit, this.path,
+ {weblinks: diff && diff.meta_b && diff.meta_b.web_links}),
+ };
},
/** Cancel any remaining diff builder rendering work. */
@@ -145,12 +265,21 @@
* @return {Promise} A promise that resolves when blame finishes rendering.
*/
loadBlame() {
- return this.$.diff.loadBlame();
+ return this.$.restAPI.getBlame(this.changeNum, this.patchRange.patchNum,
+ this.path, true)
+ .then(blame => {
+ if (!blame.length) {
+ this.fire('show-alert', {message: MSG_EMPTY_BLAME});
+ return Promise.reject(MSG_EMPTY_BLAME);
+ }
+
+ this._blame = blame;
+ });
},
/** Unload blame information for the diff. */
clearBlame() {
- this.$.diff.clearBlame();
+ this._blame = null;
},
/** @return {!Array<!HTMLElement>} */
@@ -170,5 +299,136 @@
expandAllContext() {
this.$.diff.expandAllContext();
},
+
+ /** @return {!Promise} */
+ _getLoggedIn() {
+ return this.$.restAPI.getLoggedIn();
+ },
+
+ /** @return {boolean}} */
+ _canReload() {
+ return !!this.changeNum && !!this.patchRange && !!this.path &&
+ !this.noAutoRender;
+ },
+
+ /** @return {!Promise<!Object>} */
+ _getDiff() {
+ // Wrap the diff request in a new promise so that the error handler
+ // rejects the promise, allowing the error to be handled in the .catch.
+ return new Promise((resolve, reject) => {
+ this.$.restAPI.getDiff(
+ this.changeNum,
+ this.patchRange.basePatchNum,
+ this.patchRange.patchNum,
+ this.path,
+ reject)
+ .then(resolve);
+ });
+ },
+
+ _handleGetDiffError(response) {
+ // Loading the diff may respond with 409 if the file is too large. In this
+ // case, use a toast error..
+ if (response.status === 409) {
+ this.fire('server-error', {response});
+ return;
+ }
+
+ if (this.showLoadFailure) {
+ this._errorMessage = [
+ 'Encountered error when loading the diff:',
+ response.status,
+ response.statusText,
+ ].join(' ');
+ return;
+ }
+
+ this.fire('page-error', {response});
+ },
+
+ /**
+ * Report info about the diff response.
+ */
+ _reportDiff(diff) {
+ if (!diff || !diff.content) { return; }
+
+ // Count the delta lines stemming from normal deltas, and from
+ // due_to_rebase deltas.
+ let nonRebaseDelta = 0;
+ let rebaseDelta = 0;
+ diff.content.forEach(chunk => {
+ if (chunk.ab) { return; }
+ const deltaSize = Math.max(
+ chunk.a ? chunk.a.length : 0, chunk.b ? chunk.b.length : 0);
+ if (chunk.due_to_rebase) {
+ rebaseDelta += deltaSize;
+ } else {
+ nonRebaseDelta += deltaSize;
+ }
+ });
+
+ // Find the percent of the delta from due_to_rebase chunks rounded to two
+ // digits. Diffs with no delta are considered 0%.
+ const totalDelta = rebaseDelta + nonRebaseDelta;
+ const percentRebaseDelta = !totalDelta ? 0 :
+ Math.round(100 * rebaseDelta / totalDelta);
+
+ // Report the due_to_rebase percentage in the "diff" category when
+ // applicable.
+ if (this.patchRange.basePatchNum === 'PARENT') {
+ this.$.reporting.reportInteraction(EVENT_AGAINST_PARENT);
+ } else if (percentRebaseDelta === 0) {
+ this.$.reporting.reportInteraction(EVENT_ZERO_REBASE);
+ } else {
+ this.$.reporting.reportInteraction(EVENT_NONZERO_REBASE,
+ percentRebaseDelta);
+ }
+ },
+
+ /**
+ * @param {Object} diff
+ * @return {!Promise}
+ */
+ _loadDiffAssets(diff) {
+ if (isImageDiff(diff)) {
+ return this._getImages(diff).then(images => {
+ this._baseImage = images.baseImage;
+ this._revisionImage = images.revisionImage;
+ });
+ } else {
+ this._baseImage = null;
+ this._revisionImage = null;
+ return Promise.resolve();
+ }
+ },
+
+ /**
+ * @param {Object} diff
+ * @return {boolean}
+ */
+ _computeIsImageDiff(diff) {
+ return isImageDiff(diff);
+ },
+
+ /**
+ * @param {Object} blame
+ * @return {boolean}
+ */
+ _computeIsBlameLoaded(blame) {
+ return !!blame;
+ },
+
+ /**
+ * @param {Object} diff
+ * @return {!Promise}
+ */
+ _getImages(diff) {
+ return this.$.restAPI.getImagesForDiff(this.changeNum, diff,
+ this.patchRange);
+ },
+
+ _handleDraftInteraction() {
+ this.$.reporting.recordDraftInteraction();
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index 7c2dd7b..a05d44f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -40,23 +40,437 @@
setup(() => {
sandbox = sinon.sandbox.create();
+ element = fixture('basic');
});
teardown(() => {
sandbox.restore();
});
- test('delegates reload()', () => {
- element = fixture('basic');
- const returnValue = Promise.resolve();
- const stub = sandbox.stub(element.$.diff, 'reload').returns(returnValue);
- assert.equal(element.reload(), returnValue);
- assert.isTrue(stub.calledOnce);
- assert.equal(stub.lastCall.args.length, 0);
+ test('reload() cancels before network resolves', () => {
+ const cancelStub = sandbox.stub(element.$.diff, 'cancel');
+
+ // Stub the network calls into requests that never resolve.
+ sandbox.stub(element, '_getDiff', () => new Promise(() => {}));
+
+ element.reload();
+ assert.isTrue(cancelStub.called);
+ });
+
+ suite('not logged in', () => {
+ setup(() => {
+ const getLoggedInPromise = Promise.resolve(false);
+ stub('gr-rest-api-interface', {
+ getLoggedIn() { return getLoggedInPromise; },
+ });
+ element = fixture('basic');
+ return getLoggedInPromise;
+ });
+
+ test('reload() loads files weblinks', () => {
+ const weblinksStub = sandbox.stub(Gerrit.Nav, '_generateWeblinks')
+ .returns({name: 'stubb', url: '#s'});
+ sandbox.stub(element.$.restAPI, 'getDiff').returns(Promise.resolve({
+ content: [],
+ }));
+ element.projectName = 'test-project';
+ element.path = 'test-path';
+ element.commitRange = {baseCommit: 'test-base', commit: 'test-commit'};
+ element.patchRange = {};
+ return element.reload().then(() => {
+ assert.isTrue(weblinksStub.calledTwice);
+ assert.isTrue(weblinksStub.firstCall.calledWith({
+ commit: 'test-base',
+ file: 'test-path',
+ options: {
+ weblinks: undefined,
+ },
+ repo: 'test-project',
+ type: Gerrit.Nav.WeblinkType.FILE}));
+ assert.isTrue(weblinksStub.secondCall.calledWith({
+ commit: 'test-commit',
+ file: 'test-path',
+ options: {
+ weblinks: undefined,
+ },
+ repo: 'test-project',
+ type: Gerrit.Nav.WeblinkType.FILE}));
+ assert.deepEqual(element.filesWeblinks, {
+ meta_a: [{name: 'stubb', url: '#s'}],
+ meta_b: [{name: 'stubb', url: '#s'}],
+ });
+ });
+ });
+
+ test('_getDiff handles null diff responses', done => {
+ stub('gr-rest-api-interface', {
+ getDiff() { return Promise.resolve(null); },
+ });
+ element.changeNum = 123;
+ element.patchRange = {basePatchNum: 1, patchNum: 2};
+ element.path = 'file.txt';
+ element._getDiff().then(done);
+ });
+
+ test('reload resolves on error', () => {
+ const onErrStub = sandbox.stub(element, '_handleGetDiffError');
+ const error = {ok: false, status: 500};
+ sandbox.stub(element.$.restAPI, 'getDiff',
+ (changeNum, basePatchNum, patchNum, path, onErr) => {
+ onErr(error);
+ });
+ return element.reload().then(() => {
+ assert.isTrue(onErrStub.calledOnce);
+ });
+ });
+
+ suite('_handleGetDiffError', () => {
+ let serverErrorStub;
+ let pageErrorStub;
+
+ setup(() => {
+ serverErrorStub = sinon.stub();
+ element.addEventListener('server-error', serverErrorStub);
+ pageErrorStub = sinon.stub();
+ element.addEventListener('page-error', pageErrorStub);
+ });
+
+ test('page error on HTTP-409', () => {
+ element._handleGetDiffError({status: 409});
+ assert.isTrue(serverErrorStub.calledOnce);
+ assert.isFalse(pageErrorStub.called);
+ assert.isNotOk(element._errorMessage);
+ });
+
+ test('server error on non-HTTP-409', () => {
+ element._handleGetDiffError({status: 500});
+ assert.isFalse(serverErrorStub.called);
+ assert.isTrue(pageErrorStub.calledOnce);
+ assert.isNotOk(element._errorMessage);
+ });
+
+ test('error message if showLoadFailure', () => {
+ element.showLoadFailure = true;
+ element._handleGetDiffError({status: 500, statusText: 'Failure!'});
+ assert.isFalse(serverErrorStub.called);
+ assert.isFalse(pageErrorStub.called);
+ assert.equal(element._errorMessage,
+ 'Encountered error when loading the diff: 500 Failure!');
+ });
+ });
+
+ suite('image diffs', () => {
+ let mockFile1;
+ let mockFile2;
+ setup(() => {
+ mockFile1 = {
+ body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAE' +
+ 'wsAAAAAAAAAAAAAAAAA/w==',
+ type: 'image/bmp',
+ };
+ mockFile2 = {
+ body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAE' +
+ 'wsAAAAAAAAAAAAA/////w==',
+ type: 'image/bmp',
+ };
+ sandbox.stub(element.$.restAPI,
+ 'getB64FileContents',
+ (changeId, patchNum, path, opt_parentIndex) => {
+ return Promise.resolve(opt_parentIndex === 1 ? mockFile1 :
+ mockFile2);
+ });
+
+ element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
+ element.comments = {left: [], right: []};
+ });
+
+ test('renders image diffs with same file name', done => {
+ const mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
+ meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index 2adc47d..f9c2f2c 100644',
+ '--- a/carrot.jpg',
+ '+++ b/carrot.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ sandbox.stub(element.$.restAPI, 'getDiff')
+ .returns(Promise.resolve(mockDiff));
+
+ const rendered = () => {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
+
+ // Left image rendered with the parent commit's version of the file.
+ const leftImage =
+ element.$.diff.$.diffTable.querySelector('td.left img');
+ const leftLabel =
+ element.$.diff.$.diffTable.querySelector('td.left label');
+ const leftLabelContent = leftLabel.querySelector('.label');
+ const leftLabelName = leftLabel.querySelector('.name');
+
+ const rightImage =
+ element.$.diff.$.diffTable.querySelector('td.right img');
+ const rightLabel = element.$.diff.$.diffTable.querySelector(
+ 'td.right label');
+ const rightLabelContent = rightLabel.querySelector('.label');
+ const rightLabelName = rightLabel.querySelector('.name');
+
+ assert.isNotOk(rightLabelName);
+ assert.isNotOk(leftLabelName);
+
+ let leftLoaded = false;
+ let rightLoaded = false;
+
+ leftImage.addEventListener('load', () => {
+ assert.isOk(leftImage);
+ assert.equal(leftImage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile1.body);
+ assert.equal(leftLabelContent.textContent, '1×1 image/bmp');
+ leftLoaded = true;
+ if (rightLoaded) {
+ element.removeEventListener('render', rendered);
+ done();
+ }
+ });
+
+ rightImage.addEventListener('load', () => {
+ assert.isOk(rightImage);
+ assert.equal(rightImage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile2.body);
+ assert.equal(rightLabelContent.textContent, '1×1 image/bmp');
+
+ rightLoaded = true;
+ if (leftLoaded) {
+ element.removeEventListener('render', rendered);
+ done();
+ }
+ });
+ };
+
+ element.addEventListener('render', rendered);
+
+ element.$.restAPI.getDiffPreferences().then(prefs => {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+
+ test('renders image diffs with a different file name', done => {
+ const mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
+ meta_b: {name: 'carrot2.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot2.jpg',
+ 'index 2adc47d..f9c2f2c 100644',
+ '--- a/carrot.jpg',
+ '+++ b/carrot2.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ sandbox.stub(element.$.restAPI, 'getDiff')
+ .returns(Promise.resolve(mockDiff));
+
+ const rendered = () => {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
+
+ // Left image rendered with the parent commit's version of the file.
+ const leftImage =
+ element.$.diff.$.diffTable.querySelector('td.left img');
+ const leftLabel =
+ element.$.diff.$.diffTable.querySelector('td.left label');
+ const leftLabelContent = leftLabel.querySelector('.label');
+ const leftLabelName = leftLabel.querySelector('.name');
+
+ const rightImage =
+ element.$.diff.$.diffTable.querySelector('td.right img');
+ const rightLabel = element.$.diff.$.diffTable.querySelector(
+ 'td.right label');
+ const rightLabelContent = rightLabel.querySelector('.label');
+ const rightLabelName = rightLabel.querySelector('.name');
+
+ assert.isOk(rightLabelName);
+ assert.isOk(leftLabelName);
+ assert.equal(leftLabelName.textContent, mockDiff.meta_a.name);
+ assert.equal(rightLabelName.textContent, mockDiff.meta_b.name);
+
+ let leftLoaded = false;
+ let rightLoaded = false;
+
+ leftImage.addEventListener('load', () => {
+ assert.isOk(leftImage);
+ assert.equal(leftImage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile1.body);
+ assert.equal(leftLabelContent.textContent, '1×1 image/bmp');
+ leftLoaded = true;
+ if (rightLoaded) {
+ element.removeEventListener('render', rendered);
+ done();
+ }
+ });
+
+ rightImage.addEventListener('load', () => {
+ assert.isOk(rightImage);
+ assert.equal(rightImage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile2.body);
+ assert.equal(rightLabelContent.textContent, '1×1 image/bmp');
+
+ rightLoaded = true;
+ if (leftLoaded) {
+ element.removeEventListener('render', rendered);
+ done();
+ }
+ });
+ };
+
+ element.addEventListener('render', rendered);
+
+ element.$.restAPI.getDiffPreferences().then(prefs => {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+
+ test('renders added image', done => {
+ const mockDiff = {
+ meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'ADDED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index 0000000..f9c2f2c 100644',
+ '--- /dev/null',
+ '+++ b/carrot.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ sandbox.stub(element.$.restAPI, 'getDiff')
+ .returns(Promise.resolve(mockDiff));
+
+ element.addEventListener('render', () => {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
+
+ const leftImage =
+ element.$.diff.$.diffTable.querySelector('td.left img');
+ const rightImage =
+ element.$.diff.$.diffTable.querySelector('td.right img');
+
+ assert.isNotOk(leftImage);
+ assert.isOk(rightImage);
+ done();
+ });
+
+ element.$.restAPI.getDiffPreferences().then(prefs => {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+
+ test('renders removed image', done => {
+ const mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'DELETED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index f9c2f2c..0000000 100644',
+ '--- a/carrot.jpg',
+ '+++ /dev/null',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ sandbox.stub(element.$.restAPI, 'getDiff')
+ .returns(Promise.resolve(mockDiff));
+
+ element.addEventListener('render', () => {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
+
+ const leftImage =
+ element.$.diff.$.diffTable.querySelector('td.left img');
+ const rightImage =
+ element.$.diff.$.diffTable.querySelector('td.right img');
+
+ assert.isOk(leftImage);
+ assert.isNotOk(rightImage);
+ done();
+ });
+
+ element.$.restAPI.getDiffPreferences().then(prefs => {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+
+ test('does not render disallowed image type', done => {
+ const mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg-evil',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'DELETED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index f9c2f2c..0000000 100644',
+ '--- a/carrot.jpg',
+ '+++ /dev/null',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ mockFile1.type = 'image/jpeg-evil';
+
+ sandbox.stub(element.$.restAPI, 'getDiff')
+ .returns(Promise.resolve(mockDiff));
+
+ element.addEventListener('render', () => {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
+ const leftImage =
+ element.$.diff.$.diffTable.querySelector('td.left img');
+ assert.isNotOk(leftImage);
+ done();
+ });
+
+ element.$.restAPI.getDiffPreferences().then(prefs => {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+ });
});
test('delegates cancel()', () => {
- element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'cancel');
element.reload();
assert.isTrue(stub.calledOnce);
@@ -64,7 +478,6 @@
});
test('delegates getCursorStops()', () => {
- element = fixture('basic');
const returnValue = [document.createElement('b')];
const stub = sandbox.stub(element.$.diff, 'getCursorStops')
.returns(returnValue);
@@ -74,7 +487,6 @@
});
test('delegates isRangeSelected()', () => {
- element = fixture('basic');
const returnValue = true;
const stub = sandbox.stub(element.$.diff, 'isRangeSelected')
.returns(returnValue);
@@ -84,33 +496,66 @@
});
test('delegates toggleLeftDiff()', () => {
- element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'toggleLeftDiff');
element.toggleLeftDiff();
assert.isTrue(stub.calledOnce);
assert.equal(stub.lastCall.args.length, 0);
});
- test('delegates loadBlame()', () => {
- element = fixture('basic');
- const returnValue = Promise.resolve();
- const stub = sandbox.stub(element.$.diff, 'loadBlame')
- .returns(returnValue);
- assert.equal(element.loadBlame(), returnValue);
- assert.isTrue(stub.calledOnce);
- assert.equal(stub.lastCall.args.length, 0);
- });
+ suite('blame', () => {
+ setup(() => {
+ element = fixture('basic');
+ });
- test('delegates clearBlame()', () => {
- element = fixture('basic');
- const stub = sandbox.stub(element.$.diff, 'clearBlame');
- element.clearBlame();
- assert.isTrue(stub.calledOnce);
- assert.equal(stub.lastCall.args.length, 0);
+ test('clearBlame', () => {
+ element._blame = [];
+ const setBlameSpy = sandbox.spy(element.$.diff.$.diffBuilder, 'setBlame');
+ element.clearBlame();
+ assert.isNull(element._blame);
+ assert.isTrue(setBlameSpy.calledWithExactly(null));
+ assert.equal(element.isBlameLoaded, false);
+ });
+
+ test('loadBlame', () => {
+ const mockBlame = [{id: 'commit id', ranges: [{start: 1, end: 2}]}];
+ const showAlertStub = sinon.stub();
+ element.addEventListener('show-alert', showAlertStub);
+ const getBlameStub = sandbox.stub(element.$.restAPI, 'getBlame')
+ .returns(Promise.resolve(mockBlame));
+ element.changeNum = 42;
+ element.patchRange = {patchNum: 5, basePatchNum: 4};
+ element.path = 'foo/bar.baz';
+ return element.loadBlame().then(() => {
+ assert.isTrue(getBlameStub.calledWithExactly(
+ 42, 5, 'foo/bar.baz', true));
+ assert.isFalse(showAlertStub.called);
+ assert.equal(element._blame, mockBlame);
+ assert.equal(element.isBlameLoaded, true);
+ });
+ });
+
+ test('loadBlame empty', () => {
+ const mockBlame = [];
+ const showAlertStub = sinon.stub();
+ element.addEventListener('show-alert', showAlertStub);
+ sandbox.stub(element.$.restAPI, 'getBlame')
+ .returns(Promise.resolve(mockBlame));
+ element.changeNum = 42;
+ element.patchRange = {patchNum: 5, basePatchNum: 4};
+ element.path = 'foo/bar.baz';
+ return element.loadBlame()
+ .then(() => {
+ assert.isTrue(false, 'Promise should not resolve');
+ })
+ .catch(() => {
+ assert.isTrue(showAlertStub.calledOnce);
+ assert.isNull(element._blame);
+ assert.equal(element.isBlameLoaded, false);
+ });
+ });
});
test('delegates getThreadEls()', () => {
- element = fixture('basic');
const returnValue = [document.createElement('b')];
const stub = sandbox.stub(element.$.diff, 'getThreadEls')
.returns(returnValue);
@@ -120,7 +565,6 @@
});
test('delegates addDraftAtLine(el)', () => {
- element = fixture('basic');
const param0 = document.createElement('b');
const stub = sandbox.stub(element.$.diff, 'addDraftAtLine');
element.addDraftAtLine(param0);
@@ -130,7 +574,6 @@
});
test('delegates clearDiffContent()', () => {
- element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'clearDiffContent');
element.clearDiffContent();
assert.isTrue(stub.calledOnce);
@@ -138,7 +581,6 @@
});
test('delegates expandAllContext()', () => {
- element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'expandAllContext');
element.expandAllContext();
assert.isTrue(stub.calledOnce);
@@ -146,101 +588,66 @@
});
test('passes in changeNum', () => {
- element = fixture('basic');
const value = '12345';
element.changeNum = value;
assert.equal(element.$.diff.changeNum, value);
});
test('passes in noAutoRender', () => {
- element = fixture('basic');
const value = true;
element.noAutoRender = value;
assert.equal(element.$.diff.noAutoRender, value);
});
test('passes in patchRange', () => {
- element = fixture('basic');
const value = {patchNum: 'foo', basePatchNum: 'bar'};
element.patchRange = value;
assert.equal(element.$.diff.patchRange, value);
});
test('passes in path', () => {
- element = fixture('basic');
const value = 'some/file/path';
element.path = value;
assert.equal(element.$.diff.path, value);
});
test('passes in prefs', () => {
- element = fixture('basic');
const value = {};
element.prefs = value;
assert.equal(element.$.diff.prefs, value);
});
test('passes in projectConfig', () => {
- element = fixture('basic');
const value = {};
element.projectConfig = value;
assert.equal(element.$.diff.projectConfig, value);
});
test('passes in changeNum', () => {
- element = fixture('basic');
const value = '12345';
element.changeNum = value;
assert.equal(element.$.diff.changeNum, value);
});
test('passes in projectName', () => {
- element = fixture('basic');
const value = 'Gerrit';
element.projectName = value;
assert.equal(element.$.diff.projectName, value);
});
test('passes in displayLine', () => {
- element = fixture('basic');
const value = true;
element.displayLine = value;
assert.equal(element.$.diff.displayLine, value);
});
- test('passes out isImageDiff', () => {
- element = fixture('basic');
- const value = true;
- // isImageDiff is computed, so we cannot just set it.
- sandbox.stub(element.$.diff, '_computeIsImageDiff').returns(value);
- element.$.diff._diff = {left: [], right: [], content: []};
-
- assert.equal(element.isImageDiff, value);
- });
-
test('passes in commitRange', () => {
- element = fixture('basic');
const value = {};
element.commitRange = value;
assert.equal(element.$.diff.commitRange, value);
});
- test('passes in filesWeblinks', () => {
- element = fixture('basic');
- const value = {};
- element.filesWeblinks = value;
- assert.equal(element.$.diff.filesWeblinks, value);
- });
-
- test('passes out filesWeblinks', () => {
- element = fixture('basic');
- const value = {};
- element.$.diff.filesWeblinks = value;
- assert.equal(element.filesWeblinks, value);
- });
-
test('passes in hidden', () => {
- element = fixture('basic');
const value = true;
element.hidden = value;
assert.equal(element.$.diff.hidden, value);
@@ -248,53 +655,125 @@
});
test('passes in noRenderOnPrefsChange', () => {
- element = fixture('basic');
const value = true;
element.noRenderOnPrefsChange = value;
assert.equal(element.$.diff.noRenderOnPrefsChange, value);
});
test('passes in comments', () => {
- element = fixture('basic');
const value = {left: [], right: []};
element.comments = value;
assert.equal(element.$.diff.comments, value);
});
test('passes in lineWrapping', () => {
- element = fixture('basic');
const value = true;
element.lineWrapping = value;
assert.equal(element.$.diff.lineWrapping, value);
});
test('passes in viewMode', () => {
- element = fixture('basic');
const value = 'SIDE_BY_SIDE';
element.viewMode = value;
assert.equal(element.$.diff.viewMode, value);
});
test('passes in lineOfInterest', () => {
- element = fixture('basic');
const value = {number: 123, leftSide: true};
element.lineOfInterest = value;
assert.equal(element.$.diff.lineOfInterest, value);
});
- test('passes in showLoadFailure', () => {
- element = fixture('basic');
- const value = true;
- element.showLoadFailure = value;
- assert.equal(element.$.diff.showLoadFailure, value);
- });
+ suite('_reportDiff', () => {
+ let reportStub;
- test('passes out isBlameLoaded', () => {
- element = fixture('basic');
- const value = true;
- sandbox.stub(element.$.diff, '_computeIsBlameLoaded').returns(value);
- element.$.diff._blame = {};
- assert.equal(element.isBlameLoaded, value);
+ setup(() => {
+ element = fixture('basic');
+ element.patchRange = {basePatchNum: 1};
+ reportStub = sandbox.stub(element.$.reporting, 'reportInteraction');
+ });
+
+ test('null and content-less', () => {
+ element._reportDiff(null);
+ assert.isFalse(reportStub.called);
+
+ element._reportDiff({});
+ assert.isFalse(reportStub.called);
+ });
+
+ test('diff w/ no delta', () => {
+ const diff = {
+ content: [
+ {ab: ['foo', 'bar']},
+ {ab: ['baz', 'foo']},
+ ],
+ };
+ element._reportDiff(diff);
+ assert.isTrue(reportStub.calledOnce);
+ assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
+ assert.isUndefined(reportStub.lastCall.args[1]);
+ });
+
+ test('diff w/ no rebase delta', () => {
+ const diff = {
+ content: [
+ {ab: ['foo', 'bar']},
+ {a: ['baz', 'foo']},
+ {ab: ['foo', 'bar']},
+ {a: ['baz', 'foo'], b: ['bar', 'baz']},
+ {ab: ['foo', 'bar']},
+ {b: ['baz', 'foo']},
+ {ab: ['foo', 'bar']},
+ ],
+ };
+ element._reportDiff(diff);
+ assert.isTrue(reportStub.calledOnce);
+ assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
+ assert.isUndefined(reportStub.lastCall.args[1]);
+ });
+
+ test('diff w/ some rebase delta', () => {
+ const diff = {
+ content: [
+ {ab: ['foo', 'bar']},
+ {a: ['baz', 'foo'], due_to_rebase: true},
+ {ab: ['foo', 'bar']},
+ {a: ['baz', 'foo'], b: ['bar', 'baz']},
+ {ab: ['foo', 'bar']},
+ {b: ['baz', 'foo'], due_to_rebase: true},
+ {ab: ['foo', 'bar']},
+ {a: ['baz', 'foo']},
+ ],
+ };
+ element._reportDiff(diff);
+ assert.isTrue(reportStub.calledOnce);
+ assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
+ assert.strictEqual(reportStub.lastCall.args[1], 50);
+ });
+
+ test('diff w/ all rebase delta', () => {
+ const diff = {content: [{
+ a: ['foo', 'bar'],
+ b: ['baz', 'foo'],
+ due_to_rebase: true,
+ }]};
+ element._reportDiff(diff);
+ assert.isTrue(reportStub.calledOnce);
+ assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
+ assert.strictEqual(reportStub.lastCall.args[1], 100);
+ });
+
+ test('diff against parent event', () => {
+ element.patchRange.basePatchNum = 'PARENT';
+ const diff = {content: [{
+ a: ['foo', 'bar'],
+ b: ['baz', 'foo'],
+ }]};
+ element._reportDiff(diff);
+ assert.isTrue(reportStub.calledOnce);
+ assert.equal(reportStub.lastCall.args[0], 'diff-against-parent');
+ assert.isUndefined(reportStub.lastCall.args[1]);
+ });
});
});
</script>
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 1aba403..6add75c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -18,9 +18,7 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
-<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
-<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-diff-builder/gr-diff-builder.html">
<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
<link rel="import" href="../gr-diff-highlight/gr-diff-highlight.html">
@@ -55,7 +53,6 @@
background-color: var(--table-header-background-color);
}
.image-diff .gr-diff {
- background-color: var(--table-header-background-color);
text-align: center;
}
.image-diff img {
@@ -83,6 +80,9 @@
.content {
background-color: var(--view-background-color);
}
+ .image-diff .content {
+ background-color: var(--table-header-background-color);
+ }
.full-width {
width: 100%;
}
@@ -270,26 +270,26 @@
<div>[[item]]</div>
</template>
</div>
- <div class$="[[_computeContainerClass(_loggedIn, viewMode, displayLine)]]"
+ <div class$="[[_computeContainerClass(loggedIn, viewMode, displayLine)]]"
on-tap="_handleTap">
- <gr-diff-selection diff="[[_diff]]">
+ <gr-diff-selection diff="[[diff]]">
<gr-diff-highlight
id="highlights"
- logged-in="[[_loggedIn]]"
+ logged-in="[[loggedIn]]"
comments="{{comments}}">
<gr-diff-builder
id="diffBuilder"
comments="[[comments]]"
project-name="[[projectName]]"
- diff="[[_diff]]"
+ diff="[[diff]]"
diff-path="[[path]]"
change-num="[[changeNum]]"
patch-num="[[patchRange.patchNum]]"
view-mode="[[viewMode]]"
line-wrapping="[[lineWrapping]]"
is-image-diff="[[isImageDiff]]"
- base-image="[[_baseImage]]"
- revision-image="[[_revisionImage]]"
+ base-image="[[baseImage]]"
+ revision-image="[[revisionImage]]"
parent-index="[[_parentIndex]]"
create-comment-fn="[[_createThreadGroupFn]]"
line-of-interest="[[lineOfInterest]]">
@@ -301,16 +301,16 @@
</gr-diff-highlight>
</gr-diff-selection>
</div>
- <div class$="[[_computeNewlineWarningClass(_newlineWarning, _loading)]]">
+ <div class$="[[_computeNewlineWarningClass(_newlineWarning, loading)]]">
[[_newlineWarning]]
</div>
- <div id="loadingError" class$="[[_computeErrorClass(_errorMessage)]]">
- [[_errorMessage]]
+ <div id="loadingError" class$="[[_computeErrorClass(errorMessage)]]">
+ [[errorMessage]]
</div>
<div id="sizeWarning" class$="[[_computeWarningClass(_showWarning)]]">
<p>
Prevented render because "Whole file" is enabled and this diff is very
- large (about [[_diffLength(_diff)]] lines).
+ large (about [[_diffLength(diff)]] lines).
</p>
<gr-button on-tap="_handleLimitedBypass">
Render with limited context
@@ -319,8 +319,6 @@
Render anyway (may be slow)
</gr-button>
</div>
- <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
- <gr-reporting id="reporting" category="diff"></gr-reporting>
</template>
<script src="gr-diff-line.js"></script>
<script src="gr-diff-group.js"></script>
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 4f9de6f..4f0a73b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -21,11 +21,6 @@
const ERR_COMMENT_ON_EDIT_BASE = 'You cannot comment on the base patch set ' +
'of an edit.';
const ERR_INVALID_LINE = 'Invalid line number: ';
- const MSG_EMPTY_BLAME = 'No blame information for this diff.';
-
- const EVENT_AGAINST_PARENT = 'diff-against-parent';
- const EVENT_ZERO_REBASE = 'rebase-percent-zero';
- const EVENT_NONZERO_REBASE = 'rebase-percent-nonzero';
const NO_NEWLINE_BASE = 'No newline at end of base file.';
const NO_NEWLINE_REVISION = 'No newline at end of revision file.';
@@ -64,6 +59,12 @@
* @event diff-comments-modified
*/
+ /**
+ * Fired when a draft is added or edited.
+ *
+ * @event draft-interaction
+ */
+
properties: {
changeNum: String,
noAutoRender: {
@@ -88,15 +89,8 @@
},
isImageDiff: {
type: Boolean,
- computed: '_computeIsImageDiff(_diff)',
- notify: true,
},
commitRange: Object,
- filesWeblinks: {
- type: Object,
- value() { return {}; },
- notify: true,
- },
hidden: {
type: Boolean,
reflectToAttribute: true,
@@ -123,38 +117,33 @@
*/
lineOfInterest: Object,
- /**
- * If the diff fails to load, show the failure message in the diff rather
- * than bubbling the error up to the whole page. This is useful for when
- * loading inline diffs because one diff failing need not mark the whole
- * page with a failure.
- */
- showLoadFailure: Boolean,
-
- _loading: {
+ loading: {
type: Boolean,
value: false,
observer: '_loadingChanged',
},
- _loggedIn: {
+ loggedIn: {
type: Boolean,
value: false,
},
- _diff: Object,
+ diff: {
+ type: Object,
+ observer: '_diffChanged',
+ },
_diffHeaderItems: {
type: Array,
value: [],
- computed: '_computeDiffHeaderItems(_diff.*)',
+ computed: '_computeDiffHeaderItems(diff.*)',
},
_diffTableClass: {
type: String,
value: '',
},
/** @type {?Object} */
- _baseImage: Object,
+ baseImage: Object,
/** @type {?Object} */
- _revisionImage: Object,
+ revisionImage: Object,
/**
* Whether the safety check for large diffs when whole-file is set has
@@ -172,20 +161,16 @@
_showWarning: Boolean,
/** @type {?string} */
- _errorMessage: {
+ errorMessage: {
type: String,
value: null,
},
/** @type {?Object} */
- _blame: {
+ blame: {
type: Object,
value: null,
- },
- isBlameLoaded: {
- type: Boolean,
- notify: true,
- computed: '_computeIsBlameLoaded(_blame)',
+ observer: '_blameChanged',
},
_parentIndex: {
@@ -195,7 +180,7 @@
_newlineWarning: {
type: String,
- computed: '_computeNewlineWarning(_diff)',
+ computed: '_computeNewlineWarning(diff)',
},
/**
@@ -220,56 +205,6 @@
'create-comment': '_handleCreateComment',
},
- attached() {
- this._getLoggedIn().then(loggedIn => {
- this._loggedIn = loggedIn;
- });
- },
-
- ready() {
- if (this._canRender()) {
- this.reload();
- }
- },
-
- /** @return {!Promise} */
- reload() {
- this._loading = true;
- this._errorMessage = null;
-
- const diffRequest = this._getDiff()
- .then(diff => {
- this._reportDiff(diff);
- return diff;
- })
- .catch(e => {
- this._handleGetDiffError(e);
- return null;
- });
-
- const assetRequest = diffRequest.then(diff => {
- // If the diff is null, then it's failed to load.
- if (!diff) { return null; }
-
- return this._loadDiffAssets(diff);
- });
-
- return Promise.all([diffRequest, assetRequest])
- .then(results => {
- const diff = results[0];
- if (!diff) {
- return Promise.resolve();
- }
- this.filesWeblinks = this._getFilesWeblinks(diff);
- this._diff = diff;
- return this._renderDiffTable();
- })
- .catch(err => {
- console.warn('Error encountered loading diff:', err);
- })
- .then(() => { this._loading = false; });
- },
-
/** Cancel any remaining diff builder rendering work. */
cancel() {
this.$.diffBuilder.cancel();
@@ -293,37 +228,13 @@
this.toggleClass('no-left');
},
- /**
- * Load and display blame information for the base of the diff.
- * @return {Promise} A promise that resolves when blame finishes rendering.
- */
- loadBlame() {
- return this.$.restAPI.getBlame(this.changeNum, this.patchRange.patchNum,
- this.path, true)
- .then(blame => {
- if (!blame.length) {
- this.fire('show-alert', {message: MSG_EMPTY_BLAME});
- return Promise.reject(MSG_EMPTY_BLAME);
- }
-
- this._blame = blame;
-
- this.$.diffBuilder.setBlame(blame);
- this.classList.add('showBlame');
- });
- },
-
- _computeIsBlameLoaded(blame) {
- return !!blame;
- },
-
- /**
- * Unload blame information for the diff.
- */
- clearBlame() {
- this._blame = null;
- this.$.diffBuilder.setBlame(null);
- this.classList.remove('showBlame');
+ _blameChanged(newValue) {
+ this.$.diffBuilder.setBlame(newValue);
+ if (newValue) {
+ this.classList.add('showBlame');
+ } else {
+ this.classList.remove('showBlame');
+ }
},
_handleCommentSaveOrDiscard() {
@@ -331,12 +242,6 @@
{bubbles: true}));
},
- /** @return {boolean}} */
- _canRender() {
- return !!this.changeNum && !!this.patchRange && !!this.path &&
- !this.noAutoRender;
- },
-
/** @return {!Array<!HTMLElement>} */
getThreadEls() {
let threads = [];
@@ -425,7 +330,7 @@
/** @return {boolean} */
_isValidElForComment(el) {
- if (!this._loggedIn) {
+ if (!this.loggedIn) {
this.fire('show-auth-required');
return false;
}
@@ -454,7 +359,7 @@
* @param {!Object=} opt_range
*/
_createComment(lineEl, opt_lineNum, opt_side, opt_range) {
- this.$.reporting.recordDraftInteraction();
+ this.dispatchEvent(new CustomEvent('draft-interaction', {bubbles: true}));
const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
const contentEl = contentText.parentElement;
const side = opt_side ||
@@ -674,7 +579,7 @@
_loadingChanged(newValue) {
if (newValue) {
this.cancel();
- this.clearBlame();
+ this._blame = null;
this._safetyBypass = null;
this._showWarning = false;
this.clearDiffContent();
@@ -688,7 +593,7 @@
_prefsChanged(prefs) {
if (!prefs) { return; }
- this.clearBlame();
+ this._blame = null;
const stylesToUpdate = {};
@@ -709,22 +614,32 @@
this.updateStyles(stylesToUpdate);
- if (this._diff && this.comments && !this.noRenderOnPrefsChange) {
+ if (this.diff && this.comments && !this.noRenderOnPrefsChange) {
+ this._renderDiffTable();
+ }
+ },
+
+ _diffChanged(newValue) {
+ if (newValue) {
this._renderDiffTable();
}
},
_renderDiffTable() {
- if (!this.prefs) { return Promise.resolve(); }
+ if (!this.prefs) {
+ this.dispatchEvent(new CustomEvent('render', {bubbles: true}));
+ return;
+ }
if (this.prefs.context === -1 &&
- this._diffLength(this._diff) >= LARGE_DIFF_THRESHOLD_LINES &&
+ this._diffLength(this.diff) >= LARGE_DIFF_THRESHOLD_LINES &&
this._safetyBypass === null) {
this._showWarning = true;
- return Promise.resolve();
+ this.dispatchEvent(new CustomEvent('render', {bubbles: true}));
+ return;
}
this._showWarning = false;
- return this.$.diffBuilder.render(this.comments, this._getBypassPrefs());
+ this.$.diffBuilder.render(this.comments, this._getBypassPrefs());
},
/**
@@ -741,138 +656,6 @@
this.$.diffTable.innerHTML = null;
},
- _handleGetDiffError(response) {
- // Loading the diff may respond with 409 if the file is too large. In this
- // case, use a toast error..
- if (response.status === 409) {
- this.fire('server-error', {response});
- return;
- }
-
- if (this.showLoadFailure) {
- this._errorMessage = [
- 'Encountered error when loading the diff:',
- response.status,
- response.statusText,
- ].join(' ');
- return;
- }
-
- this.fire('page-error', {response});
- },
-
- /** @return {!Promise<!Object>} */
- _getDiff() {
- // Wrap the diff request in a new promise so that the error handler
- // rejects the promise, allowing the error to be handled in the .catch.
- return new Promise((resolve, reject) => {
- this.$.restAPI.getDiff(
- this.changeNum,
- this.patchRange.basePatchNum,
- this.patchRange.patchNum,
- this.path,
- reject)
- .then(resolve);
- });
- },
-
- _getFilesWeblinks(diff) {
- if (!this.commitRange) { return {}; }
- return {
- meta_a: Gerrit.Nav.getFileWebLinks(
- this.projectName, this.commitRange.baseCommit, this.path,
- {weblinks: diff && diff.meta_a && diff.meta_a.web_links}),
- meta_b: Gerrit.Nav.getFileWebLinks(
- this.projectName, this.commitRange.commit, this.path,
- {weblinks: diff && diff.meta_b && diff.meta_b.web_links}),
- };
- },
-
- /**
- * Report info about the diff response.
- */
- _reportDiff(diff) {
- if (!diff || !diff.content) { return; }
-
- // Count the delta lines stemming from normal deltas, and from
- // due_to_rebase deltas.
- let nonRebaseDelta = 0;
- let rebaseDelta = 0;
- diff.content.forEach(chunk => {
- if (chunk.ab) { return; }
- const deltaSize = Math.max(
- chunk.a ? chunk.a.length : 0, chunk.b ? chunk.b.length : 0);
- if (chunk.due_to_rebase) {
- rebaseDelta += deltaSize;
- } else {
- nonRebaseDelta += deltaSize;
- }
- });
-
- // Find the percent of the delta from due_to_rebase chunks rounded to two
- // digits. Diffs with no delta are considered 0%.
- const totalDelta = rebaseDelta + nonRebaseDelta;
- const percentRebaseDelta = !totalDelta ? 0 :
- Math.round(100 * rebaseDelta / totalDelta);
-
- // Report the due_to_rebase percentage in the "diff" category when
- // applicable.
- if (this.patchRange.basePatchNum === 'PARENT') {
- this.$.reporting.reportInteraction(EVENT_AGAINST_PARENT);
- } else if (percentRebaseDelta === 0) {
- this.$.reporting.reportInteraction(EVENT_ZERO_REBASE);
- } else {
- this.$.reporting.reportInteraction(EVENT_NONZERO_REBASE,
- percentRebaseDelta);
- }
- },
-
- /** @return {!Promise} */
- _getLoggedIn() {
- return this.$.restAPI.getLoggedIn();
- },
-
- /**
- * @param {Object} diff
- * @return {boolean}
- */
- _computeIsImageDiff(diff) {
- if (!diff) { return false; }
-
- const isA = diff.meta_a &&
- diff.meta_a.content_type.startsWith('image/');
- const isB = diff.meta_b &&
- diff.meta_b.content_type.startsWith('image/');
-
- return !!(diff.binary && (isA || isB));
- },
-
- /**
- * @param {Object} diff
- * @return {!Promise}
- */
- _loadDiffAssets(diff) {
- if (this._computeIsImageDiff(diff)) {
- return this._getImages(diff).then(images => {
- this._baseImage = images.baseImage;
- this._revisionImage = images.revisionImage;
- });
- } else {
- this._baseImage = null;
- this._revisionImage = null;
- return Promise.resolve();
- }
- },
-
- /**
- * @param {Object} diff
- * @return {!Promise}
- */
- _getImages(diff) {
- return this.$.restAPI.getImagesForDiff(this.changeNum, diff,
- this.patchRange);
- },
-
_projectConfigChanged(projectConfig) {
const threadEls = this.getThreadEls();
for (let i = 0; i < threadEls.length; i++) {
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 97338e2..0274fae 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
@@ -48,18 +48,8 @@
sandbox.restore();
});
- test('reload cancels before network resolves', () => {
- element = fixture('basic');
- const cancelStub = sandbox.stub(element, 'cancel');
-
- // Stub the network calls into requests that never resolve.
- sandbox.stub(element, '_getDiff', () => new Promise(() => {}));
-
- element.reload();
- assert.isTrue(cancelStub.called);
- });
-
test('cancel', () => {
+ element = fixture('basic');
const cancelStub = sandbox.stub(element.$.diffBuilder, 'cancel');
element.cancel();
assert.isTrue(cancelStub.calledOnce);
@@ -208,41 +198,6 @@
element.$$('.diffContainer').classList.contains('displayLine'));
});
- test('loads files weblinks', () => {
- const weblinksStub = sandbox.stub(Gerrit.Nav, '_generateWeblinks')
- .returns({name: 'stubb', url: '#s'});
- sandbox.stub(element.$.restAPI, 'getDiff').returns(Promise.resolve({
- content: [],
- }));
- element.projectName = 'test-project';
- element.path = 'test-path';
- element.commitRange = {baseCommit: 'test-base', commit: 'test-commit'};
- element.patchRange = {};
- return element.reload().then(() => {
- assert.isTrue(weblinksStub.calledTwice);
- assert.isTrue(weblinksStub.firstCall.calledWith({
- commit: 'test-base',
- file: 'test-path',
- options: {
- weblinks: undefined,
- },
- repo: 'test-project',
- type: Gerrit.Nav.WeblinkType.FILE}));
- assert.isTrue(weblinksStub.secondCall.calledWith({
- commit: 'test-commit',
- file: 'test-path',
- options: {
- weblinks: undefined,
- },
- repo: 'test-project',
- type: Gerrit.Nav.WeblinkType.FILE}));
- assert.deepEqual(element.filesWeblinks, {
- meta_a: [{name: 'stubb', url: '#s'}],
- meta_b: [{name: 'stubb', url: '#s'}],
- });
- });
- });
-
test('remove comment', () => {
element.comments = {
meta: {
@@ -408,66 +363,29 @@
'wsAAAAAAAAAAAAA/////w==',
type: 'image/bmp',
};
- const mockCommit = {
- commit: '9a1a1d10baece5efbba10bc4ccf808a67a50ac0a',
- parents: [{
- commit: '7338aa9adfe57909f1fdaf88975cdea467d3382f',
- subject: 'Added a carrot',
- }],
- author: {
- name: 'Wyatt Allen',
- email: 'wyatta@google.com',
- date: '2016-05-23 21:44:51.000000000',
- tz: -420,
- },
- committer: {
- name: 'Wyatt Allen',
- email: 'wyatta@google.com',
- date: '2016-05-25 00:25:41.000000000',
- tz: -420,
- },
- subject: 'Updated the carrot',
- message: 'Updated the carrot\n\nChange-Id: Iabcd123\n',
- };
- const mockComments = {baseComments: [], comments: []};
-
- sandbox.stub(element.$.restAPI, 'getCommitInfo')
- .returns(Promise.resolve(mockCommit));
- sandbox.stub(element.$.restAPI,
- 'getB64FileContents',
- (changeId, patchNum, path, opt_parentIndex) => {
- return Promise.resolve(opt_parentIndex === 1 ? mockFile1 :
- mockFile2);
- });
- sandbox.stub(element.$.restAPI, '_getDiffComments')
- .returns(Promise.resolve(mockComments));
- sandbox.stub(element.$.restAPI, 'getDiffDrafts')
- .returns(Promise.resolve(mockComments));
element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
element.comments = {left: [], right: []};
+ element.isImageDiff = true;
+ element.prefs = {
+ auto_hide_diff_table_header: true,
+ context: 10,
+ cursor_blink_rate: 0,
+ font_size: 12,
+ ignore_whitespace: 'IGNORE_NONE',
+ 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',
+ };
});
test('renders image diffs with same file name', done => {
- const mockDiff = {
- meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
- meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
- lines: 560},
- intraline_status: 'OK',
- change_type: 'MODIFIED',
- diff_header: [
- 'diff --git a/carrot.jpg b/carrot.jpg',
- 'index 2adc47d..f9c2f2c 100644',
- '--- a/carrot.jpg',
- '+++ b/carrot.jpg',
- 'Binary files differ',
- ],
- content: [{skip: 66}],
- binary: true,
- };
- sandbox.stub(element.$.restAPI, 'getDiff')
- .returns(Promise.resolve(mockDiff));
-
const rendered = () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
@@ -522,10 +440,24 @@
element.addEventListener('render', rendered);
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.baseImage = mockFile1;
+ element.revisionImage = mockFile2;
+ element.diff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
+ meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index 2adc47d..f9c2f2c 100644',
+ '--- a/carrot.jpg',
+ '+++ b/carrot.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
});
test('renders image diffs with a different file name', done => {
@@ -545,8 +477,6 @@
content: [{skip: 66}],
binary: true,
};
- sandbox.stub(element.$.restAPI, 'getDiff')
- .returns(Promise.resolve(mockDiff));
const rendered = () => {
// Recognizes that it should be an image diff.
@@ -604,10 +534,11 @@
element.addEventListener('render', rendered);
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.baseImage = mockFile1;
+ element.baseImage._name = mockDiff.meta_a.name;
+ element.revisionImage = mockFile2;
+ element.revisionImage._name = mockDiff.meta_b.name;
+ element.diff = mockDiff;
});
test('renders added image', done => {
@@ -626,8 +557,6 @@
content: [{skip: 66}],
binary: true,
};
- sandbox.stub(element.$.restAPI, 'getDiff')
- .returns(Promise.resolve(mockDiff));
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
@@ -643,10 +572,8 @@
done();
});
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.revisionImage = mockFile2;
+ element.diff = mockDiff;
});
test('renders removed image', done => {
@@ -665,8 +592,6 @@
content: [{skip: 66}],
binary: true,
};
- sandbox.stub(element.$.restAPI, 'getDiff')
- .returns(Promise.resolve(mockDiff));
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
@@ -682,10 +607,8 @@
done();
});
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.baseImage = mockFile1;
+ element.diff = mockDiff;
});
test('does not render disallowed image type', done => {
@@ -706,9 +629,6 @@
};
mockFile1.type = 'image/jpeg-evil';
- sandbox.stub(element.$.restAPI, 'getDiff')
- .returns(Promise.resolve(mockDiff));
-
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
@@ -719,10 +639,8 @@
done();
});
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.baseImage = mockFile1;
+ element.diff = mockDiff;
});
});
@@ -769,67 +687,10 @@
content.click();
});
- test('_getDiff handles null diff responses', done => {
- stub('gr-rest-api-interface', {
- getDiff() { return Promise.resolve(null); },
- });
- element.changeNum = 123;
- element.patchRange = {basePatchNum: 1, patchNum: 2};
- element.path = 'file.txt';
- element._getDiff().then(done);
- });
-
- test('reload resolves on error', () => {
- const onErrStub = sandbox.stub(element, '_handleGetDiffError');
- const error = {ok: false, status: 500};
- sandbox.stub(element.$.restAPI, 'getDiff',
- (changeNum, basePatchNum, patchNum, path, onErr) => {
- onErr(error);
- });
- return element.reload().then(() => {
- assert.isTrue(onErrStub.calledOnce);
- });
- });
-
- suite('_handleGetDiffError', () => {
- let serverErrorStub;
- let pageErrorStub;
-
- setup(() => {
- serverErrorStub = sinon.stub();
- element.addEventListener('server-error', serverErrorStub);
- pageErrorStub = sinon.stub();
- element.addEventListener('page-error', pageErrorStub);
- });
-
- test('page error on HTTP-409', () => {
- element._handleGetDiffError({status: 409});
- assert.isTrue(serverErrorStub.calledOnce);
- assert.isFalse(pageErrorStub.called);
- assert.isNotOk(element._errorMessage);
- });
-
- test('server error on non-HTTP-409', () => {
- element._handleGetDiffError({status: 500});
- assert.isFalse(serverErrorStub.called);
- assert.isTrue(pageErrorStub.calledOnce);
- assert.isNotOk(element._errorMessage);
- });
-
- test('error message if showLoadFailure', () => {
- element.showLoadFailure = true;
- element._handleGetDiffError({status: 500, statusText: 'Failure!'});
- assert.isFalse(serverErrorStub.called);
- assert.isFalse(pageErrorStub.called);
- assert.equal(element._errorMessage,
- 'Encountered error when loading the diff: 500 Failure!');
- });
- });
-
suite('getCursorStops', () => {
const setupDiff = function() {
const mock = document.createElement('mock-diff-response');
- element._diff = mock.diffResponse;
+ element.diff = mock.diffResponse;
element.comments = {
left: [],
right: [],
@@ -878,15 +739,8 @@
suite('logged in', () => {
let fakeLineEl;
setup(() => {
- const getLoggedInPromise = Promise.resolve(true);
- stub('gr-rest-api-interface', {
- getLoggedIn() { return getLoggedInPromise; },
- getPreferences() {
- return Promise.resolve({time_format: 'HHMM_12'});
- },
- getAccountCapabilities() { return Promise.resolve(); },
- });
element = fixture('basic');
+ element.loggedIn = true;
element.patchRange = {};
fakeLineEl = {
@@ -895,15 +749,11 @@
contains: sandbox.stub().returns(true),
},
};
- return getLoggedInPromise;
});
test('addDraftAtLine', () => {
sandbox.stub(element, '_selectLine');
sandbox.stub(element, '_createComment');
- const loggedInErrorSpy = sandbox.spy();
- element.addEventListener('show-auth-required', loggedInErrorSpy);
- assert.isFalse(loggedInErrorSpy.called);
element.addDraftAtLine(fakeLineEl);
assert.isTrue(element._createComment
.calledWithExactly(fakeLineEl, 42));
@@ -913,12 +763,9 @@
element.patchRange.basePatchNum = element.EDIT_NAME;
sandbox.stub(element, '_selectLine');
sandbox.stub(element, '_createComment');
- const loggedInErrorSpy = sandbox.spy();
const alertSpy = sandbox.spy();
- element.addEventListener('show-auth-required', loggedInErrorSpy);
element.addEventListener('show-alert', alertSpy);
element.addDraftAtLine(fakeLineEl);
- assert.isFalse(loggedInErrorSpy.called);
assert.isTrue(alertSpy.called);
assert.isFalse(element._createComment.called);
});
@@ -928,19 +775,16 @@
element.patchRange.basePatchNum = element.PARENT_NAME;
sandbox.stub(element, '_selectLine');
sandbox.stub(element, '_createComment');
- const loggedInErrorSpy = sandbox.spy();
const alertSpy = sandbox.spy();
- element.addEventListener('show-auth-required', loggedInErrorSpy);
element.addEventListener('show-alert', alertSpy);
element.addDraftAtLine(fakeLineEl);
- assert.isFalse(loggedInErrorSpy.called);
assert.isTrue(alertSpy.called);
assert.isFalse(element._createComment.called);
});
suite('change in preferences', () => {
setup(() => {
- element._diff = {
+ element.diff = {
meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
lines: 560},
@@ -1074,7 +918,8 @@
suite('diff header', () => {
setup(() => {
- element._diff = {
+ element = fixture('basic');
+ element.diff = {
meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
lines: 560},
@@ -1087,15 +932,15 @@
test('hidden', () => {
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
+ element.push('diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', 'index 2adc47d..f9c2f2c 100644');
+ element.push('diff.diff_header', 'index 2adc47d..f9c2f2c 100644');
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', '--- a/test.jpg');
+ element.push('diff.diff_header', '--- a/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', '+++ b/test.jpg');
+ element.push('diff.diff_header', '+++ b/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', 'test');
+ element.push('diff.diff_header', 'test');
assert.equal(element._diffHeaderItems.length, 1);
flushAsynchronousOperations();
@@ -1103,13 +948,13 @@
});
test('binary files', () => {
- element._diff.binary = true;
+ element.diff.binary = true;
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
+ element.push('diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', 'test');
+ element.push('diff.diff_header', 'test');
assert.equal(element._diffHeaderItems.length, 1);
- element.push('_diff.diff_header', 'Binary files differ');
+ element.push('diff.diff_header', 'Binary files differ');
assert.equal(element._diffHeaderItems.length, 1);
});
});
@@ -1120,39 +965,49 @@
setup(() => {
element = fixture('basic');
renderStub = sandbox.stub(element.$.diffBuilder, 'render',
- () => Promise.resolve());
+ () => {
+ Promise.resolve();
+ element.$.diffBuilder.dispatchEvent(
+ new CustomEvent('render', {bubbles: true}));
+ });
const mock = document.createElement('mock-diff-response');
- element._diff = mock.diffResponse;
+ element.diff = mock.diffResponse;
element.comments = {left: [], right: []};
element.noRenderOnPrefsChange = true;
});
- test('lage render w/ context = 10', () => {
+ test('large render w/ context = 10', done => {
element.prefs = {context: 10};
sandbox.stub(element, '_diffLength', () => 10000);
- return element._renderDiffTable().then(() => {
+ element.addEventListener('render', () => {
assert.isTrue(renderStub.called);
assert.isFalse(element._showWarning);
+ done();
});
+ element._renderDiffTable();
});
- test('lage render w/ whole file and bypass', () => {
+ test('large render w/ whole file and bypass', done => {
element.prefs = {context: -1};
element._safetyBypass = 10;
sandbox.stub(element, '_diffLength', () => 10000);
- return element._renderDiffTable().then(() => {
+ element.addEventListener('render', () => {
assert.isTrue(renderStub.called);
assert.isFalse(element._showWarning);
+ done();
});
+ element._renderDiffTable();
});
- test('lage render w/ whole file and no bypass', () => {
+ test('large render w/ whole file and no bypass', done => {
element.prefs = {context: -1};
sandbox.stub(element, '_diffLength', () => 10000);
- return element._renderDiffTable().then(() => {
+ element.addEventListener('render', () => {
assert.isFalse(renderStub.called);
assert.isTrue(element._showWarning);
+ done();
});
+ element._renderDiffTable();
});
});
@@ -1161,144 +1016,19 @@
element = fixture('basic');
});
- test('clearBlame', () => {
- element._blame = [];
+ test('unsetting', () => {
+ element.blame = [];
const setBlameSpy = sandbox.spy(element.$.diffBuilder, 'setBlame');
element.classList.add('showBlame');
- element.clearBlame();
- assert.isNull(element._blame);
+ element.blame = null;
assert.isTrue(setBlameSpy.calledWithExactly(null));
assert.isFalse(element.classList.contains('showBlame'));
});
- test('loadBlame', () => {
+ test('setting', () => {
const mockBlame = [{id: 'commit id', ranges: [{start: 1, end: 2}]}];
- const showAlertStub = sinon.stub();
- element.addEventListener('show-alert', showAlertStub);
- const getBlameStub = sandbox.stub(element.$.restAPI, 'getBlame')
- .returns(Promise.resolve(mockBlame));
- element.changeNum = 42;
- element.patchRange = {patchNum: 5, basePatchNum: 4};
- element.path = 'foo/bar.baz';
- return element.loadBlame().then(() => {
- assert.isTrue(getBlameStub.calledWithExactly(
- 42, 5, 'foo/bar.baz', true));
- assert.isFalse(showAlertStub.called);
- assert.equal(element._blame, mockBlame);
- assert.isTrue(element.classList.contains('showBlame'));
- });
- });
-
- test('loadBlame empty', () => {
- const mockBlame = [];
- const showAlertStub = sinon.stub();
- element.addEventListener('show-alert', showAlertStub);
- sandbox.stub(element.$.restAPI, 'getBlame')
- .returns(Promise.resolve(mockBlame));
- element.changeNum = 42;
- element.patchRange = {patchNum: 5, basePatchNum: 4};
- element.path = 'foo/bar.baz';
- return element.loadBlame()
- .then(() => {
- assert.isTrue(false, 'Promise should not resolve');
- })
- .catch(() => {
- assert.isTrue(showAlertStub.calledOnce);
- assert.isNull(element._blame);
- assert.isFalse(element.classList.contains('showBlame'));
- });
- });
- });
-
- suite('_reportDiff', () => {
- let reportStub;
-
- setup(() => {
- element = fixture('basic');
- element.patchRange = {basePatchNum: 1};
- reportStub = sandbox.stub(element.$.reporting, 'reportInteraction');
- });
-
- test('null and content-less', () => {
- element._reportDiff(null);
- assert.isFalse(reportStub.called);
-
- element._reportDiff({});
- assert.isFalse(reportStub.called);
- });
-
- test('diff w/ no delta', () => {
- const diff = {
- content: [
- {ab: ['foo', 'bar']},
- {ab: ['baz', 'foo']},
- ],
- };
- element._reportDiff(diff);
- assert.isTrue(reportStub.calledOnce);
- assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
- assert.isUndefined(reportStub.lastCall.args[1]);
- });
-
- test('diff w/ no rebase delta', () => {
- const diff = {
- content: [
- {ab: ['foo', 'bar']},
- {a: ['baz', 'foo']},
- {ab: ['foo', 'bar']},
- {a: ['baz', 'foo'], b: ['bar', 'baz']},
- {ab: ['foo', 'bar']},
- {b: ['baz', 'foo']},
- {ab: ['foo', 'bar']},
- ],
- };
- element._reportDiff(diff);
- assert.isTrue(reportStub.calledOnce);
- assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
- assert.isUndefined(reportStub.lastCall.args[1]);
- });
-
- test('diff w/ some rebase delta', () => {
- const diff = {
- content: [
- {ab: ['foo', 'bar']},
- {a: ['baz', 'foo'], due_to_rebase: true},
- {ab: ['foo', 'bar']},
- {a: ['baz', 'foo'], b: ['bar', 'baz']},
- {ab: ['foo', 'bar']},
- {b: ['baz', 'foo'], due_to_rebase: true},
- {ab: ['foo', 'bar']},
- {a: ['baz', 'foo']},
- ],
- };
- element._reportDiff(diff);
- assert.isTrue(reportStub.calledOnce);
- assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
- assert.strictEqual(reportStub.lastCall.args[1], 50);
- });
-
- test('diff w/ all rebase delta', () => {
- const diff = {content: [{
- a: ['foo', 'bar'],
- b: ['baz', 'foo'],
- due_to_rebase: true,
- }]};
- element._reportDiff(diff);
- assert.isTrue(reportStub.calledOnce);
- assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
- assert.strictEqual(reportStub.lastCall.args[1], 100);
- });
-
- test('diff against parent event', () => {
- element.patchRange.basePatchNum = 'PARENT';
- const diff = {content: [{
- a: ['foo', 'bar'],
- b: ['baz', 'foo'],
- }]};
- element._reportDiff(diff);
- assert.isTrue(reportStub.calledOnce);
- assert.equal(reportStub.lastCall.args[0], 'diff-against-parent');
- assert.isUndefined(reportStub.lastCall.args[1]);
+ element.blame = mockBlame;
+ assert.isTrue(element.classList.contains('showBlame'));
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
index 52785ff..32ca557 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
@@ -20,7 +20,6 @@
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-icons/gr-icons.html">
-<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<dom-module id="gr-copy-clipboard">
<template>
@@ -30,15 +29,11 @@
display: flex;
flex-wrap: wrap;
}
- .text label {
- flex: 0 0 100%;
- }
.copyText {
flex-grow: 1;
margin-right: .3em;
}
- .hideInput,
- .hideLabel label {
+ .hideInput {
display: none;
}
input {
@@ -51,8 +46,7 @@
width: 1.2em;
}
</style>
- <div class$="text [[_computeLabelClass(hideLabel)]]">
- <label>[[title]]</label>
+ <div class="text">
<input id="input" is="iron-input"
class$="copyText [[_computeInputClass(hideInput)]]"
type="text"
@@ -67,8 +61,7 @@
on-tap="_copyToClipboard">
<iron-icon id="icon" icon="gr-icons:content-copy"></iron-icon>
</gr-button>
- </div>
- <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
+ </div>
</template>
<script src="gr-copy-clipboard.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
index cd8cb00..cabee36 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
@@ -24,7 +24,6 @@
properties: {
text: String,
- title: String,
buttonTitle: String,
hasTooltip: {
type: Boolean,
@@ -34,10 +33,6 @@
type: Boolean,
value: false,
},
- hideLabel: {
- type: Boolean,
- value: false,
- },
},
focusOnCopy() {
@@ -48,10 +43,6 @@
return hideInput ? 'hideInput' : '';
},
- _computeLabelClass(hideLabel) {
- return hideLabel ? 'hideLabel' : '';
- },
-
_handleInputTap(e) {
e.preventDefault();
Polymer.dom(e).rootTarget.select();
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
index c865917..d6e9dca 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
@@ -38,12 +38,8 @@
let sandbox;
setup(() => {
- stub('gr-rest-api-interface', {
- saveChangeStarred() { return Promise.resolve({ok: true}); },
- });
sandbox = sinon.sandbox.create();
element = fixture('basic');
- element.title = 'Checkout';
element.text = `git fetch http://gerrit@localhost:8080/a/test-project
refs/changes/05/5/1 && git checkout FETCH_HEAD`;
flushAsynchronousOperations();
@@ -79,12 +75,5 @@
flushAsynchronousOperations();
assert.equal(getComputedStyle(element.$.input).display, 'none');
});
-
- test('hideLabel', () => {
- assert.notEqual(getComputedStyle(element.$$('label')).display, 'none');
- element.hideLabel = true;
- flushAsynchronousOperations();
- assert.equal(getComputedStyle(element.$$('label')).display, 'none');
- });
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.html b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.html
index 83e99be..bfa7885 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.html
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.html
@@ -20,7 +20,7 @@
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../bower_components/paper-tabs/paper-tabs.html">
-<link rel="import" href="../../shared/gr-copy-clipboard/gr-copy-clipboard.html">
+<link rel="import" href="../../shared/gr-shell-command/gr-shell-command.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../../styles/shared-styles.html">
@@ -52,7 +52,7 @@
display: flex;
flex-direction: column;
}
- gr-copy-clipboard {
+ gr-shell-command {
width: 60em;
margin-bottom: .5em;
}
@@ -75,9 +75,9 @@
<template is="dom-repeat"
items="[[commands]]"
as="command">
- <gr-copy-clipboard
- title=[[command.title]]
- text=[[command.command]]></gr-copy-clipboard>
+ <gr-shell-command
+ label=[[command.title]]
+ command=[[command.command]]></gr-shell-command>
</template>
</div>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.js b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.js
index 319cd04..ca77a30 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.js
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.js
@@ -44,7 +44,7 @@
},
focusOnCopy() {
- this.$$('gr-copy-clipboard').focusOnCopy();
+ this.$$('gr-shell-command').focusOnCopy();
},
_getLoggedIn() {
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.html b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.html
index 47219a7..c59e56a 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.html
@@ -74,7 +74,7 @@
});
test('focusOnCopy', () => {
- const focusStub = sandbox.stub(element.$$('gr-copy-clipboard'),
+ const focusStub = sandbox.stub(element.$$('gr-shell-command'),
'focusOnCopy');
element.focusOnCopy();
assert.isTrue(focusStub.called);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.js
index 57cbc85..c18f753 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.js
@@ -43,10 +43,14 @@
* @param {string} method HTTP Method (GET, POST, etc)
* @param {string} url URL without base path or plugin prefix
* @param {Object=} payload Respected for POST and PUT only.
+ * @param {?function(?Response, string=)=} opt_errFn
+ * passed as null sometimes.
* @return {!Promise}
*/
- GrPluginRestApi.prototype.fetch = function(method, url, opt_payload) {
- return getRestApi().send(method, this.opt_prefix + url, opt_payload);
+ GrPluginRestApi.prototype.fetch = function(method, url, opt_payload,
+ opt_errFn) {
+ return getRestApi().send(method, this.opt_prefix + url, opt_payload,
+ opt_errFn);
};
/**
@@ -54,10 +58,13 @@
* @param {string} method HTTP Method (GET, POST, etc)
* @param {string} url URL without base path or plugin prefix
* @param {Object=} payload Respected for POST and PUT only.
+ * @param {?function(?Response, string=)=} opt_errFn
+ * passed as null sometimes.
* @return {!Promise} resolves on success, rejects on error.
*/
- GrPluginRestApi.prototype.send = function(method, url, opt_payload) {
- return this.fetch(method, url, opt_payload).then(response => {
+ GrPluginRestApi.prototype.send = function(method, url, opt_payload,
+ opt_errFn) {
+ return this.fetch(method, url, opt_payload, opt_errFn).then(response => {
if (response.status < 200 || response.status >= 300) {
return response.text().then(text => {
if (text) {
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 8d714eb..84c1803 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
@@ -1497,13 +1497,20 @@
* @return {!Promise<?Object>}
*/
getRepos(filter, reposPerPage, opt_offset) {
+ const defaultFilter = 'state:active OR state:read-only';
const offset = opt_offset || 0;
+ if (!filter) {
+ filter = defaultFilter;
+ }
+ filter = filter.trim();
+ const encodedFilter = encodeURIComponent(filter);
+
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
return this._fetchSharedCacheURL({
- url: `/projects/?d&n=${reposPerPage + 1}&S=${offset}` +
- this._computeFilter(filter),
+ url: `/projects/?n=${reposPerPage + 1}&S=${offset}` +
+ `&query=${encodedFilter}`,
anonymizedUrl: '/projects/?*',
});
},
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 193d306..b7466ef 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
@@ -930,33 +930,38 @@
});
});
- test('getRepos', () => {
- sandbox.stub(element, '_fetchSharedCacheURL');
- element.getRepos('test', 25);
- assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
- '/projects/?d&n=26&S=0&m=test');
+ suite('getRepos', () => {
+ const defaultQuery = 'state%3Aactive%20OR%20state%3Aread-only';
- element.getRepos(null, 25);
- assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
- '/projects/?d&n=26&S=0');
+ setup(() => {
+ sandbox.stub(element, '_fetchSharedCacheURL');
+ });
- element.getRepos('test', 25, 25);
- assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
- '/projects/?d&n=26&S=25&m=test');
- });
+ test('normal use', () => {
+ element.getRepos('test', 25);
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?n=26&S=0&query=test');
- test('getRepos filter', () => {
- sandbox.stub(element, '_fetchSharedCacheURL');
- element.getRepos('test/test/test', 25);
- assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
- '/projects/?d&n=26&S=0&m=test%2Ftest%2Ftest');
- });
+ element.getRepos(null, 25);
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ `/projects/?n=26&S=0&query=${defaultQuery}`);
- test('getRepos filter regex', () => {
- sandbox.stub(element, '_fetchSharedCacheURL');
- element.getRepos('^test.*', 25);
- assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
- '/projects/?d&n=26&S=0&r=%5Etest.*');
+ element.getRepos('test', 25, 25);
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?n=26&S=25&query=test');
+ });
+
+ test('with filter', () => {
+ element.getRepos('test/test/test', 25);
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?n=26&S=0&query=test%2Ftest%2Ftest');
+ });
+
+ test('with regex filter', () => {
+ element.getRepos('^test.*', 25);
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?n=26&S=0&query=%5Etest.*');
+ });
});
test('getGroups filter regex', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.html b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.html
new file mode 100644
index 0000000..fe6ed88
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.html
@@ -0,0 +1,58 @@
+<!--
+@license
+Copyright (C) 2018 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.
+-->
+
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../shared/gr-copy-clipboard/gr-copy-clipboard.html">
+
+<dom-module id="gr-shell-command">
+ <template>
+ <style include="shared-styles">
+ .commandContainer {
+ margin-bottom: .75em;
+ }
+ .commandContainer {
+ background-color: var(--shell-command-background-color);
+ padding: .5em .5em .5em 2.5em;
+ position: relative;
+ width: 100%;
+ }
+ .commandContainer:before {
+ background: var(--shell-command-decoration-background-color);
+ bottom: 0;
+ box-sizing: border-box;
+ content: '$';
+ display: block;
+ left: 0;
+ padding: .8em;
+ position: absolute;
+ top: 0;
+ width: 2em;
+ }
+ .commandContainer gr-copy-clipboard {
+ --text-container-style: {
+ border: none;
+ }
+ }
+ </style>
+ <label>[[label]]</label>
+ <div class="commandContainer">
+ <gr-copy-clipboard text="[[command]]"></gr-copy-clipboard>
+ </div>
+ </template>
+ <script src="gr-shell-command.js"></script>
+</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.js b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.js
new file mode 100644
index 0000000..2c546cc
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.js
@@ -0,0 +1,32 @@
+/**
+ * @license
+ * Copyright (C) 2018 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() {
+ 'use strict';
+
+ Polymer({
+ is: 'gr-shell-command',
+
+ properties: {
+ command: String,
+ label: String,
+ },
+
+ focusOnCopy() {
+ this.$$('gr-copy-clipboard').focusOnCopy();
+ },
+ });
+})();
diff --git a/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.html b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.html
new file mode 100644
index 0000000..a49f76f
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.html
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<!--
+@license
+Copyright (C) 2018 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-shell-command</title>
+
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.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="gr-shell-command.html">
+
+<script>void(0);</script>
+
+<test-fixture id="basic">
+ <template>
+ <gr-shell-command></gr-shell-command>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-shell-command tests', () => {
+ let element;
+ let sandbox;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ element = fixture('basic');
+ element.text = `git fetch http://gerrit@localhost:8080/a/test-project
+ refs/changes/05/5/1 && git checkout FETCH_HEAD`;
+ flushAsynchronousOperations();
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ test('focusOnCopy', () => {
+ const focusStub = sandbox.stub(element.$$('gr-copy-clipboard'),
+ 'focusOnCopy');
+ element.focusOnCopy();
+ assert.isTrue(focusStub.called);
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/styles/themes/app-theme.html b/polygerrit-ui/app/styles/themes/app-theme.html
index 81c195a..ea81796 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.html
+++ b/polygerrit-ui/app/styles/themes/app-theme.html
@@ -96,6 +96,9 @@
--diff-highlight-range-color: rgba(255, 213, 0, 0.5);
--diff-highlight-range-hover-color: rgba(255, 255, 0, 0.5);
+ --shell-command-background-color: #f5f5f5;
+ --shell-command-decoration-background-color: #ebebeb;
+
--comment-text-color: #000;
--comment-background-color: #fcfad6;
--unresolved-comment-background-color: #fcfaa6;
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.html b/polygerrit-ui/app/styles/themes/dark-theme.html
index 1f473da..8ade9ba 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.html
+++ b/polygerrit-ui/app/styles/themes/dark-theme.html
@@ -29,6 +29,7 @@
--diff-selection-background-color: #3A71D8;
--light-remove-highlight-color: rgb(53, 27, 27);
--light-add-highlight-color: rgb(24, 45, 24);
+ --light-remove-add-highlight-color: #2f3f2f;
--light-rebased-remove-highlight-color: rgb(60, 37, 8);
--light-rebased-add-highlight-color: rgb(72, 113, 101);
--dark-remove-highlight-color: rgba(255, 0, 0, 0.15);
@@ -39,6 +40,8 @@
--diff-context-control-border-color: var(--border-color);
--diff-highlight-range-color: rgba(0, 100, 200, 0.5);
--diff-highlight-range-hover-color: rgba(0, 150, 255, 0.5);
+ --shell-command-background-color: #5f5f5f;
+ --shell-command-decoration-background-color: #999999;
--comment-text-color: var(--primary-text-color);
--comment-background-color: #0B162B;
--unresolved-comment-background-color: rgb(56, 90, 154);
diff --git a/resources/com/google/gerrit/pgm/init/gerrit.sh b/resources/com/google/gerrit/pgm/init/gerrit.sh
index 5571e7c..d3f3666 100755
--- a/resources/com/google/gerrit/pgm/init/gerrit.sh
+++ b/resources/com/google/gerrit/pgm/init/gerrit.sh
@@ -443,6 +443,11 @@
echo -16 > "/proc/${PID}/oom_adj"
fi
fi
+ elif [ "$(uname -s)"=="Linux" ] && test -d "/proc/${PID}"; then
+ echo "WARNING: Could not adjust Gerrit's process for the kernel's out-of-memory killer."
+ echo " This may be caused by ${0} not being run as root."
+ echo " Consider changing the OOM score adjustment manually for Gerrit's PID=${PID} with e.g.:"
+ echo " echo '-1000' | sudo tee /proc/${PID}/oom_score_adj"
fi
TIMEOUT="$GERRIT_STARTUP_TIMEOUT"
