Merge changes I29bc8ee1,Ica77b8f5,I05ceb86c,I3be67af9,I542286a7, ...
* changes:
CheckerInput: Rename field 'blockingConditions' to 'blocking'
Test listing pending checks for checker that has an invalid query
ListChecksIT: Add tests with non-current patch set
GetCheckIT: Test more special cases
Fix binding of ChecksSubmitRule
Test that getting a check with options works if checker is invalid
GetCheckIT: Only assert things that are relevant to the tests
ListChecksIT: Let each test care about the test setup itself
CheckersByRepositoryNotes: Fix loading checkers from old revision
Add unit test for CheckersByRepositoryNotes
Test that setting an invalid URL on checks is not allowed
CreateCheckIT: Add more tests
CheckInput: Mark all fields as Nullable
Fix update finished field in check and add more update check tests
Consistently use the name 'repository' in the checks API
CheckerConfig: Set new updated timestamp only when config has changed
CheckerConfig: Don't reload config file on save
CheckerConfig#getConfigForTesting: Return Optional<Config>
Rename createdOn/updateOn in CheckerInfo to created/updated
CheckerUpdate: Remove support for specifying updated timestamp
Test that creation/last-updated timestamps for checks/checkers are reasonable
diff --git a/java/com/google/gerrit/plugins/checks/CheckJson.java b/java/com/google/gerrit/plugins/checks/CheckJson.java
index 3d55666..4b5e9f3 100644
--- a/java/com/google/gerrit/plugins/checks/CheckJson.java
+++ b/java/com/google/gerrit/plugins/checks/CheckJson.java
@@ -62,7 +62,7 @@
CheckInfo info = new CheckInfo();
info.checkerUuid = check.key().checkerUuid().get();
info.changeNumber = check.key().patchSet().changeId.id;
- info.project = check.key().project().get();
+ info.repository = check.key().repository().get();
info.patchSetId = check.key().patchSet().patchSetId;
info.state = check.state();
diff --git a/java/com/google/gerrit/plugins/checks/CheckKey.java b/java/com/google/gerrit/plugins/checks/CheckKey.java
index 3b5bc3a..526f8f2 100644
--- a/java/com/google/gerrit/plugins/checks/CheckKey.java
+++ b/java/com/google/gerrit/plugins/checks/CheckKey.java
@@ -21,14 +21,14 @@
/** Fields to identify a check. */
@AutoValue
public abstract class CheckKey {
- public abstract Project.NameKey project();
+ public abstract Project.NameKey repository();
public abstract PatchSet.Id patchSet();
public abstract CheckerUuid checkerUuid();
public static CheckKey create(
- Project.NameKey project, PatchSet.Id patchSet, CheckerUuid checkerUuid) {
- return new AutoValue_CheckKey(project, patchSet, checkerUuid);
+ Project.NameKey repositoryName, PatchSet.Id patchSetId, CheckerUuid checkerUuid) {
+ return new AutoValue_CheckKey(repositoryName, patchSetId, checkerUuid);
}
}
diff --git a/java/com/google/gerrit/plugins/checks/Checker.java b/java/com/google/gerrit/plugins/checks/Checker.java
index 67a29f8..aa43340 100644
--- a/java/com/google/gerrit/plugins/checks/Checker.java
+++ b/java/com/google/gerrit/plugins/checks/Checker.java
@@ -119,14 +119,14 @@
*
* @return the creation timestamp
*/
- public abstract Timestamp getCreatedOn();
+ public abstract Timestamp getCreated();
/**
* Returns the timestamp of when the checker was last updated.
*
* @return the last updated timestamp
*/
- public abstract Timestamp getUpdatedOn();
+ public abstract Timestamp getUpdated();
/**
* Returns the ref state of the checker.
@@ -269,9 +269,9 @@
public abstract Builder setQuery(String query);
- public abstract Builder setCreatedOn(Timestamp createdOn);
+ public abstract Builder setCreated(Timestamp created);
- public abstract Builder setUpdatedOn(Timestamp updatedOn);
+ public abstract Builder setUpdated(Timestamp updated);
public abstract Builder setRefState(ObjectId refState);
diff --git a/java/com/google/gerrit/plugins/checks/CheckerJson.java b/java/com/google/gerrit/plugins/checks/CheckerJson.java
index 27f96ec..701a169 100644
--- a/java/com/google/gerrit/plugins/checks/CheckerJson.java
+++ b/java/com/google/gerrit/plugins/checks/CheckerJson.java
@@ -30,8 +30,8 @@
info.status = checker.getStatus();
info.blocking = checker.getBlockingConditions();
info.query = checker.getQuery().orElse(null);
- info.createdOn = checker.getCreatedOn();
- info.updatedOn = checker.getUpdatedOn();
+ info.created = checker.getCreated();
+ info.updated = checker.getUpdated();
return info;
}
}
diff --git a/java/com/google/gerrit/plugins/checks/CheckerUpdate.java b/java/com/google/gerrit/plugins/checks/CheckerUpdate.java
index e555180..97165f6 100644
--- a/java/com/google/gerrit/plugins/checks/CheckerUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/CheckerUpdate.java
@@ -19,7 +19,6 @@
import com.google.gerrit.plugins.checks.api.BlockingCondition;
import com.google.gerrit.plugins.checks.api.CheckerStatus;
import com.google.gerrit.reviewdb.client.Project;
-import java.sql.Timestamp;
import java.util.Optional;
@AutoValue
@@ -60,16 +59,6 @@
/** Defines the new query for the checker. If not specified, the query remains unchanged. */
public abstract Optional<String> getQuery();
- /**
- * Defines the {@code Timestamp} to be used for the NoteDb commits of the update. If not
- * specified, the current {@code Timestamp} when creating the commit will be used.
- *
- * <p>If this {@code CheckerUpdate} is passed next to a {@link CheckerCreation} during a checker
- * creation, this {@code Timestamp} is used for the NoteDb commits of the new checker. Hence, the
- * {@link Checker#getCreatedOn()} field will match this {@code Timestamp}.
- */
- public abstract Optional<Timestamp> getUpdatedOn();
-
public abstract Builder toBuilder();
public static Builder builder() {
@@ -93,8 +82,6 @@
public abstract Builder setQuery(String query);
- public abstract Builder setUpdatedOn(Timestamp timestamp);
-
public abstract CheckerUpdate build();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/Module.java b/java/com/google/gerrit/plugins/checks/Module.java
index 987a227..102c057 100644
--- a/java/com/google/gerrit/plugins/checks/Module.java
+++ b/java/com/google/gerrit/plugins/checks/Module.java
@@ -24,6 +24,7 @@
import com.google.gerrit.plugins.checks.api.ChangeCheckAttributeFactory;
import com.google.gerrit.plugins.checks.api.ChangeCheckAttributeFactory.GetChangeOptions;
import com.google.gerrit.plugins.checks.db.NoteDbCheckersModule;
+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.git.validators.CommitValidationListener;
@@ -57,5 +58,6 @@
.to(GetChangeOptions.class);
install(new ApiModule());
+ install(new ChecksSubmitRule.Module());
}
}
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/TestModule.java b/java/com/google/gerrit/plugins/checks/acceptance/TestModule.java
index c04a4d3..3348659 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/TestModule.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/TestModule.java
@@ -14,14 +14,11 @@
package com.google.gerrit.plugins.checks.acceptance;
-import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.plugins.checks.Module;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckOperations;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckOperationsImpl;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckerOperations;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckerOperationsImpl;
-import com.google.gerrit.plugins.checks.rules.ChecksSubmitRule;
-import com.google.gerrit.server.rules.SubmitRule;
import com.google.inject.AbstractModule;
public class TestModule extends AbstractModule {
@@ -33,8 +30,5 @@
// setup in tests as realistic as possible by delegating to the original module.
bind(CheckerOperations.class).to(CheckerOperationsImpl.class);
bind(CheckOperations.class).to(CheckOperationsImpl.class);
- bind(SubmitRule.class)
- .annotatedWith(Exports.named("ChecksSubmitRule"))
- .to(ChecksSubmitRule.class);
}
}
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java
index 5d78aae..169fcc7 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java
@@ -32,6 +32,7 @@
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -41,7 +42,7 @@
public final class CheckOperationsImpl implements CheckOperations {
private final Checks checks;
- private final ChecksUpdate checksUpdate;
+ private final Provider<ChecksUpdate> checksUpdate;
private final CheckJson.Factory checkJsonFactory;
private final GitRepositoryManager repoManager;
@@ -50,7 +51,7 @@
Checks checks,
GitRepositoryManager repoManager,
CheckJson.Factory checkJsonFactory,
- @ServerInitiated ChecksUpdate checksUpdate) {
+ @ServerInitiated Provider<ChecksUpdate> checksUpdate) {
this.checks = checks;
this.repoManager = repoManager;
this.checkJsonFactory = checkJsonFactory;
@@ -65,7 +66,7 @@
@Override
public TestCheckUpdate.Builder newCheck(CheckKey key) {
return TestCheckUpdate.builder(key)
- .setCheckUpdater(u -> checksUpdate.createCheck(key, toCheckUpdate(u)));
+ .setCheckUpdater(u -> checksUpdate.get().createCheck(key, toCheckUpdate(u)));
}
final class PerCheckOperationsImpl implements PerCheckOperations {
@@ -87,7 +88,7 @@
@Override
public ImmutableMap<RevId, String> notesAsText() throws Exception {
- try (Repository repo = repoManager.openRepository(key.project());
+ try (Repository repo = repoManager.openRepository(key.repository());
RevWalk rw = new RevWalk(repo)) {
Ref checkRef =
repo.getRefDatabase().exactRef(CheckerRef.checksRef(key.patchSet().changeId));
@@ -115,7 +116,8 @@
@Override
public TestCheckUpdate.Builder forUpdate() {
return TestCheckUpdate.builder(key)
- .setCheckUpdater(testUpdate -> checksUpdate.updateCheck(key, toCheckUpdate(testUpdate)));
+ .setCheckUpdater(
+ testUpdate -> checksUpdate.get().updateCheck(key, toCheckUpdate(testUpdate)));
}
}
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckTestData.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckTestData.java
new file mode 100644
index 0000000..db977b7
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckTestData.java
@@ -0,0 +1,22 @@
+// 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.acceptance.testsuite;
+
+public class CheckTestData {
+ /** An invalid check URL that is not allowed to be set for a check. */
+ public static final String INVALID_URL = "ftp://example.com/my-check";
+
+ private CheckTestData() {}
+}
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 bbc0386..fe4d54f 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
@@ -39,6 +39,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Optional;
@@ -65,7 +66,7 @@
@Singleton
public class CheckerOperationsImpl implements CheckerOperations {
private final Checkers checkers;
- private final CheckersUpdate checkersUpdate;
+ private final Provider<CheckersUpdate> checkersUpdate;
private final GitRepositoryManager repoManager;
private final AllProjectsName allProjectsName;
private final CheckerJson checkerJson;
@@ -74,7 +75,7 @@
@Inject
public CheckerOperationsImpl(
Checkers checkers,
- @ServerInitiated CheckersUpdate checkersUpdate,
+ @ServerInitiated Provider<CheckersUpdate> checkersUpdate,
GitRepositoryManager repoManager,
AllProjectsName allProjectsName,
CheckerJson checkerJson) {
@@ -100,7 +101,7 @@
throws OrmDuplicateKeyException, ConfigInvalidException, IOException {
CheckerCreation checkerCreation = toCheckerCreation(testCheckerCreation);
CheckerUpdate checkerUpdate = toCheckerUpdate(testCheckerCreation);
- Checker checker = checkersUpdate.createChecker(checkerCreation, checkerUpdate);
+ Checker checker = checkersUpdate.get().createChecker(checkerCreation, checkerUpdate);
return checker.getUuid();
}
@@ -235,7 +236,7 @@
private void updateChecker(TestCheckerUpdate testCheckerUpdate) throws Exception {
CheckerUpdate checkerUpdate = toCheckerUpdate(testCheckerUpdate);
- checkersUpdate.updateChecker(checkerUuid, checkerUpdate);
+ checkersUpdate.get().updateChecker(checkerUuid, checkerUpdate);
if (testCheckerUpdate.forceInvalidConfig()) {
try (Repository repo = repoManager.openRepository(allProjectsName)) {
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckInfo.java b/java/com/google/gerrit/plugins/checks/api/CheckInfo.java
index be8f34a..3096fec 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckInfo.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckInfo.java
@@ -22,8 +22,8 @@
/** REST API representation of a {@link com.google.gerrit.plugins.checks.Check}. */
public class CheckInfo {
- /** Project name that this check applies to. */
- public String project;
+ /** Repository name that this check applies to. */
+ public String repository;
/** Change number that this check applies to. */
public int changeNumber;
/** Patch set ID that this check applies to. */
@@ -60,7 +60,7 @@
return false;
}
CheckInfo other = (CheckInfo) o;
- return Objects.equals(other.project, project)
+ return Objects.equals(other.repository, repository)
&& Objects.equals(other.changeNumber, changeNumber)
&& Objects.equals(other.patchSetId, patchSetId)
&& Objects.equals(other.checkerUuid, checkerUuid)
@@ -78,7 +78,7 @@
@Override
public int hashCode() {
return Objects.hash(
- project,
+ repository,
changeNumber,
patchSetId,
checkerUuid,
@@ -96,7 +96,7 @@
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
- .add("project", project)
+ .add("repository", repository)
.add("changeNumber", changeNumber)
.add("patchSetId", patchSetId)
.add("checkerUuid", checkerUuid)
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckInput.java b/java/com/google/gerrit/plugins/checks/api/CheckInput.java
index 31d8a35..633d2fb 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckInput.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckInput.java
@@ -22,9 +22,9 @@
/** Input to create or update a {@link com.google.gerrit.plugins.checks.Check}. */
public class CheckInput {
/** UUID of the checker. */
- public String checkerUuid;
+ @Nullable public String checkerUuid;
/** State of the check. */
- public CheckState state;
+ @Nullable public CheckState state;
/** Fully qualified URL to detailed result on the Checker's service. */
@Nullable public String url;
/** Date/Time at which the checker started processing this check. */
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckablePatchSetInfo.java b/java/com/google/gerrit/plugins/checks/api/CheckablePatchSetInfo.java
index c8747e1..1581585 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckablePatchSetInfo.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckablePatchSetInfo.java
@@ -16,8 +16,8 @@
/** REST API representation of a patch set for which checks are pending. */
public class CheckablePatchSetInfo {
- /** Project name. */
- public String project;
+ /** Repository name. */
+ public String repository;
/** Change number. */
public int changeNumber;
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckerInfo.java b/java/com/google/gerrit/plugins/checks/api/CheckerInfo.java
index d6af4a6..7f99d98 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckerInfo.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckerInfo.java
@@ -28,13 +28,13 @@
public CheckerStatus status;
public Set<BlockingCondition> blocking;
public String query;
- public Timestamp createdOn;
- public Timestamp updatedOn;
+ public Timestamp created;
+ public Timestamp updated;
@Override
public int hashCode() {
return Objects.hash(
- uuid, name, description, url, repository, status, blocking, query, createdOn, updatedOn);
+ uuid, name, description, url, repository, status, blocking, query, created, updated);
}
@Override
@@ -51,8 +51,8 @@
&& Objects.equals(status, o.status)
&& Objects.equals(blocking, o.blocking)
&& Objects.equals(query, o.query)
- && Objects.equals(createdOn, o.createdOn)
- && Objects.equals(updatedOn, o.updatedOn);
+ && Objects.equals(created, o.created)
+ && Objects.equals(updated, o.updated);
}
@Override
@@ -66,8 +66,8 @@
.add("status", status)
.add("blockingConditions", blocking)
.add("query", query)
- .add("createdOn", createdOn)
- .add("updatedOn", updatedOn)
+ .add("created", created)
+ .add("updated", updated)
.toString();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckerInput.java b/java/com/google/gerrit/plugins/checks/api/CheckerInput.java
index 9d609c6..1f4b498 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckerInput.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckerInput.java
@@ -23,6 +23,6 @@
public String url;
public String repository;
public CheckerStatus status;
- public Set<BlockingCondition> blockingConditions;
+ public Set<BlockingCondition> blocking;
public String query;
}
diff --git a/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java b/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
index db02463..7a02911 100644
--- a/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
+++ b/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
@@ -70,8 +70,8 @@
() ->
new ResourceNotFoundException(
String.format(
- "Patch set %s in project %s doesn't have check for checker %s.",
- checkKey.patchSet(), checkKey.project(), checkerUuid))));
+ "Patch set %s in repository %s doesn't have check for checker %s.",
+ checkKey.patchSet(), checkKey.repository(), checkerUuid))));
}
@Override
diff --git a/java/com/google/gerrit/plugins/checks/api/CreateChecker.java b/java/com/google/gerrit/plugins/checks/api/CreateChecker.java
index b5743bd..8bea2f3 100644
--- a/java/com/google/gerrit/plugins/checks/api/CreateChecker.java
+++ b/java/com/google/gerrit/plugins/checks/api/CreateChecker.java
@@ -104,9 +104,8 @@
if (input.status != null) {
checkerUpdateBuilder.setStatus(input.status);
}
- if (input.blockingConditions != null) {
- checkerUpdateBuilder.setBlockingConditions(
- ImmutableSortedSet.copyOf(input.blockingConditions));
+ if (input.blocking != null) {
+ checkerUpdateBuilder.setBlockingConditions(ImmutableSortedSet.copyOf(input.blocking));
}
if (input.query != null) {
checkerUpdateBuilder.setQuery(CheckerQuery.clean(input.query));
diff --git a/java/com/google/gerrit/plugins/checks/api/ListPendingChecks.java b/java/com/google/gerrit/plugins/checks/api/ListPendingChecks.java
index e3da451..18e2f8c 100644
--- a/java/com/google/gerrit/plugins/checks/api/ListPendingChecks.java
+++ b/java/com/google/gerrit/plugins/checks/api/ListPendingChecks.java
@@ -120,12 +120,13 @@
}
private Optional<PendingChecksInfo> getPostFilteredPendingChecks(
- Project.NameKey project, PatchSet.Id patchSetId) throws OrmException, IOException {
- CheckState checkState = getCheckState(project, patchSetId);
+ Project.NameKey repositoryName, PatchSet.Id patchSetId) throws OrmException, IOException {
+ CheckState checkState = getCheckState(repositoryName, patchSetId);
if (!states.contains(checkState)) {
return Optional.empty();
}
- return Optional.of(createPendingChecksInfo(project, patchSetId, checkerUuid, checkState));
+ return Optional.of(
+ createPendingChecksInfo(repositoryName, patchSetId, checkerUuid, checkState));
}
private CheckState getCheckState(Project.NameKey project, PatchSet.Id patchSetId)
@@ -142,14 +143,14 @@
}
private static PendingChecksInfo createPendingChecksInfo(
- Project.NameKey project,
+ Project.NameKey repositoryName,
PatchSet.Id patchSetId,
CheckerUuid checkerUuid,
CheckState checkState) {
PendingChecksInfo pendingChecksInfo = new PendingChecksInfo();
pendingChecksInfo.patchSet = new CheckablePatchSetInfo();
- pendingChecksInfo.patchSet.project = project.get();
+ pendingChecksInfo.patchSet.repository = repositoryName.get();
pendingChecksInfo.patchSet.changeNumber = patchSetId.getParentKey().get();
pendingChecksInfo.patchSet.patchSetId = patchSetId.get();
diff --git a/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java b/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
index fb5b653..2b24640 100644
--- a/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
@@ -46,7 +46,7 @@
} else if (!checkResource.getCheckerUuid().get().equals(input.checkerUuid)) {
throw new BadRequestException(
String.format(
- "checkerUuid must either be null or the same as on the resource:\n"
+ "checker UUID in input must either be null or the same as on the resource:\n"
+ "the check resource belongs to checker %s,"
+ " but in the input checker %s was specified",
checkResource.getCheckerUuid(), input.checkerUuid));
diff --git a/java/com/google/gerrit/plugins/checks/api/UpdateChecker.java b/java/com/google/gerrit/plugins/checks/api/UpdateChecker.java
index b20117b..362fde8 100644
--- a/java/com/google/gerrit/plugins/checks/api/UpdateChecker.java
+++ b/java/com/google/gerrit/plugins/checks/api/UpdateChecker.java
@@ -103,9 +103,8 @@
checkerUpdateBuilder.setStatus(input.status);
}
- if (input.blockingConditions != null) {
- checkerUpdateBuilder.setBlockingConditions(
- ImmutableSortedSet.copyOf(input.blockingConditions));
+ if (input.blocking != null) {
+ checkerUpdateBuilder.setBlockingConditions(ImmutableSortedSet.copyOf(input.blocking));
}
if (input.query != null) {
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java b/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
index dde68b0..98738d4 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
@@ -166,7 +166,7 @@
private Optional<CheckerCreation> checkerCreation = Optional.empty();
private Optional<CheckerUpdate> checkerUpdate = Optional.empty();
private Optional<Checker.Builder> updatedCheckerBuilder = Optional.empty();
- private Config config;
+ private Optional<Config> loadedConfig = Optional.empty();
private boolean isLoaded = false;
private CheckerConfig(String ref) {
@@ -242,8 +242,8 @@
}
@VisibleForTesting
- public Config getConfigForTesting() {
- return config;
+ public Optional<Config> getConfigForTesting() {
+ return loadedConfig;
}
@Override
@@ -253,12 +253,13 @@
rw.markStart(revision);
rw.sort(RevSort.REVERSE);
RevCommit earliestCommit = rw.next();
- Timestamp createdOn = new Timestamp(earliestCommit.getCommitTime() * 1000L);
- Timestamp updatedOn = new Timestamp(rw.parseCommit(revision).getCommitTime() * 1000L);
+ Timestamp created = new Timestamp(earliestCommit.getCommitTime() * 1000L);
+ Timestamp updated = new Timestamp(rw.parseCommit(revision).getCommitTime() * 1000L);
- config = readConfig(CHECKER_CONFIG_FILE);
- Checker checker = createFrom(config, createdOn, updatedOn, revision.toObjectId());
+ Config checkerConfig = readConfig(CHECKER_CONFIG_FILE);
+ Checker checker = createFrom(checkerConfig, created, updated, revision.toObjectId());
loadedChecker = Optional.of(checker);
+ loadedConfig = Optional.of(checkerConfig);
checkerUuid = Optional.of(checker.getUuid());
}
@@ -275,23 +276,33 @@
ensureThatMandatoryPropertiesAreSet();
- // Commit timestamps are internally truncated to seconds. To return the correct 'createdOn' time
+ // Commit timestamps are internally truncated to seconds. To return the correct 'created' time
// for new checkers, we explicitly need to truncate the timestamp here.
Timestamp commitTimestamp =
- TimeUtil.truncateToSecond(
- checkerUpdate.flatMap(CheckerUpdate::getUpdatedOn).orElseGet(TimeUtil::nowTs));
+ TimeUtil.truncateToSecond(new Timestamp(commit.getCommitter().getWhen().getTime()));
commit.setAuthor(new PersonIdent(commit.getAuthor(), commitTimestamp));
commit.setCommitter(new PersonIdent(commit.getCommitter(), commitTimestamp));
- Checker.Builder checker = updateChecker(commitTimestamp);
+ Config oldConfig = copyOrElseNew(loadedConfig);
+ Config newConfig = copyOrElseNew(loadedConfig);
+
+ Checker.Builder checker = updateChecker(newConfig, commitTimestamp);
+
+ if (oldConfig.toText().equals(newConfig.toText())) {
+ // Don't create a new commit if nothing was changed.
+ return false;
+ }
+
updatedCheckerBuilder = Optional.of(checker);
checkerUuid = Optional.of(checker.getUuid());
+ loadedConfig = Optional.of(newConfig);
String commitMessage = createCommitMessage(loadedChecker);
commit.setMessage(commitMessage);
checkerCreation = Optional.empty();
checkerUpdate = Optional.empty();
+
return true;
}
@@ -325,15 +336,14 @@
return Optional.empty();
}
- private Checker.Builder updateChecker(Timestamp commitTimestamp)
+ private Checker.Builder updateChecker(Config config, Timestamp commitTimestamp)
throws IOException, ConfigInvalidException {
- Config config = updateCheckerProperties();
- Timestamp createdOn = loadedChecker.map(Checker::getCreatedOn).orElse(commitTimestamp);
- return createBuilderFrom(config, createdOn, commitTimestamp);
+ updateCheckerProperties(config);
+ Timestamp created = loadedChecker.map(Checker::getCreated).orElse(commitTimestamp);
+ return createBuilderFrom(config, created, commitTimestamp);
}
- private Config updateCheckerProperties() throws IOException, ConfigInvalidException {
- Config config = readConfig(CHECKER_CONFIG_FILE);
+ private void updateCheckerProperties(Config config) throws IOException {
checkerCreation.ifPresent(
checkerCreation ->
Arrays.stream(CheckerConfigEntry.values())
@@ -343,10 +353,9 @@
Arrays.stream(CheckerConfigEntry.values())
.forEach(configEntry -> configEntry.updateConfigValue(config, checkerUpdate)));
saveConfig(CHECKER_CONFIG_FILE, config);
- return config;
}
- private Checker.Builder createBuilderFrom(Config config, Timestamp createdOn, Timestamp updatedOn)
+ private Checker.Builder createBuilderFrom(Config config, Timestamp created, Timestamp updated)
throws ConfigInvalidException {
Checker.Builder checker = Checker.builder();
@@ -361,16 +370,35 @@
}
configEntry.readFromConfig(checker.getUuid(), checker, config);
}
- return checker.setCreatedOn(createdOn).setUpdatedOn(updatedOn);
+ return checker.setCreated(created).setUpdated(updated);
}
- private Checker createFrom(
- Config config, Timestamp createdOn, Timestamp updatedOn, ObjectId refState)
+ private Checker createFrom(Config config, Timestamp created, Timestamp updated, ObjectId refState)
throws ConfigInvalidException {
- return createBuilderFrom(config, createdOn, updatedOn).setRefState(refState).build();
+ return createBuilderFrom(config, created, updated).setRefState(refState).build();
}
private static String createCommitMessage(Optional<Checker> originalChecker) {
return originalChecker.isPresent() ? "Update checker" : "Create checker";
}
+
+ private static Config copyOrElseNew(Optional<Config> config) throws ConfigInvalidException {
+ return config.isPresent() ? copyConfig(config.get()) : new Config();
+ }
+
+ /**
+ * Copies the provided config.
+ *
+ * <p><strong>Note:</strong> This method only works for configs that have no base config.
+ * Unfortunately we cannot check that the provided config has no base config, so callers must take
+ * care to use this method only for configs without base config.
+ *
+ * @param config the config that should be copied
+ * @return the copy of the config
+ */
+ private static Config copyConfig(Config config) throws ConfigInvalidException {
+ Config copiedConfig = new Config();
+ copiedConfig.fromText(config.toText());
+ return copiedConfig;
+ }
}
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotes.java b/java/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotes.java
index 65d5719..0150b56 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotes.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotes.java
@@ -146,10 +146,10 @@
return load();
}
if (ObjectId.zeroId().equals(rev)) {
- load(allProjectsName, repo, null);
+ super.load(allProjectsName, repo, null);
return this;
}
- load(allProjectsName, repo, rev);
+ super.load(allProjectsName, repo, rev);
return this;
}
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java b/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
index 3c9ad40..8f6482a 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
@@ -38,8 +38,8 @@
return newCheck.build();
}
- Check toCheck(Project.NameKey project, PatchSet.Id patchSetId, CheckerUuid checkerUuid) {
- CheckKey key = CheckKey.create(project, patchSetId, checkerUuid);
+ Check toCheck(Project.NameKey repositoryName, PatchSet.Id patchSetId, CheckerUuid checkerUuid) {
+ CheckKey key = CheckKey.create(repositoryName, patchSetId, checkerUuid);
return toCheck(key);
}
@@ -67,7 +67,7 @@
started = update.started().get();
modified = true;
}
- if (update.finished().isPresent() && update.finished().get().equals(finished)) {
+ if (update.finished().isPresent() && !update.finished().get().equals(finished)) {
finished = update.finished().get();
modified = true;
}
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersUpdate.java b/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersUpdate.java
index 6da5a2a..5c3ccd6 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersUpdate.java
@@ -33,12 +33,10 @@
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryHelper.ActionType;
-import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
-import java.sql.Timestamp;
import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.BatchRefUpdate;
@@ -227,11 +225,6 @@
@Override
public Checker updateChecker(CheckerUuid checkerUuid, CheckerUpdate checkerUpdate)
throws NoSuchCheckerException, IOException, ConfigInvalidException {
- Optional<Timestamp> updatedOn = checkerUpdate.getUpdatedOn();
- if (!updatedOn.isPresent()) {
- updatedOn = Optional.of(TimeUtil.nowTs());
- checkerUpdate = checkerUpdate.toBuilder().setUpdatedOn(updatedOn.get()).build();
- }
return updateCheckerWithRetry(checkerUuid, checkerUpdate);
}
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
index 1e3a6f9..c73a0f1 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
@@ -85,14 +85,14 @@
throws OrmException, IOException {
// TODO(gerrit-team): Instead of reading the complete notes map, read just one note.
Optional<Check> result =
- getChecksFromNoteDb(checkKey.project(), checkKey.patchSet(), GetCheckOptions.defaults())
+ getChecksFromNoteDb(checkKey.repository(), checkKey.patchSet(), GetCheckOptions.defaults())
.stream()
.filter(c -> c.key().checkerUuid().equals(checkKey.checkerUuid()))
.findAny();
if (!result.isPresent() && options.backfillChecks()) {
ChangeData changeData =
- changeDataFactory.create(checkKey.project(), checkKey.patchSet().getParentKey());
+ changeDataFactory.create(checkKey.repository(), checkKey.patchSet().getParentKey());
return checkBackfiller.getBackfilledCheckForRelevantChecker(
checkKey.checkerUuid(), changeData, checkKey.patchSet());
}
@@ -101,10 +101,10 @@
}
private ImmutableList<Check> getChecksFromNoteDb(
- Project.NameKey projectName, PatchSet.Id psId, GetCheckOptions options)
+ Project.NameKey repositoryName, PatchSet.Id psId, GetCheckOptions options)
throws OrmException, IOException {
// TODO(gerrit-team): Instead of reading the complete notes map, read just one note.
- ChangeData changeData = changeDataFactory.create(projectName, psId.getParentKey());
+ ChangeData changeData = changeDataFactory.create(repositoryName, psId.getParentKey());
PatchSet patchSet = changeData.patchSet(psId);
CheckNotes checkNotes = checkNotesFactory.create(changeData.change());
checkNotes.load();
@@ -112,7 +112,7 @@
ImmutableList<Check> existingChecks =
checkNotes.getChecks().getOrDefault(patchSet.getRevision(), NoteDbCheckMap.empty()).checks
.entrySet().stream()
- .map(e -> e.getValue().toCheck(projectName, psId, CheckerUuid.parse(e.getKey())))
+ .map(e -> e.getValue().toCheck(repositoryName, psId, CheckerUuid.parse(e.getKey())))
.collect(toImmutableList());
if (!options.backfillChecks()) {
@@ -120,7 +120,7 @@
}
ImmutableList<Checker> checkersForBackfiller =
- getCheckersForBackfiller(projectName, existingChecks);
+ getCheckersForBackfiller(repositoryName, existingChecks);
ImmutableList<Check> backfilledChecks =
checkBackfiller.getBackfilledChecksForRelevantCheckers(
checkersForBackfiller, changeData, psId);
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
index de2758e..87fb008 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
@@ -160,7 +160,7 @@
assertCheckerIsPresent(checkKey.checkerUuid());
}
- try (Repository repo = repoManager.openRepository(checkKey.project());
+ try (Repository repo = repoManager.openRepository(checkKey.repository());
ObjectInserter objectInserter = repo.newObjectInserter();
RevWalk rw = new RevWalk(repo)) {
Ref checkRef = repo.getRefDatabase().exactRef(checksRef(checkKey.patchSet().getParentKey()));
@@ -195,7 +195,7 @@
RefUpdateUtil.checkResult(refUpdate);
gitRefUpdated.fire(
- checkKey.project(), refUpdate, currentUser.map(user -> user.state()).orElse(null));
+ checkKey.repository(), refUpdate, currentUser.map(user -> user.state()).orElse(null));
return readSingleCheck(checkKey, repo, rw, newCommitId);
}
}
diff --git a/java/com/google/gerrit/plugins/checks/testing/BUILD b/java/com/google/gerrit/plugins/checks/testing/BUILD
index a613c7b..6e4e2b7 100644
--- a/java/com/google/gerrit/plugins/checks/testing/BUILD
+++ b/java/com/google/gerrit/plugins/checks/testing/BUILD
@@ -10,6 +10,7 @@
"//java/com/google/gerrit/git/testing",
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/truth",
+ "//lib/jgit/org.eclipse.jgit:jgit",
"//lib/truth",
"//lib/truth:truth-java8-extension",
"//plugins:plugin-api",
diff --git a/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java b/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
index e8715f7..60ea7cc 100644
--- a/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
+++ b/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
@@ -34,6 +34,7 @@
import com.google.gerrit.truth.OptionalSubject;
import java.sql.Timestamp;
import java.util.Optional;
+import org.eclipse.jgit.lib.Config;
public class CheckerConfigSubject extends Subject<CheckerConfigSubject, CheckerConfig> {
public static CheckerConfigSubject assertThat(CheckerConfig checkerConfig) {
@@ -84,8 +85,8 @@
check("query()").about(optionals()).that(checker().getQuery()).isEmpty();
}
- public ComparableSubject<?, Timestamp> hasCreatedOnThat() {
- return check("createdOn()").that(checker().getCreatedOn());
+ public ComparableSubject<?, Timestamp> hasCreatedThat() {
+ return check("created()").that(checker().getCreated());
}
public ObjectIdSubject hasRefStateThat() {
@@ -94,8 +95,10 @@
public IterableSubject configStringList(String name) {
isNotNull();
+ Optional<Config> checkerConfig = actual().getConfigForTesting();
+ check("configValueOf(checker.%s)", name).about(optionals()).that(checkerConfig).isPresent();
return check("configValueOf(checker.%s)", name)
- .that(actual().getConfigForTesting().getStringList("checker", null, name))
+ .that(checkerConfig.get().getStringList("checker", null, name))
.asList();
}
diff --git a/java/com/google/gerrit/plugins/checks/testing/PendingChecksInfoSubject.java b/java/com/google/gerrit/plugins/checks/testing/PendingChecksInfoSubject.java
index 72c6269..a8151a0 100644
--- a/java/com/google/gerrit/plugins/checks/testing/PendingChecksInfoSubject.java
+++ b/java/com/google/gerrit/plugins/checks/testing/PendingChecksInfoSubject.java
@@ -35,8 +35,10 @@
super(metadata, actual);
}
- public void hasProject(Project.NameKey expectedProject) {
- check("patchSet().project()").that(patchSet().project).isEqualTo(expectedProject.get());
+ public void hasRepository(Project.NameKey expectedRepositoryName) {
+ check("patchSet().repository()")
+ .that(patchSet().repository)
+ .isEqualTo(expectedRepositoryName.get());
}
public void hasPatchSet(PatchSet.Id expectedPatchSetId) {
diff --git a/javatests/com/google/gerrit/plugins/checks/CheckerDefinitionTest.java b/javatests/com/google/gerrit/plugins/checks/CheckerDefinitionTest.java
index 7b9481e..1c8c9ab 100644
--- a/javatests/com/google/gerrit/plugins/checks/CheckerDefinitionTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/CheckerDefinitionTest.java
@@ -53,8 +53,8 @@
.setRepository(new NameKey("test-repo"))
.setStatus(CheckerStatus.ENABLED)
.setUuid(CheckerUuid.parse("schema:any-id"))
- .setCreatedOn(TimeUtil.nowTs())
- .setUpdatedOn(TimeUtil.nowTs())
+ .setCreated(TimeUtil.nowTs())
+ .setUpdated(TimeUtil.nowTs())
.setRefState(ObjectId.zeroId());
}
}
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 8ef5e51..93bbdd4 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
@@ -15,16 +15,19 @@
package com.google.gerrit.plugins.checks.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.plugins.checks.Check;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckOperations.PerCheckOperations;
+import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckTestData;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckerTestData;
import com.google.gerrit.plugins.checks.api.CheckInfo;
import com.google.gerrit.plugins.checks.api.CheckInput;
@@ -33,6 +36,7 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.TestTimeUtil;
import com.google.inject.Inject;
import java.sql.Timestamp;
@@ -75,6 +79,7 @@
CheckInfo info = checksApiFactory.revision(patchSetId).create(input).get();
assertThat(info.checkerUuid).isEqualTo(checkerUuid.get());
assertThat(info.state).isEqualTo(CheckState.RUNNING);
+ assertThat(info.url).isNull();
assertThat(info.started).isNull();
assertThat(info.finished).isNull();
assertThat(info.created).isEqualTo(expectedCreationTimestamp);
@@ -92,6 +97,91 @@
}
@Test
+ public void cannotCreateCheckWithoutCheckerUuid() throws Exception {
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("checker UUID is required");
+ checksApiFactory.revision(patchSetId).create(new CheckInput());
+ }
+
+ @Test
+ public void createCheckWithStateNotStarted() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.state = CheckState.NOT_STARTED;
+
+ CheckInfo info = checksApiFactory.revision(patchSetId).create(input).get();
+ assertThat(info.state).isEqualTo(input.state);
+ assertThat(getCheck(project, patchSetId, checkerUuid).state()).isEqualTo(input.state);
+ }
+
+ @Test
+ public void createCheckWithoutState() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+
+ CheckInfo info = checksApiFactory.revision(patchSetId).create(input).get();
+ assertThat(info.state).isEqualTo(CheckState.NOT_STARTED);
+ assertThat(getCheck(project, patchSetId, checkerUuid).state())
+ .isEqualTo(CheckState.NOT_STARTED);
+ }
+
+ @Test
+ public void createCheckWithUrl() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.url = "http://example.com/my-check";
+
+ CheckInfo info = checksApiFactory.revision(patchSetId).create(input).get();
+ assertThat(info.url).isEqualTo(input.url);
+ assertThat(getCheck(project, patchSetId, checkerUuid).url()).hasValue(input.url);
+ }
+
+ @Test
+ public void cannotCreateCheckWithInvalidUrl() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.url = CheckTestData.INVALID_URL;
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("only http/https URLs supported: " + input.url);
+ checksApiFactory.revision(patchSetId).create(input);
+ }
+
+ @Test
+ public void createCheckWithStarted() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.started = TimeUtil.nowTs();
+
+ CheckInfo info = checksApiFactory.revision(patchSetId).create(input).get();
+ assertThat(info.started).isEqualTo(input.started);
+ assertThat(getCheck(project, patchSetId, checkerUuid).started()).hasValue(input.started);
+ }
+
+ @Test
+ public void createCheckWithFinished() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.finished = TimeUtil.nowTs();
+
+ CheckInfo info = checksApiFactory.revision(patchSetId).create(input).get();
+ assertThat(info.finished).isEqualTo(input.finished);
+ assertThat(getCheck(project, patchSetId, checkerUuid).finished()).hasValue(input.finished);
+ }
+
+ @Test
public void cannotCreateCheckForInvalidCheckerUuid() throws Exception {
CheckInput input = new CheckInput();
input.checkerUuid = CheckerTestData.INVALID_UUID;
@@ -227,6 +317,11 @@
// 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)
+ throws Exception {
+ return checkOperations.check(CheckKey.create(project, patchSetId, checkerUuid)).get();
+ }
+
private String noteDbContent(String uuid, Timestamp created, Timestamp updated) {
return ""
+ "{\n"
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckerIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckerIT.java
index 5474101..98a04b8 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckerIT.java
@@ -37,6 +37,8 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.testing.TestTimeUtil;
import com.google.inject.Inject;
+import java.sql.Timestamp;
+import java.time.Instant;
import java.util.concurrent.TimeUnit;
import org.junit.After;
import org.junit.Before;
@@ -49,6 +51,7 @@
@Before
public void setTimeForTesting() {
TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
+ TestTimeUtil.setClock(Timestamp.from(Instant.EPOCH));
}
@After
@@ -60,6 +63,7 @@
public void createChecker() throws Exception {
Project.NameKey repositoryName = projectOperations.newProject().create();
+ Timestamp expectedCreationTimestamp = TestTimeUtil.getCurrentTimestamp();
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
input.repository = repositoryName.get();
@@ -72,15 +76,12 @@
assertThat(info.status).isEqualTo(CheckerStatus.ENABLED);
assertThat(info.blocking).isEmpty();
assertThat(info.query).isEqualTo("status:open");
- assertThat(info.createdOn).isNotNull();
- assertThat(info.updatedOn).isEqualTo(info.createdOn);
+ assertThat(info.created).isEqualTo(expectedCreationTimestamp);
+ assertThat(info.updated).isEqualTo(info.created);
PerCheckerOperations perCheckerOps = checkerOperations.checker(info.uuid);
assertCommit(
- perCheckerOps.commit(),
- "Create checker",
- info.createdOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Create checker", info.created, perCheckerOps.get().getRefState());
assertThat(checkerOperations.sha1sOfRepositoriesWithCheckers())
.containsExactly(CheckersByRepositoryNotes.computeRepositorySha1(repositoryName));
assertThat(checkerOperations.checkersOf(repositoryName))
@@ -98,10 +99,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(info.uuid);
assertCommit(
- perCheckerOps.commit(),
- "Create checker",
- info.createdOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Create checker", info.created, perCheckerOps.get().getRefState());
}
@Test
@@ -115,10 +113,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(info.uuid);
assertCommit(
- perCheckerOps.commit(),
- "Create checker",
- info.createdOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Create checker", info.created, perCheckerOps.get().getRefState());
}
@Test
@@ -132,10 +127,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(info.uuid);
assertCommit(
- perCheckerOps.commit(),
- "Create checker",
- info.createdOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Create checker", info.created, perCheckerOps.get().getRefState());
}
@Test
@@ -149,10 +141,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(info.uuid);
assertCommit(
- perCheckerOps.commit(),
- "Create checker",
- info.createdOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Create checker", info.created, perCheckerOps.get().getRefState());
}
@Test
@@ -166,10 +155,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(info.uuid);
assertCommit(
- perCheckerOps.commit(),
- "Create checker",
- info.createdOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Create checker", info.created, perCheckerOps.get().getRefState());
}
@Test
@@ -183,10 +169,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(info.uuid);
assertCommit(
- perCheckerOps.commit(),
- "Create checker",
- info.createdOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Create checker", info.created, perCheckerOps.get().getRefState());
}
@Test
@@ -199,10 +182,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(info.uuid);
assertCommit(
- perCheckerOps.commit(),
- "Create checker",
- info.createdOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Create checker", info.created, perCheckerOps.get().getRefState());
}
@Test
@@ -346,7 +326,7 @@
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
input.repository = allProjects.get();
- input.blockingConditions = ImmutableSet.of(BlockingCondition.STATE_NOT_PASSING);
+ input.blocking = ImmutableSet.of(BlockingCondition.STATE_NOT_PASSING);
CheckerInfo info = checkersApi.create(input).get();
assertThat(info.blocking).containsExactly(BlockingCondition.STATE_NOT_PASSING);
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 3c9d886..2fe5429 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
@@ -19,6 +19,7 @@
import static com.google.common.truth.Truth.assert_;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -35,31 +36,57 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.testing.TestTimeUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.sql.Timestamp;
+import java.time.Instant;
+import java.util.concurrent.TimeUnit;
+import org.junit.After;
import org.junit.Before;
import org.junit.Test;
public class GetCheckIT extends AbstractCheckersTest {
@Inject private RequestScopeOperations requestScopeOperations;
+ private String changeId;
private PatchSet.Id patchSetId;
@Before
public void setUp() throws Exception {
- patchSetId = createChange().getPatchSetId();
+ TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
+ TestTimeUtil.setClock(Timestamp.from(Instant.EPOCH));
+
+ PushOneCommit.Result r = createChange();
+ changeId = r.getChangeId();
+ patchSetId = r.getPatchSetId();
+ }
+
+ @After
+ public void resetTime() {
+ TestTimeUtil.useSystemTime();
}
@Test
- public void getCheckReturnsProject() throws Exception {
+ public void getCheck() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).setState(CheckState.RUNNING).upsert();
+
+ assertThat(checksApiFactory.revision(patchSetId).id(checkerUuid).get())
+ .isEqualTo(checkOperations.check(checkKey).asInfo());
+ }
+
+ @Test
+ public void getCheckReturnsRepository() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).upsert();
- assertThat(getCheckInfo(patchSetId, checkerUuid).project).isEqualTo(project.get());
- assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).project)
+ assertThat(getCheckInfo(patchSetId, checkerUuid).repository).isEqualTo(project.get());
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).repository)
.isEqualTo(project.get());
}
@@ -128,11 +155,10 @@
public void getCheckReturnsCreationTimestamp() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ Timestamp expectedCreationTimestamp = TestTimeUtil.getCurrentTimestamp();
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).upsert();
- Timestamp expectedCreationTimestamp = checkOperations.check(checkKey).get().created();
-
assertThat(getCheckInfo(patchSetId, checkerUuid).created).isEqualTo(expectedCreationTimestamp);
assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).created)
.isEqualTo(expectedCreationTimestamp);
@@ -142,11 +168,10 @@
public void getCheckReturnsUpdatedTimestamp() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ Timestamp expectedUpdatedTimestamp = TestTimeUtil.getCurrentTimestamp();
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).upsert();
- Timestamp expectedUpdatedTimestamp = checkOperations.check(checkKey).get().updated();
-
assertThat(getCheckInfo(patchSetId, checkerUuid).created).isEqualTo(expectedUpdatedTimestamp);
assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).created)
.isEqualTo(expectedUpdatedTimestamp);
@@ -195,27 +220,48 @@
}
@Test
+ public void getCheckWithCheckerOptionReturnsCheckEvenIfCheckerIsInvalid() throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().repository(project).name("My Checker").create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).setState(CheckState.RUNNING).upsert();
+
+ checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+
+ CheckInfo check = getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER);
+ assertThat(check).isNotNull();
+
+ // Checker fields are not set.
+ assertThat(check.checkerName).isNull();
+ assertThat(check.blocking).isNull();
+ assertThat(check.checkerStatus).isNull();
+
+ // Check that at least some non-checker fields are set to ensure that we didn't get a completely
+ // empty CheckInfo.
+ assertThat(check.repository).isEqualTo(project.get());
+ assertThat(check.checkerUuid).isEqualTo(checkerUuid.get());
+ assertThat(check.changeNumber).isEqualTo(patchSetId.getParentKey().get());
+ assertThat(check.patchSetId).isEqualTo(patchSetId.get());
+ assertThat(check.state).isEqualTo(CheckState.RUNNING);
+ }
+
+ @Test
public void getCheckForCheckerThatDoesNotApplyToTheProject() throws Exception {
Project.NameKey otherProject = createProjectOverAPI("other", null, true, null);
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(otherProject).create();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
- CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
- checkOperations.newCheck(checkKey).upsert();
-
- assertThat(getCheckInfo(patchSetId, checkerUuid))
- .isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
}
@Test
public void getCheckForCheckerThatDoesNotApplyToTheChange() throws Exception {
CheckerUuid checkerUuid =
checkerOperations.newChecker().repository(project).query("message:not-matching").create();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
- CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
- checkOperations.newCheck(checkKey).upsert();
-
- assertThat(getCheckInfo(patchSetId, checkerUuid))
- .isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
}
@Test
@@ -226,12 +272,9 @@
.repository(project)
.query(CheckerTestData.QUERY_WITH_UNSUPPORTED_OPERATOR)
.create();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
- CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
- checkOperations.newCheck(checkKey).upsert();
-
- CheckInfo checkInfo = checksApiFactory.revision(patchSetId).id(checkerUuid).get();
- assertThat(checkInfo).isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
}
@Test
@@ -242,48 +285,79 @@
.repository(project)
.query(CheckerTestData.INVALID_QUERY)
.create();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
- CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
- checkOperations.newCheck(checkKey).upsert();
-
- CheckInfo checkInfo = checksApiFactory.revision(patchSetId).id(checkerUuid).get();
- assertThat(checkInfo).isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
}
@Test
public void getCheckForDisabledChecker() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
-
- CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
- checkOperations.newCheck(checkKey).upsert();
-
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
checkerOperations.checker(checkerUuid).forUpdate().disable().update();
- assertThat(getCheckInfo(patchSetId, checkerUuid))
- .isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
}
@Test
public void getCheckForInvalidChecker() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
- checkOperations.newCheck(checkKey).upsert();
-
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
- assertThat(getCheckInfo(patchSetId, checkerUuid))
- .isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
+ }
+
+ @Test
+ public void noBackfillForInvalidChecker() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
+
+ checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+ assertCheckNotFound(patchSetId, checkerUuid);
+ }
+
+ @Test
+ public void getChecksForMultiplePatchSets() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey1 = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey1).setState(CheckState.FAILED).upsert();
+
+ PatchSet.Id currentPatchSetId = createPatchSet();
+ CheckKey checkKey2 = CheckKey.create(project, currentPatchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey2).setState(CheckState.RUNNING).upsert();
+
+ // get check for the old patch set
+ assertThat(checksApiFactory.revision(patchSetId).id(checkerUuid).get())
+ .isEqualTo(checkOperations.check(checkKey1).asInfo());
+
+ // get check for the current patch set (2 ways)
+ assertThat(checksApiFactory.revision(currentPatchSetId).id(checkerUuid).get())
+ .isEqualTo(checkOperations.check(checkKey2).asInfo());
+ assertThat(
+ checksApiFactory
+ .currentRevision(currentPatchSetId.getParentKey())
+ .id(checkerUuid)
+ .get())
+ .isEqualTo(checkOperations.check(checkKey2).asInfo());
+ }
+
+ @Test
+ public void noBackfillForNonCurrentPatchSet() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ PatchSet.Id currentPatchSetId = createPatchSet();
+ assertCheckNotFound(patchSetId, checkerUuid);
+ assertThat(getCheckInfo(currentPatchSetId, checkerUuid)).isNotNull();
}
@Test
public void getCheckForNonExistingChecker() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
- checkOperations.newCheck(checkKey).upsert();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
checkerOperations.checker(checkerUuid).forUpdate().deleteRef().update();
- assertThat(getCheckInfo(patchSetId, checkerUuid))
- .isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
}
@Test
@@ -300,7 +374,7 @@
Timestamp psCreated = getPatchSetCreated(changeId);
CheckInfo expected = new CheckInfo();
- expected.project = checkKey.project().get();
+ expected.repository = checkKey.repository().get();
expected.changeNumber = checkKey.patchSet().getParentKey().get();
expected.patchSetId = checkKey.patchSet().get();
expected.checkerUuid = checkKey.checkerUuid().get();
@@ -318,7 +392,7 @@
Timestamp psCreated = getPatchSetCreated(changeId);
CheckInfo expected = new CheckInfo();
- expected.project = checkKey.project().get();
+ expected.repository = checkKey.repository().get();
expected.changeNumber = checkKey.patchSet().getParentKey().get();
expected.patchSetId = checkKey.patchSet().get();
expected.checkerUuid = checkKey.checkerUuid().get();
@@ -344,12 +418,9 @@
requestScopeOperations.setApiUser(user.getId());
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
- CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
- checkOperations.newCheck(checkKey).upsert();
-
- assertThat(getCheckInfo(patchSetId, checkerUuid))
- .isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
}
@Test
@@ -393,8 +464,16 @@
.hasMessageThat()
.isEqualTo(
String.format(
- "Patch set %s in project %s doesn't have check for checker %s.",
+ "Patch set %s in repository %s doesn't have check for checker %s.",
patchSetId, project, checkerUuid));
}
}
+
+ private PatchSet.Id createPatchSet() throws Exception {
+ PushOneCommit.Result r = amendChange(changeId);
+ PatchSet.Id currentPatchSetId = r.getPatchSetId();
+ assertThat(patchSetId.changeId).isEqualTo(currentPatchSetId.changeId);
+ assertThat(patchSetId.get()).isLessThan(currentPatchSetId.get());
+ return currentPatchSetId;
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
index 960e3bc..de28631 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
@@ -30,15 +30,31 @@
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.restapi.config.ListCapabilities;
import com.google.gerrit.server.restapi.config.ListCapabilities.CapabilityInfo;
+import com.google.gerrit.testing.TestTimeUtil;
import com.google.inject.Inject;
import java.sql.Timestamp;
+import java.time.Instant;
import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
public class GetCheckerIT extends AbstractCheckersTest {
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private ListCapabilities listCapabilities;
+ @Before
+ public void setUp() throws Exception {
+ TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
+ TestTimeUtil.setClock(Timestamp.from(Instant.EPOCH));
+ }
+
+ @After
+ public void resetTime() {
+ TestTimeUtil.useSystemTime();
+ }
+
@Test
public void getCheckerReturnsUuid() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
@@ -141,20 +157,18 @@
@Test
public void getCheckerReturnsCreationTimestamp() throws Exception {
+ Timestamp expectedCreationTimestamp = TestTimeUtil.getCurrentTimestamp();
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- Timestamp expectedCreationTimestamp =
- checkerOperations.checker(checkerUuid).get().getCreatedOn();
- assertThat(getCheckerInfo(checkerUuid).createdOn).isEqualTo(expectedCreationTimestamp);
+ assertThat(getCheckerInfo(checkerUuid).created).isEqualTo(expectedCreationTimestamp);
}
@Test
public void getCheckerReturnsUpdatedTimestamp() throws Exception {
+ Timestamp expectedUpdatedTimestamp = TestTimeUtil.getCurrentTimestamp();
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- Timestamp expectedUpdatedTimestamp =
- checkerOperations.checker(checkerUuid).get().getUpdatedOn();
- assertThat(getCheckerInfo(checkerUuid).updatedOn).isEqualTo(expectedUpdatedTimestamp);
+ assertThat(getCheckerInfo(checkerUuid).updated).isEqualTo(expectedUpdatedTimestamp);
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
index cdd5f29..3b3012a 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckerUuid;
@@ -39,44 +40,44 @@
import org.junit.Test;
public class ListChecksIT extends AbstractCheckersTest {
+ private String changeId;
private PatchSet.Id patchSetId;
- private CheckKey checkKey1;
- private CheckKey checkKey2;
@Before
public void setUp() throws Exception {
- patchSetId = createChange().getPatchSetId();
-
- CheckerUuid checker1Uuid = checkerOperations.newChecker().repository(project).create();
- CheckerUuid checker2Uuid =
- checkerOperations.newChecker().name("Checker Two").repository(project).create();
-
- checkKey1 = CheckKey.create(project, patchSetId, checker1Uuid);
- checkOperations.newCheck(checkKey1).setState(CheckState.RUNNING).upsert();
-
- checkKey2 = CheckKey.create(project, patchSetId, checker2Uuid);
- checkOperations.newCheck(checkKey2).setState(CheckState.RUNNING).upsert();
+ PushOneCommit.Result r = createChange();
+ changeId = r.getChangeId();
+ patchSetId = r.getPatchSetId();
}
@Test
public void listAll() throws Exception {
+ CheckerUuid checkerUuid1 = checkerOperations.newChecker().repository(project).create();
+ CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey1 = CheckKey.create(project, patchSetId, checkerUuid1);
+ checkOperations.newCheck(checkKey1).setState(CheckState.RUNNING).upsert();
+
+ CheckKey checkKey2 = CheckKey.create(project, patchSetId, checkerUuid2);
+ checkOperations.newCheck(checkKey2).setState(CheckState.FAILED).upsert();
+
Collection<CheckInfo> checks = checksApiFactory.revision(patchSetId).list();
CheckInfo expected1 = new CheckInfo();
- expected1.project = checkKey1.project().get();
- expected1.changeNumber = checkKey1.patchSet().getParentKey().get();
- expected1.patchSetId = checkKey1.patchSet().get();
- expected1.checkerUuid = checkKey1.checkerUuid().get();
+ expected1.repository = project.get();
+ expected1.changeNumber = patchSetId.getParentKey().get();
+ expected1.patchSetId = patchSetId.get();
+ expected1.checkerUuid = checkerUuid1.get();
expected1.state = CheckState.RUNNING;
expected1.created = checkOperations.check(checkKey1).get().created();
expected1.updated = expected1.created;
CheckInfo expected2 = new CheckInfo();
- expected2.project = checkKey2.project().get();
- expected2.changeNumber = checkKey2.patchSet().getParentKey().get();
- expected2.patchSetId = checkKey2.patchSet().get();
- expected2.checkerUuid = checkKey2.checkerUuid().get();
- expected2.state = CheckState.RUNNING;
+ expected2.repository = project.get();
+ expected2.changeNumber = patchSetId.getParentKey().get();
+ expected2.patchSetId = patchSetId.get();
+ expected2.checkerUuid = checkerUuid2.get();
+ expected2.state = CheckState.FAILED;
expected2.created = checkOperations.check(checkKey2).get().created();
expected2.updated = expected2.created;
@@ -85,29 +86,40 @@
@Test
public void listAllWithOptions() throws Exception {
+ String checkerName1 = "Checker One";
+ CheckerUuid checkerUuid1 =
+ checkerOperations.newChecker().name(checkerName1).repository(project).create();
+ CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey1 = CheckKey.create(project, patchSetId, checkerUuid1);
+ checkOperations.newCheck(checkKey1).setState(CheckState.RUNNING).upsert();
+
+ CheckKey checkKey2 = CheckKey.create(project, patchSetId, checkerUuid2);
+ checkOperations.newCheck(checkKey2).setState(CheckState.FAILED).upsert();
+
Collection<CheckInfo> checks =
checksApiFactory.revision(patchSetId).list(ListChecksOption.CHECKER);
CheckInfo expected1 = new CheckInfo();
- expected1.project = checkKey1.project().get();
- expected1.changeNumber = checkKey1.patchSet().getParentKey().get();
- expected1.patchSetId = checkKey1.patchSet().get();
- expected1.checkerUuid = checkKey1.checkerUuid().get();
+ expected1.repository = project.get();
+ expected1.changeNumber = patchSetId.getParentKey().get();
+ expected1.patchSetId = patchSetId.get();
+ expected1.checkerUuid = checkerUuid1.get();
expected1.state = CheckState.RUNNING;
expected1.created = checkOperations.check(checkKey1).get().created();
expected1.updated = expected1.created;
+ expected1.checkerName = checkerName1;
expected1.blocking = ImmutableSet.of();
expected1.checkerStatus = CheckerStatus.ENABLED;
CheckInfo expected2 = new CheckInfo();
- expected2.project = checkKey2.project().get();
- expected2.changeNumber = checkKey2.patchSet().getParentKey().get();
- expected2.patchSetId = checkKey2.patchSet().get();
- expected2.checkerUuid = checkKey2.checkerUuid().get();
- expected2.state = CheckState.RUNNING;
+ expected2.repository = project.get();
+ expected2.changeNumber = patchSetId.getParentKey().get();
+ expected2.patchSetId = patchSetId.get();
+ expected2.checkerUuid = checkerUuid2.get();
+ expected2.state = CheckState.FAILED;
expected2.created = checkOperations.check(checkKey2).get().created();
expected2.updated = expected2.created;
- expected2.checkerName = "Checker Two";
expected2.blocking = ImmutableSet.of();
expected2.checkerStatus = CheckerStatus.ENABLED;
@@ -116,141 +128,164 @@
@Test
public void listAllWithOptionsSkipsPopulatingCheckerFieldsForInvalidCheckers() throws Exception {
- checkerOperations.checker(checkKey1.checkerUuid()).forUpdate().forceInvalidConfig().update();
+ String checkerName1 = "Checker One";
+ CheckerUuid checkerUuid1 =
+ checkerOperations.newChecker().name(checkerName1).repository(project).create();
+ CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid1)).upsert();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid2)).upsert();
+ checkerOperations.checker(checkerUuid2).forUpdate().forceInvalidConfig().update();
List<CheckInfo> checks = checksApiFactory.revision(patchSetId).list(ListChecksOption.CHECKER);
assertThat(checks).hasSize(2);
Optional<CheckInfo> maybeCheck1 =
- checks.stream().filter(c -> c.checkerUuid.equals(checkKey1.checkerUuid().get())).findAny();
+ checks.stream().filter(c -> c.checkerUuid.equals(checkerUuid1.get())).findAny();
assertThat(maybeCheck1).isPresent();
CheckInfo check1 = maybeCheck1.get();
- assertThat(check1.checkerName).isNull();
- assertThat(check1.blocking).isNull();
- assertThat(check1.checkerStatus).isNull();
+ assertThat(check1.checkerName).isEqualTo(checkerName1);
+ assertThat(check1.blocking).isEmpty();
+ assertThat(check1.checkerStatus).isEqualTo(CheckerStatus.ENABLED);
Optional<CheckInfo> maybeCheck2 =
- checks.stream().filter(c -> c.checkerUuid.equals(checkKey2.checkerUuid().get())).findAny();
+ checks.stream().filter(c -> c.checkerUuid.equals(checkerUuid2.get())).findAny();
assertThat(maybeCheck2).isPresent();
CheckInfo check2 = maybeCheck2.get();
- assertThat(check2.checkerName).isEqualTo("Checker Two");
- assertThat(check2.blocking).isEmpty();
- assertThat(check2.checkerStatus).isEqualTo(CheckerStatus.ENABLED);
+ assertThat(check2.checkerName).isNull();
+ assertThat(check2.blocking).isNull();
+ assertThat(check2.checkerStatus).isNull();
}
@Test
public void listIncludesCheckFromCheckerThatDoesNotApplyToTheProject() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
Project.NameKey otherProject = createProjectOverAPI("other", null, true, null);
- checkerOperations
- .checker(checkKey2.checkerUuid())
- .forUpdate()
- .repository(otherProject)
- .update();
+ checkerOperations.checker(checkerUuid).forUpdate().repository(otherProject).update();
- Collection<CheckInfo> checks = checksApiFactory.revision(patchSetId).list();
- assertThat(checks)
- .containsExactly(
- checkOperations.check(checkKey1).asInfo(), checkOperations.check(checkKey2).asInfo());
+ assertThat(checksApiFactory.revision(patchSetId).list()).hasSize(1);
}
@Test
public void listIncludesCheckFromCheckerThatDoesNotApplyToTheChange() throws Exception {
- checkerOperations
- .checker(checkKey2.checkerUuid())
- .forUpdate()
- .query("message:not-matching")
- .update();
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- Collection<CheckInfo> checks = checksApiFactory.revision(patchSetId).list();
- assertThat(checks)
- .containsExactly(
- checkOperations.check(checkKey1).asInfo(), checkOperations.check(checkKey2).asInfo());
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).upsert();
+
+ checkerOperations.checker(checkerUuid).forUpdate().query("message:not-matching").update();
+
+ assertThat(checksApiFactory.revision(patchSetId).list()).hasSize(1);
}
@Test
public void listIncludesCheckFromDisabledChecker() throws Exception {
- checkerOperations.checker(checkKey2.checkerUuid()).forUpdate().disable().update();
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
+ checkerOperations.checker(checkerUuid).forUpdate().disable().update();
- Collection<CheckInfo> checks = checksApiFactory.revision(patchSetId).list();
- assertThat(checks)
- .containsExactly(
- checkOperations.check(checkKey1).asInfo(), checkOperations.check(checkKey2).asInfo());
+ assertThat(checksApiFactory.revision(patchSetId).list()).hasSize(1);
}
@Test
public void listIncludesCheckFromInvalidChecker() throws Exception {
- checkerOperations.checker(checkKey2.checkerUuid()).forUpdate().forceInvalidConfig().update();
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
+ checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
- Collection<CheckInfo> checks = checksApiFactory.revision(patchSetId).list();
- assertThat(checks)
- .containsExactly(
- checkOperations.check(checkKey1).asInfo(), checkOperations.check(checkKey2).asInfo());
+ assertThat(checksApiFactory.revision(patchSetId).list()).hasSize(1);
}
@Test
public void listIncludesCheckFromNonExistingChecker() throws Exception {
- checkerOperations.checker(checkKey2.checkerUuid()).forUpdate().deleteRef().update();
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
+ checkerOperations.checker(checkerUuid).forUpdate().deleteRef().update();
- Collection<CheckInfo> checks = checksApiFactory.revision(patchSetId).list();
- assertThat(checks)
- .containsExactly(
- checkOperations.check(checkKey1).asInfo(), checkOperations.check(checkKey2).asInfo());
+ assertThat(checksApiFactory.revision(patchSetId).list()).hasSize(1);
}
@Test
public void listBackfillsForRelevantChecker() throws Exception {
String topic = name("topic");
- Change.Id changeId = patchSetId.getParentKey();
- CheckerUuid checker3Uuid =
+ CheckerUuid checkerUuid =
checkerOperations.newChecker().repository(project).query("topic:" + topic).create();
- CheckKey checkKey3 = CheckKey.create(project, patchSetId, checker3Uuid);
- CheckInfo checkInfo1 = checkOperations.check(checkKey1).asInfo();
- CheckInfo checkInfo2 = checkOperations.check(checkKey2).asInfo();
- assertThat(checksApiFactory.revision(patchSetId).list())
- .containsExactly(checkInfo1, checkInfo2);
+ assertThat(checksApiFactory.revision(patchSetId).list()).isEmpty();
- // Update change to match checker3's query.
- gApi.changes().id(changeId.get()).topic(topic);
+ // Update change to match checker's query.
+ gApi.changes().id(patchSetId.getParentKey().get()).topic(topic);
Timestamp psCreated = getPatchSetCreated(patchSetId.getParentKey());
- CheckInfo checkInfo3 = new CheckInfo();
- checkInfo3.project = checkKey3.project().get();
- checkInfo3.changeNumber = checkKey3.patchSet().getParentKey().get();
- checkInfo3.patchSetId = checkKey3.patchSet().get();
- checkInfo3.checkerUuid = checkKey3.checkerUuid().get();
- checkInfo3.state = CheckState.NOT_STARTED;
- checkInfo3.created = psCreated;
- checkInfo3.updated = psCreated;
- assertThat(checksApiFactory.revision(patchSetId).list())
- .containsExactly(checkInfo1, checkInfo2, checkInfo3);
+ CheckInfo checkInfo = new CheckInfo();
+ checkInfo.repository = project.get();
+ checkInfo.changeNumber = patchSetId.getParentKey().get();
+ checkInfo.patchSetId = patchSetId.get();
+ checkInfo.checkerUuid = checkerUuid.get();
+ checkInfo.state = CheckState.NOT_STARTED;
+ checkInfo.created = psCreated;
+ checkInfo.updated = psCreated;
+ assertThat(checksApiFactory.revision(patchSetId).list()).containsExactly(checkInfo);
}
@Test
public void listDoesntBackfillForDisabledChecker() throws Exception {
- CheckerUuid checker3Uuid = checkerOperations.newChecker().repository(project).create();
- CheckKey checkKey3 = CheckKey.create(project, patchSetId, checker3Uuid);
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
- CheckInfo checkInfo1 = checkOperations.check(checkKey1).asInfo();
- CheckInfo checkInfo2 = checkOperations.check(checkKey2).asInfo();
Timestamp psCreated = getPatchSetCreated(patchSetId.getParentKey());
- CheckInfo checkInfo3 = new CheckInfo();
- checkInfo3.project = checkKey3.project().get();
- checkInfo3.changeNumber = checkKey3.patchSet().getParentKey().get();
- checkInfo3.patchSetId = checkKey3.patchSet().get();
- checkInfo3.checkerUuid = checkKey3.checkerUuid().get();
- checkInfo3.state = CheckState.NOT_STARTED;
- checkInfo3.created = psCreated;
- checkInfo3.updated = psCreated;
- assertThat(checksApiFactory.revision(patchSetId).list())
- .containsExactly(checkInfo1, checkInfo2, checkInfo3);
+ CheckInfo checkInfo = new CheckInfo();
+ checkInfo.repository = checkKey.repository().get();
+ checkInfo.changeNumber = checkKey.patchSet().getParentKey().get();
+ checkInfo.patchSetId = checkKey.patchSet().get();
+ checkInfo.checkerUuid = checkKey.checkerUuid().get();
+ checkInfo.state = CheckState.NOT_STARTED;
+ checkInfo.created = psCreated;
+ checkInfo.updated = psCreated;
+ assertThat(checksApiFactory.revision(patchSetId).list()).containsExactly(checkInfo);
- // Disable checker3.
- checkerOperations.checker(checker3Uuid).forUpdate().disable().update();
+ // Disable checker.
+ checkerOperations.checker(checkerUuid).forUpdate().disable().update();
+ assertThat(checksApiFactory.revision(patchSetId).list()).isEmpty();
+ }
+
+ @Test
+ public void listForMultiplePatchSets() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ // create check on the first patch set
+ CheckKey checkKey1 = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey1).setState(CheckState.SUCCESSFUL).upsert();
+
+ // create a second patch set
+ PatchSet.Id currentPatchSet = createPatchSet();
+
+ // create check on the second patch set, we expect that this check is not returned for the first
+ // patch set
+ CheckKey checkKey2 = CheckKey.create(project, currentPatchSet, checkerUuid);
+ checkOperations.newCheck(checkKey2).setState(CheckState.RUNNING).upsert();
+
+ // list checks for the old patch set
assertThat(checksApiFactory.revision(patchSetId).list())
- .containsExactly(checkInfo1, checkInfo2);
+ .containsExactly(checkOperations.check(checkKey1).asInfo());
+
+ // list checks for the new patch set (2 ways)
+ assertThat(checksApiFactory.currentRevision(currentPatchSet.getParentKey()).list())
+ .containsExactly(checkOperations.check(checkKey2).asInfo());
+ assertThat(checksApiFactory.revision(currentPatchSet).list())
+ .containsExactly(checkOperations.check(checkKey2).asInfo());
+ }
+
+ @Test
+ public void noBackfillForNonCurrentPatchSet() throws Exception {
+ checkerOperations.newChecker().repository(project).create();
+
+ PatchSet.Id currentPatchSet = createPatchSet();
+
+ assertThat(checksApiFactory.revision(patchSetId).list()).isEmpty();
+ assertThat(checksApiFactory.revision(currentPatchSet).list()).hasSize(1);
}
private Timestamp getPatchSetCreated(Change.Id changeId) throws RestApiException {
@@ -258,4 +293,12 @@
gApi.changes().id(changeId.get()).get(CURRENT_REVISION).revisions.values())
.created;
}
+
+ private PatchSet.Id createPatchSet() throws Exception {
+ PushOneCommit.Result r = amendChange(changeId);
+ PatchSet.Id currentPatchSetId = r.getPatchSetId();
+ assertThat(patchSetId.changeId).isEqualTo(currentPatchSetId.changeId);
+ assertThat(patchSetId.get()).isLessThan(currentPatchSetId.get());
+ return currentPatchSetId;
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListPendingChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListPendingChecksIT.java
index 60fe8f9..41ddec3 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListPendingChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListPendingChecksIT.java
@@ -114,7 +114,7 @@
List<PendingChecksInfo> pendingChecksList = pendingChecksApi.list(checkerUuid);
assertThat(pendingChecksList).hasSize(1);
PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
- assertThat(pendingChecks).hasProject(project);
+ assertThat(pendingChecks).hasRepository(project);
assertThat(pendingChecks).hasPatchSet(patchSetId);
assertThat(pendingChecks)
.hasPendingChecksMapThat()
@@ -149,7 +149,7 @@
pendingChecksApi.list(checkerUuid, CheckState.FAILED);
assertThat(pendingChecksList).hasSize(1);
PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
- assertThat(pendingChecks).hasProject(project);
+ assertThat(pendingChecks).hasRepository(project);
assertThat(pendingChecks).hasPatchSet(patchSetId);
assertThat(pendingChecks)
.hasPendingChecksMapThat()
@@ -195,14 +195,14 @@
// returned from the change index, which is by last updated timestamp. Use this knowledge here
// to do the assertions although the REST endpoint doesn't document a guaranteed sort order.
PendingChecksInfo pendingChecksChange2 = pendingChecksList.get(0);
- assertThat(pendingChecksChange2).hasProject(project);
+ assertThat(pendingChecksChange2).hasRepository(project);
assertThat(pendingChecksChange2).hasPatchSet(patchSetId2);
assertThat(pendingChecksChange2)
.hasPendingChecksMapThat()
.containsExactly(checkerUuid.get(), new PendingCheckInfo(CheckState.SCHEDULED));
PendingChecksInfo pendingChecksChange1 = pendingChecksList.get(1);
- assertThat(pendingChecksChange1).hasProject(project);
+ assertThat(pendingChecksChange1).hasRepository(project);
assertThat(pendingChecksChange1).hasPatchSet(patchSetId);
assertThat(pendingChecksChange1)
.hasPendingChecksMapThat()
@@ -215,7 +215,7 @@
List<PendingChecksInfo> pendingChecksList = pendingChecksApi.list(checkerUuid);
assertThat(pendingChecksList).hasSize(1);
PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
- assertThat(pendingChecks).hasProject(project);
+ assertThat(pendingChecks).hasRepository(project);
assertThat(pendingChecks).hasPatchSet(patchSetId);
assertThat(pendingChecks)
.hasPendingChecksMapThat()
@@ -318,6 +318,21 @@
}
@Test
+ public void listPendingChecksForCheckerWithInvalidQueryFails() throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations
+ .newChecker()
+ .repository(project)
+ .query(CheckerTestData.INVALID_QUERY)
+ .create();
+
+ exception.expect(RestApiException.class);
+ exception.expectMessage("Cannot list pending checks");
+ exception.expectCause(instanceOf(ConfigInvalidException.class));
+ pendingChecksApi.list(checkerUuid);
+ }
+
+ @Test
public void listPendingChecksWithoutAdministrateCheckersCapabilityWorks() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
checkOperations
@@ -330,7 +345,7 @@
pendingChecksApi.list(checkerUuid, CheckState.NOT_STARTED);
assertThat(pendingChecksList).hasSize(1);
PendingChecksInfo pendingChecks = Iterables.getOnlyElement(pendingChecksList);
- assertThat(pendingChecks).hasProject(project);
+ assertThat(pendingChecks).hasRepository(project);
assertThat(pendingChecks).hasPatchSet(patchSetId);
assertThat(pendingChecks)
.hasPendingChecksMapThat()
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 7ae4fac..debef5f 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
@@ -18,16 +18,24 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
+import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckTestData;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckerTestData;
import com.google.gerrit.plugins.checks.api.CheckInfo;
import com.google.gerrit.plugins.checks.api.CheckInput;
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 com.google.gerrit.testing.TestTimeUtil;
import com.google.inject.Inject;
+import java.sql.Timestamp;
+import java.time.Instant;
+import java.util.concurrent.TimeUnit;
+import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -39,6 +47,9 @@
@Before
public void setUp() throws Exception {
+ TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
+ TestTimeUtil.setClock(Timestamp.from(Instant.EPOCH));
+
patchSetId = createChange().getPatchSetId();
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
@@ -46,6 +57,11 @@
checkOperations.newCheck(checkKey).upsert();
}
+ @After
+ public void resetTime() {
+ TestTimeUtil.useSystemTime();
+ }
+
@Test
public void updateCheckState() throws Exception {
CheckInput input = new CheckInput();
@@ -56,6 +72,93 @@
}
@Test
+ public void cannotUpdateCheckerUuid() throws Exception {
+ CheckInput input = new CheckInput();
+ input.checkerUuid = "foo:bar";
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage(
+ "checker UUID in input must either be null or the same as on the resource");
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ }
+
+ @Test
+ public void specifyingCheckerUuidInInputThatMatchesTheCheckerUuidInTheUrlIsOkay()
+ throws Exception {
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkKey.checkerUuid().get();
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ }
+
+ @Test
+ public void updateUrl() throws Exception {
+ CheckInput input = new CheckInput();
+ input.url = "http://example.com/my-check";
+
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ assertThat(info.url).isEqualTo(input.url);
+ }
+
+ @Test
+ public void cannotSetInvalidUrl() throws Exception {
+ CheckInput input = new CheckInput();
+ input.url = CheckTestData.INVALID_URL;
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("only http/https URLs supported: " + input.url);
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ }
+
+ @Test
+ public void updateStarted() throws Exception {
+ CheckInput input = new CheckInput();
+ input.started = TimeUtil.nowTs();
+
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ assertThat(info.started).isEqualTo(input.started);
+ }
+
+ @Test
+ public void updateFinished() throws Exception {
+ CheckInput input = new CheckInput();
+ input.finished = TimeUtil.nowTs();
+
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ assertThat(info.finished).isEqualTo(input.finished);
+ }
+
+ @Test
+ public void updateWithEmptyInput() throws Exception {
+ assertThat(
+ checksApiFactory
+ .revision(patchSetId)
+ .id(checkKey.checkerUuid())
+ .update(new CheckInput()))
+ .isNotNull();
+ }
+
+ @Test
+ public void updateResultsInNewUpdatedTimestamp() throws Exception {
+ CheckInput input = new CheckInput();
+ input.state = CheckState.FAILED;
+
+ Timestamp expectedUpdateTimestamp = TestTimeUtil.getCurrentTimestamp();
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ assertThat(info.updated).isEqualTo(expectedUpdateTimestamp);
+ }
+
+ @Test
+ public void noOpUpdateDoesntResultInNewUpdatedTimestamp() throws Exception {
+ CheckInput input = new CheckInput();
+ input.state = CheckState.FAILED;
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ Timestamp expectedUpdateTimestamp = info.updated;
+
+ info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ assertThat(info.updated).isEqualTo(expectedUpdateTimestamp);
+ }
+
+ @Test
public void canUpdateCheckForDisabledChecker() throws Exception {
checkerOperations.checker(checkKey.checkerUuid()).forUpdate().disable().update();
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckerIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckerIT.java
index c7ab750..da9025a 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckerIT.java
@@ -40,6 +40,8 @@
import com.google.gerrit.testing.ConfigSuite;
import com.google.gerrit.testing.TestTimeUtil;
import com.google.inject.Inject;
+import java.sql.Timestamp;
+import java.time.Instant;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.lib.Config;
@@ -62,6 +64,7 @@
@Before
public void setTimeForTesting() {
TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
+ TestTimeUtil.setClock(Timestamp.from(Instant.EPOCH));
}
@After
@@ -89,15 +92,12 @@
assertThat(info.description).isEqualTo(input.description);
assertThat(info.url).isEqualTo(input.url);
assertThat(info.repository).isEqualTo(input.repository);
- assertThat(info.createdOn).isEqualTo(checker.getCreatedOn());
- assertThat(info.createdOn).isLessThan(info.updatedOn);
+ assertThat(info.created).isEqualTo(checker.getCreated());
+ assertThat(info.created).isLessThan(info.updated);
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
assertThat(checkerOperations.sha1sOfRepositoriesWithCheckers())
.containsExactly(CheckersByRepositoryNotes.computeRepositorySha1(repositoryName));
assertThat(checkerOperations.checkersOf(repositoryName))
@@ -141,10 +141,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
}
@Test
@@ -185,10 +182,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
}
@Test
@@ -203,10 +197,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
}
@Test
@@ -221,10 +212,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
}
@Test
@@ -239,10 +227,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
}
@Test
@@ -257,10 +242,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
}
@Test
@@ -275,10 +257,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
}
@Test
@@ -294,10 +273,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
}
@Test
@@ -313,10 +289,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
}
@Test
@@ -331,10 +304,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
}
@Test
@@ -351,10 +321,7 @@
PerCheckerOperations perCheckerOps = checkerOperations.checker(checkerUuid);
assertCommit(
- perCheckerOps.commit(),
- "Update checker",
- info.updatedOn,
- perCheckerOps.get().getRefState());
+ perCheckerOps.commit(), "Update checker", info.updated, perCheckerOps.get().getRefState());
assertThat(checkerOperations.sha1sOfRepositoriesWithCheckers())
.containsExactly(CheckersByRepositoryNotes.computeRepositorySha1(repositoryName));
assertThat(checkerOperations.checkersOf(repositoryName))
@@ -473,7 +440,7 @@
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
CheckerInput input = new CheckerInput();
- input.blockingConditions = ImmutableSet.of(BlockingCondition.STATE_NOT_PASSING);
+ input.blocking = ImmutableSet.of(BlockingCondition.STATE_NOT_PASSING);
CheckerInfo info = checkersApi.id(checkerUuid).update(input);
assertThat(info.blocking).containsExactly(BlockingCondition.STATE_NOT_PASSING);
}
@@ -549,6 +516,29 @@
}
@Test
+ public void updateResultsInNewUpdatedTimestamp() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+
+ Timestamp expectedUpdateTimestamp = TestTimeUtil.getCurrentTimestamp();
+ CheckerInput input = new CheckerInput();
+ input.name = "My Checker";
+ CheckerInfo info = checkersApi.id(checkerUuid).update(input);
+ assertThat(info.updated).isEqualTo(expectedUpdateTimestamp);
+ }
+
+ @Test
+ public void noOpUpdateDoesntResultInNewUpdatedTimestamp() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().name("My Checker").create();
+
+ Timestamp expectedUpdateTimestamp = checkerOperations.checker(checkerUuid).get().getUpdated();
+
+ CheckerInput input = new CheckerInput();
+ input.name = "My Checker";
+ CheckerInfo info = checkersApi.id(checkerUuid).update(input);
+ assertThat(info.updated).isEqualTo(expectedUpdateTimestamp);
+ }
+
+ @Test
public void updateCheckerWithoutAdministrateCheckersCapabilityFails() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckTestDataTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckTestDataTest.java
new file mode 100644
index 0000000..e81c171
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckTestDataTest.java
@@ -0,0 +1,41 @@
+// 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.acceptance.testsuite;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.plugins.checks.UrlValidator;
+import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
+import org.junit.Test;
+
+public class CheckTestDataTest extends AbstractCheckersTest {
+ @Test
+ public void verifyTestUrls() throws Exception {
+ try {
+ UrlValidator.clean(CheckTestData.INVALID_URL);
+ assert_().fail("expected BadRequestException");
+ } catch (BadRequestException e) {
+ assertMessage(e, "only http/https URLs supported", CheckTestData.INVALID_URL);
+ }
+ }
+
+ private static void assertMessage(Exception e, String... expectedMessageParts) {
+ for (String expectedMessagePart : expectedMessageParts) {
+ assertThat(e).hasMessageThat().ignoringCase().contains(expectedMessagePart);
+ }
+ }
+}
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 86c5469..4060c82 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
@@ -76,8 +76,8 @@
assertThat(foundChecker.blocking).isEmpty();
assertThat(foundChecker.url).isNull();
assertThat(foundChecker.description).isNull();
- assertThat(foundChecker.createdOn).isNotNull();
- assertThat(foundChecker.updatedOn).isNotNull();
+ assertThat(foundChecker.created).isNotNull();
+ assertThat(foundChecker.updated).isNotNull();
}
@Test
@@ -274,21 +274,21 @@
}
@Test
- public void createdOnOfExistingCheckerCanBeRetrieved() throws Exception {
+ public void createdOfExistingCheckerCanBeRetrieved() throws Exception {
CheckerInfo checker = checkersApi.create(createArbitraryCheckerInput()).get();
- Timestamp createdOn = checkerOperations.checker(checker.uuid).get().getCreatedOn();
+ Timestamp created = checkerOperations.checker(checker.uuid).get().getCreated();
- assertThat(createdOn).isEqualTo(checker.createdOn);
+ assertThat(created).isEqualTo(checker.created);
}
@Test
- public void updatedOnOfExistingCheckerCanBeRetrieved() throws Exception {
+ public void updatedOfExistingCheckerCanBeRetrieved() throws Exception {
CheckerInfo checker = checkersApi.create(createArbitraryCheckerInput()).get();
- Timestamp updatedOn = checkerOperations.checker(checker.uuid).get().getUpdatedOn();
+ Timestamp updated = checkerOperations.checker(checker.uuid).get().getUpdated();
- assertThat(updatedOn).isEqualTo(checker.updatedOn);
+ assertThat(updated).isEqualTo(checker.updated);
}
@Test
@@ -573,8 +573,8 @@
assertThat(checkerInfo.name).isEqualTo(checker.getName().get());
assertThat(checkerInfo.description).isEqualTo(checker.getDescription().get());
assertThat(checkerInfo.url).isEqualTo(checker.getUrl().get());
- assertThat(checkerInfo.createdOn).isEqualTo(checker.getCreatedOn());
- assertThat(checkerInfo.updatedOn).isEqualTo(checker.getUpdatedOn());
+ assertThat(checkerInfo.created).isEqualTo(checker.getCreated());
+ assertThat(checkerInfo.updated).isEqualTo(checker.getUpdated());
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java b/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java
index a473eef..05ba663 100644
--- a/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java
@@ -35,10 +35,6 @@
import com.google.gerrit.testing.GerritBaseTests;
import java.io.IOException;
import java.sql.Timestamp;
-import java.time.LocalDate;
-import java.time.LocalDateTime;
-import java.time.Month;
-import java.time.ZoneId;
import java.util.TimeZone;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
@@ -221,25 +217,13 @@
}
@Test
- public void createdOnDefaultsToNow() throws Exception {
+ public void createdDefaultsToNow() throws Exception {
// Git timestamps are only precise to the second.
Timestamp testStart = TimeUtil.truncateToSecond(TimeUtil.nowTs());
createArbitraryChecker(checkerUuid);
CheckerConfig checkerConfig = loadChecker(checkerUuid);
- assertThat(checkerConfig).hasCreatedOnThat().isAtLeast(testStart);
- }
-
- @Test
- public void specifiedCreatedOnIsRespectedForNewChecker() throws Exception {
- Timestamp createdOn = toTimestamp(LocalDate.of(2017, Month.DECEMBER, 11).atTime(13, 44, 10));
-
- CheckerCreation checkerCreation = getPrefilledCheckerCreationBuilder().build();
- CheckerUpdate checkerUpdate = CheckerUpdate.builder().setUpdatedOn(createdOn).build();
- createChecker(checkerCreation, checkerUpdate);
-
- CheckerConfig checkerConfig = loadChecker(checkerCreation.getCheckerUuid());
- assertThat(checkerConfig).hasCreatedOnThat().isEqualTo(createdOn);
+ assertThat(checkerConfig).hasCreatedThat().isAtLeast(testStart);
}
@Test
@@ -508,6 +492,24 @@
assertThat(updatedChecker).hasRefStateThat().isEqualTo(expectedRefStateAfterUpdate);
}
+ @Test
+ public void noNewCommitOnNoOpUpdate() throws Exception {
+ CheckerCreation checkerCreation =
+ getPrefilledCheckerCreationBuilder().setCheckerUuid(checkerUuid).build();
+ createChecker(checkerCreation);
+ ObjectId refState = getCheckerRefState(checkerUuid);
+
+ // Setting a description updates the ref.
+ CheckerUpdate checkerUpdate = CheckerUpdate.builder().setDescription("A description.").build();
+ updateChecker(checkerUuid, checkerUpdate);
+ ObjectId refState2 = getCheckerRefState(checkerUuid);
+ assertThat(refState2).isNotEqualTo(refState);
+
+ // Setting the same description again is a no-op and the ref is not updated.
+ updateChecker(checkerUuid, checkerUpdate);
+ assertThat(getCheckerRefState(checkerUuid)).isEqualTo(refState2);
+ }
+
private CheckerConfig createArbitraryChecker(CheckerUuid checkerUuid) throws Exception {
CheckerCreation checkerCreation =
getPrefilledCheckerCreationBuilder().setCheckerUuid(checkerUuid).build();
@@ -585,8 +587,4 @@
return assertThat(commit.getFullMessage()).named("commit message");
}
}
-
- private static Timestamp toTimestamp(LocalDateTime localDateTime) {
- return Timestamp.from(localDateTime.atZone(ZoneId.systemDefault()).toInstant());
- }
}
diff --git a/javatests/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotesTest.java b/javatests/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotesTest.java
new file mode 100644
index 0000000..59eca92
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotesTest.java
@@ -0,0 +1,334 @@
+// 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.db;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.truth.StringSubject;
+import com.google.gerrit.plugins.checks.CheckerRef;
+import com.google.gerrit.plugins.checks.CheckerUuid;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.util.time.TimeUtil;
+import java.io.IOException;
+import java.util.TimeZone;
+import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.junit.Before;
+import org.junit.Test;
+
+public class CheckersByRepositoryNotesTest {
+ private final TimeZone timeZone = TimeZone.getTimeZone("America/Los_Angeles");
+
+ private AllProjectsName projectName;
+ private Repository repository;
+
+ @Before
+ public void setUp() throws Exception {
+ projectName = new AllProjectsName("Test Repository");
+ repository = new InMemoryRepository(new DfsRepositoryDescription("Test Repository"));
+ }
+
+ @Test
+ public void getEmptyCheckersList() throws Exception {
+ CheckersByRepositoryNotes checkersByRepositoryNotes = loadCheckersByRepositoryNotes();
+ assertThat(checkersByRepositoryNotes.get(new Project.NameKey("some-project"))).isEmpty();
+ }
+
+ @Test
+ public void insertCheckers() throws Exception {
+ CheckersByRepositoryNotes checkersByRepositoryNotes = loadCheckersByRepositoryNotes();
+
+ Project.NameKey project1 = new Project.NameKey("some-project");
+ Project.NameKey project2 = new Project.NameKey("other-project");
+
+ CheckerUuid checkerUuid1 = CheckerUuid.parse("foo:bar");
+ checkersByRepositoryNotes.insert(checkerUuid1, project1);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project1)).containsExactly(checkerUuid1);
+ assertThat(checkersByRepositoryNotes.get(project2)).isEmpty();
+
+ CheckerUuid checkerUuid2 = CheckerUuid.parse("foo:baz");
+ checkersByRepositoryNotes.insert(checkerUuid2, project1);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project1))
+ .containsExactly(checkerUuid1, checkerUuid2)
+ .inOrder();
+ assertThat(checkersByRepositoryNotes.get(project2)).isEmpty();
+
+ CheckerUuid checkerUuid3 = CheckerUuid.parse("bar:baz");
+ checkersByRepositoryNotes.insert(checkerUuid3, project2);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project1))
+ .containsExactly(checkerUuid1, checkerUuid2)
+ .inOrder();
+ assertThat(checkersByRepositoryNotes.get(project2)).containsExactly(checkerUuid3);
+ }
+
+ @Test
+ public void insertCheckerTwice() throws Exception {
+ CheckersByRepositoryNotes checkersByRepositoryNotes = loadCheckersByRepositoryNotes();
+
+ CheckerUuid checkerUuid = CheckerUuid.parse("foo:bar");
+ Project.NameKey project = new Project.NameKey("some-project");
+
+ checkersByRepositoryNotes.insert(checkerUuid, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project)).containsExactly(checkerUuid);
+
+ ObjectId commitId = getRefsMetaCheckersState();
+ checkersByRepositoryNotes.insert(checkerUuid, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project)).containsExactly(checkerUuid);
+ assertThat(getRefsMetaCheckersState()).isEqualTo(commitId);
+ }
+
+ @Test
+ public void removeCheckers() throws Exception {
+ CheckersByRepositoryNotes checkersByRepositoryNotes = loadCheckersByRepositoryNotes();
+
+ Project.NameKey project = new Project.NameKey("some-project");
+
+ CheckerUuid checkerUuid1 = CheckerUuid.parse("bar:baz");
+ CheckerUuid checkerUuid2 = CheckerUuid.parse("foo:bar");
+ CheckerUuid checkerUuid3 = CheckerUuid.parse("foo:baz");
+
+ checkersByRepositoryNotes.insert(checkerUuid1, project);
+ checkersByRepositoryNotes.insert(checkerUuid2, project);
+ checkersByRepositoryNotes.insert(checkerUuid3, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project))
+ .containsExactly(checkerUuid1, checkerUuid2, checkerUuid3)
+ .inOrder();
+
+ checkersByRepositoryNotes.remove(checkerUuid2, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project))
+ .containsExactly(checkerUuid1, checkerUuid3)
+ .inOrder();
+
+ checkersByRepositoryNotes.remove(checkerUuid1, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project)).containsExactly(checkerUuid3).inOrder();
+
+ checkersByRepositoryNotes.remove(checkerUuid3, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project)).isEmpty();
+ }
+
+ @Test
+ public void removeNonExistingChecker() throws Exception {
+ CheckersByRepositoryNotes checkersByRepositoryNotes = loadCheckersByRepositoryNotes();
+
+ Project.NameKey project1 = new Project.NameKey("some-project");
+ Project.NameKey project2 = new Project.NameKey("other-project");
+
+ CheckerUuid checkerUuid1 = CheckerUuid.parse("foo:bar");
+ CheckerUuid checkerUuid2 = CheckerUuid.parse("foo:baz");
+
+ checkersByRepositoryNotes.insert(checkerUuid1, project1);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project1)).containsExactly(checkerUuid1);
+ assertThat(checkersByRepositoryNotes.get(project2)).isEmpty();
+
+ ObjectId commitId = getRefsMetaCheckersState();
+ checkersByRepositoryNotes.remove(checkerUuid2, project1);
+ checkersByRepositoryNotes.remove(checkerUuid1, project2);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project1)).containsExactly(checkerUuid1);
+ assertThat(checkersByRepositoryNotes.get(project2)).isEmpty();
+ assertThat(getRefsMetaCheckersState()).isEqualTo(commitId);
+ }
+
+ @Test
+ public void updateCheckers() throws Exception {
+ CheckersByRepositoryNotes checkersByRepositoryNotes = loadCheckersByRepositoryNotes();
+
+ Project.NameKey project1 = new Project.NameKey("some-project");
+ Project.NameKey project2 = new Project.NameKey("other-project");
+
+ CheckerUuid checkerUuid1 = CheckerUuid.parse("foo:bar");
+ CheckerUuid checkerUuid2 = CheckerUuid.parse("foo:baz");
+
+ checkersByRepositoryNotes.insert(checkerUuid1, project1);
+ checkersByRepositoryNotes.insert(checkerUuid2, project1);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project1))
+ .containsExactly(checkerUuid1, checkerUuid2)
+ .inOrder();
+ assertThat(checkersByRepositoryNotes.get(project2)).isEmpty();
+
+ checkersByRepositoryNotes.update(checkerUuid1, project1, project2);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project1)).containsExactly(checkerUuid2);
+ assertThat(checkersByRepositoryNotes.get(project2)).containsExactly(checkerUuid1);
+
+ checkersByRepositoryNotes.update(checkerUuid2, project1, project2);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project1)).isEmpty();
+ assertThat(checkersByRepositoryNotes.get(project2))
+ .containsExactly(checkerUuid1, checkerUuid2)
+ .inOrder();
+ }
+
+ @Test
+ public void sortOrder() throws Exception {
+ CheckersByRepositoryNotes checkersByRepositoryNotes = loadCheckersByRepositoryNotes();
+
+ Project.NameKey project = new Project.NameKey("some-project");
+
+ CheckerUuid checkerUuid1 = CheckerUuid.parse("foo:bar");
+ CheckerUuid checkerUuid2 = CheckerUuid.parse("foo:baz");
+ checkersByRepositoryNotes.insert(checkerUuid1, project);
+ checkersByRepositoryNotes.insert(checkerUuid2, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project))
+ .containsExactly(checkerUuid1, checkerUuid2)
+ .inOrder();
+
+ CheckerUuid checkerUuid3 = CheckerUuid.parse("abc:xyz");
+ CheckerUuid checkerUuid4 = CheckerUuid.parse("xyz:abc");
+
+ checkersByRepositoryNotes.insert(checkerUuid3, project);
+ checkersByRepositoryNotes.insert(checkerUuid4, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project))
+ .containsExactly(checkerUuid3, checkerUuid1, checkerUuid2, checkerUuid4)
+ .inOrder();
+ }
+
+ @Test
+ public void commitMessage() throws Exception {
+ CheckersByRepositoryNotes checkersByRepositoryNotes = loadCheckersByRepositoryNotes();
+
+ CheckerUuid checkerUuid = CheckerUuid.parse("foo:bar");
+ Project.NameKey project = new Project.NameKey("some-project");
+
+ checkersByRepositoryNotes.insert(checkerUuid, project);
+ commit(checkersByRepositoryNotes);
+ assertThatCommitMessage()
+ .isEqualTo(
+ "Update checkers by repository\n\nChecker: "
+ + checkerUuid.toString()
+ + "\nRepository: "
+ + project.get());
+ }
+
+ @Test
+ public void noOpUpdate() throws Exception {
+ CheckersByRepositoryNotes checkersByRepositoryNotes = loadCheckersByRepositoryNotes();
+
+ Project.NameKey project = new Project.NameKey("some-project");
+
+ CheckerUuid checkerUuid1 = CheckerUuid.parse("foo:bar");
+ checkersByRepositoryNotes.insert(checkerUuid1, project);
+ commit(checkersByRepositoryNotes);
+
+ ObjectId commitId = getRefsMetaCheckersState();
+
+ // Set the repository of the checker to the same value that it currently has.
+ checkersByRepositoryNotes.update(checkerUuid1, project, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(getRefsMetaCheckersState()).isEqualTo(commitId);
+
+ // Insert a new checker and remove it before commit.
+ CheckerUuid checkerUuid2 = CheckerUuid.parse("foo:baz");
+ checkersByRepositoryNotes.insert(checkerUuid2, project);
+ checkersByRepositoryNotes.remove(checkerUuid2, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(getRefsMetaCheckersState()).isEqualTo(commitId);
+ }
+
+ @Test
+ public void getCheckersFromOldRevision() throws Exception {
+ CheckersByRepositoryNotes checkersByRepositoryNotes = loadCheckersByRepositoryNotes();
+
+ Project.NameKey project = new Project.NameKey("some-project");
+
+ CheckerUuid checkerUuid1 = CheckerUuid.parse("foo:bar");
+ checkersByRepositoryNotes.insert(checkerUuid1, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project)).containsExactly(checkerUuid1);
+
+ ObjectId oldRevision = getRefsMetaCheckersState();
+
+ CheckerUuid checkerUuid2 = CheckerUuid.parse("foo:baz");
+ checkersByRepositoryNotes.insert(checkerUuid2, project);
+ commit(checkersByRepositoryNotes);
+ assertThat(checkersByRepositoryNotes.get(project))
+ .containsExactly(checkerUuid1, checkerUuid2)
+ .inOrder();
+
+ assertThat(CheckersByRepositoryNotes.load(projectName, repository, oldRevision).get(project))
+ .containsExactly(checkerUuid1);
+ }
+
+ @Test
+ public void getCheckersFromZeroIdRevision() throws Exception {
+ assertThat(
+ CheckersByRepositoryNotes.load(projectName, repository, ObjectId.zeroId())
+ .get(new Project.NameKey("some-project")))
+ .isEmpty();
+ }
+
+ @Test
+ public void getCheckersFromNullRevision() throws Exception {
+ assertThat(
+ CheckersByRepositoryNotes.load(projectName, repository, null)
+ .get(new Project.NameKey("some-project")))
+ .isEmpty();
+ }
+
+ private CheckersByRepositoryNotes loadCheckersByRepositoryNotes() throws IOException {
+ return CheckersByRepositoryNotes.load(projectName, repository);
+ }
+
+ private void commit(CheckersByRepositoryNotes checkersByRepositoryNotes) throws IOException {
+ try (MetaDataUpdate metaDataUpdate = createMetaDataUpdate()) {
+ checkersByRepositoryNotes.commit(metaDataUpdate);
+ }
+ }
+
+ private MetaDataUpdate createMetaDataUpdate() {
+ PersonIdent serverIdent =
+ new PersonIdent(
+ "Gerrit Server", "noreply@gerritcodereview.com", TimeUtil.nowTs(), timeZone);
+
+ MetaDataUpdate metaDataUpdate =
+ new MetaDataUpdate(
+ GitReferenceUpdated.DISABLED, new Project.NameKey("Test Repository"), repository);
+ metaDataUpdate.getCommitBuilder().setCommitter(serverIdent);
+ metaDataUpdate.getCommitBuilder().setAuthor(serverIdent);
+ return metaDataUpdate;
+ }
+
+ private ObjectId getRefsMetaCheckersState() throws IOException {
+ return repository.exactRef(CheckerRef.REFS_META_CHECKERS).getObjectId();
+ }
+
+ private StringSubject assertThatCommitMessage() throws IOException {
+ try (RevWalk rw = new RevWalk(repository)) {
+ RevCommit commit = rw.parseCommit(getRefsMetaCheckersState());
+ return assertThat(commit.getFullMessage()).named("commit message");
+ }
+ }
+}
diff --git a/src/main/resources/Documentation/rest-api-checkers.md b/src/main/resources/Documentation/rest-api-checkers.md
index dfa4240..40e1cf3 100644
--- a/src/main/resources/Documentation/rest-api-checkers.md
+++ b/src/main/resources/Documentation/rest-api-checkers.md
@@ -39,7 +39,7 @@
"repository": "examples/Foo",
"blocking": [],
"description": "A simple checker.",
- "created_on": "2019-01-31 09:59:32.126000000"
+ "created": "2019-01-31 09:59:32.126000000"
}
```
@@ -83,8 +83,8 @@
"name": "MyChecker",
"description": "A simple checker.",
"repository": "examples/Foo",
- "created_on": "2019-01-31 09:59:32.126000000",
- "updated_on": "2019-01-31 09:59:32.126000000"
+ "created": "2019-01-31 09:59:32.126000000",
+ "updated": "2019-01-31 09:59:32.126000000"
}
```
@@ -144,8 +144,8 @@
"description": "A simple checker.",
"repository": "examples/Foo",
"status": "ENABLED",
- "created_on": "2019-01-31 09:59:32.126000000",
- "updated_on": "2019-02-01 07:23:44.158000000"
+ "created": "2019-01-31 09:59:32.126000000",
+ "updated": "2019-02-01 07:23:44.158000000"
}
```
@@ -187,8 +187,8 @@
| `status` | | The status of the checker; one of `ENABLED` or `DISABLED`.
| `blocking` | | 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.
-| `created_on` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the checker was created.
-| `updated_on` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the checker was last updated.
+| `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.
@@ -233,8 +233,8 @@
the query for each checker in sequence. Because of this algorithm, there are
restrictions on which operators can be supported in checker queries:
-* Operators related to projects are not supported, since the algorithm already
- only considers checkers with a matching repository.
+* Operators related to projects/repositories are not supported, since the
+ algorithm already only considers checkers with a matching repository.
* Full-text operators (e.g. `message`) are not supported, since these cannot be
efficiently evaluated sequentially.
* Operators with an explicit or implied `self` user are not supported, since the
diff --git a/src/main/resources/Documentation/rest-api-checks.md b/src/main/resources/Documentation/rest-api-checks.md
index 3aeabef..c785e75 100644
--- a/src/main/resources/Documentation/rest-api-checks.md
+++ b/src/main/resources/Documentation/rest-api-checks.md
@@ -34,7 +34,7 @@
)]}'
[
{
- "project": "test-project",
+ "repository": "test-repo",
"change_number": 1,
"patch_set_id": 1,
"checker_uuid": "test:my-checker",
@@ -44,7 +44,7 @@
"updated": "2019-01-31 09:59:32.126000000"
},
{
- "project": "test-project",
+ "repository": "test-repo",
"change_number": 1,
"patch_set_id": 1,
"checker_uuid": "foo:foo-checker",
@@ -82,7 +82,7 @@
Content-Type: application/json; charset=UTF-8
)]}'
{
- "project": "test-project",
+ "repository": "test-repo",
"change_number": 1,
"patch_set_id": 1,
"checker_uuid": "test:my-checker",
@@ -129,7 +129,7 @@
Content-Type: application/json; charset=UTF-8
)]}'
{
- "project": "test-project",
+ "repository": "test-repo",
"change_number": 1,
"patch_set_id": 1,
"checker_uuid": "test:my-checker",
@@ -163,7 +163,7 @@
| Field Name | | Description |
| ----------------- | -------- | ----------- |
-| `project` | | The project name that this check applies to.
+| `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.
diff --git a/src/main/resources/Documentation/rest-api-pending-checks.md b/src/main/resources/Documentation/rest-api-pending-checks.md
index f972cdc..37d7bc5 100644
--- a/src/main/resources/Documentation/rest-api-pending-checks.md
+++ b/src/main/resources/Documentation/rest-api-pending-checks.md
@@ -54,7 +54,7 @@
[
{
"patch_set": {
- "project": "test-project",
+ "repository": "test-repo",
"change_number": 1,
"patch_set_id": 1,
}
@@ -66,7 +66,7 @@
},
{
"patch_set": {
- "project": "test-project",
+ "repository": "test-repo",
"change_number": 5,
"patch_set_id": 2,
}
@@ -87,7 +87,7 @@
| Field Name | Description |
| --------------- | ----------- |
-| `project` | The project name that this pending check applies to.
+| `repository` | The repository name that this pending check applies to.
| `change_number` | The change number that this pending check applies to.
| `patch_set_id` | The ID of the patch set that this pending check applies to.
