Merge branch 'stable-3.7' into stable-3.8

* stable-3.7:
  build: Disable ErrorProne warnings
  Remove drafts and stars from the ref_state_pattern index field
  Update git submodules
  Consistently normalize PatchSetApprovals when persisting
  Remove unused method ChangeNotes.containsComment()
  Update JGit to acf21c0bc
  Remove the useIndex option in SSH commands change id parsing
  Set version to 3.7.8-SNAPSHOT
  Set version to 3.7.7

Release-Notes: skip
Change-Id: I9caee7225ccd2cc4b094c9312bfc9115e49cd717
Forward-Compatible: checked

5 files changed, 38 insertions, 52 deletions

diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 7057ff7351..175c0fa1ca 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -65,7 +65,6 @@ import com.google.gerrit.server.ReviewerByEmailSet;
 import com.google.gerrit.server.ReviewerSet;
 import com.google.gerrit.server.StarredChangesUtil;
 import com.google.gerrit.server.cache.proto.Cache;
-import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern;
 import com.google.gerrit.server.notedb.ReviewerStateInternal;
 import com.google.gerrit.server.notedb.SubmitRequirementProtoConverter;
@@ -1697,12 +1696,6 @@ public class ChangeField {
                     RefStatePattern.create(
                             RefNames.REFS_USERS + "*/" + RefNames.EDIT_PREFIX + id + "/*")
                         .toByteArray(project));
-                result.add(
-                    RefStatePattern.create(RefNames.refsStarredChangesPrefix(id) + "*")
-                        .toByteArray(allUsers(cd)));
-                result.add(
-                    RefStatePattern.create(RefNames.refsDraftCommentsPrefix(id) + "*")
-                        .toByteArray(allUsers(cd)));
                 return result;
               },
               (cd, field) -> cd.setRefStatePatterns(field));
@@ -1747,10 +1740,6 @@ public class ChangeField {
     return in -> in.change() != null ? func.apply(in.change()) : null;
   }
 
-  private static AllUsersName allUsers(ChangeData cd) {
-    return cd.getAllUsersNameForIndexing();
-  }
-
   private static String truncateStringValueToMaxTermLength(String str) {
     return truncateStringValue(str, MAX_TERM_LENGTH);
   }
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index c5d2428198..7d3b3ca5d7 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -587,14 +587,6 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     return robotCommentNotes;
   }
 
