Merge "Add support to retrieve the account diff preferences via REST"
diff --git a/Documentation/cmd-index.txt b/Documentation/cmd-index.txt
index f82532d..780231c 100644
--- a/Documentation/cmd-index.txt
+++ b/Documentation/cmd-index.txt
@@ -156,9 +156,12 @@
 link:cmd-plugin-remove.html[gerrit plugin rm]::
     Alias for 'gerrit plugin remove'.
 
-link:cmd-test-submit-rule.html[gerrit test-submit-rule]::
+link:cmd-test-submit-rule.html[gerrit test-submit rule]::
 	Test prolog submit rules.
 
+link:cmd-test-submit-type.html[gerrit test-submit type]::
+	Test prolog submit type.
+
 link:cmd-kill.html[kill]::
 	Kills a scheduled or running task.
 
diff --git a/Documentation/cmd-test-submit-type.txt b/Documentation/cmd-test-submit-type.txt
new file mode 100644
index 0000000..f6d5fba
--- /dev/null
+++ b/Documentation/cmd-test-submit-type.txt
@@ -0,0 +1,53 @@
+gerrit test-submit type
+=======================
+
+NAME
+----
+gerrit test-submit type - Test prolog submit type with a chosen change.
+
+SYNOPSIS
+--------
+[verse]
+'ssh' -p <port> <host> 'gerrit test-submit type'
+  [-s]
+  [--no-filters]
+  CHANGE
+
+DESCRIPTION
+-----------
+Provides a way to test prolog submit type.
+
+OPTIONS
+-------
+-s::
+	Reads a rules.pl file from stdin instead of rules.pl in refs/meta/config.
+
+--no-filters::
+	Don't run the submit_type_filter/2 from the parent projects of the specified change.
+
+ACCESS
+------
+Can be used by anyone that has permission to read the specified change.
+
+EXAMPLES
+--------
+
+Test submit_type from stdin and return the submit type.
+====
+ cat rules.pl | ssh -p 29418 review.example.com gerrit test-submit type -s I78f2c6673db24e4e92ed32f604c960dc952437d9
+ "MERGE_IF_NECESSARY"
+====
+
+Test the active submit_type from the refs/meta/config branch, ignoring filters in the project parents.
+====
+ $ ssh -p 29418 review.example.com gerrit test-submit type I78f2c6673db24e4e92ed32f604c960dc952437d9 --no-filters
+ "MERGE_IF_NECESSARY"
+====
+
+SCRIPTING
+---------
+Can be used either interactively for testing new prolog submit type, or from a script to check the submit type of a change.
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/prolog-cookbook.txt b/Documentation/prolog-cookbook.txt
index d832b05..7912cf2 100644
--- a/Documentation/prolog-cookbook.txt
+++ b/Documentation/prolog-cookbook.txt
@@ -269,6 +269,7 @@
 default implementation of submit rule that is named `gerrit:default_submit` and
 its result will be filtered as described above.
 
+[[HowToWriteSubmitType]]
 How to write submit type
 ------------------------
 Writing custom submit type logic in Prolog is the similar top
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index b3a73f8..51dcdd1 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -1518,6 +1518,99 @@
   HTTP/1.1 204 No Content
 ----
 
+[[list-comments]]
+List Comments
+~~~~~~~~~~~~~
+[verse]
+'GET /changes/link:#change-id[\{change-id\}]/revisions/link:#revision-id[\{revision-id\}]/comments/'
+
+Lists the published comments of a revision.
+
+As result a map is returned that maps the file path to a list of
+link:#comment-info[CommentInfo] entries. The entries in the map are
+sorted by file path.
+
+.Request
+----
+  GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/comments/ HTTP/1.0
+----
+
+.Response
+----
+  HTTP/1.1 200 OK
+  Content-Disposition: attachment
+  Content-Type: application/json;charset=UTF-8
+
+  )]}'
+  {
+    "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java": [
+      {
+        "kind": "gerritcodereview#comment",
+        "id": "TvcXrmjM",
+        "line": 23,
+        "message": "[nit] trailing whitespace",
+        "updated": "2013-02-26 15:40:43.986000000",
+        "author": {
+          "_account_id": 1000096,
+          "name": "John Doe",
+          "email": "john.doe@example.com"
+        }
+      },
+      {
+        "kind": "gerritcodereview#comment",
+        "id": "TveXwFiA",
+        "line": 49,
+        "in_reply_to": "TfYX-Iuo",
+        "message": "Done",
+        "updated": "2013-02-26 15:40:45.328000000",
+        "author": {
+          "_account_id": 1000097,
+          "name": "Jane Roe",
+          "email": "jane.roe@example.com"
+        }
+      }
+    ]
+  }
+----
+
+[[get-comment]]
+Get Comment
+~~~~~~~~~~~
+[verse]
+'GET /changes/link:#change-id[\{change-id\}]/revisions/link:#revision-id[\{revision-id\}]/comments/link:#comment-id[\{comment-id\}]'
+
+Retrieves a published comment of a revision.
+
+.Request
+----
+  GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/comments/TvcXrmjM HTTP/1.0
+----
+
+As response a link:#comment-info[CommentInfo] entity is returned that
+describes the published comment.
+
+.Response
+----
+  HTTP/1.1 200 OK
+  Content-Disposition: attachment
+  Content-Type: application/json;charset=UTF-8
+
+  )]}'
+  {
+    "kind": "gerritcodereview#comment",
+    "id": "TvcXrmjM",
+    "path": "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java",
+    "line": 23,
+    "message": "[nit] trailing whitespace",
+    "updated": "2013-02-26 15:40:43.986000000",
+    "author": {
+      "_account_id": 1000096,
+      "name": "John Doe",
+      "email": "john.doe@example.com"
+    }
+  }
+----
+
 [[set-reviewed]]
 Set Reviewed
 ~~~~~~~~~~~~
@@ -1582,6 +1675,11 @@
   ("I8473b95934b5732ac55d26311a706c9c2bde9940")
 * a legacy numeric change ID ("4247")
 
+[[comment-id]]
+\{comment-id\}
+~~~~~~~~~~~~~~
+UUID of a published comment.
+
 [[draft-id]]
 \{draft-id\}
 ~~~~~~~~~~~~
@@ -1656,7 +1754,10 @@
 [options="header",width="50%",cols="1,^1,5"]
 |===========================
 |Field Name    ||Description
-|`value`       ||The vote that the user has given for the label.
+|`value`       |optional|
+The vote that the user has given for the label. If present and zero, the
+user is permitted to vote on the label. If absent, the user is not
+permitted to vote on that label.
 |===========================
 
 [[change-info]]
@@ -1736,7 +1837,7 @@
 |===========================
 |Field Name    ||Description
 |`kind`        ||`gerritcodereview#comment`
-|`id`          ||The URL encoded UUID of the draft comment.
+|`id`          ||The URL encoded UUID of the comment.
 |`path`        |optional|
 The path of the file for which the inline comment was done. +
 Not set if returned in a map where the key is the file path.
@@ -1753,6 +1854,10 @@
 |`updated`     ||
 The link:rest-api.html#timestamp[timestamp] of when this comment was
 written.
+|`author`      |optional|
+The author of the message as an +
+link:rest-api-accounts.html#account-info[AccountInfo] entity. +
+Unset for draft comments, assumed to be the calling user.
 |===========================
 
 [[comment-input]]
