Merge "access-control.txt: Fix ref on which Forge Committer must be assigned"
diff --git a/Documentation/dev-processes.txt b/Documentation/dev-processes.txt
index 409038c..8014977 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/Main.java b/java/Main.java
index e824a95..c04db2c 100644
--- a/java/Main.java
+++ b/java/Main.java
@@ -16,7 +16,7 @@
public final class Main {
private static final String FLOGGER_BACKEND_PROPERTY = "flogger.backend_factory";
private static final String FLOGGER_LOGGING_CONTEXT = "flogger.logging_context";
- private static final Runtime.Version MIN_JAVA_VERSION = Runtime.Version.parse("17.0.5");
+ private static final Runtime.Version MIN_JAVA_VERSION = Runtime.Version.parse("11.0.10");
// We don't do any real work here because we need to import
// the archive lookup code and we cannot import a class in
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/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 41e1910..b940ccc 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -279,7 +279,7 @@
} finally {
listLock.unlock();
}
- indexer.get().index(newProjectName);
+ evictAndReindex(newProjectName);
}
@Override
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/java/com/google/gerrit/server/restapi/change/PostReviewers.java b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
index e46f9e4..b0e58c5 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.restapi.change;
import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
+import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -71,7 +72,7 @@
ReviewerModification modification =
reviewerModifier.prepare(rsrc.getNotes(), rsrc.getUser(), input, true);
if (modification.op == null) {
- return Response.ok(modification.result);
+ return Response.withStatusCode(SC_BAD_REQUEST, modification.result);
}
try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
try (BatchUpdate bu =
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index 80c220a..979529e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -105,14 +105,14 @@
// Attempt to add overly large group as reviewers.
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
- ReviewerResult result = addReviewer(changeId, largeGroup);
+ ReviewerResult result = addReviewer(changeId, largeGroup, SC_BAD_REQUEST);
assertThat(result.input).isEqualTo(largeGroup);
assertThat(result.confirm).isNull();
assertThat(result.error).contains("has too many members to add them all as reviewers");
assertThat(result.reviewers).isNull();
// Attempt to add medium group without confirmation.
- result = addReviewer(changeId, mediumGroup);
+ result = addReviewer(changeId, mediumGroup, SC_BAD_REQUEST);
assertThat(result.input).isEqualTo(mediumGroup);
assertThat(result.confirm).isTrue();
assertThat(result.error)
@@ -162,6 +162,48 @@
}
@Test
+ public void addCcEmailWithoutAccount() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String testEmailAddress = "email@without.account";
+
+ // Add a reviewer
+ ReviewerInput ri = new ReviewerInput();
+ ri.reviewer = user.email();
+ ri.state = REVIEWER;
+ ReviewerResult result = addReviewer(changeId, ri);
+ assertThat(result.input).isEqualTo(user.email());
+ assertThat(result.confirm).isNull();
+ assertThat(result.error).isNull();
+ assertThat(result.reviewers).hasSize(1);
+
+ // Add an email address that has no account to CC
+ ReviewerInput ccInput = new ReviewerInput();
+ ccInput.reviewer = testEmailAddress;
+ ccInput.state = CC;
+ ReviewerResult resultCC = addReviewer(changeId, ccInput, SC_BAD_REQUEST);
+ assertThat(resultCC.error).contains("Account '" + testEmailAddress + "' not found");
+ assertThat(resultCC.error)
+ .contains(testEmailAddress + " does not identify a registered user or group");
+ }
+
+ @Test
+ public void addReviewerEmailWithoutAccount() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String testEmailAddress = "email@without.account";
+
+ // Add a reviewer without an account
+ ReviewerInput ri = new ReviewerInput();
+ ri.reviewer = testEmailAddress;
+ ri.state = REVIEWER;
+ ReviewerResult result = addReviewer(changeId, ri, SC_BAD_REQUEST);
+ assertThat(result.error).contains("Account '" + testEmailAddress + "' not found");
+ assertThat(result.error)
+ .contains(testEmailAddress + " does not identify a registered user or group");
+ }
+
+ @Test
public void addCcGroup() throws Exception {
List<TestAccount> users = createAccounts(6, "addCcGroup");
List<String> usernames = new ArrayList<>(6);
diff --git a/javatests/com/google/gerrit/acceptance/server/project/ProjectCacheIT.java b/javatests/com/google/gerrit/acceptance/server/project/ProjectCacheIT.java
index c81c945..0fb6b9e 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/ProjectCacheIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/ProjectCacheIT.java
@@ -143,4 +143,17 @@
inMemoryProjectCache.get(Project.nameKey("foo")); // Another invocation
assertThat(inMemoryProjectCache.stats().missCount()).isEqualTo(initialNumMisses + 1);
}
+
+ @Test
+ public void invalidatesNegativeCachingAfterProjectCreation() throws Exception {
+ long initialNumMisses = inMemoryProjectCache.stats().missCount();
+ assertThat(inMemoryProjectCache.get(Project.nameKey(name("foo")))).isEmpty();
+ assertThat(inMemoryProjectCache.stats().missCount())
+ .isEqualTo(initialNumMisses + 1); // Negative voting cached
+ Project.NameKey newProjectName =
+ createProjectOverAPI("foo", allProjects, true, /* submitType= */ null);
+ assertThat(inMemoryProjectCache.get(newProjectName)).isPresent(); // Another invocation
+ assertThat(inMemoryProjectCache.stats().missCount())
+ .isEqualTo(initialNumMisses + 3); // Two eviction happened during the project creation
+ }
}
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);
