Merge "Adapt to changed REST interfaces in Gerrit core"
diff --git a/gr-checks/gr-checks-chip-view.js b/gr-checks/gr-checks-chip-view.js
index e96cdd3..cd07acb 100644
--- a/gr-checks/gr-checks-chip-view.js
+++ b/gr-checks/gr-checks-chip-view.js
@@ -84,7 +84,16 @@
type: Boolean,
value: false
},
- pollChecksInterval: Object
+ pollChecksInterval: Object,
+ visibilityChangeListenerAdded: {
+ type: Boolean,
+ value: false
+ }
+ },
+
+ detached() {
+ clearInterval(this.pollChecksInterval);
+ this.unlisten(document, 'visibilitychange', '_onVisibililityChange');
},
observers: [
@@ -124,6 +133,14 @@
});
},
+ _onVisibililityChange() {
+ if (document.hidden) {
+ clearInterval(this.pollChecksInterval);
+ return;
+ }
+ this._pollChecksRegularly(this.change, this.revision, this.getChecks);
+ },
+
_pollChecksRegularly(change, revision, getChecks) {
if (this.pollChecksInterval) {
clearInterval(this.pollChecksInterval);
@@ -131,6 +148,10 @@
const poll = () => this._fetchChecks(change, revision, getChecks);
poll();
this.pollChecksInterval = setInterval(poll, CHECKS_POLL_INTERVAL_MS)
+ if (!this.visibilityChangeListenerAdded) {
+ this.visibilityChangeListenerAdded = true;
+ this.listen(document, 'visibilitychange', '_onVisibililityChange');
+ }
},
/**
diff --git a/gr-checks/gr-checks-view.js b/gr-checks/gr-checks-view.js
index 43643f0..f5d083f 100644
--- a/gr-checks/gr-checks-view.js
+++ b/gr-checks/gr-checks-view.js
@@ -53,13 +53,21 @@
type: Object,
value: LoadingStatus.LOADING,
},
- pollChecksInterval: Object
+ pollChecksInterval: Object,
+ visibilityChangeListenerAdded: {
+ type: Boolean,
+ value: false
+ }
},
observers: [
'_pollChecksRegularly(change, revision, getChecks)',
],
+ detached() {
+ clearInterval(this.pollChecksInterval);
+ this.unlisten(document, 'visibilitychange', '_onVisibililityChange');
+ },
_orderChecks(a, b) {
if (a.state != b.state) {
@@ -95,6 +103,14 @@
});
},
+ _onVisibililityChange() {
+ if (document.hidden) {
+ clearInterval(this.pollChecksInterval);
+ return;
+ }
+ this._pollChecksRegularly(this.change, this.revision, this.getChecks);
+ },
+
_pollChecksRegularly(change, revision, getChecks) {
if (this.pollChecksInterval) {
clearInterval(this.pollChecksInterval);
@@ -102,6 +118,10 @@
const poll = () => this._fetchChecks(change, revision, getChecks);
poll();
this.pollChecksInterval = setInterval(poll, CHECKS_POLL_INTERVAL_MS)
+ if (!this.visibilityChangeListenerAdded) {
+ this.visibilityChangeListenerAdded = true;
+ this.listen(document, 'visibilitychange', '_onVisibililityChange');
+ }
},
_checkConfigured() {
diff --git a/java/com/google/gerrit/plugins/checks/CheckJson.java b/java/com/google/gerrit/plugins/checks/CheckJson.java
index e4199e9..074d4cc 100644
--- a/java/com/google/gerrit/plugins/checks/CheckJson.java
+++ b/java/com/google/gerrit/plugins/checks/CheckJson.java
@@ -89,6 +89,7 @@
info.checkerName = checker.getName().orElse(null);
info.checkerStatus = checker.getStatus();
info.blocking = checker.getBlockingConditions();
+ info.checkerDescription = checker.getDescription().orElse(null);
});
} catch (ConfigInvalidException e) {
logger.atWarning().withCause(e).log("skipping invalid checker %s", checkerUuid);
diff --git a/java/com/google/gerrit/plugins/checks/CheckUpdate.java b/java/com/google/gerrit/plugins/checks/CheckUpdate.java
index 315160d..0b86796 100644
--- a/java/com/google/gerrit/plugins/checks/CheckUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/CheckUpdate.java
@@ -16,6 +16,7 @@
import com.google.auto.value.AutoValue;
import com.google.gerrit.plugins.checks.api.CheckState;
+import com.google.gerrit.server.util.time.TimeUtil;
import java.sql.Timestamp;
import java.util.Optional;
@@ -49,6 +50,14 @@
public abstract Builder setFinished(Timestamp finished);
+ public Builder unsetStarted() {
+ return setStarted(TimeUtil.never());
+ }
+
+ public Builder unsetFinished() {
+ return setFinished(TimeUtil.never());
+ }
+
public abstract CheckUpdate build();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/Checks.java b/java/com/google/gerrit/plugins/checks/Checks.java
index c4527e5..fa15c0e 100644
--- a/java/com/google/gerrit/plugins/checks/Checks.java
+++ b/java/com/google/gerrit/plugins/checks/Checks.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.plugins.checks.api.CombinedCheckState;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import java.io.IOException;
@@ -90,6 +91,16 @@
boolean areAllRequiredCheckersPassing(Project.NameKey projectName, PatchSet.Id patchSetId)
throws IOException, StorageException;
+ /**
+ * Computes an ETag for the checks of the given change.
+ *
+ * @param projectName the name of the project that contains the change
+ * @param changeId ID of the change for which the ETag should be computed
+ * @return ETag for the checks of the given change
+ * @throws IOException if failed to access the checks data
+ */
+ String getETag(Project.NameKey projectName, Change.Id changeId) throws IOException;
+
@AutoValue
abstract class GetCheckOptions {
public static GetCheckOptions defaults() {
diff --git a/java/com/google/gerrit/plugins/checks/ChecksETagComputation.java b/java/com/google/gerrit/plugins/checks/ChecksETagComputation.java
new file mode 100644
index 0000000..7dc1501
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/ChecksETagComputation.java
@@ -0,0 +1,53 @@
+// Copyright (C) 2019 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.plugins.checks;
+
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.change.ChangeETagComputation;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+
+/**
+ * Ensures that the ETag of a change changes when there was any check update for the change.
+ *
+ * <p>This prevents callers using ETags from potentially seeing outdated submittability and combined
+ * check state information in {@link com.google.gerrit.extensions.common.ChangeInfo} when a check
+ * for the change was created or updated.
+ */
+@Singleton
+public class ChecksETagComputation implements ChangeETagComputation {
+ private final Checks checks;
+
+ @Inject
+ ChecksETagComputation(Checks checks) {
+ this.checks = checks;
+ }
+
+ @Override
+ public String getETag(Project.NameKey projectName, Change.Id changeId) {
+ try {
+ return checks.getETag(projectName, changeId);
+ } catch (IOException e) {
+ // The change ETag will be invalidated if the checks can be accessed the next time.
+ throw new StorageException(
+ String.format(
+ "Failed to compute ETag for checks of change %s in project %s",
+ changeId, projectName));
+ }
+ }
+}
diff --git a/java/com/google/gerrit/plugins/checks/Module.java b/java/com/google/gerrit/plugins/checks/Module.java
index 8440501..140497b 100644
--- a/java/com/google/gerrit/plugins/checks/Module.java
+++ b/java/com/google/gerrit/plugins/checks/Module.java
@@ -28,6 +28,7 @@
import com.google.gerrit.plugins.checks.rules.ChecksSubmitRule;
import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.change.ChangeAttributeFactory;
+import com.google.gerrit.server.change.ChangeETagComputation;
import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.MergeValidationListener;
import com.google.gerrit.server.git.validators.RefOperationValidationListener;
@@ -56,6 +57,10 @@
.to(CheckerRefOperationValidator.class)
.in(SINGLETON);
+ DynamicSet.bind(binder(), ChangeETagComputation.class)
+ .to(ChecksETagComputation.class)
+ .in(SINGLETON);
+
DynamicSet.bind(binder(), ChangeAttributeFactory.class).to(ChangeCheckAttributeFactory.class);
bind(DynamicOptions.DynamicBean.class)
.annotatedWith(Exports.named(GetChange.class))
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java b/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java
index e476537..51fcc41 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java
@@ -38,7 +38,7 @@
sysModule = "com.google.gerrit.plugins.checks.acceptance.TestModule",
httpModule = "com.google.gerrit.plugins.checks.HttpModule")
public class AbstractCheckersTest extends LightweightPluginDaemonTest {
- @Inject protected ProjectOperations projectOperations;
+ @Inject private ProjectOperations projectOperations;
protected CheckerOperations checkerOperations;
protected CheckOperations checkOperations;
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/BUILD b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/BUILD
index 62a5299..577bfd4 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/BUILD
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/BUILD
@@ -8,6 +8,7 @@
srcs = glob(["*.java"]),
deps = [
"//java/com/google/gerrit/acceptance:lib",
+ "//java/com/google/gerrit/server/util/time",
"//plugins/checks:checks__plugin",
],
)
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
index 7600dcb..984cd31 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
@@ -277,8 +277,9 @@
}
if (testCheckerInvalidation.nonParseableConfig()) {
- try (Repository repo = repoManager.openRepository(allProjectsName)) {
- new TestRepository<>(repo)
+ try (Repository repo = repoManager.openRepository(allProjectsName);
+ TestRepository<Repository> testRepo = new TestRepository<>(repo)) {
+ testRepo
.branch(checkerUuid.toRefName())
.commit()
.add(CheckerConfig.CHECKER_CONFIG_FILE, "non-parseable-config")
@@ -287,9 +288,9 @@
}
if (testCheckerInvalidation.deleteRef()) {
- try (Repository repo = repoManager.openRepository(allProjectsName)) {
- RefUpdate ru =
- new TestRepository<>(repo).getRepository().updateRef(checkerUuid.toRefName(), true);
+ try (Repository repo = repoManager.openRepository(allProjectsName);
+ TestRepository<Repository> testRepo = new TestRepository<>(repo)) {
+ RefUpdate ru = testRepo.getRepository().updateRef(checkerUuid.toRefName(), true);
ru.setForceUpdate(true);
ru.delete();
}
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java
index 90864a6..16b4316 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java
@@ -18,6 +18,7 @@
import com.google.gerrit.acceptance.testsuite.ThrowingConsumer;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.api.CheckState;
+import com.google.gerrit.server.util.time.TimeUtil;
import java.sql.Timestamp;
import java.util.Optional;
@@ -65,6 +66,14 @@
public abstract Builder finished(Timestamp finished);
+ public Builder clearStarted() {
+ return started(TimeUtil.never());
+ }
+
+ public Builder clearFinished() {
+ return finished(TimeUtil.never());
+ }
+
abstract Builder checkUpdater(ThrowingConsumer<TestCheckUpdate> checkUpdate);
abstract TestCheckUpdate autoBuild();
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckInfo.java b/java/com/google/gerrit/plugins/checks/api/CheckInfo.java
index 77cbdef..d0d5c38 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckInfo.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckInfo.java
@@ -56,6 +56,9 @@
/** Blocking conditions that apply to this check. */
public Set<BlockingCondition> blocking;
+ /** Description of the checker that produced this check */
+ public String checkerDescription;
+
@Override
public boolean equals(Object o) {
if (!(o instanceof CheckInfo)) {
@@ -75,7 +78,8 @@
&& Objects.equals(other.updated, updated)
&& Objects.equals(other.checkerName, checkerName)
&& Objects.equals(other.checkerStatus, checkerStatus)
- && Objects.equals(other.blocking, blocking);
+ && Objects.equals(other.blocking, blocking)
+ && Objects.equals(other.checkerDescription, checkerDescription);
}
@Override
@@ -94,7 +98,8 @@
updated,
checkerName,
checkerStatus,
- blocking);
+ blocking,
+ checkerDescription);
}
@Override
@@ -114,6 +119,7 @@
.add("checkerName", checkerName)
.add("checkerStatus", checkerStatus)
.add("blocking", blocking)
+ .add("checkerDescription", checkerDescription)
.toString();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java b/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
index 61bd436..3adb2dd 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
@@ -28,7 +28,6 @@
import com.google.gerrit.plugins.checks.Checkers;
import com.google.gerrit.plugins.checks.CheckersUpdate;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.git.meta.VersionedMetaData;
import com.google.gerrit.server.util.time.TimeUtil;
@@ -152,7 +151,8 @@
* @throws IOException if the repository can't be accessed for some reason
* @throws ConfigInvalidException if the checker exists but can't be read due to an invalid format
*/
- public static CheckerConfig loadForChecker(NameKey projectName, Repository repository, Ref ref)
+ public static CheckerConfig loadForChecker(
+ Project.NameKey projectName, Repository repository, Ref ref)
throws IOException, ConfigInvalidException {
CheckerConfig checkerConfig = new CheckerConfig(ref.getName());
checkerConfig.load(projectName, repository);
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java b/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
index cb60d2c..043028a 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
@@ -9,6 +9,7 @@
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.util.time.TimeUtil;
import java.sql.Timestamp;
/** Representation of {@link Check} that can be serialized with GSON. */
@@ -74,11 +75,19 @@
modified = true;
}
if (update.started().isPresent() && !update.started().get().equals(started)) {
- started = update.started().get();
+ if (update.started().get().equals(TimeUtil.never())) {
+ started = null;
+ } else {
+ started = update.started().get();
+ }
modified = true;
}
if (update.finished().isPresent() && !update.finished().get().equals(finished)) {
- finished = update.finished().get();
+ if (update.finished().get().equals(TimeUtil.never())) {
+ finished = null;
+ } else {
+ finished = update.finished().get();
+ }
modified = true;
}
return modified;
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
index 02847e8..1e245fd 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
@@ -26,6 +26,7 @@
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.Checker;
import com.google.gerrit.plugins.checks.CheckerQuery;
+import com.google.gerrit.plugins.checks.CheckerRef;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.Checkers;
import com.google.gerrit.plugins.checks.Checks;
@@ -33,10 +34,10 @@
import com.google.gerrit.plugins.checks.api.CheckerStatus;
import com.google.gerrit.plugins.checks.api.CombinedCheckState;
import com.google.gerrit.plugins.checks.api.CombinedCheckState.CheckStateCount;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSet.Id;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -46,6 +47,10 @@
import java.util.Map;
import java.util.Optional;
import java.util.stream.Stream;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
/** Class to read checks from NoteDb. */
@Singleton
@@ -55,6 +60,7 @@
private final Checkers checkers;
private final CheckBackfiller checkBackfiller;
private final Provider<CheckerQuery> checkerQueryProvider;
+ private final GitRepositoryManager repoManager;
@Inject
NoteDbChecks(
@@ -62,12 +68,14 @@
CheckNotes.Factory checkNotesFactory,
Checkers checkers,
CheckBackfiller checkBackfiller,
- Provider<CheckerQuery> checkerQueryProvider) {
+ Provider<CheckerQuery> checkerQueryProvider,
+ GitRepositoryManager repoManager) {
this.changeDataFactory = changeDataFactory;
this.checkNotesFactory = checkNotesFactory;
this.checkers = checkers;
this.checkBackfiller = checkBackfiller;
this.checkerQueryProvider = checkerQueryProvider;
+ this.repoManager = repoManager;
}
@Override
@@ -131,15 +139,15 @@
}
@Override
- public CombinedCheckState getCombinedCheckState(NameKey projectName, Id patchSetId)
- throws IOException, StorageException {
+ public CombinedCheckState getCombinedCheckState(
+ Project.NameKey projectName, PatchSet.Id patchSetId) throws IOException, StorageException {
ImmutableListMultimap<CheckState, Boolean> statesAndRequired =
getStatesAndRequiredMap(projectName, patchSetId);
return CombinedCheckState.combine(statesAndRequired);
}
@Override
- public boolean areAllRequiredCheckersPassing(NameKey projectName, Id patchSetId)
+ public boolean areAllRequiredCheckersPassing(Project.NameKey projectName, PatchSet.Id patchSetId)
throws IOException, StorageException {
ImmutableListMultimap<CheckState, Boolean> statesAndRequired =
getStatesAndRequiredMap(projectName, patchSetId);
@@ -148,8 +156,17 @@
&& checkStateCount.inProgressRequiredCount() == 0;
}
+ @Override
+ public String getETag(Project.NameKey projectName, Change.Id changeId) throws IOException {
+ try (Repository repo = repoManager.openRepository(projectName);
+ RevWalk rw = new RevWalk(repo)) {
+ Ref checkRef = repo.getRefDatabase().exactRef(CheckerRef.checksRef(changeId));
+ return checkRef != null ? checkRef.getObjectId().getName() : ObjectId.zeroId().getName();
+ }
+ }
+
private ImmutableListMultimap<CheckState, Boolean> getStatesAndRequiredMap(
- NameKey projectName, Id patchSetId) throws IOException, StorageException {
+ Project.NameKey projectName, PatchSet.Id patchSetId) throws IOException, StorageException {
ChangeData changeData = changeDataFactory.create(projectName, patchSetId.changeId());
ImmutableMap<String, Checker> allCheckersOfProject =
checkers.checkersOf(projectName).stream()
diff --git a/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java b/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
index e74f050..8e5bb2c 100644
--- a/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
+++ b/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
@@ -32,6 +32,7 @@
import com.google.gerrit.plugins.checks.db.CheckerConfig;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.truth.OptionalSubject;
+import java.sql.Timestamp;
import java.util.Optional;
import org.eclipse.jgit.lib.Config;
@@ -87,7 +88,7 @@
check("query()").about(optionals()).that(checker().getQuery()).isEmpty();
}
- public ComparableSubject hasCreatedThat() {
+ public ComparableSubject<Timestamp> hasCreatedThat() {
return check("created()").that(checker().getCreated());
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
index e87dd42..71d468d 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
@@ -24,6 +24,7 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.SkipProjectClone;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.common.ChangeInput;
@@ -51,6 +52,7 @@
@SkipProjectClone
public class CheckerRefsIT extends AbstractCheckersTest {
+ @Inject private ProjectOperations projectOperations;
@Inject private Sequences seq;
@Inject private ChangeInserter.Factory changeInserterFactory;
@Inject private BatchUpdate.Factory updateFactory;
@@ -354,10 +356,11 @@
try (Repository git = repoManager.openRepository(project);
ObjectInserter oi = git.newObjectInserter();
ObjectReader reader = oi.newReader();
- RevWalk rw = new RevWalk(reader)) {
+ RevWalk rw = new RevWalk(reader);
+ TestRepository<Repository> testRepo = new TestRepository<>(git)) {
RevCommit head = rw.parseCommit(git.exactRef(targetRef).getObjectId());
RevCommit commit =
- new TestRepository<>(git)
+ testRepo
.commit()
.author(admin.newIdent())
.message("A change.")
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
index a8a8d19..d16df11 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
@@ -372,6 +372,36 @@
assertThat(response.getEntityContent()).isEqualTo("checks are not supported on a change edit");
}
+ @Test
+ public void creationOfCheckChangesETagOfChange() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ String oldETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.state = CheckState.RUNNING;
+ checksApiFactory.revision(patchSetId).create(input).get();
+
+ String newETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+ assertThat(newETag).isNotEqualTo(oldETag);
+ }
+
+ @Test
+ public void creationOfCheckChangesETagOfRevisionActions() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ String oldETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.state = CheckState.RUNNING;
+ checksApiFactory.revision(patchSetId).create(input).get();
+
+ String newETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+ assertThat(newETag).isNotEqualTo(oldETag);
+ }
+
// TODO(gerrit-team) More tests, especially for multiple checkers and PS and how commits behave
private Check getCheck(Project.NameKey project, PatchSet.Id patchSetId, CheckerUuid checkerUuid)
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
index 9e2a066..ecd2596 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
@@ -89,7 +89,12 @@
@Test
public void getCheckWithOptions() throws Exception {
CheckerUuid checkerUuid =
- checkerOperations.newChecker().repository(project).name("My Checker").create();
+ checkerOperations
+ .newChecker()
+ .repository(project)
+ .name("My Checker")
+ .description("Description")
+ .create();
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).state(CheckState.RUNNING).upsert();
@@ -99,7 +104,7 @@
expectedCheckInfo.checkerName = "My Checker";
expectedCheckInfo.checkerStatus = CheckerStatus.ENABLED;
expectedCheckInfo.blocking = ImmutableSortedSet.of();
-
+ expectedCheckInfo.checkerDescription = "Description";
assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER))
.isEqualTo(expectedCheckInfo);
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
index 5bfbee0..2727203 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
@@ -23,6 +23,7 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -50,6 +51,7 @@
import org.junit.Test;
public class QueryPendingChecksIT extends AbstractCheckersTest {
+ @Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
private PatchSet.Id patchSetId;
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
index 6d15caa..c6aa0c1 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
@@ -156,6 +156,16 @@
}
@Test
+ public void unsetStarted() throws Exception {
+ checkOperations.check(checkKey).forUpdate().started(TimeUtil.nowTs()).upsert();
+
+ CheckInput input = new CheckInput();
+ input.started = TimeUtil.never();
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ assertThat(info.started).isNull();
+ }
+
+ @Test
public void updateFinished() throws Exception {
CheckInput input = new CheckInput();
input.finished = TimeUtil.nowTs();
@@ -165,6 +175,16 @@
}
@Test
+ public void unsetFinished() throws Exception {
+ checkOperations.check(checkKey).forUpdate().finished(TimeUtil.nowTs()).upsert();
+
+ CheckInput input = new CheckInput();
+ input.finished = TimeUtil.never();
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ assertThat(info.finished).isNull();
+ }
+
+ @Test
public void updateWithEmptyInput() throws Exception {
assertThat(
checksApiFactory
@@ -360,4 +380,56 @@
assertThat(info.message).isEqualTo("some message");
assertThat(info.url).isEqualTo("https://www.example.com");
}
+
+ @Test
+ public void updateOfCheckChangesETagOfChange() throws Exception {
+ String oldETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+
+ CheckInput input = new CheckInput();
+ input.state = CheckState.FAILED;
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String newETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+ assertThat(newETag).isNotEqualTo(oldETag);
+ }
+
+ @Test
+ public void noOpUpdateOfCheckDoesNotChangeETagOfChange() throws Exception {
+ CheckInput input = new CheckInput();
+ input.state = CheckState.FAILED;
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String oldETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String newETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+ assertThat(newETag).isEqualTo(oldETag);
+ }
+
+ @Test
+ public void updateOfCheckChangesETagOfRevisionActions() throws Exception {
+ String oldETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+
+ CheckInput input = new CheckInput();
+ input.state = CheckState.FAILED;
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String newETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+ assertThat(newETag).isNotEqualTo(oldETag);
+ }
+
+ @Test
+ public void noOpUpdateOfCheckDoesNotChangeETagOfRevisionActions() throws Exception {
+ CheckInput input = new CheckInput();
+ input.state = CheckState.FAILED;
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String oldETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String newETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+ assertThat(newETag).isEqualTo(oldETag);
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
index 4e63bde..6458099 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
@@ -476,6 +476,18 @@
}
@Test
+ public void startedCanBeCleared() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
+ checkOperations.newCheck(checkKey).started(TimeUtil.nowTs()).upsert();
+
+ checkOperations.check(checkKey).forUpdate().clearStarted().upsert();
+
+ Optional<Timestamp> currentStarted = checkOperations.check(checkKey).get().started();
+ assertThat(currentStarted).isEmpty();
+ }
+
+ @Test
public void finishedCanBeUpdated() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
@@ -489,6 +501,18 @@
}
@Test
+ public void finishedCanBeCleared() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
+ checkOperations.newCheck(checkKey).finished(TimeUtil.nowTs()).upsert();
+
+ checkOperations.check(checkKey).forUpdate().clearFinished().upsert();
+
+ Optional<Timestamp> currentFinished = checkOperations.check(checkKey).get().finished();
+ assertThat(currentFinished).isEmpty();
+ }
+
+ @Test
public void getNotesAsText() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
PatchSet.Id patchSetId = createChange().getPatchSetId();
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
index 403a3a7..f78fee2 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
@@ -682,8 +682,9 @@
CheckerUuid checkerUuid1 = CheckerUuid.parse("test:my-checker1");
CheckerUuid checkerUuid2 = CheckerUuid.parse("test:my-checker2");
- try (Repository repo = repoManager.openRepository(allProjects)) {
- new TestRepository<>(repo)
+ try (Repository repo = repoManager.openRepository(allProjects);
+ TestRepository<Repository> testRepo = new TestRepository<>(repo)) {
+ testRepo
.branch(CheckerRef.REFS_META_CHECKERS)
.commit()
.add(
@@ -710,8 +711,9 @@
CheckerUuid checkerUuid1 = CheckerUuid.parse("test:my-checker1");
CheckerUuid checkerUuid2 = CheckerUuid.parse("test:my-checker2");
- try (Repository repo = repoManager.openRepository(allProjects)) {
- new TestRepository<>(repo)
+ try (Repository repo = repoManager.openRepository(allProjects);
+ TestRepository<Repository> testRepo = new TestRepository<>(repo)) {
+ testRepo
.branch(CheckerRef.REFS_META_CHECKERS)
.commit()
.add(
diff --git a/resources/Documentation/rest-api-checkers.md b/resources/Documentation/rest-api-checkers.md
index 40e1cf3..f05816f 100644
--- a/resources/Documentation/rest-api-checkers.md
+++ b/resources/Documentation/rest-api-checkers.md
@@ -49,7 +49,7 @@
Creates a new checker.
In the request body the data for the checker must be provided as a
-[CheckerInput](#checker-input) entity.
+[CheckerCreateInput](#checker-create-input) entity.
Note that only users with the [Administrate
Checkers](access-control.md#capability_administrateCheckers) global capability
@@ -94,12 +94,12 @@
Updates a checker.
The new property values must be set in the request body in a
-[CheckerInput](#checker-input) entity.
+[CheckerUpdateInput](#checker-update-input) entity.
This REST endpoint supports partial updates of the checker property set. Only
-properties that are set in the [CheckerInput](#checker-input) entity are
-updated. Properties that are not set in the input (or that have `null` as value)
-are not touched.
+properties that are set in the [CheckerUpdateInput](#checker-update-input)
+entity are updated. Properties that are not set in the input (or that have
+`null` as value) are not touched.
Unsetting properties:
@@ -174,6 +174,20 @@
## <a id="json-entities"> JSON Entities
+### <a id="checker-create-input"> CheckerCreateInput
+The `CheckerCreateInput` entity contains information for creating a checker.
+
+| Field Name | | Description |
+| --------------- | -------- | ----------- |
+| `uuid` | | The [UUID](#checker-id) of the checker.
+| `name` | | The name of the checker.
+| `description` | optional | The description of the checker.
+| `url` | optional | The URL of the checker.
+| `repository` | | The (exact) name of the repository for which the checker applies.
+| `status` | optional | The status of the checker; one of `ENABLED` or `DISABLED`.
+| `blocking` | optional | A list of [conditions](#blocking-conditions) that describe when the checker should block change submission.
+| `query` | optional | A [query](#query) that limits changes for which the checker is relevant.
+
### <a id="checker-info"> CheckerInfo
The `CheckerInfo` entity describes a checker.
@@ -190,12 +204,12 @@
| `created` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the checker was created.
| `updated` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the checker was last updated.
-### <a id="checker-input"> CheckerInput
-The `CheckerInput` entity contains information for creating a checker.
+### <a id="checker-update-input"> CheckerUpdateInput
+The `CheckerUpdateInput` entity contains information for updating a checker.
| Field Name | | Description |
| --------------- | -------- | ----------- |
-| `name` | optional | The name of the checker. Must be specified for checker creation.
+| `name` | optional | The name of the checker.
| `description` | optional | The description of the checker.
| `url` | optional | The URL of the checker.
| `repository` | optional | The (exact) name of the repository for which the checker applies.
diff --git a/resources/Documentation/rest-api-checks.md b/resources/Documentation/rest-api-checks.md
index 4e30e94..4bcd1a1 100644
--- a/resources/Documentation/rest-api-checks.md
+++ b/resources/Documentation/rest-api-checks.md
@@ -161,29 +161,30 @@
### <a id="check-info"> CheckInfo
The `CheckInfo` entity describes a check.
-| Field Name | | Description |
-| ----------------- | -------- | ----------- |
-| `repository` | | The repository name that this check applies to.
-| `change_number` | | The change number that this check applies to.
-| `patch_set_id` | | The patch set that this check applies to.
-| `checker_uuid` | | The [UUID](./rest-api-checkers.md#checker-id) of the checker that reported this check.
-| `state` | | The state as string-serialized form of [CheckState](#check-state)
-| `message` | optional | Short message explaining the check state.
-| `url` | optional | A fully-qualified URL pointing to the result of the check on the checker's infrastructure.
-| `started` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check started processing.
-| `finished` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check finished processing.
-| `created` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check was created.
-| `updated` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check was last updated.
-| `checker_name` | optional | The name of the checker that produced this check.<br />Only set if [checker details](#option-checker) are requested.
-| `checker_status` | optional | The [status](rest-api-checkers.md#checker-info) of the checker that produced this check.<br />Only set if [checker details](#option-checker) are requested.
-| `blocking` | optional | Set of [blocking conditions](rest-api-checkers.md#blocking-conditions) that apply to this checker.<br />Only set if [checker details](#option-checker) are requested.
+| Field Name | | Description |
+| --------------------- | -------- | ----------- |
+| `repository` | | The repository name that this check applies to.
+| `change_number` | | The change number that this check applies to.
+| `patch_set_id` | | The patch set that this check applies to.
+| `checker_uuid` | | The [UUID](./rest-api-checkers.md#checker-id) of the checker that reported this check.
+| `state` | | The state as string-serialized form of [CheckState](#check-state)
+| `message` | optional | Short message explaining the check state.
+| `url` | optional | A fully-qualified URL pointing to the result of the check on the checker's infrastructure.
+| `started` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check started processing.
+| `finished` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check finished processing.
+| `created` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check was created.
+| `updated` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check was last updated.
+| `checker_name` | optional | The name of the checker that produced this check.<br />Only set if [checker details](#option-checker) are requested.
+| `checker_status` | optional | The [status](rest-api-checkers.md#checker-info) of the checker that produced this check.<br />Only set if [checker details](#option-checker) are requested.
+| `blocking` | optional | Set of [blocking conditions](rest-api-checkers.md#blocking-conditions) that apply to this checker.<br />Only set if [checker details](#option-checker) are requested.
+| `checker_description` | optional | The description of the checker that reported this check.
### <a id="check-input"> CheckInput
The `CheckInput` entity contains information for creating or updating a check.
| Field Name | | Description |
| --------------- | -------- | ----------- |
-| `checker_uuid` | optional | The name of the checker. Must be specified for checker creation. Optional only if updating a check and referencing the checker using the [UUID](./rest-api-checkers.md#checker-id) in the URL.
+| `checker_uuid` | optional | The [UUID](#checker-id) of the checker. Must be specified for check creation. Optional only if updating a check and referencing the checker using the [UUID](./rest-api-checkers.md#checker-id) in the URL.
| `state` | optional | The state as string-serialized form of [CheckState](#check-state)
| `message` | optional | Short message explaining the check state.
| `url` | optional | A fully-qualified URL pointing to the result of the check on the checker's infrastructure.
diff --git a/resources/Documentation/rest-api-pending-checks.md b/resources/Documentation/rest-api-pending-checks.md
index c28c0a1..91771e3 100644
--- a/resources/Documentation/rest-api-pending-checks.md
+++ b/resources/Documentation/rest-api-pending-checks.md
@@ -44,7 +44,7 @@
#### Request by checker
```
- GET /plugins/@PLUGIN@/checks.pending/?query=checker=test:my-checker+(state:NOT_STARTED+OR+state:SCHEDULED) HTTP/1.0
+ GET /plugins/@PLUGIN@/checks.pending/?query=checker:test:my-checker+(state:NOT_STARTED+OR+state:SCHEDULED) HTTP/1.0
```
As response a list of [PendingChecksInfo](#pending-checks-info)