diff --git a/ReleaseNotes/ReleaseNotes-2.6.txt b/ReleaseNotes/ReleaseNotes-2.6.txt
index e1b7e16..fd59773 100644
--- a/ReleaseNotes/ReleaseNotes-2.6.txt
+++ b/ReleaseNotes/ReleaseNotes-2.6.txt
@@ -33,8 +33,12 @@
 
 * New 'Rebase If Necessary' submit type. This is similar to cherry
   pick, but honors change dependency information.
-* Submit type displayed per-change in the info block.
-* Submit type selectable per-change by `submit_type` Prolog rules.
+
+* The submit type that is used for submitting a change is shown on the
+  change screen in the info block.
++
+This is useful because the submit type of a change can now be
+link:#submit-type-from-prolog[controlled by Prolog].
 
 * The rebase button is hidden when the patch set is current.
 
@@ -53,28 +57,93 @@
 
 Access Controls
 ~~~~~~~~~~~~~~~
-* Remove Reviewer is a new permission.
-* Pushing a signed tag is a new permission.
-* Editing the topic name is a new permission.
-* Raw database access with the `gsql` command is a new global capability.
+* Allow to overrule `BLOCK` permissions on the same project
++
+It was impossible to block a permission for a group and allow the same
+permission for a sub-group of that group as the `BLOCK` permission
+always won over any `ALLOW` permission. For example, it was impossible
+to block the "Forge Committer" permission for all users and then allow
+it only for a couple of privileged users.
++
+An `ALLOW` permission has now  priority over a `BLOCK` permission when
+they are defined in the same access section of a project. To achieve the
+above mentioned policy the following could be defined:
++
+  [access "refs/heads/*"]
+    forgeCommitter = block group Anonymous Users
+    forgeCommitter = group Privileged Users
++
+Across projects the `BLOCK` permission still wins over any `ALLOW`
+permission. This way one cannot override an inherited `BLOCK`
+permission in a subproject.
++
+Overruling of `BLOCK` permissions with `ALLOW` permissions also works
+for labels i.e. permission ranges. If a dedicated 'Verifiers' group
+need to be the only group who can vote in the 'Verified' label and it
+must be ensured that even project owners cannot change this policy,
+then the following can be defined in a common parent project:
++
+  [access "refs/heads/*"]
+    label-Verified = block -1..+1 group Anonymous Users
+    label-Verified = -1..+1 group Verifiers
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=1516[issue 1516]:
+  Show global capabilities to all users that can read `refs/meta/config`
++
+Users can now propose changes to the global capabilities for review
+from the WebUI.
+
+* link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/access-control.html#category_remove_reviewer[
+  Remove Reviewer] is a new permission.
+
+* link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/access-control.html#category_push_signed[
+  Pushing a signed tag] is a new permission.
+
+* link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/access-control.html#category_edit_topic_name[
+  Editing the topic name] is a new permission.
+
+* link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/access-control.html#capability_accessDatabase[
+  Raw database access] with the `gsql` command is a new global capability.
 +
 Previously site administrators had this capability by default.  Now it has
 to be explicitly assigned, even for site administrators.
 
 * link:https://code.google.com/p/gerrit/issues/detail?id=1585[Issue 1585]:
-Viewing other users' draft changes is a new permission.
+  link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/access-control.html#category_view_drafts[
+  Viewing other users' draft changes] is a new permission.
 
 * link:https://code.google.com/p/gerrit/issues/detail?id=1675[Issue 1675]:
-Deleting and publishing other users' draft changes is a new permission.
+  link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/access-control.html#category_delete_drafts[Deleting] and
+  link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/access-control.html#category_publish_drafts[publishing]
+  other users' draft changes is a new permission.
 
-* LDAP group names are configurable, `cn` is still the default.
-* LDAP cache reduces the number of recursive group queries.
+* link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/config-gerrit.html#ldap.groupName[
+  LDAP group names] are configurable, `cn` is still the default.
+
 * Kerberos authentication to LDAP servers is now supported.
 
-* Basic project properities are now inherited by default from parent
+* Basic project properties are now inherited by default from parent
   projects: Use Content Merge, Require Contributor Agreement, Require
   Change Id, Require Signed Off By.
 
+* Allow assigning `Push` for `refs/meta/config` on `All-Projects`
++
+The `refs/meta/config` branch of the `All-Projects` project should only
+be modified by Gerrit administrators because being able to do
+modifications on this branch means that the user could assign himself
+administrator permissions.
++
+In addition to being administrator Gerrit requires that the
+administrator has the `Push` access right for `refs/meta/config` in
+order to be able to modify it (just as with all other branches
+administrators do not have edit permissions by default).
++
+The problem was that assigning the `Push` access right for
+`refs/meta/config` on the `All-Projects` project was not allowed.
++
+Having the `Push` access right for `refs/meta/config` on the
+`All-Projects` project without being administrator has no effect.
+
 Hooks
 ~~~~~
 * Change topic is passed to hooks as `--topic NAME`.
@@ -84,26 +153,55 @@
 * link:https://code.google.com/p/gerrit/issues/detail?id=1237[Issue 1237]:
 New `merge-failed` hook and stream event when a change cannot be submitted due to failed merge.
 
+* link:https://code.google.com/p/gerrit/issues/detail?id=925[Issue 925]:
+New `ref-update` hook run before a push is accepted by Gerrit.
+
 SSH
 ~~~
+* New SSH command to http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/cmd-gc.html[
+  run Git garbage collection]
++
+All GC runs are logged in a GC log file.
+
 * Descriptions are added to ssh commands.
 +
 If `gerrit` is called without arguments, it will now show a list of available
 commands with their descriptions.
 
 * `create-account --http-password` enables setting/resetting the
-  HTTP password of role accounts, for Git or JSON API access.
+  HTTP password of role accounts, for Git or REST API access.
+
+* http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/cmd-ls-user-refs.html[
+  ls-user-refs] lists which refs are visible for a given user.
 
 * `ls-projects --has-acl-for` lists projects that mention a group
   in an ACL, identifying where rights are granted.
 
-* `query` includes submit record information from Prolog rules.
-* `query` includes `resumeSortKey` in summary block.
-* `query` includes author and change size information when given
-  certain options on the command line.
+* `review` command supports project-specific labels
 
-* `test-submit-rule` tests the `can_submit` rule with a prolog script loaded from a file or stdin.
-* `ls-user-refs` lists which refs are visible for a given user.
+* `test-submit-rule` was renamed to `test-submit rule`:
++
+`rule` is now a subcommand of the `test-submit` command.
+
+* http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/cmd-test-submit-type.html[
+  test-submit type] tests the Prolog submit type with a chosen change.
+
+Query
+~~~~~
+* Allow `{}` to be used for quoting in query expressions
++
+This makes it a little easier to query for group names that contain
+a space over SSH:
++
+  ssh srv gerrit query " 'status:open NOT reviewerin:{Developer Group}' "
+
+* The query summary block includes `resumeSortKey`.
+
+* Query results include author and change size information when certain
+  options are specified.
+
+* When a file is renamed the old file name is included in the Patch
+  attribute
 
 Plugins
 ~~~~~~~
@@ -117,17 +215,55 @@
   plugin which is included as a core plugin in the Gerrit distribution and
   can be installed during site initialization.
 