-  public boolean containsComment(HumanComment c) {
-    if (containsCommentPublished(c)) {
-      return true;
-    }
-    loadDraftComments(c.author.getId(), null);
-    return draftCommentNotes.containsComment(c);
-  }
-
   public boolean containsCommentPublished(Comment c) {
     for (Comment l : getHumanComments().values()) {
       if (c.key.equals(l.key)) {
diff --git a/java/com/google/gerrit/sshd/ChangeArgumentParser.java b/java/com/google/gerrit/sshd/ChangeArgumentParser.java
index 92019ad8a3..3e9dd082f4 100644
--- a/java/com/google/gerrit/sshd/ChangeArgumentParser.java
+++ b/java/com/google/gerrit/sshd/ChangeArgumentParser.java
@@ -18,7 +18,7 @@ import com.google.gerrit.common.Nullable;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.change.ChangeFinder;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.notedb.ChangeNotes;
@@ -30,27 +30,27 @@ import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.restapi.change.ChangesCollection;
 import com.google.gerrit.sshd.BaseCommand.UnloggedFailure;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
 public class ChangeArgumentParser {
   private final ChangesCollection changesCollection;
   private final ChangeFinder changeFinder;
-  private final ChangeNotes.Factory changeNotesFactory;
   private final PermissionBackend permissionBackend;
+  private final Provider<CurrentUser> currentUserProvider;
 
   @Inject
   ChangeArgumentParser(
       ChangesCollection changesCollection,
       ChangeFinder changeFinder,
-      ChangeNotes.Factory changeNotesFactory,
-      PermissionBackend permissionBackend) {
+      PermissionBackend permissionBackend,
+      Provider<CurrentUser> currentUserProvider) {
     this.changesCollection = changesCollection;
     this.changeFinder = changeFinder;
-    this.changeNotesFactory = changeNotesFactory;
     this.permissionBackend = permissionBackend;
+    this.currentUserProvider = currentUserProvider;
   }
 
   public void addChange(String id, Map<Change.Id, ChangeResource> changes)
@@ -68,9 +68,22 @@ public class ChangeArgumentParser {
       String id,
       Map<Change.Id, ChangeResource> changes,
       @Nullable ProjectState projectState,
-      boolean useIndex)
+      @SuppressWarnings(
+              "unused") /* Issue 325821304: the useIndex parameter was introduced back in Gerrit
+                         * v2.13
+                         * when ReviewDb was around and the changeFinder was purely relying on
+                         * Lucene.
+                         * Fast-forward to v3.7 and the situation is exactly the opposite:
+                         * changeFinder uses Lucene or NoteDb depending on the format of the
+                         * change id.
+                         * TODO: The useIndex is effectively useless right now, but the method
+                         * signature needs to be preserved in a stable (almost EOL) release
+                         * like v3.7.
+                         * The method signature can be amended the parameter removed once this
+                         * change is merged to master. */
+          boolean useIndex)
       throws UnloggedFailure, PermissionBackendException {
-    List<ChangeNotes> matched = useIndex ? changeFinder.find(id) : changeFromNotesFactory(id);
+    List<ChangeNotes> matched = changeFinder.find(id);
     List<ChangeNotes> toAdd = new ArrayList<>(changes.size());
     boolean canMaintainServer;
     try {
@@ -105,26 +118,10 @@ public class ChangeArgumentParser {
     } else if (toAdd.size() > 1) {
       throw new UnloggedFailure(1, "\"" + id + "\" matches multiple changes");
     }
-    Change.Id cId = toAdd.get(0).getChangeId();
+    ChangeNotes changeNotes = toAdd.get(0);
     ChangeResource changeResource;
-    try {
-      changeResource = changesCollection.parse(cId);
-    } catch (RestApiException e) {
-      throw new UnloggedFailure(1, "\"" + id + "\" no such change", e);
-    }
-    changes.put(cId, changeResource);
-  }
-
-  private List<ChangeNotes> changeFromNotesFactory(String id) throws UnloggedFailure {
-    return changeNotesFactory.createUsingIndexLookup(parseId(id));
-  }
-
-  private List<Change.Id> parseId(String id) throws UnloggedFailure {
-    try {
-      return Arrays.asList(Change.id(Integer.parseInt(id)));
-    } catch (NumberFormatException e) {
-      throw new UnloggedFailure(2, "Invalid change ID " + id, e);
-    }
+    changeResource = changesCollection.parse(changeNotes, currentUserProvider.get());
+    changes.put(changeNotes.getChangeId(), changeResource);
   }
 
   private boolean inProject(ProjectState projectState, Project.NameKey project) {
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 5d8394919c..d654e81eb5 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -3448,6 +3448,14 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     assertQuery("reviewer:self", change);
     assertThat(indexer.reindexIfStale(project, change.getId()).get()).isTrue();
     assertQuery("reviewer:self");
+
+    // Index is not stale when a draft comment exists
+    DraftInput in = new DraftInput();
+    in.line = 1;
+    in.message = "nit: trailing whitespace";
+    in.path = Patch.COMMIT_MSG;
+    gApi.changes().id(project.get(), change.getId().get()).current().createDraft(in);
+    assertThat(indexer.reindexIfStale(project, change.getId()).get()).isFalse();
   }
 
   @Test
diff --git a/tools/BUILD b/tools/BUILD
index 70d431509f..b6a35726fb 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -76,7 +76,7 @@ java_package_configuration(
         "-Xep:BadImport:ERROR",
         "-Xep:BadInstanceof:ERROR",
         "-Xep:BadShiftAmount:ERROR",
-        "-Xep:BanJNDI:WARN",
+        "-Xep:BanJNDI:OFF",
         "-Xep:BanSerializableRead:ERROR",
         "-Xep:BigDecimalEquals:ERROR",
         "-Xep:BigDecimalLiteralDouble:ERROR",
@@ -159,7 +159,7 @@ java_package_configuration(
         "-Xep:FloatingPointLiteralPrecision:ERROR",
         "-Xep:FloggerArgumentToString:ERROR",
         "-Xep:FloggerFormatString:ERROR",
-        "-Xep:FloggerLogString:WARN",
+        "-Xep:FloggerLogString:OFF",
         "-Xep:FloggerLogVarargs:ERROR",
         "-Xep:FloggerSplitLogStatement:ERROR",
         "-Xep:FloggerStringConcatenation:ERROR",
@@ -189,7 +189,7 @@ java_package_configuration(
         "-Xep:ImmutableAnnotationChecker:ERROR",
         "-Xep:ImmutableEnumChecker:ERROR",
         "-Xep:ImmutableModification:ERROR",
-        "-Xep:ImpossibleNullComparison:WARN",
+        "-Xep:ImpossibleNullComparison:OFF",
         "-Xep:Incomparable:ERROR",
         "-Xep:IncompatibleArgumentType:ERROR",
         "-Xep:IncompatibleModifiers:ERROR",
@@ -258,7 +258,7 @@ java_package_configuration(
         "-Xep:JodaWithDurationAddedLong:ERROR",
         "-Xep:LiteByteStringUtf8:ERROR",
         "-Xep:LiteEnumValueOf:ERROR",
-        "-Xep:LiteProtoToString:WARN",
+        "-Xep:LiteProtoToString:OFF",
         "-Xep:LocalDateTemporalAmount:ERROR",
         "-Xep:LockNotBeforeTry:ERROR",
         "-Xep:LockOnBoxedPrimitive:ERROR",
@@ -290,7 +290,7 @@ java_package_configuration(
         "-Xep:MultipleParallelOrSequentialCalls:ERROR",
         "-Xep:MultipleUnaryOperatorsInMethodCall:ERROR",
         "-Xep:MustBeClosedChecker:ERROR",
-        "-Xep:MutableConstantField:WARN",
+        "-Xep:MutableConstantField:OFF",
         "-Xep:MutablePublicArray:ERROR",
         "-Xep:NCopiesOfChar:ERROR",
         "-Xep:NarrowingCompoundAssignment:ERROR",
@@ -343,7 +343,7 @@ java_package_configuration(
         "-Xep:ProtoTimestampGetSecondsGetNano:ERROR",
         "-Xep:ProtoTruthMixedDescriptors:ERROR",
         "-Xep:ProtocolBufferOrdinal:ERROR",
-        "-Xep:ProvidesMethodOutsideOfModule:WARN",
+        "-Xep:ProvidesMethodOutsideOfModule:OFF",
         "-Xep:RandomCast:ERROR",
         "-Xep:RandomModInteger:ERROR",
         "-Xep:ReachabilityFenceUsage:ERROR",