Merge "Don't update the last updated timestamp when running copy-approvals" into stable-3.5
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index fc476ec..565d370 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3985,6 +3985,46 @@
+
Defaults to true.
+[[metrics]]
+=== Section metrics
+
+[[metrics.reservoir]]metrics.reservoir::
++
+The type of data reservoir used by the metrics system to calculate the percentile
+values for timers and histograms.
+It can be set to one of the following values:
++
+* ExponentiallyDecaying: An exponentially-decaying random reservoir based on
+ Cormode et al's forward-decaying priority reservoir sampling method to produce
+ a statistically representative sampling reservoir, exponentially biased towards
+ newer entries.
+* SlidingTimeWindowArray: A sliding window that stores only the measurements made
+ in the last window using chunks of 512 samples.
+* SlidingTimeWindow: A sliding window that stores only the measurements made in
+ the last window using a skip list.
+* SlidingWindow: A sliding window that stores only the last measurements.
+* Uniform: A random sampling reservoir that uses Vitter's Algorithm R to produce
+ a statistically representative sample.
++
+Defaults to ExponentiallyDecaying.
+
+[[metrics.ExponentiallyDecaying.alpha]]metrics.ExponentiallyDecaying.alpha::
++
+The exponential decay factor; the higher this is, the more biased the reservoir
+will be towards newer values.
+
+[[metrics.reservoirType.size]]metrics.<reservoirType>.size::
++
+The number of samples to keep in the reservoir. Applies to all reservoir types
+except the sliding time-based ones.
++
+Defaults to 1028.
+
+[[metrics.reservoirType.window]]metrics.<reservoirType>.window::
++
+The window of time for keeping data in the reservoir. It only applies to sliding
+time-based reservoir types.
+
[[mimetype]]
=== Section mimetype
diff --git a/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java b/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java
index 373246a..9a652e3 100644
--- a/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java
+++ b/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java
@@ -21,11 +21,13 @@
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
import com.google.gerrit.server.config.AllProjectsConfigProvider;
import com.google.gerrit.server.config.FileBasedAllProjectsConfigProvider;
import com.google.gerrit.server.config.FileBasedGlobalPluginConfigProvider;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GlobalPluginConfigProvider;
+import com.google.gerrit.server.config.MetricsReservoirConfigImpl;
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.config.TrackingFooters;
@@ -36,6 +38,7 @@
import com.google.gerrit.testing.InMemoryRepositoryManager;
import com.google.inject.Inject;
import com.google.inject.ProvisionException;
+import com.google.inject.Scopes;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -69,6 +72,7 @@
bind(InMemoryRepositoryManager.class).in(SINGLETON);
}
+ bind(MetricsReservoirConfig.class).to(MetricsReservoirConfigImpl.class).in(Scopes.SINGLETON);
bind(MetricMaker.class).to(TestMetricMaker.class);
listener().to(CreateSchema.class);
diff --git a/java/com/google/gerrit/index/query/AndPredicate.java b/java/com/google/gerrit/index/query/AndPredicate.java
index ae13fb3..098bccb 100644
--- a/java/com/google/gerrit/index/query/AndPredicate.java
+++ b/java/com/google/gerrit/index/query/AndPredicate.java
@@ -15,15 +15,19 @@
package com.google.gerrit.index.query;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
/** Requires all predicates to be true. */
-public class AndPredicate<T> extends Predicate<T> implements Matchable<T> {
+public class AndPredicate<T> extends Predicate<T>
+ implements Matchable<T>, Comparator<Predicate<T>> {
private final List<Predicate<T>> children;
private final int cost;
@@ -35,7 +39,7 @@
protected AndPredicate(Collection<? extends Predicate<T>> that) {
List<Predicate<T>> t = new ArrayList<>(that.size());
int c = 0;
- for (Predicate<T> p : that) {
+ for (Predicate<T> p : sort(that)) {
if (getClass() == p.getClass()) {
for (Predicate<T> gp : p.getChildren()) {
t.add(gp);
@@ -114,6 +118,28 @@
&& getChildren().equals(((Predicate<?>) other).getChildren());
}
+ private ImmutableList<Predicate<T>> sort(Collection<? extends Predicate<T>> that) {
+ return that.stream().sorted(this).collect(toImmutableList());
+ }
+
+ @Override
+ public int compare(Predicate<T> a, Predicate<T> b) {
+ int ai = a instanceof DataSource ? 0 : 1;
+ int bi = b instanceof DataSource ? 0 : 1;
+ int cmp = ai - bi;
+
+ if (cmp == 0) {
+ cmp = a.estimateCost() - b.estimateCost();
+ }
+
+ if (cmp == 0 && a instanceof DataSource && b instanceof DataSource) {
+ DataSource<?> as = (DataSource<?>) a;
+ DataSource<?> bs = (DataSource<?>) b;
+ cmp = as.getCardinality() - bs.getCardinality();
+ }
+ return cmp;
+ }
+
@Override
public String toString() {
final StringBuilder r = new StringBuilder();
diff --git a/java/com/google/gerrit/index/query/AndSource.java b/java/com/google/gerrit/index/query/AndSource.java
index de24df0..2a04051 100644
--- a/java/com/google/gerrit/index/query/AndSource.java
+++ b/java/com/google/gerrit/index/query/AndSource.java
@@ -15,23 +15,13 @@
package com.google.gerrit.index.query;
import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.collect.ImmutableList.toImmutableList;
-import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Ordering;
-import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.IndexConfig;
-import com.google.gerrit.index.PaginationType;
-import com.google.gerrit.index.QueryOptions;
-import java.util.ArrayList;
import java.util.Collection;
-import java.util.Comparator;
import java.util.List;
-public class AndSource<T> extends AndPredicate<T>
- implements DataSource<T>, Comparator<Predicate<T>> {
+public class AndSource<T> extends AndPredicate<T> implements DataSource<T> {
protected final DataSource<T> source;
private final IsVisibleToPredicate<T> isVisibleToPredicate;
@@ -65,91 +55,34 @@
checkArgument(start >= 0, "negative start: %s", start);
this.isVisibleToPredicate = isVisibleToPredicate;
this.start = start;
+ this.indexConfig = indexConfig;
int c = Integer.MAX_VALUE;
DataSource<T> s = null;
int minCost = Integer.MAX_VALUE;
- for (Predicate<T> p : sort(getChildren())) {
+ for (Predicate<T> p : getChildren()) {
if (p instanceof DataSource) {
c = Math.min(c, ((DataSource<?>) p).getCardinality());
int cost = p.estimateCost();
if (cost < minCost) {
- s = toDataSource(p);
+ s = toPaginatingSource(p);
minCost = cost;
}
}
}
this.source = s;
this.cardinality = c;
- this.indexConfig = indexConfig;
}
@Override
public ResultSet<T> read() {
- if (source == null) {
- throw new StorageException("No DataSource: " + this);
- }
-
- // ResultSets are lazy. Calling #read here first and then dealing with ResultSets only when
- // requested allows the index to run asynchronous queries.
- ResultSet<T> resultSet = source.read();
- return new LazyResultSet<>(
- () -> {
- List<T> r = new ArrayList<>();
- T last = null;
- int pageResultSize = 0;
- for (T data : buffer(resultSet)) {
- if (!isMatchable() || match(data)) {
- r.add(data);
- }
- last = data;
- pageResultSize++;
- }
-
- if (last != null && source instanceof Paginated) {
- // Restart source and continue if we have not filled the
- // full limit the caller wants.
- //
- @SuppressWarnings("unchecked")
- Paginated<T> p = (Paginated<T>) source;
- QueryOptions opts = p.getOptions();
- final int limit = opts.limit();
- int pageSize = opts.pageSize();
- int pageSizeMultiplier = opts.pageSizeMultiplier();
- Object searchAfter = resultSet.searchAfter();
- int nextStart = pageResultSize;
- while (pageResultSize == pageSize && r.size() < limit) {
- pageSize = getNextPageSize(pageSize, pageSizeMultiplier);
- ResultSet<T> next =
- indexConfig.paginationType().equals(PaginationType.SEARCH_AFTER)
- ? p.restart(searchAfter, pageSize)
- : p.restart(nextStart, pageSize);
- pageResultSize = 0;
- for (T data : buffer(next)) {
- if (match(data)) {
- r.add(data);
- }
- pageResultSize++;
- }
- nextStart += pageResultSize;
- searchAfter = next.searchAfter();
- }
- }
-
- if (start >= r.size()) {
- return ImmutableList.of();
- } else if (start > 0) {
- return ImmutableList.copyOf(r.subList(start, r.size()));
- }
- return ImmutableList.copyOf(r);
- });
+ return source.read();
}
@Override
public ResultSet<FieldBundle> readRaw() {
- // TOOD(hiesel): Implement
- throw new UnsupportedOperationException("not implemented");
+ return source.readRaw();
}
@Override
@@ -170,11 +103,6 @@
return true;
}
- private Iterable<T> buffer(ResultSet<T> scanner) {
- return FluentIterable.from(Iterables.partition(scanner, 50))
- .transformAndConcat(this::transformBuffer);
- }
-
protected List<T> transformBuffer(List<T> buffer) {
return buffer;
}
@@ -184,46 +112,18 @@
return cardinality;
}
- private ImmutableList<Predicate<T>> sort(Collection<? extends Predicate<T>> that) {
- return that.stream().sorted(this).collect(toImmutableList());
- }
-
- @Override
- public int compare(Predicate<T> a, Predicate<T> b) {
- int ai = a instanceof DataSource ? 0 : 1;
- int bi = b instanceof DataSource ? 0 : 1;
- int cmp = ai - bi;
-
- if (cmp == 0) {
- cmp = a.estimateCost() - b.estimateCost();
- }
-
- if (cmp == 0 && a instanceof DataSource && b instanceof DataSource) {
- DataSource<?> as = (DataSource<?>) a;
- DataSource<?> bs = (DataSource<?>) b;
- cmp = as.getCardinality() - bs.getCardinality();
- }
- return cmp;
- }
-
@SuppressWarnings("unchecked")
- private DataSource<T> toDataSource(Predicate<T> pred) {
- return (DataSource<T>) pred;
- }
+ private PaginatingSource<T> toPaginatingSource(Predicate<T> pred) {
+ return new PaginatingSource<T>((DataSource<T>) pred, start, indexConfig) {
+ @Override
+ protected boolean match(T object) {
+ return AndSource.this.match(object);
+ }
- private int getNextPageSize(int pageSize, int pageSizeMultiplier) {
- List<Integer> possiblePageSizes = new ArrayList<>(3);
- try {
- possiblePageSizes.add(Math.multiplyExact(pageSize, pageSizeMultiplier));
- } catch (ArithmeticException e) {
- possiblePageSizes.add(Integer.MAX_VALUE);
- }
- if (indexConfig.maxPageSize() > 0) {
- possiblePageSizes.add(indexConfig.maxPageSize());
- }
- if (indexConfig.maxLimit() > 0) {
- possiblePageSizes.add(indexConfig.maxLimit());
- }
- return Ordering.natural().min(possiblePageSizes);
+ @Override
+ protected boolean isMatchable() {
+ return AndSource.this.isMatchable();
+ }
+ };
}
}
diff --git a/java/com/google/gerrit/index/query/PaginatingSource.java b/java/com/google/gerrit/index/query/PaginatingSource.java
new file mode 100644
index 0000000..fd3a218
--- /dev/null
+++ b/java/com/google/gerrit/index/query/PaginatingSource.java
@@ -0,0 +1,148 @@
+// Copyright (C) 2022 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.index.query;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Ordering;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.IndexConfig;
+import com.google.gerrit.index.PaginationType;
+import com.google.gerrit.index.QueryOptions;
+import java.util.ArrayList;
+import java.util.List;
+
+public class PaginatingSource<T> implements DataSource<T> {
+ protected final DataSource<T> source;
+ private final int start;
+ private final int cardinality;
+ private final IndexConfig indexConfig;
+
+ public PaginatingSource(DataSource<T> source, int start, IndexConfig indexConfig) {
+ checkArgument(start >= 0, "negative start: %s", start);
+ this.source = source;
+ this.start = start;
+ this.cardinality = source.getCardinality();
+ this.indexConfig = indexConfig;
+ }
+
+ @Override
+ public ResultSet<T> read() {
+ if (source == null) {
+ throw new StorageException("No DataSource: " + this);
+ }
+
+ // ResultSets are lazy. Calling #read here first and then dealing with ResultSets only when
+ // requested allows the index to run asynchronous queries.
+ ResultSet<T> resultSet = source.read();
+ return new LazyResultSet<>(
+ () -> {
+ List<T> r = new ArrayList<>();
+ T last = null;
+ int pageResultSize = 0;
+ for (T data : buffer(resultSet)) {
+ if (!isMatchable() || match(data)) {
+ r.add(data);
+ }
+ last = data;
+ pageResultSize++;
+ }
+
+ if (last != null && source instanceof Paginated) {
+ // Restart source and continue if we have not filled the
+ // full limit the caller wants.
+ //
+ @SuppressWarnings("unchecked")
+ Paginated<T> p = (Paginated<T>) source;
+ QueryOptions opts = p.getOptions();
+ final int limit = opts.limit();
+ int pageSize = opts.pageSize();
+ int pageSizeMultiplier = opts.pageSizeMultiplier();
+ Object searchAfter = resultSet.searchAfter();
+ int nextStart = pageResultSize;
+ while (pageResultSize == pageSize && r.size() <= limit) { // get 1 more than the limit
+ pageSize = getNextPageSize(pageSize, pageSizeMultiplier);
+ ResultSet<T> next =
+ indexConfig.paginationType().equals(PaginationType.SEARCH_AFTER)
+ ? p.restart(searchAfter, pageSize)
+ : p.restart(nextStart, pageSize);
+ pageResultSize = 0;
+ for (T data : buffer(next)) {
+ if (match(data)) {
+ r.add(data);
+ }
+ pageResultSize++;
+ }
+ nextStart += pageResultSize;
+ searchAfter = next.searchAfter();
+ }
+ }
+
+ if (start >= r.size()) {
+ return ImmutableList.of();
+ } else if (start > 0) {
+ return ImmutableList.copyOf(r.subList(start, r.size()));
+ }
+ return ImmutableList.copyOf(r);
+ });
+ }
+
+ @Override
+ public ResultSet<FieldBundle> readRaw() {
+ // TOOD(hiesel): Implement
+ throw new UnsupportedOperationException("not implemented");
+ }
+
+ private Iterable<T> buffer(ResultSet<T> scanner) {
+ return FluentIterable.from(Iterables.partition(scanner, 50))
+ .transformAndConcat(this::transformBuffer);
+ }
+
+ protected boolean match(T object) {
+ return true;
+ }
+
+ protected boolean isMatchable() {
+ return true;
+ }
+
+ protected List<T> transformBuffer(List<T> buffer) {
+ return buffer;
+ }
+
+ @Override
+ public int getCardinality() {
+ return cardinality;
+ }
+
+ private int getNextPageSize(int pageSize, int pageSizeMultiplier) {
+ List<Integer> possiblePageSizes = new ArrayList<>(3);
+ try {
+ possiblePageSizes.add(Math.multiplyExact(pageSize, pageSizeMultiplier));
+ } catch (ArithmeticException e) {
+ possiblePageSizes.add(Integer.MAX_VALUE);
+ }
+ if (indexConfig.maxPageSize() > 0) {
+ possiblePageSizes.add(indexConfig.maxPageSize());
+ }
+ if (indexConfig.maxLimit() > 0) {
+ possiblePageSizes.add(indexConfig.maxLimit());
+ }
+ return Ordering.natural().min(possiblePageSizes);
+ }
+}
diff --git a/java/com/google/gerrit/index/query/QueryProcessor.java b/java/com/google/gerrit/index/query/QueryProcessor.java
index 0c9a474..ec90d21 100644
--- a/java/com/google/gerrit/index/query/QueryProcessor.java
+++ b/java/com/google/gerrit/index/query/QueryProcessor.java
@@ -79,7 +79,7 @@
private final IndexCollection<?, T, ? extends Index<?, T>> indexes;
private final IndexRewriter<T> rewriter;
private final String limitField;
- private final IntSupplier permittedLimit;
+ private final IntSupplier userQueryLimit;
private final CallerFinder callerFinder;
// This class is not generally thread-safe, but programmer error may result in it being shared
@@ -100,14 +100,14 @@
IndexCollection<?, T, ? extends Index<?, T>> indexes,
IndexRewriter<T> rewriter,
String limitField,
- IntSupplier permittedLimit) {
+ IntSupplier userQueryLimit) {
this.metrics = new Metrics(metricMaker);
this.schemaDef = schemaDef;
this.indexConfig = indexConfig;
this.indexes = indexes;
this.rewriter = rewriter;
this.limitField = limitField;
- this.permittedLimit = permittedLimit;
+ this.userQueryLimit = userQueryLimit;
this.used = new AtomicBoolean(false);
this.callerFinder =
CallerFinder.builder()
@@ -230,7 +230,7 @@
checkSupportedForQueries(q);
int limit = getEffectiveLimit(q);
limits.add(limit);
- int initialPageSize = getInitialPageSize(q);
+ int initialPageSize = getInitialPageSize(limit);
if (initialPageSize == getBackendSupportedLimit()) {
initialPageSize--;
@@ -281,6 +281,9 @@
@SuppressWarnings("unchecked")
DataSource<T> s = (DataSource<T>) pred;
+ if (initialPageSize < limit && !(pred instanceof AndSource)) {
+ s = new PaginatingSource<T>(s, start, indexConfig);
+ }
sources.add(s);
}
@@ -385,14 +388,17 @@
}
private int getPermittedLimit() {
- return enforceVisibility ? permittedLimit.getAsInt() : Integer.MAX_VALUE;
+ return enforceVisibility ? userQueryLimit.getAsInt() : Integer.MAX_VALUE;
}
private int getBackendSupportedLimit() {
return indexConfig.maxLimit();
}
- public int getInitialPageSize(Predicate<T> p) {
+ public int getEffectiveLimit(Predicate<T> p) {
+ if (isNoLimit == true) {
+ return Integer.MAX_VALUE;
+ }
List<Integer> possibleLimits = new ArrayList<>(4);
possibleLimits.add(getBackendSupportedLimit());
possibleLimits.add(getPermittedLimit());
@@ -407,18 +413,11 @@
}
int result = Ordering.natural().min(possibleLimits);
// Should have short-circuited from #query or thrown some other exception before getting here.
- checkState(result > 0, "effective initial page size should be positive");
+ checkState(result > 0, "effective limit should be positive");
return result;
}
- private int getEffectiveLimit(Predicate<T> p) {
- if (isNoLimit == true) {
- return Integer.MAX_VALUE;
- }
- return getInitialPageSize(p);
- }
-
private static Optional<QueryParseException> findQueryParseException(Throwable t) {
return Throwables.getCausalChain(t).stream()
.filter(c -> c instanceof QueryParseException)
@@ -426,6 +425,14 @@
.findFirst();
}
+ protected IntSupplier getUserQueryLimit() {
+ return userQueryLimit;
+ }
+
+ protected int getInitialPageSize(int queryLimit) {
+ return queryLimit;
+ }
+
protected abstract String formatForLogging(T t);
protected abstract int getIndexSize();
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 23ffdac..efbe0e8 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -40,6 +40,7 @@
import com.google.gerrit.entities.converter.ProtoConverter;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
+import com.google.gerrit.index.PaginationType;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.FieldBundle;
@@ -322,6 +323,7 @@
private final QueryOptions opts;
private final Sort sort;
private final Function<Document, FieldBundle> rawDocumentMapper;
+ private final boolean isSearchAfterPagination;
private QuerySource(
List<ChangeSubIndex> indexes,
@@ -336,6 +338,8 @@
this.opts = opts;
this.sort = sort;
this.rawDocumentMapper = rawDocumentMapper;
+ this.isSearchAfterPagination =
+ opts.config().paginationType().equals(PaginationType.SEARCH_AFTER);
}
@Override
@@ -422,26 +426,34 @@
if (Integer.MAX_VALUE - opts.pageSize() < opts.start()) {
realPageSize = Integer.MAX_VALUE;
}
- TopFieldDocs[] hits = new TopFieldDocs[indexes.size()];
+ List<TopFieldDocs> hits = new ArrayList<>();
+ int searchAfterHitsCount = 0;
for (int i = 0; i < indexes.size(); i++) {
ChangeSubIndex subIndex = indexes.get(i);
searchers[i] = subIndex.acquire();
- if (opts.searchAfter() != null && opts.searchAfter() instanceof HashMap) {
- hits[i] =
- searchers[i].searchAfter(
- ((HashMap<ChangeSubIndex, ScoreDoc>) opts.searchAfter()).get(subIndex),
- query,
- realPageSize,
- sort,
- /* doDocScores= */ false,
- /* doMaxScore= */ false);
+ if (isSearchAfterPagination) {
+ ScoreDoc searchAfter = getSearchAfter(subIndex);
+ int maxRemainingHits = realPageSize - searchAfterHitsCount;
+ if (maxRemainingHits > 0) {
+ TopFieldDocs subIndexHits =
+ searchers[i].searchAfter(
+ searchAfter,
+ query,
+ maxRemainingHits,
+ sort,
+ /* doDocScores= */ false,
+ /* doMaxScore= */ false);
+ searchAfterHitsCount += subIndexHits.scoreDocs.length;
+ hits.add(subIndexHits);
+ searchAfterBySubIndex.put(
+ subIndex, Iterables.getLast(Arrays.asList(subIndexHits.scoreDocs), searchAfter));
+ }
} else {
- hits[i] = searchers[i].search(query, realPageSize, sort);
+ hits.add(searchers[i].search(query, realPageSize, sort));
}
- searchAfterBySubIndex.put(
- subIndex, Iterables.getLast(Arrays.asList(hits[i].scoreDocs), null));
}
- TopDocs docs = TopDocs.merge(sort, realPageSize, hits);
+ TopDocs docs =
+ TopDocs.merge(sort, realPageSize, hits.stream().toArray(TopFieldDocs[]::new));
List<Document> result = new ArrayList<>(docs.scoreDocs.length);
for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
@@ -461,6 +473,23 @@
}
}
}
+
+ /**
+ * Returns null for the first page or when pagination type is not {@link
+ * PaginationType#SEARCH_AFTER search-after}, otherwise returns the last doc from previous
+ * search on the given change sub-index.
+ *
+ * @param subIndex change sub-index
+ * @return the score doc that can be used to page result sets
+ */
+ private ScoreDoc getSearchAfter(ChangeSubIndex subIndex) {
+ if (isSearchAfterPagination
+ && opts.searchAfter() != null
+ && opts.searchAfter() instanceof Map) {
+ return ((Map<ChangeSubIndex, ScoreDoc>) opts.searchAfter()).get(subIndex);
+ }
+ return null;
+ }
}
private static class Results {
diff --git a/java/com/google/gerrit/metrics/MetricsReservoirConfig.java b/java/com/google/gerrit/metrics/MetricsReservoirConfig.java
new file mode 100644
index 0000000..ca4cb09
--- /dev/null
+++ b/java/com/google/gerrit/metrics/MetricsReservoirConfig.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2022 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.metrics;
+
+import java.time.Duration;
+
+/** Configuration of the Metrics' reservoir type and size. */
+public interface MetricsReservoirConfig {
+
+ /** Returns the reservoir type. */
+ ReservoirType reservoirType();
+
+ /** Returns the reservoir window duration. */
+ Duration reservoirWindow();
+
+ /** Returns the number of samples that the reservoir can contain */
+ int reservoirSize();
+
+ /** Returns the alpha parameter of the ExponentiallyDecaying reservoir */
+ double reservoirAlpha();
+}
diff --git a/java/com/google/gerrit/metrics/ReservoirType.java b/java/com/google/gerrit/metrics/ReservoirType.java
new file mode 100644
index 0000000..fe89752
--- /dev/null
+++ b/java/com/google/gerrit/metrics/ReservoirType.java
@@ -0,0 +1,24 @@
+// Copyright (C) 2022 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.metrics;
+
+/** Type of reservoir for collecting metrics into. */
+public enum ReservoirType {
+ ExponentiallyDecaying,
+ SlidingTimeWindowArray,
+ SlidingTimeWindow,
+ SlidingWindow,
+ Uniform;
+}
diff --git a/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java b/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java
index fcba0ee..32be18d 100644
--- a/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java
+++ b/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java
@@ -18,8 +18,10 @@
import static com.google.gerrit.metrics.dropwizard.MetricResource.METRIC_KIND;
import static com.google.gerrit.server.config.ConfigResource.CONFIG_KIND;
+import com.codahale.metrics.Histogram;
import com.codahale.metrics.Metric;
import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -41,6 +43,7 @@
import com.google.gerrit.metrics.Histogram2;
import com.google.gerrit.metrics.Histogram3;
import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.metrics.Timer1;
import com.google.gerrit.metrics.Timer2;
@@ -48,10 +51,12 @@
import com.google.gerrit.metrics.proc.JGitMetricModule;
import com.google.gerrit.metrics.proc.ProcMetricModule;
import com.google.gerrit.server.cache.CacheMetrics;
+import com.google.gerrit.server.config.MetricsReservoirConfigImpl;
import com.google.inject.Inject;
import com.google.inject.Scopes;
import com.google.inject.Singleton;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
@@ -65,8 +70,25 @@
@Singleton
public class DropWizardMetricMaker extends MetricMaker {
public static class ApiModule extends RestApiModule {
+ private final Optional<MetricsReservoirConfig> metricsReservoirConfig;
+
+ public ApiModule(MetricsReservoirConfig metricsReservoirConfig) {
+ this.metricsReservoirConfig = Optional.of(metricsReservoirConfig);
+ }
+
+ public ApiModule() {
+ this.metricsReservoirConfig = Optional.empty();
+ }
+
@Override
protected void configure() {
+ if (metricsReservoirConfig.isPresent()) {
+ bind(MetricsReservoirConfig.class).toInstance(metricsReservoirConfig.get());
+ } else {
+ bind(MetricsReservoirConfig.class)
+ .to(MetricsReservoirConfigImpl.class)
+ .in(Scopes.SINGLETON);
+ }
bind(MetricRegistry.class).in(Scopes.SINGLETON);
bind(DropWizardMetricMaker.class).in(Scopes.SINGLETON);
bind(MetricMaker.class).to(DropWizardMetricMaker.class);
@@ -89,12 +111,14 @@
private final MetricRegistry registry;
private final Map<String, BucketedMetric> bucketed;
private final Map<String, ImmutableMap<String, String>> descriptions;
+ private final MetricsReservoirConfig reservoirConfig;
@Inject
- DropWizardMetricMaker(MetricRegistry registry) {
+ DropWizardMetricMaker(MetricRegistry registry, MetricsReservoirConfig reservoirConfig) {
this.registry = registry;
this.bucketed = new ConcurrentHashMap<>();
this.descriptions = new ConcurrentHashMap<>();
+ this.reservoirConfig = reservoirConfig;
}
Iterable<String> getMetricNames() {
@@ -222,7 +246,9 @@
}
TimerImpl newTimerImpl(String name) {
- return new TimerImpl(name, registry.timer(name));
+ return new TimerImpl(
+ name,
+ registry.timer(name, () -> new Timer(DropWizardReservoirProvider.get(reservoirConfig))));
}
@Override
@@ -271,7 +297,10 @@
}
HistogramImpl newHistogramImpl(String name) {
- return new HistogramImpl(name, registry.histogram(name));
+ return new HistogramImpl(
+ name,
+ registry.histogram(
+ name, () -> new Histogram(DropWizardReservoirProvider.get(reservoirConfig))));
}
@Override
diff --git a/java/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProvider.java b/java/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProvider.java
new file mode 100644
index 0000000..3089068
--- /dev/null
+++ b/java/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProvider.java
@@ -0,0 +1,52 @@
+// Copyright (C) 2022 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.metrics.dropwizard;
+
+import com.codahale.metrics.ExponentiallyDecayingReservoir;
+import com.codahale.metrics.Reservoir;
+import com.codahale.metrics.SlidingTimeWindowArrayReservoir;
+import com.codahale.metrics.SlidingTimeWindowReservoir;
+import com.codahale.metrics.SlidingWindowReservoir;
+import com.codahale.metrics.UniformReservoir;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
+import com.google.gerrit.metrics.ReservoirType;
+import java.util.concurrent.TimeUnit;
+
+class DropWizardReservoirProvider {
+
+ private DropWizardReservoirProvider() {}
+
+ static Reservoir get(MetricsReservoirConfig config) {
+ ReservoirType reservoirType = config.reservoirType();
+ switch (reservoirType) {
+ case ExponentiallyDecaying:
+ return new ExponentiallyDecayingReservoir(config.reservoirSize(), config.reservoirAlpha());
+ case SlidingTimeWindowArray:
+ return new SlidingTimeWindowArrayReservoir(
+ config.reservoirWindow().toMillis(), TimeUnit.MILLISECONDS);
+ case SlidingTimeWindow:
+ return new SlidingTimeWindowReservoir(
+ config.reservoirWindow().toMillis(), TimeUnit.MILLISECONDS);
+ case SlidingWindow:
+ return new SlidingWindowReservoir(config.reservoirSize());
+ case Uniform:
+ return new UniformReservoir(config.reservoirSize());
+
+ default:
+ throw new IllegalArgumentException(
+ "Unsupported metrics reservoir type " + reservoirType.name());
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
index 23caca7..8e06294 100644
--- a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
+++ b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
@@ -158,7 +158,7 @@
Duration refreshAfterWrite = def.refreshAfterWrite();
if (has(def.configKey(), "refreshAfterWrite")) {
- builder.expireAfterAccess(
+ builder.refreshAfterWrite(
ConfigUtil.getTimeUnit(
cfg,
"cache",
diff --git a/java/com/google/gerrit/server/config/MetricsReservoirConfigImpl.java b/java/com/google/gerrit/server/config/MetricsReservoirConfigImpl.java
new file mode 100644
index 0000000..ac3c53a
--- /dev/null
+++ b/java/com/google/gerrit/server/config/MetricsReservoirConfigImpl.java
@@ -0,0 +1,96 @@
+// Copyright (C) 2022 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.config;
+
+import com.google.gerrit.metrics.MetricsReservoirConfig;
+import com.google.gerrit.metrics.ReservoirType;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.time.Duration;
+import java.util.Optional;
+import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.lib.Config;
+
+/** Define metrics reservoir settings based on gerrit.config */
+@Singleton
+public class MetricsReservoirConfigImpl implements MetricsReservoirConfig {
+ private static final double RESERVOIR_ALPHA_DEFAULT = 0.015;
+ private static final int METRICS_RESERVOIR_SIZE_DEFAULT = 1028;
+ private static final long METRICS_RESERVOIR_WINDOW_MSEC_DEFAULT = 60000L;
+ private static final String METRICS_SECTION = "metrics";
+ private static final String METRICS_RESERVOIR = "reservoir";
+
+ private final ReservoirType reservoirType;
+
+ private final Duration reservoirWindow;
+ private final int reservoirSize;
+ private final double reservoirAlpha;
+
+ @Inject
+ MetricsReservoirConfigImpl(@GerritServerConfig Config gerritConfig) {
+ this.reservoirType =
+ gerritConfig.getEnum(
+ METRICS_SECTION, null, METRICS_RESERVOIR, ReservoirType.ExponentiallyDecaying);
+
+ reservoirWindow =
+ Duration.ofMillis(
+ ConfigUtil.getTimeUnit(
+ gerritConfig,
+ METRICS_SECTION,
+ reservoirType.name(),
+ "window",
+ METRICS_RESERVOIR_WINDOW_MSEC_DEFAULT,
+ TimeUnit.MILLISECONDS));
+ reservoirSize =
+ gerritConfig.getInt(
+ METRICS_SECTION, reservoirType.name(), "size", METRICS_RESERVOIR_SIZE_DEFAULT);
+ reservoirAlpha =
+ Optional.ofNullable(gerritConfig.getString(METRICS_SECTION, reservoirType.name(), "alpha"))
+ .map(Double::parseDouble)
+ .orElse(RESERVOIR_ALPHA_DEFAULT);
+ }
+
+ /* (non-Javadoc)
+ * @see com.google.gerrit.server.config.MetricsConfig#reservoirType()
+ */
+ @Override
+ public ReservoirType reservoirType() {
+ return reservoirType;
+ }
+
+ /* (non-Javadoc)
+ * @see com.google.gerrit.server.config.MetricsConfig#reservoirWindow()
+ */
+ @Override
+ public Duration reservoirWindow() {
+ return reservoirWindow;
+ }
+
+ /* (non-Javadoc)
+ * @see com.google.gerrit.server.config.MetricsConfig#reservoirSize()
+ */
+ @Override
+ public int reservoirSize() {
+ return reservoirSize;
+ }
+
+ /* (non-Javadoc)
+ * @see com.google.gerrit.server.config.MetricsConfig#reservoirAlpha()
+ */
+ @Override
+ public double reservoirAlpha() {
+ return reservoirAlpha;
+ }
+}
diff --git a/java/com/google/gerrit/server/events/EventTypes.java b/java/com/google/gerrit/server/events/EventTypes.java
index 5498ec8..229ef86 100644
--- a/java/com/google/gerrit/server/events/EventTypes.java
+++ b/java/com/google/gerrit/server/events/EventTypes.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.events;
+import com.google.common.collect.ImmutableMap;
import java.util.HashMap;
import java.util.Map;
@@ -61,4 +62,15 @@
public static Class<?> getClass(String type) {
return typesByString.get(type);
}
+
+ /**
+ * Get a copy of all currently registered events.
+ *
+ * <p>The key is the one given to the evenType parameter of the {@link #register} method.
+ *
+ * @return ImmutableMap of event types, Event classes.
+ */
+ public static Map<String, Class<?>> getRegisteredEvents() {
+ return ImmutableMap.copyOf(typesByString);
+ }
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 4899974..9a59f46 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -1446,7 +1446,7 @@
// Must pass explicit user instead of injecting a provider into CreateRefControl, since
// Provider<CurrentUser> within ReceiveCommits will always return anonymous.
createRefControl.checkCreateRef(
- Providers.of(user), receivePack.getRepository(), branch, obj);
+ Providers.of(user), receivePack.getRepository(), branch, obj, /* forPush= */ true);
} catch (AuthException denied) {
rejectProhibited(cmd, denied);
return;
diff --git a/java/com/google/gerrit/server/project/CreateRefControl.java b/java/com/google/gerrit/server/project/CreateRefControl.java
index 2e76949..ab134b5 100644
--- a/java/com/google/gerrit/server/project/CreateRefControl.java
+++ b/java/com/google/gerrit/server/project/CreateRefControl.java
@@ -25,10 +25,13 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.update.RetryHelper;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.List;
import java.util.Optional;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.PersonIdent;
@@ -41,18 +44,24 @@
/** Manages access control for creating Git references (aka branches, tags). */
@Singleton
public class CreateRefControl {
+
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
private final Reachable reachable;
+ private final RetryHelper retryHelper;
@Inject
CreateRefControl(
- PermissionBackend permissionBackend, ProjectCache projectCache, Reachable reachable) {
+ PermissionBackend permissionBackend,
+ ProjectCache projectCache,
+ Reachable reachable,
+ RetryHelper retryHelper) {
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
this.reachable = reachable;
+ this.retryHelper = retryHelper;
}
/**
@@ -60,30 +69,56 @@
*
* @param user the user performing the operation
* @param repo repository on which user want to create
- * @param branch the branch the new {@link RevObject} should be created on
+ * @param destBranch the branch the new {@link RevObject} should be created on
* @param object the object the user will start the reference with
+ * @param sourceBranches the source ref from which the new ref is created from
* @throws AuthException if creation is denied; the message explains the denial.
* @throws PermissionBackendException on failure of permission checks.
* @throws ResourceConflictException if the project state does not permit the operation
*/
public void checkCreateRef(
- Provider<? extends CurrentUser> user, Repository repo, BranchNameKey branch, RevObject object)
+ Provider<? extends CurrentUser> user,
+ Repository repo,
+ BranchNameKey destBranch,
+ RevObject object,
+ boolean forPush,
+ BranchNameKey... sourceBranches)
throws AuthException, PermissionBackendException, NoSuchProjectException, IOException,
ResourceConflictException {
ProjectState ps =
- projectCache.get(branch.project()).orElseThrow(noSuchProject(branch.project()));
+ projectCache.get(destBranch.project()).orElseThrow(noSuchProject(destBranch.project()));
ps.checkStatePermitsWrite();
- PermissionBackend.ForRef perm = permissionBackend.user(user.get()).ref(branch);
+ PermissionBackend.ForRef perm = permissionBackend.user(user.get()).ref(destBranch);
if (object instanceof RevCommit) {
perm.check(RefPermission.CREATE);
- checkCreateCommit(user, repo, (RevCommit) object, ps.getNameKey(), perm);
+ if (sourceBranches.length == 0) {
+ checkCreateCommit(user, repo, (RevCommit) object, ps.getNameKey(), perm, forPush);
+ } else {
+ for (BranchNameKey src : sourceBranches) {
+ PermissionBackend.ForRef forRef = permissionBackend.user(user.get()).ref(src);
+ if (forRef.testOrFalse(RefPermission.READ)) {
+ return;
+ }
+ }
+ AuthException e =
+ new AuthException(
+ String.format(
+ "must have %s on existing ref to create new ref from it",
+ RefPermission.READ.describeForException()));
+ e.setAdvice(
+ String.format(
+ "use an existing ref visible to you, or get %s permission on the ref",
+ RefPermission.READ.describeForException()));
+ throw e;
+ }
} else if (object instanceof RevTag) {
RevTag tag = (RevTag) object;
try (RevWalk rw = new RevWalk(repo)) {
rw.parseBody(tag);
} catch (IOException e) {
- logger.atSevere().withCause(e).log("RevWalk(%s) parsing %s:", branch.project(), tag.name());
+ logger.atSevere().withCause(e).log(
+ "RevWalk(%s) parsing %s:", destBranch.project(), tag.name());
throw e;
}
@@ -97,14 +132,14 @@
RevObject target = tag.getObject();
if (target instanceof RevCommit) {
- checkCreateCommit(user, repo, (RevCommit) target, ps.getNameKey(), perm);
+ checkCreateCommit(user, repo, (RevCommit) target, ps.getNameKey(), perm, forPush);
} else {
- checkCreateRef(user, repo, branch, target);
+ checkCreateRef(user, repo, destBranch, target, forPush);
}
// If the tag has a PGP signature, allow a lower level of permission
// than if it doesn't have a PGP signature.
- PermissionBackend.ForRef forRef = permissionBackend.user(user.get()).ref(branch);
+ PermissionBackend.ForRef forRef = permissionBackend.user(user.get()).ref(destBranch);
if (tag.getRawGpgSignature() != null) {
forRef.check(RefPermission.CREATE_SIGNED_TAG);
} else {
@@ -122,13 +157,31 @@
Repository repo,
RevCommit commit,
Project.NameKey project,
- PermissionBackend.ForRef forRef)
+ PermissionBackend.ForRef forRef,
+ boolean forPush)
throws AuthException, PermissionBackendException, IOException {
try {
- // If the user has update (push) permission, they can create the ref regardless
- // of whether they are pushing any new objects along with the create.
- forRef.check(RefPermission.UPDATE);
- return;
+ // If the user has UPDATE (push) permission, they can set the ref to an arbitrary commit:
+ //
+ // * if they don't have access, we don't advertise the data, and a conforming git client
+ // would send the object along with the push as outcome of the negotation.
+ // * a malicious client could try to send the update without sending the object. This
+ // is prevented by JGit's ConnectivityChecker (see receive.checkReferencedObjectsAreReachable
+ // to switch off this costly check).
+ //
+ // Thus, when using the git command-line client, we don't need to do extra checks for users
+ // with push access.
+ //
+ // When using the REST API, there is no negotiation, and the target commit must already be on
+ // the server, so we must check that the user can see that commit.
+ if (forPush) {
+ // We can only shortcut for UPDATE permission. Pushing a tag (CREATE_TAG, CREATE_SIGNED_TAG)
+ // can also introduce new objects. While there may not be a confidentiality problem
+ // (the caller supplies the data as documented above), the permission is for creating
+ // tags to existing commits.
+ forRef.check(RefPermission.UPDATE);
+ return;
+ }
} catch (AuthException denied) {
// Fall through to check reachability.
}
@@ -145,6 +198,18 @@
return;
}
+ // Previous check only catches normal branches. Try PatchSet refs too. If we can create refs,
+ // we're not a replica, so we can always use the change index.
+ List<ChangeData> changes =
+ retryHelper
+ .changeIndexQuery(
+ "queryChangesByProjectCommitWithLimit1",
+ q -> q.enforceVisibility(true).setLimit(1).byProjectCommit(project, commit))
+ .call();
+ if (!changes.isEmpty()) {
+ return;
+ }
+
AuthException e =
new AuthException(
String.format(
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index 7a2dc44a..0f0535a 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -160,4 +160,9 @@
protected int getBatchSize() {
return sequences.changeBatchSize();
}
+
+ @Override
+ protected int getInitialPageSize(int limit) {
+ return Math.min(getUserQueryLimit().getAsInt(), limit);
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateBranch.java b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
index 2fd2d65..33549fe 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateBranch.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
@@ -47,6 +47,7 @@
import java.io.IOException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevObject;
@@ -129,7 +130,21 @@
object = rw.parseCommit(object);
}
- createRefControl.checkCreateRef(identifiedUser, repo, name, object);
+ Ref sourceRef = repo.exactRef(input.revision);
+ if (sourceRef == null) {
+ createRefControl.checkCreateRef(identifiedUser, repo, name, object, /* forPush= */ false);
+ } else {
+ if (sourceRef.isSymbolic()) {
+ sourceRef = sourceRef.getTarget();
+ }
+ createRefControl.checkCreateRef(
+ identifiedUser,
+ repo,
+ name,
+ object,
+ /* forPush= */ false,
+ BranchNameKey.create(rsrc.getNameKey(), sourceRef.getName()));
+ }
RefUpdate u = repo.updateRef(ref);
u.setExpectedOldObjectId(ObjectId.zeroId());
diff --git a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
index 9d1bdaa..a1974b9 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
@@ -47,6 +47,7 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
import org.eclipse.jgit.transport.TrackingRefUpdate;
import org.junit.Before;
@@ -94,6 +95,32 @@
}
@Test
+ public void pushNewCommitsRequiresPushPermission() throws Exception {
+ testRepo.branch("HEAD").commit().create();
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.CREATE).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+
+ PushResult r = push("HEAD:refs/heads/newbranch");
+
+ String msg = "update for creating new commit object not permitted";
+ RemoteRefUpdate rru = r.getRemoteUpdate("refs/heads/newbranch");
+ assertThat(rru.getStatus()).isNotEqualTo(Status.OK);
+ assertThat(rru.getMessage()).contains(msg);
+
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+ RemoteRefUpdate success =
+ push("HEAD:refs/heads/newbranch").getRemoteUpdate("refs/heads/newbranch");
+ assertThat(success.getStatus()).isEqualTo(Status.OK);
+ }
+
+ @Test
public void fastForwardUpdateDenied() throws Exception {
testRepo.branch("HEAD").commit().create();
PushResult r = push("HEAD:refs/heads/master");
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
index 93ce255..33a7dc5 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
@@ -26,7 +26,10 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.Account;
@@ -61,6 +64,7 @@
public class CreateBranchIT extends AbstractDaemonTest {
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private GroupOperations groupOperations;
@Inject private ExtensionRegistry extensionRegistry;
private BranchNameKey testBranch;
@@ -410,6 +414,50 @@
assertThat(ex).hasMessageThat().isEqualTo("ref must match URL");
}
+ @Test
+ public void createBranchRevisionVisibility() throws Exception {
+ AccountGroup.UUID privilegedGroupUuid =
+ groupOperations.newGroup().name(name("privilegedGroup")).create();
+ TestAccount privilegedUser =
+ accountCreator.create(
+ "privilegedUser", "privilegedUser@example.com", "privilegedUser", null);
+ groupOperations.group(privilegedGroupUuid).forUpdate().addMember(privilegedUser.id()).update();
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/heads/secret/*").group(REGISTERED_USERS))
+ .add(allow(Permission.READ).ref("refs/heads/secret/*").group(privilegedGroupUuid))
+ .add(allow(Permission.READ).ref("refs/heads/*").group(REGISTERED_USERS))
+ .add(allow(Permission.CREATE).ref("refs/heads/*").group(REGISTERED_USERS))
+ .add(allow(Permission.PUSH).ref("refs/heads/*").group(REGISTERED_USERS))
+ .update();
+ PushOneCommit push =
+ pushFactory.create(admin.newIdent(), testRepo, "Configure", "file.txt", "contents");
+ PushOneCommit.Result result = push.to("refs/heads/secret/main");
+ result.assertOkStatus();
+ RevCommit secretCommit = result.getCommit();
+ requestScopeOperations.setApiUser(privilegedUser.id());
+ BranchInfo info = gApi.projects().name(project.get()).branch("refs/heads/secret/main").get();
+ assertThat(info.revision).isEqualTo(secretCommit.name());
+ TestAccount unprivileged =
+ accountCreator.create("unprivileged", "unprivileged@example.com", "unprivileged", null);
+ requestScopeOperations.setApiUser(unprivileged.id());
+ assertThrows(
+ ResourceNotFoundException.class,
+ () -> gApi.projects().name(project.get()).branch("refs/heads/secret/main").get());
+ BranchInput branchInput = new BranchInput();
+ branchInput.ref = "public";
+ branchInput.revision = secretCommit.name();
+ assertThrows(
+ AuthException.class,
+ () -> gApi.projects().name(project.get()).branch(branchInput.ref).create(branchInput));
+
+ branchInput.revision = "refs/heads/secret/main";
+ assertThrows(
+ AuthException.class,
+ () -> gApi.projects().name(project.get()).branch(branchInput.ref).create(branchInput));
+ }
+
private void blockCreateReference() throws Exception {
projectOperations
.project(project)
diff --git a/javatests/com/google/gerrit/index/query/AndSourceTest.java b/javatests/com/google/gerrit/index/query/AndSourceTest.java
new file mode 100644
index 0000000..3ae48fb
--- /dev/null
+++ b/javatests/com/google/gerrit/index/query/AndSourceTest.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2022 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.index.query;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.collect.Lists;
+import org.junit.Test;
+
+public class AndSourceTest extends PredicateTest {
+ @Test
+ public void ensureLowerCostPredicateRunsFirst() {
+ TestMatchablePredicate p1 = new TestMatchablePredicate("predicate1", "foo", 10);
+ TestMatchablePredicate p2 = new TestMatchablePredicate("predicate2", "foo", 1);
+ AndSource<String> andSource = new AndSource<>(Lists.newArrayList(p1, p2), null);
+ andSource.match("bar");
+ assertFalse(p1.ranMatch);
+ assertTrue(p2.ranMatch);
+ }
+}
diff --git a/javatests/com/google/gerrit/index/query/PredicateTest.java b/javatests/com/google/gerrit/index/query/PredicateTest.java
index 3ec7f13..e10d774 100644
--- a/javatests/com/google/gerrit/index/query/PredicateTest.java
+++ b/javatests/com/google/gerrit/index/query/PredicateTest.java
@@ -18,7 +18,29 @@
@Ignore
public abstract class PredicateTest {
- protected static final class TestPredicate extends OperatorPredicate<String> {
+ protected static final class TestMatchablePredicate extends TestPredicate
+ implements Matchable<String> {
+ protected int cost;
+ protected boolean ranMatch = false;
+
+ protected TestMatchablePredicate(String name, String value, int cost) {
+ super(name, value);
+ this.cost = cost;
+ }
+
+ @Override
+ public boolean match(String object) {
+ ranMatch = true;
+ return false;
+ }
+
+ @Override
+ public int getCost() {
+ return cost;
+ }
+ }
+
+ protected static class TestPredicate extends OperatorPredicate<String> {
protected TestPredicate(String name, String value) {
super(name, value);
}
diff --git a/javatests/com/google/gerrit/metrics/dropwizard/BUILD b/javatests/com/google/gerrit/metrics/dropwizard/BUILD
index 98d12b2..e236f30 100644
--- a/javatests/com/google/gerrit/metrics/dropwizard/BUILD
+++ b/javatests/com/google/gerrit/metrics/dropwizard/BUILD
@@ -6,7 +6,10 @@
tags = ["metrics"],
visibility = ["//visibility:public"],
deps = [
+ "//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/metrics/dropwizard",
+ "//lib/mockito",
"//lib/truth",
+ "@dropwizard-core//jar",
],
)
diff --git a/javatests/com/google/gerrit/metrics/dropwizard/DropWizardMetricMakerTest.java b/javatests/com/google/gerrit/metrics/dropwizard/DropWizardMetricMakerTest.java
index 9b21bf6..5777779 100644
--- a/javatests/com/google/gerrit/metrics/dropwizard/DropWizardMetricMakerTest.java
+++ b/javatests/com/google/gerrit/metrics/dropwizard/DropWizardMetricMakerTest.java
@@ -15,12 +15,33 @@
package com.google.gerrit.metrics.dropwizard;
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import com.codahale.metrics.MetricRegistry;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
+import com.google.gerrit.metrics.ReservoirType;
+import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+@RunWith(MockitoJUnitRunner.class)
public class DropWizardMetricMakerTest {
- DropWizardMetricMaker metrics =
- new DropWizardMetricMaker(null /* MetricRegistry unused in tests */);
+
+ @Mock MetricsReservoirConfig reservoirConfigMock;
+
+ MetricRegistry registry;
+
+ DropWizardMetricMaker metrics;
+
+ @Before
+ public void setupMocks() {
+ registry = new MetricRegistry();
+ metrics = new DropWizardMetricMaker(registry, reservoirConfigMock);
+ }
@Test
public void shouldSanitizeUnwantedChars() throws Exception {
@@ -41,4 +62,15 @@
assertThat(metrics.sanitizeMetricName("metric//")).isEqualTo("metric");
assertThat(metrics.sanitizeMetricName("metric/submetric/")).isEqualTo("metric/submetric");
}
+
+ @Test
+ public void shouldRequestForReservoirForNewTimer() throws Exception {
+ when(reservoirConfigMock.reservoirType()).thenReturn(ReservoirType.ExponentiallyDecaying);
+
+ metrics.newTimer(
+ "foo",
+ new Description("foo description").setCumulative().setUnit(Description.Units.MILLISECONDS));
+
+ verify(reservoirConfigMock).reservoirType();
+ }
}
diff --git a/javatests/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProviderTest.java b/javatests/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProviderTest.java
new file mode 100644
index 0000000..6402b53
--- /dev/null
+++ b/javatests/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProviderTest.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2022 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.metrics.dropwizard;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.when;
+
+import com.codahale.metrics.ExponentiallyDecayingReservoir;
+import com.codahale.metrics.SlidingTimeWindowArrayReservoir;
+import com.codahale.metrics.SlidingTimeWindowReservoir;
+import com.codahale.metrics.SlidingWindowReservoir;
+import com.codahale.metrics.UniformReservoir;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
+import com.google.gerrit.metrics.ReservoirType;
+import java.time.Duration;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class DropWizardReservoirProviderTest {
+ private static final int SLIDING_WINDOW_INTERVAL = 1;
+ private static final int SLIDING_WINDOW_SIZE = 256;
+
+ @Mock private MetricsReservoirConfig configMock;
+
+ @Test
+ public void shouldInstantiateReservoirProviderBasedOnMetricsConfig() {
+ when(configMock.reservoirType()).thenReturn(ReservoirType.ExponentiallyDecaying);
+ assertThat(DropWizardReservoirProvider.get(configMock))
+ .isInstanceOf(ExponentiallyDecayingReservoir.class);
+
+ when(configMock.reservoirType()).thenReturn(ReservoirType.SlidingTimeWindow);
+ when(configMock.reservoirWindow()).thenReturn(Duration.ofMinutes(1));
+ assertThat(DropWizardReservoirProvider.get(configMock))
+ .isInstanceOf(SlidingTimeWindowReservoir.class);
+
+ when(configMock.reservoirType()).thenReturn(ReservoirType.SlidingTimeWindowArray);
+ when(configMock.reservoirWindow()).thenReturn(Duration.ofMinutes(SLIDING_WINDOW_INTERVAL));
+ assertThat(DropWizardReservoirProvider.get(configMock))
+ .isInstanceOf(SlidingTimeWindowArrayReservoir.class);
+
+ when(configMock.reservoirType()).thenReturn(ReservoirType.SlidingWindow);
+ when(configMock.reservoirSize()).thenReturn(SLIDING_WINDOW_SIZE);
+ assertThat(DropWizardReservoirProvider.get(configMock))
+ .isInstanceOf(SlidingWindowReservoir.class);
+
+ when(configMock.reservoirType()).thenReturn(ReservoirType.Uniform);
+ assertThat(DropWizardReservoirProvider.get(configMock)).isInstanceOf(UniformReservoir.class);
+ }
+}
diff --git a/javatests/com/google/gerrit/metrics/proc/ProcMetricModuleTest.java b/javatests/com/google/gerrit/metrics/proc/ProcMetricModuleTest.java
index 33919e7..ea89ae9 100644
--- a/javatests/com/google/gerrit/metrics/proc/ProcMetricModuleTest.java
+++ b/javatests/com/google/gerrit/metrics/proc/ProcMetricModuleTest.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.Mockito.mock;
import com.codahale.metrics.Counter;
import com.codahale.metrics.Gauge;
@@ -31,7 +32,9 @@
import com.google.gerrit.metrics.Description.FieldOrdering;
import com.google.gerrit.metrics.Field;
import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
import com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker;
+import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
@@ -179,7 +182,14 @@
@Before
public void setup() {
- Injector injector = Guice.createInjector(new DropWizardMetricMaker.ApiModule());
+ Injector injector =
+ Guice.createInjector(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ install(new DropWizardMetricMaker.ApiModule(mock(MetricsReservoirConfig.class)));
+ }
+ });
LifecycleManager mgr = new LifecycleManager();
mgr.add(injector);
diff --git a/javatests/com/google/gerrit/server/events/EventTypesTest.java b/javatests/com/google/gerrit/server/events/EventTypesTest.java
index c822d6c..7e97f184 100644
--- a/javatests/com/google/gerrit/server/events/EventTypesTest.java
+++ b/javatests/com/google/gerrit/server/events/EventTypesTest.java
@@ -48,4 +48,16 @@
Class<?> clazz = EventTypes.getClass("does-not-exist-event");
assertThat(clazz).isNull();
}
+
+ @Test
+ public void getRegisteredEventsGetsANewlyRegisteredEvent() {
+ EventTypes.register(TestEvent.TYPE, TestEvent.class);
+ assertThat(EventTypes.getRegisteredEvents()).containsEntry(TestEvent.TYPE, TestEvent.class);
+ }
+
+ @Test
+ public void getRegisteredEventsGetsTypeGivenAtRegistration() {
+ EventTypes.register("alternate-type", TestEvent.class);
+ assertThat(EventTypes.getRegisteredEvents()).containsEntry("alternate-type", TestEvent.class);
+ }
}
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
index e2eaa1d..4894eb3 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
@@ -97,7 +97,7 @@
Predicate<ChangeData> in = parse("foo:a file:b");
Predicate<ChangeData> out = rewrite(in);
assertThat(AndChangeSource.class).isSameInstanceAs(out.getClass());
- assertThat(out.getChildren()).containsExactly(query(in.getChild(1)), in.getChild(0)).inOrder();
+ assertThat(out.getChildren()).containsExactly(query(parse("file:b")), parse("foo:a")).inOrder();
}
@Test
@@ -126,9 +126,9 @@
.inOrder();
// Same at the assertions above, that were added for readability
- assertThat(out.getChild(0)).isEqualTo(query(in.getChild(0)));
+ assertThat(out.getChild(0)).isEqualTo(query(parse("-status:abandoned")));
assertThat(indexedSubTree.getChildren())
- .containsExactly(query(in.getChild(1).getChild(1)), in.getChild(1).getChild(0))
+ .containsExactly(query(parse("file:b")), parse("foo:a"))
.inOrder();
}
@@ -137,7 +137,9 @@
Predicate<ChangeData> in = parse("-foo:a (file:b OR file:c)");
Predicate<ChangeData> out = rewrite(in);
assertThat(out.getClass()).isSameInstanceAs(AndChangeSource.class);
- assertThat(out.getChildren()).containsExactly(query(in.getChild(1)), in.getChild(0)).inOrder();
+ assertThat(out.getChildren())
+ .containsExactly(query(parse("file:b OR file:c")), parse("-foo:a"))
+ .inOrder();
}
@Test
@@ -146,7 +148,8 @@
Predicate<ChangeData> out = rewrite(in);
assertThat(out.getClass()).isSameInstanceAs(OrSource.class);
assertThat(out.getChildren())
- .containsExactly(query(or(in.getChild(0), in.getChild(2))), in.getChild(1), in.getChild(3))
+ .containsExactly(
+ query(or(parse("file:a"), parse("file:c"))), parse("foo:b"), parse("foo:d"))
.inOrder();
}
@@ -156,7 +159,7 @@
Predicate<ChangeData> out = rewrite(in);
assertThat(AndChangeSource.class).isSameInstanceAs(out.getClass());
assertThat(out.getChildren())
- .containsExactly(query(and(in.getChild(0), in.getChild(2))), in.getChild(1))
+ .containsExactly(query(and(parse("status:new"), parse("file:a"))), parse("bar:p"))
.inOrder();
}
@@ -166,7 +169,7 @@
Predicate<ChangeData> out = rewrite(in);
assertThat(out.getClass()).isEqualTo(AndChangeSource.class);
assertThat(out.getChildren())
- .containsExactly(query(and(in.getChild(0), in.getChild(2))), in.getChild(1))
+ .containsExactly(query(and(parse("status:new"), parse("file:a"))), parse("bar:p"))
.inOrder();
}
@@ -176,7 +179,7 @@
Predicate<ChangeData> out = rewrite(in);
assertThat(out.getClass()).isEqualTo(AndChangeSource.class);
assertThat(out.getChildren())
- .containsExactly(query(and(in.getChild(0), in.getChild(2))), in.getChild(1))
+ .containsExactly(query(and(parse("status:new OR file:a"), parse("file:b"))), parse("bar:p"))
.inOrder();
}
@@ -186,7 +189,7 @@
Predicate<ChangeData> out = rewrite(in, options(0, 5));
assertThat(out.getClass()).isEqualTo(AndChangeSource.class);
assertThat(out.getChildren())
- .containsExactly(query(in.getChild(1), 5), parse("limit:5"), parse("limit:5"))
+ .containsExactly(query(parse("file:a"), 5), parse("limit:5"), parse("limit:5"))
.inOrder();
}
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index 16f7199..e0a69a0 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -646,7 +646,10 @@
.getRaw(
Account.id(userInfo._accountId),
QueryOptions.create(
- IndexConfig.createDefault(), 0, 1, schema.getStoredFields().keySet()));
+ IndexConfig.fromConfig(config).build(),
+ 0,
+ 1,
+ schema.getStoredFields().keySet()));
assertThat(rawFields).isPresent();
if (schema.useLegacyNumericFields()) {
diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD
index ec1f612..802bf54 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -21,6 +21,7 @@
"//java/com/google/gerrit/acceptance/config",
"//java/com/google/gerrit/acceptance/testsuite/project",
"//java/com/google/gerrit/common:annotations",
+ "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/httpd",
diff --git a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
index 6a83fb9..9717bfb 100644
--- a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
@@ -15,13 +15,18 @@
package com.google.gerrit.server.query.change;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
+import static com.google.gerrit.common.data.GlobalCapability.QUERY_LIMIT;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.testing.InMemoryModule;
import com.google.gerrit.testing.InMemoryRepositoryManager.Repo;
import com.google.inject.Guice;
+import com.google.inject.Inject;
import com.google.inject.Injector;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
@@ -29,6 +34,8 @@
import org.junit.Test;
public abstract class LuceneQueryChangesTest extends AbstractQueryChangesTest {
+ @Inject protected AllProjectsName allProjects;
+
@Override
protected Injector createInjector() {
Config luceneConfig = new Config(config);
@@ -66,4 +73,29 @@
() -> assertQuery("owner: \"" + nameEmail + "\"\\", change1));
assertThat(thrown).hasMessageThat().contains("Cannot create full-text query with value: \\");
}
+
+ @Test
+ public void openAndClosedChanges() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+
+ // create 3 closed changes
+ Change change1 = insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
+ Change change2 = insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
+ Change change3 = insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
+
+ // create 3 new changes
+ Change change4 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW));
+ Change change5 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW));
+ Change change6 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW));
+
+ // Set queryLimit to 1
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, 1))
+ .update();
+
+ Change[] expected = new Change[] {change6, change5, change4, change3, change2, change1};
+ assertQuery(newQuery("project:repo").withNoLimit(), expected);
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
index 568b5a0..6e4fec1 100644
--- a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
+++ b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
@@ -374,7 +374,7 @@
.getRaw(
uuid,
QueryOptions.create(
- IndexConfig.createDefault(),
+ IndexConfig.fromConfig(config).build(),
0,
10,
indexes.getSearchIndex().getSchema().getStoredFields().keySet()));
