diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java index 05cd2358ce..2aaf4c513c 100644 --- a/java/com/google/gerrit/server/change/ChangeJson.java +++ b/java/com/google/gerrit/server/change/ChangeJson.java @@ -296,7 +296,6 @@ public class ChangeJson { Map cache = Maps.newHashMapWithExpectedSize(in.size()); for (QueryResult r : in) { List infos = toChangeInfos(r.entities(), cache); - infos.forEach(c -> cache.put(new Change.Id(c._number), c)); if (!infos.isEmpty() && r.more()) { infos.get(infos.size() - 1)._moreChanges = true; } @@ -417,14 +416,29 @@ public class ChangeJson { List changes, Map cache) { try (Timer0.Context ignored = metrics.toChangeInfosLatency.start()) { List changeInfos = new ArrayList<>(changes.size()); - for (ChangeData cd : changes) { - ChangeInfo i = cache.get(cd.getId()); - if (i != null) { + for (int i = 0; i < changes.size(); i++) { + // We can only cache and re-use an entity if it's not the last in the list. The last entity + // may later get _moreChanges set. If it was cached or re-used, that setting would propagate + // to the original entity yielding wrong results. + // This problem has two sides where 'last in the list' has to be respected: + // (1) Caching + // (2) Reusing + boolean isCacheable = i != changes.size() - 1; + ChangeData cd = changes.get(i); + ChangeInfo info = cache.get(cd.getId()); + if (info != null && isCacheable) { + changeInfos.add(info); continue; } + + // Compute and cache if possible try { ensureLoaded(Collections.singleton(cd)); - changeInfos.add(format(cd, Optional.empty(), false, ChangeInfo::new)); + info = format(cd, Optional.empty(), false, ChangeInfo::new); + changeInfos.add(info); + if (isCacheable) { + cache.put(new Change.Id(info._number), info); + } } catch (OrmException | RuntimeException e) { logger.atWarning().withCause(e).log( "Omitting corrupt change %s from results", cd.getId());