Merge "Change the structure of Submission Id"
diff --git a/Documentation/backup.txt b/Documentation/backup.txt
new file mode 100644
index 0000000..cd247af
--- /dev/null
+++ b/Documentation/backup.txt
@@ -0,0 +1,270 @@
+= Gerrit Code Review - Backup
+
+A Gerrit Code Review site contains data that needs to be backed up regularly.
+This document describes best practices for backing up review data.
+
+[#mand-backup]
+== Data which must be backed up
+
+[#mand-backup-git]
+Git repositories::
++
+The bare Git repositories managed by Gerrit are typically stored in the
+`${SITE}/git` directory. However, the locations can be customized in
+`${site}/etc/gerrit.config`. They contain the history of the respective
+projects, and since 2.15 if you are using _NoteDb_, and for 3.0 and newer,
+also change and review metadata, user accounts and groups.
++
+
+[#mand-backup-db]
+SQL database::
++
+Gerrit releases in the 2.x series store some data in the database you
+have chosen when installing Gerrit. If you are using 2.16 and have
+migrated to _NoteDb_ only the schema version is stored in the database.
++
+If you are using h2 you need to backup the `.db` files in the folder
+`${SITE}/db`.
++
+For all other database types refer to their backup documentation.
++
+Gerrit release 3.0 and newer store all primary data in _NoteDb_ inside
+the git repositories of the Gerrit site. Only the review flag marking in
+the UI when you have reviewed a changed file is stored in a relational
+database. If you are using h2 this database is named
+`account_patch_reviews.h2.db`.
+
+[#optional-backup]
+== Data optional to be backed up
+
+[#data-optional-backup-index]
+Search index::
++
+The _Lucene_ search index is stored in the `${SITE}/index` folder.
+It can be recomputed from primary data in the git repositories but
+reindexing may take a long time hence backing up the index makes sense
+for production installations.
++
+If you have chosen to use _Elastic Search_ for indexing,
+refer to its
+link:https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-snapshots.html[backup documentation].
+
+[#optional-backup-cache]
+Caches::
++
+Gerrit uses many caches which populate automatically. Some of the caches
+are persisted in the directory `${SITE}/cache` to retain the cached data
+across restarts. Since repopulating persistent caches takes time and server
+resources it makes sense to include them in backups to avoid unnecessary
+higher load and degraded performance when a Gerrit site has been restored
+from backup and caches need to be repopulated.
+
+[#optional-backup-config]
+Configuration::
++
+Gerrit configuration files are located in the directory `${SITE}/etc`
+and should be backed up or versioned in a git repository. The `etc`
+directory also contains secrets which should be handled separately
++
+* `secure.config` contains passwords and `auth.registerEmailPrivateKey`
+* public and private SSH host keys
++
+You may consider to use the
+link:https://gerrit.googlesource.com/plugins/secure-config/[secure-config plugin]
+to encrypt these secrets.
+
+[#optional-backup-plugin-data]
+Plugin Data::
++
+The `${SITE}/data/` directory is used by plugins storing data like e.g.
+the delete-project and the replication plugin.
+
+[#optional-backup-libs]
+Libraries::
++
+The `${SITE}/lib/` directory contains libraries used as statically loaded
+plugin or providing additional dependencies needed by Gerrit plugins.
+
+[#optional-backup-plugins]
+Plugins::
++
+The `${SITE}/plugins/` directory contains the installed Gerrit plugins.
+
+[#optional-backup-static]
+Static Resources::
++
+The `${SITE}/static/` directory contains static resources used to customize the
+Gerrit UI and email templates.
+
+[#optional-backup-logs]
+Logs::
++
+The `${SITE}/logs/` directory contains Gerrit server log files. Logs can still
+be written when the server is in read-only mode.
+
+[#cons-backup]
+== Consistent backups
+
+There are several ways to ensure consistency when backing up primary data.
+
+[#cons-backup-snapshot]
+=== Filesystem snapshots
+
+Gerrit 3.0 or newer::
++
+* all primary data is stored in git
+* Use a file system like lvm, zfs, btrfs or nfs supporting snapshots.
+Create a snapshot and then archive the snapshot.
+
+Gerrit 2.x::
++
+Gerrit 2.16 can use _NoteDb_ to store almost all this data which
+simplifies creating backups since consistency between database and git
+repositories is no longer critical. If you migrated to _NoteDb_ you can
+follow the backup procedure for 3.0 and higher and additionally take
+a backup of the database, which only contains the schema version,
+hence consistency between git and database is no longer critical since
+the schema version only changes during upgrade. If you didn't migrate
+to _NoteDb_ then follow the backup procedure for older 2.x Gerrit versions.
++
+Older 2.x Gerrit versions store change meta data, review comments, votes,
+accounts and group information in a SQL database. Creating consistent backups
+where git repositories and the data stored in the database are backed up
+consistently requires to turn the server read-only or to shut it down
+while creating the backup since there is no integrated transaction handling
+between git repositories and the SQL database. Also crons and currently
+running cron jobs (e.g. repacking repositories) which affect the repositories
+may need to be shut down.
+Use a file system supporting snapshots to keep the period where the gerrit
+server is read-only or down as short as possible.
+
+[#cons-backup-read-only]
+=== Turn master read-only for backup
+
+Make the server read-only before taking the backup. This means read-access
+is still available during backup, because only write operations have to be
+stopped to ensure consistency. This can be implemented using the
+link:https://gerrit.googlesource.com/plugins/readonly/[_readonly_] plugin.
+
+[#cons-backup-replicate]
+=== Replicate data for backup
+
+Replicating the git repositories can backup the most critical repository data
+but does not backup repository meta-data such as the project description
+file, ref-logs, git configs, and alternate configs.
+
+Replicate all git repositories to another file system using
+`git clone --mirror`,
+or the
+link:https://gerrit.googlesource.com/plugins/replication[replication plugin]
+or the
+link:https://gerrit.googlesource.com/plugins/pull-replication[pull-replication plugin].
+Best you use a filesystem supporting snapshots to create a backup archive
+of such a replica.
+
+For 2.x Gerrit versions also set up a database replica for the data stored in the
+SQL database. If you are using 2.16 and migrated to _NoteDb_ you may consider to
+skip setting up a database replica, instead take a backup of the database which only
+contains the current schema version in this case.
+In addition you need to ensure that no write operations are in flight before you
+take the replica offline. Otherwise the database backup might be inconsistent
+with the backup of the git repositories.
+
+Do not skip backing up the replica, the replica alone IS NOT a backup.
+Imagine someone deleted a project by mistake and this deletion got replicated.
+Replication of repository deletions can be switched off using the
+link:https://gerrit.googlesource.com/plugins/replication/+/refs/heads/master/src/main/resources/Documentation/config.md[server option]
+`remote.NAME.replicateProjectDeletions`.
+
+If you are using Gerrit replica to offload read traffic you can use one of these
+replica for creating backups.
+
+[#cons-backup-offline]
+=== Take master offline for backup
+
+Shutdown the server before taking a backup. This is simple but means downtime
+for the users. Also crons and currently running cron jobs (e.g. repacking
+repositories) which affect the repositories may need to be shut down.
+
+[#backup-methods]
+== Backup methods
+
+[#backup-methods-snapshots]
+=== Filesystem snapshots
+
+Filesystems supporting copy on write snapshots::
++
+Use a file system supporting copy-on-write snapshots like
+link:https://btrfs.wiki.kernel.org/index.php/SysadminGuide#Snapshots[btrfs]
+or
+https://wiki.debian.org/ZFS#Snapshots[zfs].
+
+
+Other filesystems supporting snapshots::
+https://wiki.archlinux.org/index.php/LVM#Snapshots[lvm] or nfs.
++
+Create a snapshot and then archive the snapshot to another storage.
++
+While snapshots are great for creating high quality backups quickly, they are
+not ideal as a format for storing backup data. Snapshots typically depend and
+reside on the same storage infrastructure as the original disk images.
+Therefore, it’s crucial that you archive these snapshots and store them
+elsewhere.
+
+3.0 or newer::
+Snapshot the complete site directory
+
+2.x::
+Similar, but the data of the database should be stored on the very same volume
+on the same machine, so that the snapshot is taken atomically over both
+the git data and the database data. Because everything should be ACID, it can safely
+crash-recover - as if the power has been plugged and the server got booted up again.
+(Actually more safe than that, because the filesystem knows about taking the snapshot,
+and also about the pending writes it can sync.)
+
+In addition to that, using filesystem snapshots allows to:
+
+* easy and fast roll back without having to access remote backup data (e.g. to restore
+accidental rm -rf git/ back in seconds).
+* incremental transfer of consistent snapshots
+* save a lot of data while still keeping multiple "known consistent states"
+
+[#backup-methods-other]
+=== Other backup methods
+
+To ensure consistent backups these backup methods require to turn the server into
+read-only mode while a backup is running.
+
+* create an archive like `tar.gz` to backup the site
+* `rsync`
+* plain old `cp`
+
+[#backup-methods-test]
+== Test backups
+
+Test backups and fire drill restoring backups to ensure the backups aren't
+corrupt or incomplete and you can restore a backup quickly.
+
+[#backup-dr]
+== Disaster recovery
+
+[#backup-dr-repl]
+=== Replicate backup archives
+
+To enable disaster recovery at least replicate backup archives to another data center.
+And fire drill restoring a new site using the backup.
+
+[#backup-dr-multi-site]
+=== Multi-site setup
+
+Use the https://gerrit.googlesource.com/plugins/multi-site[multi-site plugin]
+to install Gerrit with multiple sites installed in different datacenters
+across different regions. This ensures that in case of a severe problem with
+one of the sites, the other sites can still serve your repositories.
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index 925d399..436b918 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -304,6 +304,12 @@
bazel test --test_tag_filters=-docker //...
----
+To exclude tests that require very recent git client version:
+
+----
+ bazel test --test_tag_filters=-git-protocol-v2 //...
+----
+
To ignore cached test results:
----
@@ -324,6 +330,7 @@
* edit
* elastic
* git
+* git-protocol-v2
* notedb
* pgm
* rest
diff --git a/Documentation/images/user-review-ui-change-complex-reply-dialogue.png b/Documentation/images/user-review-ui-change-complex-reply-dialogue.png
new file mode 100644
index 0000000..1286852
--- /dev/null
+++ b/Documentation/images/user-review-ui-change-complex-reply-dialogue.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-page-download.png b/Documentation/images/user-review-ui-change-page-download.png
new file mode 100644
index 0000000..63c4ee3
--- /dev/null
+++ b/Documentation/images/user-review-ui-change-page-download.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-page-patchset-dropdown.png b/Documentation/images/user-review-ui-change-page-patchset-dropdown.png
new file mode 100644
index 0000000..f71473e
--- /dev/null
+++ b/Documentation/images/user-review-ui-change-page-patchset-dropdown.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-relation-chain.png b/Documentation/images/user-review-ui-change-relation-chain.png
new file mode 100644
index 0000000..19942f1
--- /dev/null
+++ b/Documentation/images/user-review-ui-change-relation-chain.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-reply-dialogue.png b/Documentation/images/user-review-ui-change-reply-dialogue.png
new file mode 100644
index 0000000..3c95852
--- /dev/null
+++ b/Documentation/images/user-review-ui-change-reply-dialogue.png
Binary files differ
diff --git a/Documentation/index.txt b/Documentation/index.txt
index 77e0ed4..63671cd 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -9,6 +9,7 @@
. link:intro-quick.html[Product Overview]
. link:intro-how-gerrit-works.html[How Gerrit Works]
. link:intro-gerrit-walkthrough.html[Basic Gerrit Walkthrough]
+.. link:intro-gerrit-walkthrough-github.html[Basic Gerrit Walkthrough -- For GitHub Users]
. link:dev-community.html[Gerrit Community]
.. link:dev-contributing.html[Contributor Guide]
diff --git a/Documentation/install.txt b/Documentation/install.txt
index 2b6cc6e..fe32029 100644
--- a/Documentation/install.txt
+++ b/Documentation/install.txt
@@ -249,6 +249,11 @@
* http://www.kernel.org/pub/software/scm/git/docs/git-daemon.html[git-daemon]
+[[backup]]
+== Backup
+
+See the link:backup.html[backup documentation].
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/intro-gerrit-walkthrough-github.txt b/Documentation/intro-gerrit-walkthrough-github.txt
new file mode 100644
index 0000000..39a779d
--- /dev/null
+++ b/Documentation/intro-gerrit-walkthrough-github.txt
@@ -0,0 +1,263 @@
+= Basic Gerrit Walkthrough -- For GitHub Users
+
+
+[NOTE]
+====
+This document aims to provide a concise description of the core principles of
+code review in Gerrit for people that were previously using Pull Requests on
+Github or similar concepts. Nothing in this document is meant to state that
+one or the other might be better, but only aims to help new users understand
+Gerrit more readily. We use Github as the point of comparison since it seems
+to be the most popular service.
+====
+
+To illustrate the differences in a meaningful order, we will walk you through
+the process of cloning a repo, making a change, asking for code review,
+iterating on the code and finally having it submitted to the code base. This
+document also does not aim to describe all features of Gerrit. Please refer to
+the link:intro-gerrit-walkthrough.html[Basic Gerrit Walkthrough] or
+link:index.html[the rest of the documentation] for a more complete overview and additional pointers.
+
+[[tldr]]
+== tl;dr
+
+Here’s how getting code reviewed and submitted with Gerrit is different from
+doing the same with GitHub:
+
+* You need the add a commit-msg hook script when you clone a repo for the first
+time using a snippet you can find e.g. https://gerrit-review.googlesource.com/admin/repos/gerrit[here];
+* Your review will be on a single commit instead of a branch. You use
+`git commit --amend` to modify a code change.
+* Instead of using the Web UI to create a pull request, you use
+`git push origin HEAD:refs/for/master` to upload new local commits that are
+ready for review to Gerrit. You will find the URL to the review in the output of
+the push command.
+* As a reviewer, Gerrit offers a number of so-called labels to vote on, one of
+which is Code-Review. You indicate a negative, neutral or positive review using
+a -1, 0 or +1 vote.
+* To be able to submit (== merge) a change, you usually need a +2 Code-Review
+vote and possibly additional positive votes, depending on the configuration of
+the project you are contributing to.
+
+[[clone]]
+== 1. Cloning a Repository
+
+[NOTE]
+====
+Both GitHub and Gerrit provide simple Git repository hosting (of course both can
+do much more). In the simplest setup, you could just use both as such without
+any code review to push code. We will assume that this is not what you want to
+do and focus on the use case where your change requires a review.
+====
+
+The first step to working with the code is to clone the repo. For both, Gerrit
+and GitHub, you can simply use the `git clone` command.
+
+For Gerrit, there is an additional step before you can start making changes. For
+reasons we explain below, you’ll have to add a https://gerrit-review.googlesource.com/Documentation/user-changeid.html[commit-msg hook] script. This will
+append the Gerrit Change-Id to every commit message such that Gerrit can track
+commits through the review process. To make this process a little easier in
+Gerrit, you can find a command snippet for cloning and adding the commit-msg
+hook on the repository page (e.g. https://gerrit-review.googlesource.com/admin/repos/gerrit[here]).
+
+[[create-change]]
+== 2. Making a Change
+
+*Branches*
+
+Now that you have the code in the git repo on your machine, you can start making
+changes. With GitHub, you would usually create a new branch and then start
+committing to it. This branch would then contain all the changes you share with
+your code reviewers in the next step. Your local branch will usually also be
+pushed to the remote server. This can be handy to back up your work or hand-off
+work to another device or developer.
+
+With Gerrit, you can also create a new local branch to develop in. While not
+required, it can be considered a best practice to sandbox this change from other
+changes you might be making. In contrast to the GitHub model, your local branch
+will not have to be pushed to the remote in Gerrit, at least not for the
+purposes of code review.
+
+*Commits*
+In Gerrit, a single commit is the unit of code that will be reviewed. With
+GitHub, you can commit to your branch as much as you like and the sum of all
+your commits on that branch will get reviewed. As a single commit gets reviewed
+in Gerrit, you need to `git commit --amend` when you iterate on the same change as
+opposed to only using `git commit` with GitHub (see Section 5 for more). You can,
+however, also add another commit on top of your existing commit in Gerrit, which
+will create a second change (and thus another review) that is based on your
+first change. Gerrit will show the relationship between these two changes as a
+so-called relation chain. This also means that your second change can only be
+submitted after the first was successfully merged. In many basic use cases, this
+situation is however not what you want.
+
+image::images/user-review-ui-change-relation-chain.png[Relation chain display on the change page.]
+
+With GitHub, you may be pushing your branch to the remote for non-code-review
+purposes, as mentioned above. You usually do not do this with Gerrit, as
+Gerrit-managed repos often only have one or a few branches on the server that
+can only be merged into via code review.
+
+[[request-review]]
+== 3. Asking for Code Review
+
+After you are satisfied with the changes you made, you’ll usually want/need to
+get your code reviewed. In GitHub, you would push your branch to the remote, go
+to the Web UI and create a pull request. In Gerrit, you need to push your commit
+(or the series of changes/commits) to the remote first, since you usually
+develop in a local branch only. While you can often just use git push with
+GitHub, you need to do a slightly different thing for Gerrit. Gerrit uses a
+“magic” branch that tells the server that this code is supposed to be reviewed.
+To send the changes you made on your local branch to review and being eventually
+merged into the remote’s master branch, you use
+`git push origin HEAD:refs/for/master`. There are also link:user-upload.html#_git_push[a number of Gerrit change
+options] you can trigger from the CLI this way.
+
+After successfully pushing your change to Gerrit, you will already find the URL
+for viewing your change in Gerrit’s Web UI in the response you get from the
+server. The description of the Gerrit code review that was just created is equal
+to the commit message of that one commit the change is based on. In GitHub, you
+might have described your change in the message you can create when creating the
+pull request in the GitHub Web UI.
+
+Next, you would go and visit your Gerrit change in the Web UI to get your change
+ready for review (choose reviewers, cc people, check for failing CI builds or
+tests, etc.), very similar to what you do on Github. Reviewers will be notified
+via email once you add them. By default, anyone can add reviewers to a Gerrit
+change. In GitHub, this ability is reserved for certain users, so you may have
+relied on others adding reviewers for you before. This can be the case in a
+Gerrit project, but it is also often expected that the change owner (usually the
+creator of the change) adds reviewers to get the review process started.
+
+[[reviewing]]
+== 4. Reviewing a Change
+
+Switching perspectives briefly, reviewing a change is fairly similar between
+GitHub and Gerrit. You, as a reviewer, will be notified of a change you have
+been added to via email or see an “incoming” change on your Gerrit dashboard.
+The dashboard is the central overview of changes going on within a Gerrit
+instance. By default, the dashboard shows changes that you are involved in, in
+any way. You can also see all changes on a Gerrit server by using the top menu
+(“Changes” -> “Open”). This view is more similar to what you see on Github, when
+you navigate to the Pull Requests tab of the project/repository you are working
+on. Note, however, that a single Gerrit instance can host multiple projects
+(also referred to as repositories; a list can be found, for example, https://gerrit-review.googlesource.com/admin/repos[here]). Your
+dashboard and other lists of changes will show all changes across the
+projects/repositories by default.
+
+Back to your dashboard, you can click on the change you want to review. You can
+also access this from the email you received. You will see the same view that
+you saw as an author. In the middle of the change page, you can find the list of
+files that have been modified, just like what you find in the “Files changed”
+tab of GitHub. Also similarly, you can leave comments by highlighting a piece of
+the code and pressing ‘c’. All comments you make are in a draft state and thus
+only visible to you, like on GitHub. When you are done with your review, you
+need to click the “Reply” button at the top of the change page to send your
+assessment to the change owner alongside a “change message” summarizing your
+findings and/or adding higher level comments. Replying to a change makes your
+draft comments and the change message visible on the change page for everyone
+that has view access to this change. This again is fairly similar to GitHub,
+except for Gerrit’s voting labels.
+
+image::images/user-review-ui-change-reply-dialogue.png[Reply dialogue for a Gerrit change.]
+
+As you can see in the screenshot of the reply dialogue, the voting labels are in
+the bottom part of the dialogue. They can be fairly simple as in this case, but
+there can also be a larger number of labels you might be able to vote on. Labels
+can be used to distinguish different aspects of a review (e.g. whether or not
+the licensing of included libraries is okay), outcome of CI systems (e.g.
+whether or not a format checker passed, a build completed successfully, etc.) or
+as a flag that is read by bots to do something with a change. An example of a
+more complex label setup can be seen in this screenshot from the Android Gerrit
+instance.
+
+image::images/user-review-ui-change-complex-reply-dialogue.png[Reply dialogue for a change on the Android project.]
+
+In the simplest case shown above, voting -1 on the Code-Review label equals
+requesting changes on a GitHub pull request, 0 equals just having comments and
++1 means that you think this change looks good. Usually, Gerrit changes require
+a +2 vote on the Code-Review label to be submitted (merged in GitHub terms, see
+Section 6 below). Being able to vote +2 on Code-Review is often restricted to
+maintainers of a given project, so they can have a final say on a change. These
+practices can however vary between projects, as labels and voting permissions
+are configurable.
+
+[[iterate]]
+== 5. Iterating on the Change
+
+After your reviewers got back to you as a change owner, you realize that you
+need to make a few updates to the code in your change. As mentioned in Section 2
+(Making a Change), you’ll have to amend the commit that this review was based
+on. To do that, you might have to checkout the respective commit first if it is
+not at the tip of your local branch, for example if you stacked multiple changes
+on top of each other. Another common use case is to not have a local branch but
+to work in the so-called https://www.git-tower.com/learn/git/faq/detached-head-when-checkout-commit["detached HEAD"] mode. In that case you can use the
+“Download” button on the files tab to copy a command that fetches and checks out
+the commit underlying your change. Make sure to select the latest patchset,
+though!
+
+image::images/user-review-ui-change-page-download.png[Using the “Download” button to copy a command that checks out a given patchset for a change.]
+
+After checking out the commit, you then make the changes as usual. When you
+think you are done, you can commit with the `--amend` flag to change the commit
+you currently have checked out.
+
+When you `git commit --amend` to iterate on your change, you might be worried that
+you are changing your previous commit and may thus lose that state of your work.
+However, here the Change-Id appended to your commit message comes into play.
+While the SHA1 hash of your change (the commit ID used by Git) might change, the
+Change-Id stays the same (in fact it is the SHA1 hash of the very first version
+of that commit). When this amended commit is uploaded to the Gerrit server,
+Gerrit knows that this commit is really an iteration of that previous commit
+(and the associated review) and will preserve both, the old and the new state.
+All previous states of your commit will be visible in the Gerrit UI as so-called
+patchsets (and link:intro-user.html#change-ref[from the Git repo]).
+
+image::images/user-review-ui-change-page-patchset-dropdown.png[Screenshot of the patchset dropdown above the file list, showing all iterations a commit went through.]
+
+After iterating as much as needed, your reviewers will finally be satisfied.
+With GitHub, you would have a string of additional commits in the branch you
+used for opening the pull request. In Gerrit, you still only have that one
+commit in your local branch. All the iterations are available as patchsets in
+the Web UI as well as from the special branch mentioned above.
+
+[[submit]]
+== 6. Submitting a Change
+
+Finally, it is time to submit your change. As mentioned above, the precondition
+for this in Gerrit is usually at least a +2 vote on the Code-Review label. With
+GitHub, an authorized person must have given an “Approve” vote. Once this
+precondition has been met, anyone with submit permission can submit the change
+in Gerrit. To do that, you click the “Submit” button in the Gerrit Web UI just
+as you would click the “Merge Pull Request” button in GitHub. Both, Gerrit and
+GitHub, allow different merge strategies, that can be enabled by project
+administrators. In Gerrit, a merge strategy is configured for each project and
+cannot be changed at submit time while this may be possible with GitHub,
+depending on project configuration.
+
+A merge can fail due to conflicts with competing edits on the target branch.
+With GitHub, you may be able to resolve some simple conflicts directly from the
+Web UI. In Gerrit, you can attempt to rebase a change from the Web UI. If there
+are no conflicts, a new patchset will automatically appear. Otherwise, similar
+to GitHub, you need to resolve conflicts on the command line with your local
+clone of the repository. While you resolve conflicts that arise from a
+`git merge` for GitHub, you will need to link:intro-user.html#rebase[use `git rebase` with your change] on
+Gerrit.
+
+After resolving locally, with GitHub, you end up with another commit on your
+pull request branch and push it to the server, which should then allow you to
+finish merging the change. With Gerrit, resolving the conflict through rebasing
+your commit/change results in another amended version of that same commit and
+you upload it to Gerrit, resulting in a new patchset just like your previous
+iterations addressing reviewer comments. This new patchset will usually require
+another round of reviewer votes, as Gerrit will not copy votes from a previous
+patchset by default.
+
+
+GERRIT
+------
+
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt
index b13ae83..16929ae 100644
--- a/Documentation/intro-user.txt
+++ b/Documentation/intro-user.txt
@@ -538,7 +538,9 @@
----
$ git push origin HEAD:refs/for/master%wip
----
-Alternatively, click *WIP* from the Change screen.
+Alternatively, click *WIP* from the *More* menu on the Change screen.
+The Change screen updates with a yellow header, indicating that
+the change is in a Work-in-Progress state.
To mark the change as ready for review, append `%ready` to your push
request.
@@ -546,18 +548,12 @@
----
$ git push origin HEAD:refs/for/master%ready
----
-Alternatively, click *Ready* from the Change screen.
+Alternatively, click *Start Review* from the Change screen.
Change owners, project owners, site administrators and members of a group that
was granted "Toggle Work In Progress state" permission can mark changes as
`work-in-progress` and `ready`.
-[[wip-polygerrit]]
-In the new PolyGerrit UI, you can mark a change as WIP, by selecting *WIP* from
-the *More* menu. The Change screen updates with a yellow header, indicating that
-the change is in a Work-in-Progress state. To mark a change as ready for review,
-click *Start Review*.
-
[[private-changes]]
== Private Changes
diff --git a/Documentation/js-api.txt b/Documentation/js-api.txt
index 030541d..893ab36 100644
--- a/Documentation/js-api.txt
+++ b/Documentation/js-api.txt
@@ -150,6 +150,13 @@
parameter the URL of the plugin is returned. If passed a string
the argument is appended to the plugin URL.
+A plugin's URL is where this plugin is loaded, it doesn't
+necessary to be the same as the Gerrit host. Use `window.location`
+if you need to access the Gerrit host info.
+
+For preloaded plugins, the plugin url is based on a global
+configuration of where to load all plugins, default to current host.
+
[source,javascript]
----
self.url(); // "https://gerrit-review.googlesource.com/plugins/demo/"
diff --git a/Jenkinsfile b/Jenkinsfile
index f21c7897..988f839 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -88,6 +88,10 @@
}
}
+def hasChangeNumber() {
+ env.GERRIT_CHANGE_NUMBER?.trim()
+}
+
def postCheck(check) {
def gerritPostUrl = Globals.gerritUrl +
"a/changes/${check.changeNum}/revisions/${check.sha1}/checks"
@@ -185,17 +189,17 @@
def collectBuilds() {
def builds = [:]
- if (env.GERRIT_CHANGE_NUMBER == "") {
+ if (hasChangeNumber()) {
+ builds["Gerrit-codestyle"] = prepareBuildsForMode("Gerrit-codestyle")
+ Builds.modes.each {
+ builds["Gerrit-verification(${it})"] = prepareBuildsForMode("Gerrit-verifier-bazel", it)
+ }
+ } else {
builds["java8"] = { -> build "Gerrit-bazel-${env.BRANCH_NAME}" }
if (env.BRANCH_NAME == "master") {
builds["java11"] = { -> build "Gerrit-bazel-java11-${env.BRANCH_NAME}" }
}
- } else {
- builds["Gerrit-codestyle"] = prepareBuildsForMode("Gerrit-codestyle")
- Builds.modes.each {
- builds["Gerrit-verification(${it})"] = prepareBuildsForMode("Gerrit-verifier-bazel", it)
- }
}
return builds
}
@@ -282,7 +286,7 @@
node ('master') {
- if (env.GERRIT_CHANGE_NUMBER != "") {
+ if (hasChangeNumber()) {
stage('Preparing'){
gerritReview labels: ['Verified': 0, 'Code-Style': 0]
@@ -293,7 +297,7 @@
parallel(collectBuilds())
- if (env.GERRIT_CHANGE_NUMBER != "") {
+ if (hasChangeNumber()) {
stage('Retry Flaky Builds'){
def flakyBuildsModes = findFlakyBuilds()
if (flakyBuildsModes.size() > 0){
diff --git a/WORKSPACE b/WORKSPACE
index 307e28c..0804bec 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -32,9 +32,9 @@
http_archive(
name = "io_bazel_rules_closure",
- sha256 = "0409f8bd2a8b6fd1db289cdc0acb394dafd69f60a86d0169bc6495e648e01587",
- strip_prefix = "rules_closure-18f8acf24ae0d03a9c3ee872ff91dcfbf383d69e",
- urls = ["https://github.com/bazelbuild/rules_closure/archive/18f8acf24ae0d03a9c3ee872ff91dcfbf383d69e.tar.gz"],
+ sha256 = "03c3b16f205085817fd89cfdcb2220a0138647ee7992be9cef291b069dd90301",
+ strip_prefix = "rules_closure-196a45f0ede2faec11dcc6c60fbc5e7471f4bd58",
+ urls = ["https://github.com/bazelbuild/rules_closure/archive/196a45f0ede2faec11dcc6c60fbc5e7471f4bd58.tar.gz"],
)
# File is specific to Polymer and copied from the Closure Github -- should be
diff --git a/java/com/google/gerrit/common/data/SubmitTypeRecord.java b/java/com/google/gerrit/common/data/SubmitTypeRecord.java
index d16da96..afb3bac 100644
--- a/java/com/google/gerrit/common/data/SubmitTypeRecord.java
+++ b/java/com/google/gerrit/common/data/SubmitTypeRecord.java
@@ -65,7 +65,7 @@
StringBuilder sb = new StringBuilder();
sb.append(status);
if (status == Status.RULE_ERROR && errorMessage != null) {
- sb.append('(').append(errorMessage).append(")");
+ sb.append(" (").append(errorMessage).append(")");
}
if (type != null) {
sb.append('[');
diff --git a/java/com/google/gerrit/exceptions/InvalidMergeStrategyException.java b/java/com/google/gerrit/exceptions/InvalidMergeStrategyException.java
new file mode 100644
index 0000000..d9c5776
--- /dev/null
+++ b/java/com/google/gerrit/exceptions/InvalidMergeStrategyException.java
@@ -0,0 +1,23 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.exceptions;
+
+public class InvalidMergeStrategyException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+
+ public InvalidMergeStrategyException(String strategy) {
+ super("invalid merge strategy: " + strategy);
+ }
+}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 0535397..bd32c0b 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1670,18 +1670,37 @@
if (!res.isCommitted()) {
res.reset();
traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
- StringBuilder msg = new StringBuilder("Internal server error");
ImmutableList<String> userMessages =
globals.exceptionHooks.stream()
.map(h -> h.getUserMessage(err))
.filter(Optional::isPresent)
.map(Optional::get)
.collect(toImmutableList());
+
+ Optional<Integer> statusCode =
+ globals.exceptionHooks.stream()
+ .map(h -> h.getStatusCode(err))
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .findFirst();
+ if (statusCode.isPresent() && statusCode.get() < 400) {
+ StringBuilder msg = new StringBuilder();
+ if (userMessages.size() == 1) {
+ msg.append(userMessages.get(0));
+ } else {
+ userMessages.forEach(m -> msg.append("\n* ").append(m));
+ }
+
+ res.setStatus(statusCode.get());
+ logger.atFinest().withCause(err).log("REST call finished: %d", statusCode.get().intValue());
+ return replyText(req, res, true, msg.toString());
+ }
+
+ StringBuilder msg = new StringBuilder("Internal server error");
if (!userMessages.isEmpty()) {
- msg.append("\n");
userMessages.forEach(m -> msg.append("\n* ").append(m));
}
- return replyError(req, res, SC_INTERNAL_SERVER_ERROR, msg.toString(), err);
+ return replyError(req, res, statusCode.orElse(SC_INTERNAL_SERVER_ERROR), msg.toString(), err);
}
return 0;
}
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 568fb60..da8411f 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -538,14 +538,14 @@
private Injector createWebInjector() {
final List<Module> modules = new ArrayList<>();
- if (sshd) {
- modules.add(new ProjectQoSFilter.Module());
- }
modules.add(RequestContextFilter.module());
modules.add(RequestMetricsFilter.module());
modules.add(H2CacheBasedWebSession.module());
modules.add(sysInjector.getInstance(GerritAuthModule.class));
modules.add(sysInjector.getInstance(GitOverHttpModule.class));
+ if (sshd) {
+ modules.add(new ProjectQoSFilter.Module());
+ }
modules.add(AllRequestFilter.module());
modules.add(sysInjector.getInstance(WebModule.class));
modules.add(sysInjector.getInstance(RequireSslFilter.Module.class));
diff --git a/java/com/google/gerrit/prettify/BUILD b/java/com/google/gerrit/prettify/BUILD
index 7c1241a..0a15fda 100644
--- a/java/com/google/gerrit/prettify/BUILD
+++ b/java/com/google/gerrit/prettify/BUILD
@@ -7,5 +7,7 @@
deps = [
"//lib:guava",
"//lib:jgit",
+ "//lib/auto:auto-value",
+ "//lib/auto:auto-value-annotations",
],
)
diff --git a/java/com/google/gerrit/prettify/common/SparseFileContent.java b/java/com/google/gerrit/prettify/common/SparseFileContent.java
index 348f9b2..1249b65 100644
--- a/java/com/google/gerrit/prettify/common/SparseFileContent.java
+++ b/java/com/google/gerrit/prettify/common/SparseFileContent.java
@@ -14,160 +14,175 @@
package com.google.gerrit.prettify.common;
-import java.util.ArrayList;
-import java.util.List;
+import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
-public class SparseFileContent {
- protected List<Range> ranges;
- protected int size;
+/**
+ * A class to store subset of a file's lines in a memory efficient way. Internally, it stores lines
+ * as a list of ranges. Each range represents continuous set of lines and has information about line
+ * numbers in original file (zero-based).
+ *
+ * <p>{@link SparseFileContent.Accessor} must be used to work with the stored content.
+ */
+@AutoValue
+public abstract class SparseFileContent {
+ abstract ImmutableList<Range> getRanges();
- private transient int currentRangeIdx;
+ public abstract int getSize();
- public SparseFileContent() {
- ranges = new ArrayList<>();
+ public static SparseFileContent create(ImmutableList<Range> ranges, int size) {
+ return new AutoValue_SparseFileContent(ranges, size);
}
- public int size() {
- return size;
+ @VisibleForTesting
+ public int getRangesCount() {
+ return getRanges().size();
}
- public void setSize(int s) {
- size = s;
+ public Accessor createAccessor() {
+ return new Accessor(this);
}
- public String get(int idx) {
- final String line = getLine(idx);
- if (line == null) {
- throw new ArrayIndexOutOfBoundsException(idx);
- }
- return line;
- }
+ /**
+ * Provide a methods to work with the content of a {@link SparseFileContent}.
+ *
+ * <p>The class hides internal representation of a {@link SparseFileContent} and provides
+ * convenient way for accessing a content.
+ */
+ public static class Accessor {
+ private final SparseFileContent content;
+ private int currentRangeIdx;
- public boolean contains(int idx) {
- return getLine(idx) != null;
- }
-
- public int first() {
- return ranges.isEmpty() ? size() : ranges.get(0).base;
- }
-
- public int next(int idx) {
- // Most requests are sequential in nature, fetching the next
- // line from the current range, or the immediate next range.
- //
- int high = ranges.size();
- if (currentRangeIdx < high) {
- Range cur = ranges.get(currentRangeIdx);
- if (cur.contains(idx + 1)) {
- return idx + 1;
- }
-
- if (++currentRangeIdx < high) {
- // Its not plus one, its the base of the next range.
- //
- return ranges.get(currentRangeIdx).base;
- }
+ private Accessor(SparseFileContent content) {
+ this.content = content;
}
- // Binary search for the current value, since we know its a sorted list.
- //
- int low = 0;
- do {
- final int mid = (low + high) / 2;
- final Range cur = ranges.get(mid);
+ public String get(int idx) {
+ final String line = getLine(idx);
+ if (line == null) {
+ throw new ArrayIndexOutOfBoundsException(idx);
+ }
+ return line;
+ }
- if (cur.contains(idx)) {
+ public int getSize() {
+ return content.getSize();
+ }
+
+ public boolean contains(int idx) {
+ return getLine(idx) != null;
+ }
+
+ public int first() {
+ return content.getRanges().isEmpty() ? getSize() : content.getRanges().get(0).getBase();
+ }
+
+ public int next(int idx) {
+ // Most requests are sequential in nature, fetching the next
+ // line from the current range, or the immediate next range.
+ //
+ ImmutableList<Range> ranges = content.getRanges();
+ int high = ranges.size();
+ if (currentRangeIdx < high) {
+ Range cur = ranges.get(currentRangeIdx);
if (cur.contains(idx + 1)) {
- // Trivial plus one case above failed due to wrong currentRangeIdx.
- // Reset the cache so we don't miss in the future.
- //
- currentRangeIdx = mid;
return idx + 1;
}
- if (mid + 1 < ranges.size()) {
- // Its the base of the next range.
- currentRangeIdx = mid + 1;
- return ranges.get(currentRangeIdx).base;
- }
-
- // No more lines in the file.
- //
- return size();
- }
-
- if (idx < cur.base) {
- high = mid;
- } else {
- low = mid + 1;
- }
- } while (low < high);
-
- return size();
- }
-
- private String getLine(int idx) {
- // Most requests are sequential in nature, fetching the next
- // line from the current range, or the next range.
- //
- int high = ranges.size();
- if (currentRangeIdx < high) {
- Range cur = ranges.get(currentRangeIdx);
- if (cur.contains(idx)) {
- return cur.get(idx);
- }
-
- if (++currentRangeIdx < high) {
- final Range next = ranges.get(currentRangeIdx);
- if (next.contains(idx)) {
- return next.get(idx);
+ if (++currentRangeIdx < high) {
+ // Its not plus one, its the base of the next range.
+ //
+ return ranges.get(currentRangeIdx).getBase();
}
}
+
+ // Binary search for the current value, since we know its a sorted list.
+ //
+ int low = 0;
+ do {
+ final int mid = (low + high) / 2;
+ final Range cur = ranges.get(mid);
+
+ if (cur.contains(idx)) {
+ if (cur.contains(idx + 1)) {
+ // Trivial plus one case above failed due to wrong currentRangeIdx.
+ // Reset the cache so we don't miss in the future.
+ //
+ currentRangeIdx = mid;
+ return idx + 1;
+ }
+
+ if (mid + 1 < ranges.size()) {
+ // Its the base of the next range.
+ currentRangeIdx = mid + 1;
+ return ranges.get(currentRangeIdx).getBase();
+ }
+
+ // No more lines in the file.
+ //
+ return getSize();
+ }
+
+ if (idx < cur.getBase()) {
+ high = mid;
+ } else {
+ low = mid + 1;
+ }
+ } while (low < high);
+
+ return getSize();
}
- // Binary search for the range, since we know its a sorted list.
- //
- if (ranges.isEmpty()) {
+ private String getLine(int idx) {
+ // Most requests are sequential in nature, fetching the next
+ // line from the current range, or the next range.
+ //
+ ImmutableList<Range> ranges = content.getRanges();
+ int high = ranges.size();
+ if (currentRangeIdx < high) {
+ Range cur = ranges.get(currentRangeIdx);
+ if (cur.contains(idx)) {
+ return cur.get(idx);
+ }
+
+ if (++currentRangeIdx < high) {
+ final Range next = ranges.get(currentRangeIdx);
+ if (next.contains(idx)) {
+ return next.get(idx);
+ }
+ }
+ }
+
+ // Binary search for the range, since we know its a sorted list.
+ //
+ if (ranges.isEmpty()) {
+ return null;
+ }
+
+ int low = 0;
+ do {
+ final int mid = (low + high) / 2;
+ final Range cur = ranges.get(mid);
+ if (cur.contains(idx)) {
+ currentRangeIdx = mid;
+ return cur.get(idx);
+ }
+ if (idx < cur.getBase()) {
+ high = mid;
+ } else {
+ low = mid + 1;
+ }
+ } while (low < high);
return null;
}
-
- int low = 0;
- do {
- final int mid = (low + high) / 2;
- final Range cur = ranges.get(mid);
- if (cur.contains(idx)) {
- currentRangeIdx = mid;
- return cur.get(idx);
- }
- if (idx < cur.base) {
- high = mid;
- } else {
- low = mid + 1;
- }
- } while (low < high);
- return null;
- }
-
- public void addLine(int i, String content) {
- final Range r;
- if (!ranges.isEmpty() && i == last().end()) {
- r = last();
- } else {
- r = new Range(i);
- ranges.add(r);
- }
- r.lines.add(content);
- }
-
- private Range last() {
- return ranges.get(ranges.size() - 1);
}
@Override
- public String toString() {
+ public final String toString() {
final StringBuilder b = new StringBuilder();
b.append("SparseFileContent[\n");
- for (Range r : ranges) {
+ for (Range r : getRanges()) {
b.append(" ");
b.append(r.toString());
b.append('\n');
@@ -176,33 +191,32 @@
return b.toString();
}
- static class Range {
- protected int base;
- protected List<String> lines;
-
- private Range(int b) {
- base = b;
- lines = new ArrayList<>();
+ @AutoValue
+ abstract static class Range {
+ static Range create(int base, ImmutableList<String> lines) {
+ return new AutoValue_SparseFileContent_Range(base, lines);
}
- protected Range() {}
+ abstract int getBase();
+
+ abstract ImmutableList<String> getLines();
private String get(int i) {
- return lines.get(i - base);
+ return getLines().get(i - getBase());
}
private int end() {
- return base + lines.size();
+ return getBase() + getLines().size();
}
private boolean contains(int i) {
- return base <= i && i < end();
+ return getBase() <= i && i < end();
}
@Override
- public String toString() {
+ public final String toString() {
// Usage of [ and ) is intentional to denote inclusive/exclusive range
- return "Range[" + base + "," + end() + ")";
+ return "Range[" + getBase() + "," + end() + ")";
}
}
}
diff --git a/java/com/google/gerrit/prettify/common/SparseFileContentBuilder.java b/java/com/google/gerrit/prettify/common/SparseFileContentBuilder.java
new file mode 100644
index 0000000..04fb5d1
--- /dev/null
+++ b/java/com/google/gerrit/prettify/common/SparseFileContentBuilder.java
@@ -0,0 +1,90 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.prettify.common;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.prettify.common.SparseFileContent.Range;
+
+/**
+ * A builder for creating immutable {@link SparseFileContent}. Lines can be only be added in
+ * sequential (increased) order
+ */
+public class SparseFileContentBuilder {
+ private final ImmutableList.Builder<Range> ranges;
+ private final int size;
+ private int lastRangeBase;
+ private int lastRangeEnd;
+ private ImmutableList.Builder<String> lastRangeLines;
+
+ public SparseFileContentBuilder(int size) {
+ ranges = new ImmutableList.Builder<>();
+ startNextRange(0);
+ this.size = size;
+ }
+
+ public void addLine(int lineNumber, String content) {
+ if (lineNumber < 0) {
+ throw new IllegalArgumentException("Line number must be non-negative");
+ }
+ // if (lineNumber >= size) {
+ // The following 4 tests are failed if you uncomment this condition:
+ //
+ //
+ // diffOfFileWithMultilineRebaseHunkRemovingNewlineAtEndOfFileAndWithCommentReturnsFileContents
+ //
+ // diffOfFileWithMultilineRebaseHunkAddingNewlineAtEndOfFileAndWithCommentReturnsFileContents
+ //
+ //
+ // diffOfFileWithMultilineRebaseHunkRemovingNewlineAtEndOfFileAndWithCommentReturnsFileContents
+ //
+ // diffOfFileWithMultilineRebaseHunkAddingNewlineAtEndOfFileAndWithCommentReturnsFileContents
+ // Tests are failed because there are some bug with diff calculation.
+ // The condition must be uncommented after all these bugs are fixed.
+ // Also don't forget to remove ignore from for SparseFileContentBuilder
+ // throw new IllegalArgumentException(String.format("The zero-based line number %d is after
+ // the end of file. The file size is %d line(s).", lineNumber, size));
+ // }
+ if (lineNumber < lastRangeEnd) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Invalid line number %d. You are trying to add a line before an already added line"
+ + " %d",
+ lineNumber, lastRangeEnd));
+ }
+ if (lineNumber > lastRangeEnd) {
+ finishLastRange();
+ startNextRange(lineNumber);
+ }
+ lastRangeLines.add(content);
+ lastRangeEnd++;
+ }
+
+ private void startNextRange(int base) {
+ lastRangeLines = new ImmutableList.Builder<>();
+ lastRangeBase = lastRangeEnd = base;
+ }
+
+ private void finishLastRange() {
+ if (lastRangeEnd > lastRangeBase) {
+ ranges.add(Range.create(lastRangeBase, lastRangeLines.build()));
+ lastRangeLines = null;
+ }
+ }
+
+ public SparseFileContent build() {
+ finishLastRange();
+ return SparseFileContent.create(ranges.build(), size);
+ }
+}
diff --git a/java/com/google/gerrit/prettify/common/testing/BUILD b/java/com/google/gerrit/prettify/common/testing/BUILD
new file mode 100644
index 0000000..5057fdb
--- /dev/null
+++ b/java/com/google/gerrit/prettify/common/testing/BUILD
@@ -0,0 +1,14 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+package(default_testonly = True)
+
+java_library(
+ name = "testing",
+ srcs = glob(["*.java"]),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//java/com/google/gerrit/prettify:server",
+ "//lib:guava",
+ "//lib/truth",
+ ],
+)
diff --git a/java/com/google/gerrit/prettify/common/testing/SparseFileContentSubject.java b/java/com/google/gerrit/prettify/common/testing/SparseFileContentSubject.java
new file mode 100644
index 0000000..c1fe1ec
--- /dev/null
+++ b/java/com/google/gerrit/prettify/common/testing/SparseFileContentSubject.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.prettify.common.testing;
+
+import static com.google.common.truth.Truth.assertAbout;
+
+import com.google.common.truth.FailureMetadata;
+import com.google.common.truth.IntegerSubject;
+import com.google.common.truth.MapSubject;
+import com.google.common.truth.Subject;
+import com.google.gerrit.prettify.common.SparseFileContent;
+import java.util.HashMap;
+import java.util.Map;
+
+public class SparseFileContentSubject extends Subject {
+ public static SparseFileContentSubject assertThat(SparseFileContent sparseFileContent) {
+ return assertAbout(sparseFileContent()).that(sparseFileContent);
+ }
+
+ private final SparseFileContent sparseFileContent;
+
+ private SparseFileContentSubject(FailureMetadata metadata, SparseFileContent actual) {
+ super(metadata, actual);
+ this.sparseFileContent = actual;
+ }
+
+ private static Subject.Factory<SparseFileContentSubject, SparseFileContent> sparseFileContent() {
+ return SparseFileContentSubject::new;
+ }
+
+ public IntegerSubject getSize() {
+ isNotNull();
+ return check("size()").that(sparseFileContent.getSize());
+ }
+
+ public IntegerSubject getRangesCount() {
+ isNotNull();
+ return check("rangesCount()").that(sparseFileContent.getRangesCount());
+ }
+
+ public MapSubject lines() {
+ isNotNull();
+ Map<Integer, String> lines = new HashMap<>();
+ SparseFileContent.Accessor accessor = sparseFileContent.createAccessor();
+ int size = accessor.getSize();
+ int idx = accessor.first();
+ while (idx < size) {
+ lines.put(idx, accessor.get(idx));
+ idx = accessor.next(idx);
+ }
+ return check("lines()").that(lines);
+ }
+}
diff --git a/java/com/google/gerrit/server/ExceptionHook.java b/java/com/google/gerrit/server/ExceptionHook.java
index db44b4b..019a715 100644
--- a/java/com/google/gerrit/server/ExceptionHook.java
+++ b/java/com/google/gerrit/server/ExceptionHook.java
@@ -34,6 +34,12 @@
* <p>Only affects operations that are executed with {@link
* com.google.gerrit.server.update.RetryHelper}.
*
+ * <p>Should return {@code true} only for exceptions that are caused by temporary issues where a
+ * retry of the operation has a chance to succeed.
+ *
+ * <p>If {@code false} is returned the operation is still retried once to capture a trace, unless
+ * {@link #skipRetryWithTrace(Throwable)} skips the auto-retry.
+ *
* @param throwable throwable that was thrown while executing the operation
* @return whether the operation should be retried
*/
@@ -42,6 +48,34 @@
}
/**
+ * Whether auto-retrying of an operation with tracing should be skipped for the given throwable.
+ *
+ * <p>Only affects operations that are executed with {@link
+ * com.google.gerrit.server.update.RetryHelper}.
+ *
+ * <p>This method is only called for exceptions for which the operation should not be retried
+ * ({@link #shouldRetry(Throwable)} returned {@code false}).
+ *
+ * <p>By default this method returns {@code false}, so that by default traces for unexpected
+ * exceptions are captured, which allows to investigate them.
+ *
+ * <p>Implementors may use this method to skip retry with tracing for exceptions that occur due to
+ * known causes that are permanent and where a trace is not needed for the investigation. For
+ * example, if an operation fails because persisted data is corrupt, it makes no sense to retry
+ * the operation with a trace, because the trace will not help with fixing the corrupt data.
+ *
+ * <p>This method is only invoked if retry with tracing is enabled on the server ({@code
+ * retry.retryWithTraceOnFailure} in {@code gerrit.config} is set to {@code true}).
+ *
+ * @param throwable throwable that was thrown while executing the operation
+ * @return whether auto-retrying of an operation with tracing should be skipped for the given
+ * throwable
+ */
+ default boolean skipRetryWithTrace(Throwable throwable) {
+ return false;
+ }
+
+ /**
* Formats the cause of an exception for use in metrics.
*
* <p>This method allows implementors to group exceptions that have the same cause into one metric
@@ -55,7 +89,9 @@
}
/**
- * Returns an error message that should be returned to the user.
+ * Returns a message that should be returned to the user.
+ *
+ * <p>This message is included into the HTTP response that is sent to the user.
*
* @param throwable throwable that was thrown while executing an operation
* @return error message that should be returned to the user, {@link Optional#empty()} if no
@@ -64,4 +100,25 @@
default Optional<String> getUserMessage(Throwable throwable) {
return Optional.empty();
}
+
+ /**
+ * Returns the HTTP status code that should be returned to the user.
+ *
+ * <p>If no value is returned ({@link Optional#empty()}) the HTTP status code defaults to {@code
+ * 500 Internal Server Error}.
+ *
+ * <p>{@link #getUserMessage(Throwable)} allows to define which message should be included into
+ * the body of the HTTP response.
+ *
+ * <p>Implementors may use this method to change the status code for certain exceptions (e.g.
+ * using this method it would be possible to return {@code 409 Conflict} for {@link
+ * com.google.gerrit.git.LockFailureException}s instead of {@code 500 Internal Server Error}).
+ *
+ * @param throwable throwable that was thrown while executing an operation
+ * @return HTTP status code that should be returned to the user, {@link Optional#empty()} if the
+ * exception should result in {@code 500 Internal Server Error}
+ */
+ default Optional<Integer> getStatusCode(Throwable throwable) {
+ return Optional.empty();
+ }
}
diff --git a/java/com/google/gerrit/server/diff/DiffInfoCreator.java b/java/com/google/gerrit/server/diff/DiffInfoCreator.java
new file mode 100644
index 0000000..c29ffc8
--- /dev/null
+++ b/java/com/google/gerrit/server/diff/DiffInfoCreator.java
@@ -0,0 +1,299 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.diff;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.data.PatchScript;
+import com.google.gerrit.common.data.PatchScript.DisplayMethod;
+import com.google.gerrit.common.data.PatchScript.PatchScriptFileInfo;
+import com.google.gerrit.entities.Patch;
+import com.google.gerrit.extensions.common.ChangeType;
+import com.google.gerrit.extensions.common.DiffInfo;
+import com.google.gerrit.extensions.common.DiffInfo.ContentEntry;
+import com.google.gerrit.extensions.common.DiffInfo.FileMeta;
+import com.google.gerrit.extensions.common.DiffInfo.IntraLineStatus;
+import com.google.gerrit.extensions.common.DiffWebLinkInfo;
+import com.google.gerrit.extensions.common.WebLinkInfo;
+import com.google.gerrit.jgit.diff.ReplaceEdit;
+import com.google.gerrit.prettify.common.SparseFileContent;
+import com.google.gerrit.server.change.FileContentUtil;
+import com.google.gerrit.server.project.ProjectState;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import org.eclipse.jgit.diff.Edit;
+
+/** Creates and fills a new {@link DiffInfo} object based on diff between files. */
+public class DiffInfoCreator {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private static final ImmutableMap<Patch.ChangeType, ChangeType> CHANGE_TYPE =
+ Maps.immutableEnumMap(
+ new ImmutableMap.Builder<Patch.ChangeType, ChangeType>()
+ .put(Patch.ChangeType.ADDED, ChangeType.ADDED)
+ .put(Patch.ChangeType.MODIFIED, ChangeType.MODIFIED)
+ .put(Patch.ChangeType.DELETED, ChangeType.DELETED)
+ .put(Patch.ChangeType.RENAMED, ChangeType.RENAMED)
+ .put(Patch.ChangeType.COPIED, ChangeType.COPIED)
+ .put(Patch.ChangeType.REWRITE, ChangeType.REWRITE)
+ .build());
+
+ private final DiffWebLinksProvider webLinksProvider;
+ private final boolean intraline;
+ private final ProjectState state;
+
+ public DiffInfoCreator(
+ ProjectState state, DiffWebLinksProvider webLinksProvider, boolean intraline) {
+ this.webLinksProvider = webLinksProvider;
+ this.state = state;
+ this.intraline = intraline;
+ }
+
+ /* Returns the {@link DiffInfo} to display for end-users */
+ public DiffInfo create(PatchScript ps, DiffSide sideA, DiffSide sideB) {
+ DiffInfo result = new DiffInfo();
+
+ ImmutableList<DiffWebLinkInfo> links = webLinksProvider.getDiffLinks();
+ result.webLinks = links.isEmpty() ? null : links;
+
+ if (ps.isBinary()) {
+ result.binary = true;
+ }
+ result.metaA = createFileMeta(sideA).orElse(null);
+ result.metaB = createFileMeta(sideB).orElse(null);
+
+ if (intraline) {
+ if (ps.hasIntralineTimeout()) {
+ result.intralineStatus = IntraLineStatus.TIMEOUT;
+ } else if (ps.hasIntralineFailure()) {
+ result.intralineStatus = IntraLineStatus.FAILURE;
+ } else {
+ result.intralineStatus = IntraLineStatus.OK;
+ }
+ logger.atFine().log("intralineStatus = %s", result.intralineStatus);
+ }
+
+ result.changeType = CHANGE_TYPE.get(ps.getChangeType());
+ logger.atFine().log("changeType = %s", result.changeType);
+ if (result.changeType == null) {
+ throw new IllegalStateException("unknown change type: " + ps.getChangeType());
+ }
+
+ if (ps.getPatchHeader().size() > 0) {
+ result.diffHeader = ps.getPatchHeader();
+ }
+ result.content = calculateDiffContentEntries(ps);
+ return result;
+ }
+
+ private static List<ContentEntry> calculateDiffContentEntries(PatchScript ps) {
+ ContentCollector contentCollector = new ContentCollector(ps);
+ Set<Edit> editsDueToRebase = ps.getEditsDueToRebase();
+ for (Edit edit : ps.getEdits()) {
+ logger.atFine().log("next edit = %s", edit);
+
+ if (edit.getType() == Edit.Type.EMPTY) {
+ logger.atFine().log("skip empty edit");
+ continue;
+ }
+ contentCollector.addCommon(edit.getBeginA());
+
+ checkState(
+ contentCollector.nextA == edit.getBeginA(),
+ "nextA = %s; want %s",
+ contentCollector.nextA,
+ edit.getBeginA());
+ checkState(
+ contentCollector.nextB == edit.getBeginB(),
+ "nextB = %s; want %s",
+ contentCollector.nextB,
+ edit.getBeginB());
+ switch (edit.getType()) {
+ case DELETE:
+ case INSERT:
+ case REPLACE:
+ List<Edit> internalEdit =
+ edit instanceof ReplaceEdit ? ((ReplaceEdit) edit).getInternalEdits() : null;
+ boolean dueToRebase = editsDueToRebase.contains(edit);
+ contentCollector.addDiff(edit.getEndA(), edit.getEndB(), internalEdit, dueToRebase);
+ break;
+ case EMPTY:
+ default:
+ throw new IllegalStateException();
+ }
+ }
+ contentCollector.addCommon(ps.getA().getSize());
+
+ return contentCollector.lines;
+ }
+
+ private Optional<FileMeta> createFileMeta(DiffSide side) {
+ PatchScriptFileInfo fileInfo = side.fileInfo();
+ if (fileInfo.displayMethod == DisplayMethod.NONE) {
+ return Optional.empty();
+ }
+ FileMeta result = new FileMeta();
+ result.name = side.fileName();
+ result.contentType =
+ FileContentUtil.resolveContentType(
+ state, side.fileName(), fileInfo.mode, fileInfo.mimeType);
+ result.lines = fileInfo.content.getSize();
+ ImmutableList<WebLinkInfo> links = webLinksProvider.getFileWebLinks(side.type());
+ result.webLinks = links.isEmpty() ? null : links;
+ result.commitId = fileInfo.commitId;
+ return Optional.of(result);
+ }
+
+ private static class ContentCollector {
+
+ private final List<ContentEntry> lines;
+ private final SparseFileContent.Accessor fileA;
+ private final SparseFileContent.Accessor fileB;
+ private final boolean ignoreWS;
+
+ private int nextA;
+ private int nextB;
+
+ ContentCollector(PatchScript ps) {
+ lines = Lists.newArrayListWithExpectedSize(ps.getEdits().size() + 2);
+ fileA = ps.getA().createAccessor();
+ fileB = ps.getB().createAccessor();
+ ignoreWS = ps.isIgnoreWhitespace();
+ }
+
+ void addCommon(int end) {
+ logger.atFine().log("addCommon: end = %d", end);
+
+ end = Math.min(end, fileA.getSize());
+ logger.atFine().log("end = %d", end);
+
+ if (nextA >= end) {
+ logger.atFine().log("nextA >= end: nextA = %d, end = %d", nextA, end);
+ return;
+ }
+
+ while (nextA < end) {
+ logger.atFine().log("nextA < end: nextA = %d, end = %d", nextA, end);
+
+ if (!fileA.contains(nextA)) {
+ logger.atFine().log("fileA does not contain nextA: nextA = %d", nextA);
+
+ int endRegion = Math.min(end, nextA == 0 ? fileA.first() : fileA.next(nextA - 1));
+ int len = endRegion - nextA;
+ entry().skip = len;
+ nextA = endRegion;
+ nextB += len;
+
+ logger.atFine().log("setting: nextA = %d, nextB = %d", nextA, nextB);
+ continue;
+ }
+
+ ContentEntry e = null;
+ for (int i = nextA; i == nextA && i < end; i = fileA.next(i), nextA++, nextB++) {
+ if (ignoreWS && fileB.contains(nextB)) {
+ if (e == null || e.common == null) {
+ logger.atFine().log("create new common entry: nextA = %d, nextB = %d", nextA, nextB);
+ e = entry();
+ e.a = Lists.newArrayListWithCapacity(end - nextA);
+ e.b = Lists.newArrayListWithCapacity(end - nextA);
+ e.common = true;
+ }
+ e.a.add(fileA.get(nextA));
+ e.b.add(fileB.get(nextB));
+ } else {
+ if (e == null || e.common != null) {
+ logger.atFine().log(
+ "create new non-common entry: nextA = %d, nextB = %d", nextA, nextB);
+ e = entry();
+ e.ab = Lists.newArrayListWithCapacity(end - nextA);
+ }
+ e.ab.add(fileA.get(nextA));
+ }
+ }
+ }
+ }
+
+ void addDiff(int endA, int endB, List<Edit> internalEdit, boolean dueToRebase) {
+ logger.atFine().log(
+ "addDiff: endA = %d, endB = %d, numberOfInternalEdits = %d, dueToRebase = %s",
+ endA, endB, internalEdit != null ? internalEdit.size() : 0, dueToRebase);
+
+ int lenA = endA - nextA;
+ int lenB = endB - nextB;
+ logger.atFine().log("lenA = %d, lenB = %d", lenA, lenB);
+ checkState(lenA > 0 || lenB > 0);
+
+ logger.atFine().log("create non-common entry");
+ ContentEntry e = entry();
+ if (lenA > 0) {
+ logger.atFine().log("lenA > 0: lenA = %d", lenA);
+ e.a = Lists.newArrayListWithCapacity(lenA);
+ for (; nextA < endA; nextA++) {
+ e.a.add(fileA.get(nextA));
+ }
+ }
+ if (lenB > 0) {
+ logger.atFine().log("lenB > 0: lenB = %d", lenB);
+ e.b = Lists.newArrayListWithCapacity(lenB);
+ for (; nextB < endB; nextB++) {
+ e.b.add(fileB.get(nextB));
+ }
+ }
+ if (internalEdit != null && !internalEdit.isEmpty()) {
+ logger.atFine().log("processing internal edits");
+
+ e.editA = Lists.newArrayListWithCapacity(internalEdit.size() * 2);
+ e.editB = Lists.newArrayListWithCapacity(internalEdit.size() * 2);
+ int lastA = 0;
+ int lastB = 0;
+ for (Edit edit : internalEdit) {
+ logger.atFine().log("internal edit = %s", edit);
+
+ if (edit.getBeginA() != edit.getEndA()) {
+ logger.atFine().log(
+ "edit.getBeginA() != edit.getEndA(): edit.getBeginA() = %d, edit.getEndA() = %d",
+ edit.getBeginA(), edit.getEndA());
+ e.editA.add(
+ ImmutableList.of(edit.getBeginA() - lastA, edit.getEndA() - edit.getBeginA()));
+ lastA = edit.getEndA();
+ logger.atFine().log("lastA = %d", lastA);
+ }
+ if (edit.getBeginB() != edit.getEndB()) {
+ logger.atFine().log(
+ "edit.getBeginB() != edit.getEndB(): edit.getBeginB() = %d, edit.getEndB() = %d",
+ edit.getBeginB(), edit.getEndB());
+ e.editB.add(
+ ImmutableList.of(edit.getBeginB() - lastB, edit.getEndB() - edit.getBeginB()));
+ lastB = edit.getEndB();
+ logger.atFine().log("lastB = %d", lastB);
+ }
+ }
+ }
+ e.dueToRebase = dueToRebase ? true : null;
+ }
+
+ private ContentEntry entry() {
+ ContentEntry e = new ContentEntry();
+ lines.add(e);
+ return e;
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/diff/DiffSide.java b/java/com/google/gerrit/server/diff/DiffSide.java
new file mode 100644
index 0000000..28c7810
--- /dev/null
+++ b/java/com/google/gerrit/server/diff/DiffSide.java
@@ -0,0 +1,37 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.diff;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.common.data.PatchScript.PatchScriptFileInfo;
+
+/** Contains settings for one of two sides in diff view. Each diff view has exactly 2 sides. */
+@AutoValue
+public abstract class DiffSide {
+ public enum Type {
+ SIDE_A,
+ SIDE_B
+ }
+
+ public static DiffSide create(PatchScriptFileInfo fileInfo, String fileName, Type type) {
+ return new AutoValue_DiffSide(fileInfo, fileName, type);
+ }
+
+ public abstract PatchScriptFileInfo fileInfo();
+
+ public abstract String fileName();
+
+ public abstract Type type();
+}
diff --git a/java/com/google/gerrit/server/diff/DiffWebLinksProvider.java b/java/com/google/gerrit/server/diff/DiffWebLinksProvider.java
new file mode 100644
index 0000000..0f71b17
--- /dev/null
+++ b/java/com/google/gerrit/server/diff/DiffWebLinksProvider.java
@@ -0,0 +1,29 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.diff;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.extensions.common.DiffWebLinkInfo;
+import com.google.gerrit.extensions.common.WebLinkInfo;
+
+/** Provider for different types of links which can be displayed in a diff view. */
+public interface DiffWebLinksProvider {
+
+ /** Returns links associated with the diff view */
+ ImmutableList<DiffWebLinkInfo> getDiffLinks();
+
+ /** Returns links associated with the diff side */
+ ImmutableList<WebLinkInfo> getFileWebLinks(DiffSide.Type fileInfoType);
+}
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 8475d03..4495317 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -40,6 +40,7 @@
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.exceptions.InvalidMergeStrategyException;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -265,7 +266,8 @@
boolean ignoreIdenticalTree,
boolean allowConflicts)
throws MissingObjectException, IncorrectObjectTypeException, IOException,
- MergeIdenticalTreeException, MergeConflictException, MethodNotAllowedException {
+ MergeIdenticalTreeException, MergeConflictException, MethodNotAllowedException,
+ InvalidMergeStrategyException {
ThreeWayMerger m = newThreeWayMerger(inserter, repoConfig);
m.setBase(originalCommit.getParent(parentIndex));
@@ -431,7 +433,8 @@
PersonIdent committerIndent,
String commitMsg,
RevWalk rw)
- throws IOException, MergeIdenticalTreeException, MergeConflictException {
+ throws IOException, MergeIdenticalTreeException, MergeConflictException,
+ InvalidMergeStrategyException {
if (!MergeStrategy.THEIRS.getName().equals(mergeStrategy)
&& rw.isMergedInto(originalCommit, mergeTip)) {
@@ -745,7 +748,7 @@
BranchNameKey destBranch,
CodeReviewCommit mergeTip,
CodeReviewCommit n)
- throws IntegrationException {
+ throws IntegrationException, InvalidMergeStrategyException {
ThreeWayMerger m = newThreeWayMerger(inserter, repoConfig);
try {
if (m.merge(mergeTip, n)) {
@@ -866,7 +869,8 @@
.collect(joining(",", "Merge changes ", merged.size() > 5 ? ", ..." : ""));
}
- public ThreeWayMerger newThreeWayMerger(ObjectInserter inserter, Config repoConfig) {
+ public ThreeWayMerger newThreeWayMerger(ObjectInserter inserter, Config repoConfig)
+ throws InvalidMergeStrategyException {
return newThreeWayMerger(inserter, repoConfig, mergeStrategyName());
}
@@ -890,7 +894,8 @@
}
public static ThreeWayMerger newThreeWayMerger(
- ObjectInserter inserter, Config repoConfig, String strategyName) {
+ ObjectInserter inserter, Config repoConfig, String strategyName)
+ throws InvalidMergeStrategyException {
Merger m = newMerger(inserter, repoConfig, strategyName);
checkArgument(
m instanceof ThreeWayMerger,
@@ -899,9 +904,12 @@
return (ThreeWayMerger) m;
}
- public static Merger newMerger(ObjectInserter inserter, Config repoConfig, String strategyName) {
+ public static Merger newMerger(ObjectInserter inserter, Config repoConfig, String strategyName)
+ throws InvalidMergeStrategyException {
MergeStrategy strategy = MergeStrategy.get(strategyName);
- checkArgument(strategy != null, "invalid merge strategy: %s", strategyName);
+ if (strategy == null) {
+ throw new InvalidMergeStrategyException(strategyName);
+ }
return strategy.newMerger(
new ObjectInserter.Filter() {
@Override
diff --git a/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java b/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
index f7472b9..060000a 100644
--- a/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
+++ b/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
@@ -32,6 +32,8 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.lib.ObjectId;
@@ -105,11 +107,25 @@
Collection<Ref> result;
try {
+ // The security filtering assumes to receive the same refMap, independently from the ref
+ // prefix offset
result = forProject.filter(refs.values(), getDelegate(), RefFilterOptions.defaults());
} catch (PermissionBackendException e) {
throw new IOException("");
}
- return result.stream().collect(toMap(Ref::getName, r -> r));
+ return buildPrefixRefMap(prefix, result);
+ }
+
+ private Map<String, Ref> buildPrefixRefMap(String prefix, Collection<Ref> refs) {
+ int prefixSlashPos = prefix.lastIndexOf('/') + 1;
+ if (prefixSlashPos > 0) {
+ return refs.stream()
+ .collect(
+ Collectors.toMap(
+ (Ref ref) -> ref.getName().substring(prefixSlashPos), Function.identity()));
+ }
+
+ return refs.stream().collect(toMap(Ref::getName, r -> r));
}
@Override
diff --git a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
index 1435c5e..0a17303 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
@@ -17,6 +17,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Comparator.comparing;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.data.CommentDetail;
import com.google.gerrit.common.data.PatchScript;
@@ -24,11 +25,11 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.Patch;
-import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Patch.ChangeType;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.prettify.common.EditList;
-import com.google.gerrit.prettify.common.SparseFileContent;
+import com.google.gerrit.prettify.common.SparseFileContentBuilder;
import com.google.gerrit.server.mime.FileTypeRegistry;
import com.google.inject.Inject;
import eu.medsea.mimeutil.MimeType;
@@ -39,10 +40,8 @@
import java.util.List;
import java.util.Objects;
import java.util.Optional;
+import java.util.Set;
import org.eclipse.jgit.diff.Edit;
-import org.eclipse.jgit.errors.CorruptObjectException;
-import org.eclipse.jgit.errors.IncorrectObjectTypeException;
-import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectId;
@@ -59,28 +58,17 @@
private static final Comparator<Edit> EDIT_SORT = comparing(Edit::getBeginA);
- private Repository db;
- private Project.NameKey projectKey;
- private ObjectReader reader;
private Change change;
private DiffPreferencesInfo diffPrefs;
- private ComparisonType comparisonType;
- private ObjectId aId;
- private ObjectId bId;
private List<Edit> edits;
private final FileTypeRegistry registry;
- private final PatchListCache patchListCache;
private int context;
+ private IntraLineDiffCalculator intralineDiffCalculator;
+ private SidesResolver sidesResolver;
@Inject
- PatchScriptBuilder(FileTypeRegistry ftr, PatchListCache plc) {
+ PatchScriptBuilder(FileTypeRegistry ftr) {
registry = ftr;
- patchListCache = plc;
- }
-
- void setRepository(Repository r, Project.NameKey projectKey) {
- this.db = r;
- this.projectKey = projectKey;
}
void setChange(Change c) {
@@ -98,62 +86,39 @@
}
}
- void setTrees(ComparisonType ct, ObjectId a, ObjectId b) {
- comparisonType = ct;
- aId = a;
- bId = b;
+ void setIntraLineDiffCalculator(IntraLineDiffCalculator calculator) {
+ intralineDiffCalculator = calculator;
}
- PatchScript toPatchScript(PatchListEntry content, CommentDetail comments, List<Patch> history)
- throws IOException {
- reader = db.newObjectReader();
- try {
- return build(content, comments, history);
- } finally {
- reader.close();
- }
+ void setSidesResolver(SidesResolver resolver) {
+ sidesResolver = resolver;
}
- private PatchScript build(PatchListEntry content, CommentDetail comments, List<Patch> history)
+ PatchScript toPatchScript(PatchFileChange content, CommentDetail comments, List<Patch> history)
throws IOException {
- boolean intralineFailure = false;
- boolean intralineTimeout = false;
+ return build(content, comments, history);
+ }
- SideResolver resolver = new SideResolver();
- Side a = resolver.resolve(oldName(content), null, aId);
- Side b = resolver.resolve(newName(content), a, bId);
+ private PatchScript build(PatchFileChange content, CommentDetail comments, List<Patch> history)
+ throws IOException {
- edits = new ArrayList<>(content.getEdits());
+ ResolvedSides sides = sidesResolver.resolveSides(registry, oldName(content), newName(content));
+ PatchSide a = sides.a;
+ PatchSide b = sides.b;
+
+ ImmutableList<Edit> contentEdits = content.getEdits();
ImmutableSet<Edit> editsDueToRebase = content.getEditsDueToRebase();
- if (isModify(content) && diffPrefs.intralineDifference && isIntralineModeAllowed(b)) {
- IntraLineDiff d =
- patchListCache.getIntraLineDiff(
- IntraLineDiffKey.create(a.id, b.id, diffPrefs.ignoreWhitespace),
- IntraLineDiffArgs.create(
- a.src, b.src, edits, editsDueToRebase, projectKey, bId, b.path));
- if (d != null) {
- switch (d.getStatus()) {
- case EDIT_LIST:
- edits = new ArrayList<>(d.getEdits());
- break;
+ IntraLineDiffCalculatorResult intralineResult = IntraLineDiffCalculatorResult.NO_RESULT;
- case DISABLED:
- break;
-
- case ERROR:
- intralineFailure = true;
- break;
-
- case TIMEOUT:
- intralineTimeout = true;
- break;
- }
- } else {
- intralineFailure = true;
- }
+ if (isModify(content) && intralineDiffCalculator != null && isIntralineModeAllowed(b)) {
+ intralineResult =
+ intralineDiffCalculator.calculateIntraLineDiff(
+ contentEdits, editsDueToRebase, a.id, b.id, a.src, b.src, b.treeId, b.path);
}
+ edits = new ArrayList<>(intralineResult.edits.orElse(contentEdits));
+
correctForDifferencesInNewlineAtEnd(a, b);
if (comments != null) {
@@ -198,8 +163,8 @@
b.fileMode,
content.getHeaderLines(),
diffPrefs,
- a.dst,
- b.dst,
+ a.dst.build(),
+ b.dst.build(),
edits,
editsDueToRebase,
a.displayMethod,
@@ -209,14 +174,14 @@
comments,
history,
hugeFile,
- intralineFailure,
- intralineTimeout,
+ intralineResult.failure,
+ intralineResult.timeout,
content.getPatchType() == Patch.PatchType.BINARY,
- aId == null ? null : aId.getName(),
- bId == null ? null : bId.getName());
+ a.treeId == null ? null : a.treeId.getName(),
+ b.treeId == null ? null : b.treeId.getName());
}
- private static boolean isModify(PatchListEntry content) {
+ private static boolean isModify(PatchFileChange content) {
switch (content.getChangeType()) {
case MODIFIED:
case COPIED:
@@ -231,7 +196,7 @@
}
}
- private static String oldName(PatchListEntry entry) {
+ private static String oldName(PatchFileChange entry) {
switch (entry.getChangeType()) {
case ADDED:
return null;
@@ -246,7 +211,7 @@
}
}
- private static String newName(PatchListEntry entry) {
+ private static String newName(PatchFileChange entry) {
switch (entry.getChangeType()) {
case DELETED:
return null;
@@ -260,7 +225,7 @@
}
}
- private static boolean isIntralineModeAllowed(Side side) {
+ private static boolean isIntralineModeAllowed(PatchSide side) {
// The intraline diff cache keys are the same for these cases. It's better to not show
// intraline results than showing completely wrong diffs or to run into a server error.
return !Patch.isMagic(side.path) && !isSubmoduleCommit(side.mode);
@@ -270,7 +235,7 @@
return mode.getObjectType() == Constants.OBJ_COMMIT;
}
- private void correctForDifferencesInNewlineAtEnd(Side a, Side b) {
+ private void correctForDifferencesInNewlineAtEnd(PatchSide a, PatchSide b) {
// a.src.size() is the size ignoring a newline at the end whereas a.size() considers it.
int aSize = a.src.size();
int bSize = b.src.size();
@@ -312,11 +277,11 @@
return list.isEmpty() ? Optional.empty() : Optional.ofNullable(list.get(list.size() - 1));
}
- private boolean isNewlineAtEndDeleted(Side a, Side b) {
+ private boolean isNewlineAtEndDeleted(PatchSide a, PatchSide b) {
return !a.src.isMissingNewlineAtEnd() && b.src.isMissingNewlineAtEnd();
}
- private boolean isNewlineAtEndAdded(Side a, Side b) {
+ private boolean isNewlineAtEndAdded(PatchSide a, PatchSide b) {
return a.src.isMissingNewlineAtEnd() && !b.src.isMissingNewlineAtEnd();
}
@@ -434,7 +399,7 @@
return last.getEndA() + (b - last.getEndB());
}
- private void packContent(Side a, Side b, boolean ignoredWhitespace) {
+ private void packContent(PatchSide a, PatchSide b, boolean ignoredWhitespace) {
EditList list = new EditList(edits, context, a.size(), b.size());
for (EditList.Hunk hunk : list.getHunks()) {
while (hunk.next()) {
@@ -468,8 +433,8 @@
}
}
- private static class Side {
-
+ private static class PatchSide {
+ final ObjectId treeId;
final String path;
final ObjectId id;
final FileMode mode;
@@ -478,9 +443,10 @@
final MimeType mimeType;
final DisplayMethod displayMethod;
final PatchScript.FileMode fileMode;
- final SparseFileContent dst;
+ final SparseFileContentBuilder dst;
- public Side(
+ private PatchSide(
+ ObjectId treeId,
String path,
ObjectId id,
FileMode mode,
@@ -489,6 +455,7 @@
MimeType mimeType,
DisplayMethod displayMethod,
PatchScript.FileMode fileMode) {
+ this.treeId = treeId;
this.path = path;
this.id = id;
this.mode = mode;
@@ -497,8 +464,7 @@
this.mimeType = mimeType;
this.displayMethod = displayMethod;
this.fileMode = fileMode;
- dst = new SparseFileContent();
- dst.setSize(size());
+ dst = new SparseFileContentBuilder(size());
}
int size() {
@@ -521,15 +487,64 @@
}
}
- private class SideResolver {
+ interface SidesResolver {
- Side resolve(final String path, final Side other, final ObjectId within) throws IOException {
+ ResolvedSides resolveSides(FileTypeRegistry typeRegistry, String oldName, String newName)
+ throws IOException;
+ }
+
+ private static class ResolvedSides {
+ // Not an @AutoValue because PatchSide can't be AutoValue
+ public final PatchSide a;
+ public final PatchSide b;
+
+ ResolvedSides(PatchSide a, PatchSide b) {
+ this.a = a;
+ this.b = b;
+ }
+ }
+
+ static class SidesResolverImpl implements SidesResolver {
+
+ private final Repository db;
+ private ComparisonType comparisonType;
+ private ObjectId aId;
+ private ObjectId bId;
+
+ SidesResolverImpl(Repository db) {
+ this.db = db;
+ }
+
+ void setTrees(ComparisonType comparisonType, ObjectId a, ObjectId b) {
+ this.comparisonType = comparisonType;
+ this.aId = a;
+ this.bId = b;
+ }
+
+ @Override
+ public ResolvedSides resolveSides(FileTypeRegistry typeRegistry, String oldName, String newName)
+ throws IOException {
+ try (ObjectReader reader = db.newObjectReader()) {
+ PatchSide a = resolve(typeRegistry, reader, oldName, null, aId);
+ PatchSide b = resolve(typeRegistry, reader, newName, a, bId);
+ return new ResolvedSides(a, b);
+ }
+ }
+
+ PatchSide resolve(
+ final FileTypeRegistry registry,
+ final ObjectReader reader,
+ final String path,
+ final PatchSide other,
+ final ObjectId within)
+ throws IOException {
try {
boolean isCommitMsg = Patch.COMMIT_MSG.equals(path);
boolean isMergeList = Patch.MERGE_LIST.equals(path);
if (isCommitMsg || isMergeList) {
if (comparisonType.isAgainstParentOrAutoMerge() && Objects.equals(aId, within)) {
return createSide(
+ within,
path,
ObjectId.zeroId(),
FileMode.MISSING,
@@ -554,6 +569,7 @@
displayMethod = DisplayMethod.DIFF;
}
return createSide(
+ within,
path,
within,
mode,
@@ -563,7 +579,7 @@
displayMethod,
false);
}
- final TreeWalk tw = find(path, within);
+ final TreeWalk tw = find(reader, path, within);
ObjectId id = tw != null ? tw.getObjectId(0) : ObjectId.zeroId();
FileMode mode = tw != null ? tw.getFileMode(0) : FileMode.MISSING;
boolean reuse =
@@ -598,13 +614,15 @@
displayMethod = DisplayMethod.IMG;
}
}
- return createSide(path, id, mode, srcContent, src, mimeType, displayMethod, reuse);
+ return createSide(within, path, id, mode, srcContent, src, mimeType, displayMethod, reuse);
+
} catch (IOException err) {
throw new IOException("Cannot read " + within.name() + ":" + path, err);
}
}
- private Side createSide(
+ private PatchSide createSide(
+ ObjectId treeId,
String path,
ObjectId id,
FileMode mode,
@@ -629,12 +647,11 @@
} else if (mode == FileMode.GITLINK) {
fileMode = PatchScript.FileMode.GITLINK;
}
- return new Side(path, id, mode, srcContent, src, mimeType, displayMethod, fileMode);
+ return new PatchSide(
+ treeId, path, id, mode, srcContent, src, mimeType, displayMethod, fileMode);
}
- private TreeWalk find(String path, ObjectId within)
- throws MissingObjectException, IncorrectObjectTypeException, CorruptObjectException,
- IOException {
+ private TreeWalk find(ObjectReader reader, String path, ObjectId within) throws IOException {
if (path == null || within == null) {
return null;
}
@@ -649,4 +666,59 @@
return (a.getBits() & FileMode.TYPE_FILE) == FileMode.TYPE_FILE
&& (b.getBits() & FileMode.TYPE_FILE) == FileMode.TYPE_FILE;
}
+
+ static class IntraLineDiffCalculatorResult {
+ // Not an @AutoValue because Edit is mutable
+ final boolean failure;
+ final boolean timeout;
+ private final Optional<ImmutableList<Edit>> edits;
+
+ private IntraLineDiffCalculatorResult(
+ Optional<ImmutableList<Edit>> edits, boolean failure, boolean timeout) {
+ this.failure = failure;
+ this.timeout = timeout;
+ this.edits = edits;
+ }
+
+ static final IntraLineDiffCalculatorResult NO_RESULT =
+ new IntraLineDiffCalculatorResult(Optional.empty(), false, false);
+ static final IntraLineDiffCalculatorResult FAILURE =
+ new IntraLineDiffCalculatorResult(Optional.empty(), true, false);
+ static final IntraLineDiffCalculatorResult TIMEOUT =
+ new IntraLineDiffCalculatorResult(Optional.empty(), false, true);
+
+ static IntraLineDiffCalculatorResult success(ImmutableList<Edit> edits) {
+ return new IntraLineDiffCalculatorResult(Optional.of(edits), false, false);
+ }
+ }
+
+ interface IntraLineDiffCalculator {
+
+ IntraLineDiffCalculatorResult calculateIntraLineDiff(
+ ImmutableList<Edit> edits,
+ Set<Edit> editsDueToRebase,
+ ObjectId aId,
+ ObjectId bId,
+ Text aSrc,
+ Text bSrc,
+ ObjectId bTreeId,
+ String bPath);
+ }
+
+ interface PatchFileChange {
+
+ ImmutableList<Edit> getEdits();
+
+ ImmutableSet<Edit> getEditsDueToRebase();
+
+ List<String> getHeaderLines();
+
+ String getNewName();
+
+ String getOldName();
+
+ ChangeType getChangeType();
+
+ Patch.PatchType getPatchType();
+ }
}
diff --git a/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index ffeda3d..2c8de1d 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -17,6 +17,8 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.CommentDetail;
@@ -27,6 +29,7 @@
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.Patch.ChangeType;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -38,6 +41,10 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LargeObjectException;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.patch.PatchScriptBuilder.IntraLineDiffCalculator;
+import com.google.gerrit.server.patch.PatchScriptBuilder.IntraLineDiffCalculatorResult;
+import com.google.gerrit.server.patch.PatchScriptBuilder.PatchFileChange;
+import com.google.gerrit.server.patch.PatchScriptBuilder.SidesResolverImpl;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -53,15 +60,19 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import java.util.concurrent.Callable;
+import org.eclipse.jgit.diff.Edit;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
public class PatchScriptFactory implements Callable<PatchScript> {
+
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
+
PatchScriptFactory create(
ChangeNotes notes,
String fileName,
@@ -225,7 +236,7 @@
loadCommentsAndHistory(content.getChangeType(), content.getOldName(), content.getNewName());
- return b.toPatchScript(content, comments, history);
+ return b.toPatchScript(new PatchListFileChange(content), comments, history);
} catch (PatchListNotAvailableException e) {
throw new NoSuchChangeException(changeId, e);
} catch (IOException e) {
@@ -256,10 +267,15 @@
private PatchScriptBuilder newBuilder(PatchList list, Repository git) {
final PatchScriptBuilder b = builderFactory.get();
- b.setRepository(git, notes.getProjectName());
b.setChange(notes.getChange());
b.setDiffPrefs(diffPrefs);
- b.setTrees(list.getComparisonType(), list.getOldId(), list.getNewId());
+ if (diffPrefs.intralineDifference) {
+ b.setIntraLineDiffCalculator(
+ new IntraLineDiffCalculatorImpl(patchListCache, notes.getProjectName(), diffPrefs));
+ }
+ SidesResolverImpl sidesResolver = new SidesResolverImpl(git);
+ sidesResolver.setTrees(list.getComparisonType(), list.getOldId(), list.getNewId());
+ b.setSidesResolver(sidesResolver);
return b;
}
@@ -402,4 +418,98 @@
}
}
}
+
+ private static class IntraLineDiffCalculatorImpl implements IntraLineDiffCalculator {
+
+ private final PatchListCache patchListCache;
+ private final Project.NameKey projectKey;
+ private final DiffPreferencesInfo diffPrefs;
+
+ public IntraLineDiffCalculatorImpl(
+ PatchListCache patchListCache, Project.NameKey projectKey, DiffPreferencesInfo diffPrefs) {
+ this.patchListCache = patchListCache;
+ this.projectKey = projectKey;
+ this.diffPrefs = diffPrefs;
+ }
+
+ @Override
+ public IntraLineDiffCalculatorResult calculateIntraLineDiff(
+ ImmutableList<Edit> edits,
+ Set<Edit> editsDueToRebase,
+ ObjectId aId,
+ ObjectId bId,
+ Text aSrc,
+ Text bSrc,
+ ObjectId bTreeId,
+ String bPath) {
+ IntraLineDiff d =
+ patchListCache.getIntraLineDiff(
+ IntraLineDiffKey.create(aId, bId, diffPrefs.ignoreWhitespace),
+ IntraLineDiffArgs.create(
+ aSrc, bSrc, edits, editsDueToRebase, projectKey, bTreeId, bPath));
+ if (d == null) {
+ return IntraLineDiffCalculatorResult.FAILURE;
+ }
+ switch (d.getStatus()) {
+ case EDIT_LIST:
+ return IntraLineDiffCalculatorResult.success(d.getEdits());
+
+ case DISABLED:
+ return IntraLineDiffCalculatorResult.NO_RESULT;
+
+ case ERROR:
+ return IntraLineDiffCalculatorResult.FAILURE;
+
+ case TIMEOUT:
+ return IntraLineDiffCalculatorResult.TIMEOUT;
+
+ default:
+ return IntraLineDiffCalculatorResult.NO_RESULT;
+ }
+ }
+ }
+
+ private static class PatchListFileChange implements PatchFileChange {
+
+ private final PatchListEntry patchListEntry;
+
+ PatchListFileChange(PatchListEntry patchListEntry) {
+ this.patchListEntry = patchListEntry;
+ }
+
+ @Override
+ public ImmutableList<Edit> getEdits() {
+ return patchListEntry.getEdits();
+ }
+
+ @Override
+ public ImmutableSet<Edit> getEditsDueToRebase() {
+ return patchListEntry.getEditsDueToRebase();
+ }
+
+ @Override
+ public String getNewName() {
+ return patchListEntry.getNewName();
+ }
+
+ @Override
+ public String getOldName() {
+ return patchListEntry.getOldName();
+ }
+
+ @Override
+ public ChangeType getChangeType() {
+ return patchListEntry.getChangeType();
+ }
+
+ @Override
+ public List<String> getHeaderLines() {
+ return patchListEntry.getHeaderLines();
+ }
+
+ @Override
+ public Patch.PatchType getPatchType() {
+ return patchListEntry.getPatchType();
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/project/RefUtil.java b/java/com/google/gerrit/server/project/RefUtil.java
index 1dac751..5d6379a 100644
--- a/java/com/google/gerrit/server/project/RefUtil.java
+++ b/java/com/google/gerrit/server/project/RefUtil.java
@@ -19,6 +19,7 @@
import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -46,16 +47,15 @@
try {
ObjectId revid = repo.resolve(baseRevision);
if (revid == null) {
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(baseRevision);
}
return revid;
} catch (IOException err) {
logger.atSevere().withCause(err).log(
"Cannot resolve \"%s\" in project \"%s\"", baseRevision, projectName.get());
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(baseRevision);
} catch (RevisionSyntaxException err) {
- logger.atSevere().withCause(err).log("Invalid revision syntax \"%s\"", baseRevision);
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(baseRevision);
}
}
@@ -66,7 +66,7 @@
try {
rw.markStart(rw.parseCommit(revid));
} catch (IncorrectObjectTypeException err) {
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(revid.name());
}
RefDatabase refDb = repo.getRefDatabase();
Iterable<Ref> refs =
@@ -86,11 +86,11 @@
rw.checkConnectivity();
return rw;
} catch (IncorrectObjectTypeException | MissingObjectException err) {
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(revid.name());
} catch (IOException err) {
logger.atSevere().withCause(err).log(
"Repository \"%s\" may be corrupt; suggest running git fsck", repo.getDirectory());
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(revid.name());
}
}
@@ -125,8 +125,8 @@
public static final String MESSAGE = "Invalid Revision";
- InvalidRevisionException() {
- super(MESSAGE);
+ InvalidRevisionException(@Nullable String invalidRevision) {
+ super(MESSAGE + ": " + invalidRevision);
}
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index f4a8b75..310f71b 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -27,6 +27,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.InvalidMergeStrategyException;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.SubmitType;
@@ -339,8 +340,8 @@
bu.execute();
}
return ins.getChange();
- } catch (IllegalArgumentException e) {
- throw new BadRequestException(e.getMessage(), e);
+ } catch (InvalidMergeStrategyException e) {
+ throw new BadRequestException(e.getMessage());
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetDiff.java b/java/com/google/gerrit/server/restapi/change/GetDiff.java
index a6536ce..82bdc34 100644
--- a/java/com/google/gerrit/server/restapi/change/GetDiff.java
+++ b/java/com/google/gerrit/server/restapi/change/GetDiff.java
@@ -14,27 +14,18 @@
package com.google.gerrit.server.restapi.change;
-import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.util.cli.Localizable.localizable;
import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.PatchScript;
-import com.google.gerrit.common.data.PatchScript.DisplayMethod;
-import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
-import com.google.gerrit.extensions.common.ChangeType;
import com.google.gerrit.extensions.common.DiffInfo;
-import com.google.gerrit.extensions.common.DiffInfo.ContentEntry;
-import com.google.gerrit.extensions.common.DiffInfo.FileMeta;
-import com.google.gerrit.extensions.common.DiffInfo.IntraLineStatus;
import com.google.gerrit.extensions.common.DiffWebLinkInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -44,12 +35,12 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.jgit.diff.ReplaceEdit;
-import com.google.gerrit.prettify.common.SparseFileContent;
import com.google.gerrit.server.WebLinks;
-import com.google.gerrit.server.change.FileContentUtil;
import com.google.gerrit.server.change.FileResource;
import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.diff.DiffInfoCreator;
+import com.google.gerrit.server.diff.DiffSide;
+import com.google.gerrit.server.diff.DiffWebLinksProvider;
import com.google.gerrit.server.git.LargeObjectException;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchScriptFactory;
@@ -60,10 +51,7 @@
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
import java.io.IOException;
-import java.util.List;
-import java.util.Set;
import java.util.concurrent.TimeUnit;
-import org.eclipse.jgit.diff.Edit;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.NamedOptionDef;
@@ -76,17 +64,6 @@
public class GetDiff implements RestReadView<FileResource> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private static final ImmutableMap<Patch.ChangeType, ChangeType> CHANGE_TYPE =
- Maps.immutableEnumMap(
- new ImmutableMap.Builder<Patch.ChangeType, ChangeType>()
- .put(Patch.ChangeType.ADDED, ChangeType.ADDED)
- .put(Patch.ChangeType.MODIFIED, ChangeType.MODIFIED)
- .put(Patch.ChangeType.DELETED, ChangeType.DELETED)
- .put(Patch.ChangeType.RENAMED, ChangeType.RENAMED)
- .put(Patch.ChangeType.COPIED, ChangeType.COPIED)
- .put(Patch.ChangeType.REWRITE, ChangeType.REWRITE)
- .build());
-
private final ProjectCache projectCache;
private final PatchScriptFactory.Factory patchScriptFactoryFactory;
private final Revisions revisions;
@@ -164,111 +141,18 @@
psf.setLoadHistory(false);
psf.setLoadComments(context != DiffPreferencesInfo.WHOLE_FILE_CONTEXT);
PatchScript ps = psf.call();
- ContentCollector contentCollector = new ContentCollector(ps);
- Set<Edit> editsDueToRebase = ps.getEditsDueToRebase();
- for (Edit edit : ps.getEdits()) {
- logger.atFine().log("next edit = %s", edit);
-
- if (edit.getType() == Edit.Type.EMPTY) {
- logger.atFine().log("skip empty edit");
- continue;
- }
- contentCollector.addCommon(edit.getBeginA());
-
- checkState(
- contentCollector.nextA == edit.getBeginA(),
- "nextA = %s; want %s",
- contentCollector.nextA,
- edit.getBeginA());
- checkState(
- contentCollector.nextB == edit.getBeginB(),
- "nextB = %s; want %s",
- contentCollector.nextB,
- edit.getBeginB());
- switch (edit.getType()) {
- case DELETE:
- case INSERT:
- case REPLACE:
- List<Edit> internalEdit =
- edit instanceof ReplaceEdit ? ((ReplaceEdit) edit).getInternalEdits() : null;
- boolean dueToRebase = editsDueToRebase.contains(edit);
- contentCollector.addDiff(edit.getEndA(), edit.getEndB(), internalEdit, dueToRebase);
- break;
- case EMPTY:
- default:
- throw new IllegalStateException();
- }
- }
- contentCollector.addCommon(ps.getA().size());
-
- ProjectState state = projectCache.get(resource.getRevision().getChange().getProject());
-
- DiffInfo result = new DiffInfo();
- String revA = basePatchSet != null ? basePatchSet.refName() : ps.getFileInfoA().commitId;
- String revB =
- resource.getRevision().getEdit().isPresent()
- ? resource.getRevision().getEdit().get().getRefName()
- : resource.getRevision().getPatchSet().refName();
- logger.atFine().log("revA = %s, revB = %s", revA, revB);
-
- ImmutableList<DiffWebLinkInfo> links =
- webLinks.getDiffLinks(
- state.getName(),
- resource.getPatchKey().patchSetId().changeId().get(),
- basePatchSet != null ? basePatchSet.id().get() : null,
- revA,
+ Project.NameKey projectName = resource.getRevision().getChange().getProject();
+ ProjectState state = projectCache.get(projectName);
+ DiffSide sideA =
+ DiffSide.create(
+ ps.getFileInfoA(),
MoreObjects.firstNonNull(ps.getOldName(), ps.getNewName()),
- resource.getPatchKey().patchSetId().get(),
- revB,
- ps.getNewName());
- result.webLinks = links.isEmpty() ? null : links;
-
- if (ps.isBinary()) {
- result.binary = true;
- }
- if (ps.getDisplayMethodA() != DisplayMethod.NONE) {
- result.metaA = new FileMeta();
- result.metaA.name = MoreObjects.firstNonNull(ps.getOldName(), ps.getNewName());
- result.metaA.contentType =
- FileContentUtil.resolveContentType(
- state, result.metaA.name, ps.getFileModeA(), ps.getMimeTypeA());
- result.metaA.lines = ps.getA().size();
- result.metaA.webLinks = getFileWebLinks(state.getProject(), revA, result.metaA.name);
- result.metaA.commitId = ps.getFileInfoA().commitId;
- }
-
- if (ps.getDisplayMethodB() != DisplayMethod.NONE) {
- result.metaB = new FileMeta();
- result.metaB.name = ps.getNewName();
- result.metaB.contentType =
- FileContentUtil.resolveContentType(
- state, result.metaB.name, ps.getFileModeB(), ps.getMimeTypeB());
- result.metaB.lines = ps.getB().size();
- result.metaB.webLinks = getFileWebLinks(state.getProject(), revB, result.metaB.name);
- result.metaB.commitId = ps.getFileInfoB().commitId;
- }
-
- if (intraline) {
- if (ps.hasIntralineTimeout()) {
- result.intralineStatus = IntraLineStatus.TIMEOUT;
- } else if (ps.hasIntralineFailure()) {
- result.intralineStatus = IntraLineStatus.FAILURE;
- } else {
- result.intralineStatus = IntraLineStatus.OK;
- }
- logger.atFine().log("intralineStatus = %s", result.intralineStatus);
- }
-
- result.changeType = CHANGE_TYPE.get(ps.getChangeType());
- logger.atFine().log("changeType = %s", result.changeType);
- if (result.changeType == null) {
- throw new IllegalStateException("unknown change type: " + ps.getChangeType());
- }
-
- if (ps.getPatchHeader().size() > 0) {
- result.diffHeader = ps.getPatchHeader();
- }
- result.content = contentCollector.lines;
+ DiffSide.Type.SIDE_A);
+ DiffSide sideB = DiffSide.create(ps.getFileInfoB(), ps.getNewName(), DiffSide.Type.SIDE_B);
+ DiffWebLinksProvider webLinksProvider =
+ new DiffWebLinksProviderImpl(sideA, sideB, projectName, basePatchSet, webLinks, resource);
+ DiffInfoCreator diffInfoCreator = new DiffInfoCreator(state, webLinksProvider, intraline);
+ DiffInfo result = diffInfoCreator.create(ps, sideA, sideB);
Response<DiffInfo> r = Response.ok(result);
if (resource.isCacheable()) {
@@ -282,9 +166,69 @@
}
}
- private List<WebLinkInfo> getFileWebLinks(Project project, String rev, String file) {
- ImmutableList<WebLinkInfo> links = webLinks.getFileLinks(project.getName(), rev, file);
- return links.isEmpty() ? null : links;
+ private static class DiffWebLinksProviderImpl implements DiffWebLinksProvider {
+
+ private final WebLinks webLinks;
+ private final Project.NameKey projectName;
+ private final DiffSide sideA;
+ private final DiffSide sideB;
+ private final String revA;
+ private final String revB;
+ private final FileResource resource;
+ @Nullable private final PatchSet basePatchSet;
+
+ DiffWebLinksProviderImpl(
+ DiffSide sideA,
+ DiffSide sideB,
+ Project.NameKey projectName,
+ @Nullable PatchSet basePatchSet,
+ WebLinks webLinks,
+ FileResource resource) {
+ this.projectName = projectName;
+ this.webLinks = webLinks;
+ this.basePatchSet = basePatchSet;
+ this.resource = resource;
+ this.sideA = sideA;
+ this.sideB = sideB;
+
+ revA = basePatchSet != null ? basePatchSet.refName() : sideA.fileInfo().commitId;
+
+ RevisionResource revision = resource.getRevision();
+ revB =
+ revision
+ .getEdit()
+ .map(edit -> edit.getRefName())
+ .orElseGet(() -> revision.getPatchSet().refName());
+
+ logger.atFine().log("revA = %s, revB = %s", revA, revB);
+ }
+
+ @Override
+ public ImmutableList<DiffWebLinkInfo> getDiffLinks() {
+ return webLinks.getDiffLinks(
+ projectName.get(),
+ resource.getPatchKey().patchSetId().changeId().get(),
+ basePatchSet != null ? basePatchSet.id().get() : null,
+ revA,
+ sideA.fileName(),
+ resource.getPatchKey().patchSetId().get(),
+ revB,
+ sideB.fileName());
+ }
+
+ @Override
+ public ImmutableList<WebLinkInfo> getFileWebLinks(DiffSide.Type type) {
+ String rev;
+ DiffSide side;
+ if (type == DiffSide.Type.SIDE_A) {
+ rev = revA;
+ side = sideA;
+ } else {
+ rev = revB;
+ side = sideB;
+ }
+ return webLinks.getFileLinks(projectName.get(), rev, side.fileName());
+ }
}
public GetDiff setBase(String base) {
@@ -312,141 +256,6 @@
return this;
}
- private static class ContentCollector {
- final List<ContentEntry> lines;
- final SparseFileContent fileA;
- final SparseFileContent fileB;
- final boolean ignoreWS;
-
- int nextA;
- int nextB;
-
- ContentCollector(PatchScript ps) {
- lines = Lists.newArrayListWithExpectedSize(ps.getEdits().size() + 2);
- fileA = ps.getA();
- fileB = ps.getB();
- ignoreWS = ps.isIgnoreWhitespace();
- }
-
- void addCommon(int end) {
- logger.atFine().log("addCommon: end = %d", end);
-
- end = Math.min(end, fileA.size());
- logger.atFine().log("end = %d", end);
-
- if (nextA >= end) {
- logger.atFine().log("nextA >= end: nextA = %d, end = %d", nextA, end);
- return;
- }
-
- while (nextA < end) {
- logger.atFine().log("nextA < end: nextA = %d, end = %d", nextA, end);
-
- if (!fileA.contains(nextA)) {
- logger.atFine().log("fileA does not contain nextA: nextA = %d", nextA);
-
- int endRegion = Math.min(end, nextA == 0 ? fileA.first() : fileA.next(nextA - 1));
- int len = endRegion - nextA;
- entry().skip = len;
- nextA = endRegion;
- nextB += len;
-
- logger.atFine().log("setting: nextA = %d, nextB = %d", nextA, nextB);
- continue;
- }
-
- ContentEntry e = null;
- for (int i = nextA; i == nextA && i < end; i = fileA.next(i), nextA++, nextB++) {
- if (ignoreWS && fileB.contains(nextB)) {
- if (e == null || e.common == null) {
- logger.atFine().log("create new common entry: nextA = %d, nextB = %d", nextA, nextB);
- e = entry();
- e.a = Lists.newArrayListWithCapacity(end - nextA);
- e.b = Lists.newArrayListWithCapacity(end - nextA);
- e.common = true;
- }
- e.a.add(fileA.get(nextA));
- e.b.add(fileB.get(nextB));
- } else {
- if (e == null || e.common != null) {
- logger.atFine().log(
- "create new non-common entry: nextA = %d, nextB = %d", nextA, nextB);
- e = entry();
- e.ab = Lists.newArrayListWithCapacity(end - nextA);
- }
- e.ab.add(fileA.get(nextA));
- }
- }
- logger.atFine().log("nextA = %d, nextB = %d", nextA, nextB);
- }
- }
-
- void addDiff(int endA, int endB, List<Edit> internalEdit, boolean dueToRebase) {
- logger.atFine().log(
- "addDiff: endA = %d, endB = %d, numberOfInternalEdits = %d, dueToRebase = %s",
- endA, endB, internalEdit != null ? internalEdit.size() : 0, dueToRebase);
-
- int lenA = endA - nextA;
- int lenB = endB - nextB;
- logger.atFine().log("lenA = %d, lenB = %d", lenA, lenB);
- checkState(lenA > 0 || lenB > 0);
-
- logger.atFine().log("create non-common entry");
- ContentEntry e = entry();
- if (lenA > 0) {
- logger.atFine().log("lenA > 0: lenA = %d", lenA);
- e.a = Lists.newArrayListWithCapacity(lenA);
- for (; nextA < endA; nextA++) {
- e.a.add(fileA.get(nextA));
- }
- }
- if (lenB > 0) {
- logger.atFine().log("lenB > 0: lenB = %d", lenB);
- e.b = Lists.newArrayListWithCapacity(lenB);
- for (; nextB < endB; nextB++) {
- e.b.add(fileB.get(nextB));
- }
- }
- if (internalEdit != null && !internalEdit.isEmpty()) {
- logger.atFine().log("processing internal edits");
-
- e.editA = Lists.newArrayListWithCapacity(internalEdit.size() * 2);
- e.editB = Lists.newArrayListWithCapacity(internalEdit.size() * 2);
- int lastA = 0;
- int lastB = 0;
- for (Edit edit : internalEdit) {
- logger.atFine().log("internal edit = %s", edit);
-
- if (edit.getBeginA() != edit.getEndA()) {
- logger.atFine().log(
- "edit.getBeginA() != edit.getEndA(): edit.getBeginA() = %d, edit.getEndA() = %d",
- edit.getBeginA(), edit.getEndA());
- e.editA.add(
- ImmutableList.of(edit.getBeginA() - lastA, edit.getEndA() - edit.getBeginA()));
- lastA = edit.getEndA();
- logger.atFine().log("lastA = %d", lastA);
- }
- if (edit.getBeginB() != edit.getEndB()) {
- logger.atFine().log(
- "edit.getBeginB() != edit.getEndB(): edit.getBeginB() = %d, edit.getEndB() = %d",
- edit.getBeginB(), edit.getEndB());
- e.editB.add(
- ImmutableList.of(edit.getBeginB() - lastB, edit.getEndB() - edit.getBeginB()));
- lastB = edit.getEndB();
- logger.atFine().log("lastB = %d", lastB);
- }
- }
- }
- e.dueToRebase = dueToRebase ? true : null;
- }
-
- private ContentEntry entry() {
- ContentEntry e = new ContentEntry();
- lines.add(e);
- return e;
- }
- }
-
@Deprecated
enum IgnoreWhitespace {
NONE(DiffPreferencesInfo.Whitespace.IGNORE_NONE),
@@ -462,6 +271,7 @@
}
public static class ContextOptionHandler extends OptionHandler<Short> {
+
public ContextOptionHandler(CmdLineParser parser, OptionDef option, Setter<Short> setter) {
super(parser, option, setter);
}
diff --git a/java/com/google/gerrit/server/restapi/change/Mergeable.java b/java/com/google/gerrit/server/restapi/change/Mergeable.java
index 9b17ed6..cce8923 100644
--- a/java/com/google/gerrit/server/restapi/change/Mergeable.java
+++ b/java/com/google/gerrit/server/restapi/change/Mergeable.java
@@ -17,7 +17,6 @@
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.MergeableInfo;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -133,10 +132,10 @@
return Response.ok(result);
}
- private SubmitType getSubmitType(ChangeData cd) {
+ private SubmitType getSubmitType(ChangeData cd) throws ResourceConflictException {
SubmitTypeRecord rec = submitRuleEvaluator.getSubmitType(cd);
if (rec.status != SubmitTypeRecord.Status.OK) {
- throw new StorageException("Submit type rule failed: " + rec);
+ throw new ResourceConflictException("submit type rule error: " + rec.errorMessage);
}
return rec.type;
}
diff --git a/java/com/google/gerrit/server/restapi/project/CheckMergeability.java b/java/com/google/gerrit/server/restapi/project/CheckMergeability.java
index 69a6da8..4864fde 100644
--- a/java/com/google/gerrit/server/restapi/project/CheckMergeability.java
+++ b/java/com/google/gerrit/server/restapi/project/CheckMergeability.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.project;
+import com.google.gerrit.exceptions.InvalidMergeStrategyException;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.MergeableInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -120,7 +121,7 @@
result.conflicts = ((ResolveMerger) m).getUnmergedPaths();
}
}
- } catch (IllegalArgumentException e) {
+ } catch (InvalidMergeStrategyException e) {
throw new BadRequestException(e.getMessage());
}
return Response.ok(result);
diff --git a/java/com/google/gerrit/server/restapi/project/CreateBranch.java b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
index 56948c1..67213c5 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateBranch.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
@@ -16,6 +16,7 @@
import static com.google.gerrit.entities.RefNames.isConfigRef;
+import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.RefNames;
@@ -93,7 +94,10 @@
if (input.ref != null && !ref.equals(input.ref)) {
throw new BadRequestException("ref must match URL");
}
- if (input.revision == null) {
+ if (input.revision != null) {
+ input.revision = input.revision.trim();
+ }
+ if (Strings.isNullOrEmpty(input.revision)) {
input.revision = Constants.HEAD;
}
while (ref.startsWith("/")) {
diff --git a/java/com/google/gerrit/server/restapi/project/CreateTag.java b/java/com/google/gerrit/server/restapi/project/CreateTag.java
index dca6e9a..8fdf5e4 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateTag.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateTag.java
@@ -88,7 +88,10 @@
if (input.ref != null && !ref.equals(input.ref)) {
throw new BadRequestException("ref must match URL");
}
- if (input.revision == null) {
+ if (input.revision != null) {
+ input.revision = input.revision.trim();
+ }
+ if (Strings.isNullOrEmpty(input.revision)) {
input.revision = Constants.HEAD;
}
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index 4f9a67c..f3636c9 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -335,6 +335,12 @@
if (retryWithTraceOnFailure
&& opts.retryWithTrace().isPresent()
&& opts.retryWithTrace().get().test(t)) {
+ // Exception hooks may identify exceptions for which retrying with trace should be
+ // skipped.
+ if (exceptionHooks.stream().anyMatch(h -> h.skipRetryWithTrace(t))) {
+ return false;
+ }
+
String caller = opts.caller().orElse("N/A");
String cause = formatCause(t);
if (!traceContext.isTracing()) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
index abfc23d..7ecbe69 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
@@ -179,15 +179,15 @@
assertThat(toChangeNumbers(query("is:abandoned"))).containsExactly(id4);
}
+ /**
+ * When indexMergeable is disabled then the abandonIfMergeable option is ineffective and the auto
+ * abandon behaves as though it were set to its default value (true).
+ */
@Test
@UseClockStep
@GerritConfig(name = "changeCleanup.abandonAfter", value = "1w")
@GerritConfig(name = "changeCleanup.abandonIfMergeable", value = "false")
@GerritConfig(name = "index.change.indexMergeable", value = "false")
- /**
- * When indexMergeable is disabled then the abandonIfMergeable option is ineffective and the auto
- * abandon behaves as though it were set to its default value (true).
- */
public void abandonedIfMergeableWhenMergeableOperatorIsDisabled() throws Exception {
ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
index e5ef5ba..85d383e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
@@ -36,9 +36,11 @@
import com.google.gerrit.extensions.api.projects.BranchInfo;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.inject.Inject;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
import org.junit.Test;
@@ -146,6 +148,116 @@
"Not allowed to create group branch.");
}
+ @Test
+ public void createWithRevision() throws Exception {
+ RevCommit revision = projectOperations.project(project).getHead("master");
+
+ // update master so that points to a different revision than the revision on which we create the
+ // new branch
+ pushTo("refs/heads/master");
+ assertThat(projectOperations.project(project).getHead("master")).isNotEqualTo(revision);
+
+ BranchInput input = new BranchInput();
+ input.revision = revision.name();
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(revision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch())).isEqualTo(revision);
+ }
+
+ @Test
+ public void createWithoutSpecifyingRevision() throws Exception {
+ // If revision is not specified, the branch is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = null;
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void createWithEmptyRevision() throws Exception {
+ // If revision is not specified, the branch is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "";
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void createRevisionIsTrimmed() throws Exception {
+ RevCommit revision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "\t" + revision.name();
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(revision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch())).isEqualTo(revision);
+ }
+
+ @Test
+ public void createWithBranchNameAsRevision() throws Exception {
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "master";
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void createWithFullBranchNameAsRevision() throws Exception {
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "refs/heads/master";
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void cannotCreateWithNonExistingBranchNameAsRevision() throws Exception {
+ assertCreateFails(
+ testBranch,
+ "refs/heads/non-existing",
+ BadRequestException.class,
+ "invalid revision \"refs/heads/non-existing\"");
+ }
+
+ @Test
+ public void cannotCreateWithNonExistingRevision() throws Exception {
+ assertCreateFails(
+ testBranch,
+ "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
+ BadRequestException.class,
+ "invalid revision \"deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\"");
+ }
+
+ @Test
+ public void cannotCreateWithInvalidRevision() throws Exception {
+ assertCreateFails(
+ testBranch,
+ "invalid\trevision",
+ BadRequestException.class,
+ "invalid revision \"invalid\trevision\"");
+ }
+
private void blockCreateReference() throws Exception {
projectOperations
.project(project)
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
index 3d1a148..3becb81 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
@@ -41,6 +41,7 @@
import com.google.inject.Inject;
import java.sql.Timestamp;
import java.util.List;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Test;
@NoHttpd
@@ -357,6 +358,53 @@
assertThat(thrown).hasMessageThat().contains("Invalid base revision");
}
+ @Test
+ public void noBaseRevision() throws Exception {
+ grantTagPermissions();
+
+ // If revision is not specified, the tag is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ TagInput input = new TagInput();
+ input.ref = "test";
+ input.revision = null;
+
+ TagInfo result = tag(input.ref).create(input).get();
+ assertThat(result.ref).isEqualTo(R_TAGS + input.ref);
+ assertThat(result.revision).isEqualTo(expectedRevision.name());
+ }
+
+ @Test
+ public void emptyBaseRevision() throws Exception {
+ grantTagPermissions();
+
+ // If revision is not specified, the tag is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ TagInput input = new TagInput();
+ input.ref = "test";
+ input.revision = "";
+
+ TagInfo result = tag(input.ref).create(input).get();
+ assertThat(result.ref).isEqualTo(R_TAGS + input.ref);
+ assertThat(result.revision).isEqualTo(expectedRevision.name());
+ }
+
+ @Test
+ public void baseRevisionIsTrimmed() throws Exception {
+ grantTagPermissions();
+
+ RevCommit revision = projectOperations.project(project).getHead("master");
+
+ TagInput input = new TagInput();
+ input.ref = "test";
+ input.revision = "\t" + revision.name();
+
+ TagInfo result = tag(input.ref).create(input).get();
+ assertThat(result.ref).isEqualTo(R_TAGS + input.ref);
+ assertThat(result.revision).isEqualTo(revision.name());
+ }
+
private void assertTagList(FluentIterable<String> expected, List<TagInfo> actual)
throws Exception {
assertThat(actual).hasSize(expected.size());
diff --git a/javatests/com/google/gerrit/elasticsearch/BUILD b/javatests/com/google/gerrit/elasticsearch/BUILD
index 955930b..6521166 100644
--- a/javatests/com/google/gerrit/elasticsearch/BUILD
+++ b/javatests/com/google/gerrit/elasticsearch/BUILD
@@ -30,6 +30,11 @@
"//lib:jgit",
]
+HTTP_TEST_DEPS = [
+ "//lib/httpcomponents:httpasyncclient",
+ "//lib/httpcomponents:httpclient",
+]
+
QUERY_TESTS_DEP = "//javatests/com/google/gerrit/server/query/%s:abstract_query_tests"
TYPES = [
@@ -66,7 +71,7 @@
size = "large",
srcs = [src],
tags = ELASTICSEARCH_TAGS,
- deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name],
+ deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name] + HTTP_TEST_DEPS,
) for name, src in ELASTICSEARCH_TESTS_V6.items()]
[junit_tests(
@@ -74,10 +79,7 @@
size = "large",
srcs = [src],
tags = ELASTICSEARCH_TAGS,
- deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name] + [
- "//lib/httpcomponents:httpasyncclient",
- "//lib/httpcomponents:httpclient",
- ],
+ deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name] + HTTP_TEST_DEPS,
) for name, src in ELASTICSEARCH_TESTS_V7.items()]
junit_tests(
diff --git a/javatests/com/google/gerrit/integration/git/BUILD b/javatests/com/google/gerrit/integration/git/BUILD
index 6a6f5ad..4184f1f 100644
--- a/javatests/com/google/gerrit/integration/git/BUILD
+++ b/javatests/com/google/gerrit/integration/git/BUILD
@@ -3,5 +3,5 @@
acceptance_tests(
srcs = glob(["*IT.java"]),
group = "git",
- labels = ["git"],
+ labels = ["git-protocol-v2"],
)
diff --git a/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
index 22981f0..8577c16 100644
--- a/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
+++ b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.integration.git;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.deny;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -30,9 +29,13 @@
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.inject.Inject;
@@ -40,14 +43,20 @@
import java.net.InetSocketAddress;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
+import org.junit.BeforeClass;
import org.junit.Test;
@UseSsh
public class GitProtocolV2IT extends StandaloneSiteTest {
+ private static final String ADMIN_PASSWORD = "secret";
private final String[] SSH_KEYGEN_CMD =
new String[] {"ssh-keygen", "-t", "rsa", "-q", "-P", "", "-f"};
private final String[] GIT_LS_REMOTE =
new String[] {"git", "-c", "protocol.version=2", "ls-remote", "-o", "trace=12345"};
+ private final String[] GIT_CLONE_MIRROR =
+ new String[] {"git", "-c", "protocol.version=2", "clone", "--mirror"};
+ private final String[] GIT_FETCH = new String[] {"git", "-c", "protocol.version=2", "fetch"};
+ private final String[] GIT_INIT = new String[] {"git", "init"};
private final String GIT_SSH_COMMAND =
"ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i";
@@ -56,16 +65,21 @@
@Inject private ProjectOperations projectOperations;
@Inject private @TestSshServerAddress InetSocketAddress sshAddress;
@Inject private @GerritServerConfig Config config;
+ @Inject private AllProjectsName allProjectsName;
- @Test
- public void testGitWireProtocolV2WithSsh() throws Exception {
+ @BeforeClass
+ public static void assertGitClientVersion() throws Exception {
// Minimum required git-core version that supports wire protocol v2 is 2.18.0
GitClientVersion requiredGitVersion = new GitClientVersion(2, 18, 0);
GitClientVersion actualGitVersion =
- new GitClientVersion(execute(ImmutableList.of("git", "version")));
- // If not found, test succeeds with assumption violation
- assume().that(actualGitVersion).isAtLeast(requiredGitVersion);
+ new GitClientVersion(execute(ImmutableList.of("git", "version"), new File("/")));
+ // If git client version cannot be updated, consider to skip this tests. Due to
+ // an existing issue in bazel, JUnit assumption violation feature cannot be used.
+ assertThat(actualGitVersion).isAtLeast(requiredGitVersion);
+ }
+ @Test
+ public void testGitWireProtocolV2WithSsh() throws Exception {
try (ServerContext ctx = startServer()) {
ctx.getInjector().injectMembers(this);
@@ -122,7 +136,7 @@
.commit;
// Prepare new change on secret branch
- in = new ChangeInput(project.get(), "secret", "Test secret change");
+ in = new ChangeInput(project.get(), ADMIN_PASSWORD, "Test secret change");
in.newBranch = true;
// Create new change and retrieve SHA1 for the created patch set
@@ -193,9 +207,137 @@
}
}
+ @Test
+ public void testGitWireProtocolV2HidesRefMetaConfig() throws Exception {
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+ String url = config.getString("gerrit", null, "canonicalweburl");
+
+ // Create project
+ Project.NameKey allRefsVisibleProject = Project.nameKey("all-refs-visible");
+ gApi.projects().create(allRefsVisibleProject.get());
+
+ // Set protocol.version=2 in target repository
+ execute(
+ ImmutableList.of("git", "config", "protocol.version", "2"),
+ sitePaths
+ .site_path
+ .resolve("git")
+ .resolve(allRefsVisibleProject.get() + Constants.DOT_GIT)
+ .toFile());
+
+ // Set up project permission to allow reading all refs
+ projectOperations
+ .project(allRefsVisibleProject)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/heads/*").group(SystemGroupBackend.ANONYMOUS_USERS))
+ .add(
+ allow(Permission.READ)
+ .ref("refs/changes/*")
+ .group(SystemGroupBackend.ANONYMOUS_USERS))
+ .update();
+
+ // Create new change and retrieve refs for the created patch set
+ ChangeInput visibleChangeIn =
+ new ChangeInput(allRefsVisibleProject.get(), "master", "Test public change");
+ visibleChangeIn.newBranch = true;
+ int visibleChangeNumber = gApi.changes().create(visibleChangeIn).info()._number;
+ Change.Id changeId = Change.id(visibleChangeNumber);
+ String visibleChangeNumberRef = RefNames.patchSetRef(PatchSet.id(changeId, 1));
+ String visibleChangeNumberMetaRef = RefNames.changeMetaRef(changeId);
+
+ // Read refs from target repository using git wire protocol v2 over HTTP anonymously
+ String outAnonymousLsRemote =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_CLONE_MIRROR)
+ .add(url + "/" + allRefsVisibleProject.get())
+ .build(),
+ ImmutableMap.of("GIT_TRACE_PACKET", "1"));
+
+ assertThat(outAnonymousLsRemote).contains("git< version 2");
+ assertThat(outAnonymousLsRemote).doesNotContain(RefNames.REFS_CONFIG);
+ assertThat(outAnonymousLsRemote).contains(visibleChangeNumberRef);
+ assertThat(outAnonymousLsRemote).contains(visibleChangeNumberMetaRef);
+ }
+ }
+
+ @Test
+ public void testGitWireProtocolV2FetchIndividualRef() throws Exception {
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+
+ // Setup admin password
+ gApi.accounts().id(admin.username()).setHttpPassword(ADMIN_PASSWORD);
+
+ // Get authenticated Git/HTTP URL
+ String urlWithCredentials =
+ config
+ .getString("gerrit", null, "canonicalweburl")
+ .replace("http://", "http://" + admin.username() + ":" + ADMIN_PASSWORD + "@");
+
+ // Create project
+ Project.NameKey privateProject = Project.nameKey("private-project");
+ gApi.projects().create(privateProject.get());
+
+ // Set protocol.version=2 in target repository
+ execute(
+ ImmutableList.of("git", "config", "protocol.version", "2"),
+ sitePaths
+ .site_path
+ .resolve("git")
+ .resolve(privateProject.get() + Constants.DOT_GIT)
+ .toFile());
+
+ // Disallow general read permissions for anonymous users
+ projectOperations
+ .project(allProjectsName)
+ .forUpdate()
+ .add(deny(Permission.READ).ref("refs/*").group(SystemGroupBackend.ANONYMOUS_USERS))
+ .add(
+ allow(Permission.READ)
+ .ref("refs/heads/master")
+ .group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+
+ // Set up project permission to allow registered users fetching changes/*
+ projectOperations
+ .project(privateProject)
+ .forUpdate()
+ .add(
+ allow(Permission.READ)
+ .ref("refs/changes/*")
+ .group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+
+ // Create new change and retrieve refs for the created patch set
+ ChangeInput visibleChangeIn =
+ new ChangeInput(privateProject.get(), "master", "Test private change");
+ visibleChangeIn.newBranch = true;
+ int visibleChangeNumber = gApi.changes().create(visibleChangeIn).info()._number;
+ Change.Id changeId = Change.id(visibleChangeNumber);
+ String visibleChangeNumberRef = RefNames.patchSetRef(PatchSet.id(changeId, 1));
+
+ // Fetch a single ref using git wire protocol v2 over HTTP with authentication
+ execute(GIT_INIT);
+
+ String outFetchRef =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_FETCH)
+ .add(urlWithCredentials + "/" + privateProject.get())
+ .add(visibleChangeNumberRef)
+ .build(),
+ ImmutableMap.of("GIT_TRACE_PACKET", "1"));
+
+ assertThat(outFetchRef).contains("git< version 2");
+ assertThat(outFetchRef).contains(visibleChangeNumberRef);
+ }
+ }
+
private void setUpUserAuthentication(String username) throws Exception {
// Assign HTTP password to user
- gApi.accounts().id(username).setHttpPassword("secret");
+ gApi.accounts().id(username).setHttpPassword(ADMIN_PASSWORD);
// Generate private/public key for user
execute(
@@ -221,6 +363,10 @@
assertThat(out).contains(commit);
}
+ private String execute(String... cmds) throws Exception {
+ return execute(ImmutableList.<String>builder().add(cmds).build());
+ }
+
private String execute(ImmutableList<String> cmd) throws Exception {
return execute(cmd, sitePaths.data_dir.toFile(), ImmutableMap.of());
}
diff --git a/javatests/com/google/gerrit/prettify/BUILD b/javatests/com/google/gerrit/prettify/BUILD
new file mode 100644
index 0000000..0eb7cee
--- /dev/null
+++ b/javatests/com/google/gerrit/prettify/BUILD
@@ -0,0 +1,13 @@
+load("//tools/bzl:junit.bzl", "junit_tests")
+
+junit_tests(
+ name = "prettify_tests",
+ srcs = glob(["**/*.java"]),
+ deps = [
+ "//java/com/google/gerrit/prettify:server",
+ "//java/com/google/gerrit/prettify/common/testing",
+ "//java/com/google/gerrit/testing:gerrit-test-util",
+ "//lib:guava",
+ "//lib/truth",
+ ],
+)
diff --git a/javatests/com/google/gerrit/prettify/common/SparseFileContentBuilderTest.java b/javatests/com/google/gerrit/prettify/common/SparseFileContentBuilderTest.java
new file mode 100644
index 0000000..a751d50
--- /dev/null
+++ b/javatests/com/google/gerrit/prettify/common/SparseFileContentBuilderTest.java
@@ -0,0 +1,170 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.prettify.common;
+
+import static com.google.gerrit.prettify.common.testing.SparseFileContentSubject.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableMap;
+import org.junit.Ignore;
+import org.junit.Test;
+
+public class SparseFileContentBuilderTest {
+
+ @Test
+ public void addLineWithNegativeNumber() {
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(10);
+ assertThrows(IllegalArgumentException.class, () -> builder.addLine(-1, "First line"));
+
+ assertThrows(IllegalArgumentException.class, () -> builder.addLine(-5, "First line"));
+ }
+
+ @Test
+ @Ignore
+ public void addLineNumberZeroFileSize() {
+ // Temporary ignore - see comments in SparseFileContentBuilder.build() method
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(0);
+ assertThrows(IllegalArgumentException.class, () -> builder.addLine(0, "First line"));
+ }
+
+ @Test
+ @Ignore
+ public void addLineNumberNonZeroFileSize() {
+ // Temporary ignore - see comments in SparseFileContentBuilder.build() method
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(5);
+ assertThrows(IllegalArgumentException.class, () -> builder.addLine(5, "First line"));
+ assertThrows(IllegalArgumentException.class, () -> builder.addLine(6, "First line"));
+ assertThrows(IllegalArgumentException.class, () -> builder.addLine(7, "First line"));
+ }
+
+ @Test
+ public void addLineIncorrectOrder() {
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(5);
+
+ builder.addLine(0, "First line");
+ builder.addLine(1, "Second line");
+ builder.addLine(3, "Third line");
+ builder.addLine(4, "Fourth line");
+ assertThrows(IllegalArgumentException.class, () -> builder.addLine(4, "Other Line"));
+
+ assertThrows(IllegalArgumentException.class, () -> builder.addLine(2, "Other Line"));
+ }
+
+ @Test
+ public void emptyContentZeroSize() {
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(0);
+
+ SparseFileContent content = builder.build();
+ assertThat(content).getSize().isEqualTo(0);
+ assertThat(content).getRangesCount().isEqualTo(0);
+ assertThat(content).lines().isEmpty();
+ }
+
+ @Test
+ public void emptyContentNonZeroSize() {
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(4);
+ SparseFileContent content = builder.build();
+ assertThat(content).getSize().isEqualTo(4);
+ assertThat(content).getRangesCount().isEqualTo(0);
+ assertThat(content).lines().isEmpty();
+ }
+
+ @Test
+ public void oneLineContentLineNumberZero() {
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(1);
+
+ builder.addLine(0, "First line");
+ SparseFileContent content = builder.build();
+ assertThat(content).getSize().isEqualTo(1);
+ assertThat(content).getRangesCount().isEqualTo(1);
+ assertThat(content).lines().containsExactlyEntriesIn(ImmutableMap.of(0, "First line"));
+ }
+
+ @Test
+ public void oneLineContentLineNumberNotZero() {
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(6);
+
+ builder.addLine(5, "First line");
+ SparseFileContent content = builder.build();
+ assertThat(content).getSize().isEqualTo(6);
+ assertThat(content).getRangesCount().isEqualTo(1);
+ assertThat(content).lines().containsExactlyEntriesIn(ImmutableMap.of(5, "First line"));
+ }
+
+ @Test
+ public void multiLineContinuousContentStartingFromZero() {
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(5);
+
+ builder.addLine(0, "First line");
+ builder.addLine(1, "Second line");
+ builder.addLine(2, "Third line");
+ SparseFileContent content = builder.build();
+ assertThat(content).getSize().isEqualTo(5);
+ assertThat(content).getRangesCount().isEqualTo(1);
+ assertThat(content)
+ .lines()
+ .containsExactlyEntriesIn(
+ ImmutableMap.of(
+ 0, "First line",
+ 1, "Second line",
+ 2, "Third line"));
+ }
+
+ @Test
+ public void multiLineContentStartingFromNonZeroLine() {
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(8);
+
+ builder.addLine(5, "First line");
+ builder.addLine(6, "Second line");
+ builder.addLine(7, "Third line");
+ SparseFileContent content = builder.build();
+ assertThat(content).getSize().isEqualTo(8);
+ assertThat(content).getRangesCount().isEqualTo(1);
+ assertThat(content)
+ .lines()
+ .containsExactlyEntriesIn(
+ ImmutableMap.of(
+ 5, "First line",
+ 6, "Second line",
+ 7, "Third line"));
+ }
+
+ @Test
+ public void multiLineContentWithGaps() {
+ SparseFileContentBuilder builder = new SparseFileContentBuilder(10000);
+ builder.addLine(0, "First line");
+ builder.addLine(1, "Second line");
+ builder.addLine(3, "Third line");
+ builder.addLine(4, "Fourth line");
+ builder.addLine(5, "Fifth line");
+ builder.addLine(6, "Sixth line");
+ builder.addLine(10, "Seventh line");
+ SparseFileContent content = builder.build();
+ assertThat(content).getSize().isEqualTo(10000);
+ assertThat(content).getRangesCount().isEqualTo(3);
+ assertThat(content)
+ .lines()
+ .containsExactlyEntriesIn(
+ ImmutableMap.builder()
+ .put(0, "First line")
+ .put(1, "Second line")
+ .put(3, "Third line")
+ .put(4, "Fourth line")
+ .put(5, "Fifth line")
+ .put(6, "Sixth line")
+ .put(10, "Seventh line")
+ .build());
+ }
+}
diff --git a/modules/jgit b/modules/jgit
index 0356613..a7e454b 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit 0356613f48ebee2e3d2d65780e71d9e0b43a752e
+Subproject commit a7e454bc51d359c2d46b19fd559f770cad8fd7d4
diff --git a/plugins/gitiles b/plugins/gitiles
index 3531010..22b8e24 160000
--- a/plugins/gitiles
+++ b/plugins/gitiles
@@ -1 +1 @@
-Subproject commit 3531010e04d9d548fe1fd93662ca85ae25d4a9a6
+Subproject commit 22b8e242b5eaa9eae817b776bc862b096479ceaa
diff --git a/plugins/replication b/plugins/replication
index ef5ffc7..b3ed3c8 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit ef5ffc7508627f8c2f86e03c556703aa28c2f134
+Subproject commit b3ed3c8f9a8bab51beed794de5be8fd6da44231a
diff --git a/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior.html b/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior.html
index 8f9bf00..67e4ca6 100644
--- a/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior.html
+++ b/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior.html
@@ -81,6 +81,12 @@
return path;
},
+ isMagicPath(path) {
+ return !!path &&
+ (path === Gerrit.PathListBehavior.COMMIT_MESSAGE_PATH || path ===
+ Gerrit.PathListBehavior.MERGE_LIST_PATH);
+ },
+
computeTruncatedPath(path) {
return Gerrit.PathListBehavior.truncatePath(
Gerrit.PathListBehavior.computeDisplayPath(path));
diff --git a/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior_test.html b/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior_test.html
index 924c98c..12b981c 100644
--- a/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior_test.html
+++ b/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior_test.html
@@ -56,6 +56,14 @@
assert.equal(name('/MERGE_LIST'), 'Merge list');
});
+ test('isMagicPath', () => {
+ const isMagic = Gerrit.PathListBehavior.isMagicPath;
+ assert.isFalse(isMagic(undefined));
+ assert.isFalse(isMagic('/foo.cc'));
+ assert.isTrue(isMagic('/COMMIT_MSG'));
+ assert.isTrue(isMagic('/MERGE_LIST'));
+ });
+
test('truncatePath with long path should add ellipsis', () => {
const truncatePath = Gerrit.PathListBehavior.truncatePath;
let path = 'level1/level2/level3/level4/file.js';
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.html b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.html
index e6eef75..04bdab7 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.html
@@ -197,7 +197,10 @@
assert.equal(Polymer.dom(element.root)
.querySelectorAll('.sectionTitle').length, 3);
assert.equal(element.$$('.breadcrumbText').innerText, 'Test Repo');
- assert.equal(element.$$('#pageSelect').items.length, 6);
+ assert.equal(
+ element.shadowRoot.querySelector('#pageSelect').items.length,
+ 6
+ );
done();
});
});
@@ -439,13 +442,16 @@
element.reload().then(() => {
assert.deepEqual(element._filteredLinks, expectedFilteredLinks);
assert.deepEqual(element._subsectionLinks, expectedSubsectionLinks);
- assert.equal(element.$$('#pageSelect').value, 'repoaccess');
+ assert.equal(
+ element.shadowRoot.querySelector('#pageSelect').value,
+ 'repoaccess'
+ );
assert.isTrue(element._selectedIsCurrentPage.calledOnce);
// Doesn't trigger navigation from the page select menu.
assert.isFalse(Gerrit.Nav.navigateToRelativeUrl.called);
// When explicitly changed, navigation is called
- element.$$('#pageSelect').value = 'repo';
+ element.shadowRoot.querySelector('#pageSelect').value = 'repo';
assert.isTrue(element._selectedIsCurrentPage.calledTwice);
assert.isTrue(Gerrit.Nav.navigateToRelativeUrl.calledOnce);
done();
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
index 6cb7e4a..7919b28 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
@@ -385,7 +385,8 @@
assert.isFalse(element._originalExclusiveValue);
assert.isNotOk(element.permission.value.modified);
- MockInteractions.tap(element.$$('#exclusiveToggle'));
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#exclusiveToggle'));
flushAsynchronousOperations();
assert.isTrue(element.permission.value.exclusive);
assert.isTrue(element.permission.value.modified);
@@ -405,21 +406,25 @@
});
test('Exclusive hidden for owner permission', () => {
- assert.equal(getComputedStyle(element.$$('#exclusiveToggle')).display,
- 'flex');
+ assert.equal(getComputedStyle(element.shadowRoot
+ .querySelector('#exclusiveToggle')).display,
+ 'flex');
element.set(['permission', 'id'], 'owner');
flushAsynchronousOperations();
- assert.equal(getComputedStyle(element.$$('#exclusiveToggle')).display,
- 'none');
+ assert.equal(getComputedStyle(element.shadowRoot
+ .querySelector('#exclusiveToggle')).display,
+ 'none');
});
test('Exclusive hidden for any global permissions', () => {
- assert.equal(getComputedStyle(element.$$('#exclusiveToggle')).display,
- 'flex');
+ assert.equal(getComputedStyle(element.shadowRoot
+ .querySelector('#exclusiveToggle')).display,
+ 'flex');
element.section = 'GLOBAL_CAPABILITIES';
flushAsynchronousOperations();
- assert.equal(getComputedStyle(element.$$('#exclusiveToggle')).display,
- 'none');
+ assert.equal(getComputedStyle(element.shadowRoot
+ .querySelector('#exclusiveToggle')).display,
+ 'none');
});
});
});
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
index 90eaba5..fc7dea8 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
@@ -282,7 +282,8 @@
id: 'test-project',
};
flushAsynchronousOperations();
- assert.equal(getComputedStyle(element.$$('#editInheritFromInput'))
+ assert.equal(getComputedStyle(element.shadowRoot
+ .querySelector('#editInheritFromInput'))
.display, 'none');
MockInteractions.tap(element.$.editBtn);
@@ -300,7 +301,8 @@
assert.notEqual(getComputedStyle(element.$.saveBtn).display, 'none');
assert.isTrue(element.$.saveBtn.disabled);
}
- assert.notEqual(getComputedStyle(element.$$('#editInheritFromInput'))
+ assert.notEqual(getComputedStyle(element.shadowRoot
+ .querySelector('#editInheritFromInput'))
.display, 'none');
// Save button should be enabled after access is modified
@@ -365,7 +367,7 @@
id: 'test-project',
};
flushAsynchronousOperations();
- element.$$('#editInheritFromInput').fire('commit');
+ element.shadowRoot.querySelector('#editInheritFromInput').fire('commit');
sandbox.spy(element, '_handleAccessModified');
element.fire('access-modified');
assert.isTrue(element._handleAccessModified.called);
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index 6703651..0817762 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -269,25 +269,6 @@
Do you really want to delete the edit?
</div>
</gr-dialog>
- <gr-dialog
- id="showRevertSubmissionChangesDialog"
- class="confirmDialog"
- confirm-label="Close"
- cancel-label=''
- on-confirm="_handleShowRevertSubmissionChangesConfirm">
- <div class="header" slot="header">
- Reverted Changes
- </div>
- <div class="main" slot="main">
- <template is="dom-repeat" items="[[_revertChanges]]">
- <div>
- <a href$="[[item.link]]" target="_blank">
- Change [[item._number]]
- </a>
- </div>
- </template>
- </div>
- </gr-dialog>
</gr-overlay>
<gr-js-api-interface id="jsAPI"></gr-js-api-interface>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 8ad2a06..9d2854a 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -420,10 +420,6 @@
type: Boolean,
value: true,
},
- _revertChanges: {
- type: Array,
- value: [],
- },
};
}
@@ -1262,7 +1258,6 @@
_handleResponse(action, response) {
if (!response) { return; }
return this.$.restAPI.getResponseObject(response).then(obj => {
- let revertChanges = [];
switch (action.__key) {
case ChangeActions.REVERT:
this._waitForChangeReachable(obj._number)
@@ -1288,27 +1283,11 @@
Gerrit.Nav.navigateToChange(this.change);
break;
case ChangeActions.REVERT_SUBMISSION:
- revertChanges = obj.revert_changes || [];
- revertChanges = revertChanges.map(change => {
- change.link = '/q/' + encodeURIComponent(change.change_id);
- return change;
- });
- // list of reverted changes can never be 0
- if (revertChanges.length === 1) {
- // redirect to the change if only 1 change is reverted
- const change = revertChanges[0];
- this._waitForChangeReachable(change._number).then(success => {
- if (success) {
- Gerrit.Nav.navigateToChange(change);
- } else {
- console.error('Change ' + change._number + ' not reachable');
- }
- });
- } else {
- // show multiple reverted changes in a dialog
- this._revertChanges = revertChanges;
- this._showActionDialog(this.$.showRevertSubmissionChangesDialog);
- }
+ if (!obj.revert_changes || !obj.revert_changes.length) return;
+ /* If there is only 1 change then gerrit will automatically
+ redirect to that change */
+ Gerrit.Nav.navigateToSearchQuery('topic: ' +
+ obj.revert_changes[0].topic);
break;
default:
this.dispatchEvent(new CustomEvent('reload-change',
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 532c573..50f6fb0 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -466,7 +466,9 @@
element._handleDeleteEditTap();
assert.isFalse(element.$.confirmDeleteEditDialog.hidden);
MockInteractions.tap(
- element.$$('#confirmDeleteEditDialog').$$('gr-button[primary]'));
+ element.shadowRoot
+ .querySelector('#confirmDeleteEditDialog')
+ .$$('gr-button[primary]'));
flushAsynchronousOperations();
assert.equal(fireActionStub.lastCall.args[0], '/edit');
@@ -980,9 +982,12 @@
test('shows confirm dialog', () => {
element._handleDeleteTap();
- assert.isFalse(element.$$('#confirmDeleteDialog').hidden);
+ assert.isFalse(element.shadowRoot
+ .querySelector('#confirmDeleteDialog').hidden);
MockInteractions.tap(
- element.$$('#confirmDeleteDialog').$$('gr-button[primary]'));
+ element.shadowRoot
+ .querySelector('#confirmDeleteDialog')
+ .$$('gr-button[primary]'));
flushAsynchronousOperations();
assert.isTrue(fireActionStub.calledWith('/', deleteAction, false));
});
@@ -990,9 +995,12 @@
test('hides delete confirm on cancel', () => {
element._handleDeleteTap();
MockInteractions.tap(
- element.$$('#confirmDeleteDialog').$$('gr-button:not([primary])'));
+ element.shadowRoot
+ .querySelector('#confirmDeleteDialog')
+ .$$('gr-button:not([primary])'));
flushAsynchronousOperations();
- assert.isTrue(element.$$('#confirmDeleteDialog').hidden);
+ assert.isTrue(element.shadowRoot
+ .querySelector('#confirmDeleteDialog').hidden);
assert.isFalse(fireActionStub.called);
});
});
@@ -1438,7 +1446,6 @@
suite('happy path', () => {
let sendStub;
- let waitForChangeReachableStub;
setup(() => {
sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({isLatest: true}));
@@ -1446,8 +1453,6 @@
.returns(Promise.resolve({}));
getResponseObjectStub = sandbox.stub(element.$.restAPI,
'getResponseObject');
- waitForChangeReachableStub = sandbox.stub(element,
- '_waitForChangeReachable').returns(Promise.resolve(true));
sandbox.stub(Gerrit.Nav,
'navigateToChange').returns(Promise.resolve(true));
});
@@ -1463,12 +1468,15 @@
});
suite('single changes revert', () => {
+ let navigateToSearchQueryStub;
setup(() => {
getResponseObjectStub
.returns(Promise.resolve({revert_changes: [
{change_id: 12345},
]}));
showActionDialogStub = sandbox.stub(element, '_showActionDialog');
+ navigateToSearchQueryStub = sandbox.stub(Gerrit.Nav,
+ 'navigateToSearchQuery');
});
test('revert submission single change', done => {
@@ -1476,7 +1484,7 @@
'/revert_submission', false, cleanup).then(res => {
element._handleResponse({__key: 'revert_submission'}, {}).
then(() => {
- assert.isTrue(waitForChangeReachableStub.called);
+ assert.isTrue(navigateToSearchQueryStub.called);
done();
});
});
@@ -1485,12 +1493,16 @@
suite('multiple changes revert', () => {
let showActionDialogStub;
+ let navigateToSearchQueryStub;
setup(() => {
getResponseObjectStub
.returns(Promise.resolve({revert_changes: [
- {change_id: 12345}, {change_id: 23456},
+ {change_id: 12345, topic: 'T'},
+ {change_id: 23456, topic: 'T'},
]}));
showActionDialogStub = sandbox.stub(element, '_showActionDialog');
+ navigateToSearchQueryStub = sandbox.stub(Gerrit.Nav,
+ 'navigateToSearchQuery');
});
test('revert submission multiple change', done => {
@@ -1498,7 +1510,9 @@
'/revert_submission', false, cleanup).then(res => {
element._handleResponse({__key: 'revert_submission'}, {}).then(
() => {
- assert.isTrue(showActionDialogStub.called);
+ assert.isFalse(showActionDialogStub.called);
+ assert.isTrue(navigateToSearchQueryStub.calledWith(
+ 'topic: T'));
done();
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 5de378b..b9e9e6c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -437,7 +437,8 @@
}
_handleFileTabChange(e) {
- const selectedIndex = this.$$('#primaryTabs').selected;
+ const selectedIndex = this.shadowRoot
+ .querySelector('#primaryTabs').selected;
this._showFileTabContent = selectedIndex === 0;
// Initial tab is the static files list.
const newSelectedTab =
@@ -458,8 +459,8 @@
console.warn(e.detail.tab + ' tab not found');
return;
}
- this.$$('#primaryTabs').selected = idx + 1;
- this.$$('#primaryTabs').scrollIntoView();
+ this.shadowRoot.querySelector('#primaryTabs').selected = idx + 1;
+ this.shadowRoot.querySelector('#primaryTabs').scrollIntoView();
this.$.reporting.reportInteraction('show-tab', e.detail.tab);
}
@@ -799,7 +800,7 @@
// Selected has to be set after the paper-tabs are visible because
// the selected underline depends on calculations made by the browser.
this.$.commentTabs.selected = 0;
- const primaryTabs = this.$$('#primaryTabs');
+ const primaryTabs = this.shadowRoot.querySelector('#primaryTabs');
if (primaryTabs) primaryTabs.selected = 0;
}
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js
index ab849de..9329ba5 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js
@@ -68,7 +68,8 @@
}
getFocusStops() {
- const links = this.$$('#archives').querySelectorAll('a');
+ const links = this.shadowRoot
+ .querySelector('#archives').querySelectorAll('a');
return {
start: this.$.closeButton,
end: links[links.length - 1],
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
index ea7ea8f..10efaff 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
@@ -216,8 +216,8 @@
test('expand/collapse buttons are toggled correctly', () => {
element.shownFileCount = 10;
flushAsynchronousOperations();
- const expandBtn = element.$$('#expandBtn');
- const collapseBtn = element.$$('#collapseBtn');
+ const expandBtn = element.shadowRoot.querySelector('#expandBtn');
+ const collapseBtn = element.shadowRoot.querySelector('#collapseBtn');
assert.notEqual(getComputedStyle(expandBtn).display, 'none');
assert.equal(getComputedStyle(collapseBtn).display, 'none');
element.filesExpanded = GrFileListConstants.FilesExpandedState.SOME;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 318633d..0a7f1ce 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -116,6 +116,9 @@
cursor: pointer;
flex: 1;
text-decoration: none;
+ /* Wrap it into multiple lines if too long. */
+ white-space: normal;
+ word-break: break-word;
}
.path:hover :first-child {
text-decoration: underline;
@@ -197,10 +200,6 @@
.truncatedFileName {
display: none;
}
- .expanded .fullFileName {
- white-space: normal;
- word-wrap: break-word;
- }
.mobile {
display: none;
}
@@ -247,7 +246,9 @@
margin: -2px 0;
padding: var(--spacing-s) 0;
}
- @media screen and (max-width: 50em) {
+
+ /** small screen breakpoint: 768px */
+ @media screen and (max-width: 55em) {
.desktop {
display: none;
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index fb57164..9943192 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -339,9 +339,9 @@
}
_calculatePatchChange(files) {
- const magicFilesExcluded = files.filter(files => {
- return files.__path !== '/COMMIT_MSG' && files.__path !== '/MERGE_LIST';
- });
+ const magicFilesExcluded = files.filter(files =>
+ !this.isMagicPath(files.__path)
+ );
return magicFilesExcluded.reduce((acc, obj) => {
const inserted = obj.lines_inserted ? obj.lines_inserted : 0;
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
index 9caf13d..b0747f4 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
@@ -231,7 +231,8 @@
.concat(_.times(11, randomMessage));
flushAsynchronousOperations();
- MockInteractions.tap(element.$$('#collapse-messages')); // Expand all.
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages')); // Expand all.
flushAsynchronousOperations();
let messages = getMessages();
@@ -255,8 +256,10 @@
.concat(_.times(11, randomMessage));
flushAsynchronousOperations();
- MockInteractions.tap(element.$$('#collapse-messages')); // Expand all.
- MockInteractions.tap(element.$$('#collapse-messages')); // Collapse all.
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages')); // Expand all.
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages')); // Collapse all.
flushAsynchronousOperations();
let messages = getMessages();
@@ -283,13 +286,15 @@
MockInteractions.tap(allMessageEls[1]);
assert.isTrue(allMessageEls[1]._expanded);
- MockInteractions.tap(element.$$('#collapse-messages'));
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages'));
allMessageEls = getMessages();
for (const message of allMessageEls) {
assert.isTrue(message._expanded);
}
- MockInteractions.tap(element.$$('#collapse-messages'));
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages'));
allMessageEls = getMessages();
for (const message of allMessageEls) {
assert.isFalse(message._expanded);
@@ -297,28 +302,33 @@
});
test('expand/collapse from external keypress', () => {
- MockInteractions.tap(element.$$('#collapse-messages'));
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages'));
let allMessageEls = getMessages();
for (const message of allMessageEls) {
assert.isTrue(message._expanded);
}
// Expand/collapse all text also changes.
- assert.equal(element.$$('#collapse-messages').textContent.trim(),
- 'Collapse all');
+ assert.equal(element.shadowRoot
+ .querySelector('#collapse-messages').textContent.trim(),
+ 'Collapse all');
- MockInteractions.tap(element.$$('#collapse-messages'));
+ MockInteractions.tap(element.shadowRoot
+ .querySelector('#collapse-messages'));
allMessageEls = getMessages();
for (const message of allMessageEls) {
assert.isFalse(message._expanded);
}
// Expand/collapse all text also changes.
- assert.equal(element.$$('#collapse-messages').textContent.trim(),
- 'Expand all');
+ assert.equal(element.shadowRoot
+ .querySelector('#collapse-messages').textContent.trim(),
+ 'Expand all');
});
test('hide messages does not appear when no automated messages', () => {
- assert.isOk(element.$$('#automatedMessageToggleContainer[hidden]'));
+ assert.isOk(element.shadowRoot
+ .querySelector('#automatedMessageToggleContainer[hidden]'));
});
test('scroll to message', () => {
@@ -476,7 +486,8 @@
});
test('hide autogenerated button is not hidden', () => {
- assert.isNotOk(element.$$('#automatedMessageToggle[hidden]'));
+ assert.isNotOk(element.shadowRoot
+ .querySelector('#automatedMessageToggle[hidden]'));
});
test('autogenerated messages are not hidden initially', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index badd62a..e44f669 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -187,7 +187,7 @@
});
test('keep drafts with reply', done => {
- MockInteractions.tap(element.$$('#includeComments'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#includeComments'));
assert.equal(element._includeComments, false);
// Async tick is needed because iron-selector content is distributed and
@@ -460,7 +460,8 @@
flushAsynchronousOperations();
assert.isFalse(element._reviewersMutated);
assert.isTrue(element.$.ccs.allowAnyInput);
- assert.isFalse(element.$$('#reviewers').allowAnyInput);
+ assert.isFalse(element.shadowRoot
+ .querySelector('#reviewers').allowAnyInput);
element.$.ccs.dispatchEvent(new CustomEvent('account-text-changed',
{bubbles: true, composed: true}));
assert.isTrue(element._reviewersMutated);
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
index d169728..3f2f4bd 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
@@ -24,7 +24,6 @@
CATEGORY_RPC: 'RPC Timing',
// Reported events - alphabetize below.
APP_STARTED: 'App Started',
- PAGE_LOADED: 'Page Loaded',
};
// Plugin-related reporting constants.
@@ -298,38 +297,21 @@
console.error('pageLoaded should be called after window.onload');
this.async(this.pageLoaded, 100);
} else {
- const loadTime = this.performanceTiming.loadEventEnd -
+ const perfEvents = Object.keys(this.performanceTiming.toJSON());
+ perfEvents.forEach(
+ eventName => this._reportPerformanceTiming(eventName)
+ );
+ }
+ },
+
+ _reportPerformanceTiming(eventName) {
+ const eventTiming = this.performanceTiming[eventName];
+ if (eventTiming > 0) {
+ const elapsedTime = eventTiming -
this.performanceTiming.navigationStart;
+ // NavResTime - Navigation and resource timings.
this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
- TIMING.PAGE_LOADED, loadTime, true);
-
- const requestStart = this.performanceTiming.requestStart -
- this.performanceTiming.navigationStart;
- this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
- 'requestStart', requestStart, true);
-
- const responseEnd = this.performanceTiming.responseEnd -
- this.performanceTiming.navigationStart;
- this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
- 'responseEnd', responseEnd, true);
-
- const domLoading = this.performanceTiming.domLoading -
- this.performanceTiming.navigationStart;
- this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
- 'domLoading', domLoading, true);
-
- const domContentLoadedEventStart =
- this.performanceTiming.domContentLoadedEventStart -
- this.performanceTiming.navigationStart;
- this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
- 'domContentLoadedEventStart', domContentLoadedEventStart, true);
-
- if (this.performanceTiming.redirectEnd > 0) {
- const redirectEnd = this.performanceTiming.redirectEnd -
- this.performanceTiming.navigationStart;
- this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
- 'redirectEnd', redirectEnd, true);
- }
+ `NavResTime - ${eventName}`, elapsedTime, true);
}
},
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
index c2c0297..430b41e 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
@@ -52,6 +52,7 @@
navigationStart: 1,
loadEventEnd: 2,
};
+ fakePerformance.toJSON = () => fakePerformance;
sinon.stub(element, 'performanceTiming',
{get() { return fakePerformance; }});
sandbox.stub(element, 'reporter');
@@ -83,7 +84,7 @@
element.pageLoaded();
assert.isTrue(
element.reporter.calledWithExactly(
- 'timing-report', 'UI Latency', 'Page Loaded',
+ 'timing-report', 'UI Latency', 'NavResTime - loadEventEnd',
fakePerformance.loadEventEnd - fakePerformance.navigationStart,
true)
);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index 039a99fa..fc4173c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -35,9 +35,9 @@
const LEFT_SIDE_CLASS = 'target-side-left';
const RIGHT_SIDE_CLASS = 'target-side-right';
- class GrDiffCursor extends Polymer.GestureEventListeners(
- Polymer.LegacyElementMixin(
- Polymer.Element)) {
+ class GrDiffCursor extends Polymer.mixinBehaviors([Gerrit.FireBehavior],
+ Polymer.GestureEventListeners(
+ Polymer.LegacyElementMixin(Polymer.Element))) {
static get is() { return 'gr-diff-cursor'; }
static get properties() {
@@ -105,6 +105,23 @@
];
}
+ ready() {
+ super.ready();
+ Polymer.RenderStatus.afterNextRender(this, () => {
+ /*
+ This represents the diff cursor is ready for interaction coming from
+ client components. It is more then Polymer "ready" lifecycle, as no
+ "ready" events are automatically fired by Polymer, it means
+ the cursor is completely interactable - in this case attached and
+ painted on the page. We name it "ready" instead of "rendered" as the
+ long-term goal is to make gr-diff-cursor a javascript class - not a DOM
+ element with an actual lifecycle. This will be triggered only once
+ per element.
+ */
+ this.fire('ready', null, {bubbles: false});
+ });
+ }
+
attached() {
super.attached();
// Catch when users are scrolling as the view loads.
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 626ab33..dff9e79 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
@@ -42,6 +42,12 @@
</template>
</test-fixture>
+<test-fixture id="empty">
+ <template>
+ <div></div>
+ </template>
+</test-fixture>
+
<script>
suite('gr-diff-cursor tests', () => {
let sandbox;
@@ -370,4 +376,24 @@
});
});
});
+
+ suite('gr-diff-cursor event tests', () => {
+ let sandbox;
+ let someEmptyDiv;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ someEmptyDiv = fixture('empty');
+ });
+
+ teardown(() => sandbox.restore());
+
+ test('ready is fired after component is rendered', done => {
+ const cursorElement = document.createElement('gr-diff-cursor');
+ cursorElement.addEventListener('ready', () => {
+ done();
+ });
+ someEmptyDiv.appendChild(cursorElement);
+ });
+ });
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 40a02a3..cd53510 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -277,7 +277,7 @@
</span>
</div>
<div class="rightControls">
- <span class$="blameLoader [[_computeBlameLoaderClass(_isImageDiff)]]">
+ <span class$="blameLoader [[_computeBlameLoaderClass(_isImageDiff, _path)]]">
<gr-button
link
disabled="[[_isBlameLoading]]"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 4c401cf..2f2b3863 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -1101,8 +1101,8 @@
});
}
- _computeBlameLoaderClass(isImageDiff) {
- return !isImageDiff ? 'show' : '';
+ _computeBlameLoaderClass(isImageDiff, path) {
+ return !this.isMagicPath(path) && !isImageDiff ? 'show' : '';
}
_getRevisionInfo(change) {
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
index 2d7dc9c..b7e52a1 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
@@ -82,7 +82,7 @@
});
test('open', () => {
- MockInteractions.tap(element.$$('#open'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#open'));
element.patchNum = 1;
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element._hideAllDialogs.called);
@@ -101,7 +101,7 @@
});
test('cancel', () => {
- MockInteractions.tap(element.$$('#open'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#open'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.openDialog.disabled);
openAutoCcmplete.noDebounce = true;
@@ -129,7 +129,7 @@
test('delete', () => {
deleteStub.returns(Promise.resolve({ok: true}));
- MockInteractions.tap(element.$$('#delete'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#delete'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.deleteDialog.disabled);
assert.isFalse(queryStub.called);
@@ -152,7 +152,7 @@
test('delete fails', () => {
deleteStub.returns(Promise.resolve({ok: false}));
- MockInteractions.tap(element.$$('#delete'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#delete'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.deleteDialog.disabled);
assert.isFalse(queryStub.called);
@@ -173,7 +173,7 @@
});
test('cancel', () => {
- MockInteractions.tap(element.$$('#delete'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#delete'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.deleteDialog.disabled);
element.$.deleteDialog.querySelector('gr-autocomplete').text =
@@ -204,7 +204,7 @@
test('rename', () => {
renameStub.returns(Promise.resolve({ok: true}));
- MockInteractions.tap(element.$$('#rename'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#rename'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.renameDialog.disabled);
assert.isFalse(queryStub.called);
@@ -232,7 +232,7 @@
test('rename fails', () => {
renameStub.returns(Promise.resolve({ok: false}));
- MockInteractions.tap(element.$$('#rename'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#rename'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.renameDialog.disabled);
assert.isFalse(queryStub.called);
@@ -258,7 +258,7 @@
});
test('cancel', () => {
- MockInteractions.tap(element.$$('#rename'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#rename'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.renameDialog.disabled);
element.$.renameDialog.querySelector('gr-autocomplete').text =
@@ -285,13 +285,14 @@
});
test('restore hidden by default', () => {
- assert.isTrue(element.$$('#restore').classList.contains('invisible'));
+ assert.isTrue(element.shadowRoot
+ .querySelector('#restore').classList.contains('invisible'));
});
test('restore', () => {
restoreStub.returns(Promise.resolve({ok: true}));
element._path = 'src/test.cpp';
- MockInteractions.tap(element.$$('#restore'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#restore'));
return showDialogSpy.lastCall.returnValue.then(() => {
MockInteractions.tap(element.$.restoreDialog.$$('gr-button[primary]'));
flushAsynchronousOperations();
@@ -309,7 +310,7 @@
test('restore fails', () => {
restoreStub.returns(Promise.resolve({ok: false}));
element._path = 'src/test.cpp';
- MockInteractions.tap(element.$$('#restore'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#restore'));
return showDialogSpy.lastCall.returnValue.then(() => {
MockInteractions.tap(element.$.restoreDialog.$$('gr-button[primary]'));
flushAsynchronousOperations();
@@ -325,7 +326,7 @@
test('cancel', () => {
element._path = 'src/test.cpp';
- MockInteractions.tap(element.$$('#restore'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#restore'));
return showDialogSpy.lastCall.returnValue.then(() => {
MockInteractions.tap(element.$.restoreDialog.$$('gr-button'));
assert.isFalse(navStub.called);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
index 89b28d5..0567777 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
@@ -90,6 +90,7 @@
as="comment">
<gr-comment
comment="{{comment}}"
+ comments="{{comments}}"
robot-button-disabled="[[_shouldDisableAction(_showActions, _lastComment)]]"
change-num="[[changeNum]]"
patch-num="[[patchNum]]"
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
index 8687e77..779e247 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
@@ -373,7 +373,7 @@
const comment = e.detail.comment;
const msg = comment.message;
const quoteStr = '> ' + msg.replace(NEWLINE_PATTERN, '\n> ') + '\n\n';
- const response = quoteStr + 'Please Fix';
+ const response = quoteStr + 'Please fix.';
this._createReplyComment(comment, response, false, true);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html
index 17e0b1f..ceadae1 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html
@@ -420,7 +420,7 @@
});
assert.equal(drafts.length, 1);
assert.equal(
- drafts[0].message, '> is this a crossover episode!?\n\nPlease Fix');
+ drafts[0].message, '> is this a crossover episode!?\n\nPlease fix.');
assert.equal(drafts[0].in_reply_to, 'baf0414d_60047215');
assert.isTrue(drafts[0].unresolved);
done();
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
index cb87bd9..d1d60f1 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
@@ -60,7 +60,7 @@
display: none;
}
.header {
- align-items: baseline;
+ align-items: center;
cursor: pointer;
display: flex;
margin: calc(0px - var(--spacing-m)) calc(0px - var(--spacing-m)) 0 calc(0px - var(--spacing-m));
@@ -271,7 +271,7 @@
secondary
class$="action delete [[_computeDeleteButtonClass(_isAdmin, draft)]]"
on-click="_handleCommentDelete">
- (Delete)
+ <iron-icon id="icon" icon="gr-icons:delete"></iron-icon>
</gr-button>
<span class="date" on-click="_handleAnchorClick">
<gr-date-formatter
@@ -348,14 +348,16 @@
</div>
<div class="robotActions" hidden$="[[!_showRobotActions]]">
<template is="dom-if" if="[[isRobotComment]]">
- <gr-button
- link
- secondary
- class="action fix"
- on-click="_handleFix"
- disabled="[[robotButtonDisabled]]">
- Please Fix
- </gr-button>
+ <template is="dom-if" if="[[!_hasHumanReply]]">
+ <gr-button
+ link
+ secondary
+ class="action fix"
+ on-click="_handleFix"
+ disabled="[[robotButtonDisabled]]">
+ Please Fix
+ </gr-button>
+ </template>
<gr-endpoint-decorator name="robot-comment-controls">
<gr-endpoint-param name="comment" value="[[comment]]">
</gr-endpoint-param>
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
index 053eeee..eb4081e 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
@@ -81,6 +81,9 @@
notify: true,
observer: '_commentChanged',
},
+ comments: {
+ type: Array,
+ },
isRobotComment: {
type: Boolean,
value: false,
@@ -119,6 +122,7 @@
/** @type {?} */
projectConfig: Object,
robotButtonDisabled: Boolean,
+ _hasHumanReply: Boolean,
_isAdmin: {
type: Boolean,
value: false,
@@ -165,6 +169,7 @@
'_loadLocalDraft(changeNum, patchNum, comment)',
'_isRobotComment(comment)',
'_calculateActionstoShow(showActions, isRobotComment)',
+ '_computeHasHumanReply(comment, comments.*)',
];
}
@@ -196,14 +201,15 @@
}
get textarea() {
- return this.$$('#editTextarea');
+ return this.shadowRoot.querySelector('#editTextarea');
}
get confirmDeleteOverlay() {
if (!this._overlays.confirmDelete) {
this._enableOverlay = true;
Polymer.dom.flush();
- this._overlays.confirmDelete = this.$$('#confirmDeleteOverlay');
+ this._overlays.confirmDelete = this.shadowRoot
+ .querySelector('#confirmDeleteOverlay');
}
return this._overlays.confirmDelete;
}
@@ -212,7 +218,8 @@
if (!this._overlays.confirmDiscard) {
this._enableOverlay = true;
Polymer.dom.flush();
- this._overlays.confirmDiscard = this.$$('#confirmDiscardOverlay');
+ this._overlays.confirmDiscard = this.shadowRoot
+ .querySelector('#confirmDiscardOverlay');
}
return this._overlays.confirmDiscard;
}
@@ -308,6 +315,15 @@
}
}
+ _computeHasHumanReply() {
+ if (!this.comment || !this.comments) return;
+ // hide please fix button for robot comment that has human reply
+ this._hasHumanReply = this.comments.some(c => {
+ return c.in_reply_to && c.in_reply_to === this.comment.id
+ && !c.robot_id;
+ });
+ }
+
/**
* @param {!Object=} opt_mixin
*
@@ -354,7 +370,7 @@
if (editing) {
this.async(() => {
Polymer.dom.flush();
- this.textarea.putCursorAtEnd();
+ this.textarea && this.textarea.putCursorAtEnd();
}, 1);
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
index ad8b5e4..81dfaeb 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
@@ -867,9 +867,145 @@
done();
});
element.isRobotComment = true;
+ element.comments = [element.comment];
flushAsynchronousOperations();
MockInteractions.tap(element.$$('.fix'));
});
+
+ test('do not show Please Fix button if human reply exists', () => {
+ element.comments = [
+ {
+ robot_id: 'happy_robot_id',
+ robot_run_id: '5838406743490560',
+ fix_suggestions: [
+ {
+ fix_id: '478ff847_3bf47aaf',
+ description: 'Make the smiley happier by giving it a nose.',
+ replacements: [
+ {
+ path: 'Documentation/config-gerrit.txt',
+ range: {
+ start_line: 10,
+ start_character: 7,
+ end_line: 10,
+ end_character: 9,
+ },
+ replacement: ':-)',
+ },
+ ],
+ },
+ ],
+ author: {
+ _account_id: 1030912,
+ name: 'Alice Kober-Sotzek',
+ email: 'aliceks@google.com',
+ avatars: [
+ {
+ url: '/s32-p/photo.jpg',
+ height: 32,
+ },
+ {
+ url: '/AaAdOFzPlFI/s56-p/photo.jpg',
+ height: 56,
+ },
+ {
+ url: '/AaAdOFzPlFI/s100-p/photo.jpg',
+ height: 100,
+ },
+ {
+ url: '/AaAdOFzPlFI/s120-p/photo.jpg',
+ height: 120,
+ },
+ ],
+ },
+ patch_set: 1,
+ id: 'eb0d03fd_5e95904f',
+ line: 10,
+ updated: '2017-04-04 15:36:17.000000000',
+ message: 'This is a robot comment with a fix.',
+ unresolved: false,
+ __commentSide: 'right',
+ collapsed: false,
+ },
+ {
+ __draft: true,
+ __draftID: '0.wbrfbwj89sa',
+ __date: '2019-12-04T13:41:03.689Z',
+ path: 'Documentation/config-gerrit.txt',
+ patchNum: 1,
+ side: 'REVISION',
+ __commentSide: 'right',
+ line: 10,
+ in_reply_to: 'eb0d03fd_5e95904f',
+ message: '> This is a robot comment with a fix.\n\nPlease fix.',
+ unresolved: true,
+ },
+ ];
+ element.comment = element.comments[0];
+ flushAsynchronousOperations();
+ assert.isNull(element.$$('robotActions gr-button'));
+ });
+
+ test('show Please Fix if no human reply', () => {
+ element.comments = [
+ {
+ robot_id: 'happy_robot_id',
+ robot_run_id: '5838406743490560',
+ fix_suggestions: [
+ {
+ fix_id: '478ff847_3bf47aaf',
+ description: 'Make the smiley happier by giving it a nose.',
+ replacements: [
+ {
+ path: 'Documentation/config-gerrit.txt',
+ range: {
+ start_line: 10,
+ start_character: 7,
+ end_line: 10,
+ end_character: 9,
+ },
+ replacement: ':-)',
+ },
+ ],
+ },
+ ],
+ author: {
+ _account_id: 1030912,
+ name: 'Alice Kober-Sotzek',
+ email: 'aliceks@google.com',
+ avatars: [
+ {
+ url: '/s32-p/photo.jpg',
+ height: 32,
+ },
+ {
+ url: '/AaAdOFzPlFI/s56-p/photo.jpg',
+ height: 56,
+ },
+ {
+ url: '/AaAdOFzPlFI/s100-p/photo.jpg',
+ height: 100,
+ },
+ {
+ url: '/AaAdOFzPlFI/s120-p/photo.jpg',
+ height: 120,
+ },
+ ],
+ },
+ patch_set: 1,
+ id: 'eb0d03fd_5e95904f',
+ line: 10,
+ updated: '2017-04-04 15:36:17.000000000',
+ message: 'This is a robot comment with a fix.',
+ unresolved: false,
+ __commentSide: 'right',
+ collapsed: false,
+ },
+ ];
+ element.comment = element.comments[0];
+ flushAsynchronousOperations();
+ assert.isNotNull(element.$$('.robotActions gr-button'));
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.js
index 2d66cfa..0ec3d6a 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.js
@@ -50,7 +50,11 @@
return url.pathname;
}
const base = Gerrit.BaseUrlBehavior.getBaseUrl();
- const pathname = url.pathname.replace(base, '');
+ let pathname = url.pathname.replace(base, '');
+ // Load from ASSETS_PATH
+ if (window.ASSETS_PATH && url.href.includes(window.ASSETS_PATH)) {
+ pathname = url.href.replace(window.ASSETS_PATH, '');
+ }
// Site theme is server from predefined path.
if (pathname === '/static/gerrit-theme.html') {
return 'gerrit-theme';
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils_test.html
index 128738d..b43796f 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils_test.html
@@ -72,6 +72,15 @@
'gerrit-theme'
);
});
+
+ test('with ASSETS_PATH', () => {
+ window.ASSETS_PATH = 'http://cdn.com/2';
+ assert.equal(
+ getPluginNameFromUrl(`${window.ASSETS_PATH}/plugins/a.html`),
+ 'a'
+ );
+ window.ASSETS_PATH = undefined;
+ });
});
});
</script>
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
index 537e55b..bdce91f 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
@@ -82,6 +82,28 @@
'http://test.com/plugins/testplugin/static/test.js');
});
+ test('url for preloaded plugin without ASSETS_PATH', () => {
+ let plugin;
+ Gerrit.install(p => { plugin = p; }, '0.1',
+ 'preloaded:testpluginB');
+ assert.equal(plugin.url(),
+ `${window.location.origin}/plugins/testpluginB/`);
+ assert.equal(plugin.url('/static/test.js'),
+ `${window.location.origin}/plugins/testpluginB/static/test.js`);
+ });
+
+ test('url for preloaded plugin without ASSETS_PATH', () => {
+ const oldAssetsPath = window.ASSETS_PATH;
+ window.ASSETS_PATH = 'http://test.com';
+ let plugin;
+ Gerrit.install(p => { plugin = p; }, '0.1',
+ 'preloaded:testpluginC');
+ assert.equal(plugin.url(), `${window.ASSETS_PATH}/plugins/testpluginC/`);
+ assert.equal(plugin.url('/static/test.js'),
+ `${window.ASSETS_PATH}/plugins/testpluginC/static/test.js`);
+ window.ASSETS_PATH = oldAssetsPath;
+ });
+
test('_send on failure rejects with response text', () => {
sendStub.returns(Promise.resolve(
{status: 400, text() { return Promise.resolve('text'); }}));
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.js
index 4be38b6..081ce55 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.js
@@ -113,7 +113,7 @@
this._pluginListLoaded = true;
plugins.forEach(path => {
- const url = this._urlFor(path);
+ const url = this._urlFor(path, window.ASSETS_PATH);
// Skip if preloaded, for bundling.
if (this.isPluginPreloaded(url)) return;
@@ -128,11 +128,11 @@
});
if (this._isPathEndsWith(url, '.html')) {
- this._importHtmlPlugin(url, opts && opts[path]);
+ this._importHtmlPlugin(path, opts && opts[path]);
} else if (this._isPathEndsWith(url, '.js')) {
- this._loadJsPlugin(url);
+ this._loadJsPlugin(path);
} else {
- this._failToLoad(`Unrecognized plugin url ${url}`, url);
+ this._failToLoad(`Unrecognized plugin path ${path}`, path);
}
});
@@ -181,14 +181,15 @@
return;
}
- const pluginObject = this.getPlugin(src);
+ const url = this._urlFor(src);
+ const pluginObject = this.getPlugin(url);
let plugin = pluginObject && pluginObject.plugin;
if (!plugin) {
- plugin = new Plugin(src);
+ plugin = new Plugin(url);
}
try {
callback(plugin);
- this._pluginInstalled(src, plugin);
+ this._pluginInstalled(url, plugin);
} catch (e) {
this._failToLoad(`${e.name}: ${e.message}`, src);
}
@@ -313,38 +314,79 @@
}
_importHtmlPlugin(pluginUrl, opts = {}) {
- // onload (second param) needs to be a function. When null or undefined
- // were passed, plugins were not loaded correctly.
+ const urlWithAP = this._urlFor(pluginUrl, window.ASSETS_PATH);
+ const urlWithoutAP = this._urlFor(pluginUrl);
+ let onerror = null;
+ if (urlWithAP !== urlWithoutAP) {
+ onerror = () => this._loadHtmlPlugin(urlWithoutAP, opts.sync);
+ }
+ this._loadHtmlPlugin(urlWithAP, opts.sync, onerror);
+ }
+
+ _loadHtmlPlugin(url, sync, onerror) {
+ if (!onerror) {
+ onerror = () => {
+ this._failToLoad(`${url} import error`, url);
+ };
+ }
+
(Polymer.importHref || Polymer.Base.importHref)(
- this._urlFor(pluginUrl), () => {},
- () => this._failToLoad(`${pluginUrl} import error`, pluginUrl),
- !opts.sync);
+ url, () => {},
+ onerror,
+ !sync);
}
_loadJsPlugin(pluginUrl) {
- this._createScriptTag(this._urlFor(pluginUrl));
+ const urlWithAP = this._urlFor(pluginUrl, window.ASSETS_PATH);
+ const urlWithoutAP = this._urlFor(pluginUrl);
+ let onerror = null;
+ if (urlWithAP !== urlWithoutAP) {
+ onerror = () => this._createScriptTag(urlWithoutAP);
+ }
+
+ this._createScriptTag(urlWithAP, onerror);
}
- _createScriptTag(url) {
+ _createScriptTag(url, onerror) {
+ if (!onerror) {
+ onerror = () => this._failToLoad(`${url} load error`, url);
+ }
+
const el = document.createElement('script');
el.defer = true;
el.setAttribute('src', url);
- el.onerror = () => this._failToLoad(`${url} load error`, url);
+ el.onerror = onerror;
return document.body.appendChild(el);
}
- _urlFor(pathOrUrl) {
+ _urlFor(pathOrUrl, assetsPath) {
if (!pathOrUrl) {
return pathOrUrl;
}
+
+ // theme is per host, should always load from assetsPath
+ const isThemeFile = pathOrUrl.endsWith('static/gerrit-theme.html');
+ const shouldTryLoadFromAssetsPathFirst = !isThemeFile && assetsPath;
if (pathOrUrl.startsWith(PRELOADED_PROTOCOL) ||
pathOrUrl.startsWith('http')) {
// Plugins are loaded from another domain or preloaded.
+ if (pathOrUrl.includes(location.host)
+ && shouldTryLoadFromAssetsPathFirst) {
+ // if is loading from host server, try replace with cdn when assetsPath provided
+ return pathOrUrl
+ .replace(location.origin, assetsPath);
+ }
return pathOrUrl;
}
+
if (!pathOrUrl.startsWith('/')) {
pathOrUrl = '/' + pathOrUrl;
}
+
+ if (shouldTryLoadFromAssetsPathFirst) {
+ return assetsPath + pathOrUrl;
+ }
+
return window.location.origin + getBaseUrl() + pathOrUrl;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.html
index 8c1ec96..151c340 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.html
@@ -325,11 +325,11 @@
let loadJsPluginStub;
setup(() => {
importHtmlPluginStub = sandbox.stub();
- sandbox.stub(Gerrit._pluginLoader, '_importHtmlPlugin', url => {
+ sandbox.stub(Gerrit._pluginLoader, '_loadHtmlPlugin', url => {
importHtmlPluginStub(url);
});
loadJsPluginStub = sandbox.stub();
- sandbox.stub(Gerrit._pluginLoader, '_loadJsPlugin', url => {
+ sandbox.stub(Gerrit._pluginLoader, '_createScriptTag', url => {
loadJsPluginStub(url);
});
});
@@ -346,8 +346,8 @@
assert.isTrue(failToLoadStub.calledOnce);
assert.isTrue(failToLoadStub.calledWithExactly(
- `Unrecognized plugin url ${url}/foo/bar`,
- `${url}/foo/bar`
+ 'Unrecognized plugin path foo/bar',
+ 'foo/bar'
));
});
@@ -407,6 +407,72 @@
});
});
+ suite('With ASSETS_PATH', () => {
+ let importHtmlPluginStub;
+ let loadJsPluginStub;
+ setup(() => {
+ window.ASSETS_PATH = 'https://cdn.com';
+ importHtmlPluginStub = sandbox.stub();
+ sandbox.stub(Gerrit._pluginLoader, '_loadHtmlPlugin', url => {
+ importHtmlPluginStub(url);
+ });
+ loadJsPluginStub = sandbox.stub();
+ sandbox.stub(Gerrit._pluginLoader, '_createScriptTag', url => {
+ loadJsPluginStub(url);
+ });
+ });
+
+ teardown(() => {
+ window.ASSETS_PATH = '';
+ });
+
+ test('Should try load plugins from assets path instead', () => {
+ Gerrit._loadPlugins([
+ 'foo/bar.js',
+ 'foo/bar.html',
+ ]);
+
+ assert.isTrue(importHtmlPluginStub.calledOnce);
+ assert.isTrue(
+ importHtmlPluginStub.calledWithExactly(`https://cdn.com/foo/bar.html`)
+ );
+ assert.isTrue(loadJsPluginStub.calledOnce);
+ assert.isTrue(
+ loadJsPluginStub.calledWithExactly(`https://cdn.com/foo/bar.js`));
+ });
+
+ test('Should honor original path if exists', () => {
+ Gerrit._loadPlugins([
+ 'http://e.com/foo/bar.html',
+ 'http://e.com/foo/bar.js',
+ ]);
+
+ assert.isTrue(importHtmlPluginStub.calledOnce);
+ assert.isTrue(
+ importHtmlPluginStub.calledWithExactly(`http://e.com/foo/bar.html`)
+ );
+ assert.isTrue(loadJsPluginStub.calledOnce);
+ assert.isTrue(
+ loadJsPluginStub.calledWithExactly(`http://e.com/foo/bar.js`));
+ });
+
+ test('Should try replace current host with assetsPath', () => {
+ const host = window.location.origin;
+ Gerrit._loadPlugins([
+ `${host}/foo/bar.html`,
+ `${host}/foo/bar.js`,
+ ]);
+
+ assert.isTrue(importHtmlPluginStub.calledOnce);
+ assert.isTrue(
+ importHtmlPluginStub.calledWithExactly(`https://cdn.com/foo/bar.html`)
+ );
+ assert.isTrue(loadJsPluginStub.calledOnce);
+ assert.isTrue(
+ loadJsPluginStub.calledWithExactly(`https://cdn.com/foo/bar.js`));
+ });
+ });
+
test('adds js plugins will call the body', () => {
Gerrit._loadPlugins([
'http://e.com/foo/bar.js',
@@ -489,12 +555,10 @@
test('installing preloaded plugin', () => {
let plugin;
- window.ASSETS_PATH = 'http://blips.com/chitz';
Gerrit.install(p => { plugin = p; }, '0.1', 'preloaded:foo');
assert.strictEqual(plugin.getPluginName(), 'foo');
assert.strictEqual(plugin.url('/some/thing.html'),
- 'http://blips.com/chitz/plugins/foo/some/thing.html');
- delete window.ASSETS_PATH;
+ `${window.location.origin}/plugins/foo/some/thing.html`);
});
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
index 6c306d9..6dc0309 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
@@ -17,8 +17,6 @@
(function(window) {
'use strict';
- const PRELOADED_PROTOCOL = 'preloaded:';
-
const PANEL_ENDPOINTS_MAPPING = {
CHANGE_SCREEN_BELOW_COMMIT_INFO_BLOCK: 'change-view-integration',
CHANGE_SCREEN_BELOW_CHANGE_INFO_BLOCK: 'change-metadata-item',
@@ -26,6 +24,7 @@
// Import utils methods
const {
+ PRELOADED_PROTOCOL,
getPluginNameFromUrl,
send,
} = window._apiUtils;
@@ -66,13 +65,6 @@
this._url = new URL(opt_url);
this._name = getPluginNameFromUrl(this._url);
- if (this._url.protocol === PRELOADED_PROTOCOL) {
- // Original plugin URL is used in plugin assets URLs calculation.
- const assetsBaseUrl = window.ASSETS_PATH ||
- (window.location.origin + Gerrit.BaseUrlBehavior.getBaseUrl());
- this._url = new URL(assetsBaseUrl + '/plugins/' + this._name +
- '/static/' + this._name + '.js');
- }
}
Plugin._sharedAPIElement = document.createElement('gr-js-api-interface');
@@ -139,9 +131,15 @@
Plugin.prototype.url = function(opt_path) {
const relPath = '/plugins/' + this._name + (opt_path || '/');
+ const sameOriginPath = window.location.origin +
+ `${Gerrit.BaseUrlBehavior.getBaseUrl()}${relPath}`;
if (window.location.origin === this._url.origin) {
// Plugin loaded from the same origin as gr-app, getBaseUrl in effect.
- return this._url.origin + Gerrit.BaseUrlBehavior.getBaseUrl() + relPath;
+ return sameOriginPath;
+ } else if (this._url.protocol === PRELOADED_PROTOCOL) {
+ // Plugin is preloaded, load plugin with ASSETS_PATH or location.origin
+ return window.ASSETS_PATH ? `${window.ASSETS_PATH}${relPath}`
+ : sameOriginPath;
} else {
// Plugin loaded from assets bundle, expect assets placed along with it.
return this._url.href.split('/plugins/' + this._name)[0] + relPath;
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html
index 8f87b38..f99d5d9 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html
@@ -126,11 +126,13 @@
});
test('createNew link appears correctly', () => {
- assert.isFalse(element.$$('#createNewContainer').classList
+ assert.isFalse(element.shadowRoot
+ .querySelector('#createNewContainer').classList
.contains('show'));
element.createNew = true;
flushAsynchronousOperations();
- assert.isTrue(element.$$('#createNewContainer').classList
+ assert.isTrue(element.shadowRoot
+ .querySelector('#createNewContainer').classList
.contains('show'));
});
@@ -139,7 +141,7 @@
element.addEventListener('create-clicked', clickHandler);
element.createNew = true;
flushAsynchronousOperations();
- MockInteractions.tap(element.$$('#createNew'));
+ MockInteractions.tap(element.shadowRoot.querySelector('#createNew'));
assert.isTrue(clickHandler.called);
});
diff --git a/polygerrit-ui/app/styles/themes/app-theme.html b/polygerrit-ui/app/styles/themes/app-theme.html
index 3a620d2..87230e6 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.html
+++ b/polygerrit-ui/app/styles/themes/app-theme.html
@@ -42,28 +42,34 @@
--vote-text-color-disliked: #d32f2f;
/* background colors */
+ /* primary background colors */
+ --background-color-primary: #ffffff;
+ --background-color-secondary: #f8f9fa;
+ --background-color-tertiary: #f1f3f4;
+ /* directly derived from primary background colors */
+ --chip-background-color: var(--background-color-tertiary);
+ --default-button-background-color: var(--background-color-primary);
+ --dialog-background-color: var(--background-color-primary);
+ --dropdown-background-color: var(--background-color-primary);
+ --expanded-background-color: var(--background-color-tertiary);
+ --secondary-button-background-color: var(--background-color-primary);
+ --select-background-color: var(--background-color-secondary);
+ --shell-command-background-color: var(--background-color-secondary);
+ --shell-command-decoration-background-color: var(--background-color-tertiary);
+ --table-header-background-color: var(--background-color-secondary);
+ --table-subheader-background-color: var(--background-color-tertiary);
+ --view-background-color: var(--background-color-primary);
+ /* unique background colors */
--assignee-highlight-color: #fcfad6;
- --chip-background-color: #eee;
--comment-background-color: #fcfad6;
--robot-comment-background-color: #e8f0fe;
- --default-button-background-color: white;
- --dialog-background-color: white;
- --dropdown-background-color: white;
--edit-mode-background-color: #ebf5fb;
--emphasis-color: #fff9c4;
- --expanded-background-color: #eee;
--hover-background-color: rgba(161, 194, 250, 0.2);
--primary-button-background-color: #2a66d9;
- --secondary-button-background-color: white;
- --select-background-color: #f8f8f8;
--selection-background-color: rgba(161, 194, 250, 0.1);
- --shell-command-background-color: #f5f5f5;
- --shell-command-decoration-background-color: #ebebeb;
- --table-header-background-color: #fafafa;
- --table-subheader-background-color: #eaeaea;
--tooltip-background-color: #333;
--unresolved-comment-background-color: #fcfaa6;
- --view-background-color: white;
--vote-color-approved: #9fcc6b;
--vote-color-disliked: #f7c4cb;
--vote-color-neutral: #ebf5fb;
@@ -71,7 +77,7 @@
--vote-color-rejected: #f7a1ad;
/* misc colors */
- --border-color: #ddd;
+ --border-color: #dadce0;
/* fonts */
--font-family: 'Roboto', -apple-system, BlinkMacSystemFont, 'Segoe UI', Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol';
@@ -103,9 +109,9 @@
--spacing-xxl: 24px;
/* header and footer */
- --footer-background-color: #eee;
+ --footer-background-color: var(--background-color-tertiary);
--footer-border-top: 1px solid var(--border-color);
- --header-background-color: #eee;
+ --header-background-color: var(--background-color-tertiary);
--header-border-bottom: 1px solid var(--border-color);
--header-border-image: '';
--header-box-shadow: none;
@@ -164,9 +170,13 @@
--syntax-title-color: #0000c0;
--syntax-type-color: #2a66d9;
--syntax-variable-color: var(--primary-text-color);
+
/* misc */
--border-radius: 4px;
--reply-overlay-z-index: 1000;
+
+ /* paper and iron component overrides */
+ --iron-overlay-backdrop-background-color: black;
--iron-overlay-backdrop-opacity: 0.32;
--iron-overlay-backdrop: {
transition: none;
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.html b/polygerrit-ui/app/styles/themes/dark-theme.html
index 4a91774f..78bc48b 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.html
+++ b/polygerrit-ui/app/styles/themes/dark-theme.html
@@ -30,7 +30,7 @@
--primary-text-color: #e8eaed;
--link-color: #8ab4f8;
--comment-text-color: var(--primary-text-color);
- --deemphasized-text-color: #9e9e9e;
+ --deemphasized-text-color: #9aa0a6;
--default-button-text-color: #8ab4f8;
--error-text-color: red;
--primary-button-text-color: var(--primary-text-color);
@@ -42,28 +42,24 @@
--vote-text-color-disliked: #d32f2f;
/* background colors */
+ /* primary background colors */
+ --background-color-primary: #202124;
+ --background-color-secondary: #2f3034;
+ --background-color-tertiary: #3b3d3f;
+ /* directly derived from primary background colors */
+ /* empty, because inheriting from app-theme is just fine
+ /* unique background colors */
--assignee-highlight-color: #3a361c;
- --chip-background-color: #131416;
--comment-background-color: #0b162b;
--robot-comment-background-color: #e8f0fe;
- --default-button-background-color: #3c4043;
- --dialog-background-color: #131416;
- --dropdown-background-color: #131416;
--edit-mode-background-color: #5c0a36;
--emphasis-color: #383f4a;
- --expanded-background-color: #26282b;
--hover-background-color: rgba(161, 194, 250, 0.2);
--primary-button-background-color: var(--link-color);
--secondary-button-background-color: var(--primary-text-color);
- --select-background-color: #3c4043;
--selection-background-color: rgba(161, 194, 250, 0.1);
- --shell-command-background-color: #5f5f5f;
- --shell-command-decoration-background-color: #999;
- --table-header-background-color: #131416;
- --table-subheader-background-color: rgba(158, 158, 158, 0.24);
--tooltip-background-color: #111;
--unresolved-comment-background-color: #385a9a;
- --view-background-color: #131416;
--vote-color-approved: #7fb66b;
--vote-color-disliked: #bf6874;
--vote-color-neutral: #597280;
@@ -79,9 +75,9 @@
/* spacing */
/* header and footer */
- --footer-background-color: #131416;
+ --footer-background-color: var(--background-color-tertiary);
--footer-border-top: 1px solid var(--border-color);
- --header-background-color: #3c4043;
+ --header-background-color: var(--background-color-tertiary);
--header-border-bottom: 1px solid var(--border-color);
--header-padding: 0 var(--spacing-l);
--header-text-color: var(--primary-text-color);
@@ -92,7 +88,7 @@
--dark-rebased-remove-highlight-color: rgba(255, 139, 6, 0.15);
--dark-remove-highlight-color: #62110f;
--diff-blank-background-color: #212121;
- --diff-context-control-background-color: #131416;
+ --diff-context-control-background-color: #333311;
--diff-context-control-border-color: var(--border-color);
--diff-context-control-color: var(--deemphasized-text-color);
--diff-highlight-range-color: rgba(0, 100, 200, 0.5);
@@ -137,6 +133,9 @@
/* misc */
+ /* paper and iron component overrides */
+ --iron-overlay-backdrop-background-color: white;
+
/* rules applied to <html> */
background-color: var(--view-background-color);
}
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 6a29da6..e16c0a0 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -23,8 +23,8 @@
maven_jar(
name = "dropwizard-core",
- artifact = "io.dropwizard.metrics:metrics-core:4.1.1",
- sha1 = "ebfafc716d9c3b6151dc7c2c09ce925a163a4f21",
+ artifact = "io.dropwizard.metrics:metrics-core:4.1.2",
+ sha1 = "bba231bbf3024c19e75622ec168821cbbd4261a4",
)
SSHD_VERS = "2.3.0"
