Merge branch 'stable-3.7'
* stable-3.7: (35 commits)
Set version to 3.7.0-rc1
Set version to 3.6.2
Add an option to periodically warm the project_list cache
Improve test coverage for internal change query pagination
prolog_rules cache: only use memoryLimit from cache.projects
Revert "Ensure that quoted messages show up in the reply comment"
Set version to 3.5.4-SNAPSHOT
Set version to 3.5.3
Don't update the last updated timestamp when running copy-approvals
CreateRefControl: Check permissions on source refs
Set version to 3.4.7-SNAPSHOT
Set version to 3.4.6
Fix javadoc summary for Google Java Style
Don't depend on predicate order as they get sorted based on cost
Fix DefaultMemoryCacheFactory to correctly set refreshAfterWrite
Update git submodules
Fix Bazel build on Apple M2 ARM64 chip
Shortcut CreateRefControl#checkCreateCommit only for push processing
Paginate internal change index queries
Fix index pagination to set 'more_changes' correctly
...
Release-Notes: skip
Change-Id: I7bd89a8be78f0c00f7477c6b84b137c80944d42e
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index fa1807f..234389e 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1188,7 +1188,7 @@
cache `"prolog_rules"`::
+
Caches parsed `rules.pl` contents for each project. This cache uses the same
-size as the `projects` cache, and cannot be configured independently.
+size as the `projects` cache when `cache.prolog_rules.memoryLimit` is not set.
cache `"pure_revert"`::
+
@@ -1309,6 +1309,21 @@
+
Default is the number of CPUs.
+[[cache.project_list.interval]]cache.project_list.interval::
++
+The link:#schedule-configuration-interval[interval] for running
+the project_list cache warmer.
+
+By default, if `cache.project_list.maxAge` is set, `interval` will be set to
+half its value. If `cache.project_list.maxAge` is not set or `interval` is set
+to `-1`, it is disabled.
+
+[[cache.project_list.startTime]]cache.project_list.startTime::
++
+The link:#schedule-configuration-startTime[start time] for running
+the project_list cache warmer.
+
+Default is 00:00 if the project_list cache warmer is enabled.
[[capability]]
=== Section capability
@@ -4017,6 +4032,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/Documentation/user-search.txt b/Documentation/user-search.txt
index 1c4750b..9527a56 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -343,7 +343,9 @@
By default full text matching is used, but regular expressions can be
enabled by starting with `^`.
The link:http://www.brics.dk/automaton/[dk.brics.automaton library,role=external,window=_blank]
-is used for the evaluation of such patterns.
+is used for the evaluation of such patterns. Note, that searching with
+regular expressions is limited to the first 32766 bytes of the
+commit message due to limitations in Lucene.
[[comment]]
comment:'TEXT'::
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/entities/StoredCommentLinkInfo.java b/java/com/google/gerrit/entities/StoredCommentLinkInfo.java
index c653a41..75bc034 100644
--- a/java/com/google/gerrit/entities/StoredCommentLinkInfo.java
+++ b/java/com/google/gerrit/entities/StoredCommentLinkInfo.java
@@ -86,7 +86,7 @@
* on it's own.
*/
public static StoredCommentLinkInfo disabled(String name) {
- return builder(name).setOverrideOnly(true).build();
+ return builder(name).setOverrideOnly(true).setEnabled(false).build();
}
/** Creates and returns a new {@link StoredCommentLinkInfo.Builder} instance. */
diff --git a/java/com/google/gerrit/index/query/AndPredicate.java b/java/com/google/gerrit/index/query/AndPredicate.java
index fd8ca96..23ae312 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);
@@ -116,6 +120,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 45a00b6..959694b 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 6419468..755ae7b 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -38,6 +38,7 @@
import com.google.gerrit.entities.converter.ChangeProtoConverter;
import com.google.gerrit.entities.converter.ProtoConverter;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.PaginationType;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.SchemaFieldDefs.SchemaField;
@@ -310,6 +311,7 @@
private final QueryOptions opts;
private final Sort sort;
private final Function<Document, FieldBundle> rawDocumentMapper;
+ private final boolean isSearchAfterPagination;
private QuerySource(
List<ChangeSubIndex> indexes,
@@ -324,6 +326,8 @@
this.opts = opts;
this.sort = sort;
this.rawDocumentMapper = rawDocumentMapper;
+ this.isSearchAfterPagination =
+ opts.config().paginationType().equals(PaginationType.SEARCH_AFTER);
}
@Override
@@ -410,28 +414,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
- && ((HashMap<?, ?>) opts.searchAfter()).get(subIndex) instanceof ScoreDoc) {
- hits[i] =
- searchers[i].searchAfter(
- (ScoreDoc) ((HashMap<?, ?>) 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++) {
@@ -451,6 +461,24 @@
}
}
}
+
+ /**
+ * 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
+ && ((Map<?, ?>) opts.searchAfter()).get(subIndex) instanceof ScoreDoc) {
+ return (ScoreDoc) ((Map<?, ?>) 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/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index f93886c..18919ed 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -57,6 +57,7 @@
import com.google.gerrit.server.config.DefaultUrlFormatter.DefaultUrlFormatterModule;
import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
import com.google.gerrit.server.config.EnablePeerIPInReflogRecordProvider;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
import com.google.gerrit.server.config.SysExecutorModule;
@@ -95,6 +96,7 @@
import com.google.gerrit.server.rules.SubmitRule;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.inject.Injector;
+import com.google.inject.Key;
import com.google.inject.Module;
import com.google.inject.TypeLiteral;
import com.google.inject.util.Providers;
@@ -102,6 +104,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Set;
+import org.eclipse.jgit.lib.Config;
/** Module for programs that perform batch operations on a site. */
public class BatchProgramModule extends FactoryModule {
@@ -203,7 +206,7 @@
// Submit rules
DynamicSet.setOf(binder(), SubmitRule.class);
factory(SubmitRuleEvaluator.Factory.class);
- modules.add(new PrologModule());
+ modules.add(new PrologModule(getConfig()));
modules.add(new DefaultSubmitRuleModule());
modules.add(new IgnoreSelfApprovalRuleModule());
@@ -226,4 +229,8 @@
.stream()
.forEach(this::install);
}
+
+ protected Config getConfig() {
+ return parentInjector.getInstance(Key.get(Config.class, GerritServerConfig.class));
+ }
}
diff --git a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
index f672d11..3b104dd 100644
--- a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
+++ b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
@@ -93,7 +93,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/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 8fee2d8..e5b063b 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -284,7 +284,7 @@
install(new GroupDbModule());
install(new GroupModule());
install(new NoteDbModule());
- install(new PrologModule());
+ install(new PrologModule(cfg));
install(new DefaultSubmitRuleModule());
install(new IgnoreSelfApprovalRuleModule());
install(new ReceiveCommitsModule());
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 cad52f6..61a41cc 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -1450,7 +1450,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/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 4c38346..670f472a 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -85,6 +85,7 @@
import java.sql.Timestamp;
import java.time.Instant;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
@@ -119,6 +120,12 @@
private static final Gson GSON = OutputFormat.JSON_COMPACT.newGson();
+ /**
+ * To avoid the non-google dependency on org.apache.lucene.index.IndexWriter.MAX_TERM_LENGTH it is
+ * redefined here.
+ */
+ public static final int MAX_TERM_LENGTH = (1 << 15) - 2;
+
// TODO: Rename LEGACY_ID to NUMERIC_ID
/** Legacy change ID. */
public static final FieldDef<ChangeData, String> LEGACY_ID_STR =
@@ -899,7 +906,8 @@
/** Commit message of the current patch set. */
public static final FieldDef<ChangeData, String> COMMIT_MESSAGE_EXACT =
- exact(ChangeQueryBuilder.FIELD_MESSAGE_EXACT).build(ChangeData::commitMessage);
+ exact(ChangeQueryBuilder.FIELD_MESSAGE_EXACT)
+ .build(cd -> truncateStringValueToMaxTermLength(cd.commitMessage()));
/** Summary or inline comment. */
public static final FieldDef<ChangeData, Iterable<String>> COMMENT =
@@ -1420,4 +1428,46 @@
private static AllUsersName allUsers(ChangeData cd) {
return cd.getAllUsersNameForIndexing();
}
+
+ private static String truncateStringValueToMaxTermLength(String str) {
+ return truncateStringValue(str, MAX_TERM_LENGTH);
+ }
+
+ @VisibleForTesting
+ static String truncateStringValue(String str, int maxBytes) {
+ if (maxBytes < 0) {
+ throw new IllegalArgumentException("maxBytes < 0 not allowed");
+ }
+
+ if (maxBytes == 0) {
+ return "";
+ }
+
+ if (str.length() > maxBytes) {
+ if (Character.isHighSurrogate(str.charAt(maxBytes - 1))) {
+ str = str.substring(0, maxBytes - 1);
+ } else {
+ str = str.substring(0, maxBytes);
+ }
+ }
+ byte[] strBytes = str.getBytes(UTF_8);
+ if (strBytes.length > maxBytes) {
+ while (maxBytes > 0 && (strBytes[maxBytes] & 0xC0) == 0x80) {
+ maxBytes -= 1;
+ }
+ if (maxBytes > 0) {
+ if (strBytes.length >= maxBytes && (strBytes[maxBytes - 1] & 0xE0) == 0xC0) {
+ maxBytes -= 1;
+ }
+ if (strBytes.length >= maxBytes && (strBytes[maxBytes - 1] & 0xF0) == 0xE0) {
+ maxBytes -= 1;
+ }
+ if (strBytes.length >= maxBytes && (strBytes[maxBytes - 1] & 0xF8) == 0xF0) {
+ maxBytes -= 1;
+ }
+ }
+ return new String(Arrays.copyOfRange(strBytes, 0, maxBytes), UTF_8);
+ }
+ return str;
+ }
}
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index 3c76e04..eebaa8f 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -21,6 +21,8 @@
import static java.util.stream.Collectors.toCollection;
import com.google.auto.value.AutoValue;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -133,15 +135,17 @@
// Perform an initial ref filtering with all the refs the caller asked for. If we find tags that
// we have to investigate separately (deferred tags) then perform a reachability check starting
// from all visible branches (refs/heads/*).
- ImmutableMap<Change.Id, ChangeData> visibleChanges =
- GitVisibleChangeFilter.getVisibleChanges(
- searchingChangeDataProvider,
- changeNotesFactory,
- changeDataFactory,
- projectState.getNameKey(),
- permissionBackendForProject,
- repo,
- changes(refs));
+ Supplier<ImmutableMap<Change.Id, ChangeData>> visibleChanges =
+ Suppliers.memoize(
+ () ->
+ GitVisibleChangeFilter.getVisibleChanges(
+ searchingChangeDataProvider,
+ changeNotesFactory,
+ changeDataFactory,
+ projectState.getNameKey(),
+ permissionBackendForProject,
+ repo,
+ changes(refs)));
Result initialRefFilter = filterRefs(new ArrayList<>(refs), opts, visibleChanges);
ImmutableList.Builder<Ref> visibleRefs = ImmutableList.builder();
visibleRefs.addAll(initialRefFilter.visibleRefs());
@@ -183,7 +187,9 @@
* compute will be returned as part of {@link Result#visibleRefs()}.
*/
Result filterRefs(
- List<Ref> refs, RefFilterOptions opts, ImmutableMap<Change.Id, ChangeData> visibleChanges)
+ List<Ref> refs,
+ RefFilterOptions opts,
+ Supplier<ImmutableMap<Change.Id, ChangeData>> visibleChanges)
throws PermissionBackendException {
logger.atFinest().log("Filter refs (refs = %s)", refs);
if (!projectState.statePermitsRead()) {
@@ -254,9 +260,9 @@
// most recent changes).
if (hasAccessDatabase) {
resultRefs.add(ref);
- } else if (!visibleChanges.containsKey(changeId)) {
+ } else if (!visibleChanges.get().containsKey(changeId)) {
logger.atFinest().log("Filter out invisible change ref %s", refName);
- } else if (RefNames.isRefsEdit(refName) && !visibleEdit(refName, visibleChanges)) {
+ } else if (RefNames.isRefsEdit(refName) && !visibleEdit(refName, visibleChanges.get())) {
logger.atFinest().log("Filter out invisible change edit ref %s", refName);
} else {
// Change is visible
diff --git a/java/com/google/gerrit/server/project/CreateRefControl.java b/java/com/google/gerrit/server/project/CreateRefControl.java
index 0d015d4..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,12 +157,33 @@
Repository repo,
RevCommit commit,
Project.NameKey project,
- PermissionBackend.ForRef forRef)
+ PermissionBackend.ForRef forRef,
+ boolean forPush)
throws AuthException, PermissionBackendException, IOException {
- // 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.
- if (forRef.test(RefPermission.UPDATE)) {
- return;
+ try {
+ // 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.
}
if (reachable.fromRefs(
project,
@@ -142,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/project/PeriodicProjectListCacheWarmer.java b/java/com/google/gerrit/server/project/PeriodicProjectListCacheWarmer.java
new file mode 100644
index 0000000..caffb45
--- /dev/null
+++ b/java/com/google/gerrit/server/project/PeriodicProjectListCacheWarmer.java
@@ -0,0 +1,100 @@
+// 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.project;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.ScheduleConfig;
+import com.google.gerrit.server.git.WorkQueue;
+import com.google.inject.Inject;
+import java.time.Duration;
+import org.eclipse.jgit.lib.Config;
+
+public class PeriodicProjectListCacheWarmer implements Runnable {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ public static class LifeCycle implements LifecycleListener {
+ protected final Config config;
+ protected final WorkQueue queue;
+ protected final PeriodicProjectListCacheWarmer runner;
+
+ @Inject
+ LifeCycle(
+ @GerritServerConfig Config config, WorkQueue queue, PeriodicProjectListCacheWarmer runner) {
+ this.config = config;
+ this.queue = queue;
+ this.runner = runner;
+ }
+
+ @Override
+ public void start() {
+ long interval = -1L;
+ String intervalString = config.getString("cache", ProjectCacheImpl.CACHE_LIST, "interval");
+ if (!"-1".equals(intervalString)) {
+ long maxAge =
+ config.getTimeUnit("cache", ProjectCacheImpl.CACHE_LIST, "maxAge", -1L, MILLISECONDS);
+ interval =
+ config.getTimeUnit(
+ "cache",
+ ProjectCacheImpl.CACHE_LIST,
+ "interval",
+ getHalfDuration(maxAge),
+ MILLISECONDS);
+ }
+
+ if (interval == -1L) {
+ logger.atWarning().log("project_list cache warmer is disabled");
+ return;
+ }
+
+ String startTime = config.getString("cache", ProjectCacheImpl.CACHE_LIST, "startTime");
+ if (startTime == null) {
+ startTime = "00:00";
+ }
+
+ runner.run();
+ queue.scheduleAtFixedRate(runner, ScheduleConfig.Schedule.createOrFail(interval, startTime));
+ }
+
+ @Override
+ public void stop() {
+ // handled by WorkQueue.stop() already
+ }
+
+ private long getHalfDuration(long duration) {
+ if (duration < 0) {
+ return duration;
+ }
+ return Duration.ofMillis(duration).dividedBy(2L).toMillis();
+ }
+ }
+
+ protected final ProjectCache cache;
+
+ @Inject
+ PeriodicProjectListCacheWarmer(ProjectCache cache) {
+ this.cache = cache;
+ }
+
+ @Override
+ public void run() {
+ logger.atFine().log("Loading project_list cache");
+ cache.all();
+ logger.atFine().log("Finished loading project_list cache");
+ }
+}
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 3aa3783..52a524f 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -95,7 +95,7 @@
public static final String PERSISTED_CACHE_NAME = "persisted_projects";
- private static final String CACHE_LIST = "project_list";
+ public static final String CACHE_LIST = "project_list";
public static Module module() {
return new CacheModule() {
@@ -147,6 +147,13 @@
listener().to(ProjectCacheWarmer.class);
}
});
+ install(
+ new LifecycleModule() {
+ @Override
+ protected void configure() {
+ listener().to(PeriodicProjectListCacheWarmer.LifeCycle.class);
+ }
+ });
}
};
}
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 2184938..c39b1f4 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateBranch.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
@@ -51,6 +51,7 @@
import java.util.Map;
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;
@@ -134,7 +135,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/java/com/google/gerrit/server/rules/PrologModule.java b/java/com/google/gerrit/server/rules/PrologModule.java
index 5cf4220..ebb5ec0 100644
--- a/java/com/google/gerrit/server/rules/PrologModule.java
+++ b/java/com/google/gerrit/server/rules/PrologModule.java
@@ -18,12 +18,19 @@
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.rules.RulesCache.RulesCacheModule;
+import org.eclipse.jgit.lib.Config;
public class PrologModule extends FactoryModule {
+ protected final Config config;
+
+ public PrologModule(Config config) {
+ this.config = config;
+ }
+
@Override
protected void configure() {
install(new EnvironmentModule());
- install(new RulesCacheModule());
+ install(new RulesCacheModule(config));
bind(PrologEnvironment.Args.class);
factory(PrologRuleEvaluator.Factory.class);
diff --git a/java/com/google/gerrit/server/rules/RulesCache.java b/java/com/google/gerrit/server/rules/RulesCache.java
index 706804a..773c75e 100644
--- a/java/com/google/gerrit/server/rules/RulesCache.java
+++ b/java/com/google/gerrit/server/rules/RulesCache.java
@@ -17,6 +17,7 @@
import static com.googlecode.prolog_cafe.lang.PrologMachineCopy.save;
import com.google.common.base.Joiner;
+import com.google.common.base.Strings;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.Project;
@@ -73,11 +74,25 @@
@Singleton
public class RulesCache {
public static class RulesCacheModule extends CacheModule {
+ protected final Config config;
+
+ public RulesCacheModule(Config config) {
+ this.config = config;
+ }
+
@Override
protected void configure() {
- cache(RulesCache.CACHE_NAME, ObjectId.class, PrologMachineCopy.class)
- // This cache is auxiliary to the project cache, so size it the same.
- .configKey(ProjectCacheImpl.CACHE_NAME);
+ if (has(ProjectCacheImpl.CACHE_NAME, "memoryLimit")) {
+ // As this cache is auxiliary to the project cache, so size it the same when available
+ cache(RulesCache.CACHE_NAME, ObjectId.class, PrologMachineCopy.class)
+ .maximumWeight(config.getLong("cache", ProjectCacheImpl.CACHE_NAME, "memoryLimit", 0));
+ } else {
+ cache(RulesCache.CACHE_NAME, ObjectId.class, PrologMachineCopy.class);
+ }
+ }
+
+ private boolean has(String name, String var) {
+ return !Strings.isNullOrEmpty(config.getString("cache", name, var));
}
}
diff --git a/java/com/google/gerrit/sshd/SshLogJsonLayout.java b/java/com/google/gerrit/sshd/SshLogJsonLayout.java
index 0edad6f..321cf56 100644
--- a/java/com/google/gerrit/sshd/SshLogJsonLayout.java
+++ b/java/com/google/gerrit/sshd/SshLogJsonLayout.java
@@ -84,7 +84,7 @@
String metricString = getMdcString(event, P_MESSAGE);
if (metricString != null && !metricString.isEmpty()) {
- List<String> ssh_metrics = SPLITTER.splitToList(" ");
+ List<String> ssh_metrics = SPLITTER.splitToList(metricString);
this.timeNegotiating = ssh_metrics.get(0);
this.timeSearchReuse = ssh_metrics.get(1);
this.timeSearchSizes = ssh_metrics.get(2);
diff --git a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
index e76e2f6..f94aa12 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 c30b46c2..d45c90b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
@@ -31,7 +31,10 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ExtensionRegistry;
import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
+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;
@@ -68,6 +71,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;
@@ -513,6 +517,50 @@
}
}
+ @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 171ca27..8cb8d17 100644
--- a/javatests/com/google/gerrit/index/query/PredicateTest.java
+++ b/javatests/com/google/gerrit/index/query/PredicateTest.java
@@ -18,7 +18,30 @@
@Ignore
public abstract class PredicateTest {
- protected static final class TestPredicate extends OperatorPredicate<String> {
+ @SuppressWarnings("ProtectedMembersInFinalClass")
+ 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> {
private 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/ChangeFieldTest.java b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
index 3364540..0bdf5cd 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
@@ -168,6 +168,55 @@
assertThat(ChangeField.DELETED.setIfPossible(cd, new FakeStoredValue(null))).isTrue();
}
+ @Test
+ public void shortStringIsNotTruncated() {
+ assertThat(ChangeField.truncateStringValue("short string", 20)).isEqualTo("short string");
+ String two_byte_str = String.format("short string %s", new String(Character.toChars(956)));
+ assertThat(ChangeField.truncateStringValue(two_byte_str, 20)).isEqualTo(two_byte_str);
+ String three_byte_str = String.format("short string %s", new String(Character.toChars(43421)));
+ assertThat(ChangeField.truncateStringValue(three_byte_str, 20)).isEqualTo(three_byte_str);
+ String four_byte_str = String.format("short string %s", new String(Character.toChars(132878)));
+ assertThat(ChangeField.truncateStringValue(four_byte_str, 20)).isEqualTo(four_byte_str);
+ assertThat(ChangeField.truncateStringValue("", 6)).isEqualTo("");
+ assertThat(ChangeField.truncateStringValue("", 0)).isEqualTo("");
+ }
+
+ @Test
+ public void longStringIsTruncated() {
+ assertThat(ChangeField.truncateStringValue("longer string", 6)).isEqualTo("longer");
+ assertThat(ChangeField.truncateStringValue("longer string", 0)).isEqualTo("");
+
+ String two_byte_str =
+ String.format(
+ "multibytechars %1$s%1$s%1$s%1$s present", new String(Character.toChars(956)));
+ assertThat(ChangeField.truncateStringValue(two_byte_str, 16)).isEqualTo("multibytechars ");
+ assertThat(ChangeField.truncateStringValue(two_byte_str, 17))
+ .isEqualTo(String.format("multibytechars %1$s", new String(Character.toChars(956))));
+ assertThat(ChangeField.truncateStringValue(two_byte_str, 18))
+ .isEqualTo(String.format("multibytechars %1$s", new String(Character.toChars(956))));
+
+ String three_byte_str =
+ String.format(
+ "multibytechars %1$s%1$s%1$s%1$s present", new String(Character.toChars(43421)));
+ assertThat(ChangeField.truncateStringValue(three_byte_str, 16)).isEqualTo("multibytechars ");
+ assertThat(ChangeField.truncateStringValue(three_byte_str, 17)).isEqualTo("multibytechars ");
+ assertThat(ChangeField.truncateStringValue(three_byte_str, 18))
+ .isEqualTo(String.format("multibytechars %1$s", new String(Character.toChars(43421))));
+ assertThat(ChangeField.truncateStringValue(three_byte_str, 21))
+ .isEqualTo(String.format("multibytechars %1$s%1$s", new String(Character.toChars(43421))));
+
+ String four_byte_str =
+ String.format(
+ "multibytechars %1$s%1$s%1$s%1$s present", new String(Character.toChars(132878)));
+ assertThat(ChangeField.truncateStringValue(four_byte_str, 16)).isEqualTo("multibytechars ");
+ assertThat(ChangeField.truncateStringValue(four_byte_str, 17)).isEqualTo("multibytechars ");
+ assertThat(ChangeField.truncateStringValue(four_byte_str, 18)).isEqualTo("multibytechars ");
+ assertThat(ChangeField.truncateStringValue(four_byte_str, 19))
+ .isEqualTo(String.format("multibytechars %1$s", new String(Character.toChars(132878))));
+ assertThat(ChangeField.truncateStringValue(four_byte_str, 23))
+ .isEqualTo(String.format("multibytechars %1$s%1$s", new String(Character.toChars(132878))));
+ }
+
private static SubmitRecord record(SubmitRecord.Status status, SubmitRecord.Label... labels) {
SubmitRecord r = new SubmitRecord();
r.status = status;
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
index 27e5b4f..3d0fd25 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/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
index c8cc713..ef92139 100644
--- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
+++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
@@ -728,6 +728,7 @@
ProjectConfig cfg = read(rev);
assertThat(cfg.getCommentLinkSections())
.containsExactly(StoredCommentLinkInfo.enabled("bugzilla"));
+ assertThat(Iterables.getOnlyElement(cfg.getCommentLinkSections()).getEnabled()).isNull();
}
@Test
@@ -739,6 +740,7 @@
ProjectConfig cfg = read(rev);
assertThat(cfg.getCommentLinkSections())
.containsExactly(StoredCommentLinkInfo.disabled("bugzilla"));
+ assertThat(Iterables.getOnlyElement(cfg.getCommentLinkSections()).getEnabled()).isFalse();
}
@Test
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index 322a934..b0e705b 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -630,7 +630,8 @@
.getSearchIndex()
.getRaw(
Account.id(userInfo._accountId),
- QueryOptions.create(IndexConfig.createDefault(), 0, 1, schema.getStoredFields()));
+ QueryOptions.create(
+ IndexConfig.fromConfig(config).build(), 0, 1, schema.getStoredFields()));
assertThat(rawFields).isPresent();
if (schema.hasField(AccountField.ID_FIELD_SPEC)) {
diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD
index d3c6d84..32a646e 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/FakeQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
index af42afb..262fb9d9 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
@@ -99,10 +99,37 @@
AbstractFakeIndex<?, ?, ?> idx =
(AbstractFakeIndex<?, ?, ?>) changeIndexCollection.getSearchIndex();
- // 2 index searches are expected. The first index search will run with size 2 (i.e.
- // the configured query-limit), and then we will paginate to get the remaining 2
+ // 2 index searches are expected. The first index search will run with size 3 (i.e.
+ // the configured query-limit+1), and then we will paginate to get the remaining
// changes with the second index search.
newQuery("status:new").withNoLimit().get();
assertThat(idx.getQueryCount()).isEqualTo(2);
}
+
+ @Test
+ @UseClockStep
+ @SuppressWarnings("unchecked")
+ public void internalQueriesPaginate() throws Exception {
+ // create 4 changes
+ TestRepository<InMemoryRepositoryManager.Repo> testRepo = createProject("repo");
+ insert(testRepo, newChange(testRepo));
+ insert(testRepo, newChange(testRepo));
+ insert(testRepo, newChange(testRepo));
+ insert(testRepo, newChange(testRepo));
+
+ // Set queryLimit to 2
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, 2))
+ .update();
+
+ AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
+
+ // 2 index searches are expected. The first index search will run with size 3 (i.e.
+ // the configured query-limit+1), and then we will paginate to get the remaining
+ // changes with the second index search.
+ queryProvider.get().query(queryBuilder.parse("status:new"));
+ assertThat(idx.getQueryCount()).isEqualTo(2);
+ }
}
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 a7b73d6..1ca4571 100644
--- a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
+++ b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
@@ -382,7 +382,7 @@
.getRaw(
uuid,
QueryOptions.create(
- IndexConfig.createDefault(),
+ IndexConfig.fromConfig(config).build(),
0,
10,
indexes.getSearchIndex().getSchema().getStoredFields()));
diff --git a/tools/BUILD b/tools/BUILD
index c06cfdb..f2d887e 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -1,5 +1,6 @@
load(
"@bazel_tools//tools/jdk:default_java_toolchain.bzl",
+ "NONPREBUILT_TOOLCHAIN_CONFIGURATION",
"default_java_toolchain",
)
load("@rules_java//java:defs.bzl", "java_package_configuration")
@@ -8,6 +9,7 @@
default_java_toolchain(
name = "error_prone_warnings_toolchain_java11",
+ configuration = NONPREBUILT_TOOLCHAIN_CONFIGURATION,
package_configuration = [
":error_prone",
],
diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml
index cae04ef..829bbde 100644
--- a/tools/maven/gerrit-acceptance-framework_pom.xml
+++ b/tools/maven/gerrit-acceptance-framework_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-acceptance-framework</artifactId>
- <version>3.6.2-SNAPSHOT</version>
+ <version>3.7.0-rc1</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Acceptance Test Framework</name>
<description>Framework for Gerrit's acceptance tests</description>
diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml
index ff629ca..aab1146 100644
--- a/tools/maven/gerrit-extension-api_pom.xml
+++ b/tools/maven/gerrit-extension-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-extension-api</artifactId>
- <version>3.6.2-SNAPSHOT</version>
+ <version>3.7.0-rc1</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Extension API</name>
<description>API for Gerrit Extensions</description>
diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml
index 170cea0..968fb9f 100644
--- a/tools/maven/gerrit-plugin-api_pom.xml
+++ b/tools/maven/gerrit-plugin-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-plugin-api</artifactId>
- <version>3.6.2-SNAPSHOT</version>
+ <version>3.7.0-rc1</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Plugin API</name>
<description>API for Gerrit Plugins</description>
diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml
index a8c2fe1..ec8fde2 100644
--- a/tools/maven/gerrit-war_pom.xml
+++ b/tools/maven/gerrit-war_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-war</artifactId>
- <version>3.6.2-SNAPSHOT</version>
+ <version>3.7.0-rc1</version>
<packaging>war</packaging>
<name>Gerrit Code Review - WAR</name>
<description>Gerrit WAR</description>
diff --git a/version.bzl b/version.bzl
index 5940f9e..42a6651 100644
--- a/version.bzl
+++ b/version.bzl
@@ -2,4 +2,4 @@
# Used by :api_install and :api_deploy targets
# when talking to the destination repository.
#
-GERRIT_VERSION = "3.6.2-SNAPSHOT"
+GERRIT_VERSION = "3.7.0-rc1"
