Fail with ConfigInvalidException if checker config contains invalid status
If in a checker config file the status is set to a value that cannot be
resolved to a CheckerStatus enum we failed with an
IllegalArgumentException, but since this is an invalid config it's
better to throw ConfigInvalidException.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ibb8c6ac666ff58196663003e1fe037bdd64a664b
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 34c5ad3..e3ff24e 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
@@ -268,6 +268,20 @@
}
}
+ if (testCheckerUpdate.forceInvalidStatus()) {
+ try (Repository repo = repoManager.openRepository(allProjectsName)) {
+ TestRepository<Repository> testRepo = new TestRepository<>(repo);
+ Config checkerConfig =
+ readConfig(testRepo, checkerUuid.toRefName(), CheckerConfig.CHECKER_CONFIG_FILE);
+ checkerConfig.setString("checker", null, "status", "invalid");
+ testRepo
+ .branch(checkerUuid.toRefName())
+ .commit()
+ .add(CheckerConfig.CHECKER_CONFIG_FILE, checkerConfig.toText())
+ .create();
+ }
+ }
+
if (testCheckerUpdate.deleteRef()) {
try (Repository repo = repoManager.openRepository(allProjectsName)) {
RefUpdate ru =
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerUpdate.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerUpdate.java
index 76d41c0..99f6f92 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerUpdate.java
@@ -47,6 +47,8 @@
public abstract boolean forceInvalidBlockingCondition();
+ public abstract boolean forceInvalidStatus();
+
public abstract boolean deleteRef();
abstract ThrowingConsumer<TestCheckerUpdate> checkerUpdater();
@@ -56,6 +58,7 @@
.checkerUpdater(checkerUpdater)
.forceInvalidConfig(false)
.forceInvalidBlockingCondition(false)
+ .forceInvalidStatus(false)
.deleteRef(false);
}
@@ -100,6 +103,12 @@
abstract Builder forceInvalidBlockingCondition(boolean forceInvalidBlockingCondition);
+ public Builder forceInvalidStatus() {
+ return forceInvalidStatus(true);
+ }
+
+ abstract Builder forceInvalidStatus(boolean forceInvalidStatus);
+
public Builder deleteRef() {
return deleteRef(true);
}
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckerConfigEntry.java b/java/com/google/gerrit/plugins/checks/db/CheckerConfigEntry.java
index d1d49ef..3734bf2 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckerConfigEntry.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckerConfigEntry.java
@@ -229,7 +229,11 @@
throw new ConfigInvalidException(
String.format("status of checker %s not set", checkerUuid));
}
- checker.setStatus(config.getEnum(SECTION_NAME, null, super.keyName, CheckerStatus.ENABLED));
+ try {
+ checker.setStatus(config.getEnum(SECTION_NAME, null, super.keyName, CheckerStatus.ENABLED));
+ } catch (IllegalArgumentException e) {
+ throw new ConfigInvalidException(e.getMessage());
+ }
}
@Override
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 2cb2ec8..0d7baf9 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
@@ -222,6 +222,16 @@
}
@Test
+ public void getCheckerWithInvalidStatusFails() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+ checkerOperations.checker(checkerUuid).forUpdate().forceInvalidStatus().update();
+
+ exception.expect(RestApiException.class);
+ exception.expectMessage("Cannot retrieve checker " + checkerUuid);
+ getCheckerInfo(checkerUuid);
+ }
+
+ @Test
public void getCheckerByNameFails() throws Exception {
String name = "my-checker";
checkerOperations.newChecker().name(name).create();
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
index ffe26fc..f52257e 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
@@ -90,12 +90,14 @@
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
CheckerUuid invalidCheckerUuid1 = checkerOperations.newChecker().create();
CheckerUuid invalidCheckerUuid2 = checkerOperations.newChecker().create();
+ CheckerUuid invalidCheckerUuid3 = checkerOperations.newChecker().create();
checkerOperations.checker(invalidCheckerUuid1).forUpdate().forceInvalidConfig().update();
checkerOperations
.checker(invalidCheckerUuid2)
.forUpdate()
.forceInvalidBlockingCondition()
.update();
+ checkerOperations.checker(invalidCheckerUuid3).forUpdate().forceInvalidStatus().update();
List<CheckerInfo> allCheckers = checkersApi.all();
assertThat(allCheckers).containsExactly(checkerOperations.checker(checkerUuid).asInfo());
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 b282e94..e256e35 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
@@ -528,6 +528,21 @@
}
@Test
+ public void statusCanBeMadeInvalid() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+
+ checkerOperations.checker(checkerUuid).forUpdate().forceInvalidStatus().update();
+
+ try {
+ checkers.getChecker(checkerUuid);
+ assert_().fail("expected ConfigInvalidException");
+ } catch (ConfigInvalidException e) {
+ // expected
+ assertThat(e.getMessage()).contains("Invalid value: checker.status");
+ }
+ }
+
+ @Test
public void refCanBeDeleted() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
