Merge "Return 400 BAD_REQUEST when a user cannot be added as reviewer by email" into stable-3.9
diff --git a/Documentation/dev-processes.txt b/Documentation/dev-processes.txt
index 41543ab..8b7ba4b 100644
--- a/Documentation/dev-processes.txt
+++ b/Documentation/dev-processes.txt
@@ -56,7 +56,7 @@
a year in June. Non-Google link:dev-roles.html#maintainer[maintainers]
can nominate themselves by posting an informal application on the
non-public mailto:gerritcodereview-community-managers@googlegroups.com[
-community manager mailing list] when the call for nomiations is sent to
+community manager mailing list] when the call for nominations is sent to
the maintainers list by a community manager.
The list with all candidates will be published at the beginning of the
diff --git a/java/com/google/gerrit/index/query/AndSource.java b/java/com/google/gerrit/index/query/AndSource.java
index 3adf881..6de0712 100644
--- a/java/com/google/gerrit/index/query/AndSource.java
+++ b/java/com/google/gerrit/index/query/AndSource.java
@@ -17,11 +17,12 @@
import static com.google.common.base.Preconditions.checkArgument;
import com.google.gerrit.index.IndexConfig;
+import com.google.gerrit.index.PaginationType;
import java.util.Collection;
import java.util.List;
public class AndSource<T> extends AndPredicate<T> implements DataSource<T> {
- protected final DataSource<T> source;
+ protected final FilteredSource<T> filteredSource;
private final int start;
private final int cardinality;
@@ -58,18 +59,18 @@
if (selectedSource == null) {
throw new IllegalArgumentException("No DataSource Found");
}
- this.source = toPaginatingSource(selectedSource);
+ this.filteredSource = toDataSource(selectedSource);
this.cardinality = c;
}
@Override
public ResultSet<T> read() {
- return source.read();
+ return filteredSource.read();
}
@Override
public ResultSet<FieldBundle> readRaw() {
- return source.readRaw();
+ return filteredSource.readRaw();
}
@Override
@@ -91,17 +92,44 @@
}
@SuppressWarnings("unchecked")
- private PaginatingSource<T> toPaginatingSource(Predicate<T> pred) {
- return new PaginatingSource<>((DataSource<T>) pred, start, indexConfig) {
- @Override
- protected boolean match(T object) {
- return AndSource.this.match(object);
- }
+ private FilteredSource<T> toDataSource(Predicate<T> pred) {
+ if (indexConfig.paginationType().equals(PaginationType.NONE)) {
+ return new DatasourceWithoutPagination((DataSource<T>) pred, start, indexConfig);
+ }
+ return new DatasourceWithPagination((DataSource<T>) pred, start, indexConfig);
+ }
- @Override
- protected boolean isMatchable() {
- return AndSource.this.isMatchable();
- }
- };
+ private class DatasourceWithoutPagination extends FilteredSource<T> {
+
+ public DatasourceWithoutPagination(DataSource<T> source, int start, IndexConfig indexConfig) {
+ super(source, start, indexConfig);
+ }
+
+ @Override
+ protected boolean match(T object) {
+ return AndSource.this.match(object);
+ }
+
+ @Override
+ protected boolean isMatchable() {
+ return AndSource.this.isMatchable();
+ }
+ }
+
+ private class DatasourceWithPagination extends PaginatingSource<T> {
+
+ public DatasourceWithPagination(DataSource<T> source, int start, IndexConfig indexConfig) {
+ super(source, start, indexConfig);
+ }
+
+ @Override
+ protected boolean match(T object) {
+ return AndSource.this.match(object);
+ }
+
+ @Override
+ protected boolean isMatchable() {
+ return AndSource.this.isMatchable();
+ }
}
}
diff --git a/java/com/google/gerrit/index/query/FilteredSource.java b/java/com/google/gerrit/index/query/FilteredSource.java
new file mode 100644
index 0000000..fb31eb6
--- /dev/null
+++ b/java/com/google/gerrit/index/query/FilteredSource.java
@@ -0,0 +1,94 @@
+// Copyright (C) 2023 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.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.IndexConfig;
+import java.util.ArrayList;
+import java.util.List;
+
+public class FilteredSource<T> implements DataSource<T> {
+
+ protected final DataSource<T> source;
+ protected final int start;
+ protected final int cardinality;
+ protected final IndexConfig indexConfig;
+ private static final int PARTITION_SIZE = 50;
+
+ public FilteredSource(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 defined.");
+ }
+ // 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<>();
+ for (T data : buffer(resultSet)) {
+ if (!isMatchable() || match(data)) {
+ r.add(data);
+ }
+ }
+ 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() {
+ return source.readRaw();
+ }
+
+ protected Iterable<T> buffer(ResultSet<T> scanner) {
+ return FluentIterable.from(Iterables.partition(scanner, PARTITION_SIZE))
+ .transformAndConcat(this::transformBuffer);
+ }
+
+ protected List<T> transformBuffer(List<T> buffer) {
+ return buffer;
+ }
+
+ @Override
+ public int getCardinality() {
+ return cardinality;
+ }
+
+ protected boolean match(T object) {
+ return true;
+ }
+
+ protected boolean isMatchable() {
+ return true;
+ }
+}
diff --git a/java/com/google/gerrit/index/query/PaginatingSource.java b/java/com/google/gerrit/index/query/PaginatingSource.java
index 98a0ed3..be789ee 100644
--- a/java/com/google/gerrit/index/query/PaginatingSource.java
+++ b/java/com/google/gerrit/index/query/PaginatingSource.java
@@ -14,11 +14,7 @@
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;
@@ -27,18 +23,10 @@
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 class PaginatingSource<T> extends FilteredSource<T> {
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;
+ super(source, start, indexConfig);
}
@Override
@@ -63,13 +51,7 @@
pageResultSize++;
}
- if (last != null
- && source instanceof Paginated
- // TODO: this fix is only for the stable branches and the real refactoring would be to
- // restore the logic
- // for the filtering in AndSource (L58 - 64) as per
- // https://gerrit-review.googlesource.com/c/gerrit/+/345634/9
- && !indexConfig.paginationType().equals(PaginationType.NONE)) {
+ if (last != null && source instanceof Paginated) {
// Restart source and continue if we have not filled the
// full limit the caller wants.
//
@@ -117,34 +99,6 @@
throw new UnsupportedOperationException("not implemented");
}
- private Iterable<T> buffer(ResultSet<T> scanner) {
- return FluentIterable.from(Iterables.partition(scanner, 50))
- .transformAndConcat(this::transformBuffer);
- }
-
- /**
- * Checks whether the given object matches.
- *
- * @param object the object to be matched
- * @return whether the given object matches
- */
- 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 {
diff --git a/java/com/google/gerrit/index/query/QueryProcessor.java b/java/com/google/gerrit/index/query/QueryProcessor.java
index d847a06..f49cecb 100644
--- a/java/com/google/gerrit/index/query/QueryProcessor.java
+++ b/java/com/google/gerrit/index/query/QueryProcessor.java
@@ -33,6 +33,7 @@
import com.google.gerrit.index.IndexCollection;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.IndexRewriter;
+import com.google.gerrit.index.PaginationType;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.SchemaDefinitions;
import com.google.gerrit.metrics.Description;
@@ -296,7 +297,9 @@
@SuppressWarnings("unchecked")
DataSource<T> s = (DataSource<T>) pred;
- if (initialPageSize < limit && !(pred instanceof AndSource)) {
+ if (!indexConfig.paginationType().equals(PaginationType.NONE)
+ && initialPageSize < limit
+ && !(pred instanceof AndSource)) {
s = new PaginatingSource<>(s, start, indexConfig);
}
sources.add(s);
diff --git a/java/com/google/gerrit/server/query/change/AndChangeSource.java b/java/com/google/gerrit/server/query/change/AndChangeSource.java
index e4f768e..6c03425 100644
--- a/java/com/google/gerrit/server/query/change/AndChangeSource.java
+++ b/java/com/google/gerrit/server/query/change/AndChangeSource.java
@@ -33,7 +33,8 @@
@Override
public boolean hasChange() {
- return source instanceof ChangeDataSource && ((ChangeDataSource) source).hasChange();
+ return filteredSource instanceof ChangeDataSource
+ && ((ChangeDataSource) filteredSource).hasChange();
}
@Override
diff --git a/javatests/com/google/gerrit/index/query/AndSourceTest.java b/javatests/com/google/gerrit/index/query/AndSourceTest.java
index 068ae8c..6f20bd7 100644
--- a/javatests/com/google/gerrit/index/query/AndSourceTest.java
+++ b/javatests/com/google/gerrit/index/query/AndSourceTest.java
@@ -20,15 +20,24 @@
import static org.junit.Assert.assertTrue;
import com.google.common.collect.Lists;
+import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.testing.ConfigSuite;
+import org.eclipse.jgit.lib.Config;
import org.junit.Test;
+import org.junit.runner.RunWith;
+@RunWith(ConfigSuite.class)
public class AndSourceTest extends PredicateTest {
+
+ @ConfigSuite.Parameter public Config config;
+
@Test
public void ensureLowerCostPredicateRunsFirst() {
TestDataSourcePredicate p1 = new TestDataSourcePredicate("predicate1", "foo", 10, 10);
TestDataSourcePredicate p2 = new TestDataSourcePredicate("predicate2", "foo", 1, 10);
- AndSource<String> andSource = new AndSource<>(Lists.newArrayList(p1, p2), null);
+ AndSource<String> andSource =
+ new AndSource<>(Lists.newArrayList(p1, p2), IndexConfig.fromConfig(config).build());
assertFalse(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 d789201..1853d98 100644
--- a/javatests/com/google/gerrit/index/query/PredicateTest.java
+++ b/javatests/com/google/gerrit/index/query/PredicateTest.java
@@ -14,10 +14,32 @@
package com.google.gerrit.index.query;
+import com.google.gerrit.testing.ConfigSuite;
+import org.eclipse.jgit.lib.Config;
import org.junit.Ignore;
@Ignore
public abstract class PredicateTest {
+
+ @ConfigSuite.Default
+ public static Config defaultConfig() {
+ return com.google.gerrit.testing.IndexConfig.create();
+ }
+
+ @ConfigSuite.Config
+ public static Config searchAfterPaginationType() {
+ Config config = defaultConfig();
+ config.setString("index", null, "paginationType", "SEARCH_AFTER");
+ return config;
+ }
+
+ @ConfigSuite.Config
+ public static Config nonePaginationType() {
+ Config config = defaultConfig();
+ config.setString("index", null, "paginationType", "NONE");
+ return config;
+ }
+
@SuppressWarnings("ProtectedMembersInFinalClass")
protected static class TestDataSourcePredicate extends TestMatchablePredicate<String>
implements DataSource<String> {
diff --git a/polygerrit-ui/app/styles/gr-modal-styles.ts b/polygerrit-ui/app/styles/gr-modal-styles.ts
index b1bcf51..a8b5dc4 100644
--- a/polygerrit-ui/app/styles/gr-modal-styles.ts
+++ b/polygerrit-ui/app/styles/gr-modal-styles.ts
@@ -28,3 +28,13 @@
opacity: var(--modal-opacity, 0.6);
}
`;
+
+const $_documentContainer = document.createElement('template');
+$_documentContainer.innerHTML = `<dom-module id="gr-modal-styles">
+ <template>
+ <style>
+ ${modalStyles.cssText}
+ </style>
+ </template>
+</dom-module>`;
+document.head.appendChild($_documentContainer.content);