+Prolog
+~~~~~~
+[[submit-type-from-prolog]]
+* link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/prolog-cookbook.html#HowToWriteSubmitType[
+  Support controlling the submit type for changes from Prolog]
++
+Similarly like the `submit_rule` there is now a `submit_type` predicate
+which returns the allowed submit type for a change. When the
+`submit_type` predicate is not provided in the `rules.pl` then the
+project default submit type is used for all changes of that project.
++
+Filtering the results of the `submit_type` is also supported in the
+same way like filtering the results of the `submit_rule`. Using a
+`submit_type_filter` predicate one can enforce a particular submit type
+from a parent project.
+
+* Plugins can contribute Prolog facts/predicates from Java.
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=288[Issue 288]:
+  Expose basic commit statistics for the Prolog rule engine
++
+A new method `gerrit:commit_stats(-Files,-Insertions, -Deletions)` was
+added.
+
+* A new `max_with_block` predicate was added for more convenient usage
+
 Email
 ~~~~~
+* Notify project watchers if draft change is published
+* Notify users mentioned in commit footer on draft publish
+* Add new notify type that allows watching of new patch sets
+* link:https://code.google.com/p/gerrit/issues/detail?id=1686[Issue 1686]:
+  Add new notify type that allows watching abandoning of changes
+* link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/user-notify.html[
+  Notifications configured in `project.config`] can now be addressed
+  using any of To, CC, or BCC headers.
 * link:https://code.google.com/p/gerrit/issues/detail?id=1531[Issue 1531]:
 Email footers now include `Gerrit-HasComments: {Yes|No}`.
-* Notifications configured in `project.config` can now be addressed
-  using any of To, CC, or BCC headers.
 * `#if($email.hasInlineComments())` can be used in templates to test
   if there are comments to be included in this email.
 * Notification emails are sent to included groups.
 * Comment notification emails are sent to project watchers.
 * "Change Merged" emails include the diff output when `sendemail.includeDiff` is enabled.
+* When link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/rest-api-changes.html#set-review[
+  posting a review via REST] the caller can control email delivery
++
+This may help automated systems to be less noisy. Tools can now choose
+which review updates should send email, and which categories of users
+on a change should get that email.
 
 Labels
 ~~~~~~
@@ -138,6 +274,95 @@
 * Labels are no longer global; projects may define their own labels,
   with inheritance.
 
+Performance
+~~~~~~~~~~~
+* Bitmap Optimizations
++
+On running the http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/cmd-gc.html[
+garbage collection] Jgit creates bitmap data that is saved to an
+auxiliary file. The bitmap optimizations improve the clone and fetch
+performance. git-core will ignore the bitmap data.
+
+* Improve suggest user performance when adding a reviewer.
++
+Do not check the visibility of the change for each suggested account if
+the ref is visible by all registered users.
++
+On a system with about 2-3000 users, where most of the projects are
+visible by every registered user, this improves the performance of the
+suggesting reviewer by a factor of 1000 at least.
+
+* Cache RefControl.isVisible()
++
+For Git repositories with many changes the time for calculating visible
+refs is reduced by 30-50%.
+
+* Allow admins to disable magic ref check on upload
++
+Some sites manage to run their repositories in a way that prevents
+users from ever being able to create `refs/for`, `refs/drafts` or
+`refs/publish` names in a repository. Allow admins on those servers
+to disable this (somewhat) expensive check before every upload.
+
+* Permit ProjectCacheClock to be completely disabled
++
+Some admins may just want to require all updates to projects to be
+made through the web interface, and avoid the small expense of a
+background thread ticking off changes.
+
+* Batch read Change objects during query
+
+* Default `core.streamFileThreshold` to a larger value
++
+If this value is not configured by the server administrator
+performance on larger text files suffers considerably and
+Gerrit may grind to a halt and be unable to answer users.
++
+Default to either 25% of the available JVM heap or ~2048m.
+
+* Improve performance of ReceiveCommits for repositories with many refs
++
+Avoid adding `refs/changes/` and `refs/tags/` to RevWalk's as
+uninteresting since JGit RevWalk doesn't perform well when a large
+number of objects is marked as uninteresting.
+
+* PatchSet.isRef()-optimizations.
++
+PatchSet.isRef() is used extensively when preparing for a ref
+advertisment and the regular expression used by isRefs() was notably
+costly in these circumstances, especially since it could not be
+pre-compiled.
++
+The regular expression is removed and the check is now directly
+implemented. As result the performance of `git ls-remote` could be
+increased upto 15%.
+
+* New config option `receive.checkReferencedObjectsAreReachable`
++
+If set to true, Gerrit will validate that all referenced objects that
+are not included in the received pack are reachable by the user.
++
+Carrying out this check on Git repositories with many refs and commits
+can be a very CPU-heavy operation. For non public Gerrit servers it may
+make sense to disable this check, which is now possible.
+
+* Cache config value in LdapAuthBackend
+
+* Perform a single /accounts/self/capabilities on page load
++
+This joins up 3 requests into a single call, which should speed up
+initial page load for most users.
+
+* Only gzip compress responses that are smaller compressed
+
+* Caching of changes
++
+During Ref Advertisments (via VisibleRefFilter), all changes need to
+be fetched from the database to allow Gerrit to figure out which change
+refs are visible and should be advertised to the user. To reduce
+database traffic a cache for changes was introduced. This cache is
+disabled by default since it can mess-up multi-server setups.
+
 Upgrades
 ~~~~~~~~
 * link:https://code.google.com/p/gerrit/issues/detail?id=1619[Issue 1619]:
@@ -178,26 +403,46 @@
 
 SSH
 ~~~
-* `plugin ls` shows status of enabled plugins as "ENABLED".
 * `review --restore` allows a review score to be added on the restored change.
+
 * link:https://code.google.com/p/gerrit/issues/detail?id=1721[Issue 1721]:
-`review --message` only adds the message once.
+  `review --message` only adds the message once.
 
 * `ls-groups` prints "N/A" if the group's name is not set.
 
+* `set-project-parent --children-of`: Fix getting parent for level 1 projects
++
+For direct child projects of the `All-Projects` project the name of the
+parent project was incorrectly retrieved if the parent name was not
+explicitly stored as `All-Projects` in the project.config file.
+
+Query
+~~~~~
+* link:https://code.google.com/p/gerrit/issues/detail?id=1729[Issue 1729]:
+  Fix query by 'label:Verified=0'
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=1772[Issue 1772]:
+  Set `_more_changes` if result is limited due to configured query limit
+
+* Fix query cost for "status:merged commit:c0ffee"
+
 Email
 ~~~~~
-* Missing email templates are added to the site initialization.
 * Merge failure emails are only sent once per day.
 * Unused macros are removed from the mail templates.
 * Unnecessary ellipses are no longer applied to email subjects.
-* The diff output from an "octopus merge" is made more readable in change notification emails.
+* The empty diff output from an "octopus merge" is now explained in change notification emails.
 * link:https://code.google.com/p/gerrit/issues/detail?id=1480[Issue 1480]:
 Proper error message is shown when registering an email address fails.
 
 * link:https://code.google.com/p/gerrit/issues/detail?id=1692[Issue 1692]:
 Review comments are sorted before being added to notification emails.
 
+* Fix watching of 'All Comments' on `All-Projects`
++
+If a user is watching 'All Comments' on `All-Projects` this should
+apply to all projects.
+
 Documentation
 -------------
 
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java
index 6e3431a..0e7bd21 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java
@@ -218,12 +218,15 @@
             ad.setCanRemove(removableReviewers.contains(id));
             byUser.put(id, ad);
           }
