Merge branch 'stable-3.5' into stable-3.6

* stable-3.5:
  ChangeNotes.scanChangeIds: Return metaId with ChangeIds
  ChangeNotes: Stop recording scanned patchSetRefs
  Fix Bazel build on Apple M2 ARM64 chip
  Work around build errors on MacOS 13.3 and XCode 14.3
  Bump bazel version
  Bazel: Switch to using toolchain resolution for java rules
  Update git submodules
  Bazel: Adapt gerrit_plugin rule to removal of resource_jars attribute
  Demote some error prone bug pattern to allow Bazel update
  Fix SameNameButDifferent bug pattern flagged by error prone
  ResourceServletTest: Remove unused method
  Suppress unused method errors flagged by error prone
  JavaCacheSerializer: Ignore BanSerializableRead bug pattern
  Fix ReturnValueIgnored bug pattern flagged by error prone
  Fix UnnecessaryMethodReference bug pattern flagged by error prone
  Fix "Redundant specification of type arguments" warning
  Fix ObjectEqualsForPrimitives bug pattern flagged by error prone
  Fix InlineMeSuggester bug pattern flagged by error prone
  Bump error-prone annotations version to 2.10.0
  Fix FloggerArgumentToString bug pattern flagged by error prone
  Fix default encoding issues flagged by error prone
  Fix Flogger issues flagged by error prone
  Fix UnnecessaryAssignment bug pattern flagged by error prone
  Demote JavaUtilDate error prone bug pattern to warning severity
  Bazel: Simplify java 11 toolchain definition
  Revert "Bazel: Use javadoc from host java runtime"
  Document that building with Java 8 is no longer supported
  Fix parsing legacy labels for users with comma
  Don't allow index searches with size greater than maxPageSize

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

4 files changed, 48 insertions, 49 deletions

diff --git a/java/com/google/gerrit/index/QueryOptions.java b/java/com/google/gerrit/index/QueryOptions.java
index 91c8d1a5e5..29ab6d0119 100644
--- a/java/com/google/gerrit/index/QueryOptions.java
+++ b/java/com/google/gerrit/index/QueryOptions.java
@@ -73,7 +73,10 @@ public abstract class QueryOptions {
     int backendLimit = config().maxLimit();
     int limit = Ints.saturatedCast((long) limit() + start());
     limit = Math.min(limit, backendLimit);
-    int pageSize = Math.min(Ints.saturatedCast((long) pageSize() + start()), backendLimit);
+    int pageSize =
+        Math.min(
+            Math.min(Ints.saturatedCast((long) pageSize() + start()), config().maxPageSize()),
+            backendLimit);
     return create(config(), 0, null, pageSize, pageSizeMultiplier(), limit, fields());
   }
 
diff --git a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
index 99dacd95ff..ace3d6c1d2 100644
--- a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
+++ b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
@@ -21,6 +21,7 @@ import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.common.util.concurrent.ListenableFuture;
@@ -38,7 +39,6 @@ import com.google.gerrit.server.index.IndexExecutor;
 import com.google.gerrit.server.index.OnlineReindexMode;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult;
-import com.google.gerrit.server.notedb.ChangeNotes.Factory.ScanResult;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.inject.Inject;
@@ -49,6 +49,7 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Repository;
 
@@ -107,10 +108,14 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
 
     public abstract int slices();
 
-    public abstract ScanResult scanResult();
+    public abstract ImmutableMap<Change.Id, ObjectId> metaIdByChange();
 
-    private static ProjectSlice create(Project.NameKey name, int slice, int slices, ScanResult sr) {
-      return new AutoValue_AllChangesIndexer_ProjectSlice(name, slice, slices, sr);
+    private static ProjectSlice create(
+        Project.NameKey name,
+        int slice,
+        int slices,
+        ImmutableMap<Change.Id, ObjectId> metaIdByChange) {
+      return new AutoValue_AllChangesIndexer_ProjectSlice(name, slice, slices, metaIdByChange);
     }
   }
 
@@ -191,10 +196,10 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
       Project.NameKey project,
       int slice,
       int slices,
