Merge "Reduce usage of ProjectControl"
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 3d132d3..535161a 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -220,6 +220,7 @@
thus `^refs/heads/.*/name` will fail because `refs/heads//name`
is not a valid reference, but `^refs/heads/.+/name` will work.
+[[sharded-user-id]]
References can have the user name or the sharded account ID of the
current user automatically included, creating dynamic access controls
that change to match the currently logged in user. For example to
diff --git a/Documentation/config-accounts.txt b/Documentation/config-accounts.txt
new file mode 100644
index 0000000..ec271f3
--- /dev/null
+++ b/Documentation/config-accounts.txt
@@ -0,0 +1,411 @@
+= Gerrit Code Review - Accounts
+
+== Overview
+
+Starting from 2.15 Gerrit accounts are fully stored in
+link:dev-note-db.html[NoteDb].
+
+The account data consists of a sequence number (account ID), account
+properties (full name, preferred email, registration date, status,
+inactive flag), preferences (general, diff and edit preferences),
+project watches, SSH keys, external IDs, starred changes and reviewed
+flags.
+
+Most account data is stored in a special link:#all-users[All-Users]
+repository, which has one branch per user. Within the user branch there
+are Git config files for the link:#account-properties[
+account properties], the link:#preferences[account preferences] and the
+link:#project-watches[project watches]. In addition there is an
+`authorized_keys` file for the link:#ssh-keys[SSH keys] that follows
+the standard OpenSSH file format.
+
+The account data in the user branch is versioned and the Git history of
+this branch serves as an audit log.
+
+The link:#external-ids[external IDs] are stored as Git Notes inside the
+`All-Users` repository in the `refs/meta/external-ids` notes branch.
+Storing all external IDs in a notes branch ensures that each external
+ID is only used once.
+
+The link:#starred-changes[starred changes] are represented as
+independent refs in the `All-Users` repository. They are not stored in
+the user branch, since this data doesn't need versioning.
+
+The link:#reviewed-flags[reviewed flags] are not stored in Git, but are
+persisted in a database table. This is because there is a high volume
+of reviewed flags and storing them in Git would be inefficient.
+
+Since accessing the account data in Git is not fast enough for account
+queries, e.g. when suggesting reviewers, Gerrit has a
+link:#account-index[secondary index for accounts].
+
+[[all-users]]
+== `All-Users` repository
+
+The `All-Users` repository is a special repository that only contains
+user-specific information. It contains one branch per user. The user
+branch is formatted as `refs/users/CD/ABCD`, where `CD/ABCD` is the
+link:access-control.html#sharded-user-id[sharded account ID], e.g. the
+user branch for account `1000856` is `refs/users/56/1000856`. The
+account IDs in the user refs are sharded so that there is a good
+distribution of the Git data in the storage system.
+
+A user branch must exist for each account, as it represents the
+account. The files in the user branch are all optional. This means
+having a user branch with a tree that is completely empty is also a
+valid account definition.
+
+Updates to the user branch are done through the
+link:rest-api-accounts.html[Gerrit REST API], but users can also
+manually fetch their user branch and push changes back to Gerrit. On
+push the user data is evaluated and invalid user data is rejected.
+
+To hide the implementation detail of the sharded account ID in the ref
+name Gerrit offers a magic `refs/users/self` ref that is automatically
+resolved to the user branch of the calling user. The user can then use
+this ref to fetch from and push to the own user branch. E.g. if user
+`1000856` pushes to `refs/users/self`, the branch
+`refs/users/56/1000856` is updated. In Gerrit `self` is an established
+term to refer to the calling user (e.g. in change queries). This is why
+the magic ref for the own user branch is called `refs/users/self`.
+
+A user branch should only be readable and writeable by the user to whom
+the account belongs. To assign permissions on the user branches the
+normal branch permission system is used. In the permission system the
+user branches are specified as `refs/users/${shardeduserid}`. The
+`${shardeduserid}` variable is resolved to the sharded account ID. This
+variable is used to assign default access rights on all user branches
+that apply only to the owning user. The following permissions are set
+by default when a Gerrit site is newly installed or upgraded to a
+version which supports user branches:
+
+.All-Users project.config
+----
+[access "refs/users/${shardeduserid}"]
+ exclusiveGroupPermissions = read push submit
+ read = group Registered Users
+ push = group Registered Users
+ label-Code-Review = -2..+2 group Registered Users
+ submit = group Registered Users
+----
+
+The user branch contains several files with account data which are
+described link:#account-data-in-user-branch[below].
+
+In addition to the user branches the `All-Users` repository also
+contains a branch for the link:#external-ids[external IDs] and special
+refs for the link:#starred-changes[starred changes].
+
+Also the next available value of the link:#account-sequence[account
+sequence] is stored in the `All-Users` repository.
+
+[[account-index]]
+== Account Index
+
+There are several situations in which Gerrit needs to query accounts,
+e.g.:
+
+* For sending email notifications to project watchers.
+* For reviewer suggestions.
+
+Accessing the account data in Git is not fast enough for account
+queries, since it requires accessing all user branches and parsing
+all files in each of them. To overcome this Gerrit has a secondary
+index for accounts. The account index is either based on
+link:config-gerrit.html#index.type[Lucene or Elasticsearch].
+
+Via the link:rest-api-accounts.html#query-account[Query Account] REST
+endpoint link:user-search-accounts.html[generic account queries] are
+supported.
+
+Accounts are automatically reindexed on any update. The
+link:rest-api-accounts.html#index-account[Index Account] REST endpoint
+allows to reindex an account manually. In addition the
+link:pgm-reindex.html[reindex] program can be used to reindex all
+accounts offline.
+
+[[account-data-in-user-branch]]
+== Account Data in User Branch
+
+A user branch contains several Git config files with the account data:
+
+* `account.config`:
++
+Stores the link:#account-properties[account properties].
+
+* `preferences.config`:
++
+Stores the link:#preferences[user preferences] of the account.
+
+* `watch.config`:
++
+Stores the link:#project-watches[project watches] of the account.
+
+In addition it contains an
+link:https://en.wikibooks.org/wiki/OpenSSH/Client_Configuration_Files#.7E.2F.ssh.2Fauthorized_keys[
+authorized_keys] file with the link:#ssh-keys[SSH keys] of the account.
+
+[[account-properties]]
+=== Account Properties
+
+The account properties are stored in the user branch in the
+`account.config` file:
+
+----
+[account]
+ fullName = John Doe
+ preferredEmail = john.doe@example.com
+ status = OOO
+ active = false
+----
+
+For active accounts the `active` parameter can be omitted.
+
+The registration date is not contained in the `account.config` file but
+is derived from the timestamp of the first commit on the user branch.
+
+When users update their account properties by pushing to the user
+branch, it is verified that the preferred email exists in the external
+IDs.
+
+Users are not allowed to flip the active value themselves; only
+administrators and users with the
+link:access-control.html#capability_modifyAccount[Modify Account]
+global capability are allowed to change it.
+
+Since all data in the `account.config` file is optional the
+`account.config` file may be absent from some user branches.
+
+[[preferences]]
+=== Preferences
+
+The account properties are stored in the user branch in the
+`preferences.config` file. There are separate sections for
+link:intro-user.html#preferences[general],
+link:user-review-ui.html#diff-preferences[diff] and edit preferences:
+
+----
+[general]
+ showSiteHeader = false
+[diff]
+ hideTopMenu = true
+[edit]
+ lineLength = 80
+----
+
+The parameter names match the names that are used in the preferences REST API:
+
+* link:rest-api-accounts.html#preferences-info[General Preferences]
+* link:rest-api-accounts.html#diff-preferences-info[Diff Preferences]
+* link:rest-api-accounts.html#edit-preferences-info[Edit Preferences]
+
+If the value for a preference is the same as the default value for this
+preference, it can be omitted in the `preference.config` file.
+
+Defaults for general and diff preferences that apply for all accounts
+can be configured in the `refs/users/default` branch in the `All-Users`
+repository.
+
+[[project-watches]]
+=== Project Watches
+
+Users can configure watches on projects to receive email notifications
+for changes of that project.
+
+A watch configuration consists of the project name and an optional
+filter query. If a filter query is specified, email notifications will
+be sent only for changes of that project that match this query.
+
+In addition, each watch configuration can contain a list of
+notification types that determine for which events email notifications
+should be sent. E.g. a user can configure that email notifications
+should only be sent if a new patch set is uploaded and when the change
+gets submitted, but not on other events.
+
+Project watches are stored in a `watch.config` file in the user branch:
+
+----
+[project "foo"]
+ notify = * [ALL_COMMENTS]
+ notify = branch:master [ALL_COMMENTS, NEW_PATCHSETS]
+ notify = branch:master owner:self [SUBMITTED_CHANGES]
+----
+
+The `watch.config` file has one project section for all project watches
+of a project. The project name is used as subsection name and the
+filters with the notification types, that decide for which events email
+notifications should be sent, are represented as `notify` values in the
+subsection. A `notify` value is formatted as
+"<filter> [<comma-separated-list-of-notification-types>]". The
+supported notification types are described in the
+link:user-notify.html#notify.name.type[Email Notifications documentation].
+
+For a change event, a notification will be sent if any `notify` value
+of the corresponding project has both a filter that matches the change
+and a notification type that matches the event.
+
+In order to send email notifications on change events, Gerrit needs to
+find all accounts that watch the corresponding project. To make this
+lookup fast the secondary account index is used. The account index
+contains a repeated field that stores the projects that are being
+watched by an account. After the accounts that watch the project have
+been retrieved from the index, the complete watch configuration is
+available from the account cache and Gerrit can check if any watch
+matches the change and the event.
+
+[[ssh-keys]]
+=== SSH Keys
+
+SSH keys are stored in the user branch in an `authorized_keys` file,
+which is the
+link:https://en.wikibooks.org/wiki/OpenSSH/Client_Configuration_Files#.7E.2F.ssh.2Fauthorized_keys[
+standard OpenSSH file format] for storing SSH keys:
+
+----
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCgug5VyMXQGnem2H1KVC4/HcRcD4zzBqSuJBRWVonSSoz3RoAZ7bWXCVVGwchtXwUURD689wFYdiPecOrWOUgeeyRq754YWRhU+W28vf8IZixgjCmiBhaL2gt3wff6pP+NXJpTSA4aeWE5DfNK5tZlxlSxqkKOS8JRSUeNQov5Tw== john.doe@example.com
+# DELETED
+# INVALID ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDm5yP7FmEoqzQRDyskX+9+N0q9GrvZeh5RG52EUpE4ms/Ujm3ewV1LoGzc/lYKJAIbdcZQNJ9+06EfWZaIRA3oOwAPe1eCnX+aLr8E6Tw2gDMQOGc5e9HfyXpC2pDvzauoZNYqLALOG3y/1xjo7IH8GYRS2B7zO/Mf9DdCcCKSfw== john.doe@example.com
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCaS7RHEcZ/zjl9hkWkqnm29RNr2OQ/TZ5jk2qBVMH3BgzPsTsEs+7ag9tfD8OCj+vOcwm626mQBZoR2e3niHa/9gnHBHFtOrGfzKbpRjTWtiOZbB9HF+rqMVD+Dawo/oicX/dDg7VAgOFSPothe6RMhbgWf84UcK5aQd5eP5y+tQ== john.doe@example.com
+----
+
+When the SSH API is used, Gerrit needs an efficient way to lookup SSH
+keys by username. Since the username can be easily resolved to an
+account ID (via the account cache), accessing the SSH keys in the user
+branch is fast.
+
+To identify SSH keys in the REST API Gerrit uses
+link:rest-api-accounts.html#ssh-key-id[sequence numbers per account].
+This is why the order of the keys in the `authorized_keys` file is
+used to determines the sequence numbers of the keys (the sequence
+numbers start at 1).
+
+To keep the sequence numbers intact when a key is deleted, a
+'# DELETED' line is inserted at the position where the key was deleted.
+
+Invalid keys are marked with the prefix '# INVALID'.
+
+[[external-ids]]
+== External IDs
+
+External IDs are used to link external identities, such as an LDAP
+account or an OAUTH identity, to an account in Gerrit.
+
+External IDs are stored as Git Notes in the `All-Users` repository. The
+name of the notes branch is `refs/meta/external-ids`.
+
+As note key the SHA1 of the external ID key is used. This ensures that
+an external ID is used only once (e.g. an external ID can never be
+assigned to multiple accounts at a point in time).
+
+The note content is a Git config file:
+
+----
+[externalId "username:jdoe"]
+ accountId = 1003407
+ email = jdoe@example.com
+ password = bcrypt:4:LCbmSBDivK/hhGVQMfkDpA==:XcWn0pKYSVU/UJgOvhidkEtmqCp6oKB7
+----
+
+The config file has one `externalId` section. The external ID key which
+consists of scheme and ID in the format '<scheme>:<id>' is used as
+subsection name.
+
+The `accountId` field is mandatory, the `email` and `password` fields
+are optional.
+
+The external IDs are maintained by Gerrit, this means users are not
+allowed to manually edit their external IDs. Only users with the
+link:access-control.html#capability_accessDatabase[Access Database] can
+push updates to the `refs/meta/external-ids` branch. However Gerrit
+rejects pushes if:
+
+* any external ID config file cannot be parsed
+* if a note key does not match the SHA of the external ID key in the
+ note content
+* external IDs for non-existing accounts are contained
+* invalid emails are contained
+* any email is not unique (the same email is assigned to multiple
+ accounts)
+* hashed passwords of external IDs with scheme `username` cannot be
+ decoded
+
+[[starred-changes]]
+== Starred Changes
+
+link:dev-stars.html[Starred changes] allow users to mark changes as
+favorites and receive email notifications for them.
+
+Each starred change is a tuple of an account ID, a change ID and a
+label.
+
+To keep track of a change that is starred by an account, Gerrit creates
+a `refs/starred-changes/YY/XXXX/ZZZZZZZ` ref in the `All-Users`
+repository, where `YY/XXXX` is the sharded numeric change ID and
+`ZZZZZZZ` is the account ID.
+
+A starred-changes ref points to a blob that contains the list of labels
+that the account set on the change. The label list is stored as UTF-8
+text with one label per line.
+
+Since JGit has explicit optimizations for looking up refs by prefix
+when the prefix ends with '/', this ref format is optimized to find
+starred changes by change ID. Finding starred changes by change ID is
+e.g. needed when a change is updated so that all users that have
+the link:dev-stars.html#default-star[default star] on the change can be
+notified by email.
+
+Gerrit also needs an efficient way to find all changes that were
+starred by an account, e.g. to provide results for the
+link:user-search.html#is-starred[is:starred] query operator. With the
+ref format as described above the lookup of starred changes by account
+ID is expensive, as this requires a scan of the full
+`refs/starred-changes/*` namespace. To overcome this the users that
+have starred a change are stored in the change index together with the
+star labels.
+
+[[reviewed-flags]]
+== Reviewed Flags
+
+When reviewing a patch set in the Gerrit UI, the reviewer can mark
+files in the patch set as reviewed. These markers are called ‘Reviewed
+Flags’ and are private to the user. A reviewed flag is a tuple of patch
+set ID, file and account ID.
+
+Each user can have many thousands of reviewed flags and over time the
+number can grow without bounds.
+
+The high amount of reviewed flags makes a storage in Git unsuitable
+because each update requires opening the repository and committing a
+change, which is a high overhead for flipping a bit. Therefore the
+reviewed flags are stored in a database table. By default they are
+stored in a local H2 database, but there is an extension point that
+allows to plug in alternate implementations for storing the reviewed
+flags. To replace the storage for reviewed flags a plugin needs to
+implement the link:dev-plugins.html#account-patch-review-store[
+AccountPatchReviewStore] interface. E.g. to support a multi-master
+setup where reviewed flags should be replicated between the master
+nodes one could implement a store for the reviewed flags that is
+based on MySQL with replication.
+
+[[account-sequence]]
+== Account Sequence
+
+The next available account sequence number is stored as UTF-8 text in a
+blob pointed to by the `refs/sequences/accounts` ref in the `All-Users`
+repository.
+
+Multiple processes share the same sequence by incrementing the counter
+using normal git ref updates. To amortize the cost of these ref
+updates, processes increment the counter by a larger number and hand
+out numbers from that range in memory until they run out. The size of
+the account ID batch that each process retrieves at once is controlled
+by the link:config-gerrit.html#notedb.accounts.sequenceBatchSize[
+notedb.accounts.sequenceBatchSize] parameter in the `gerrit.config`
+file.
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 932f4da..c889b5d 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3326,8 +3326,8 @@
[[notedb.accounts.sequenceBatchSize]]notedb.accounts.sequenceBatchSize::
+
-The current account sequence number is stored as UTF-8 text in a blob
-pointed to by the `refs/sequences/accounts` ref in the `All-Users`
+The next available account sequence number is stored as UTF-8 text in a
+blob pointed to by the `refs/sequences/accounts` ref in the `All-Users`
repository. Multiple processes share the same sequence by incrementing
the counter using normal git ref updates. To amortize the cost of these
ref updates, processes increment the counter by a larger number and
diff --git a/Documentation/config-sso.txt b/Documentation/config-sso.txt
index 04949bf..bdd52f7 100644
--- a/Documentation/config-sso.txt
+++ b/Documentation/config-sso.txt
@@ -50,8 +50,8 @@
=== Database Schema
-User identities obtained from OpenID providers are stored into the
-`account_external_ids` table.
+User identities obtained from OpenID providers are stored as
+link:config-accounts.html#external-ids[external IDs].
=== Multiple Identities
@@ -135,11 +135,10 @@
=== Database Schema
-User identities are stored in the `account_external_ids` table.
-The user string obtained from the authorization header has the prefix
-"gerrit:" and is stored in the `external_id` field. For example,
-if a username was "foo" then the external_id field would be populated
-with "gerrit:foo".
+User identities are stored as
+link:config-accounts.html#external-ids[external IDs] with "gerrit" as
+scheme. The user string obtained from the authorization header is
+stored as ID of the external ID.
== Computer Associates Siteminder
@@ -193,11 +192,10 @@
=== Database Schema
-User identities are stored in the `account_external_ids` table.
-The user string obtained from Siteminder (e.g. the value in the
-"SM_USER" HTTP header) has the prefix "gerrit:" and is stored in the
-`external_id` field. For example, if a Siteminder username was "foo"
-then the external_id field would be populated with "gerrit:foo".
+User identities are stored as
+link:config-accounts.html#external-ids[external IDs] with "gerrit" as
+scheme. The user string obtained from Siteminder (e.g. the value in the
+"SM_USER" HTTP header) is stored as ID in the external ID.
GERRIT
------
diff --git a/Documentation/index.txt b/Documentation/index.txt
index da211b6..24c538f 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -67,6 +67,7 @@
. link:config-auto-site-initialization.html[Automatic Site Initialization on Startup]
. link:pgm-index.html[Server Side Administrative Tools]
. link:note-db.html[NoteDb]
+. link:config-accounts.html[Accounts]
== Developer
. Getting Started
diff --git a/Documentation/note-db.txt b/Documentation/note-db.txt
index b926bba..728d630 100644
--- a/Documentation/note-db.txt
+++ b/Documentation/note-db.txt
@@ -26,9 +26,9 @@
- Storing change metadata is fully implemented in the 2.15 release. Admins may
use an link:#offline-migration[offline] or link:#online-migration[online] tool
to migrate change data from ReviewDb.
-- Storing account data is fully implemented in the 2.15 release. Account data is
- migrated automatically during the upgrade process by running `gerrit.war
- init`.
+- Storing link:config-accounts.html[account data] is fully implemented in the
+ 2.15 release. Account data is migrated automatically during the upgrade
+ process by running `gerrit.war init`.
- Account and change metadata on the servers behind `googlesource.com` is fully
migrated to NoteDb. In other words, if you use
link:https://gerrit-review.googlesource.com/[gerrit-review], you're already
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 09fea83..fad4b9c 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -334,6 +334,12 @@
server.
--
+[[tracking-ids]]
+--
+* `TRACKING_IDS`: include references to external tracking systems
+ as link:#tracking-id-info[TrackingIdInfo].
+--
+
.Request
----
GET /changes/?q=97&o=CURRENT_REVISION&o=CURRENT_COMMIT&o=CURRENT_FILES&o=DOWNLOAD_COMMANDS HTTP/1.0
@@ -5739,6 +5745,10 @@
Only set if link:#current-revision[the current revision] is requested
(in which case it will only contain a key for the current revision) or
if link:#all-revisions[all revisions] are requested.
+|`tracking_ids` |optional|
+A list of link:#tracking-id-info[TrackingIdInfo] entities describing
+references to external tracking systems. Only set if
+link:#tracking-ids[tracking ids] are requested.
|`_more_changes` |optional, not set if `false`|
Whether the query would deliver more results if not limited. +
Only set on the last change that is returned.
@@ -7063,6 +7073,17 @@
The topic will be deleted if not set.
|===========================
+[[tracking-id-info]]
+=== TrackingIdInfo
+The `TrackingIdInfo` entity describes a reference to an external tracking system.
+
+[options="header",cols="1,6"]
+|======================
+|Field Name|Description
+|`system` |The name of the external tracking system.
+|`id` |The tracking id.
+|======================
+
[[voting-range-info]]
=== VotingRangeInfo
The `VotingRangeInfo` entity describes the continuous voting range from min
diff --git a/WORKSPACE b/WORKSPACE
index 57066a1..5057012 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -581,10 +581,10 @@
maven_jar(
name = "blame_cache",
- artifact = "com/google/gitiles:blame-cache:0.2-3",
+ artifact = "com/google/gitiles:blame-cache:0.2-4",
attach_source = False,
repository = GERRIT,
- sha1 = "fc31fb07fab42b2b4f645e80449fae403e83bcb6",
+ sha1 = "e68fa6fcb6402e9a781cd33b2e0ecec85d1b786d",
)
# Keep this version of Soy synchronized with the version used in Gitiles.
diff --git a/contrib/abandon_stale.py b/contrib/abandon_stale.py
index 9ac6b4b..fb9eb12 100755
--- a/contrib/abandon_stale.py
+++ b/contrib/abandon_stale.py
@@ -152,7 +152,7 @@
query_terms += ["owner:%s" % options.owner]
query = "%20".join(query_terms)
while True:
- q = query + "&n=%d&S=%d" % (step, offset)
+ q = query + "&o=DETAILED_ACCOUNTS&n=%d&S=%d" % (step, offset)
logging.debug("Query: %s", q)
url = "/changes/?q=" + q
result = gerrit.get(url)
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
index 806469e..846b85a 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
@@ -116,7 +116,6 @@
accountsUpdate
.create()
.insert(
- db,
id,
a -> {
a.setFullName(fullName);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 0d68f4a..b7d368a 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -45,7 +45,6 @@
import com.google.common.util.concurrent.AtomicLongMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AccountCreator;
-import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestAccount;
@@ -78,6 +77,7 @@
import com.google.gerrit.gpg.PublicKeyStore;
import com.google.gerrit.gpg.testutil.TestKey;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.account.AccountConfig;
@@ -89,7 +89,6 @@
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilderImpl;
import com.google.gerrit.server.project.RefPattern;
@@ -257,40 +256,19 @@
Account.Id nonExistingAccountId = new Account.Id(999999);
AtomicBoolean consumerCalled = new AtomicBoolean();
Account account =
- accountsUpdate.create().update(db, nonExistingAccountId, a -> consumerCalled.set(true));
+ accountsUpdate.create().update(nonExistingAccountId, a -> consumerCalled.set(true));
assertThat(account).isNull();
assertThat(consumerCalled.get()).isFalse();
}
@Test
- public void updateAccountThatIsMissingInNoteDb() throws Exception {
- String name = "bar";
- TestAccount bar = accountCreator.create(name);
- assertUserBranch(bar.getId(), name, null);
-
- // delete user branch
- try (Repository repo = repoManager.openRepository(allUsers)) {
- AccountsUpdate.deleteUserBranch(
- repo, allUsers, GitReferenceUpdated.DISABLED, null, serverIdent.get(), bar.getId());
- assertThat(repo.exactRef(RefNames.refsUsers(bar.getId()))).isNull();
- }
-
- String status = "OOO";
- Account account = accountsUpdate.create().update(db, bar.getId(), a -> a.setStatus(status));
- assertThat(account).isNotNull();
- assertThat(account.getFullName()).isEqualTo(name);
- assertThat(account.getStatus()).isEqualTo(status);
- assertUserBranch(bar.getId(), name, status);
- }
-
- @Test
public void updateAccountWithoutAccountConfigNoteDb() throws Exception {
TestAccount anonymousCoward = accountCreator.create();
assertUserBranchWithoutAccountConfig(anonymousCoward.getId());
String status = "OOO";
Account account =
- accountsUpdate.create().update(db, anonymousCoward.getId(), a -> a.setStatus(status));
+ accountsUpdate.create().update(anonymousCoward.getId(), a -> a.setStatus(status));
assertThat(account).isNotNull();
assertThat(account.getFullName()).isNull();
assertThat(account.getStatus()).isEqualTo(status);
@@ -713,7 +691,7 @@
String prefix = "foo.preferred";
String prefEmail = prefix + "@example.com";
TestAccount foo = accountCreator.create(name("foo"));
- accountsUpdate.create().update(db, foo.id, a -> a.setPreferredEmail(prefEmail));
+ accountsUpdate.create().update(foo.id, a -> a.setPreferredEmail(prefEmail));
// verify that the account is still found when using the preferred email to lookup the account
ImmutableSet<Account.Id> accountsByPrefEmail = emails.getAccountFor(prefEmail);
@@ -835,14 +813,14 @@
}
@Test
- public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmit() throws Exception {
- String userRefName = RefNames.refsUsers(admin.id);
+ public void pushAccountConfigToUserBranchForReviewAndSubmit() throws Exception {
+ String userRef = RefNames.refsUsers(admin.id);
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, userRefName + ":userRef");
+ fetch(allUsersRepo, userRef + ":userRef");
allUsersRepo.reset("userRef");
Config ac = getAccountConfig(allUsersRepo);
- ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_STATUS, "OOO");
+ ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_STATUS, "out-of-office");
PushOneCommit.Result r =
pushFactory
@@ -853,18 +831,204 @@
"Update account config",
AccountConfig.ACCOUNT_CONFIG,
ac.toText())
- .to(MagicBranch.NEW_CHANGE + userRefName);
+ .to(MagicBranch.NEW_CHANGE + userRef);
r.assertOkStatus();
accountIndexedCounter.assertNoReindex();
- assertThat(r.getChange().change().getDest().get()).isEqualTo(userRefName);
+ assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
+
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).current().submit();
+ accountIndexedCounter.assertReindexOf(admin);
+
+ AccountInfo info = gApi.accounts().self().get();
+ assertThat(info.email).isEqualTo(admin.email);
+ assertThat(info.name).isEqualTo(admin.fullName);
+ assertThat(info.status).isEqualTo("out-of-office");
+ }
+
+ @Test
+ public void pushAccountConfigWithPrefEmailThatDoesNotExistAsExtIdToUserBranchForReviewAndSubmit()
+ throws Exception {
+ TestAccount foo = accountCreator.create(name("foo"));
+ String userRef = RefNames.refsUsers(foo.id);
+ accountIndexedCounter.clear();
+
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ String email = "some.email@example.com";
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_PREFERRED_EMAIL, email);
+
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
+
+ setApiUser(foo);
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).current().submit();
+
+ accountIndexedCounter.assertReindexOf(foo);
+
+ AccountInfo info = gApi.accounts().self().get();
+ assertThat(info.email).isEqualTo(email);
+ assertThat(info.name).isEqualTo(foo.fullName);
+ }
+
+ @Test
+ public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmitIfConfigIsInvalid()
+ throws Exception {
+ String userRef = RefNames.refsUsers(admin.id);
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ "invalid config")
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
+
gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
exception.expect(ResourceConflictException.class);
exception.expectMessage(
- String.format("update of %s not allowed", AccountConfig.ACCOUNT_CONFIG));
+ String.format(
+ "invalid account configuration: commit '%s' has an invalid '%s' file for account '%s':"
+ + " Invalid config file %s in commit %s",
+ r.getCommit().name(),
+ AccountConfig.ACCOUNT_CONFIG,
+ admin.id,
+ AccountConfig.ACCOUNT_CONFIG,
+ r.getCommit().name()));
gApi.changes().id(r.getChangeId()).current().submit();
}
@Test
+ public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmitIfPreferredEmailIsInvalid()
+ throws Exception {
+ String userRef = RefNames.refsUsers(admin.id);
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ String noEmail = "no.email";
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_PREFERRED_EMAIL, noEmail);
+
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
+
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage(
+ String.format(
+ "invalid account configuration: invalid preferred email '%s' for account '%s'",
+ noEmail, admin.id));
+ gApi.changes().id(r.getChangeId()).current().submit();
+ }
+
+ @Test
+ public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmitIfOwnAccountIsDeactivated()
+ throws Exception {
+ String userRef = RefNames.refsUsers(admin.id);
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setBoolean(AccountConfig.ACCOUNT, null, AccountConfig.KEY_ACTIVE, false);
+
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
+
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage("invalid account configuration: cannot deactivate own account");
+ gApi.changes().id(r.getChangeId()).current().submit();
+ }
+
+ @Test
+ public void pushAccountConfigToUserBranchForReviewDeactivateOtherAccount() throws Exception {
+ TestAccount foo = accountCreator.create(name("foo"));
+ assertThat(gApi.accounts().id(foo.id.get()).getActive()).isTrue();
+ String userRef = RefNames.refsUsers(foo.id);
+ accountIndexedCounter.clear();
+
+ AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Administrators"));
+ grant(allUsers, userRef, Permission.PUSH, false, adminGroup.getGroupUUID());
+ grantLabel("Code-Review", -2, 2, allUsers, userRef, false, adminGroup.getGroupUUID(), false);
+ grant(allUsers, userRef, Permission.SUBMIT, false, adminGroup.getGroupUUID());
+
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setBoolean(AccountConfig.ACCOUNT, null, AccountConfig.KEY_ACTIVE, false);
+
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
+
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).current().submit();
+ accountIndexedCounter.assertReindexOf(foo);
+
+ assertThat(gApi.accounts().id(foo.id.get()).getActive()).isFalse();
+ }
+
+ @Test
public void pushWatchConfigToUserBranch() throws Exception {
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
fetch(allUsersRepo, RefNames.refsUsers(admin.id) + ":userRef");
@@ -906,13 +1070,70 @@
}
@Test
- public void pushAccountConfigToUserBranchIsRejected() throws Exception {
+ public void pushAccountConfigToUserBranch() throws Exception {
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
fetch(allUsersRepo, RefNames.refsUsers(admin.id) + ":userRef");
allUsersRepo.reset("userRef");
Config ac = getAccountConfig(allUsersRepo);
- ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_STATUS, "OOO");
+ ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_STATUS, "out-of-office");
+
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(RefNames.REFS_USERS_SELF)
+ .assertOkStatus();
+ accountIndexedCounter.assertReindexOf(admin);
+
+ AccountInfo info = gApi.accounts().self().get();
+ assertThat(info.email).isEqualTo(admin.email);
+ assertThat(info.name).isEqualTo(admin.fullName);
+ assertThat(info.status).isEqualTo("out-of-office");
+ }
+
+ @Test
+ public void pushAccountConfigToUserBranchIsRejectedIfConfigIsInvalid() throws Exception {
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, RefNames.refsUsers(admin.id) + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ "invalid config")
+ .to(RefNames.REFS_USERS_SELF);
+ r.assertErrorStatus("invalid account configuration");
+ r.assertMessage(
+ String.format(
+ "commit '%s' has an invalid '%s' file for account '%s':"
+ + " Invalid config file %s in commit %s",
+ r.getCommit().name(),
+ AccountConfig.ACCOUNT_CONFIG,
+ admin.id,
+ AccountConfig.ACCOUNT_CONFIG,
+ r.getCommit().name()));
+ accountIndexedCounter.assertNoReindex();
+ }
+
+ @Test
+ public void pushAccountConfigToUserBranchIsRejectedIfPreferredEmailIsInvalid() throws Exception {
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, RefNames.refsUsers(admin.id) + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ String noEmail = "no.email";
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_PREFERRED_EMAIL, noEmail);
PushOneCommit.Result r =
pushFactory
@@ -924,7 +1145,138 @@
AccountConfig.ACCOUNT_CONFIG,
ac.toText())
.to(RefNames.REFS_USERS_SELF);
- r.assertErrorStatus("account update not allowed");
+ r.assertErrorStatus("invalid account configuration");
+ r.assertMessage(
+ String.format("invalid preferred email '%s' for account '%s'", noEmail, admin.id));
+ accountIndexedCounter.assertNoReindex();
+ }
+
+ @Test
+ public void pushAccountConfigToUserBranchInvalidPreferredEmailButNotChanged() throws Exception {
+ TestAccount foo = accountCreator.create(name("foo"));
+ String userRef = RefNames.refsUsers(foo.id);
+
+ String noEmail = "no.email";
+ accountsUpdate.create().update(foo.id, a -> a.setPreferredEmail(noEmail));
+ accountIndexedCounter.clear();
+
+ AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Administrators"));
+ grant(allUsers, userRef, Permission.PUSH, false, adminGroup.getGroupUUID());
+
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ String status = "in vacation";
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_STATUS, status);
+
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(userRef)
+ .assertOkStatus();
+ accountIndexedCounter.assertReindexOf(foo);
+
+ AccountInfo info = gApi.accounts().id(foo.id.get()).get();
+ assertThat(info.email).isEqualTo(noEmail);
+ assertThat(info.name).isEqualTo(foo.fullName);
+ assertThat(info.status).isEqualTo(status);
+ }
+
+ @Test
+ public void pushAccountConfigToUserBranchIfPreferredEmailDoesNotExistAsExtId() throws Exception {
+ TestAccount foo = accountCreator.create(name("foo"));
+ String userRef = RefNames.refsUsers(foo.id);
+ accountIndexedCounter.clear();
+
+ AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Administrators"));
+ grant(allUsers, userRef, Permission.PUSH, false, adminGroup.getGroupUUID());
+
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ String email = "some.email@example.com";
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_PREFERRED_EMAIL, email);
+
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(userRef)
+ .assertOkStatus();
+ accountIndexedCounter.assertReindexOf(foo);
+
+ AccountInfo info = gApi.accounts().id(foo.id.get()).get();
+ assertThat(info.email).isEqualTo(email);
+ assertThat(info.name).isEqualTo(foo.fullName);
+ }
+
+ @Test
+ public void pushAccountConfigToUserBranchIsRejectedIfOwnAccountIsDeactivated() throws Exception {
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, RefNames.refsUsers(admin.id) + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setBoolean(AccountConfig.ACCOUNT, null, AccountConfig.KEY_ACTIVE, false);
+
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(RefNames.REFS_USERS_SELF);
+ r.assertErrorStatus("invalid account configuration");
+ r.assertMessage("cannot deactivate own account");
+ accountIndexedCounter.assertNoReindex();
+ }
+
+ @Test
+ public void pushAccountConfigToUserBranchDeactivateOtherAccount() throws Exception {
+ TestAccount foo = accountCreator.create(name("foo"));
+ assertThat(gApi.accounts().id(foo.id.get()).getActive()).isTrue();
+ String userRef = RefNames.refsUsers(foo.id);
+ accountIndexedCounter.clear();
+
+ AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Administrators"));
+ grant(allUsers, userRef, Permission.PUSH, false, adminGroup.getGroupUUID());
+
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
+
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setBoolean(AccountConfig.ACCOUNT, null, AccountConfig.KEY_ACTIVE, false);
+
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountConfig.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(userRef)
+ .assertOkStatus();
+ accountIndexedCounter.assertReindexOf(foo);
+
+ assertThat(gApi.accounts().id(foo.id.get()).getActive()).isFalse();
}
@Test
@@ -1024,7 +1376,6 @@
@Test
@Sandboxed
- @GerritConfig(name = "user.readAccountsFromGit", value = "true")
public void deleteUserBranchWithAccessDatabaseCapability() throws Exception {
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
grant(
@@ -1282,25 +1633,24 @@
}
@Test
- @GerritConfig(name = "user.readAccountsFromGit", value = "true")
public void checkMetaId() throws Exception {
// metaId is set when account is loaded
- assertThat(accounts.get(db, admin.getId()).getMetaId()).isEqualTo(getMetaId(admin.getId()));
+ assertThat(accounts.get(admin.getId()).getMetaId()).isEqualTo(getMetaId(admin.getId()));
// metaId is set when account is created
AccountsUpdate au = accountsUpdate.create();
Account.Id accountId = new Account.Id(seq.nextAccountId());
- Account account = au.insert(db, accountId, a -> {});
+ Account account = au.insert(accountId, a -> {});
assertThat(account.getMetaId()).isEqualTo(getMetaId(accountId));
// metaId is set when account is updated
- Account updatedAccount = au.update(db, accountId, a -> a.setFullName("foo"));
+ Account updatedAccount = au.update(accountId, a -> a.setFullName("foo"));
assertThat(account.getMetaId()).isNotEqualTo(updatedAccount.getMetaId());
assertThat(updatedAccount.getMetaId()).isEqualTo(getMetaId(accountId));
// metaId is set when account is replaced
Account newAccount = new Account(accountId, TimeUtil.nowTs());
- au.replace(db, newAccount);
+ au.replace(newAccount);
assertThat(updatedAccount.getMetaId()).isNotEqualTo(newAccount.getMetaId());
assertThat(newAccount.getMetaId()).isEqualTo(getMetaId(accountId));
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 69315a2..ed19575 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -87,6 +87,7 @@
import com.google.gerrit.extensions.common.MergeInput;
import com.google.gerrit.extensions.common.MergePatchSetInput;
import com.google.gerrit.extensions.common.RevisionInfo;
+import com.google.gerrit.extensions.common.TrackingIdInfo;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -3190,4 +3191,29 @@
return true;
}
}
+
+ @Test
+ @GerritConfig(name = "trackingid.jira-bug.footer", value = "Bug:")
+ @GerritConfig(name = "trackingid.jira-bug.match", value = "JRA\\d{2,8}")
+ @GerritConfig(name = "trackingid.jira-bug.system", value = "JIRA")
+ public void trackingIds() throws Exception {
+ PushOneCommit push =
+ pushFactory.create(
+ db,
+ admin.getIdent(),
+ testRepo,
+ PushOneCommit.SUBJECT + "\n\n" + "Bug:JRA001",
+ PushOneCommit.FILE_NAME,
+ PushOneCommit.FILE_CONTENT);
+ PushOneCommit.Result result = push.to("refs/for/master");
+ result.assertOkStatus();
+
+ ChangeInfo change =
+ gApi.changes().id(result.getChangeId()).get(EnumSet.of(ListChangesOption.TRACKING_IDS));
+ Collection<TrackingIdInfo> trackingIds = change.trackingIds;
+ assertThat(trackingIds).isNotNull();
+ assertThat(trackingIds).hasSize(1);
+ assertThat(trackingIds.iterator().next().system).isEqualTo("JIRA");
+ assertThat(trackingIds.iterator().next().id).isEqualTo("JRA001");
+ }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 44e5eb0..6face43 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -877,10 +877,7 @@
r.getChangeId());
revision(r).review(new ReviewInput().label("Patch-Set-Lock", 1));
r = push.to("refs/for/master");
- r.assertErrorStatus(
- "cannot add patch set to "
- + r.getChange().change().getChangeId()
- + ". Change is patch set locked.");
+ r.assertErrorStatus("cannot add patch set to " + r.getChange().change().getChangeId() + ".");
}
@Test
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index 87436e7..239c296 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -33,6 +33,7 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testutil.ConfigSuite;
@@ -225,6 +226,7 @@
String changeId2 =
createChangeWithTopic(testRepo, "foo2", "touching b", "b.txt", "real content")
.getChangeId();
+ int changeNum2 = gApi.changes().id(changeId2).info()._number;
approve(changeId2);
// collide with the other change in the same topic
@@ -243,7 +245,7 @@
assertThat(info.enabled).isNull();
assertThat(info.label).isEqualTo("Submit whole topic");
assertThat(info.method).isEqualTo("POST");
- assertThat(info.title).isEqualTo("Problems with change(s): 2");
+ assertThat(info.title).isEqualTo("Problems with change(s): " + changeNum2);
} else {
noSubmitWholeTopicAssertions(actions, 1);
}
@@ -357,9 +359,11 @@
}
@Test
- public void revisionActionVisitor() throws Exception {
+ public void currentRevisionActionVisitor() throws Exception {
String id = createChange().getChangeId();
+ amendChange(id);
ChangeInfo origChange = gApi.changes().id(id).get(EnumSet.of(ListChangesOption.CHANGE_ACTIONS));
+ Change.Id changeId = new Change.Id(origChange._number);
class Visitor implements ActionVisitor {
@Override
@@ -373,7 +377,7 @@
assertThat(changeInfo).isNotNull();
assertThat(changeInfo._number).isEqualTo(origChange._number);
assertThat(revisionInfo).isNotNull();
- assertThat(revisionInfo._number).isEqualTo(1);
+ assertThat(revisionInfo._number).isEqualTo(2);
if (name.equals("cherrypick")) {
return false;
}
@@ -393,24 +397,23 @@
// Test different codepaths within ActionJson...
// ...via revision API.
- visitedRevisionActionsAssertions(origActions, gApi.changes().id(id).current().actions());
+ visitedCurrentRevisionActionsAssertions(origActions, gApi.changes().id(id).current().actions());
// ...via change API with option.
EnumSet<ListChangesOption> opts = EnumSet.of(CURRENT_ACTIONS, CURRENT_REVISION);
ChangeInfo changeInfo = gApi.changes().id(id).get(opts);
RevisionInfo revisionInfo = Iterables.getOnlyElement(changeInfo.revisions.values());
- visitedRevisionActionsAssertions(origActions, revisionInfo.actions);
+ visitedCurrentRevisionActionsAssertions(origActions, revisionInfo.actions);
// ...via ChangeJson directly.
- ChangeData cd = changeDataFactory.create(db, project, new Change.Id(origChange._number));
+ ChangeData cd = changeDataFactory.create(db, project, changeId);
revisionInfo =
changeJsonFactory
.create(opts)
- .getRevisionInfo(cd.changeControl(), Iterables.getOnlyElement(cd.patchSets()));
- visitedRevisionActionsAssertions(origActions, revisionInfo.actions);
+ .getRevisionInfo(cd.changeControl(), cd.patchSet(new PatchSet.Id(changeId, 1)));
}
- private void visitedRevisionActionsAssertions(
+ private void visitedCurrentRevisionActionsAssertions(
Map<String, ActionInfo> origActions, Map<String, ActionInfo> newActions) {
assertThat(newActions).isNotNull();
Set<String> expectedNames = new TreeSet<>(origActions.keySet());
@@ -422,6 +425,50 @@
assertThat(rebase.label).isEqualTo("All Your Base");
}
+ @Test
+ public void oldRevisionActionVisitor() throws Exception {
+ String id = createChange().getChangeId();
+ amendChange(id);
+ ChangeInfo origChange = gApi.changes().id(id).get(EnumSet.of(ListChangesOption.CHANGE_ACTIONS));
+
+ class Visitor implements ActionVisitor {
+ @Override
+ public boolean visit(String name, ActionInfo actionInfo, ChangeInfo changeInfo) {
+ return true; // Do nothing; implicitly called for CURRENT_ACTIONS.
+ }
+
+ @Override
+ public boolean visit(
+ String name, ActionInfo actionInfo, ChangeInfo changeInfo, RevisionInfo revisionInfo) {
+ assertThat(changeInfo).isNotNull();
+ assertThat(changeInfo._number).isEqualTo(origChange._number);
+ assertThat(revisionInfo).isNotNull();
+ assertThat(revisionInfo._number).isEqualTo(1);
+ if (name.equals("description")) {
+ actionInfo.label = "Describify";
+ }
+ return true;
+ }
+ }
+
+ Map<String, ActionInfo> origActions = gApi.changes().id(id).revision(1).actions();
+ assertThat(origActions.keySet()).containsExactly("description");
+ assertThat(origActions.get("description").label).isEqualTo("Edit Description");
+
+ Visitor v = new Visitor();
+ visitorHandle = actionVisitors.add(v);
+
+ // Unlike for the current revision, actions for old revisions are only available via the
+ // revision API.
+ Map<String, ActionInfo> newActions = gApi.changes().id(id).revision(1).actions();
+ assertThat(newActions).isNotNull();
+ assertThat(newActions.keySet()).isEqualTo(origActions.keySet());
+
+ ActionInfo description = newActions.get("description");
+ assertThat(description).isNotNull();
+ assertThat(description.label).isEqualTo("Describify");
+ }
+
private void commonActionsAssertions(Map<String, ActionInfo> actions) {
assertThat(actions).hasSize(4);
assertThat(actions).containsKey("cherrypick");
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/revision/RevisionIT.java
index 89fdeff..220254b 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/revision/RevisionIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/revision/RevisionIT.java
@@ -23,6 +23,8 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.util.Base64;
import org.junit.Test;
@@ -75,4 +77,34 @@
response.assertBadRequest();
assertThat(response.getEntityContent()).isEqualTo("invalid parent");
}
+
+ @Test
+ public void getReview() throws Exception {
+ PushOneCommit.Result r = createChange();
+ ObjectId ps1Commit = r.getCommit();
+ r = amendChange(r.getChangeId());
+ ObjectId ps2Commit = r.getCommit();
+
+ ChangeInfo info1 = checkRevisionReview(r, 1, ps1Commit);
+ assertThat(info1.currentRevision).isNull();
+
+ ChangeInfo info2 = checkRevisionReview(r, 2, ps2Commit);
+ assertThat(info2.currentRevision).isEqualTo(ps2Commit.name());
+ }
+
+ private ChangeInfo checkRevisionReview(
+ PushOneCommit.Result r, int psNum, ObjectId expectedRevision) throws Exception {
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+
+ RestResponse response =
+ adminRestSession.get("/changes/" + r.getChangeId() + "/revisions/" + psNum + "/review");
+ response.assertOK();
+ ChangeInfo info = newGson().fromJson(response.getReader(), ChangeInfo.class);
+
+ // Check for DETAILED_ACCOUNTS, DETAILED_LABELS, and specified revision.
+ assertThat(info.owner.name).isNotNull();
+ assertThat(info.labels.get("Code-Review").all).hasSize(1);
+ assertThat(info.revisions.keySet()).containsExactly(expectedRevision.name());
+ return info;
+ }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
index 4c3a992..b99e99d 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
@@ -126,7 +126,7 @@
public void missingOwner() throws Exception {
TestAccount owner = accountCreator.create("missing");
ChangeControl ctl = insertChange(owner);
- accountsUpdate.create().deleteByKey(db, owner.getId());
+ accountsUpdate.create().deleteByKey(owner.getId());
assertProblems(ctl, null, problem("Missing change owner: " + owner.getId()));
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListChangesOption.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListChangesOption.java
index 787725c..ee7d039 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListChangesOption.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListChangesOption.java
@@ -72,7 +72,10 @@
REVIEWER_UPDATES(19),
/** Set the submittable boolean. */
- SUBMITTABLE(20);
+ SUBMITTABLE(20),
+
+ /** If tracking Ids are included, include detailed tracking Ids info. */
+ TRACKING_IDS(21);
private final int value;
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java
index 0ae45dd5..706482f 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -70,4 +70,5 @@
public List<ProblemInfo> problems;
public List<PluginDefinedInfo> plugins;
+ public Collection<TrackingIdInfo> trackingIds;
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/TrackingIdInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/TrackingIdInfo.java
new file mode 100644
index 0000000..0c5ed68
--- /dev/null
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/TrackingIdInfo.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.common;
+
+public class TrackingIdInfo {
+ public String system;
+ public String id;
+
+ public TrackingIdInfo(String system, String id) {
+ this.system = system;
+ this.id = id;
+ }
+}
diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
index 001c988..07a4fe3 100644
--- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
+++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
@@ -116,7 +116,7 @@
schemaCreator.create(db);
userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId();
// Note: does not match any key in TestKeys.
- accountsUpdate.create().update(db, userId, a -> a.setPreferredEmail("user@example.com"));
+ accountsUpdate.create().update(userId, a -> a.setPreferredEmail("user@example.com"));
user = reloadUser();
requestContext.setContext(
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LocalComments.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LocalComments.java
index 392dc01..44652cf 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LocalComments.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LocalComments.java
@@ -128,7 +128,7 @@
}
private String getReplyCommentName() {
- return "savedReplyComment-" + PageLinks.toChangeId(project, changeId);
+ return "savedReplyComment~" + PageLinks.toChangeId(project, changeId);
}
public static void saveInlineComments() {
@@ -194,9 +194,9 @@
}
private static boolean isInlineComment(String key) {
- return key.startsWith("patchCommentEdit-")
- || key.startsWith("patchReply-")
- || key.startsWith("patchComment-");
+ return key.startsWith("patchCommentEdit~")
+ || key.startsWith("patchReply~")
+ || key.startsWith("patchComment~");
}
private static InlineComment getInlineComment(String key) {
@@ -206,9 +206,9 @@
CommentRange range;
StorageBackend storage = new StorageBackend();
- String[] elements = key.split("-");
+ String[] elements = key.split("~");
int offset = 1;
- if (key.startsWith("patchReply-") || key.startsWith("patchCommentEdit-")) {
+ if (key.startsWith("patchReply~") || key.startsWith("patchCommentEdit~")) {
offset = 2;
}
ProjectChangeId id = ProjectChangeId.create(elements[offset + 0]);
@@ -232,9 +232,9 @@
}
CommentInfo info = CommentInfo.create(path, side, line, range, false);
info.message(storage.getItem(key));
- if (key.startsWith("patchReply-")) {
+ if (key.startsWith("patchReply~")) {
info.inReplyTo(elements[1]);
- } else if (key.startsWith("patchCommentEdit-")) {
+ } else if (key.startsWith("patchCommentEdit~")) {
info.id(elements[1]);
}
InlineComment inlineComment = new InlineComment(id.getProject(), psId, info);
@@ -245,22 +245,22 @@
if (psId == null) {
return null;
}
- String result = "patchComment-";
+ String result = "patchComment~";
if (comment.id() != null) {
- result = "patchCommentEdit-" + comment.id() + "-";
+ result = "patchCommentEdit~" + comment.id() + "~";
} else if (comment.inReplyTo() != null) {
- result = "patchReply-" + comment.inReplyTo() + "-";
+ result = "patchReply~" + comment.inReplyTo() + "~";
}
result += PageLinks.toChangeId(project, changeId);
- result += "-" + psId.getId() + "-" + btoa(comment.path()) + "-" + comment.side() + "-";
+ result += "~" + psId.getId() + "~" + btoa(comment.path()) + "~" + comment.side() + "~";
if (comment.hasRange()) {
result +=
"R"
+ comment.range().startLine()
+ ","
+ comment.range().startCharacter()
- + "-"
+ + "~"
+ comment.range().endLine()
+ ","
+ comment.range().endCharacter();
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
index 01aec6e..1f095e0 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
@@ -239,9 +239,9 @@
} catch (NumberFormatException nfe) {
return null;
}
- try (ReviewDb db = schema.open()) {
- return auth(accounts.get(db, id));
- } catch (OrmException | IOException | ConfigInvalidException e) {
+ try {
+ return auth(accounts.get(id));
+ } catch (IOException | ConfigInvalidException e) {
getServletContext().log("cannot query database", e);
return null;
}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/AccountsOnInit.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/AccountsOnInit.java
index d81d58c..2beb50a 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/AccountsOnInit.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/AccountsOnInit.java
@@ -17,17 +17,14 @@
import static com.google.common.base.Preconditions.checkArgument;
import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.common.collect.ImmutableSet;
import com.google.gerrit.pgm.init.api.AllUsersNameOnInitProvider;
import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdentProvider;
import com.google.gerrit.server.account.AccountConfig;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.config.SitePaths;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.File;
import java.io.IOException;
@@ -62,9 +59,7 @@
this.allUsers = allUsers.get();
}
- public void insert(ReviewDb db, Account account) throws OrmException, IOException {
- db.accounts().insert(ImmutableSet.of(account));
-
+ public void insert(Account account) throws IOException {
File path = getPath();
if (path != null) {
try (Repository repo = new FileRepository(path);
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java
index e4a1cd5..86468c9 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java
@@ -119,7 +119,7 @@
Account a = new Account(id, TimeUtil.nowTs());
a.setFullName(name);
a.setPreferredEmail(email);
- accounts.insert(db, a);
+ accounts.insert(a);
AccountGroup adminGroup =
groupsOnInit.getExistingGroup(db, new AccountGroup.NameKey("Administrators"));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java
index 2e0fe2b..b13c43b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java
@@ -181,7 +181,7 @@
List<ChangeControl> ctls = new ArrayList<>(cds.size());
if (!indexConfig.separateChangeSubIndexes()) {
for (ChangeData cd : cds) {
- ctls.add(cd.changeControl(user));
+ checkedAdd(cd, ctls, user);
}
return ctls;
}
@@ -195,9 +195,18 @@
Set<Change.Id> seen = Sets.newHashSetWithExpectedSize(cds.size());
for (ChangeData cd : cds) {
if (seen.add(cd.getId())) {
- ctls.add(cd.changeControl(user));
+ checkedAdd(cd, ctls, user);
}
}
return ctls;
}
+
+ private static void checkedAdd(ChangeData cd, List<ChangeControl> ctls, CurrentUser user)
+ throws OrmException {
+ try {
+ ctls.add(cd.changeControl(user));
+ } catch (NoSuchChangeException e) {
+ // Ignore
+ }
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/DynamicOptions.java b/gerrit-server/src/main/java/com/google/gerrit/server/DynamicOptions.java
index 6267dca..4854112 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/DynamicOptions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/DynamicOptions.java
@@ -138,7 +138,7 @@
public void parseDynamicBeans(CmdLineParser clp) {
for (Entry<String, DynamicBean> e : beansByPlugin.entrySet()) {
- clp.parseWithPrefix(e.getKey(), e.getValue());
+ clp.parseWithPrefix("--" + e.getKey(), e.getValue());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
index 16901ed..5ac7ac4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
@@ -208,7 +208,7 @@
private Optional<AccountState> load(ReviewDb db, Account.Id who)
throws OrmException, IOException, ConfigInvalidException {
- Account account = accounts.get(db, who);
+ Account account = accounts.get(who);
if (account == null) {
return Optional.empty();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
index 7d91415..3224601 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
@@ -144,7 +144,7 @@
}
// return the identity to the caller.
- update(db, who, id);
+ update(who, id);
return new AuthResult(id.accountId(), who.getExternalIdKey(), false);
}
} catch (OrmException | ConfigInvalidException e) {
@@ -152,7 +152,7 @@
}
}
- private void update(ReviewDb db, AuthRequest who, ExternalId extId)
+ private void update(AuthRequest who, ExternalId extId)
throws OrmException, IOException, ConfigInvalidException {
IdentifiedUser user = userFactory.create(extId.accountId());
List<Consumer<Account>> accountUpdates = new ArrayList<>();
@@ -188,8 +188,7 @@
}
if (!accountUpdates.isEmpty()) {
- Account account =
- accountsUpdateFactory.create().update(db, user.getAccountId(), accountUpdates);
+ Account account = accountsUpdateFactory.create().update(user.getAccountId(), accountUpdates);
if (account == null) {
throw new OrmException("Account " + user.getAccountId() + " has been deleted");
}
@@ -214,7 +213,6 @@
AccountsUpdate accountsUpdate = accountsUpdateFactory.create();
account =
accountsUpdate.insert(
- db,
newId,
a -> {
a.setFullName(who.getDisplayName());
@@ -224,7 +222,7 @@
ExternalId existingExtId = externalIds.get(extId.key());
if (existingExtId != null && !existingExtId.accountId().equals(extId.accountId())) {
// external ID is assigned to another account, do not overwrite
- accountsUpdate.delete(db, account);
+ accountsUpdate.delete(account);
throw new AccountException(
"Cannot assign external ID \""
+ extId.key().get()
@@ -277,7 +275,7 @@
+ "\" to account "
+ newId
+ "; name already in use.";
- handleSettingUserNameFailure(db, account, extId, message, e, false);
+ handleSettingUserNameFailure(account, extId, message, e, false);
} catch (InvalidUserNameException e) {
String message =
"Cannot assign user name \""
@@ -285,10 +283,10 @@
+ "\" to account "
+ newId
+ "; name does not conform.";
- handleSettingUserNameFailure(db, account, extId, message, e, false);
+ handleSettingUserNameFailure(account, extId, message, e, false);
} catch (OrmException e) {
String message = "Cannot assign user name";
- handleSettingUserNameFailure(db, account, extId, message, e, true);
+ handleSettingUserNameFailure(account, extId, message, e, true);
}
}
@@ -302,7 +300,6 @@
* deletes the newly created account and throws an {@link AccountUserNameException}. In any case
* the error message is logged.
*
- * @param db the database
* @param account the newly created account
* @param extId the newly created external id
* @param errorMessage the error message
@@ -313,12 +310,7 @@
* @throws OrmException thrown if cleaning the database failed
*/
private void handleSettingUserNameFailure(
- ReviewDb db,
- Account account,
- ExternalId extId,
- String errorMessage,
- Exception e,
- boolean logException)
+ Account account, ExternalId extId, String errorMessage, Exception e, boolean logException)
throws AccountUserNameException, OrmException, IOException, ConfigInvalidException {
if (logException) {
log.error(errorMessage, e);
@@ -333,7 +325,7 @@
// such an account cannot be used for uploading changes,
// this is why the best we can do here is to fail early and cleanup
// the database
- accountsUpdateFactory.create().delete(db, account);
+ accountsUpdateFactory.create().delete(account);
externalIdsUpdateFactory.create().delete(extId);
throw new AccountUserNameException(errorMessage, e);
}
@@ -350,34 +342,31 @@
*/
public AuthResult link(Account.Id to, AuthRequest who)
throws AccountException, OrmException, IOException, ConfigInvalidException {
- try (ReviewDb db = schema.open()) {
- ExternalId extId = externalIds.get(who.getExternalIdKey());
- if (extId != null) {
- if (!extId.accountId().equals(to)) {
- throw new AccountException("Identity in use by another account");
- }
- update(db, who, extId);
- } else {
- externalIdsUpdateFactory
- .create()
- .insert(ExternalId.createWithEmail(who.getExternalIdKey(), to, who.getEmailAddress()));
-
- if (who.getEmailAddress() != null) {
- accountsUpdateFactory
- .create()
- .update(
- db,
- to,
- a -> {
- if (a.getPreferredEmail() == null) {
- a.setPreferredEmail(who.getEmailAddress());
- }
- });
- }
+ ExternalId extId = externalIds.get(who.getExternalIdKey());
+ if (extId != null) {
+ if (!extId.accountId().equals(to)) {
+ throw new AccountException("Identity in use by another account");
}
+ update(who, extId);
+ } else {
+ externalIdsUpdateFactory
+ .create()
+ .insert(ExternalId.createWithEmail(who.getExternalIdKey(), to, who.getEmailAddress()));
- return new AuthResult(to, who.getExternalIdKey(), false);
+ if (who.getEmailAddress() != null) {
+ accountsUpdateFactory
+ .create()
+ .update(
+ to,
+ a -> {
+ if (a.getPreferredEmail() == null) {
+ a.setPreferredEmail(who.getEmailAddress());
+ }
+ });
+ }
}
+
+ return new AuthResult(to, who.getExternalIdKey(), false);
}
/**
@@ -437,40 +426,36 @@
return;
}
- try (ReviewDb db = schema.open()) {
- List<ExternalId> extIds = new ArrayList<>(extIdKeys.size());
- for (ExternalId.Key extIdKey : extIdKeys) {
- ExternalId extId = externalIds.get(extIdKey);
- if (extId != null) {
- if (!extId.accountId().equals(from)) {
- throw new AccountException(
- "Identity '" + extIdKey.get() + "' in use by another account");
- }
- extIds.add(extId);
- } else {
- throw new AccountException("Identity '" + extIdKey.get() + "' not found");
+ List<ExternalId> extIds = new ArrayList<>(extIdKeys.size());
+ for (ExternalId.Key extIdKey : extIdKeys) {
+ ExternalId extId = externalIds.get(extIdKey);
+ if (extId != null) {
+ if (!extId.accountId().equals(from)) {
+ throw new AccountException("Identity '" + extIdKey.get() + "' in use by another account");
}
+ extIds.add(extId);
+ } else {
+ throw new AccountException("Identity '" + extIdKey.get() + "' not found");
}
+ }
- externalIdsUpdateFactory.create().delete(extIds);
+ externalIdsUpdateFactory.create().delete(extIds);
- if (extIds.stream().anyMatch(e -> e.email() != null)) {
- accountsUpdateFactory
- .create()
- .update(
- db,
- from,
- a -> {
- if (a.getPreferredEmail() != null) {
- for (ExternalId extId : extIds) {
- if (a.getPreferredEmail().equals(extId.email())) {
- a.setPreferredEmail(null);
- break;
- }
+ if (extIds.stream().anyMatch(e -> e.email() != null)) {
+ accountsUpdateFactory
+ .create()
+ .update(
+ from,
+ a -> {
+ if (a.getPreferredEmail() != null) {
+ for (ExternalId extId : extIds) {
+ if (a.getPreferredEmail().equals(extId.email())) {
+ a.setPreferredEmail(null);
+ break;
}
}
- });
- }
+ }
+ });
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
index 894f7a1..94f63a7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
@@ -98,7 +98,7 @@
Matcher m = Pattern.compile("^.* \\(([1-9][0-9]*)\\)$").matcher(nameOrEmail);
if (m.matches()) {
Account.Id id = Account.Id.parse(m.group(1));
- if (exists(db, id)) {
+ if (accounts.get(id) != null) {
return Collections.singleton(id);
}
return Collections.emptySet();
@@ -106,7 +106,7 @@
if (nameOrEmail.matches("^[1-9][0-9]*$")) {
Account.Id id = Account.Id.parse(nameOrEmail);
- if (exists(db, id)) {
+ if (accounts.get(id) != null) {
return Collections.singleton(id);
}
return Collections.emptySet();
@@ -122,11 +122,6 @@
return findAllByNameOrEmail(db, nameOrEmail);
}
- private boolean exists(ReviewDb db, Account.Id id)
- throws OrmException, IOException, ConfigInvalidException {
- return accounts.get(db, id) != null;
- }
-
/**
* Locate exactly one account matching the name or name/email string.
*
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java
index cb82879..cc35438 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java
@@ -20,12 +20,9 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -36,7 +33,6 @@
import java.util.Set;
import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -46,47 +42,35 @@
public class Accounts {
private static final Logger log = LoggerFactory.getLogger(Accounts.class);
- private final boolean readFromGit;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
private final OutgoingEmailValidator emailValidator;
@Inject
Accounts(
- @GerritServerConfig Config cfg,
GitRepositoryManager repoManager,
AllUsersName allUsersName,
OutgoingEmailValidator emailValidator) {
- this.readFromGit = cfg.getBoolean("user", null, "readAccountsFromGit", true);
this.repoManager = repoManager;
this.allUsersName = allUsersName;
this.emailValidator = emailValidator;
}
- public Account get(ReviewDb db, Account.Id accountId)
- throws OrmException, IOException, ConfigInvalidException {
- if (readFromGit) {
- try (Repository repo = repoManager.openRepository(allUsersName)) {
- return read(repo, accountId);
- }
+ public Account get(Account.Id accountId) throws IOException, ConfigInvalidException {
+ try (Repository repo = repoManager.openRepository(allUsersName)) {
+ return read(repo, accountId);
}
-
- return db.accounts().get(accountId);
}
- public List<Account> get(ReviewDb db, Collection<Account.Id> accountIds)
- throws OrmException, IOException, ConfigInvalidException {
- if (readFromGit) {
- List<Account> accounts = new ArrayList<>(accountIds.size());
- try (Repository repo = repoManager.openRepository(allUsersName)) {
- for (Account.Id accountId : accountIds) {
- accounts.add(read(repo, accountId));
- }
+ public List<Account> get(Collection<Account.Id> accountIds)
+ throws IOException, ConfigInvalidException {
+ List<Account> accounts = new ArrayList<>(accountIds.size());
+ try (Repository repo = repoManager.openRepository(allUsersName)) {
+ for (Account.Id accountId : accountIds) {
+ accounts.add(read(repo, accountId));
}
- return accounts;
}
-
- return db.accounts().get(accountIds).toList();
+ return accounts;
}
/**
@@ -94,23 +78,19 @@
*
* @return all accounts
*/
- public List<Account> all(ReviewDb db) throws OrmException, IOException {
- if (readFromGit) {
- Set<Account.Id> accountIds = allIds();
- List<Account> accounts = new ArrayList<>(accountIds.size());
- try (Repository repo = repoManager.openRepository(allUsersName)) {
- for (Account.Id accountId : accountIds) {
- try {
- accounts.add(read(repo, accountId));
- } catch (Exception e) {
- log.error(String.format("Ignoring invalid account %s", accountId.get()), e);
- }
+ public List<Account> all() throws IOException {
+ Set<Account.Id> accountIds = allIds();
+ List<Account> accounts = new ArrayList<>(accountIds.size());
+ try (Repository repo = repoManager.openRepository(allUsersName)) {
+ for (Account.Id accountId : accountIds) {
+ try {
+ accounts.add(read(repo, accountId));
+ } catch (Exception e) {
+ log.error(String.format("Ignoring invalid account %s", accountId.get()), e);
}
}
- return accounts;
}
-
- return db.accounts().all().toList();
+ return accounts;
}
/**
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java
index 2f3f657..0085303 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java
@@ -16,11 +16,8 @@
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.externalids.ExternalIds;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.ArrayList;
@@ -28,22 +25,19 @@
@Singleton
public class AccountsConsistencyChecker {
- private final Provider<ReviewDb> dbProvider;
private final Accounts accounts;
private final ExternalIds externalIds;
@Inject
- AccountsConsistencyChecker(
- Provider<ReviewDb> dbProvider, Accounts accounts, ExternalIds externalIds) {
- this.dbProvider = dbProvider;
+ AccountsConsistencyChecker(Accounts accounts, ExternalIds externalIds) {
this.accounts = accounts;
this.externalIds = externalIds;
}
- public List<ConsistencyProblemInfo> check() throws OrmException, IOException {
+ public List<ConsistencyProblemInfo> check() throws IOException {
List<ConsistencyProblemInfo> problems = new ArrayList<>();
- for (Account account : accounts.all(dbProvider.get())) {
+ for (Account account : accounts.all()) {
if (account.getPreferredEmail() != null) {
if (!externalIds
.byAccount(account.getId())
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java
index f6f7918..6f11015 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java
@@ -17,12 +17,10 @@
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.AllUsersName;
@@ -32,7 +30,6 @@
import com.google.gerrit.server.index.change.ReindexAfterRefUpdate;
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
import com.google.gwtorm.server.OrmDuplicateKeyException;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -50,7 +47,7 @@
/**
* Updates accounts.
*
- * <p>The account updates are written to both ReviewDb and NoteDb.
+ * <p>The account updates are written to NoteDb.
*
* <p>In NoteDb accounts are represented as user branches in the All-Users repository. Optionally a
* user branch can contain a 'account.config' file that stores account properties, such as full
@@ -189,24 +186,19 @@
/**
* Inserts a new account.
*
- * @param db ReviewDb
* @param accountId ID of the new account
* @param init consumer to populate the new account
* @return the newly created account
- * @throws OrmException if updating the database fails
* @throws OrmDuplicateKeyException if the account already exists
* @throws IOException if updating the user branch fails
* @throws ConfigInvalidException if any of the account fields has an invalid value
*/
- public Account insert(ReviewDb db, Account.Id accountId, Consumer<Account> init)
- throws OrmException, IOException, ConfigInvalidException {
+ public Account insert(Account.Id accountId, Consumer<Account> init)
+ throws OrmDuplicateKeyException, IOException, ConfigInvalidException {
AccountConfig accountConfig = read(accountId);
Account account = accountConfig.getNewAccount();
init.accept(account);
- // Create in ReviewDb
- db.accounts().insert(ImmutableSet.of(account));
-
// Create in NoteDb
commitNew(accountConfig);
return account;
@@ -217,17 +209,15 @@
*
* <p>Changing the registration date of an account is not supported.
*
- * @param db ReviewDb
* @param accountId ID of the account
* @param consumer consumer to update the account, only invoked if the account exists
* @return the updated account, {@code null} if the account doesn't exist
- * @throws OrmException if updating the database fails
* @throws IOException if updating the user branch fails
* @throws ConfigInvalidException if any of the account fields has an invalid value
*/
- public Account update(ReviewDb db, Account.Id accountId, Consumer<Account> consumer)
- throws OrmException, IOException, ConfigInvalidException {
- return update(db, accountId, ImmutableList.of(consumer));
+ public Account update(Account.Id accountId, Consumer<Account> consumer)
+ throws IOException, ConfigInvalidException {
+ return update(accountId, ImmutableList.of(consumer));
}
/**
@@ -235,44 +225,26 @@
*
* <p>Changing the registration date of an account is not supported.
*
- * @param db ReviewDb
* @param accountId ID of the account
* @param consumers consumers to update the account, only invoked if the account exists
* @return the updated account, {@code null} if the account doesn't exist
- * @throws OrmException if updating the database fails
* @throws IOException if updating the user branch fails
* @throws ConfigInvalidException if any of the account fields has an invalid value
*/
- public Account update(ReviewDb db, Account.Id accountId, List<Consumer<Account>> consumers)
- throws OrmException, IOException, ConfigInvalidException {
+ public Account update(Account.Id accountId, List<Consumer<Account>> consumers)
+ throws IOException, ConfigInvalidException {
if (consumers.isEmpty()) {
return null;
}
- // Update in ReviewDb
- Account reviewDbAccount =
- db.accounts()
- .atomicUpdate(
- accountId,
- a -> {
- consumers.stream().forEach(c -> c.accept(a));
- return a;
- });
-
- // Update in NoteDb
AccountConfig accountConfig = read(accountId);
Account account = accountConfig.getAccount();
if (account != null) {
consumers.stream().forEach(c -> c.accept(account));
commit(accountConfig);
- return account;
- } else if (reviewDbAccount != null) {
- // user branch doesn't exist yet
- accountConfig.setAccount(reviewDbAccount);
- commitNew(accountConfig);
}
- return reviewDbAccount;
+ return account;
}
/**
@@ -281,25 +253,18 @@
* <p>The existing account with the same account ID is overwritten by the given account. Choosing
* to overwrite an account means that any updates that were done to the account by a racing
* request after the account was read are lost. Updates are also lost if the account was read from
- * a stale account index. This is why using {@link #update(ReviewDb,
- * com.google.gerrit.reviewdb.client.Account.Id, Consumer)} to do an atomic update is always
- * preferred.
+ * a stale account index. This is why using {@link
+ * #update(com.google.gerrit.reviewdb.client.Account.Id, Consumer)} to do an atomic update is
+ * always preferred.
*
* <p>Changing the registration date of an account is not supported.
*
- * @param db ReviewDb
* @param account the new account
- * @throws OrmException if updating the database fails
* @throws IOException if updating the user branch fails
* @throws ConfigInvalidException if any of the account fields has an invalid value
- * @see #update(ReviewDb, com.google.gerrit.reviewdb.client.Account.Id, Consumer)
+ * @see #update(com.google.gerrit.reviewdb.client.Account.Id, Consumer)
*/
- public void replace(ReviewDb db, Account account)
- throws OrmException, IOException, ConfigInvalidException {
- // Update in ReviewDb
- db.accounts().update(ImmutableSet.of(account));
-
- // Update in NoteDb
+ public void replace(Account account) throws IOException, ConfigInvalidException {
AccountConfig accountConfig = read(account.getId());
accountConfig.setAccount(account);
commit(accountConfig);
@@ -308,32 +273,20 @@
/**
* Deletes the account.
*
- * @param db ReviewDb
* @param account the account that should be deleted
- * @throws OrmException if updating the database fails
* @throws IOException if updating the user branch fails
*/
- public void delete(ReviewDb db, Account account) throws OrmException, IOException {
- // Delete in ReviewDb
- db.accounts().delete(ImmutableSet.of(account));
-
- // Delete in NoteDb
- deleteUserBranch(account.getId());
+ public void delete(Account account) throws IOException {
+ deleteByKey(account.getId());
}
/**
* Deletes the account.
*
- * @param db ReviewDb
* @param accountId the ID of the account that should be deleted
- * @throws OrmException if updating the database fails
* @throws IOException if updating the user branch fails
*/
- public void deleteByKey(ReviewDb db, Account.Id accountId) throws OrmException, IOException {
- // Delete in ReviewDb
- db.accounts().deleteKeys(ImmutableSet.of(accountId));
-
- // Delete in NoteDb
+ public void deleteByKey(Account.Id accountId) throws IOException {
deleteUserBranch(accountId);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
index 70cbb6d..87da24b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
@@ -174,7 +174,6 @@
accountsUpdate
.create()
.insert(
- db,
id,
a -> {
a.setFullName(input.name);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
index 792e71d..7537230 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
@@ -23,7 +23,6 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.PutName.Input;
@@ -46,7 +45,6 @@
private final Provider<CurrentUser> self;
private final Realm realm;
private final PermissionBackend permissionBackend;
- private final Provider<ReviewDb> dbProvider;
private final AccountsUpdate.Server accountsUpdate;
@Inject
@@ -54,12 +52,10 @@
Provider<CurrentUser> self,
Realm realm,
PermissionBackend permissionBackend,
- Provider<ReviewDb> dbProvider,
AccountsUpdate.Server accountsUpdate) {
this.self = self;
this.realm = realm;
this.permissionBackend = permissionBackend;
- this.dbProvider = dbProvider;
this.accountsUpdate = accountsUpdate;
}
@@ -74,7 +70,7 @@
}
public Response<String> apply(IdentifiedUser user, Input input)
- throws MethodNotAllowedException, ResourceNotFoundException, OrmException, IOException,
+ throws MethodNotAllowedException, ResourceNotFoundException, IOException,
ConfigInvalidException {
if (input == null) {
input = new Input();
@@ -86,9 +82,7 @@
String newName = input.name;
Account account =
- accountsUpdate
- .create()
- .update(dbProvider.get(), user.getAccountId(), a -> a.setFullName(newName));
+ accountsUpdate.create().update(user.getAccountId(), a -> a.setFullName(newName));
if (account == null) {
throw new ResourceNotFoundException("account not found");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
index 98d4ac5..b3f8fc5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
@@ -19,7 +19,6 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.PutPreferred.Input;
@@ -39,18 +38,15 @@
static class Input {}
private final Provider<CurrentUser> self;
- private final Provider<ReviewDb> dbProvider;
private final PermissionBackend permissionBackend;
private final AccountsUpdate.Server accountsUpdate;
@Inject
PutPreferred(
Provider<CurrentUser> self,
- Provider<ReviewDb> dbProvider,
PermissionBackend permissionBackend,
AccountsUpdate.Server accountsUpdate) {
this.self = self;
- this.dbProvider = dbProvider;
this.permissionBackend = permissionBackend;
this.accountsUpdate = accountsUpdate;
}
@@ -66,13 +62,12 @@
}
public Response<String> apply(IdentifiedUser user, String email)
- throws ResourceNotFoundException, OrmException, IOException, ConfigInvalidException {
+ throws ResourceNotFoundException, IOException, ConfigInvalidException {
AtomicBoolean alreadyPreferred = new AtomicBoolean(false);
Account account =
accountsUpdate
.create()
.update(
- dbProvider.get(),
user.getAccountId(),
a -> {
if (email.equals(a.getPreferredEmail())) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
index 136fc68..1df67c3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
@@ -21,7 +21,6 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.PutStatus.Input;
@@ -48,18 +47,15 @@
}
private final Provider<CurrentUser> self;
- private final Provider<ReviewDb> dbProvider;
private final PermissionBackend permissionBackend;
private final AccountsUpdate.Server accountsUpdate;
@Inject
PutStatus(
Provider<CurrentUser> self,
- Provider<ReviewDb> dbProvider,
PermissionBackend permissionBackend,
AccountsUpdate.Server accountsUpdate) {
this.self = self;
- this.dbProvider = dbProvider;
this.permissionBackend = permissionBackend;
this.accountsUpdate = accountsUpdate;
}
@@ -75,7 +71,7 @@
}
public Response<String> apply(IdentifiedUser user, Input input)
- throws ResourceNotFoundException, OrmException, IOException, ConfigInvalidException {
+ throws ResourceNotFoundException, IOException, ConfigInvalidException {
if (input == null) {
input = new Input();
}
@@ -84,10 +80,7 @@
Account account =
accountsUpdate
.create()
- .update(
- dbProvider.get(),
- user.getAccountId(),
- a -> a.setStatus(Strings.nullToEmpty(newStatus)));
+ .update(user.getAccountId(), a -> a.setStatus(Strings.nullToEmpty(newStatus)));
if (account == null) {
throw new ResourceNotFoundException("account not found");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetInactiveFlag.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetInactiveFlag.java
index b0487de..1698387 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetInactiveFlag.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetInactiveFlag.java
@@ -19,11 +19,8 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -32,23 +29,20 @@
@Singleton
public class SetInactiveFlag {
- private final Provider<ReviewDb> dbProvider;
private final AccountsUpdate.Server accountsUpdate;
@Inject
- SetInactiveFlag(Provider<ReviewDb> dbProvider, AccountsUpdate.Server accountsUpdate) {
- this.dbProvider = dbProvider;
+ SetInactiveFlag(AccountsUpdate.Server accountsUpdate) {
this.accountsUpdate = accountsUpdate;
}
public Response<?> deactivate(IdentifiedUser user)
- throws RestApiException, OrmException, IOException, ConfigInvalidException {
+ throws RestApiException, IOException, ConfigInvalidException {
AtomicBoolean alreadyInactive = new AtomicBoolean(false);
Account account =
accountsUpdate
.create()
.update(
- dbProvider.get(),
user.getAccountId(),
a -> {
if (!a.isActive()) {
@@ -67,13 +61,12 @@
}
public Response<String> activate(IdentifiedUser user)
- throws ResourceNotFoundException, OrmException, IOException, ConfigInvalidException {
+ throws ResourceNotFoundException, IOException, ConfigInvalidException {
AtomicBoolean alreadyActive = new AtomicBoolean(false);
Account account =
accountsUpdate
.create()
.update(
- dbProvider.get(),
user.getAccountId(),
a -> {
if (a.isActive()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/avatar/AvatarProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/avatar/AvatarProvider.java
index 72134c1..21a1556 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/avatar/AvatarProvider.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/avatar/AvatarProvider.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.avatar;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.server.IdentifiedUser;
/**
@@ -24,6 +25,7 @@
*/
@ExtensionPoint
public interface AvatarProvider {
+
/**
* Get avatar URL.
*
@@ -46,4 +48,36 @@
* possible.
*/
String getChangeAvatarUrl(IdentifiedUser forUser);
+
+ /**
+ * Set the avatar image URL for specified user and specified size.
+ *
+ * <p>It is the default method (not interface method declaration) for back compatibility with old
+ * code.
+ *
+ * @param forUser The user for which need to change the avatar image.
+ * @param url The avatar image URL for the specified user.
+ * @param imageSize The avatar image size in pixels. If imageSize have a zero value this indicates
+ * to set URL for default size that provider determines.
+ * @throws Exception if an error occurred.
+ */
+ default void setUrl(IdentifiedUser forUser, String url, int imageSize) throws Exception {
+ throw new NotImplementedException();
+ }
+
+ /**
+ * Indicates whether or not the provider allows to set the image URL.
+ *
+ * <p>It is the default method (not interface method declaration) for back compatibility with old
+ * code.
+ *
+ * @return
+ * <ul>
+ * <li>true - avatar image URL could be set.
+ * <li>false - avatar image URL could not be set (for example not Implemented).
+ * </ul>
+ */
+ default boolean canSetUrl() {
+ return false;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 7208de1..0a44b34 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -34,6 +34,7 @@
import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED;
import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWER_UPDATES;
import static com.google.gerrit.extensions.client.ListChangesOption.SUBMITTABLE;
+import static com.google.gerrit.extensions.client.ListChangesOption.TRACKING_IDS;
import static com.google.gerrit.extensions.client.ListChangesOption.WEB_LINKS;
import static com.google.gerrit.server.CommonConverters.toGitPerson;
import static java.util.stream.Collectors.toList;
@@ -48,6 +49,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.MultimapBuilder;
@@ -75,6 +77,7 @@
import com.google.gerrit.extensions.common.PushCertificateInfo;
import com.google.gerrit.extensions.common.ReviewerUpdateInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
+import com.google.gerrit.extensions.common.TrackingIdInfo;
import com.google.gerrit.extensions.common.VotingRangeInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.config.DownloadCommand;
@@ -104,6 +107,7 @@
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.api.accounts.AccountInfoComparator;
import com.google.gerrit.server.api.accounts.GpgApiAdapter;
+import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.index.change.ChangeField;
@@ -215,7 +219,7 @@
private final ChangeIndexCollection indexes;
private final ApprovalsUtil approvalsUtil;
private final RemoveReviewerControl removeReviewerControl;
-
+ private final TrackingFooters trackingFooters;
private boolean lazyLoad = true;
private AccountLoader accountLoader;
private FixInput fix;
@@ -247,6 +251,7 @@
ChangeIndexCollection indexes,
ApprovalsUtil approvalsUtil,
RemoveReviewerControl removeReviewerControl,
+ TrackingFooters trackingFooters,
@Assisted Iterable<ListChangesOption> options) {
this.db = db;
this.userProvider = user;
@@ -273,6 +278,7 @@
this.approvalsUtil = approvalsUtil;
this.removeReviewerControl = removeReviewerControl;
this.options = Sets.immutableEnumSet(options);
+ this.trackingFooters = trackingFooters;
}
public ChangeJson lazyLoad(boolean load) {
@@ -586,7 +592,7 @@
// This block must come after the ChangeInfo is mostly populated, since
// it will be passed to ActionVisitors as-is.
if (needRevisions) {
- out.revisions = revisions(ctl, cd, src, out);
+ out.revisions = revisions(ctl, cd, src, limitToPsId, out);
if (out.revisions != null) {
for (Map.Entry<String, RevisionInfo> entry : out.revisions.entrySet()) {
if (entry.getValue().isCurrent) {
@@ -601,6 +607,15 @@
actionJson.addChangeActions(out, ctl);
}
+ if (has(TRACKING_IDS)) {
+ ListMultimap<String, String> set = trackingFooters.extract(cd.commitFooters());
+ out.trackingIds =
+ set.entries()
+ .stream()
+ .map(e -> new TrackingIdInfo(e.getKey(), e.getValue()))
+ .collect(toList());
+ }
+
return out;
}
@@ -1202,14 +1217,26 @@
}
private Map<String, RevisionInfo> revisions(
- ChangeControl ctl, ChangeData cd, Map<PatchSet.Id, PatchSet> map, ChangeInfo changeInfo)
+ ChangeControl ctl,
+ ChangeData cd,
+ Map<PatchSet.Id, PatchSet> map,
+ Optional<PatchSet.Id> limitToPsId,
+ ChangeInfo changeInfo)
throws PatchListNotAvailableException, GpgException, OrmException, IOException {
Map<String, RevisionInfo> res = new LinkedHashMap<>();
try (Repository repo = openRepoIfNecessary(ctl);
RevWalk rw = newRevWalk(repo)) {
for (PatchSet in : map.values()) {
- if ((has(ALL_REVISIONS) || in.getId().equals(ctl.getChange().currentPatchSetId()))
- && ctl.isPatchVisible(in, db.get())) {
+ PatchSet.Id id = in.getId();
+ boolean want = false;
+ if (has(ALL_REVISIONS)) {
+ want = true;
+ } else if (limitToPsId.isPresent()) {
+ want = id.equals(limitToPsId.get());
+ } else {
+ want = id.equals(ctl.getChange().currentPatchSetId());
+ }
+ if (want && ctl.isPatchVisible(in, db.get())) {
res.put(in.getRevision().get(), toRevisionInfo(ctl, cd, in, repo, rw, false, changeInfo));
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java
index 56f637e..46d8063 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java
@@ -224,10 +224,10 @@
private void checkOwner() {
try {
- if (accounts.get(db.get(), change().getOwner()) == null) {
+ if (accounts.get(change().getOwner()) == null) {
problem("Missing change owner: " + change().getOwner());
}
- } catch (OrmException | IOException | ConfigInvalidException e) {
+ } catch (IOException | ConfigInvalidException e) {
error("Failed to look up owner", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
index e58ab74..6be83b3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -127,7 +127,7 @@
import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.MergeValidationListener;
import com.google.gerrit.server.git.validators.MergeValidators;
-import com.google.gerrit.server.git.validators.MergeValidators.AccountValidator;
+import com.google.gerrit.server.git.validators.MergeValidators.AccountMergeValidator;
import com.google.gerrit.server.git.validators.MergeValidators.ProjectConfigValidator;
import com.google.gerrit.server.git.validators.OnSubmitValidationListener;
import com.google.gerrit.server.git.validators.OnSubmitValidators;
@@ -393,7 +393,7 @@
bind(AnonymousUser.class);
factory(AbandonOp.Factory.class);
- factory(AccountValidator.Factory.class);
+ factory(AccountMergeValidator.Factory.class);
factory(RefOperationValidators.Factory.class);
factory(OnSubmitValidators.Factory.class);
factory(MergeValidators.Factory.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 286ba12..fde5ba3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -2341,11 +2341,7 @@
try {
permissions.change(notes).database(db).check(ChangePermission.ADD_PATCH_SET);
} catch (AuthException no) {
- String locked = ".";
- if (projectControl.controlFor(notes).isPatchSetLocked(db)) {
- locked = ". Change is patch set locked.";
- }
- reject(inputCommand, "cannot add patch set to " + ontoChange + locked);
+ reject(inputCommand, "cannot add patch set to " + ontoChange + ".");
return false;
}
@@ -2726,7 +2722,6 @@
accountsUpdate
.create()
.update(
- db,
user.getAccountId(),
a -> {
if (Strings.isNullOrEmpty(a.getFullName())) {
@@ -2736,7 +2731,7 @@
if (account != null && Strings.isNullOrEmpty(account.getFullName())) {
user.getAccount().setFullName(account.getFullName());
}
- } catch (OrmException | IOException | ConfigInvalidException e) {
+ } catch (IOException | ConfigInvalidException e) {
logWarn("Cannot default full_name", e);
} finally {
defaultName = false;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/AccountValidator.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/AccountValidator.java
new file mode 100644
index 0000000..24a50cc
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/AccountValidator.java
@@ -0,0 +1,91 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git.validators;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountConfig;
+import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import javax.inject.Inject;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+public class AccountValidator {
+
+ private final Provider<IdentifiedUser> self;
+ private final OutgoingEmailValidator emailValidator;
+
+ @Inject
+ public AccountValidator(Provider<IdentifiedUser> self, OutgoingEmailValidator emailValidator) {
+ this.self = self;
+ this.emailValidator = emailValidator;
+ }
+
+ public List<String> validate(
+ Account.Id accountId, RevWalk rw, @Nullable ObjectId oldId, ObjectId newId)
+ throws IOException {
+ Account oldAccount = null;
+ if (oldId != null && !ObjectId.zeroId().equals(oldId)) {
+ try {
+ oldAccount = loadAccount(accountId, rw, oldId);
+ } catch (ConfigInvalidException e) {
+ // ignore, maybe the new commit is repairing it now
+ }
+ }
+
+ Account newAccount;
+ try {
+ newAccount = loadAccount(accountId, rw, newId);
+ } catch (ConfigInvalidException e) {
+ return ImmutableList.of(
+ String.format(
+ "commit '%s' has an invalid '%s' file for account '%s': %s",
+ newId.name(), AccountConfig.ACCOUNT_CONFIG, accountId.get(), e.getMessage()));
+ }
+
+ List<String> messages = new ArrayList<>();
+ if (accountId.equals(self.get().getAccountId()) && !newAccount.isActive()) {
+ messages.add("cannot deactivate own account");
+ }
+
+ if (newAccount.getPreferredEmail() != null
+ && (oldAccount == null
+ || !newAccount.getPreferredEmail().equals(oldAccount.getPreferredEmail()))) {
+ if (!emailValidator.isValid(newAccount.getPreferredEmail())) {
+ messages.add(
+ String.format(
+ "invalid preferred email '%s' for account '%s'",
+ newAccount.getPreferredEmail(), accountId.get()));
+ }
+ }
+
+ return ImmutableList.copyOf(messages);
+ }
+
+ private Account loadAccount(Account.Id accountId, RevWalk rw, ObjectId commit)
+ throws IOException, ConfigInvalidException {
+ rw.reset();
+ AccountConfig accountConfig = new AccountConfig(null, accountId);
+ accountConfig.load(rw, commit);
+ return accountConfig.getAccount();
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 41381e8..b384405 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -32,7 +32,6 @@
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountConfig;
import com.google.gerrit.server.account.WatchConfig;
import com.google.gerrit.server.account.externalids.ExternalIdsConsistencyChecker;
import com.google.gerrit.server.config.AllUsersName;
@@ -58,11 +57,9 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
-import java.util.Objects;
import java.util.regex.Pattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.notes.NoteMap;
@@ -70,7 +67,6 @@
import org.eclipse.jgit.revwalk.FooterLine;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.util.SystemReader;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -88,6 +84,7 @@
private final DynamicSet<CommitValidationListener> pluginValidators;
private final AllUsersName allUsers;
private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker;
+ private final AccountValidator accountValidator;
private final String installCommitMsgHookCommand;
private final ProjectCache projectCache;
@@ -99,12 +96,14 @@
DynamicSet<CommitValidationListener> pluginValidators,
AllUsersName allUsers,
ExternalIdsConsistencyChecker externalIdsConsistencyChecker,
+ AccountValidator accountValidator,
ProjectCache projectCache) {
this.gerritIdent = gerritIdent;
this.canonicalWebUrl = canonicalWebUrl;
this.pluginValidators = pluginValidators;
this.allUsers = allUsers;
this.externalIdsConsistencyChecker = externalIdsConsistencyChecker;
+ this.accountValidator = accountValidator;
this.installCommitMsgHookCommand =
cfg != null ? cfg.getString("gerrit", null, "installCommitMsgHookCommand") : null;
this.projectCache = projectCache;
@@ -133,7 +132,7 @@
new BannedCommitsValidator(rejectCommits),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
- new AccountValidator(allUsers)));
+ new AccountCommitValidator(allUsers, accountValidator)));
}
public CommitValidators forGerritCommits(
@@ -158,7 +157,7 @@
new ConfigValidator(branch, user, rw, allUsers),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
- new AccountValidator(allUsers)));
+ new AccountCommitValidator(allUsers, accountValidator)));
}
public CommitValidators forMergedCommits(PermissionBackend.ForRef perm, IdentifiedUser user) {
@@ -709,11 +708,13 @@
}
/** Rejects updates to 'account.config' in user branches. */
- public static class AccountValidator implements CommitValidationListener {
+ public static class AccountCommitValidator implements CommitValidationListener {
private final AllUsersName allUsers;
+ private final AccountValidator accountValidator;
- public AccountValidator(AllUsersName allUsers) {
+ public AccountCommitValidator(AllUsersName allUsers, AccountValidator accountValidator) {
this.allUsers = allUsers;
+ this.accountValidator = accountValidator;
}
@Override
@@ -725,7 +726,7 @@
if (receiveEvent.command.getRefName().startsWith(MagicBranch.NEW_CHANGE)) {
// no validation on push for review, will be checked on submit by
- // MergeValidators.AccountValidator
+ // MergeValidators.AccountMergeValidator
return Collections.emptyList();
}
@@ -735,15 +736,19 @@
}
try {
- ObjectId newBlobId = getAccountConfigBlobId(receiveEvent.revWalk, receiveEvent.commit);
-
- ObjectId oldId = receiveEvent.command.getOldId();
- ObjectId oldBlobId =
- !ObjectId.zeroId().equals(oldId)
- ? getAccountConfigBlobId(receiveEvent.revWalk, oldId)
- : null;
- if (!Objects.equals(oldBlobId, newBlobId)) {
- throw new CommitValidationException("account update not allowed");
+ List<String> errorMessages =
+ accountValidator.validate(
+ accountId,
+ receiveEvent.revWalk,
+ receiveEvent.command.getOldId(),
+ receiveEvent.commit);
+ if (!errorMessages.isEmpty()) {
+ throw new CommitValidationException(
+ "invalid account configuration",
+ errorMessages
+ .stream()
+ .map(m -> new CommitValidationMessage(m, true))
+ .collect(toList()));
}
} catch (IOException e) {
String m = String.format("Validating update for account %s failed", accountId.get());
@@ -752,14 +757,6 @@
}
return Collections.emptyList();
}
-
- private ObjectId getAccountConfigBlobId(RevWalk rw, ObjectId id) throws IOException {
- RevCommit commit = rw.parseCommit(id);
- try (TreeWalk tw =
- TreeWalk.forPath(rw.getObjectReader(), AccountConfig.ACCOUNT_CONFIG, commit.getTree())) {
- return tw != null ? tw.getObjectId(0) : null;
- }
- }
}
private static CommitValidationMessage invalidEmail(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
index af9f6d5..fd524b4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.git.validators;
+import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType;
import com.google.gerrit.extensions.registration.DynamicMap;
@@ -47,6 +48,7 @@
import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -55,7 +57,7 @@
private final DynamicSet<MergeValidationListener> mergeValidationListeners;
private final ProjectConfigValidator.Factory projectConfigValidatorFactory;
- private final AccountValidator.Factory accountValidatorFactory;
+ private final AccountMergeValidator.Factory accountValidatorFactory;
public interface Factory {
MergeValidators create();
@@ -65,7 +67,7 @@
MergeValidators(
DynamicSet<MergeValidationListener> mergeValidationListeners,
ProjectConfigValidator.Factory projectConfigValidatorFactory,
- AccountValidator.Factory accountValidatorFactory) {
+ AccountMergeValidator.Factory accountValidatorFactory) {
this.mergeValidationListeners = mergeValidationListeners;
this.projectConfigValidatorFactory = projectConfigValidatorFactory;
this.accountValidatorFactory = accountValidatorFactory;
@@ -222,23 +224,26 @@
}
}
- public static class AccountValidator implements MergeValidationListener {
+ public static class AccountMergeValidator implements MergeValidationListener {
public interface Factory {
- AccountValidator create();
+ AccountMergeValidator create();
}
private final Provider<ReviewDb> dbProvider;
private final AllUsersName allUsersName;
private final ChangeData.Factory changeDataFactory;
+ private final AccountValidator accountValidator;
@Inject
- public AccountValidator(
+ public AccountMergeValidator(
Provider<ReviewDb> dbProvider,
AllUsersName allUsersName,
- ChangeData.Factory changeDataFactory) {
+ ChangeData.Factory changeDataFactory,
+ AccountValidator accountValidator) {
this.dbProvider = dbProvider;
this.allUsersName = allUsersName;
this.changeDataFactory = changeDataFactory;
+ this.accountValidator = accountValidator;
}
@Override
@@ -250,30 +255,33 @@
PatchSet.Id patchSetId,
IdentifiedUser caller)
throws MergeValidationException {
- if (!allUsersName.equals(destProject.getProject().getNameKey())
- || Account.Id.fromRef(destBranch.get()) == null) {
+ Account.Id accountId = Account.Id.fromRef(destBranch.get());
+ if (!allUsersName.equals(destProject.getProject().getNameKey()) || accountId == null) {
return;
}
- if (commit.getParentCount() > 1) {
- // for merge commits we cannot ensure that the 'account.config' file is not modified, since
- // for merge commits file modifications that come in through the merge don't appear in the
- // file list that is returned by ChangeData#currentFilePaths()
- throw new MergeValidationException("cannot submit merge commit to user branch");
- }
-
ChangeData cd =
changeDataFactory.create(
dbProvider.get(), destProject.getProject().getNameKey(), patchSetId.getParentKey());
try {
- if (cd.currentFilePaths().contains(AccountConfig.ACCOUNT_CONFIG)) {
- throw new MergeValidationException(
- String.format("update of %s not allowed", AccountConfig.ACCOUNT_CONFIG));
+ if (!cd.currentFilePaths().contains(AccountConfig.ACCOUNT_CONFIG)) {
+ return;
}
} catch (OrmException e) {
log.error("Cannot validate account update", e);
throw new MergeValidationException("account validation unavailable");
}
+
+ try (RevWalk rw = new RevWalk(repo)) {
+ List<String> errorMessages = accountValidator.validate(accountId, rw, null, commit);
+ if (!errorMessages.isEmpty()) {
+ throw new MergeValidationException(
+ "invalid account configuration: " + Joiner.on("; ").join(errorMessages));
+ }
+ } catch (IOException e) {
+ log.error("Cannot validate account update", e);
+ throw new MergeValidationException("account validation unavailable");
+ }
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 78bd167..6597951 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -319,7 +319,7 @@
}
/** Is the current patch set locked against state changes? */
- public boolean isPatchSetLocked(ReviewDb db) throws OrmException {
+ boolean isPatchSetLocked(ReviewDb db) throws OrmException {
if (getChange().getStatus() == Change.Status.MERGED) {
return false;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index a932923..88bff63 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -615,14 +615,11 @@
}
throw new IllegalStateException("user already specified: " + changeControl.getUser());
}
- try {
- if (change != null) {
- changeControl = changeControlFactory.controlFor(db, change, user);
- } else {
- changeControl = changeControlFactory.controlFor(db, project(), legacyId, user);
- }
- } catch (NoSuchChangeException e) {
- throw new OrmException(e);
+
+ if (change != null) {
+ changeControl = changeControlFactory.controlFor(db, change, user);
+ } else {
+ changeControl = changeControlFactory.controlFor(db, project(), legacyId, user);
}
return changeControl;
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index c5eea0f..2f84a58 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -18,7 +18,6 @@
import static java.util.stream.Collectors.toList;
import static org.junit.Assert.fail;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.accounts.Accounts.QueryRequest;
@@ -397,14 +396,9 @@
public void reindex() throws Exception {
AccountInfo user1 = newAccountWithFullName("tester", "Test Usre");
- // update account in ReviewDb without reindex so that account index is stale
- String newName = "Test User";
+ // update account without reindex so that account index is stale
Account.Id accountId = new Account.Id(user1._accountId);
- Account account = accounts.get(db, accountId);
- account.setFullName(newName);
- db.accounts().update(ImmutableSet.of(account));
-
- // update account in NoteDb without reindex so that account index is stale
+ String newName = "Test User";
try (Repository repo = repoManager.openRepository(allUsers)) {
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsers, repo);
PersonIdent ident = serverIdent.get();
@@ -499,7 +493,6 @@
accountsUpdate
.create()
.update(
- db,
id,
a -> {
a.setFullName(fullName);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 9ab4db0..ea9af4c 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -214,7 +214,7 @@
userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId();
String email = "user@example.com";
externalIdsUpdate.create().insert(ExternalId.createEmail(userId, email));
- accountsUpdate.create().update(db, userId, a -> a.setPreferredEmail(email));
+ accountsUpdate.create().update(userId, a -> a.setPreferredEmail(email));
user = userFactory.create(userId);
requestContext.setContext(newRequestContext(userId));
}
@@ -2469,7 +2469,6 @@
accountsUpdate
.create()
.update(
- db,
id,
a -> {
a.setFullName(fullName);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
index 09b2af9..6b94e02 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
@@ -323,7 +323,6 @@
accountsUpdate
.create()
.update(
- db,
id,
a -> {
a.setFullName(fullName);
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java
index c510680..747c01a 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java
@@ -32,6 +32,7 @@
import com.google.gerrit.server.ioutil.ColumnFormatter;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
+import com.google.gerrit.util.cli.Options;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -44,7 +45,7 @@
runsAt = MASTER_OR_SLAVE
)
public class ListGroupsCommand extends SshCommand {
- @Inject private MyListGroups impl;
+ @Inject @Options public MyListGroups impl;
@Override
public void run() throws Exception {
@@ -54,11 +55,6 @@
impl.display(stdout);
}
- @Override
- protected void parseCommandLine() throws UnloggedFailure {
- parseCommandLine(impl);
- }
-
private static class MyListGroups extends ListGroups {
@Option(
name = "--verbose",
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListProjectsCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListProjectsCommand.java
index 0c3cdcb..776a8b8 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListProjectsCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListProjectsCommand.java
@@ -19,6 +19,7 @@
import com.google.gerrit.server.project.ListProjects;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
+import com.google.gerrit.util.cli.Options;
import com.google.inject.Inject;
import java.util.List;
@@ -28,7 +29,7 @@
runsAt = MASTER_OR_SLAVE
)
final class ListProjectsCommand extends SshCommand {
- @Inject private ListProjects impl;
+ @Inject @Options public ListProjects impl;
@Override
public void run() throws Exception {
@@ -43,9 +44,4 @@
}
impl.display(out);
}
-
- @Override
- protected void parseCommandLine() throws UnloggedFailure {
- parseCommandLine(impl);
- }
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginLsCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginLsCommand.java
index 0fdd105..717d797 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginLsCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginLsCommand.java
@@ -25,6 +25,7 @@
import com.google.gerrit.server.plugins.ListPlugins;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
+import com.google.gerrit.util.cli.Options;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject;
import java.util.Map;
@@ -33,21 +34,13 @@
@RequiresCapability(GlobalCapability.VIEW_PLUGINS)
@CommandMetaData(name = "ls", description = "List the installed plugins", runsAt = MASTER_OR_SLAVE)
final class PluginLsCommand extends SshCommand {
- @Inject private ListPlugins list;
-
- @Option(
- name = "--all",
- aliases = {"-a"},
- usage = "List all plugins, including disabled plugins"
- )
- private boolean all;
+ @Inject @Options public ListPlugins list;
@Option(name = "--format", usage = "output format")
private OutputFormat format = OutputFormat.TEXT;
@Override
public void run() throws Exception {
- list.setAll(all);
Map<String, PluginInfo> output = list.apply(TopLevelResource.INSTANCE);
if (format.isJson()) {
@@ -75,9 +68,4 @@
private String status(Boolean disabled) {
return disabled != null && disabled.booleanValue() ? "DISABLED" : "ENABLED";
}
-
- @Override
- protected void parseCommandLine() throws UnloggedFailure {
- parseCommandLine(this);
- }
}
diff --git a/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/CmdLineParser.java b/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/CmdLineParser.java
index bb293cc..0a75bfc 100644
--- a/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/CmdLineParser.java
+++ b/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/CmdLineParser.java
@@ -48,9 +48,11 @@
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.ResourceBundle;
+import java.util.Set;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.IllegalAnnotationError;
@@ -381,7 +383,7 @@
}
private static String getPrefixedName(String prefix, String name) {
- return "--" + prefix + name;
+ return prefix + name;
}
}
@@ -393,11 +395,19 @@
MyParser(Object bean) {
super(bean);
+ parseAdditionalOptions("", bean, new HashSet<>());
ensureOptionsInitialized();
}
// NOTE: Argument annotations on bean are ignored.
public void parseWithPrefix(String prefix, Object bean) {
+ parseWithPrefix(prefix, bean, new HashSet<>());
+ }
+
+ private void parseWithPrefix(String prefix, Object bean, Set<Object> parsedBeans) {
+ if (!parsedBeans.add(bean)) {
+ return;
+ }
// recursively process all the methods/fields.
for (Class<?> c = bean.getClass(); c != null; c = c.getSuperclass()) {
for (Method m : c.getDeclaredMethods()) {
@@ -411,6 +421,31 @@
if (o != null) {
addOption(Setters.create(f, bean), new PrefixedOption(prefix, o));
}
+ if (f.isAnnotationPresent(Options.class)) {
+ try {
+ parseWithPrefix(
+ prefix + f.getAnnotation(Options.class).prefix(), f.get(bean), parsedBeans);
+ } catch (IllegalAccessException e) {
+ throw new IllegalAnnotationError(e);
+ }
+ }
+ }
+ }
+ }
+
+ private void parseAdditionalOptions(String prefix, Object bean, Set<Object> parsedBeans) {
+ for (Class<?> c = bean.getClass(); c != null; c = c.getSuperclass()) {
+ for (Field f : c.getDeclaredFields()) {
+ if (f.isAnnotationPresent(Options.class)) {
+ Object additionalBean = null;
+ try {
+ additionalBean = f.get(bean);
+ } catch (IllegalAccessException e) {
+ throw new IllegalAnnotationError(e);
+ }
+ parseWithPrefix(
+ prefix + f.getAnnotation(Options.class).prefix(), additionalBean, parsedBeans);
+ }
}
}
}
diff --git a/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/Options.java b/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/Options.java
new file mode 100644
index 0000000..96613df
--- /dev/null
+++ b/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/Options.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.util.cli;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Marks a field that refers to a class with @Option annotations
+ *
+ * <p>Any @Option annotations found on the referred class will be handled as if they were found on
+ * the referring class.
+ */
+@Retention(RUNTIME)
+@Target({FIELD})
+public @interface Options {
+ String prefix() default "";
+}
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.html b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.html
index 7217fae..c804933 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.html
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.html
@@ -33,6 +33,7 @@
<link rel="import" href="../gr-group-members/gr-group-members.html">
<link rel="import" href="../gr-plugin-list/gr-plugin-list.html">
<link rel="import" href="../gr-project/gr-project.html">
+<link rel="import" href="../gr-project-access/gr-project-access.html">
<link rel="import" href="../gr-project-commands/gr-project-commands.html">
<link rel="import" href="../gr-project-detail-list/gr-project-detail-list.html">
@@ -127,6 +128,11 @@
project="[[params.project]]"></gr-project-commands>
</main>
</template>
+ <template is="dom-if" if="[[_showProjectAccess]]" restamp="true">
+ <main class="table">
+ <gr-project-access project="[[params.project]]"></gr-project-access>
+ </main>
+ </template>
<template is="dom-if" if="[[params.placeholder]]" restamp="true">
<gr-placeholder title="Admin" path="[[path]]"></gr-placeholder>
</template>
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
index 9ca6fd4..e75c636 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
@@ -69,6 +69,7 @@
_showProjectList: Boolean,
_showProjectDetailList: Boolean,
_showPluginList: Boolean,
+ _showProjectAccess: Boolean,
},
behaviors: [
@@ -110,6 +111,13 @@
view: 'gr-project',
url: `/admin/projects/${this.encodeURL(this._projectName, true)}`,
children: [{
+ name: 'Access',
+ detailType: 'access',
+ view: 'gr-project-access',
+ url: `/admin/projects/` +
+ `${this.encodeURL(this._projectName, true)},access`,
+ },
+ {
name: 'Commands',
detailType: 'commands',
view: 'gr-project-commands',
@@ -187,6 +195,7 @@
this.set('_showProjectDetailList',
params.adminView === 'gr-project-detail-list');
this.set('_showPluginList', params.adminView === 'gr-plugin-list');
+ this.set('_showProjectAccess', params.adminView === 'gr-project-access');
if (params.project !== this._projectName) {
this._projectName = params.project || '';
// Reloads the admin menu.
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
index 2877f14..bfced55 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
@@ -21,6 +21,7 @@
<link rel="import" href="../../../bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
<link rel="import" href="../../shared/gr-autocomplete/gr-autocomplete.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-overlay/gr-overlay.html">
@@ -107,7 +108,7 @@
<template is="dom-repeat" items="[[_groupMembers]]">
<tr>
<td class="nameColumn">
- <a href$="[[_memberUrl(item)]]">[[item.name]]</a>
+ <gr-account-link account="[[item]]"></gr-account-link>
</td>
<td>[[item.email]]</td>
<td hidden$="[[!_groupOwner]]">
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
index 57b5c60..cde6c66 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
@@ -93,18 +93,6 @@
return this._loading || this._loading === undefined;
},
- _memberUrl(item) {
- if (item.email) {
- item = item.email;
- } else if (item.username) {
- item = item.username;
- } else {
- item = item.name;
- }
- return this.getBaseUrl() + '/q/owner:' + this.encodeURL(item, true) +
- ' status:open';
- },
-
_groupUrl(item) {
return this.getBaseUrl() + '/admin/groups/' + this.encodeURL(item, true);
},
diff --git a/polygerrit-ui/app/elements/admin/gr-project-access/gr-project-access.html b/polygerrit-ui/app/elements/admin/gr-project-access/gr-project-access.html
new file mode 100644
index 0000000..d776763
--- /dev/null
+++ b/polygerrit-ui/app/elements/admin/gr-project-access/gr-project-access.html
@@ -0,0 +1,55 @@
+<!--
+Copyright (C) 2017 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+
+<link rel="import" href="../../../behaviors/gr-access-behavior/gr-access-behavior.html">
+<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
+<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
+
+<link rel="import" href="../../../styles/gr-menu-page-styles.html">
+<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
+<link rel="import" href="../gr-access-section/gr-access-section.html">
+
+<script src="../../../scripts/util.js"></script>
+
+<dom-module id="gr-project-access">
+ <template>
+ <style include="shared-styles"></style>
+ <style include="gr-menu-page-styles"></style>
+ <main>
+ <template
+ is="dom-repeat"
+ items="{{_sections}}"
+ as="section">
+ <gr-access-section
+ capabilities="[[_capabilities]]"
+ section="{{section}}"
+ labels="[[_labels]]"
+ editing="[[_editing]]"></gr-access-section>
+ </template>
+ <template is="dom-if" if="[[_inheritsFrom]]">
+ <h3 id="inheritsFrom">Rights Inherit From
+ <a href$="[[_computeParentHref(_inheritsFrom.name)]]" rel="noopener">
+ [[_inheritsFrom.name]]</a>
+ </h3>
+ </template>
+ </main>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
+ </template>
+ <script src="gr-project-access.js"></script>
+</dom-module>
diff --git a/polygerrit-ui/app/elements/admin/gr-project-access/gr-project-access.js b/polygerrit-ui/app/elements/admin/gr-project-access/gr-project-access.js
new file mode 100644
index 0000000..808279a
--- /dev/null
+++ b/polygerrit-ui/app/elements/admin/gr-project-access/gr-project-access.js
@@ -0,0 +1,80 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+(function() {
+ 'use strict';
+
+ Polymer({
+ is: 'gr-project-access',
+
+ properties: {
+ project: {
+ type: String,
+ observer: '_projectChanged',
+ },
+
+ _capabilities: Object,
+ /** @type {?} */
+ _inheritsFrom: Object,
+ _labels: Object,
+ _local: Object,
+ _editing: {
+ type: Boolean,
+ value: false,
+ },
+ _sections: Array,
+ },
+
+ behaviors: [
+ Gerrit.AccessBehavior,
+ Gerrit.BaseUrlBehavior,
+ Gerrit.URLEncodingBehavior,
+ ],
+
+ _projectChanged(project) {
+ const promises = [];
+ if (!this._sections) {
+ this._sections = [];
+ }
+ promises.push(this.$.restAPI.getProjectAccessRights(project).then(res => {
+ this._inheritsFrom = res.inherits_from;
+ this._local = res.local;
+ return this.toSortedArray(this._local);
+ }));
+
+ promises.push(this.$.restAPI.getCapabilities().then(res => {
+ return res;
+ }));
+
+ promises.push(this.$.restAPI.getProject(project).then(res => {
+ return res.labels;
+ }));
+
+ return Promise.all(promises).then(value => {
+ this._capabilities = value[1];
+ this._labels = value[2];
+
+ // Use splice instead of setting _sections directly so that dom-repeat
+ // renders new sections properly. Otherwise, gr-access-section is not
+ // aware that the section has updated.
+ this.splice(...['_sections', 0, this._sections.length]
+ .concat(value[0]));
+ });
+ },
+
+ _computeParentHref(projectName) {
+ return this.getBaseUrl() +
+ `/admin/projects/${this.encodeURL(projectName, true)},access`;
+ },
+ });
+})();
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/admin/gr-project-access/gr-project-access_test.html b/polygerrit-ui/app/elements/admin/gr-project-access/gr-project-access_test.html
new file mode 100644
index 0000000..ef7412e
--- /dev/null
+++ b/polygerrit-ui/app/elements/admin/gr-project-access/gr-project-access_test.html
@@ -0,0 +1,118 @@
+<!DOCTYPE html>
+<!--
+Copyright (C) 2017 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-project-access</title>
+
+<script src="../../../bower_components/page/page.js"></script>
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../../test/common-test-setup.html"/>
+<link rel="import" href="gr-project-access.html">
+
+<script>void(0);</script>
+
+<test-fixture id="basic">
+ <template>
+ <gr-project-access></gr-project-access>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-project-access tests', () => {
+ let element;
+ let sandbox;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ element = fixture('basic');
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ test('_projectChanged called when project name changes', () => {
+ sandbox.stub(element, '_projectChanged');
+ element.project = 'New Project';
+ assert.isTrue(element._projectChanged.called);
+ });
+
+ test('_projectChanged', done => {
+ const capabilitiesRes = {
+ accessDatabase: {
+ id: 'accessDatabase',
+ name: 'Access Database',
+ },
+ };
+ const accessRes = {
+ local: {
+ GLOBAL_CAPABILITIES: {
+ permissions: {
+ accessDatabase: {
+ rules: {
+ 123: {},
+ },
+ },
+ },
+ },
+ },
+ };
+ const projectRes = {
+ labels: {
+ 'Code-Review': {},
+ },
+ };
+ const accessStub = sandbox.stub(element.$.restAPI,
+ 'getProjectAccessRights') .returns(Promise.resolve(accessRes));
+ const capabilitiesStub = sandbox.stub(element.$.restAPI,
+ 'getCapabilities').returns(Promise.resolve(capabilitiesRes));
+ const projectStub = sandbox.stub(element.$.restAPI, 'getProject').returns(
+ Promise.resolve(projectRes));
+
+ element._projectChanged('New Project').then(() => {
+ assert.isTrue(accessStub.called);
+ assert.isTrue(capabilitiesStub.called);
+ assert.isTrue(projectStub.called);
+ assert.isNotOk(element._inheritsFrom);
+ assert.deepEqual(element._local, accessRes.local);
+ assert.deepEqual(element._sections,
+ element.toSortedArray(accessRes.local));
+ assert.deepEqual(element._labels, projectRes.labels);
+ done();
+ });
+ });
+
+ test('_computeParentHref', () => {
+ const projectName = 'test-project';
+ assert.equal(element._computeParentHref(projectName),
+ '/admin/projects/test-project,access');
+ });
+
+ test('inherit section', () => {
+ sandbox.stub(element, '_computeParentHref');
+ assert.isNotOk(Polymer.dom(element.root).querySelector('#inheritsFrom'));
+ assert.isFalse(element._computeParentHref.called);
+ element._inheritsFrom = {
+ name: 'another-project',
+ };
+ flushAsynchronousOperations();
+ assert.isOk(Polymer.dom(element.root).querySelector('#inheritsFrom'));
+ assert.isTrue(element._computeParentHref.called);
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
index 35537e7..152ef3d 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
@@ -94,11 +94,12 @@
</template>
<template is="dom-repeat" items="[[changeSection.results]]" as="change">
<gr-change-list-item
- selected$="[[_computeItemSelected(index, sectionIndex, selectedIndex)]]"
+ selected$="[[_computeItemSelected(sectionIndex, index, selectedIndex)]]"
assigned$="[[_computeItemAssigned(account, change)]]"
needs-review$="[[_computeItemNeedsReview(account, change, showReviewedState)]]"
change="[[change]]"
visible-change-table-columns="[[visibleChangeTableColumns]]"
+ show-number="[[showNumber]]"
show-star="[[showStar]]"
label-names="[[labelNames]]"></gr-change-list-item>
</template>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index 63b42bb..412d67b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -179,12 +179,24 @@
return `${this.getBaseUrl()}/q/${this.encodeURL(query, true)}`;
},
- _computeItemSelected(index, sectionIndex, selectedIndex) {
+ /**
+ * Maps an index local to a particular section to the absolute index
+ * across all the changes on the page.
+ *
+ * @param sectionIndex {number} index of section
+ * @param localIndex {number} index of row within section
+ * @return {number} absolute index of row in the aggregate dashboard
+ */
+ _computeItemAbsoluteIndex(sectionIndex, localIndex) {
let idx = 0;
for (let i = 0; i < sectionIndex; i++) {
- idx += this.sections[i].length;
+ idx += this.sections[i].results.length;
}
- idx += index;
+ return idx + localIndex;
+ },
+
+ _computeItemSelected(sectionIndex, index, selectedIndex) {
+ const idx = this._computeItemAbsoluteIndex(sectionIndex, index);
return idx == selectedIndex;
},
@@ -199,21 +211,13 @@
return account._account_id === change.assignee._account_id;
},
- _getAggregatesectionsLen(sections) {
- sections = sections || [];
- let len = 0;
- for (const section of this.sections) {
- len += section.length;
- }
- return len;
- },
-
_handleJKey(e) {
if (this.shouldSuppressKeyboardShortcut(e) ||
this.modifierPressed(e)) { return; }
e.preventDefault();
- const len = this._getAggregatesectionsLen(this.sections);
+ // Compute absolute index of item that would come after final item.
+ const len = this._computeItemAbsoluteIndex(this.sections.length, 0);
if (this.selectedIndex === len - 1) { return; }
this.selectedIndex += 1;
},
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
index 582a559..704606c 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
@@ -148,6 +148,11 @@
});
test('keyboard shortcuts', done => {
+ sandbox.stub(element, '_computeLabelNames');
+ element.sections = [
+ {results: new Array(1)},
+ {results: new Array(2)},
+ ];
element.selectedIndex = 0;
element.changes = [
{_number: 0},
@@ -163,7 +168,10 @@
assert.isTrue(elementItems[0].hasAttribute('selected'));
MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
assert.equal(element.selectedIndex, 1);
+ assert.isTrue(elementItems[1].hasAttribute('selected'));
MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
+ assert.equal(element.selectedIndex, 2);
+ assert.isTrue(elementItems[2].hasAttribute('selected'));
const navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
assert.equal(element.selectedIndex, 2);
@@ -484,5 +492,24 @@
'is:open ((reviewer:self -is:ignored) OR assignee:self)'),
'/q/is:open+((reviewer:self+-is:ignored)+OR+assignee:self)');
});
+
+ test('_computeItemAbsoluteIndex', () => {
+ sandbox.stub(element, '_computeLabelNames');
+ element.sections = [
+ {results: new Array(1)},
+ {results: new Array(2)},
+ {results: new Array(3)},
+ ];
+
+ assert.equal(element._computeItemAbsoluteIndex(0, 0), 0);
+ // Out of range but no matter.
+ assert.equal(element._computeItemAbsoluteIndex(0, 1), 1);
+
+ assert.equal(element._computeItemAbsoluteIndex(1, 0), 1);
+ assert.equal(element._computeItemAbsoluteIndex(1, 1), 2);
+ assert.equal(element._computeItemAbsoluteIndex(1, 2), 3);
+ assert.equal(element._computeItemAbsoluteIndex(2, 0), 3);
+ assert.equal(element._computeItemAbsoluteIndex(3, 0), 6);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index 1e2cc70f..76b57fa 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -14,6 +14,95 @@
(function() {
'use strict';
+ const RoutePattern = {
+ ROOT: '/',
+ DASHBOARD: '/dashboard/(.*)',
+ ADMIN_PLACEHOLDER: '/admin/(.*)',
+ AGREEMENTS: /^\/settings\/(agreements|new-agreement)/,
+ REGISTER: /^\/register(\/.*)?/,
+
+ // Matches /admin/groups/<group>
+ GROUP: /^\/admin\/groups\/([^,]+)$/,
+
+ // Matches /admin/groups/<group>,info (backwords compat with gwtui)
+ // Redirects to /admin/groups/<group>
+ GROUP_INFO: /^\/admin\/groups\/(.+),info$/,
+
+ // Matches /admin/groups/<group>,audit-log
+ GROUP_AUDIT_LOG: /^\/admin\/groups\/(.+),audit-log$/,
+
+ // Matches /admin/groups/<group>,members
+ GROUP_MEMBERS: /^\/admin\/groups\/(.+),members$/,
+
+ // Matches /admin/groups[,<offset>][/].
+ GROUP_LIST_OFFSET: /^\/admin\/groups(,(\d+))?(\/)?$/,
+ GROUP_LIST_FILTER: '/admin/groups/q/filter::filter',
+ GROUP_LIST_FILTER_OFFSET: '/admin/groups/q/filter::filter,:offset',
+
+ // Matches /admin/projects/<project>
+ PROJECT: /^\/admin\/projects\/([^,]+)$/,
+
+ // Matches /admin/projects/<project>,commands.
+ PROJECT_COMMANDS: /^\/admin\/projects\/(.+),commands$/,
+
+ // Matches /admin/projects/<project>,access.
+ PROJECT_ACCESS: /^\/admin\/projects\/(.+),access$/,
+
+ // Matches /admin/projects[,<offset>][/].
+ PROJECT_LIST_OFFSET: /^\/admin\/projects(,(\d+))?(\/)?$/,
+ PROJECT_LIST_FILTER: '/admin/projects/q/filter::filter',
+ PROJECT_LIST_FILTER_OFFSET: '/admin/projects/q/filter::filter,:offset',
+
+ // Matches /admin/projects/<project>,branches[,<offset>].
+ BRANCH_LIST_OFFSET: /^\/admin\/projects\/(.+),branches(,(.+))?$/,
+ BRANCH_LIST_FILTER: '/admin/projects/:project,branches/q/filter::filter',
+ BRANCH_LIST_FILTER_OFFSET:
+ '/admin/projects/:project,branches/q/filter::filter,:offset',
+
+ // Matches /admin/projects/<project>,tags[,<offset>].
+ TAG_LIST_OFFSET: /^\/admin\/projects\/(.+),tags(,(.+))?$/,
+ TAG_LIST_FILTER: '/admin/projects/:project,tags/q/filter::filter',
+ TAG_LIST_FILTER_OFFSET:
+ '/admin/projects/:project,tags/q/filter::filter,:offset',
+
+ PLUGIN_LIST: /^\/admin\/plugins(\/)?$/,
+
+ // Matches /admin/plugins[,<offset>][/].
+ PLUGIN_LIST_OFFSET: /^\/admin\/plugins(,(\d+))?(\/)?$/,
+ PLUGIN_LIST_FILTER: '/admin/plugins/q/filter::filter',
+ PLUGIN_LIST_FILTER_OFFSET: '/admin/plugins/q/filter::filter,:offset',
+
+ QUERY: '/q/:query',
+ QUERY_OFFSET: '/q/:query,:offset',
+
+ // Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>][/].
+ CHNAGE_LEGACY: /^\/c\/(\d+)\/?(((\d+|edit)(\.\.(\d+|edit))?))?\/?$/,
+ CHANGE_NUMBER_LEGACY: /^\/(\d+)\/?/,
+
+ // Matches
+ // /c/<project>/+/<changeNum>/
+ // [<basePatchNum|edit>..][<patchNum|edit>]/[path].
+ // TODO(kaspern): Migrate completely to project based URLs, with backwards
+ // compatibility for change-only.
+ // eslint-disable-next-line max-len
+ CHANGE_OR_DIFF: /^\/c\/(.+)\/\+\/(\d+)(\/?((\d+|edit)(\.\.(\d+|edit))?(\/(.+))?))?\/?$/,
+
+ // Matches /c/<changeNum>/[<basePatchNum>..]<patchNum>/<path>.
+ DIFF_LEGACY: /^\/c\/(\d+)\/((\d+|edit)(\.\.(\d+|edit))?)\/(.+)/,
+
+ SETTINGS: /^\/settings\/?/,
+ SETTINGS_LEGACY: /^\/settings\/VE\/(\S+)/,
+ };
+
+ /**
+ * Pattern to recognize and parse the diff line locations as they appear in
+ * the hash of diff URLs. In this format, a number on its own indicates that
+ * line number in the revision of the diff. A number prefixed by either an 'a'
+ * or a 'b' indicates that line number of the base of the diff.
+ * @type {RegExp}
+ */
+ const LINE_ADDRESS_PATTERN = /^([ab]?)(\d+)$/;
+
// Polymer makes `app` intrinsically defined on the window by virtue of the
// custom element having the id "app", but it is made explicit here.
const app = document.querySelector('#app');
@@ -48,8 +137,9 @@
},
behaviors: [
- Gerrit.URLEncodingBehavior,
+ Gerrit.BaseUrlBehavior,
Gerrit.PatchSetBehavior,
+ Gerrit.URLEncodingBehavior,
],
start() {
@@ -66,7 +156,7 @@
},
_generateUrl(params) {
- const base = window.Gerrit.BaseUrlBehavior.getBaseUrl();
+ const base = this.getBaseUrl();
let url = '';
if (params.view === Gerrit.Nav.View.SEARCH) {
@@ -133,6 +223,13 @@
return base + url;
},
+ /**
+ * Given an object of parameters, potentially including a `patchNum` or a
+ * `basePatchNum` or both, return a string representation of that range. If
+ * no range is indicated in the params, the empty string is returned.
+ * @param {!Object} params
+ * @return {string}
+ */
_getPatchRangeExpression(params) {
let range = '';
if (params.patchNum) { range = '' + params.patchNum; }
@@ -193,9 +290,15 @@
return needsRedirect;
},
- _redirectToLogin(data) {
- const basePath = window.Gerrit.BaseUrlBehavior.getBaseUrl() || '';
- page('/login/' + encodeURIComponent(data.substring(basePath.length)));
+ /**
+ * Redirect the user to login using the given return-URL for redirection
+ * after authentication success.
+ * @param {string} returnUrl
+ */
+ _redirectToLogin(returnUrl) {
+ const basePath = this.getBaseUrl() || '';
+ page(
+ '/login/' + encodeURIComponent(returnUrl.substring(basePath.length)));
},
/**
@@ -209,8 +312,67 @@
return canonicalPath.split('#').slice(1).join('#');
},
+ _parseLineAddress(hash) {
+ const match = hash.match(LINE_ADDRESS_PATTERN);
+ if (!match) { return null; }
+ return {
+ leftSide: !!match[1],
+ lineNum: parseInt(match[2], 10),
+ };
+ },
+
+ /**
+ * Check to see if the user is logged in and return a promise that only
+ * resolves if the user is logged in. If the user us not logged in, the
+ * promise is rejected and the page is redirected to the login flow.
+ * @param {!Object} data The parsed route data.
+ * @return {!Promise<!Object>} A promise yielding the original route data
+ * (if it resolves).
+ */
+ _redirectIfNotLoggedIn(data) {
+ return this._restAPI.getLoggedIn().then(loggedIn => {
+ if (loggedIn) {
+ return Promise.resolve();
+ } else {
+ this._redirectToLogin(data.canonicalPath);
+ return Promise.reject();
+ }
+ });
+ },
+
+ /** Page.js middleware that warms the REST API's logged-in cache line. */
+ _loadUserMiddleware(ctx, next) {
+ this._restAPI.getLoggedIn().then(() => { next(); });
+ },
+
+ /**
+ * Map a route to a method on the router.
+ *
+ * @param {!string|!RegExp} pattern The page.js pattern for the route.
+ * @param {!string} handlerName The method name for the handler. If the
+ * route is matched, the handler will be executed with `this` referring
+ * to the component. Its return value will be discarded so that it does
+ * not interfere with page.js.
+ * @param {?boolean=} opt_authRedirect If true, then auth is checked before
+ * executing the handler. If the user is not logged in, it will redirect
+ * to the login flow and the handler will not be executed. The login
+ * redirect specifies the matched URL to be used after successfull auth.
+ */
+ _mapRoute(pattern, handlerName, opt_authRedirect) {
+ if (!this[handlerName]) {
+ console.error('Attempted to map route to unknown method: ',
+ handlerName);
+ return;
+ }
+ page(pattern, this._loadUserMiddleware.bind(this), data => {
+ const promise = opt_authRedirect ?
+ this._redirectIfNotLoggedIn(data) : Promise.resolve();
+ promise.then(() => { this[handlerName](data); });
+ });
+ },
+
_startRouter() {
- const base = window.Gerrit.BaseUrlBehavior.getBaseUrl();
+ const base = this.getBaseUrl();
if (base) {
page.base(base);
}
@@ -227,7 +389,7 @@
// Fire asynchronously so that the URL is changed by the time the event
// is processed.
this.async(() => {
- app.fire('location-change', {
+ this.fire('location-change', {
hash: window.location.hash,
pathname: window.location.pathname,
});
@@ -236,476 +398,475 @@
next();
});
- const loadUser = (ctx, next) => {
- this._restAPI.getLoggedIn().then(() => { next(); });
- };
+ this._mapRoute(RoutePattern.ROOT, '_handleRootRoute');
- // Routes.
- page('/', loadUser, data => {
- if (data.querystring.match(/^closeAfterLogin/)) {
- // Close child window on redirect after login.
- window.close();
- }
- let hash = this._getHashFromCanonicalPath(data.canonicalPath);
- // For backward compatibility with GWT links.
- if (hash) {
- // In certain login flows the server may redirect to a hash without
- // a leading slash, which page.js doesn't handle correctly.
- if (hash[0] !== '/') {
- hash = '/' + hash;
- }
- if (hash.includes('/ /') && data.canonicalPath.includes('/+/')) {
- // Path decodes all '+' to ' ' -- this breaks project-based URLs.
- // See Issue 6888.
- hash = hash.replace('/ /', '/+/');
- }
- let newUrl = base + hash;
- if (hash.startsWith('/VE/')) {
- newUrl = base + '/settings' + hash;
- }
- this._redirect(newUrl);
- return;
- }
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._redirect('/dashboard/self');
- } else {
- this._redirect('/q/status:open');
- }
- });
- });
+ this._mapRoute(RoutePattern.DASHBOARD, '_handleDashboardRoute');
- page('/dashboard/(.*)', loadUser, data => {
- if (!data.params[0]) {
- page.redirect('/dashboard/self');
- return;
- }
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (!loggedIn) {
- if (data.params[0].toLowerCase() === 'self') {
- this._redirectToLogin(data.canonicalPath);
- } else {
- this._redirect('/q/owner:' + data.params[0]);
- }
- } else {
- this._setParams({
- view: Gerrit.Nav.View.DASHBOARD,
- user: data.params[0],
- });
- }
- });
- });
+ this._mapRoute(RoutePattern.GROUP_INFO, '_handleGroupInfoRoute', true);
- // Matches /admin/groups/<group>,info (backwords compat with gwtui)
- // Redirects to /admin/groups/<group>
- page(/^\/admin\/groups\/(.+),info$/, loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._redirect(
- '/admin/groups/' + encodeURIComponent(data.params[0]));
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.GROUP_AUDIT_LOG, '_handleGroupAuditLogRoute',
+ true);
- // Matches /admin/groups/<group>,audit-log
- page(/^\/admin\/groups\/(.+),audit-log$/, loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-group-audit-log',
- detailType: 'audit-log',
- groupId: data.params[0],
- });
- } else {
- this._redirect('/login/' + encodeURIComponent(data.canonicalPath));
- }
- });
- });
+ this._mapRoute(RoutePattern.GROUP_MEMBERS, '_handleGroupMembersRoute');
- // Matches /admin/groups/<group>,members
- page(/^\/admin\/groups\/(.+),members$/, loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-group-members',
- detailType: 'members',
- groupId: data.params[0],
- });
- });
+ this._mapRoute(RoutePattern.GROUP_LIST_OFFSET,
+ '_handleGroupListOffsetRoute', true);
- // Matches /admin/groups[,<offset>][/].
- page(/^\/admin\/groups(,(\d+))?(\/)?$/, loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-admin-group-list',
- offset: data.params[1] || 0,
- filter: null,
- });
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.GROUP_LIST_FILTER_OFFSET,
+ '_handleGroupListFilterOffsetRoute', true);
- page('/admin/groups/q/filter::filter,:offset', loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-admin-group-list',
- offset: data.params.offset,
- filter: data.params.filter,
- });
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.GROUP_LIST_FILTER,
+ '_handleGroupListFilterRoute', true);
- page('/admin/groups/q/filter::filter', loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-admin-group-list',
- filter: data.params.filter || null,
- });
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.GROUP, '_handleGroupRoute', true);
- // Matches /admin/groups/<group>
- page(/^\/admin\/groups\/([^,]+)$/, loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-group',
- groupId: data.params[0],
- });
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.PROJECT_COMMANDS,
+ '_handleProjectCommandsRoute', true);
- // Matches /admin/projects/<project>,commands.
- page(/^\/admin\/projects\/(.+),commands$/, loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-project-commands',
- detailType: 'commands',
- project: data.params[0],
- });
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.PROJECT_ACCESS,
+ '_handleProjectAccessRoute', true);
- // Matches /admin/projects/<project>,branches[,<offset>].
- page(/^\/admin\/projects\/(.+),branches(,(.+))?$/, loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-project-detail-list',
- detailType: 'branches',
- project: data.params[0],
- offset: data.params[2] || 0,
- filter: null,
- });
- });
+ this._mapRoute(RoutePattern.BRANCH_LIST_OFFSET,
+ '_handleBranchListOffsetRoute');
- page('/admin/projects/:project,branches/q/filter::filter,:offset',
- loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-project-detail-list',
- detailType: 'branches',
- project: data.params.project,
- offset: data.params.offset,
- filter: data.params.filter,
- });
- });
+ this._mapRoute(RoutePattern.BRANCH_LIST_FILTER_OFFSET,
+ '_handleBranchListFilterOffsetRoute');
- page('/admin/projects/:project,branches/q/filter::filter',
- loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-project-detail-list',
- detailType: 'branches',
- project: data.params.project,
- filter: data.params.filter || null,
- });
- });
+ this._mapRoute(RoutePattern.BRANCH_LIST_FILTER,
+ '_handleBranchListFilterRoute');
- // Matches /admin/projects/<project>,tags[,<offset>].
- page(/^\/admin\/projects\/(.+),tags(,(.+))?$/, loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-project-detail-list',
- detailType: 'tags',
- project: data.params[0],
- offset: data.params[2] || 0,
- filter: null,
- });
- });
+ this._mapRoute(RoutePattern.TAG_LIST_OFFSET,
+ '_handleTagListOffsetRoute');
- page('/admin/projects/:project,tags/q/filter::filter,:offset',
- loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-project-detail-list',
- detailType: 'tags',
- project: data.params.project,
- offset: data.params.offset,
- filter: data.params.filter,
- });
- });
+ this._mapRoute(RoutePattern.TAG_LIST_FILTER_OFFSET,
+ '_handleTagListFilterOffsetRoute');
- page('/admin/projects/:project,tags/q/filter::filter',
- loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-project-detail-list',
- detailType: 'tags',
- project: data.params.project,
- filter: data.params.filter || null,
- });
- });
+ this._mapRoute(RoutePattern.TAG_LIST_FILTER,
+ '_handleTagListFilterRoute');
- // Matches /admin/projects[,<offset>][/].
- page(/^\/admin\/projects(,(\d+))?(\/)?$/, loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-admin-project-list',
- offset: data.params[1] || 0,
- filter: null,
- });
- });
+ this._mapRoute(RoutePattern.PROJECT_LIST_OFFSET,
+ '_handleProjectListOffsetRoute');
- page('/admin/projects/q/filter::filter,:offset', loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-admin-project-list',
- offset: data.params.offset,
- filter: data.params.filter,
- });
- });
+ this._mapRoute(RoutePattern.PROJECT_LIST_FILTER_OFFSET,
+ '_handleProjectListFilterOffsetRoute');
- page('/admin/projects/q/filter::filter', loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-admin-project-list',
- filter: data.params.filter || null,
- });
- });
+ this._mapRoute(RoutePattern.PROJECT_LIST_FILTER,
+ '_handleProjectListFilterRoute');
- // Matches /admin/projects/<project>
- page(/^\/admin\/projects\/([^,]+)$/, loadUser, data => {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- project: data.params[0],
- adminView: 'gr-project',
- });
- });
+ this._mapRoute(RoutePattern.PROJECT, '_handleProjectRoute');
- // Matches /admin/plugins[,<offset>][/].
- page(/^\/admin\/plugins(,(\d+))?(\/)?$/, loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-plugin-list',
- offset: data.params[1] || 0,
- filter: null,
- });
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.PLUGIN_LIST_OFFSET,
+ '_handlePluginListOffsetRoute', true);
- page('/admin/plugins/q/filter::filter,:offset', loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-plugin-list',
- offset: data.params.offset,
- filter: data.params.filter,
- });
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.PLUGIN_LIST_FILTER_OFFSET,
+ '_handlePluginListFilterOffsetRoute', true);
- page('/admin/plugins/q/filter::filter', loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-plugin-list',
- filter: data.params.filter || null,
- });
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.PLUGIN_LIST_FILTER,
+ '_handlePluginListFilterRoute', true);
- page(/^\/admin\/plugins(\/)?$/, loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.ADMIN,
- adminView: 'gr-plugin-list',
- });
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.PLUGIN_LIST, '_handlePluginListRoute', true);
- page('/admin/(.*)', loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- data.params.view = Gerrit.Nav.View.ADMIN;
- data.params.placeholder = true;
- this._setParams(data.params);
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
+ this._mapRoute(RoutePattern.ADMIN_PLACEHOLDER,
+ '_handleAdminPlaceholderRoute', true);
- const queryHandler = data => {
- data.params.view = Gerrit.Nav.View.SEARCH;
- this._setParams(data.params);
- };
+ this._mapRoute(RoutePattern.QUERY_OFFSET, '_handleQueryRoute');
+ this._mapRoute(RoutePattern.QUERY, '_handleQueryRoute');
- page('/q/:query,:offset', queryHandler);
- page('/q/:query', queryHandler);
+ this._mapRoute(RoutePattern.CHANGE_NUMBER_LEGACY,
+ '_handleChangeNumberLegacyRoute');
- page(/^\/(\d+)\/?/, ctx => {
- this._redirect('/c/' + encodeURIComponent(ctx.params[0]));
- });
+ this._mapRoute(RoutePattern.CHANGE_OR_DIFF, '_handleChangeOrDiffRoute');
- // Matches
- // /c/<project>/+/<changeNum>/
- // [<basePatchNum|edit>..][<patchNum|edit>]/[path].
- // TODO(kaspern): Migrate completely to project based URLs, with backwards
- // compatibility for change-only.
- // eslint-disable-next-line max-len
- page(/^\/c\/(.+)\/\+\/(\d+)(\/?((\d+|edit)(\.\.(\d+|edit))?(\/(.+))?))?\/?$/,
- ctx => {
- // Parameter order is based on the regex group number matched.
- const params = {
- project: ctx.params[0],
- changeNum: ctx.params[1],
- basePatchNum: ctx.params[4],
- patchNum: ctx.params[6],
- path: ctx.params[8],
- view: ctx.params[8] ?
- Gerrit.Nav.View.DIFF : Gerrit.Nav.View.CHANGE,
- hash: ctx.hash,
- };
- const needsRedirect = this._normalizePatchRangeParams(params);
- if (needsRedirect) {
- this._redirect(this._generateUrl(params));
- } else {
- this._setParams(params);
- this._restAPI.setInProjectLookup(params.changeNum,
- params.project);
- }
- });
+ this._mapRoute(RoutePattern.CHNAGE_LEGACY, '_handleChnageLegacyRoute');
- // Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>][/].
- page(/^\/c\/(\d+)\/?(((\d+|edit)(\.\.(\d+|edit))?))?\/?$/, ctx => {
- // Parameter order is based on the regex group number matched.
- const params = {
- changeNum: ctx.params[0],
- basePatchNum: ctx.params[3],
- patchNum: ctx.params[5],
- view: Gerrit.Nav.View.CHANGE,
- };
+ this._mapRoute(RoutePattern.DIFF_LEGACY, '_handleDiffLegacyRoute');
- this._normalizeLegacyRouteParams(params);
- });
+ this._mapRoute(RoutePattern.AGREEMENTS, '_handleAgreementsRoute', true);
- // Matches /c/<changeNum>/[<basePatchNum>..]<patchNum>/<path>.
- page(/^\/c\/(\d+)\/((\d+|edit)(\.\.(\d+|edit))?)\/(.+)/, ctx => {
- // Check if path has an '@' which indicates it was using GWT style line
- // numbers. Even if the filename had an '@' in it, it would have already
- // been URI encoded. Redirect to hash version of path.
- if (ctx.path.includes('@')) {
- this._redirect(ctx.path.replace('@', '#'));
- return;
- }
+ this._mapRoute(RoutePattern.SETTINGS_LEGACY,
+ '_handleSettingsLegacyRoute', true);
- // Parameter order is based on the regex group number matched.
- const params = {
- changeNum: ctx.params[0],
- basePatchNum: ctx.params[2],
- patchNum: ctx.params[4],
- path: ctx.params[5],
- hash: ctx.hash,
- view: Gerrit.Nav.View.DIFF,
- };
+ this._mapRoute(RoutePattern.SETTINGS, '_handleSettingsRoute', true);
- this._normalizeLegacyRouteParams(params);
- });
-
- page(/^\/settings\/(agreements|new-agreement)/, loadUser, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- data.params.view = Gerrit.Nav.View.AGREEMENTS;
- this._setParams(data.params);
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
-
- page(/^\/settings\/VE\/(\S+)/, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({
- view: Gerrit.Nav.View.SETTINGS,
- emailToken: data.params[0],
- });
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
-
- page(/^\/settings\/?/, data => {
- this._restAPI.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this._setParams({view: Gerrit.Nav.View.SETTINGS});
- } else {
- this._redirectToLogin(data.canonicalPath);
- }
- });
- });
-
- page(/^\/register(\/.*)?/, ctx => {
- this._setParams({justRegistered: true});
- const path = ctx.params[0] || '/';
- if (path[0] !== '/') { return; }
- page.show(base + path);
- });
+ this._mapRoute(RoutePattern.REGISTER, '_handleRegisterRoute');
page.start();
},
+
+ /**
+ * @param {!Object} data
+ * @return {Promise|null} if handling the route involves asynchrony, then a
+ * promise is returned. Otherwise, synchronous handling returns null.
+ */
+ _handleRootRoute(data) {
+ if (data.querystring.match(/^closeAfterLogin/)) {
+ // Close child window on redirect after login.
+ window.close();
+ return null;
+ }
+ let hash = this._getHashFromCanonicalPath(data.canonicalPath);
+ // For backward compatibility with GWT links.
+ if (hash) {
+ // In certain login flows the server may redirect to a hash without
+ // a leading slash, which page.js doesn't handle correctly.
+ if (hash[0] !== '/') {
+ hash = '/' + hash;
+ }
+ if (hash.includes('/ /') && data.canonicalPath.includes('/+/')) {
+ // Path decodes all '+' to ' ' -- this breaks project-based URLs.
+ // See Issue 6888.
+ hash = hash.replace('/ /', '/+/');
+ }
+ const base = this.getBaseUrl();
+ let newUrl = base + hash;
+ if (hash.startsWith('/VE/')) {
+ newUrl = base + '/settings' + hash;
+ }
+ this._redirect(newUrl);
+ return null;
+ }
+ return this._restAPI.getLoggedIn().then(loggedIn => {
+ if (loggedIn) {
+ this._redirect('/dashboard/self');
+ } else {
+ this._redirect('/q/status:open');
+ }
+ });
+ },
+
+ _handleDashboardRoute(data) {
+ if (!data.params[0]) {
+ this._redirect('/dashboard/self');
+ return;
+ }
+
+ return this._restAPI.getLoggedIn().then(loggedIn => {
+ if (!loggedIn) {
+ if (data.params[0].toLowerCase() === 'self') {
+ this._redirectToLogin(data.canonicalPath);
+ } else {
+ // TODO: encode user or use _generateUrl.
+ this._redirect('/q/owner:' + data.params[0]);
+ }
+ } else {
+ this._setParams({
+ view: Gerrit.Nav.View.DASHBOARD,
+ user: data.params[0],
+ });
+ }
+ });
+ },
+
+ _handleGroupInfoRoute(data) {
+ this._redirect('/admin/groups/' + encodeURIComponent(data.params[0]));
+ },
+
+ _handleGroupAuditLogRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-group-audit-log',
+ detailType: 'audit-log',
+ groupId: data.params[0],
+ });
+ },
+
+ _handleGroupMembersRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-group-members',
+ detailType: 'members',
+ groupId: data.params[0],
+ });
+ },
+
+ _handleGroupListOffsetRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-group-list',
+ offset: data.params[1] || 0,
+ filter: null,
+ });
+ },
+
+ _handleGroupListFilterOffsetRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-group-list',
+ offset: data.params.offset,
+ filter: data.params.filter,
+ });
+ },
+
+ _handleGroupListFilterRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-group-list',
+ filter: data.params.filter || null,
+ });
+ },
+
+ _handleGroupRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-group',
+ groupId: data.params[0],
+ });
+ },
+
+ _handleProjectCommandsRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-commands',
+ detailType: 'commands',
+ project: data.params[0],
+ });
+ },
+
+ _handleProjectAccessRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-access',
+ detailType: 'access',
+ project: data.params[0],
+ });
+ },
+
+ _handleBranchListOffsetRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'branches',
+ project: data.params[0],
+ offset: data.params[2] || 0,
+ filter: null,
+ });
+ },
+
+ _handleBranchListFilterOffsetRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'branches',
+ project: data.params.project,
+ offset: data.params.offset,
+ filter: data.params.filter,
+ });
+ },
+
+ _handleBranchListFilterRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'branches',
+ project: data.params.project,
+ filter: data.params.filter || null,
+ });
+ },
+
+ _handleTagListOffsetRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'tags',
+ project: data.params[0],
+ offset: data.params[2] || 0,
+ filter: null,
+ });
+ },
+
+ _handleTagListFilterOffsetRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'tags',
+ project: data.params.project,
+ offset: data.params.offset,
+ filter: data.params.filter,
+ });
+ },
+
+ _handleTagListFilterRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'tags',
+ project: data.params.project,
+ filter: data.params.filter || null,
+ });
+ },
+
+ _handleProjectListOffsetRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-project-list',
+ offset: data.params[1] || 0,
+ filter: null,
+ });
+ },
+
+ _handleProjectListFilterOffsetRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-project-list',
+ offset: data.params.offset,
+ filter: data.params.filter,
+ });
+ },
+
+ _handleProjectListFilterRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-project-list',
+ filter: data.params.filter || null,
+ });
+ },
+
+ _handleProjectRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ project: data.params[0],
+ adminView: 'gr-project',
+ });
+ },
+
+ _handlePluginListOffsetRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-plugin-list',
+ offset: data.params[1] || 0,
+ filter: null,
+ });
+ },
+
+ _handlePluginListFilterOffsetRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-plugin-list',
+ offset: data.params.offset,
+ filter: data.params.filter,
+ });
+ },
+
+ _handlePluginListFilterRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-plugin-list',
+ filter: data.params.filter || null,
+ });
+ },
+
+ _handlePluginListRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-plugin-list',
+ });
+ },
+
+ _handleAdminPlaceholderRoute(data) {
+ data.params.view = Gerrit.Nav.View.ADMIN;
+ data.params.placeholder = true;
+ this._setParams(data.params);
+ },
+
+ _handleQueryRoute(data) {
+ data.params.view = Gerrit.Nav.View.SEARCH;
+ this._setParams(data.params);
+ },
+
+ _handleChangeNumberLegacyRoute(ctx) {
+ this._redirect('/c/' + encodeURIComponent(ctx.params[0]));
+ },
+
+ _handleChangeOrDiffRoute(ctx) {
+ const isDiffView = ctx.params[8];
+
+ // Parameter order is based on the regex group number matched.
+ const params = {
+ project: ctx.params[0],
+ changeNum: ctx.params[1],
+ basePatchNum: ctx.params[4],
+ patchNum: ctx.params[6],
+ path: ctx.params[8],
+ view: isDiffView ? Gerrit.Nav.View.DIFF : Gerrit.Nav.View.CHANGE,
+ };
+
+ if (isDiffView) {
+ const address = this._parseLineAddress(ctx.hash);
+ if (address) {
+ params.leftSide = address.leftSide;
+ params.lineNum = address.lineNum;
+ }
+ }
+
+ const needsRedirect = this._normalizePatchRangeParams(params);
+ if (needsRedirect) {
+ this._redirect(this._generateUrl(params));
+ } else {
+ this._setParams(params);
+ this._restAPI.setInProjectLookup(params.changeNum, params.project);
+ }
+ },
+
+ _handleChnageLegacyRoute(ctx) {
+ // Parameter order is based on the regex group number matched.
+ const params = {
+ changeNum: ctx.params[0],
+ basePatchNum: ctx.params[3],
+ patchNum: ctx.params[5],
+ view: Gerrit.Nav.View.CHANGE,
+ };
+
+ this._normalizeLegacyRouteParams(params);
+ },
+
+ _handleDiffLegacyRoute(ctx) {
+ // Check if path has an '@' which indicates it was using GWT style line
+ // numbers. Even if the filename had an '@' in it, it would have already
+ // been URI encoded. Redirect to hash version of path.
+ if (ctx.path.includes('@')) {
+ this._redirect(ctx.path.replace('@', '#'));
+ return;
+ }
+
+ // Parameter order is based on the regex group number matched.
+ const params = {
+ changeNum: ctx.params[0],
+ basePatchNum: ctx.params[2],
+ patchNum: ctx.params[4],
+ path: ctx.params[5],
+ view: Gerrit.Nav.View.DIFF,
+ };
+
+ const address = this._parseLineAddress(ctx.hash);
+ if (address) {
+ params.leftSide = address.leftSide;
+ params.lineNum = address.lineNum;
+ }
+
+ this._normalizeLegacyRouteParams(params);
+ },
+
+ _handleAgreementsRoute(data) {
+ data.params.view = Gerrit.Nav.View.AGREEMENTS;
+ this._setParams(data.params);
+ },
+
+ _handleSettingsLegacyRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.SETTINGS,
+ emailToken: data.params[0],
+ });
+ },
+
+ _handleSettingsRoute(data) {
+ this._setParams({view: Gerrit.Nav.View.SETTINGS});
+ },
+
+ _handleRegisterRoute(ctx) {
+ this._setParams({justRegistered: true});
+ const path = ctx.params[0] || '/';
+ if (path[0] !== '/') { return; }
+ this._redirect(this.getBaseUrl() + path);
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
index 031cf85..19dfe36 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
@@ -65,6 +65,144 @@
assert.equal(hash, 'foo#bar#baz');
});
+ suite('_parseLineAddress', () => {
+ test('returns null for empty and invalid hashes', () => {
+ let actual = element._parseLineAddress('');
+ assert.isNull(actual);
+
+ actual = element._parseLineAddress('foobar');
+ assert.isNull(actual);
+
+ actual = element._parseLineAddress('foo123');
+ assert.isNull(actual);
+
+ actual = element._parseLineAddress('123bar');
+ assert.isNull(actual);
+ });
+
+ test('parses correctly', () => {
+ let actual = element._parseLineAddress('1234');
+ assert.isOk(actual);
+ assert.equal(actual.lineNum, 1234);
+ assert.isFalse(actual.leftSide);
+
+ actual = element._parseLineAddress('a4');
+ assert.isOk(actual);
+ assert.equal(actual.lineNum, 4);
+ assert.isTrue(actual.leftSide);
+
+ actual = element._parseLineAddress('b77');
+ assert.isOk(actual);
+ assert.equal(actual.lineNum, 77);
+ assert.isTrue(actual.leftSide);
+ });
+ });
+
+ test('_startRouter requires auth for the right handlers', () => {
+ // This test encodes the lists of route handler methods that gr-router
+ // automatically checks for authentication before triggering.
+
+ const requiresAuth = {};
+ const doesNotRequireAuth = {};
+ sandbox.stub(Gerrit.Nav, 'setup');
+ sandbox.stub(window.page, 'start');
+ sandbox.stub(window.page, 'base');
+ sandbox.stub(window, 'page');
+ sandbox.stub(element, '_mapRoute', (pattern, methodName, usesAuth) => {
+ if (usesAuth) {
+ requiresAuth[methodName] = true;
+ } else {
+ doesNotRequireAuth[methodName] = true;
+ }
+ });
+ element._startRouter();
+
+ const actualRequiresAuth = Object.keys(requiresAuth);
+ actualRequiresAuth.sort();
+ const actualDoesNotRequireAuth = Object.keys(doesNotRequireAuth);
+ actualDoesNotRequireAuth.sort();
+
+ const shouldRequireAutoAuth = [
+ '_handleAdminPlaceholderRoute',
+ '_handleAgreementsRoute',
+ '_handleGroupAuditLogRoute',
+ '_handleGroupInfoRoute',
+ '_handleGroupListFilterOffsetRoute',
+ '_handleGroupListFilterRoute',
+ '_handleGroupListOffsetRoute',
+ '_handleGroupRoute',
+ '_handlePluginListFilterOffsetRoute',
+ '_handlePluginListFilterRoute',
+ '_handlePluginListOffsetRoute',
+ '_handlePluginListRoute',
+ '_handleProjectAccessRoute',
+ '_handleProjectCommandsRoute',
+ '_handleSettingsLegacyRoute',
+ '_handleSettingsRoute',
+ ];
+ assert.deepEqual(actualRequiresAuth, shouldRequireAutoAuth);
+
+ const unauthenticatedHandlers = [
+ '_handleBranchListFilterOffsetRoute',
+ '_handleBranchListFilterRoute',
+ '_handleBranchListOffsetRoute',
+ '_handleChangeNumberLegacyRoute',
+ '_handleChangeOrDiffRoute',
+ '_handleChnageLegacyRoute',
+ '_handleDiffLegacyRoute',
+ '_handleGroupMembersRoute',
+ '_handleProjectListFilterOffsetRoute',
+ '_handleProjectListFilterRoute',
+ '_handleProjectListOffsetRoute',
+ '_handleProjectRoute',
+ '_handleQueryRoute',
+ '_handleRegisterRoute',
+ '_handleTagListFilterOffsetRoute',
+ '_handleTagListFilterRoute',
+ '_handleTagListOffsetRoute',
+ ];
+
+ // Handler names that check authentication themselves, and thus don't need
+ // it performed for them.
+ const selfAuthenticatingHandlers = [
+ '_handleDashboardRoute',
+ '_handleRootRoute',
+ ];
+
+ const shouldNotRequireAuth = unauthenticatedHandlers
+ .concat(selfAuthenticatingHandlers);
+ shouldNotRequireAuth.sort();
+
+ assert.deepEqual(actualDoesNotRequireAuth, shouldNotRequireAuth);
+ });
+
+ test('_redirectIfNotLoggedIn while logged in', () => {
+ sandbox.stub(element._restAPI, 'getLoggedIn')
+ .returns(Promise.resolve(true));
+ const data = {canonicalPath: ''};
+ const redirectStub = sandbox.stub(element, '_redirectToLogin');
+ return element._redirectIfNotLoggedIn(data).then(() => {
+ assert.isFalse(redirectStub.called);
+ });
+ });
+
+ test('_redirectIfNotLoggedIn while logged out', () => {
+ sandbox.stub(element._restAPI, 'getLoggedIn')
+ .returns(Promise.resolve(false));
+ const redirectStub = sandbox.stub(element, '_redirectToLogin');
+ const data = {canonicalPath: ''};
+ return new Promise(resolve => {
+ element._redirectIfNotLoggedIn(data)
+ .then(() => {
+ assert.isTrue(false, 'Should never execute');
+ })
+ .catch(() => {
+ assert.isTrue(redirectStub.calledOnce);
+ resolve();
+ });
+ });
+ });
+
suite('generateUrl', () => {
test('search', () => {
let params = {
@@ -234,5 +372,670 @@
});
});
});
+
+ suite('route handlers', () => {
+ let redirectStub;
+ let setParamsStub;
+
+ // Simple route handlers are direct mappings from parsed route data to a
+ // new set of app.params. This test helper asserts that passing `data`
+ // into `methodName` results in setting the params specified in `params`.
+ function assertDataToParams(data, methodName, params) {
+ element[methodName](data);
+ assert.deepEqual(setParamsStub.lastCall.args[0], params);
+ }
+
+ setup(() => {
+ redirectStub = sandbox.stub(element, '_redirect');
+ setParamsStub = sandbox.stub(element, '_setParams');
+ });
+
+ test('_handleAdminPlaceholderRoute', () => {
+ element._handleAdminPlaceholderRoute({params: {}});
+ assert.equal(setParamsStub.lastCall.args[0].view,
+ Gerrit.Nav.View.ADMIN);
+ assert.isTrue(setParamsStub.lastCall.args[0].placeholder);
+ });
+
+ test('_handleAgreementsRoute', () => {
+ element._handleAgreementsRoute({params: {}});
+ assert.isTrue(setParamsStub.calledOnce);
+ assert.equal(setParamsStub.lastCall.args[0].view,
+ Gerrit.Nav.View.AGREEMENTS);
+ });
+
+ test('_handleSettingsLegacyRoute', () => {
+ const data = {params: {0: 'my-token'}};
+ assertDataToParams(data, '_handleSettingsLegacyRoute', {
+ view: Gerrit.Nav.View.SETTINGS,
+ emailToken: 'my-token',
+ });
+ });
+
+ test('_handleSettingsRoute', () => {
+ const data = {};
+ assertDataToParams(data, '_handleSettingsRoute', {
+ view: Gerrit.Nav.View.SETTINGS,
+ });
+ });
+
+ suite('_handleRootRoute', () => {
+ test('closes for closeAfterLogin', () => {
+ const data = {querystring: 'closeAfterLogin', canonicalPath: ''};
+ const closeStub = sandbox.stub(window, 'close');
+ const result = element._handleRootRoute(data);
+ assert.isNotOk(result);
+ assert.isTrue(closeStub.called);
+ assert.isFalse(redirectStub.called);
+ });
+
+ test('redirects to dahsboard if logged in', () => {
+ sandbox.stub(element._restAPI, 'getLoggedIn')
+ .returns(Promise.resolve(true));
+ const data = {
+ canonicalPath: '/', path: '/', querystring: '', hash: '',
+ };
+ const result = element._handleRootRoute(data);
+ assert.isOk(result);
+ return result.then(() => {
+ assert.isTrue(redirectStub.calledWithExactly('/dashboard/self'));
+ });
+ });
+
+ test('redirects to open changes if not logged in', () => {
+ sandbox.stub(element._restAPI, 'getLoggedIn')
+ .returns(Promise.resolve(false));
+ const data = {
+ canonicalPath: '/', path: '/', querystring: '', hash: '',
+ };
+ const result = element._handleRootRoute(data);
+ assert.isOk(result);
+ return result.then(() => {
+ assert.isTrue(redirectStub.calledWithExactly('/q/status:open'));
+ });
+ });
+
+ suite('GWT hash-path URLs', () => {
+ test('redirects hash-path URLs', () => {
+ const data = {
+ canonicalPath: '/#/foo/bar/baz',
+ hash: '/foo/bar/baz',
+ querystring: '',
+ };
+ const result = element._handleRootRoute(data);
+ assert.isNotOk(result);
+ assert.isTrue(redirectStub.called);
+ assert.isTrue(redirectStub.calledWithExactly('/foo/bar/baz'));
+ });
+
+ test('redirects hash-path URLs w/o leading slash', () => {
+ const data = {
+ canonicalPath: '/#foo/bar/baz',
+ querystring: '',
+ hash: 'foo/bar/baz',
+ };
+ const result = element._handleRootRoute(data);
+ assert.isNotOk(result);
+ assert.isTrue(redirectStub.called);
+ assert.isTrue(redirectStub.calledWithExactly('/foo/bar/baz'));
+ });
+
+ test('normalizes "/ /" in hash to "/+/"', () => {
+ const data = {
+ canonicalPath: '/#/foo/bar/+/123/4',
+ querystring: '',
+ hash: '/foo/bar/ /123/4',
+ };
+ const result = element._handleRootRoute(data);
+ assert.isNotOk(result);
+ assert.isTrue(redirectStub.called);
+ assert.isTrue(redirectStub.calledWithExactly('/foo/bar/+/123/4'));
+ });
+
+ test('prepends baseurl to hash-path', () => {
+ const data = {
+ canonicalPath: '/#/foo/bar',
+ querystring: '',
+ hash: '/foo/bar',
+ };
+ sandbox.stub(element, 'getBaseUrl').returns('/baz');
+ const result = element._handleRootRoute(data);
+ assert.isNotOk(result);
+ assert.isTrue(redirectStub.called);
+ assert.isTrue(redirectStub.calledWithExactly('/baz/foo/bar'));
+ });
+
+ test('normalizes /VE/ settings hash-paths', () => {
+ const data = {
+ canonicalPath: '/#/VE/foo/bar',
+ querystring: '',
+ hash: '/VE/foo/bar',
+ };
+ const result = element._handleRootRoute(data);
+ assert.isNotOk(result);
+ assert.isTrue(redirectStub.called);
+ assert.isTrue(redirectStub.calledWithExactly(
+ '/settings/VE/foo/bar'));
+ });
+
+ test('does not drop "inner hashes"', () => {
+ const data = {
+ canonicalPath: '/#/foo/bar#baz',
+ querystring: '',
+ hash: '/foo/bar',
+ };
+ const result = element._handleRootRoute(data);
+ assert.isNotOk(result);
+ assert.isTrue(redirectStub.called);
+ assert.isTrue(redirectStub.calledWithExactly('/foo/bar#baz'));
+ });
+ });
+ });
+
+ suite('_handleDashboardRoute', () => {
+ let reidrectToLoginStub;
+
+ setup(() => {
+ reidrectToLoginStub = sandbox.stub(element, '_redirectToLogin');
+ });
+
+ test('no user specified', () => {
+ const data = {canonicalPath: '/dashboard', params: {}};
+ const result = element._handleDashboardRoute(data);
+ assert.isNotOk(result);
+ assert.isFalse(setParamsStub.called);
+ assert.isFalse(reidrectToLoginStub.called);
+ assert.isTrue(redirectStub.called);
+ assert.equal(redirectStub.lastCall.args[0], '/dashboard/self');
+ });
+
+ test('own dahsboard but signed out redirects to login', () => {
+ sandbox.stub(element._restAPI, 'getLoggedIn')
+ .returns(Promise.resolve(false));
+ const data = {canonicalPath: '/dashboard', params: {0: 'seLF'}};
+ const result = element._handleDashboardRoute(data);
+ assert.isOk(result);
+ return result.then(() => {
+ assert.isTrue(reidrectToLoginStub.calledOnce);
+ assert.isFalse(redirectStub.called);
+ assert.isFalse(setParamsStub.called);
+ });
+ });
+
+ test('non-self dahsboard but signed out does not redirect', () => {
+ sandbox.stub(element._restAPI, 'getLoggedIn')
+ .returns(Promise.resolve(false));
+ const data = {canonicalPath: '/dashboard', params: {0: 'foo'}};
+ const result = element._handleDashboardRoute(data);
+ assert.isOk(result);
+ return result.then(() => {
+ assert.isFalse(reidrectToLoginStub.called);
+ assert.isFalse(setParamsStub.called);
+ assert.isTrue(redirectStub.calledOnce);
+ assert.equal(redirectStub.lastCall.args[0], '/q/owner:foo');
+ });
+ });
+
+ test('dahsboard while signed in sets params', () => {
+ sandbox.stub(element._restAPI, 'getLoggedIn')
+ .returns(Promise.resolve(true));
+ const data = {canonicalPath: '/dashboard', params: {0: 'foo'}};
+ const result = element._handleDashboardRoute(data);
+ assert.isOk(result);
+ return result.then(() => {
+ assert.isFalse(reidrectToLoginStub.called);
+ assert.isFalse(redirectStub.called);
+ assert.isTrue(setParamsStub.calledOnce);
+ assert.deepEqual(setParamsStub.lastCall.args[0], {
+ view: Gerrit.Nav.View.DASHBOARD,
+ user: 'foo',
+ });
+ });
+ });
+ });
+
+ suite('group routes', () => {
+ test('_handleGroupInfoRoute', () => {
+ const data = {params: {0: 1234}};
+ element._handleGroupInfoRoute(data);
+ assert.isTrue(redirectStub.calledOnce);
+ assert.equal(redirectStub.lastCall.args[0], '/admin/groups/1234');
+ });
+
+ test('_handleGroupAuditLogRoute', () => {
+ const data = {params: {0: 1234}};
+ assertDataToParams(data, '_handleGroupAuditLogRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-group-audit-log',
+ detailType: 'audit-log',
+ groupId: 1234,
+ });
+ });
+
+ test('_handleGroupMembersRoute', () => {
+ const data = {params: {0: 1234}};
+ assertDataToParams(data, '_handleGroupMembersRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-group-members',
+ detailType: 'members',
+ groupId: 1234,
+ });
+ });
+
+ test('_handleGroupListOffsetRoute', () => {
+ const data = {params: {}};
+ assertDataToParams(data, '_handleGroupListOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-group-list',
+ offset: 0,
+ filter: null,
+ });
+
+ data.params[1] = 42;
+ assertDataToParams(data, '_handleGroupListOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-group-list',
+ offset: 42,
+ filter: null,
+ });
+ });
+
+ test('_handleGroupListFilterOffsetRoute', () => {
+ const data = {params: {filter: 'foo', offset: 42}};
+ assertDataToParams(data, '_handleGroupListFilterOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-group-list',
+ offset: 42,
+ filter: 'foo',
+ });
+ });
+
+ test('_handleGroupListFilterRoute', () => {
+ const data = {params: {filter: 'foo'}};
+ assertDataToParams(data, '_handleGroupListFilterRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-group-list',
+ filter: 'foo',
+ });
+ });
+
+ test('_handleGroupRoute', () => {
+ const data = {params: {0: 4321}};
+ assertDataToParams(data, '_handleGroupRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-group',
+ groupId: 4321,
+ });
+ });
+ });
+
+ suite('project routes', () => {
+ test('_handleProjectRoute', () => {
+ const data = {params: {0: 4321}};
+ assertDataToParams(data, '_handleProjectRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project',
+ project: 4321,
+ });
+ });
+
+ test('_handleProjectCommandsRoute', () => {
+ const data = {params: {0: 4321}};
+ assertDataToParams(data, '_handleProjectCommandsRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-commands',
+ detailType: 'commands',
+ project: 4321,
+ });
+ });
+
+ test('_handleProjectAccessRoute', () => {
+ const data = {params: {0: 4321}};
+ assertDataToParams(data, '_handleProjectAccessRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-access',
+ detailType: 'access',
+ project: 4321,
+ });
+ });
+
+ suite('branch list routes', () => {
+ test('_handleBranchListOffsetRoute', () => {
+ const data = {params: {0: 4321}};
+ assertDataToParams(data, '_handleBranchListOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'branches',
+ project: 4321,
+ offset: 0,
+ filter: null,
+ });
+
+ data.params[2] = 42;
+ assertDataToParams(data, '_handleBranchListOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'branches',
+ project: 4321,
+ offset: 42,
+ filter: null,
+ });
+ });
+
+ test('_handleBranchListFilterOffsetRoute', () => {
+ const data = {params: {project: 4321, filter: 'foo', offset: 42}};
+ assertDataToParams(data, '_handleBranchListFilterOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'branches',
+ project: 4321,
+ offset: 42,
+ filter: 'foo',
+ });
+ });
+
+ test('_handleBranchListFilterRoute', () => {
+ const data = {params: {project: 4321, filter: 'foo'}};
+ assertDataToParams(data, '_handleBranchListFilterRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'branches',
+ project: 4321,
+ filter: 'foo',
+ });
+ });
+ });
+
+ suite('tag list routes', () => {
+ test('_handleTagListOffsetRoute', () => {
+ const data = {params: {0: 4321}};
+ assertDataToParams(data, '_handleTagListOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'tags',
+ project: 4321,
+ offset: 0,
+ filter: null,
+ });
+ });
+
+ test('_handleTagListFilterOffsetRoute', () => {
+ const data = {params: {project: 4321, filter: 'foo', offset: 42}};
+ assertDataToParams(data, '_handleTagListFilterOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'tags',
+ project: 4321,
+ offset: 42,
+ filter: 'foo',
+ });
+ });
+
+ test('_handleTagListFilterRoute', () => {
+ const data = {params: {project: 4321}};
+ assertDataToParams(data, '_handleTagListFilterRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'tags',
+ project: 4321,
+ filter: null,
+ });
+
+ data.params.filter = 'foo';
+ assertDataToParams(data, '_handleTagListFilterRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-project-detail-list',
+ detailType: 'tags',
+ project: 4321,
+ filter: 'foo',
+ });
+ });
+ });
+
+ suite('project list routes', () => {
+ test('_handleProjectListOffsetRoute', () => {
+ const data = {params: {}};
+ assertDataToParams(data, '_handleProjectListOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-project-list',
+ offset: 0,
+ filter: null,
+ });
+
+ data.params[1] = 42;
+ assertDataToParams(data, '_handleProjectListOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-project-list',
+ offset: 42,
+ filter: null,
+ });
+ });
+
+ test('_handleProjectListFilterOffsetRoute', () => {
+ const data = {params: {filter: 'foo', offset: 42}};
+ assertDataToParams(data, '_handleProjectListFilterOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-project-list',
+ offset: 42,
+ filter: 'foo',
+ });
+ });
+
+ test('_handleProjectListFilterRoute', () => {
+ const data = {params: {}};
+ assertDataToParams(data, '_handleProjectListFilterRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-project-list',
+ filter: null,
+ });
+
+ data.params.filter = 'foo';
+ assertDataToParams(data, '_handleProjectListFilterRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-admin-project-list',
+ filter: 'foo',
+ });
+ });
+ });
+ });
+
+ suite('plugin routes', () => {
+ test('_handlePluginListOffsetRoute', () => {
+ const data = {params: {}};
+ assertDataToParams(data, '_handlePluginListOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-plugin-list',
+ offset: 0,
+ filter: null,
+ });
+
+ data.params[1] = 42;
+ assertDataToParams(data, '_handlePluginListOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-plugin-list',
+ offset: 42,
+ filter: null,
+ });
+ });
+
+ test('_handlePluginListFilterOffsetRoute', () => {
+ const data = {params: {filter: 'foo', offset: 42}};
+ assertDataToParams(data, '_handlePluginListFilterOffsetRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-plugin-list',
+ offset: 42,
+ filter: 'foo',
+ });
+ });
+
+ test('_handlePluginListFilterRoute', () => {
+ const data = {params: {}};
+ assertDataToParams(data, '_handlePluginListFilterRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-plugin-list',
+ filter: null,
+ });
+
+ data.params.filter = 'foo';
+ assertDataToParams(data, '_handlePluginListFilterRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-plugin-list',
+ filter: 'foo',
+ });
+ });
+
+ test('_handlePluginListRoute', () => {
+ const data = {params: {}};
+ assertDataToParams(data, '_handlePluginListRoute', {
+ view: Gerrit.Nav.View.ADMIN,
+ adminView: 'gr-plugin-list',
+ });
+ });
+ });
+
+ suite('change/diff routes', () => {
+ test('_handleChangeNumberLegacyRoute', () => {
+ const data = {params: {0: 12345}};
+ element._handleChangeNumberLegacyRoute(data);
+ assert.isTrue(redirectStub.calledOnce);
+ assert.isTrue(redirectStub.calledWithExactly('/c/12345'));
+ });
+
+ test('_handleChnageLegacyRoute', () => {
+ const normalizeRouteStub = sandbox.stub(element,
+ '_normalizeLegacyRouteParams');
+ const ctx = {
+ params: [
+ 1234, // 0 Change number
+ null, // 1 Unused
+ null, // 2 Unused
+ 6, // 3 Base patch number
+ null, // 4 Unused
+ 9, // 5 Patch number
+ ],
+ };
+ element._handleChnageLegacyRoute(ctx);
+ assert.isTrue(normalizeRouteStub.calledOnce);
+ assert.deepEqual(normalizeRouteStub.lastCall.args[0], {
+ changeNum: 1234,
+ basePatchNum: 6,
+ patchNum: 9,
+ view: Gerrit.Nav.View.CHANGE,
+ });
+ });
+
+ test('_handleDiffLegacyRoute', () => {
+ const normalizeRouteStub = sandbox.stub(element,
+ '_normalizeLegacyRouteParams');
+ const ctx = {
+ params: [
+ 1234, // 0 Change number
+ null, // 1 Unused
+ 3, // 2 Base patch number
+ null, // 3 Unused
+ 8, // 4 Patch number
+ 'foo/bar', // 5 Diff path
+ ],
+ path: '/c/1234/3..8/foo/bar',
+ hash: 'b123',
+ };
+ element._handleDiffLegacyRoute(ctx);
+ assert.isFalse(redirectStub.called);
+ assert.isTrue(normalizeRouteStub.calledOnce);
+ assert.deepEqual(normalizeRouteStub.lastCall.args[0], {
+ changeNum: 1234,
+ basePatchNum: 3,
+ patchNum: 8,
+ view: Gerrit.Nav.View.DIFF,
+ path: 'foo/bar',
+ lineNum: 123,
+ leftSide: true,
+ });
+ });
+
+ test('_handleDiffLegacyRoute w/ @', () => {
+ const normalizeRouteStub = sandbox.stub(element,
+ '_normalizeLegacyRouteParams');
+ const ctx = {path: '/c/1234/3..8/foo/bar@b123'};
+ element._handleDiffLegacyRoute(ctx);
+ assert.isFalse(normalizeRouteStub.called);
+ assert.isTrue(redirectStub.calledOnce);
+ assert.isTrue(redirectStub.calledWithExactly(
+ '/c/1234/3..8/foo/bar#b123'));
+ });
+
+ suite('_handleChangeOrDiffRoute', () => {
+ let normalizeRangeStub;
+
+ function makeParams(path, hash) {
+ return {
+ params: [
+ 'foo/bar', // 0 Project
+ 1234, // 1 Change number
+ null, // 2 Unused
+ null, // 3 Unused
+ 4, // 4 Base patch number
+ null, // 5 Unused
+ 7, // 6 Patch number
+ null, // 7 Unused,
+ path, // 8 Diff path
+ ],
+ hash,
+ };
+ }
+
+ setup(() => {
+ normalizeRangeStub = sandbox.stub(element,
+ '_normalizePatchRangeParams');
+ sandbox.stub(element._restAPI, 'setInProjectLookup');
+ });
+
+ test('needs redirect', () => {
+ normalizeRangeStub.returns(true);
+ sandbox.stub(element, '_generateUrl').returns('foo');
+ const ctx = makeParams(null, '');
+ element._handleChangeOrDiffRoute(ctx);
+ assert.isTrue(normalizeRangeStub.called);
+ assert.isFalse(setParamsStub.called);
+ assert.isTrue(redirectStub.calledOnce);
+ assert.isTrue(redirectStub.calledWithExactly('foo'));
+ });
+
+ test('change view', () => {
+ normalizeRangeStub.returns(false);
+ sandbox.stub(element, '_generateUrl').returns('foo');
+ const ctx = makeParams(null, '');
+ assertDataToParams(ctx, '_handleChangeOrDiffRoute', {
+ view: Gerrit.Nav.View.CHANGE,
+ project: 'foo/bar',
+ changeNum: 1234,
+ basePatchNum: 4,
+ patchNum: 7,
+ path: null,
+ });
+ assert.isFalse(redirectStub.called);
+ assert.isTrue(normalizeRangeStub.called);
+ });
+
+ test('diff view', () => {
+ normalizeRangeStub.returns(false);
+ sandbox.stub(element, '_generateUrl').returns('foo');
+ const ctx = makeParams('foo/bar/baz', 'b44');
+ assertDataToParams(ctx, '_handleChangeOrDiffRoute', {
+ view: Gerrit.Nav.View.DIFF,
+ project: 'foo/bar',
+ changeNum: 1234,
+ basePatchNum: 4,
+ patchNum: 7,
+ path: 'foo/bar/baz',
+ leftSide: true,
+ lineNum: 44,
+ });
+ assert.isFalse(redirectStub.called);
+ assert.isTrue(normalizeRangeStub.called);
+ });
+ });
+ });
+ });
});
</script>
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 409fd09..56be27d 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
@@ -26,8 +26,6 @@
RIGHT: 'right',
};
- const HASH_PATTERN = /^[ab]?\d+$/;
-
Polymer({
is: 'gr-diff-view',
@@ -467,7 +465,7 @@
_paramsChanged(value) {
if (value.view !== Gerrit.Nav.View.DIFF) { return; }
- this._loadHash(this.params.hash);
+ this._initCursor(this.params);
this._changeNum = value.changeNum;
this._patchRange = {
@@ -539,18 +537,16 @@
},
/**
- * If the URL hash is a diff address then configure the diff cursor.
+ * If the params specify a diff address then configure the diff cursor.
*/
- _loadHash(hash) {
- hash = (hash || '').replace(/^#/, '');
- if (!HASH_PATTERN.test(hash)) { return; }
- if (hash[0] === 'a' || hash[0] === 'b') {
+ _initCursor(params) {
+ if (params.lineNum === undefined) { return; }
+ if (params.leftSide) {
this.$.cursor.side = DiffSides.LEFT;
- hash = hash.substring(1);
} else {
this.$.cursor.side = DiffSides.RIGHT;
}
- this.$.cursor.initialLineNumber = parseInt(hash, 10);
+ this.$.cursor.initialLineNumber = params.lineNum;
},
_pathChanged(path) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index 250e0f5..e239bf0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -509,7 +509,7 @@
getDiffComments() { return Promise.resolve({}); },
});
sandbox.stub(element.$.diff, 'reload');
- sandbox.stub(element, '_loadHash');
+ sandbox.stub(element, '_initCursor');
element._loggedIn = true;
element.params = {
@@ -522,7 +522,7 @@
};
flush(() => {
- assert.isTrue(element._loadHash.lastCall.calledWithExactly(10));
+ assert.isTrue(element._initCursor.calledOnce);
done();
});
});
@@ -573,31 +573,31 @@
assert.equal(select.nativeSelect.value, 'SIDE_BY_SIDE');
});
- test('_loadHash', () => {
+ test('_initCursor', () => {
assert.isNotOk(element.$.cursor.initialLineNumber);
- // Ignores invalid hashes:
- element._loadHash('not valid');
+ // Does nothing when params specify no cursor address:
+ element._initCursor({});
assert.isNotOk(element.$.cursor.initialLineNumber);
- // Ignores null hash:
- element._loadHash(null);
+ // Does nothing when params specify side but no number:
+ element._initCursor({leftSide: true});
assert.isNotOk(element.$.cursor.initialLineNumber);
- // Revision hash:
- element._loadHash('234');
+ // Revision hash: specifies lineNum but not side.
+ element._initCursor({lineNum: 234});
assert.equal(element.$.cursor.initialLineNumber, 234);
assert.equal(element.$.cursor.side, 'right');
- // Base hash:
- element._loadHash('b345');
+ // Base hash: specifies lineNum and side.
+ element._initCursor({leftSide: true, lineNum: 345});
assert.equal(element.$.cursor.initialLineNumber, 345);
assert.equal(element.$.cursor.side, 'left');
- // GWT-style base hash:
- element._loadHash('a123');
+ // Specifies right side:
+ element._initCursor({leftSide: false, lineNum: 123});
assert.equal(element.$.cursor.initialLineNumber, 123);
- assert.equal(element.$.cursor.side, 'left');
+ assert.equal(element.$.cursor.side, 'right');
});
test('_shortenPath with long path should add ellipsis', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 0f30171..d20d7bf 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -313,7 +313,7 @@
const isOnParent =
this._getIsParentCommentByLineAndContent(lineEl, contentEl);
const threadEl = this._getOrCreateThreadAtLineRange(contentEl, patchNum,
- side, isOnParent);
+ side, isOnParent, opt_range);
threadEl.addOrEditDraft(opt_lineNum, opt_range);
},
@@ -326,6 +326,20 @@
},
/**
+ * @param {string} commentSide
+ * @param {!Object=} opt_range
+ */
+ _getRangeString(commentSide, opt_range) {
+ return opt_range ?
+ 'range-' +
+ opt_range.startLine + '-' +
+ opt_range.startChar + '-' +
+ opt_range.endLine + '-' +
+ opt_range.endChar + '-' +
+ commentSide : 'line-' + commentSide;
+ },
+
+ /**
* @param {!Object} contentEl
* @param {number} patchNum
* @param {string} commentSide
@@ -334,13 +348,7 @@
*/
_getOrCreateThreadAtLineRange(contentEl, patchNum, commentSide,
isOnParent, opt_range) {
- const rangeToCheck = opt_range ?
- 'range-' +
- opt_range.startLine + '-' +
- opt_range.startChar + '-' +
- opt_range.endLine + '-' +
- opt_range.endChar + '-' +
- commentSide : 'line-' + commentSide;
+ const rangeToCheck = this._getRangeString(commentSide, opt_range);
// Check if thread group exists.
let threadGroupEl = this._getThreadGroupForLine(contentEl);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index a769a96..f540c34 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -224,6 +224,20 @@
});
});
+ test('_getRangeString', () => {
+ const side = 'PARENT';
+ const range = {
+ startLine: 1,
+ startChar: 1,
+ endLine: 1,
+ endChar: 2,
+ };
+ assert.equal(element._getRangeString(side, range),
+ 'range-1-1-1-2-PARENT');
+ assert.equal(element._getRangeString(side, null),
+ 'line-PARENT');
+ }),
+
test('thread groups', () => {
const contentEl = document.createElement('div');
const commentSide = 'left';
diff --git a/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.js b/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.js
index a90158f..7a120c0 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.js
@@ -32,7 +32,8 @@
_computeOwnerLink(account) {
if (!account) { return; }
return Gerrit.Nav.getUrlForOwner(
- account.email || account.username || account._account_id);
+ account.email || account.username || account.name ||
+ account._account_id);
},
_computeShowEmail(account) {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index e838818..684a967 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -201,6 +201,11 @@
return this._fetchSharedCacheURL('/config/server/info');
},
+ getProject(project) {
+ return this._fetchSharedCacheURL(
+ '/projects/' + encodeURIComponent(project));
+ },
+
getProjectConfig(project) {
return this._fetchSharedCacheURL(
'/projects/' + encodeURIComponent(project) + '/config');
@@ -1016,6 +1021,17 @@
);
},
+ getProjectAccessRights(projectName) {
+ return this._fetchSharedCacheURL(
+ `/projects/${encodeURIComponent(projectName)}/access`);
+ },
+
+ setProjectAccessRights(projectName, projectInfo) {
+ return this.send(
+ 'POST', `/projects/${encodeURIComponent(projectName)}/access`,
+ projectInfo);
+ },
+
/**
* @param {string} inputVal
* @param {number} opt_n
@@ -1679,6 +1695,10 @@
});
},
+ getCapabilities(token) {
+ return this.fetchJSON('/config/server/capabilities');
+ },
+
setAssignee(changeNum, assignee) {
const p = {assignee};
return this.getChangeURLAndSend(changeNum, 'PUT', null, '/assignee', p);
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 3a3c242..1df8d98 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -45,6 +45,7 @@
'admin/gr-permission/gr-permission_test.html',
'admin/gr-plugin-list/gr-plugin-list_test.html',
'admin/gr-project/gr-project_test.html',
+ 'admin/gr-project-access/gr-project-access_test.html',
'admin/gr-project-commands/gr-project-commands_test.html',
'admin/gr-project-detail-list/gr-project-detail-list_test.html',
'admin/gr-rule-editor/gr-rule-editor_test.html',