-          ad.votable(name);
-          ad.value(name, ai.value());
-          if (formatValue(ai.value()).equals(max)) {
-            ad.approved(name);
-          } else if (formatValue(ai.value()).equals(min)) {
-            ad.rejected(name);
+          if (ai.has_value()) {
+            ad.votable(name);
+            ad.value(name, ai.value());
+            String fv = formatValue(ai.value());
+            if (fv.equals(max)) {
+              ad.approved(name);
+            } else if (ai.value() < 0 && fv.equals(min)) {
+              ad.rejected(name);
+            }
           }
         }
       }
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
index f4453da..2e1b972 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
@@ -150,6 +150,7 @@
   }
 
   public static class ApprovalInfo extends AccountInfo {
+    public final native boolean has_value() /*-{ return this.hasOwnProperty('value'); }-*/;
     public final native short value() /*-{ return this.value; }-*/;
 
     protected ApprovalInfo() {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchSetSelectBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchSetSelectBox.java
index 7f7a36e..df12b70 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchSetSelectBox.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchSetSelectBox.java
@@ -190,10 +190,10 @@
       return null;
     }
 
-    Patch.Key key =
-        (idSideA == null) ? patchKey : (new Patch.Key(idSideA, patchKey.get()));
+    Patch.Key key = (idActive == null) ? //
+        patchKey : (new Patch.Key(idActive, patchKey.get()));
 
-    String sideURL = (side == Side.A) ? "1" : "0";
+    String sideURL = (idActive == null) ? "1" : "0";
     final String base = GWT.getHostPageBaseURL() + "cat/";
 
     Image image = new Image(Gerrit.RESOURCES.downloadIcon());
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
index cdd9fae..8d79d66 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
@@ -137,9 +137,6 @@
   @Column(id = 5, length = 16, notNull = false)
   protected String changeSortKey;
 
-  /** Label name copied from corresponding {@link ApprovalCategory}. */
-  protected String label;
-
   protected PatchSetApproval() {
   }
 
@@ -200,15 +197,11 @@
   }
 
   public String getLabel() {
-    return label;
-  }
-
-  public void setLabel(String label) {
-    this.label = label;
+    return getLabelId().get();
   }
 
   public boolean isSubmit() {
-    return LabelId.SUBMIT.get().equals(getLabelId().get());
+    return LabelId.SUBMIT.get().equals(getLabel());
   }
 
   @Override
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 43ae87e..bc98c1b 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
@@ -25,7 +25,9 @@
 import static com.google.gerrit.common.changes.ListChangesOption.LABELS;
 
 import com.google.common.base.Joiner;
+import com.google.common.base.Objects;
 import com.google.common.base.Strings;
+import com.google.common.collect.HashBasedTable;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.LinkedListMultimap;
@@ -34,6 +36,7 @@
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
+import com.google.common.collect.Table;
 import com.google.gerrit.common.changes.ListChangesOption;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
@@ -301,7 +304,7 @@
     if (ps == null) {
       return ImmutableList.of();
     }
-    cd.setSubmitRecords(ctl.canSubmit(db.get(), ps, cd, true, false));
+    cd.setSubmitRecords(ctl.canSubmit(db.get(), ps, cd, true, false, true));
     return cd.getSubmitRecords();
   }
 
@@ -426,42 +429,45 @@
       return;
     }
 
+    // All users ever added, even if they can't vote on one or all labels.
     Set<Account.Id> allUsers = Sets.newHashSet();
