Merge branch 'stable-2.16' into stable-3.0

* stable-2.16:
  Revert "Migrate from old-style legacy .java provider to the new JavaInfo."
  Catch all exceptions for reporting on Schema_130 migration
  Migrate from old-style legacy .java provider to the new JavaInfo.
  Update git submodules
  AuthRequest: Fix Javadoc for return values
  ChangeApi: Add methods to get change comments/drafts as list
  Update git submodules
  ListChangeComments: Extend ListChangeDrafts
  Upgrade Go Bazel rules to the latest version
  detach -> detached
  Fix anchor tag for settings page

Change-Id: I248de55870ecfa2583533cd63ed91874550b6b67

WORKSPACE

@@ -83,8 +83,11 @@ closure_repositories(
 # Golang support for PolyGerrit local dev server.
 http_archive(
     name = "io_bazel_rules_go",
-    sha256 = "6776d68ebb897625dead17ae510eac3d5f6342367327875210df44dbe2aeeb19",
+    sha256 = "f04d2373bcaf8aa09bccb08a98a57e721306c8f6043a2a0ee610fd6853dcde3d",
-    url = "https://github.com/bazelbuild/rules_go/releases/download/0.17.1/rules_go-0.17.1.tar.gz",
+    urls = [
+        "https://storage.googleapis.com/bazel-mirror/github.com/bazelbuild/rules_go/releases/download/0.18.6/rules_go-0.18.6.tar.gz",
+        "https://github.com/bazelbuild/rules_go/releases/download/0.18.6/rules_go-0.18.6.tar.gz",
+    ],
 )
 
 load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")

java/com/google/gerrit/extensions/api/changes/ChangeApi.java

@@ -326,6 +326,15 @@ public interface ChangeApi {
    */
   Map<String, List<CommentInfo>> comments() throws RestApiException;
 
+  /**
+   * Get all published comments on a change as a list.
+   *
+   * @return comments as a list; comments have the {@code revision} field set to indicate their
+   *     patch set.
+   * @throws RestApiException
+   */
+  List<CommentInfo> commentsAsList() throws RestApiException;
+
   /**
    * Get all robot comments on a change.
    *
@@ -344,6 +353,15 @@ public interface ChangeApi {
    */
   Map<String, List<CommentInfo>> drafts() throws RestApiException;
 
+  /**
+   * Get all draft comments for the current user on a change as a list.
+   *
+   * @return drafts as a list; comments have the {@code revision} field set to indicate their patch
+   *     set.
+   * @throws RestApiException
+   */
+  List<CommentInfo> draftsAsList() throws RestApiException;
+
   ChangeInfo check() throws RestApiException;
 
   ChangeInfo check(FixInput fix) throws RestApiException;
@@ -551,6 +569,11 @@ public interface ChangeApi {
       throw new NotImplementedException();
     }
 
+    @Override
+    public List<CommentInfo> commentsAsList() throws RestApiException {
+      throw new NotImplementedException();
+    }
+
     @Override
     public Map<String, List<RobotCommentInfo>> robotComments() throws RestApiException {
       throw new NotImplementedException();
@@ -561,6 +584,11 @@ public interface ChangeApi {
       throw new NotImplementedException();
     }
 
+    @Override
+    public List<CommentInfo> draftsAsList() throws RestApiException {
+      throw new NotImplementedException();
+    }
+
     @Override
     public ChangeInfo check() throws RestApiException {
       throw new NotImplementedException();

java/com/google/gerrit/server/api/changes/ChangeApiImpl.java

@@ -556,6 +556,15 @@ class ChangeApiImpl implements ChangeApi {
     }
   }
 
+  @Override
+  public List<CommentInfo> commentsAsList() throws RestApiException {
+    try {
+      return listComments.getComments(change);
+    } catch (Exception e) {
+      throw asRestApiException("Cannot get comments", e);
+    }
+  }
+
   @Override
   public Map<String, List<RobotCommentInfo>> robotComments() throws RestApiException {
     try {
@@ -574,6 +583,15 @@ class ChangeApiImpl implements ChangeApi {
     }
   }
 
+  @Override
+  public List<CommentInfo> draftsAsList() throws RestApiException {
+    try {
+      return listDrafts.getComments(change);
+    } catch (Exception e) {
+      throw asRestApiException("Cannot get drafts", e);
+    }
+  }
+
   @Override
   public ChangeInfo check() throws RestApiException {
     try {

java/com/google/gerrit/server/auth/AuthRequest.java

@@ -31,7 +31,7 @@ public abstract class AuthRequest {
   /**
    * Returns the username to be authenticated.
    *
-   * @return username for authentication or null for anonymous access.
+   * @return username for authentication or {@code empty} for anonymous access.
    */
   public final Optional<String> getUsername() {
     return username;
@@ -40,7 +40,7 @@ public abstract class AuthRequest {
   /**
    * Returns the user's credentials
    *
-   * @return user's credentials or null
+   * @return user's credentials or {@code empty}.
    */
   public final Optional<String> getPassword() {
     return password;

java/com/google/gerrit/server/restapi/change/ListChangeComments.java

@@ -14,44 +14,37 @@
 
 package com.google.gerrit.server.restapi.change;
 
-import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.reviewdb.client.Comment;
-import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.server.CommentsUtil;
 import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
-import java.util.List;
-import java.util.Map;
 
 @Singleton
-public class ListChangeComments implements RestReadView<ChangeResource> {
+public class ListChangeComments extends ListChangeDrafts {
-  private final ChangeData.Factory changeDataFactory;
-  private final Provider<CommentJson> commentJson;
-  private final CommentsUtil commentsUtil;
-
   @Inject
   ListChangeComments(
       ChangeData.Factory changeDataFactory,
       Provider<CommentJson> commentJson,
       CommentsUtil commentsUtil) {
-    this.changeDataFactory = changeDataFactory;
+    super(changeDataFactory, commentJson, commentsUtil);
-    this.commentJson = commentJson;
-    this.commentsUtil = commentsUtil;
   }
 
   @Override
-  public Map<String, List<CommentInfo>> apply(ChangeResource rsrc)
+  protected Iterable<Comment> listComments(ChangeResource rsrc) {
-      throws AuthException, PermissionBackendException {
     ChangeData cd = changeDataFactory.create(rsrc.getNotes());
-    return commentJson
+    return commentsUtil.publishedByChange(cd.notes());
-        .get()
+  }
-        .setFillAccounts(true)
+
-        .setFillPatchSet(true)
+  @Override
-        .newCommentFormatter()
+  protected boolean includeAuthorInfo() {
-        .format(commentsUtil.publishedByChange(cd.notes()));
+    return true;
+  }
+
+  @Override
+  public boolean requireAuthentication() {
+    return false;
   }
 }

java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java

@@ -22,6 +22,7 @@ import com.google.gerrit.server.CommentsUtil;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.restapi.change.CommentJson.CommentFormatter;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -30,9 +31,9 @@ import java.util.Map;
 
 @Singleton
 public class ListChangeDrafts implements RestReadView<ChangeResource> {
-  private final ChangeData.Factory changeDataFactory;
+  protected final ChangeData.Factory changeDataFactory;
-  private final Provider<CommentJson> commentJson;
+  protected final Provider<CommentJson> commentJson;
-  private final CommentsUtil commentsUtil;
+  protected final CommentsUtil commentsUtil;
 
   @Inject
   ListChangeDrafts(
@@ -44,20 +45,41 @@ public class ListChangeDrafts implements RestReadView<ChangeResource> {
     this.commentsUtil = commentsUtil;
   }
 
+  protected Iterable<Comment> listComments(ChangeResource rsrc) {
+    ChangeData cd = changeDataFactory.create(rsrc.getNotes());
+    return commentsUtil.draftByChangeAuthor(cd.notes(), rsrc.getUser().getAccountId());
+  }
+
+  protected boolean includeAuthorInfo() {
+    return false;
+  }
+
+  public boolean requireAuthentication() {
+    return true;
+  }
+
   @Override
   public Map<String, List<CommentInfo>> apply(ChangeResource rsrc)
       throws AuthException, PermissionBackendException {
-    if (!rsrc.getUser().isIdentifiedUser()) {
+    if (requireAuthentication() && !rsrc.getUser().isIdentifiedUser()) {
       throw new AuthException("Authentication required");
     }
-    ChangeData cd = changeDataFactory.create(rsrc.getNotes());
+    return getCommentFormatter().format(listComments(rsrc));
-    List<Comment> drafts =
+  }
-        commentsUtil.draftByChangeAuthor(cd.notes(), rsrc.getUser().getAccountId());
+
+  public List<CommentInfo> getComments(ChangeResource rsrc)
+      throws AuthException, PermissionBackendException {
+    if (requireAuthentication() && !rsrc.getUser().isIdentifiedUser()) {
+      throw new AuthException("Authentication required");
+    }
+    return getCommentFormatter().formatAsList(listComments(rsrc));
+  }
+
+  private CommentFormatter getCommentFormatter() {
     return commentJson
         .get()
-        .setFillAccounts(false)
+        .setFillAccounts(includeAuthorInfo())
         .setFillPatchSet(true)
-        .newCommentFormatter()
+        .newCommentFormatter();
-        .format(drafts);
   }
 }

javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java

@@ -111,6 +111,11 @@ public class CommentsIT extends AbstractDaemonTest {
       assertThat(result).hasSize(1);
       CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
       assertThat(comment).isEqualTo(infoToDraft(path).apply(actual));
+
+      List<CommentInfo> list = getDraftCommentsAsList(changeId);
+      assertThat(list).hasSize(1);
+      actual = list.get(0);
+      assertThat(comment).isEqualTo(infoToDraft(path).apply(actual));
     }
   }
 
@@ -133,6 +138,10 @@ public class CommentsIT extends AbstractDaemonTest {
       assertThat(result).hasSize(1);
       assertThat(Lists.transform(result.get(path), infoToDraft(path)))
           .containsExactly(c1, c2, c3, c4);
+
+      List<CommentInfo> list = getDraftCommentsAsList(changeId);
+      assertThat(list).hasSize(4);
+      assertThat(Lists.transform(list, infoToDraft(path))).containsExactly(c1, c2, c3, c4);
     }
   }
 
@@ -235,6 +244,9 @@ public class CommentsIT extends AbstractDaemonTest {
       assertThat(result).isNotEmpty();
       assertThat(Lists.transform(result.get(file), infoToInput(file)))
           .containsExactly(c1, c2, c3, c4);
+
+      List<CommentInfo> list = getPublishedCommentsAsList(changeId);
+      assertThat(Lists.transform(list, infoToInput(file))).containsExactly(c1, c2, c3, c4);
     }
 
     // for the commit message comments on the auto-merge are not possible
@@ -253,6 +265,9 @@ public class CommentsIT extends AbstractDaemonTest {
       Map<String, List<CommentInfo>> result = getPublishedComments(changeId, revId);
       assertThat(result).isNotEmpty();
       assertThat(Lists.transform(result.get(file), infoToInput(file))).containsExactly(c1, c2, c3);
+
+      List<CommentInfo> list = getPublishedCommentsAsList(changeId);
+      assertThat(Lists.transform(list, infoToInput(file))).containsExactly(c1, c2, c3);
     }
   }
 
@@ -277,6 +292,7 @@ public class CommentsIT extends AbstractDaemonTest {
     String changeId = r.getChangeId();
     String revId = r.getCommit().getName();
     assertThat(getPublishedComments(changeId, revId)).isEmpty();
+    assertThat(getPublishedCommentsAsList(changeId)).isEmpty();
 
     List<CommentInput> expectedComments = new ArrayList<>();
     for (Integer line : lines) {
@@ -293,6 +309,10 @@ public class CommentsIT extends AbstractDaemonTest {
     List<CommentInfo> actualComments = result.get(file);
     assertThat(Lists.transform(actualComments, infoToInput(file)))
         .containsExactlyElementsIn(expectedComments);
+
+    List<CommentInfo> list = getPublishedCommentsAsList(changeId);
+    assertThat(Lists.transform(list, infoToInput(file)))
+        .containsExactlyElementsIn(expectedComments);
   }
 
   @Test
@@ -1093,11 +1113,19 @@ public class CommentsIT extends AbstractDaemonTest {
     return gApi.changes().id(changeId).revision(revId).comments();
   }
 
+  private List<CommentInfo> getPublishedCommentsAsList(String changeId) throws Exception {
+    return gApi.changes().id(changeId).commentsAsList();
+  }
+
   private Map<String, List<CommentInfo>> getDraftComments(String changeId, String revId)
       throws Exception {
     return gApi.changes().id(changeId).revision(revId).drafts();
   }
 
+  private List<CommentInfo> getDraftCommentsAsList(String changeId) throws Exception {
+    return gApi.changes().id(changeId).draftsAsList();
+  }
+
   private CommentInfo getDraftComment(String changeId, String revId, String uuid) throws Exception {
     return gApi.changes().id(changeId).revision(revId).draft(uuid).get();
   }

polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js

@@ -158,6 +158,9 @@
     ],
 
     attached() {
+      // Polymer 2: anchor tag won't work on shadow DOM
+      // we need to manually calling scrollIntoView when hash changed
+      this.listen(window, 'location-change', '_handleLocationChange');
       this.fire('title-change', {title: 'Settings'});
 
       this._isDark = !!window.localStorage.getItem('dark-theme');
@@ -214,9 +217,28 @@
 
       this._loadingPromise = Promise.all(promises).then(() => {
         this._loading = false;
+
+        // Handle anchor tag for initial load
+        this._handleLocationChange();
       });
     },
 
+    detached() {
+      this.unlisten(window, 'location-change', '_handleLocationChange');
+    },
+
+    _handleLocationChange() {
+      // Handle anchor tag after dom attached
+      const urlHash = window.location.hash;
+      if (urlHash) {
+        // Use shadowRoot for Polymer 2
+        const elem = (this.shadowRoot || document).querySelector(urlHash);
+        if (elem) {
+          elem.scrollIntoView();
+        }
+      }
+    },
+
     reloadAccountDetail() {
       Promise.all([
         this.$.accountInfo.loadData(),