-      ScanResult scanResult,
+      ImmutableMap<Change.Id, ObjectId> metaIdByChange,
       Task done,
       Task failed) {
-    return new ProjectIndexer(indexer, project, slice, slices, scanResult, done, failed);
+    return new ProjectIndexer(indexer, project, slice, slices, metaIdByChange, done, failed);
   }
 
   private class ProjectIndexer implements Callable<Void> {
@@ -202,7 +207,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
     private final Project.NameKey project;
     private final int slice;
     private final int slices;
-    private final ScanResult scanResult;
+    private final ImmutableMap<Change.Id, ObjectId> metaIdByChange;
     private final ProgressMonitor done;
     private final ProgressMonitor failed;
 
@@ -211,14 +216,14 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
         Project.NameKey project,
         int slice,
         int slices,
-        ScanResult scanResult,
+        ImmutableMap<Change.Id, ObjectId> metaIdByChange,
         ProgressMonitor done,
         ProgressMonitor failed) {
       this.indexer = indexer;
       this.project = project;
       this.slice = slice;
       this.slices = slices;
-      this.scanResult = scanResult;
+      this.metaIdByChange = metaIdByChange;
       this.done = done;
       this.failed = failed;
     }
@@ -232,7 +237,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
       // but the goal is to invalidate that cache as infrequently as we possibly can. And besides,
       // we don't have concrete proof that improving packfile locality would help.
       notesFactory
-          .scan(scanResult, project, id -> (id.get() % slices) == slice)
+          .scan(metaIdByChange, project, id -> (id.get() % slices) == slice)
           .forEach(r -> index(r));
       OnlineReindexMode.end();
       return null;
