Merge "Reject uploading project config changes if they contain duplicate SRs"
diff --git a/.bazelversion b/.bazelversion
index 0062ac9..c7cb131 100644
--- a/.bazelversion
+++ b/.bazelversion
@@ -1 +1 @@
-5.0.0
+5.3.1
diff --git a/java/com/google/gerrit/index/testing/TestIndexedFields.java b/java/com/google/gerrit/index/testing/TestIndexedFields.java
index f80b8a1..7a120b7 100644
--- a/java/com/google/gerrit/index/testing/TestIndexedFields.java
+++ b/java/com/google/gerrit/index/testing/TestIndexedFields.java
@@ -182,7 +182,10 @@
public static final IndexedField<TestIndexedData, Entities.Change> STORED_PROTO_FIELD =
IndexedField.<TestIndexedData, Entities.Change>builder(
- "TestChange", new TypeToken<Entities.Change>() {})
+ "TestChange",
+ new TypeToken<Entities.Change>() {
+ private static final long serialVersionUID = 1L;
+ })
.stored()
.build(getter(), setter(), ChangeProtoConverter.INSTANCE);
@@ -192,7 +195,10 @@
public static final IndexedField<TestIndexedData, Iterable<Entities.Change>>
ITERABLE_STORED_PROTO_FIELD =
IndexedField.<TestIndexedData, Iterable<Entities.Change>>builder(
- "IterableTestChange", new TypeToken<Iterable<Entities.Change>>() {})
+ "IterableTestChange",
+ new TypeToken<Iterable<Entities.Change>>() {
+ private static final long serialVersionUID = 1L;
+ })
.stored()
.build(getter(), setter(), ChangeProtoConverter.INSTANCE);
diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
index 7b47248..6471984 100644
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -542,7 +542,7 @@
}
}
ScoreDoc searchAfter = scoreDoc;
- return new ListResultSet<T>(b.build()) {
+ return new ListResultSet<>(b.build()) {
@Override
public Object searchAfter() {
return searchAfter;
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index e7b2336..1e34b4f 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -414,10 +414,12 @@
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) {
+ if (opts.searchAfter() != null
+ && opts.searchAfter() instanceof HashMap
+ && ((HashMap<?, ?>) opts.searchAfter()).get(subIndex) instanceof ScoreDoc) {
hits[i] =
searchers[i].searchAfter(
- ((HashMap<ChangeSubIndex, ScoreDoc>) opts.searchAfter()).get(subIndex),
+ (ScoreDoc) ((HashMap<?, ?>) opts.searchAfter()).get(subIndex),
query,
realPageSize,
sort,
diff --git a/java/com/google/gerrit/server/StarredChangesUtil.java b/java/com/google/gerrit/server/StarredChangesUtil.java
index 0f5629e..cfecd8e 100644
--- a/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -32,7 +32,6 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.git.GitUpdateFailureException;
@@ -194,7 +193,7 @@
}
}
- public void star(Account.Id accountId, Project.NameKey project, Change.Id changeId, Operation op)
+ public void star(Account.Id accountId, Change.Id changeId, Operation op)
throws IllegalLabelException {
try (Repository repo = repoManager.openRepository(allUsers)) {
String refName = RefNames.refsStarredChanges(changeId, accountId);
diff --git a/java/com/google/gerrit/server/account/externalids/AllExternalIds.java b/java/com/google/gerrit/server/account/externalids/AllExternalIds.java
index e718bcb..14aa368 100644
--- a/java/com/google/gerrit/server/account/externalids/AllExternalIds.java
+++ b/java/com/google/gerrit/server/account/externalids/AllExternalIds.java
@@ -45,6 +45,13 @@
return new AutoValue_AllExternalIds(byKey.build(), byAccount.build(), byEmail.build());
}
+ static AllExternalIds create(
+ ImmutableMap<ExternalId.Key, ExternalId> byKey,
+ ImmutableSetMultimap<Account.Id, ExternalId> byAccount,
+ ImmutableSetMultimap<String, ExternalId> byEmail) {
+ return new AutoValue_AllExternalIds(byKey, byAccount, byEmail);
+ }
+
public abstract ImmutableMap<ExternalId.Key, ExternalId> byKey();
public abstract ImmutableSetMultimap<Account.Id, ExternalId> byAccount();
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
index 27672bd..1edb284 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
@@ -253,7 +253,7 @@
}
}
}
- return new AutoValue_AllExternalIds(byKey.build(), byAccount.build(), byEmail.build());
+ return AllExternalIds.create(byKey.build(), byAccount.build(), byEmail.build());
}
private AllExternalIds reloadAllExternalIds(ObjectId notesRev)
diff --git a/java/com/google/gerrit/server/git/MultiProgressMonitor.java b/java/com/google/gerrit/server/git/MultiProgressMonitor.java
index 3498129..c76c78e 100644
--- a/java/com/google/gerrit/server/git/MultiProgressMonitor.java
+++ b/java/com/google/gerrit/server/git/MultiProgressMonitor.java
@@ -595,7 +595,8 @@
private void send(StringBuilder s) {
String progress = s.toString();
- logger.atInfo().atMostEvery(1, MINUTES).log(CharMatcher.javaIsoControl().removeFrom(progress));
+ logger.atInfo().atMostEvery(1, MINUTES).log(
+ "%s", CharMatcher.javaIsoControl().removeFrom(progress));
if (!clientDisconnected) {
try {
out.write(Constants.encode(progress));
diff --git a/java/com/google/gerrit/server/git/WorkQueue.java b/java/com/google/gerrit/server/git/WorkQueue.java
index 715cb30..f516a0c 100644
--- a/java/com/google/gerrit/server/git/WorkQueue.java
+++ b/java/com/google/gerrit/server/git/WorkQueue.java
@@ -51,8 +51,8 @@
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
-import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jgit.lib.Config;
/** Delayed execution of tasks using a background thread pool. */
@@ -523,18 +523,23 @@
* <ol>
* <li>{@link #SLEEPING}: if scheduled with a non-zero delay.
* <li>{@link #READY}: waiting for an available worker thread.
+ * <li>{@link #STARTING}: onStart() actively executing on a worker thread.
* <li>{@link #RUNNING}: actively executing on a worker thread.
+ * <li>{@link #STOPPING}: onStop() actively executing on a worker thread.
* <li>{@link #DONE}: finished executing, if not periodic.
* </ol>
*/
public enum State {
// Ordered like this so ordinal matches the order we would
// prefer to see tasks sorted in: done before running,
- // running before ready, ready before sleeping.
+ // stopping before running, running before starting,
+ // starting before ready, ready before sleeping.
//
DONE,
CANCELLED,
+ STOPPING,
RUNNING,
+ STARTING,
READY,
SLEEPING,
OTHER
@@ -544,15 +549,16 @@
private final RunnableScheduledFuture<V> task;
private final Executor executor;
private final int taskId;
- private final AtomicBoolean running;
private final Instant startTime;
+ // runningState is non-null when listener or task code is running in an executor thread
+ private final AtomicReference<State> runningState = new AtomicReference<>();
+
Task(Runnable runnable, RunnableScheduledFuture<V> task, Executor executor, int taskId) {
this.runnable = runnable;
this.task = task;
this.executor = executor;
this.taskId = taskId;
- this.running = new AtomicBoolean();
this.startTime = Instant.now();
}
@@ -563,10 +569,13 @@
public State getState() {
if (isCancelled()) {
return State.CANCELLED;
+ }
+
+ State r = runningState.get();
+ if (r != null) {
+ return r;
} else if (isDone() && !isPeriodic()) {
return State.DONE;
- } else if (running.get()) {
- return State.RUNNING;
}
final long delay = getDelay(TimeUnit.MILLISECONDS);
@@ -587,14 +596,14 @@
@Override
public boolean cancel(boolean mayInterruptIfRunning) {
if (task.cancel(mayInterruptIfRunning)) {
- // Tiny abuse of running: if the task needs to know it was
- // canceled (to clean up resources) and it hasn't started
+ // Tiny abuse of runningState: if the task needs to know it
+ // was canceled (to clean up resources) and it hasn't started
// yet the task's run method won't execute. So we tag it
// as running and allow it to clean up. This ensures we do
// not invoke cancel twice.
//
if (runnable instanceof CancelableRunnable) {
- if (running.compareAndSet(false, true)) {
+ if (runningState.compareAndSet(null, State.RUNNING)) {
((CancelableRunnable) runnable).cancel();
} else if (runnable instanceof CanceledWhileRunning) {
((CanceledWhileRunning) runnable).setCanceledWhileRunning();
@@ -654,18 +663,21 @@
@Override
public void run() {
- if (running.compareAndSet(false, true)) {
+ if (runningState.compareAndSet(null, State.STARTING)) {
String oldThreadName = Thread.currentThread().getName();
try {
executor.onStart(this);
+ runningState.set(State.RUNNING);
Thread.currentThread().setName(oldThreadName + "[" + task.toString() + "]");
task.run();
} finally {
Thread.currentThread().setName(oldThreadName);
+ runningState.set(State.STOPPING);
executor.onStop(this);
if (isPeriodic()) {
- running.set(false);
+ runningState.set(null);
} else {
+ runningState.set(State.DONE);
executor.remove(this);
}
}
diff --git a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
index 6cdc9ae..a39cbe5 100644
--- a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
+++ b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
@@ -112,6 +112,10 @@
private static ProjectSlice create(Project.NameKey name, int slice, int slices, ScanResult sr) {
return new AutoValue_AllChangesIndexer_ProjectSlice(name, slice, slices, sr);
}
+
+ private static ProjectSlice oneSlice(Project.NameKey name, ScanResult sr) {
+ return new AutoValue_AllChangesIndexer_ProjectSlice(name, 0, 1, sr);
+ }
}
@Override
@@ -178,47 +182,35 @@
public Callable<Void> reindexProject(
ChangeIndexer indexer, Project.NameKey project, Task done, Task failed) {
try (Repository repo = repoManager.openRepository(project)) {
- return reindexProject(
- indexer, project, 0, 1, ChangeNotes.Factory.scanChangeIds(repo), done, failed);
+ return reindexProjectSlice(
+ indexer,
+ ProjectSlice.oneSlice(project, ChangeNotes.Factory.scanChangeIds(repo)),
+ done,
+ failed);
} catch (IOException e) {
logger.atSevere().log("%s", e.getMessage());
return null;
}
}
- public Callable<Void> reindexProject(
- ChangeIndexer indexer,
- Project.NameKey project,
- int slice,
- int slices,
- ScanResult scanResult,
- Task done,
- Task failed) {
- return new ProjectIndexer(indexer, project, slice, slices, scanResult, done, failed);
+ public Callable<Void> reindexProjectSlice(
+ ChangeIndexer indexer, ProjectSlice projectSlice, Task done, Task failed) {
+ return new ProjectSliceIndexer(indexer, projectSlice, done, failed);
}
- private class ProjectIndexer implements Callable<Void> {
+ private class ProjectSliceIndexer implements Callable<Void> {
private final ChangeIndexer indexer;
- private final Project.NameKey project;
- private final int slice;
- private final int slices;
- private final ScanResult scanResult;
+ private final ProjectSlice projectSlice;
private final ProgressMonitor done;
private final ProgressMonitor failed;
- private ProjectIndexer(
+ private ProjectSliceIndexer(
ChangeIndexer indexer,
- Project.NameKey project,
- int slice,
- int slices,
- ScanResult scanResult,
+ ProjectSlice projectSlice,
ProgressMonitor done,
ProgressMonitor failed) {
this.indexer = indexer;
- this.project = project;
- this.slice = slice;
- this.slices = slices;
- this.scanResult = scanResult;
+ this.projectSlice = projectSlice;
this.done = done;
this.failed = failed;
}
@@ -232,7 +224,10 @@
// but the goal is to invalidate that cache as infrequently as we possibly can. And besides,
// we don't have concrete proof that improving packfile locality would help.
notesFactory
- .scan(scanResult, project, id -> (id.get() % slices) == slice)
+ .scan(
+ projectSlice.scanResult(),
+ projectSlice.name(),
+ id -> (id.get() % projectSlice.slices()) == projectSlice.slice())
.forEach(r -> index(r));
OnlineReindexMode.end();
return null;
@@ -271,7 +266,7 @@
@Override
public String toString() {
- return "Index all changes of project " + project.get();
+ return "Index project slice " + projectSlice;
}
}
@@ -351,12 +346,9 @@
ProjectSlice projectSlice = ProjectSlice.create(name, slice, slices, sr);
ListenableFuture<?> future =
executor.submit(
- reindexProject(
+ reindexProjectSlice(
indexerFactory.create(executor, index),
- name,
- slice,
- slices,
- projectSlice.scanResult(),
+ projectSlice,
doneTask,
failedTask));
String description = "project " + name + " (" + slice + "/" + slices + ")";
diff --git a/java/com/google/gerrit/server/restapi/account/StarredChanges.java b/java/com/google/gerrit/server/restapi/account/StarredChanges.java
index 12abf3d..173f24b 100644
--- a/java/com/google/gerrit/server/restapi/account/StarredChanges.java
+++ b/java/com/google/gerrit/server/restapi/account/StarredChanges.java
@@ -131,10 +131,7 @@
try {
starredChangesUtil.star(
- self.get().getAccountId(),
- change.getProject(),
- change.getId(),
- StarredChangesUtil.Operation.ADD);
+ self.get().getAccountId(), change.getId(), StarredChangesUtil.Operation.ADD);
} catch (MutuallyExclusiveLabelsException e) {
throw new ResourceConflictException(e.getMessage());
} catch (IllegalLabelException e) {
@@ -182,10 +179,7 @@
throw new AuthException("not allowed remove starred change");
}
starredChangesUtil.star(
- self.get().getAccountId(),
- rsrc.getChange().getProject(),
- rsrc.getChange().getId(),
- StarredChangesUtil.Operation.REMOVE);
+ self.get().getAccountId(), rsrc.getChange().getId(), StarredChangesUtil.Operation.REMOVE);
return Response.none();
}
}
diff --git a/java/com/google/gerrit/server/restapi/config/GetSummary.java b/java/com/google/gerrit/server/restapi/config/GetSummary.java
index d0a1498..23b7a80 100644
--- a/java/com/google/gerrit/server/restapi/config/GetSummary.java
+++ b/java/com/google/gerrit/server/restapi/config/GetSummary.java
@@ -90,14 +90,22 @@
private TaskSummaryInfo getTaskSummary() {
Collection<Task<?>> pending = workQueue.getTasks();
int tasksTotal = pending.size();
+ int tasksStopping = 0;
int tasksRunning = 0;
+ int tasksStarting = 0;
int tasksReady = 0;
int tasksSleeping = 0;
for (Task<?> task : pending) {
switch (task.getState()) {
+ case STOPPING:
+ tasksStopping++;
+ break;
case RUNNING:
tasksRunning++;
break;
+ case STARTING:
+ tasksStarting++;
+ break;
case READY:
tasksReady++;
break;
@@ -113,7 +121,9 @@
TaskSummaryInfo taskSummary = new TaskSummaryInfo();
taskSummary.total = toInteger(tasksTotal);
+ taskSummary.stopping = toInteger(tasksStopping);
taskSummary.running = toInteger(tasksRunning);
+ taskSummary.starting = toInteger(tasksStarting);
taskSummary.ready = toInteger(tasksReady);
taskSummary.sleeping = toInteger(tasksSleeping);
return taskSummary;
@@ -247,7 +257,9 @@
public static class TaskSummaryInfo {
public Integer total;
+ public Integer stopping;
public Integer running;
+ public Integer starting;
public Integer ready;
public Integer sleeping;
}
diff --git a/java/com/google/gerrit/sshd/commands/ShowQueue.java b/java/com/google/gerrit/sshd/commands/ShowQueue.java
index 4254e5b..00361ad 100644
--- a/java/com/google/gerrit/sshd/commands/ShowQueue.java
+++ b/java/com/google/gerrit/sshd/commands/ShowQueue.java
@@ -134,7 +134,9 @@
switch (task.state) {
case DONE:
case CANCELLED:
+ case STARTING:
case RUNNING:
+ case STOPPING:
case READY:
start = format(task.state);
break;
@@ -204,8 +206,12 @@
return "....... done";
case CANCELLED:
return "..... killed";
+ case STOPPING:
+ return "... stopping";
case RUNNING:
return "";
+ case STARTING:
+ return "starting ...";
case READY:
return "waiting ....";
case SLEEPING:
diff --git a/javatests/com/google/gerrit/acceptance/server/util/TaskListenerIT.java b/javatests/com/google/gerrit/acceptance/server/util/TaskListenerIT.java
index c2de4f7..23f2609 100644
--- a/javatests/com/google/gerrit/acceptance/server/util/TaskListenerIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/util/TaskListenerIT.java
@@ -236,6 +236,29 @@
assertAwaitQueueSize(--size);
}
+ @Test
+ public void states() throws Exception {
+ executor.execute(runnable);
+ listener.onStart.assertAwait();
+ assertStateIs(Task.State.STARTING);
+
+ listener.onStart.complete();
+ runnable.run.assertAwait();
+ assertStateIs(Task.State.RUNNING);
+
+ runnable.run.complete();
+ listener.onStop.assertAwait();
+ assertStateIs(Task.State.STOPPING);
+
+ listener.onStop.complete();
+ assertAwaitQueueIsEmpty();
+ assertStateIs(Task.State.DONE);
+ }
+
+ private void assertStateIs(Task.State state) {
+ assertThat(forwarder.task.getState()).isEqualTo(state);
+ }
+
private int assertQueueBlockedOnExecution(Runnable runnable) {
int expectedSize = workQueue.getTasks().size() + 1;
executor.execute(runnable);
@@ -247,6 +270,10 @@
assertThat(workQueue.getTasks().size()).isEqualTo(size);
}
+ private void assertAwaitQueueIsEmpty() throws InterruptedException {
+ assertAwaitQueueSize(0);
+ }
+
/** Fails if the waiting time elapses before the count is reached, otherwise passes */
private void assertAwaitQueueSize(int size) throws InterruptedException {
long i = 0;
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
index 01ae960..af42afb 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
@@ -55,7 +55,6 @@
@Test
@UseClockStep
- @SuppressWarnings("unchecked")
public void stopQueryIfNoMoreResults() throws Exception {
// create 2 visible changes
TestRepository<InMemoryRepositoryManager.Repo> testRepo = createProject("repo");
@@ -72,7 +71,8 @@
.add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
.update();
- AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
+ AbstractFakeIndex<?, ?, ?> idx =
+ (AbstractFakeIndex<?, ?, ?>) changeIndexCollection.getSearchIndex();
newQuery("status:new").withLimit(5).get();
// Since the limit of the query (i.e. 5) is more than the total number of changes (i.e. 4),
// only 1 index search is expected.
@@ -81,7 +81,6 @@
@Test
@UseClockStep
- @SuppressWarnings("unchecked")
public void noLimitQueryPaginates() throws Exception {
TestRepository<InMemoryRepositoryManager.Repo> testRepo = createProject("repo");
// create 4 changes
@@ -97,7 +96,8 @@
.add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, 2))
.update();
- AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
+ 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
diff --git a/plugins/gitiles b/plugins/gitiles
index 648b1df..24529d2 160000
--- a/plugins/gitiles
+++ b/plugins/gitiles
@@ -1 +1 @@
-Subproject commit 648b1df92b887ed5011e3a2fbf18fd2b9e8622b3
+Subproject commit 24529d232268ac51fd6850770f70dc0fcd732dd8
diff --git a/plugins/yarn.lock b/plugins/yarn.lock
index e012bd1..ec05cad 100644
--- a/plugins/yarn.lock
+++ b/plugins/yarn.lock
@@ -10,9 +10,9 @@
"@babel/highlight" "^7.18.6"
"@babel/helper-validator-identifier@^7.18.6":
- version "7.18.6"
- resolved "https://registry.yarnpkg.com/@babel/helper-validator-identifier/-/helper-validator-identifier-7.18.6.tgz#9c97e30d31b2b8c72a1d08984f2ca9b574d7a076"
- integrity sha512-MmetCkz9ej86nJQV+sFCxoGGrUbU3q02kgLciwkrt9QqEB7cP39oKEY0PakknEO0Gu20SskMRi+AYZ3b1TpN9g==
+ version "7.19.1"
+ resolved "https://registry.yarnpkg.com/@babel/helper-validator-identifier/-/helper-validator-identifier-7.19.1.tgz#7eea834cf32901ffdc1a7ee555e2f9c27e249ca2"
+ integrity sha512-awrNfaMtnHUr653GgGEs++LlAvW6w+DcPrOliSMXWCKo597CwL5Acf/wWdNkf/tfEQE3mjkeD1YOVZOUV/od1w==
"@babel/highlight@^7.18.6":
version "7.18.6"
@@ -35,16 +35,11 @@
resolved "https://registry.yarnpkg.com/@gerritcodereview/typescript-api/-/typescript-api-3.7.0.tgz#ae3886b5c4ddc6a02659a11d577e1df0b6158727"
integrity sha512-8zeZClN1gur+Isrn02bRXJ0wUjYvK99jQxg36ZbDelrGDglXKddf8QQkZmaH9sYIRcCFDLlh5+ZlRUTcXTuDVA==
-"@lit/reactive-element@^1.0.0", "@lit/reactive-element@^1.4.0":
+"@lit/reactive-element@^1.0.0", "@lit/reactive-element@^1.3.0", "@lit/reactive-element@^1.4.0":
version "1.4.1"
resolved "https://registry.yarnpkg.com/@lit/reactive-element/-/reactive-element-1.4.1.tgz#3f587eec5708692135bc9e94cf396130604979f3"
integrity sha512-qDv4851VFSaBWzpS02cXHclo40jsbAjRXnebNXpm0uVg32kCneZPo9RYVQtrTNICtZ+1wAYHu1ZtxWSWMbKrBw==
-"@lit/reactive-element@^1.3.0":
- version "1.3.2"
- resolved "https://registry.yarnpkg.com/@lit/reactive-element/-/reactive-element-1.3.2.tgz#43e470537b6ec2c23510c07812616d5aa27a17cd"
- integrity sha512-A2e18XzPMrIh35nhIdE4uoqRzoIpEU5vZYuQN4S3Ee1zkGdYC27DP12pewbw/RLgPHzaE4kx/YqxMzebOpm0dA==
-
"@nodelib/fs.scandir@2.1.5":
version "2.1.5"
resolved "https://registry.yarnpkg.com/@nodelib/fs.scandir/-/fs.scandir-2.1.5.tgz#7619c2eb21b25483f6d167548b4cfd5a7488c3d5"
@@ -131,9 +126,9 @@
"@polymer/polymer" "^3.0.5"
"@polymer/polymer@^3.0.5", "@polymer/polymer@^3.4.1":
- version "3.4.1"
- resolved "https://registry.yarnpkg.com/@polymer/polymer/-/polymer-3.4.1.tgz#333bef25711f8411bb5624fb3eba8212ef8bee96"
- integrity sha512-KPWnhDZibtqKrUz7enIPOiO4ZQoJNOuLwqrhV2MXzIt3VVnUVJVG5ORz4Z2sgO+UZ+/UZnPD0jqY+jmw/+a9mQ==
+ version "3.5.1"
+ resolved "https://registry.yarnpkg.com/@polymer/polymer/-/polymer-3.5.1.tgz#4b5234e43b8876441022bcb91313ab3c4a29f0c8"
+ integrity sha512-JlAHuy+1qIC6hL1ojEUfIVD58fzTpJAoCxFwV5yr0mYTXV1H8bz5zy0+rC963Cgr9iNXQ4T9ncSjC2fkF9BQfw==
dependencies:
"@webcomponents/shadycss" "^1.9.1"
@@ -289,9 +284,9 @@
integrity sha512-Y4XFY5VJAuw0FgAqPNd6NNoV44jbq9Bz2L7Rh/J6jLTiHBSBJa9fxqQIvkIld4GsoDOcCbvzOUAbLPsSKKg+uA==
"@types/node@*":
- version "18.7.18"
- resolved "https://registry.yarnpkg.com/@types/node/-/node-18.7.18.tgz#633184f55c322e4fb08612307c274ee6d5ed3154"
- integrity sha512-m+6nTEOadJZuTPkKR/SYK3A2d7FZrgElol9UP1Kae90VVU4a6mxnPuLiIW1m4Cq4gZ/nWb9GrdVXJCoCazDAbg==
+ version "18.8.3"
+ resolved "https://registry.yarnpkg.com/@types/node/-/node-18.8.3.tgz#ce750ab4017effa51aed6a7230651778d54e327c"
+ integrity sha512-0os9vz6BpGwxGe9LOhgP/ncvYN5Tx1fNcd2TM3rD/aCGBkysb+ZWpXEocG24h6ZzOi13+VB8HndAQFezsSOw1w==
"@types/parse5@^6.0.1":
version "6.0.3"
@@ -1070,44 +1065,28 @@
vary "^1.1.2"
lit-element@^3.2.0:
- version "3.2.0"
- resolved "https://registry.yarnpkg.com/lit-element/-/lit-element-3.2.0.tgz#9c981c55dfd9a8f124dc863edb62cc529d434db7"
- integrity sha512-HbE7yt2SnUtg5DCrWt028oaU4D5F4k/1cntAFHTkzY8ZIa8N0Wmu92PxSxucsQSOXlODFrICkQ5x/tEshKi13g==
+ version "3.2.2"
+ resolved "https://registry.yarnpkg.com/lit-element/-/lit-element-3.2.2.tgz#d148ab6bf4c53a33f707a5168e087725499e5f2b"
+ integrity sha512-6ZgxBR9KNroqKb6+htkyBwD90XGRiqKDHVrW/Eh0EZ+l+iC+u+v+w3/BA5NGi4nizAVHGYvQBHUDuSmLjPp7NQ==
dependencies:
"@lit/reactive-element" "^1.3.0"
lit-html "^2.2.0"
-lit-html@^2.0.0, lit-html@^2.3.0:
- version "2.3.1"
- resolved "https://registry.yarnpkg.com/lit-html/-/lit-html-2.3.1.tgz#56f15104ea75c0a702904893e3409d0e89e2a2b9"
- integrity sha512-FyKH6LTW6aBdkfNhNSHyZTnLgJSTe5hMk7HFtc/+DcN1w74C215q8B+Cfxc2OuIEpBNcEKxgF64qL8as30FDHA==
+lit-html@^2.0.0, lit-html@^2.2.0, lit-html@^2.4.0:
+ version "2.4.0"
+ resolved "https://registry.yarnpkg.com/lit-html/-/lit-html-2.4.0.tgz#b510430f39a56ec959167ed1187241a4e3ab1574"
+ integrity sha512-G6qXu4JNUpY6aaF2VMfaszhO9hlWw0hOTRFDmuMheg/nDYGB+2RztUSOyrzALAbr8Nh0Y7qjhYkReh3rPnplVg==
dependencies:
"@types/trusted-types" "^2.0.2"
-lit-html@^2.2.0:
- version "2.2.6"
- resolved "https://registry.yarnpkg.com/lit-html/-/lit-html-2.2.6.tgz#e70679605420a34c4f3cbd0c483b2fb1fff781df"
- integrity sha512-xOKsPmq/RAKJ6dUeOxhmOYFjcjf0Q7aSdfBJgdJkOfCUnkmmJPxNrlZpRBeVe1Gg50oYWMlgm6ccAE/SpJgSdw==
- dependencies:
- "@types/trusted-types" "^2.0.2"
-
-lit@^2.0.0:
- version "2.3.1"
- resolved "https://registry.yarnpkg.com/lit/-/lit-2.3.1.tgz#2cf1c2042da1e44c7a7cc72dff2d72303fd26f48"
- integrity sha512-TejktDR4mqG3qB32Y8Lm5Lye3c8SUehqz7qRsxe1PqGYL6me2Ef+jeQAEqh20BnnGncv4Yxy2njEIT0kzK1WCw==
+lit@^2.0.0, lit@^2.2.3:
+ version "2.4.0"
+ resolved "https://registry.yarnpkg.com/lit/-/lit-2.4.0.tgz#e33a0f463e17408f6e7f71464e6a266e60a5bb77"
+ integrity sha512-fdgzxEtLrZFQU/BqTtxFQCLwlZd9bdat+ltzSFjvWkZrs7eBmeX0L5MHUMb3kYIkuS8Xlfnii/iI5klirF8/Xg==
dependencies:
"@lit/reactive-element" "^1.4.0"
lit-element "^3.2.0"
- lit-html "^2.3.0"
-
-lit@^2.2.3:
- version "2.2.6"
- resolved "https://registry.yarnpkg.com/lit/-/lit-2.2.6.tgz#4ef223e88517c000b0c01baf2e3535e61a75a5b5"
- integrity sha512-K2vkeGABfSJSfkhqHy86ujchJs3NR9nW1bEEiV+bXDkbiQ60Tv5GUausYN2mXigZn8lC1qXuc46ArQRKYmumZw==
- dependencies:
- "@lit/reactive-element" "^1.3.0"
- lit-element "^3.2.0"
- lit-html "^2.2.0"
+ lit-html "^2.4.0"
log-update@^4.0.0:
version "4.0.0"
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
index 7fe070c..66ad38c 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
@@ -30,7 +30,6 @@
import {ReviewInput} from '../../../types/common';
import {getAppContext} from '../../../services/app-context';
import {assertIsDefined} from '../../../utils/common-util';
-import {CURRENT} from '../../../utils/patch-set-util';
import {fireReload} from '../../../utils/event-util';
import {submitRequirementsStyles} from '../../../styles/gr-submit-requirements-styles';
import {
@@ -321,7 +320,11 @@
},
};
return this.restApiService
- .saveChangeReview(this.change._number, CURRENT, review)
+ .saveChangeReview(
+ this.change._number,
+ this.change.current_revision,
+ review
+ )
.then(() => {
fireReload(this, true);
});
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts
index 2117ec5..4e78e8a 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts
@@ -18,7 +18,7 @@
createSubmitRequirementResultInfo,
} from '../../../test/test-data-generators';
import {ParsedChangeInfo} from '../../../types/types';
-import {query, queryAndAssert} from '../../../test/test-utils';
+import {query, queryAndAssert, stubRestApi} from '../../../test/test-utils';
import {GrButton} from '../../shared/gr-button/gr-button';
import {ChangeStatus, SubmitRequirementResultInfo} from '../../../api/rest-api';
@@ -321,6 +321,22 @@
assert.isUndefined(query(element, '.quickApprove'));
});
+ test('uses patchset from change', async () => {
+ const saveChangeReview = stubRestApi('saveChangeReview').resolves();
+ const element = await fixture<GrSubmitRequirementHovercard>(
+ html`<gr-submit-requirement-hovercard
+ .requirement=${submitRequirement}
+ .change=${change}
+ .account=${account}
+ ></gr-submit-requirement-hovercard>`
+ );
+
+ queryAndAssert<GrButton>(element, '.quickApprove > gr-button').click();
+
+ assert.equal(saveChangeReview.callCount, 1);
+ assert.equal(saveChangeReview.firstCall.args[1], change.current_revision);
+ });
+
test('override button renders', async () => {
const submitRequirement: SubmitRequirementResultInfo = {
...createSubmitRequirementResultInfo(),
diff --git a/polygerrit-ui/app/models/checks/checks-model.ts b/polygerrit-ui/app/models/checks/checks-model.ts
index 2b92529..db208d5 100644
--- a/polygerrit-ui/app/models/checks/checks-model.ts
+++ b/polygerrit-ui/app/models/checks/checks-model.ts
@@ -151,7 +151,12 @@
};
}
-const FETCH_RESULT_TIMEOUT_MS = 10000;
+/**
+ * Android's Checks Plugin has a 15s timeout internally. So we are using
+ * something slightly larger, so that we get a proper error from the plugin,
+ * if they run into timeout issues.
+ */
+const FETCH_RESULT_TIMEOUT_MS = 16000;
/**
* Can be used in `reduce()` to collect all results from all runs from all
@@ -185,9 +190,11 @@
private checkToPluginMap = new Map<string, string>();
- private changeNum?: NumericChangeId;
+ // visible for testing
+ changeNum?: NumericChangeId;
- private latestPatchNum?: PatchSetNumber;
+ // visible for testing
+ latestPatchNum?: PatchSetNumber;
private readonly documentVisibilityChange$ = new BehaviorSubject(undefined);
@@ -384,6 +391,9 @@
this.reporting.time(Timing.CHECKS_LOAD);
this.subscriptions = [
this.changeModel.changeNum$.subscribe(x => (this.changeNum = x)),
+ this.changeModel.latestPatchNum$.subscribe(
+ x => (this.latestPatchNum = x)
+ ),
this.pluginsModel.checksPlugins$.subscribe(plugins => {
for (const plugin of plugins) {
this.register(plugin);
@@ -750,8 +760,10 @@
patchset === ChecksPatchset.LATEST
? this.changeModel.latestPatchNum$
: this.checksSelectedPatchsetNumber$,
- this.reloadSubjects[pluginName].pipe(throttleTime(1000)),
- timer(0, pollIntervalMs),
+ this.reloadSubjects[pluginName].pipe(
+ throttleTime(1000, undefined, {trailing: true, leading: true})
+ ),
+ pollIntervalMs === 0 ? from([0]) : timer(0, pollIntervalMs),
this.documentVisibilityChange$,
])
.pipe(
diff --git a/polygerrit-ui/app/models/checks/checks-model_test.ts b/polygerrit-ui/app/models/checks/checks-model_test.ts
index 83ed464..3489c5a 100644
--- a/polygerrit-ui/app/models/checks/checks-model_test.ts
+++ b/polygerrit-ui/app/models/checks/checks-model_test.ts
@@ -7,6 +7,7 @@
import './checks-model';
import {ChecksModel, ChecksPatchset, ChecksProviderState} from './checks-model';
import {
+ Action,
Category,
CheckRun,
ChecksApiConfig,
@@ -22,6 +23,7 @@
import {assert} from '@open-wc/testing';
import {testResolver} from '../../test/common-test-setup';
import {changeViewModelToken} from '../views/change';
+import {NumericChangeId, PatchSetNumber} from '../../api/rest-api';
const PLUGIN_NAME = 'test-plugin';
@@ -42,8 +44,12 @@
},
];
-const CONFIG: ChecksApiConfig = {
- fetchPollingIntervalSeconds: 1000,
+const CONFIG_POLLING_5S: ChecksApiConfig = {
+ fetchPollingIntervalSeconds: 5,
+};
+
+const CONFIG_POLLING_NONE: ChecksApiConfig = {
+ fetchPollingIntervalSeconds: 0,
};
function createProvider(): ChecksProvider {
@@ -82,13 +88,67 @@
const provider = createProvider();
const fetchSpy = sinon.spy(provider, 'fetch');
- model.register({pluginName: 'test-plugin', provider, config: CONFIG});
+ model.register({
+ pluginName: 'test-plugin',
+ provider,
+ config: CONFIG_POLLING_NONE,
+ });
await waitUntil(() => change === undefined);
const testChange = createParsedChange();
model.changeModel.updateStateChange(testChange);
await waitUntil(() => change === testChange);
await waitUntilCalled(fetchSpy, 'fetch');
+
+ assert.equal(
+ model.latestPatchNum,
+ testChange.revisions[testChange.current_revision]
+ ._number as PatchSetNumber
+ );
+ assert.equal(model.changeNum, testChange._number);
+ });
+
+ test('reload throttle', async () => {
+ const clock = sinon.useFakeTimers();
+ let change: ParsedChangeInfo | undefined = undefined;
+ model.changeModel.change$.subscribe(c => (change = c));
+ const provider = createProvider();
+ const fetchSpy = sinon.spy(provider, 'fetch');
+
+ model.register({
+ pluginName: 'test-plugin',
+ provider,
+ config: CONFIG_POLLING_NONE,
+ });
+ await waitUntil(() => change === undefined);
+
+ const testChange = createParsedChange();
+ model.changeModel.updateStateChange(testChange);
+ await waitUntil(() => change === testChange);
+ clock.tick(1);
+ assert.equal(fetchSpy.callCount, 1);
+
+ // The second reload call will be processed, but only after a 1s throttle.
+ model.reload('test-plugin');
+ clock.tick(100);
+ assert.equal(fetchSpy.callCount, 1);
+ // 2000 ms is greater than the 1000 ms throttle time.
+ clock.tick(2000);
+ assert.equal(fetchSpy.callCount, 2);
+ });
+
+ test('triggerAction', async () => {
+ model.changeNum = 314 as NumericChangeId;
+ model.latestPatchNum = 13 as PatchSetNumber;
+ const action: Action = {
+ name: 'test action',
+ callback: () => undefined,
+ };
+ const spy = sinon.spy(action, 'callback');
+ model.triggerAction(action, undefined, 'none');
+ assert.isTrue(spy.calledOnce);
+ assert.equal(spy.lastCall.args[0], 314);
+ assert.equal(spy.lastCall.args[1], 13);
});
test('model.updateStateSetProvider', () => {
@@ -204,4 +264,58 @@
assert.lengthOf(current.runs[0].results!, 1);
assert.equal(current.runs[0].results![0].summary, 'new');
});
+
+ test('polls for changes', async () => {
+ const clock = sinon.useFakeTimers();
+ let change: ParsedChangeInfo | undefined = undefined;
+ model.changeModel.change$.subscribe(c => (change = c));
+ const provider = createProvider();
+ const fetchSpy = sinon.spy(provider, 'fetch');
+
+ model.register({
+ pluginName: 'test-plugin',
+ provider,
+ config: CONFIG_POLLING_5S,
+ });
+ await waitUntil(() => change === undefined);
+ clock.tick(1);
+ const testChange = createParsedChange();
+ model.changeModel.updateStateChange(testChange);
+ await waitUntil(() => change === testChange);
+ await waitUntilCalled(fetchSpy, 'fetch');
+ clock.tick(1);
+ const pollCount = fetchSpy.callCount;
+
+ // polling should continue while we wait
+ clock.tick(CONFIG_POLLING_5S.fetchPollingIntervalSeconds * 1000 * 2);
+
+ assert.isTrue(fetchSpy.callCount > pollCount);
+ });
+
+ test('does not poll when config specifies 0 seconds', async () => {
+ const clock = sinon.useFakeTimers();
+ let change: ParsedChangeInfo | undefined = undefined;
+ model.changeModel.change$.subscribe(c => (change = c));
+ const provider = createProvider();
+ const fetchSpy = sinon.spy(provider, 'fetch');
+
+ model.register({
+ pluginName: 'test-plugin',
+ provider,
+ config: CONFIG_POLLING_NONE,
+ });
+ await waitUntil(() => change === undefined);
+ clock.tick(1);
+ const testChange = createParsedChange();
+ model.changeModel.updateStateChange(testChange);
+ await waitUntil(() => change === testChange);
+ await waitUntilCalled(fetchSpy, 'fetch');
+ clock.tick(1);
+ const pollCount = fetchSpy.callCount;
+
+ // polling should not happen
+ clock.tick(60 * 1000);
+
+ assert.equal(fetchSpy.callCount, pollCount);
+ });
});
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index 826ec66..a2d3506 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -155,11 +155,18 @@
super(undefined);
this.state$.subscribe(s => {
if (s?.usp || s?.forceReload || s?.openReplyDialog) {
- this.updateState({
- usp: undefined,
- forceReload: undefined,
- openReplyDialog: undefined,
- });
+ // We had been running into issues with directly calling
+ // `this.updateState()` without `setTimeout()`, because we want to
+ // first allow all subscribers to process the first state update before
+ // creating another one. For some unknown reason some subscribers even
+ // got the state updates out of order.
+ setTimeout(() => {
+ this.updateState({
+ usp: undefined,
+ forceReload: undefined,
+ openReplyDialog: undefined,
+ });
+ }, 0);
}
});
}
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 117e7cc..e148da9 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -99,6 +99,8 @@
--purple-100: #e9d2fd;
--purple-50: #f3e8fd;
--purple-tonal: #523272;
+ --deep-purple-800: #4527a0;
+ --deep-purple-600: #5e35b1;
--pink-800: #b80672;
--pink-500: #f538a0;
--pink-50: #fde7f3;
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index d11e372..2330041 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -219,8 +219,8 @@
--dark-remove-highlight-color: #62110f;
--light-remove-highlight-color: #320404;
- --dark-rebased-add-highlight-color: rgba(11, 255, 155, 0.15);
- --light-rebased-add-highlight-color: #487165;
+ --dark-rebased-add-highlight-color: var(--deep-purple-800);
+ --light-rebased-add-highlight-color: var(--deep-purple-600);
--dark-rebased-remove-highlight-color: rgba(255, 139, 6, 0.15);
--light-rebased-remove-highlight-color: #2f3f2f;
diff --git a/tools/BUILD b/tools/BUILD
index 8d6d48f..c06cfdb 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -44,10 +44,11 @@
"-XepDisableWarningsInGeneratedCode",
# The XepDisableWarningsInGeneratedCode disables only warnings, but
# not errors. We should manually exclude all files generated by
- # AutoValue; such files always start $AutoValue_.....
+ # AutoValue; such files always start AutoValue_..., $AutoValue_...
+ # or $$AutoValue_...
# XepExcludedPaths is a regexp. If you need more paths - use | as
# separator.
- "-XepExcludedPaths:.*/\\\\$$AutoValue_.*\\.java",
+ "-XepExcludedPaths:.*/\\\\$$?\\\\$$?AutoValue_.*\\.java",
"-Xep:AlmostJavadoc:ERROR",
"-Xep:AlwaysThrows:ERROR",
"-Xep:AmbiguousMethodReference:ERROR",
@@ -66,7 +67,7 @@
"-Xep:AutoValueConstructorOrderChecker:ERROR",
"-Xep:AutoValueFinalMethods:ERROR",
"-Xep:AutoValueImmutableFields:ERROR",
- # "-Xep:AutoValueSubclassLeaked:WARN",
+ "-Xep:AutoValueSubclassLeaked:ERROR",
"-Xep:BadAnnotationImplementation:ERROR",
"-Xep:BadComparable:ERROR",
"-Xep:BadImport:ERROR",
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 37d8b9c..d6f2ef63 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -135,8 +135,8 @@
maven_jar(
name = "error-prone-annotations",
- artifact = "com.google.errorprone:error_prone_annotations:2.10.0",
- sha1 = "9bc20b94d3ac42489cf6ce1e42509c86f6f861a1",
+ artifact = "com.google.errorprone:error_prone_annotations:2.15.0",
+ sha1 = "38c8485a652f808c8c149150da4e5c2b0bd17f9a",
)
FLOGGER_VERS = "0.7.4"