-    Multimap<Account.Id, PatchSetApproval> current = HashMultimap.create();
-    PatchSet.Id psId = baseCtrl.getChange().currentPatchSetId();
-    for (PatchSetApproval psa : labelNormalizer.normalize(
-        baseCtrl, cd.allApprovals(db))) {
+    ListMultimap<PatchSet.Id, PatchSetApproval> allApprovals =
+        cd.allApprovalsMap(db);
+    for (PatchSetApproval psa : allApprovals.values()) {
       allUsers.add(psa.getAccountId());
-      if (psa.getPatchSetId().equals(psId)) {
-        current.put(psa.getAccountId(), psa);
-      }
+    }
+
+    List<PatchSetApproval> currentList = labelNormalizer.normalize(
+        baseCtrl, allApprovals.get(baseCtrl.getChange().currentPatchSetId()));
+    // Most recent, normalized vote on each label for the current patch set by
+    // each user (may be 0).
+    Table<Account.Id, String, PatchSetApproval> current = HashBasedTable.create(
+        allUsers.size(), baseCtrl.getLabelTypes().getLabelTypes().size());
+    for (PatchSetApproval psa : currentList) {
+      current.put(psa.getAccountId(), psa.getLabel(), psa);
     }
 
     for (Account.Id accountId : allUsers) {
       IdentifiedUser user = userFactory.create(accountId);
       ChangeControl ctl = baseCtrl.forUser(user);
-      Map<String, ApprovalInfo> byLabel =
-          Maps.newHashMapWithExpectedSize(labels.size());
-
-      for (String name : labels.keySet()) {
-        LabelType lt = ctl.getLabelTypes().byLabel(name);
-        if (lt != null && labelNormalizer.canVote(ctl, lt, accountId)) {
-          ApprovalInfo ai = approvalInfo(accountId, 0);
-          byLabel.put(name, ai);
-          labels.get(name).addApproval(ai);
-        }
-      }
-      for (PatchSetApproval psa : current.get(accountId)) {
-        LabelType lt = ctl.getLabelTypes().byLabel(psa.getLabelId());
+      for (Map.Entry<String, LabelInfo> e : labels.entrySet()) {
+        LabelType lt = ctl.getLabelTypes().byLabel(e.getKey());
         if (lt == null) {
+          // Ignore submit record for undefined label; likely the submit rule
+          // author didn't intend for the label to show up in the table.
           continue;
         }
-
-        ApprovalInfo info = byLabel.get(lt.getName());
-        if (info == null) {
-          continue;
+        Integer value;
+        PatchSetApproval psa = current.get(accountId, lt.getName());
+        if (psa != null) {
+          value = Integer.valueOf(psa.getValue());
+        } else {
+          // Either the user cannot vote on this label, or there just wasn't a
+          // dummy approval for this label. Explicitly check whether the user
+          // can vote on this label.
+          value = labelNormalizer.canVote(ctl, lt, accountId) ? 0 : null;
         }
-        info.value = psa.getValue();
+        e.getValue().addApproval(approvalInfo(accountId, value));
       }
     }
   }
@@ -519,7 +525,7 @@
         short val = psa.getValue();
         ApprovalInfo info = byLabel.get(type.getName());
         if (info != null) {
-          info.value = val;
+          info.value = Integer.valueOf(val);
         }
 
         LabelInfo li = labels.get(type.getName());
@@ -544,7 +550,7 @@
     return labels;
   }
 
-  private ApprovalInfo approvalInfo(Account.Id id, int value) {
+  private ApprovalInfo approvalInfo(Account.Id id, Integer value) {
     ApprovalInfo ai = new ApprovalInfo(id);
     ai.value = value;
     accountLoader.put(ai);
@@ -619,7 +625,7 @@
         continue;
       }
       for (ApprovalInfo ai : label.all) {
-        if (ctl.canRemoveReviewer(ai._id, ai.value)) {
+        if (ctl.canRemoveReviewer(ai._id, Objects.firstNonNull(ai.value, 0))) {
           removable.add(ai._id);
         } else {
           fixed.add(ai._id);
@@ -914,7 +920,7 @@
   }
 
   static class ApprovalInfo extends AccountInfo {
-    int value;
+    Integer value;
 
     ApprovalInfo(Account.Id id) {
       super(id);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentInfo.java
new file mode 100644
index 0000000..798b9c6
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentInfo.java
@@ -0,0 +1,55 @@
+// Copyright (C) 2013 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.change;
+
+package com.google.gerrit.server.change;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.extensions.restapi.Url;
+import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.server.account.AccountInfo;
+
+import java.sql.Timestamp;
+
+public class CommentInfo {
+  static enum Side {
+    PARENT, REVISION;
+  }
+
+  final String kind = "gerritcodereview#comment";
+  String id;
+  String path;
+  Side side;
+  Integer line;
+  String inReplyTo;
+  String message;
+  Timestamp updated;
+  AccountInfo author;
+
+  CommentInfo(PatchLineComment c, AccountInfo.Loader accountLoader) {
+    id = Url.encode(c.getKey().get());
+    path = c.getKey().getParentKey().getFileName();
+    if (c.getSide() == 0) {
+      side = Side.PARENT;
+    }
+    if (c.getLine() > 0) {
+      line = c.getLine();
+    }
+    inReplyTo = Url.encode(c.getParentUuid());
+    message = Strings.emptyToNull(c.getMessage());
+    updated = c.getWrittenOn();
+    if (accountLoader != null) {
+      author = accountLoader.get(c.getAuthor());
+    }
+  }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentResource.java
new file mode 100644
index 0000000..ec47d01
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentResource.java
@@ -0,0 +1,51 @@
+// Copyright (C) 2013 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.change;
+
+import com.google.gerrit.extensions.restapi.RestResource;
+import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.inject.TypeLiteral;
+
+public class CommentResource implements RestResource {
+  public static final TypeLiteral<RestView<CommentResource>> COMMENT_KIND =
+      new TypeLiteral<RestView<CommentResource>>() {};
+
+  private final RevisionResource rev;
+  private final PatchLineComment comment;
+
+  CommentResource(RevisionResource rev, PatchLineComment c) {
+    this.rev = rev;
+    this.comment = c;
+  }
+
+  public PatchSet getPatchSet() {
+    return rev.getPatchSet();
+  }
+
+  PatchLineComment getComment() {
+    return comment;
+  }
+
+  String getId() {
+    return comment.getKey().get();
+  }
+
+  Account.Id getAuthorId() {
+    return comment.getAuthor();
+  }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Comments.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Comments.java
new file mode 100644
index 0000000..91cfbf8
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Comments.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2013 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.change;
+
+import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.restapi.ChildCollection;
+import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+class Comments implements ChildCollection<RevisionResource, CommentResource> {
+  private final DynamicMap<RestView<CommentResource>> views;
+  private final Provider<ListComments> list;
+  private final Provider<ReviewDb> dbProvider;
+
+  @Inject
+  Comments(DynamicMap<RestView<CommentResource>> views,
+      Provider<ListComments> list,
+      Provider<ReviewDb> dbProvider) {
+    this.views = views;
+    this.list = list;
+    this.dbProvider = dbProvider;
+  }
+
+  @Override
+  public DynamicMap<RestView<CommentResource>> views() {
+    return views;
+  }
+
+  @Override
+  public RestView<RevisionResource> list() {
+    return list.get();
+  }
+
+  @Override
+  public CommentResource parse(RevisionResource rev, IdString id)
+      throws ResourceNotFoundException, OrmException {
+    String uuid = id.get();
+    for (PatchLineComment c : dbProvider.get().patchComments()
+        .publishedByPatchSet(rev.getPatchSet().getId())) {
+      if (uuid.equals(c.getKey().get())) {
+        return new CommentResource(rev, c);
+      }
+    }
+    throw new ResourceNotFoundException(id);
+  }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java
index 348be97..5157af4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java
@@ -42,7 +42,7 @@
   }
 
   @Override
-  public Response<GetDraft.Comment> apply(RevisionResource rsrc, Input in)
+  public Response<CommentInfo> apply(RevisionResource rsrc, Input in)
       throws AuthException, BadRequestException, ResourceConflictException, OrmException {
     if (Strings.isNullOrEmpty(in.path)) {
       throw new BadRequestException("path must be non-empty");
@@ -60,9 +60,9 @@
         rsrc.getAccountId(),
         Url.decode(in.inReplyTo));
     c.setStatus(Status.DRAFT);
-    c.setSide(in.side == GetDraft.Side.PARENT ? (short) 0 : (short) 1);
+    c.setSide(in.side == CommentInfo.Side.PARENT ? (short) 0 : (short) 1);
     c.setMessage(in.message.trim());
     db.get().patchComments().insert(Collections.singleton(c));
-    return Response.created(new GetDraft.Comment(c));
+    return Response.created(new CommentInfo(c, null));
   }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetComment.java
new file mode 100644
index 0000000..68b0435
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetComment.java
@@ -0,0 +1,38 @@
+// Copyright (C) 2013 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.change;
+
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.account.AccountInfo;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+
+class GetComment implements RestReadView<CommentResource> {
+
+  private final AccountInfo.Loader.Factory accountLoaderFactory;
+
+  @Inject
+  GetComment(AccountInfo.Loader.Factory accountLoaderFactory) {
+    this.accountLoaderFactory = accountLoaderFactory;
+  }
+
+  @Override
+  public Object apply(CommentResource rsrc) throws OrmException {
+    AccountInfo.Loader accountLoader = accountLoaderFactory.create(true);
+    CommentInfo ci = new CommentInfo(rsrc.getComment(), accountLoader);
+    accountLoader.fill();
+    return ci;
+  }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDraft.java
index 596659d..6b36048 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDraft.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDraft.java
@@ -14,49 +14,15 @@
 
 package com.google.gerrit.server.change;
 
-import com.google.common.base.Strings;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.extensions.restapi.Url;
-import com.google.gerrit.reviewdb.client.PatchLineComment;
-
-import java.sql.Timestamp;
 
 class GetDraft implements RestReadView<DraftResource> {
   @Override
   public Object apply(DraftResource rsrc) throws AuthException,
       BadRequestException, ResourceConflictException, Exception {
-    return new Comment(rsrc.getComment());
-  }
-
-  static enum Side {
-    PARENT, REVISION;
-  }
-
-  static class Comment {
-    final String kind = "gerritcodereview#comment";
-    String id;
-    String path;
-    Side side;
-    Integer line;
-    String inReplyTo;
-    String message;
-    Timestamp updated;
-
-    Comment(PatchLineComment c) {
-      id = Url.encode(c.getKey().get());
-      path = c.getKey().getParentKey().getFileName();
-      if (c.getSide() == 0) {
-        side = Side.PARENT;
-      }
-      if (c.getLine() > 0) {
-        line = c.getLine();
-      }
-      inReplyTo = Url.encode(c.getParentUuid());
-      message = Strings.emptyToNull(c.getMessage());
-      updated = c.getWrittenOn();
-    }
+    return new CommentInfo(rsrc.getComment(), null);
   }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java
new file mode 100644
index 0000000..23a71e8
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java
@@ -0,0 +1,40 @@
+// Copyright (C) 2013 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.change;
+
+import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.account.AccountInfo;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+class ListComments extends ListDrafts {
+  @Inject
+  ListComments(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf) {
+    super(db, alf);
+  }
+
+  @Override
+  protected boolean includeAuthorInfo() {
+    return true;
+  }
+
+  protected Iterable<PatchLineComment> listComments(RevisionResource rsrc)
+      throws OrmException {
+    return db.get().patchComments()
+        .publishedByPatchSet(rsrc.getPatchSet().getId());
+  }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java
index 564363e..47863ac 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java
@@ -24,8 +24,10 @@
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.change.GetDraft.Comment;
-import com.google.gerrit.server.change.GetDraft.Side;
+import com.google.gerrit.server.account.AccountInfo;
+import com.google.gerrit.server.change.CommentInfo;
+import com.google.gerrit.server.change.CommentInfo.Side;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -35,23 +37,36 @@
 import java.util.Map;
 
 class ListDrafts implements RestReadView<RevisionResource> {
-  private final Provider<ReviewDb> db;
+  protected final Provider<ReviewDb> db;
+  private final AccountInfo.Loader.Factory accountLoaderFactory;
 
   @Inject
-  ListDrafts(Provider<ReviewDb> db) {
+  ListDrafts(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf) {
     this.db = db;
+    this.accountLoaderFactory = alf;
+  }
+
+  protected Iterable<PatchLineComment> listComments(RevisionResource rsrc)
+      throws OrmException {
+    return db.get().patchComments()
+        .draftByPatchSetAuthor(
+            rsrc.getPatchSet().getId(),
+            rsrc.getAccountId());
+  }
+
+  protected boolean includeAuthorInfo() {
+    return false;
   }
 
   @Override
   public Object apply(RevisionResource rsrc) throws AuthException,
       BadRequestException, ResourceConflictException, Exception {
-    Map<String, List<Comment>> out = Maps.newTreeMap();
-    for (PatchLineComment c : db.get().patchComments()
-        .draftByPatchSetAuthor(
-            rsrc.getPatchSet().getId(),
-            rsrc.getAccountId())) {
-      Comment o = new Comment(c);
-      List<Comment> list = out.get(o.path);
+    Map<String, List<CommentInfo>> out = Maps.newTreeMap();
+    AccountInfo.Loader accountLoader =
+        includeAuthorInfo() ? accountLoaderFactory.create(true) : null;
+    for (PatchLineComment c : listComments(rsrc)) {
+      CommentInfo o = new CommentInfo(c, accountLoader);
+      List<CommentInfo> list = out.get(o.path);
       if (list == null) {
         list = Lists.newArrayList();
         out.put(o.path, list);
@@ -59,10 +74,10 @@
       o.path = null;
       list.add(o);
     }
-    for (List<Comment> list : out.values()) {
-      Collections.sort(list, new Comparator<Comment>() {
+    for (List<CommentInfo> list : out.values()) {
+      Collections.sort(list, new Comparator<CommentInfo>() {
         @Override
-        public int compare(Comment a, Comment b) {
+        public int compare(CommentInfo a, CommentInfo b) {
           int c = firstNonNull(a.side, Side.REVISION).ordinal()
                 - firstNonNull(b.side, Side.REVISION).ordinal();
           if (c == 0) {
@@ -75,6 +90,9 @@
         }
       });
     }
+    if (accountLoader != null) {
+      accountLoader.fill();
+    }
     return out;
   }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
index 7e175a7..690faf3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
@@ -15,6 +15,7 @@
 package com.google.gerrit.server.change;
 
 import static com.google.gerrit.server.change.ChangeResource.CHANGE_KIND;
+import static com.google.gerrit.server.change.CommentResource.COMMENT_KIND;
 import static com.google.gerrit.server.change.DraftResource.DRAFT_KIND;
 import static com.google.gerrit.server.change.PatchResource.PATCH_KIND;
 import static com.google.gerrit.server.change.ReviewerResource.REVIEWER_KIND;
@@ -33,9 +34,11 @@
     bind(Revisions.class);
     bind(Reviewers.class);
     bind(Drafts.class);
+    bind(Comments.class);
     bind(Patches.class);
 
     DynamicMap.mapOf(binder(), CHANGE_KIND);
+    DynamicMap.mapOf(binder(), COMMENT_KIND);
     DynamicMap.mapOf(binder(), DRAFT_KIND);
     DynamicMap.mapOf(binder(), PATCH_KIND);
     DynamicMap.mapOf(binder(), REVIEWER_KIND);
@@ -70,6 +73,9 @@
     put(DRAFT_KIND).to(PutDraft.class);
     delete(DRAFT_KIND).to(DeleteDraft.class);
 
+    child(REVISION_KIND, "comments").to(Comments.class);
+    get(COMMENT_KIND).to(GetComment.class);
+
     child(REVISION_KIND, "files").to(Patches.class);
     put(PATCH_KIND, "reviewed").to(PutReviewed.class);
     delete(PATCH_KIND, "reviewed").to(DeleteReviewed.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 66b939b..044e266 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -92,7 +92,7 @@
 
   static class Comment {
     String id;
-    GetDraft.Side side;
+    CommentInfo.Side side;
     int line;
     String inReplyTo;
     String message;
@@ -288,7 +288,7 @@
         }
         e.setStatus(PatchLineComment.Status.PUBLISHED);
         e.setWrittenOn(timestamp);
-        e.setSide(c.side == GetDraft.Side.PARENT ? (short) 0 : (short) 1);
+        e.setSide(c.side == CommentInfo.Side.PARENT ? (short) 0 : (short) 1);
         e.setMessage(c.message);
         (create ? ins : upd).add(e);
       }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java
index d5eaa9b..befb8d7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java
@@ -23,7 +23,7 @@
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.change.GetDraft.Side;
+import com.google.gerrit.server.change.CommentInfo.Side;
 import com.google.gerrit.server.change.PutDraft.Input;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -85,12 +85,12 @@
     } else {
       db.get().patchComments().update(Collections.singleton(update(c, in)));
     }
-    return new GetDraft.Comment(c);
+    return new CommentInfo(c, null);
   }
 
   private PatchLineComment update(PatchLineComment e, Input in) {
     if (in.side != null) {
-      e.setSide(in.side == GetDraft.Side.PARENT ? (short) 0 : (short) 1);
+      e.setSide(in.side == CommentInfo.Side.PARENT ? (short) 0 : (short) 1);
     }
     if (in.line != null) {
       e.setLine(in.line);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
index 6d965ad..734d524 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -100,7 +100,8 @@
     ChangeData cd = new ChangeData(ctl);
     PatchSet ps = cd.currentPatchSet(db);
     if (ps != null) {
-      for (SubmitRecord rec : ctl.canSubmit(db.get(), ps, cd, true, false)) {
+      for (SubmitRecord rec :
+          ctl.canSubmit(db.get(), ps, cd, true, false, true)) {
         if (rec.labels == null) {
           continue;
         }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
index 6411289..3f09916 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
@@ -108,7 +108,6 @@
   private PatchSetApproval copy(PatchSetApproval src, ChangeControl ctl) {
     PatchSetApproval dest = new PatchSetApproval(src.getPatchSetId(), src);
     dest.cache(ctl.getChange());
-    dest.setLabel(src.getLabel());
     return dest;
   }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index 57e2706..02caa19 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -27,7 +27,6 @@
 import com.google.common.collect.Sets;
 import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.common.data.Capable;
-import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.data.SubmitTypeRecord;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Branch;
@@ -160,8 +159,7 @@
       final ProjectCache pc, final LabelNormalizer fs,
       final GitReferenceUpdated gru, final MergedSender.Factory msf,
       final MergeFailSender.Factory mfsf,
-      final LabelTypes labelTypes, final PatchSetInfoFactory psif,
-      final IdentifiedUser.GenericFactory iuf,
+      final PatchSetInfoFactory psif, final IdentifiedUser.GenericFactory iuf,
       final ChangeControl.GenericFactory changeControlFactory,
       final MergeQueue mergeQueue, @Assisted final Branch.NameKey branch,
       final ChangeHooks hooks, final AccountCache accountCache,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 14508a4..308f5db 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -1022,10 +1022,6 @@
       this.draft = cmd.getRefName().startsWith(MagicBranch.NEW_DRAFT_CHANGE);
     }
 
-    boolean isRef(Ref ref) {
-      return ctl != null && ref.getName().equals(ctl.getRefName());
-    }
-
     boolean isDraft() {
       return draft;
     }
@@ -1262,7 +1258,10 @@
     try {
       Set<ObjectId> existing = Sets.newHashSet();
       walk.markStart(walk.parseCommit(magicBranch.cmd.getNewId()));
-      markHeadsAsUninteresting(walk, existing);
+      markHeadsAsUninteresting(
+          walk,
+          existing,
+          magicBranch.ctl != null ? magicBranch.ctl.getRefName() : null);
 
       List<ChangeLookup> pending = Lists.newArrayList();
       final Set<Change.Key> newChangeIds = new HashSet<Change.Key>();
@@ -1361,14 +1360,17 @@
     return newChanges;
   }
 
-  private void markHeadsAsUninteresting(final RevWalk walk, Set<ObjectId> existing) {
+  private void markHeadsAsUninteresting(
+      final RevWalk walk,
+      Set<ObjectId> existing,
+      @Nullable String forRef) {
     for (Ref ref : allRefs.values()) {
       if (ref.getObjectId() == null) {
         continue;
       } else if (ref.getName().startsWith("refs/changes/")) {
         existing.add(ref.getObjectId());
       } else if (ref.getName().startsWith(R_HEADS)
-          || (magicBranch != null && magicBranch.isRef(ref))) {
+          || (forRef != null && forRef.equals(ref.getName()))) {
         try {
           walk.markUninteresting(walk.parseCommit(ref.getObjectId()));
         } catch (IOException e) {
@@ -1952,7 +1954,7 @@
     try {
       Set<ObjectId> existing = Sets.newHashSet();
       walk.markStart(walk.parseCommit(cmd.getNewId()));
-      markHeadsAsUninteresting(walk, existing);
+      markHeadsAsUninteresting(walk, existing, cmd.getRefName());
 
       RevCommit c;
       while ((c = walk.next()) != null) {
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 d0185dd..48b0731 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
@@ -316,7 +316,7 @@
   }
 
   public List<SubmitRecord> getSubmitRecords(ReviewDb db, PatchSet patchSet) {
-    return canSubmit(db, patchSet, null, false, true);
+    return canSubmit(db, patchSet, null, false, true, false);
   }
 
   public boolean canSubmit() {
@@ -324,12 +324,13 @@
   }
 
   public List<SubmitRecord> canSubmit(ReviewDb db, PatchSet patchSet) {
-    return canSubmit(db, patchSet, null, false, false);
+    return canSubmit(db, patchSet, null, false, false, false);
   }
 
- @SuppressWarnings("unchecked")
+  @SuppressWarnings("unchecked")
   public List<SubmitRecord> canSubmit(ReviewDb db, PatchSet patchSet,
-      @Nullable ChangeData cd, boolean fastEvalLabels, boolean allowClosed) {
+      @Nullable ChangeData cd, boolean fastEvalLabels, boolean allowClosed,
+      boolean allowDraft) {
     if (!allowClosed && change.getStatus().isClosed()) {
       SubmitRecord rec = new SubmitRecord();
       rec.status = SubmitRecord.Status.CLOSED;
@@ -340,23 +341,9 @@
       return ruleError("Patch set " + patchSet.getPatchSetId() + " is not current");
     }
 
-    try {
-      if (change.getStatus() == Change.Status.DRAFT) {
-        if (!isDraftVisible(db, cd)) {
-          return ruleError("Patch set " + patchSet.getPatchSetId() + " not found");
-        } else {
-          return ruleError("Cannot submit draft changes");
-        }
-      }
-      if (patchSet.isDraft()) {
-        if (!isDraftVisible(db, cd)) {
-          return ruleError("Patch set " + patchSet.getPatchSetId() + " not found");
-        } else {
-          return ruleError("Cannot submit draft patch sets");
-        }
-      }
-    } catch (OrmException err) {
-      return logRuleError("Cannot read patch set " + patchSet.getId(), err);
+    if ((change.getStatus() == Change.Status.DRAFT || patchSet.isDraft())
+        && !allowDraft) {
+      return cannotSubmitDraft(db, patchSet, cd);
     }
 
     List<Term> results;
@@ -387,6 +374,21 @@
     return resultsToSubmitRecord(evaluator.getSubmitRule(), results);
   }
 
+  private List<SubmitRecord> cannotSubmitDraft(ReviewDb db, PatchSet patchSet,
+      ChangeData cd) {
+    try {
+      if (!isDraftVisible(db, cd)) {
+        return ruleError("Patch set " + patchSet.getPatchSetId() + " not found");
+      } else if (patchSet.isDraft()) {
+        return ruleError("Cannot submit draft patch sets");
+      } else {
+        return ruleError("Cannot submit draft changes");
+      }
+    } catch (OrmException err) {
+      return logRuleError("Cannot read patch set " + patchSet.getId(), err);
+    }
+  }
+
   /**
    * Convert the results from Prolog Cafe's format to Gerrit's common format.
    *
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 ad266ab..506fabc 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
@@ -388,7 +388,14 @@
     return ImmutableList.copyOf(allApprovalsMap(db).values());
   }
 
-  private ListMultimap<PatchSet.Id, PatchSetApproval> allApprovalsMap(
+  /**
+   * @param db review database.
+   * @return all patch set approvals for the change (regardless of whether
+   *     {@link #limitToPatchSets(Collection)} was previously called), keyed by
+   *     ID, ordered by timestamp within each patch set.
+   * @throws OrmException an error occurred reading the database.
+   */
+  public ListMultimap<PatchSet.Id, PatchSetApproval> allApprovalsMap(
       Provider<ReviewDb> db) throws OrmException {
     if (allApprovals == null) {
       allApprovals = ArrayListMultimap.create();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
index c781d97..fc35df3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
@@ -283,7 +283,7 @@
             PatchSet.Id psId = d.getChange().currentPatchSetId();
             PatchSet patchSet = db.get().patchSets().get(psId);
             List<SubmitRecord> submitResult = d.changeControl().canSubmit( //
-                db.get(), patchSet, null, false, true);
+                db.get(), patchSet, null, false, true, true);
             eventFactory.addSubmitRecords(c, submitResult);
           }
 
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java
new file mode 100644
index 0000000..21d1ce4
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java
@@ -0,0 +1,224 @@
+// Copyright (C) 2013 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.change;
+
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.replay;
+
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Patch;
+import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.server.PatchLineCommentAccess;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.account.AccountInfo;
+import com.google.gerrit.server.change.CommentInfo.Side;
+import com.google.gwtorm.server.ListResultSet;
+import com.google.gwtorm.server.ResultSet;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.TypeLiteral;
+
+import junit.framework.TestCase;
+
+import org.easymock.IAnswer;
+
+import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+public class CommentsTest extends TestCase {
+
+  private Injector injector;
+  private RevisionResource revRes1;
+  private RevisionResource revRes2;
+  private PatchLineComment plc1;
+  private PatchLineComment plc2;
+  private PatchLineComment plc3;
+
+  @Override
+  protected void setUp() throws Exception {
+    @SuppressWarnings("unchecked")
+    final DynamicMap<RestView<CommentResource>> views =
+        createMock(DynamicMap.class);
+    final TypeLiteral<DynamicMap<RestView<CommentResource>>> viewsType =
+        new TypeLiteral<DynamicMap<RestView<CommentResource>>>() {};
+    final AccountInfo.Loader.Factory alf =
+        createMock(AccountInfo.Loader.Factory.class);
+    final ReviewDb db = createMock(ReviewDb.class);
+
+    AbstractModule mod = new AbstractModule() {
+      @Override
+      protected void configure() {
+        bind(viewsType).toInstance(views);
+        bind(AccountInfo.Loader.Factory.class).toInstance(alf);
+        bind(ReviewDb.class).toInstance(db);
+      }};
+
+    Account.Id account1 = new Account.Id(1);
+    Account.Id account2 = new Account.Id(2);
+    AccountInfo.Loader accountLoader = createMock(AccountInfo.Loader.class);
+    accountLoader.fill();
+    expectLastCall().anyTimes();
+    expect(accountLoader.get(account1))
+        .andReturn(new AccountInfo(account1)).anyTimes();
+    expect(accountLoader.get(account2))
+        .andReturn(new AccountInfo(account2)).anyTimes();
+    expect(alf.create(true)).andReturn(accountLoader).anyTimes();
+    replay(accountLoader, alf);
+
+    revRes1 = createMock(RevisionResource.class);
+    revRes2 = createMock(RevisionResource.class);
+
+    PatchLineCommentAccess plca = createMock(PatchLineCommentAccess.class);
+    expect(db.patchComments()).andReturn(plca).anyTimes();
+
+    Change.Id changeId = new Change.Id(123);
+    PatchSet.Id psId1 = new PatchSet.Id(changeId, 1);
+    PatchSet ps1 = new PatchSet(psId1);
+    expect(revRes1.getPatchSet()).andReturn(ps1).anyTimes();
+    PatchSet.Id psId2 = new PatchSet.Id(changeId, 2);
+    PatchSet ps2 = new PatchSet(psId2);
+    expect(revRes2.getPatchSet()).andReturn(ps2);
+
+    long timeBase = System.currentTimeMillis();
+    plc1 = newPatchLineComment(psId1, "Comment1", null,
+        "FileOne.txt", Side.REVISION, 1, account1, timeBase,
+        "First Comment");
+    plc2 = newPatchLineComment(psId1, "Comment2", "Comment1",
+        "FileOne.txt", Side.REVISION, 1, account2, timeBase + 1000,
+        "Reply to First Comment");
+    plc3 = newPatchLineComment(psId1, "Comment3", "Comment1",
+        "FileOne.txt", Side.PARENT, 1, account1, timeBase + 2000,
+        "First Parent Comment");
+
+    expect(plca.publishedByPatchSet(psId1))
+        .andAnswer(results(plc1, plc2, plc3)).anyTimes();
+    expect(plca.publishedByPatchSet(psId2))
+        .andAnswer(results()).anyTimes();
+
+    replay(db, revRes1, revRes2, plca);
+    injector = Guice.createInjector(mod);
+  }
+
+  public void testListComments() throws Exception {
+    // test ListComments for patch set 1
+    assertListComments(injector, revRes1, ImmutableMap.of(
+        "FileOne.txt", Lists.newArrayList(plc3, plc1, plc2)));
+
+    // test ListComments for patch set 2
+    assertListComments(injector, revRes2,
+        Collections.<String, ArrayList<PatchLineComment>>emptyMap());
+  }
+
+  public void testGetComment() throws Exception {
+    // test GetComment for existing comment
+    assertGetComment(injector, revRes1, plc1, plc1.getKey().get());
+
+    // test GetComment for non-existent comment
+    assertGetComment(injector, revRes1, null, "BadComment");
+  }
+
+  private static IAnswer<ResultSet<PatchLineComment>> results(
+      final PatchLineComment... comments) {
+    return new IAnswer<ResultSet<PatchLineComment>>() {
+      @Override
+      public ResultSet<PatchLineComment> answer() throws Throwable {
+        return new ListResultSet<PatchLineComment>(Lists.newArrayList(comments));
+      }};
+  }
+
+  private static void assertGetComment(Injector inj, RevisionResource res,
+      PatchLineComment expected, String uuid) throws Exception {
+    GetComment getComment = inj.getInstance(GetComment.class);
+    Comments comments = inj.getInstance(Comments.class);
+    try {
+      CommentResource commentRes = comments.parse(res, IdString.fromUrl(uuid));
+      if (expected == null) {
+        fail("Expected no comment");
+      }
+      CommentInfo actual = (CommentInfo) getComment.apply(commentRes);
+      assertComment(expected, actual);
+    } catch (ResourceNotFoundException e) {
+      if (expected != null) {
+        fail("Expected to find comment");
+      }
+    }
+  }
+
+  private static void assertListComments(Injector inj, RevisionResource res,
+      Map<String, ArrayList<PatchLineComment>> expected) throws Exception {
+    Comments comments = inj.getInstance(Comments.class);
+    RestReadView<RevisionResource> listView =
+        (RestReadView<RevisionResource>) comments.list();
+    @SuppressWarnings("unchecked")
+    Map<String, List<CommentInfo>> actual =
+        (Map<String, List<CommentInfo>>) listView.apply(res);
+    assertNotNull(actual);
+    assertEquals(expected.size(), actual.size());
+    assertEquals(expected.keySet(), actual.keySet());
+    for (String filename : expected.keySet()) {
+      List<PatchLineComment> expectedComments = expected.get(filename);
+      List<CommentInfo> actualComments = actual.get(filename);
+      assertNotNull(actualComments);
+      assertEquals(expectedComments.size(), actualComments.size());
+      for (int i = 0; i < expectedComments.size(); i++) {
+        assertComment(expectedComments.get(i), actualComments.get(i));
+      }
+    }
+  }
+
+  private static void assertComment(PatchLineComment plc, CommentInfo ci) {
+    assertEquals(plc.getKey().get(), ci.id);
+    assertEquals(plc.getParentUuid(), ci.inReplyTo);
+    assertEquals("gerritcodereview#comment", ci.kind);
+    assertEquals(plc.getMessage(), ci.message);
+    assertNotNull(ci.author);
+    assertEquals(plc.getAuthor(), ci.author._id);
+    assertEquals(plc.getLine(), (int) ci.line);
+    assertEquals(plc.getSide() == 0 ? Side.PARENT : Side.REVISION,
+        Objects.firstNonNull(ci.side, Side.REVISION));
+    assertEquals(plc.getWrittenOn(), ci.updated);
+  }
+
+  private static PatchLineComment newPatchLineComment(PatchSet.Id psId,
+      String uuid, String inReplyToUuid, String filename, Side side, int line,
+      Account.Id authorId, long millis, String message) {
+    Patch.Key p = new Patch.Key(psId, filename);
+    PatchLineComment.Key id = new PatchLineComment.Key(p, uuid);
+    PatchLineComment plc =
+        new PatchLineComment(id, line, authorId, inReplyToUuid);
+    plc.setMessage(message);
+    plc.setSide(side == CommentInfo.Side.PARENT ? (short) 0 : (short) 1);
+    plc.setStatus(Status.PUBLISHED);
+    plc.setWrittenOn(new Timestamp(millis));
+    return plc;
+  }
+}