@@ -337,8 +342,9 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
       @Override
       public Void call() throws IOException {
         try (Repository repo = repoManager.openRepository(name)) {
-          ScanResult sr = ChangeNotes.Factory.scanChangeIds(repo);
-          int size = sr.all().size();
+          ImmutableMap<Change.Id, ObjectId> metaIdByChange =
+              ChangeNotes.Factory.scanChangeIds(repo);
+          int size = metaIdByChange.size();
           if (size > 0) {
             changeCount.addAndGet(size);
             int slices = 1 + size / PROJECT_SLICE_MAX_REFS;
@@ -351,7 +357,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
             projTask.updateTotal(slices);
 
             for (int slice = 0; slice < slices; slice++) {
-              ProjectSlice projectSlice = ProjectSlice.create(name, slice, slices, sr);
+              ProjectSlice projectSlice = ProjectSlice.create(name, slice, slices, metaIdByChange);
               ListenableFuture<?> future =
                   executor.submit(
                       reindexProject(
@@ -359,7 +365,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
                           name,
                           slice,
                           slices,
-                          projectSlice.scanResult(),
+                          projectSlice.metaIdByChange(),
                           doneTask,
                           failedTask));
               String description = "project " + name + " (" + slice + "/" + slices + ")";
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 3095cd2b63..00de505600 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -26,6 +26,7 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.ImmutableSortedSet;
@@ -33,8 +34,6 @@ import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Multimaps;
 import com.google.common.collect.Ordering;
-import com.google.common.collect.Sets;
-import com.google.common.collect.Sets.SetView;
 import com.google.common.flogger.FluentLogger;
 import com.google.errorprone.annotations.FormatMethod;
 import com.google.gerrit.common.Nullable;
@@ -70,6 +69,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Predicate;
@@ -114,27 +114,18 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       this.projectCache = projectCache;
     }
 
-    @AutoValue
-    public abstract static class ScanResult {
-      abstract ImmutableSet<Change.Id> fromPatchSetRefs();
-
-      abstract ImmutableSet<Change.Id> fromMetaRefs();
-
-      public SetView<Change.Id> all() {
-        return Sets.union(fromPatchSetRefs(), fromMetaRefs());
-      }
-    }
-
-    public static ScanResult scanChangeIds(Repository repo) throws IOException {
-      ImmutableSet.Builder<Change.Id> fromPs = ImmutableSet.builder();
-      ImmutableSet.Builder<Change.Id> fromMeta = ImmutableSet.builder();
+    public static ImmutableMap<Change.Id, ObjectId> scanChangeIds(Repository repo)
+        throws IOException {
+      ImmutableMap.Builder<Change.Id, ObjectId> metaIdByChange = ImmutableMap.builder();
       for (Ref r : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES)) {
-        Change.Id id = Change.Id.fromRef(r.getName());
-        if (id != null) {
-          (r.getName().endsWith(RefNames.META_SUFFIX) ? fromMeta : fromPs).add(id);
+        if (r.getName().endsWith(RefNames.META_SUFFIX)) {
+          Change.Id id = Change.Id.fromRef(r.getName());
+          if (id != null) {
+            metaIdByChange.put(id, r.getObjectId());
+          }
         }
       }
-      return new AutoValue_ChangeNotes_Factory_ScanResult(fromPs.build(), fromMeta.build());
+      return metaIdByChange.build();
     }
 
     public ChangeNotes createChecked(Change c) {
@@ -300,27 +291,25 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     }
 
     public Stream<ChangeNotesResult> scan(
-        ScanResult sr, Project.NameKey project, Predicate<Change.Id> changeIdPredicate) {
-      Stream<Change.Id> idStream = sr.all().stream();
+        ImmutableMap<Change.Id, ObjectId> metaIdByChange,
+        Project.NameKey project,
+        Predicate<Change.Id> changeIdPredicate) {
+      Stream<Map.Entry<Change.Id, ObjectId>> metaByIdStream = metaIdByChange.entrySet().stream();
       if (changeIdPredicate != null) {
-        idStream = idStream.filter(changeIdPredicate);
+        metaByIdStream = metaByIdStream.filter(e -> changeIdPredicate.test(e.getKey()));
       }
-      return idStream.map(id -> scanOneChange(project, sr, id)).filter(Objects::nonNull);
+      return metaByIdStream.map(e -> scanOneChange(project, e)).filter(Objects::nonNull);
     }
 
     @Nullable
-    private ChangeNotesResult scanOneChange(Project.NameKey project, ScanResult sr, Change.Id id) {
-      if (!sr.fromMetaRefs().contains(id)) {
-        // Stray patch set refs can happen due to normal error conditions, e.g. failed
-        // push processing, so aren't worth even a warning.
-        return null;
-      }
-
+    private ChangeNotesResult scanOneChange(
+        Project.NameKey project, Map.Entry<Change.Id, ObjectId> metaIdByChangeId) {
+      Change.Id id = metaIdByChangeId.getKey();
       // TODO(dborowitz): See discussion in BatchUpdate#newChangeContext.
       try {
         Change change = ChangeNotes.Factory.newChange(project, id);
         logger.atFine().log("adding change %s found in project %s", id, project);
-        return toResult(change);
+        return toResult(change, metaIdByChangeId.getValue());
       } catch (InvalidServerIdException ise) {
         logger.atWarning().withCause(ise).log(
             "skipping change %d in project %s because of an invalid server id", id.get(), project);
@@ -329,8 +318,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     }
 
     @Nullable
-    private ChangeNotesResult toResult(Change rawChangeFromNoteDb) {
-      ChangeNotes n = new ChangeNotes(args, rawChangeFromNoteDb, true, null);
+    private ChangeNotesResult toResult(Change rawChangeFromNoteDb, ObjectId metaId) {
+      ChangeNotes n = new ChangeNotes(args, rawChangeFromNoteDb, true, null, metaId);
       try {
         n.load();
       } catch (Exception e) {
diff --git a/tools/BUILD b/tools/BUILD
index 4e4e5f0d32..26db1faf9b 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -156,6 +156,7 @@ java_package_configuration(
         "-Xep:FloatingPointLiteralPrecision:ERROR",
         "-Xep:FloggerArgumentToString:ERROR",
         "-Xep:FloggerFormatString:ERROR",
+        "-Xep:FloggerLogString:WARN",
         "-Xep:FloggerLogVarargs:ERROR",
         "-Xep:FloggerSplitLogStatement:ERROR",
         "-Xep:FloggerStringConcatenation:ERROR",