Merge "Add re run support for checks in the table"
diff --git a/java/com/google/gerrit/plugins/checks/CheckJson.java b/java/com/google/gerrit/plugins/checks/CheckJson.java
index 074d4cc..535fbc4 100644
--- a/java/com/google/gerrit/plugins/checks/CheckJson.java
+++ b/java/com/google/gerrit/plugins/checks/CheckJson.java
@@ -86,7 +86,7 @@
.getChecker(checkerUuid)
.ifPresent(
checker -> {
- info.checkerName = checker.getName().orElse(null);
+ info.checkerName = checker.getName();
info.checkerStatus = checker.getStatus();
info.blocking = checker.getBlockingConditions();
info.checkerDescription = checker.getDescription().orElse(null);
diff --git a/java/com/google/gerrit/plugins/checks/Checker.java b/java/com/google/gerrit/plugins/checks/Checker.java
index 075e2c9..348bd5e 100644
--- a/java/com/google/gerrit/plugins/checks/Checker.java
+++ b/java/com/google/gerrit/plugins/checks/Checker.java
@@ -39,11 +39,11 @@
/**
* Returns the display name of the checker.
*
- * <p>Checkers may not have a name, in this case {@link Optional#empty()} is returned.
+ * <p>The same name may be used by multiple checkers.
*
* @return display name of the checker
*/
- public abstract Optional<String> getName();
+ public abstract String getName();
/**
* Returns the description of the checker.
diff --git a/java/com/google/gerrit/plugins/checks/CheckerCreation.java b/java/com/google/gerrit/plugins/checks/CheckerCreation.java
index f814ce4..294f354 100644
--- a/java/com/google/gerrit/plugins/checks/CheckerCreation.java
+++ b/java/com/google/gerrit/plugins/checks/CheckerCreation.java
@@ -14,7 +14,10 @@
package com.google.gerrit.plugins.checks;
+import static com.google.common.base.Preconditions.checkState;
+
import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.reviewdb.client.Project;
@AutoValue
@@ -26,6 +29,13 @@
*/
public abstract CheckerUuid getCheckerUuid();
+ /**
+ * Defines the name the checker should have.
+ *
+ * <p>The same name may be used by multiple checkers.
+ */
+ public abstract String getName();
+
/** Defines the repository for which the checker applies. */
public abstract Project.NameKey getRepository();
@@ -37,8 +47,24 @@
public abstract static class Builder {
public abstract Builder setCheckerUuid(CheckerUuid checkerUuid);
+ public abstract Builder setName(String name);
+
public abstract Builder setRepository(Project.NameKey repository);
- public abstract CheckerCreation build();
+ abstract CheckerCreation autoBuild();
+
+ public CheckerCreation build() {
+ CheckerCreation checkerCreation = autoBuild();
+ checkState(!checkerCreation.getName().trim().isEmpty(), "checker name cannot be empty");
+ checkState(
+ !checkerCreation.getRepository().get().trim().isEmpty(),
+ "repository name cannot be empty");
+ return checkerCreation;
+ }
+
+ @VisibleForTesting
+ public CheckerCreation buildWithoutValidationForTesting() {
+ return autoBuild();
+ }
}
}
diff --git a/java/com/google/gerrit/plugins/checks/CheckerJson.java b/java/com/google/gerrit/plugins/checks/CheckerJson.java
index 701a169..55fdf91 100644
--- a/java/com/google/gerrit/plugins/checks/CheckerJson.java
+++ b/java/com/google/gerrit/plugins/checks/CheckerJson.java
@@ -23,7 +23,7 @@
public CheckerInfo format(Checker checker) {
CheckerInfo info = new CheckerInfo();
info.uuid = checker.getUuid().get();
- info.name = checker.getName().orElse(null);
+ info.name = checker.getName();
info.description = checker.getDescription().orElse(null);
info.url = checker.getUrl().orElse(null);
info.repository = checker.getRepository().get();
diff --git a/java/com/google/gerrit/plugins/checks/CheckerUpdate.java b/java/com/google/gerrit/plugins/checks/CheckerUpdate.java
index 97165f6..463cb1d 100644
--- a/java/com/google/gerrit/plugins/checks/CheckerUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/CheckerUpdate.java
@@ -14,7 +14,10 @@
package com.google.gerrit.plugins.checks;
+import static com.google.common.base.Preconditions.checkState;
+
import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSortedSet;
import com.google.gerrit.plugins.checks.api.BlockingCondition;
import com.google.gerrit.plugins.checks.api.CheckerStatus;
@@ -82,6 +85,27 @@
public abstract Builder setQuery(String query);
- public abstract CheckerUpdate build();
+ abstract CheckerUpdate autoBuild();
+
+ public CheckerUpdate build() {
+ CheckerUpdate checkerUpdate = autoBuild();
+
+ if (checkerUpdate.getName().isPresent()) {
+ checkState(!checkerUpdate.getName().get().trim().isEmpty(), "checker name cannot be empty");
+ }
+
+ if (checkerUpdate.getRepository().isPresent()) {
+ checkState(
+ !checkerUpdate.getRepository().get().get().trim().isEmpty(),
+ "repository name cannot be empty");
+ }
+
+ return checkerUpdate;
+ }
+
+ @VisibleForTesting
+ public CheckerUpdate buildWithoutValidationForTesting() {
+ return autoBuild();
+ }
}
}
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 984cd31..e96f0c2 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
@@ -115,8 +115,13 @@
checkerCreation
.uuid()
.orElseGet(() -> CheckerUuid.parse("test:checker-" + checkerCounter.incrementAndGet()));
+ String checkerName = checkerCreation.name().orElse("Test Checker");
Project.NameKey repository = checkerCreation.repository().orElse(allProjectsName);
- return CheckerCreation.builder().setCheckerUuid(checkerUuid).setRepository(repository).build();
+ return CheckerCreation.builder()
+ .setCheckerUuid(checkerUuid)
+ .setName(checkerName)
+ .setRepository(repository)
+ .build();
}
private static CheckerUpdate toCheckerUpdate(TestCheckerCreation checkerCreation) {
@@ -268,6 +273,10 @@
unsetValueInCheckerConfig("uuid");
}
+ if (testCheckerInvalidation.unsetName()) {
+ unsetValueInCheckerConfig("name");
+ }
+
if (testCheckerInvalidation.unsetRepository()) {
unsetValueInCheckerConfig("repository");
}
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java
index f845ddd..850d7c8 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java
@@ -29,6 +29,8 @@
public abstract boolean unsetUuid();
+ public abstract boolean unsetName();
+
public abstract boolean unsetRepository();
public abstract boolean unsetStatus();
@@ -45,6 +47,7 @@
.invalidBlockingCondition(false)
.invalidStatus(false)
.unsetUuid(false)
+ .unsetName(false)
.unsetRepository(false)
.unsetStatus(false)
.deleteRef(false);
@@ -80,7 +83,13 @@
return unsetUuid(true);
}
- abstract Builder unsetUuid(boolean unsetUuid);
+ abstract Builder unsetUuid(boolean unsetName);
+
+ public Builder unsetName() {
+ return unsetName(true);
+ }
+
+ abstract Builder unsetName(boolean unsetName);
public Builder unsetRepository() {
return unsetRepository(true);
diff --git a/java/com/google/gerrit/plugins/checks/api/CreateChecker.java b/java/com/google/gerrit/plugins/checks/api/CreateChecker.java
index ff533a7..8ae0513 100644
--- a/java/com/google/gerrit/plugins/checks/api/CreateChecker.java
+++ b/java/com/google/gerrit/plugins/checks/api/CreateChecker.java
@@ -99,15 +99,20 @@
CheckerUuid.tryParse(input.uuid)
.orElseThrow(() -> new BadRequestException("invalid uuid: " + uuidStr));
+ String name = CheckerName.clean(input.name);
+ if (name.isEmpty()) {
+ throw new BadRequestException("name is required");
+ }
+
Project.NameKey repository = resolveRepository(input.repository);
CheckerCreation.Builder checkerCreationBuilder =
- CheckerCreation.builder().setCheckerUuid(checkerUuid).setRepository(repository);
+ CheckerCreation.builder()
+ .setCheckerUuid(checkerUuid)
+ .setName(name)
+ .setRepository(repository);
CheckerUpdate.Builder checkerUpdateBuilder = CheckerUpdate.builder();
- String name = CheckerName.clean(input.name);
- if (!name.isEmpty()) {
- checkerUpdateBuilder.setName(name);
- }
+
if (input.description != null && !input.description.trim().isEmpty()) {
checkerUpdateBuilder.setDescription(input.description.trim());
}
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java b/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
index 3adb2dd..2594a96 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
@@ -311,6 +311,11 @@
}
private void ensureThatMandatoryPropertiesAreSet() throws ConfigInvalidException {
+ if (getNewName().equals(Optional.of(""))) {
+ throw new ConfigInvalidException(
+ String.format("Name of the checker %s must be defined", describeForError()));
+ }
+
if (getNewRepository().equals(Optional.of(""))) {
throw new ConfigInvalidException(
String.format("Repository of the checker %s must be defined", describeForError()));
@@ -321,6 +326,16 @@
checkState(isLoaded, "Checker %s not loaded yet", describeForError());
}
+ private Optional<String> getNewName() {
+ if (checkerUpdate.isPresent()) {
+ return checkerUpdate.get().getName().map(Strings::nullToEmpty).map(String::trim);
+ }
+ if (checkerCreation.isPresent()) {
+ return Optional.of(Strings.nullToEmpty(checkerCreation.get().getName()).trim());
+ }
+ return Optional.empty();
+ }
+
private Optional<String> getNewRepository() {
if (checkerUpdate.isPresent()) {
return checkerUpdate
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckerConfigEntry.java b/java/com/google/gerrit/plugins/checks/db/CheckerConfigEntry.java
index 0b2c5ab..eabcfb2 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckerConfigEntry.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckerConfigEntry.java
@@ -91,16 +91,22 @@
*/
NAME("name") {
@Override
- void readFromConfig(CheckerUuid checkerUuid, Checker.Builder checker, Config config) {
+ void readFromConfig(CheckerUuid checkerUuid, Checker.Builder checker, Config config)
+ throws ConfigInvalidException {
String name = config.getString(SECTION_NAME, null, super.keyName);
- if (name != null) {
- checker.setName(name);
+ if (name == null) {
+ throw new ConfigInvalidException(
+ String.format(
+ "%s.%s is not set in config file for checker %s",
+ SECTION_NAME, super.keyName, checkerUuid));
}
+ checker.setName(name);
}
@Override
void initNewConfig(Config config, CheckerCreation checkerCreation) {
- // Do nothing. Name key will be set by updateConfigValue.
+ String checkerName = checkerCreation.getName();
+ config.setString(SECTION_NAME, null, super.keyName, checkerName);
}
@Override
diff --git a/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java b/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
index 8e5bb2c..2b4e356 100644
--- a/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
+++ b/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
@@ -52,10 +52,8 @@
check("uuid()").that(checker().getUuid()).isEqualTo(expectedUuid);
}
- public OptionalSubject<StringSubject, String> hasNameThat() {
- return check("name()")
- .about(optionals())
- .that(checker().getName(), StandardSubjectBuilder::that);
+ public void hasName(String expectedName) {
+ check("name()").that(checker().getName()).isEqualTo(expectedName);
}
public OptionalSubject<StringSubject, String> hasDescriptionThat() {
diff --git a/javatests/com/google/gerrit/plugins/checks/CheckerCreationTest.java b/javatests/com/google/gerrit/plugins/checks/CheckerCreationTest.java
new file mode 100644
index 0000000..36968d3
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/checks/CheckerCreationTest.java
@@ -0,0 +1,83 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.checks;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.reviewdb.client.Project;
+import org.junit.Test;
+
+public class CheckerCreationTest {
+ private final CheckerUuid checkerUuid = CheckerUuid.parse("test:my-checker");
+ private final String checkerName = "My Checker";
+ private final Project.NameKey checkerRepository = Project.nameKey("my-repo");
+
+ @Test
+ public void nameCannotBeEmpty() {
+ IllegalStateException thrown =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ CheckerCreation.builder()
+ .setCheckerUuid(checkerUuid)
+ .setName("")
+ .setRepository(checkerRepository)
+ .build());
+ assertThat(thrown).hasMessageThat().contains(String.format("checker name cannot be empty"));
+ }
+
+ @Test
+ public void nameCannotBeEmptyAfterTrim() {
+ IllegalStateException thrown =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ CheckerCreation.builder()
+ .setCheckerUuid(checkerUuid)
+ .setName(" ")
+ .setRepository(checkerRepository)
+ .build());
+ assertThat(thrown).hasMessageThat().contains(String.format("checker name cannot be empty"));
+ }
+
+ @Test
+ public void repositoryCannotBeEmpty() {
+ IllegalStateException thrown =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ CheckerCreation.builder()
+ .setCheckerUuid(checkerUuid)
+ .setName(checkerName)
+ .setRepository(Project.nameKey(""))
+ .build());
+ assertThat(thrown).hasMessageThat().contains(String.format("repository name cannot be empty"));
+ }
+
+ @Test
+ public void repositoryCannotBeEmptyAfterTrim() {
+ IllegalStateException thrown =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ CheckerCreation.builder()
+ .setCheckerUuid(checkerUuid)
+ .setName(checkerName)
+ .setRepository(Project.nameKey(" "))
+ .build());
+ assertThat(thrown).hasMessageThat().contains(String.format("repository name cannot be empty"));
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/checks/CheckerDefinitionTest.java b/javatests/com/google/gerrit/plugins/checks/CheckerDefinitionTest.java
index 9d090ce..f0e967a 100644
--- a/javatests/com/google/gerrit/plugins/checks/CheckerDefinitionTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/CheckerDefinitionTest.java
@@ -50,6 +50,7 @@
private Checker.Builder newChecker() {
return Checker.builder()
+ .setName("My Checker")
.setRepository(Project.nameKey("test-repo"))
.setStatus(CheckerStatus.ENABLED)
.setUuid(CheckerUuid.parse("schema:any-id"))
diff --git a/javatests/com/google/gerrit/plugins/checks/CheckerUpdateTest.java b/javatests/com/google/gerrit/plugins/checks/CheckerUpdateTest.java
new file mode 100644
index 0000000..ae2ed4d
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/checks/CheckerUpdateTest.java
@@ -0,0 +1,57 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.checks;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.reviewdb.client.Project;
+import org.junit.Test;
+
+public class CheckerUpdateTest {
+ @Test
+ public void nameCannotBeEmpty() {
+ IllegalStateException thrown =
+ assertThrows(
+ IllegalStateException.class, () -> CheckerUpdate.builder().setName("").build());
+ assertThat(thrown).hasMessageThat().contains(String.format("checker name cannot be empty"));
+ }
+
+ @Test
+ public void nameCannotBeEmptyAfterTrim() {
+ IllegalStateException thrown =
+ assertThrows(
+ IllegalStateException.class, () -> CheckerUpdate.builder().setName(" ").build());
+ assertThat(thrown).hasMessageThat().contains(String.format("checker name cannot be empty"));
+ }
+
+ @Test
+ public void repositoryCannotBeEmpty() {
+ IllegalStateException thrown =
+ assertThrows(
+ IllegalStateException.class,
+ () -> CheckerUpdate.builder().setRepository(Project.nameKey("")).build());
+ assertThat(thrown).hasMessageThat().contains(String.format("repository name cannot be empty"));
+ }
+
+ @Test
+ public void repositoryCannotBeEmptyAfterTrim() {
+ IllegalStateException thrown =
+ assertThrows(
+ IllegalStateException.class,
+ () -> CheckerUpdate.builder().setRepository(Project.nameKey(" ")).build());
+ assertThat(thrown).hasMessageThat().contains(String.format("repository name cannot be empty"));
+ }
+}
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 7514878..fa36005 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckerIT.java
@@ -78,10 +78,11 @@
Timestamp expectedCreationTimestamp = TestTimeUtil.getCurrentTimestamp();
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = repositoryName.get();
CheckerInfo info = checkersApi.create(input).get();
assertThat(info.uuid).isEqualTo("test:my-checker");
- assertThat(info.name).isNull();
+ assertThat(info.name).isEqualTo("My Checker");
assertThat(info.description).isNull();
assertThat(info.url).isNull();
assertThat(info.repository).isEqualTo(input.repository);
@@ -104,6 +105,7 @@
public void createCheckerWithDescription() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.description = "some description";
input.repository = allProjects.get();
CheckerInfo info = checkersApi.create(input).get();
@@ -118,6 +120,7 @@
public void createCheckerWithUrl() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.url = "http://example.com/my-checker";
input.repository = allProjects.get();
CheckerInfo info = checkersApi.create(input).get();
@@ -129,27 +132,24 @@
}
@Test
- public void createCheckerWithName() throws Exception {
+ public void createCheckerWithoutNameFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
- input.name = "my-checker";
input.repository = allProjects.get();
- CheckerInfo info = checkersApi.create(input).get();
- assertThat(info.name).isEqualTo("my-checker");
- PerCheckerOperations perCheckerOps = checkerOperations.checker(info.uuid);
- assertCommit(
- perCheckerOps.commit(), "Create checker", info.created, perCheckerOps.get().getRefState());
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> checkersApi.create(input));
+ assertThat(thrown).hasMessageThat().contains("name is required");
}
@Test
public void createCheckerNameIsTrimmed() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
- input.name = " my-checker ";
+ input.name = " My Checker ";
input.repository = allProjects.get();
CheckerInfo info = checkersApi.create(input).get();
- assertThat(info.name).isEqualTo("my-checker");
+ assertThat(info.name).isEqualTo("My Checker");
PerCheckerOperations perCheckerOps = checkerOperations.checker(info.uuid);
assertCommit(
@@ -160,6 +160,7 @@
public void createCheckerDescriptionIsTrimmed() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.description = " some description ";
input.repository = allProjects.get();
CheckerInfo info = checkersApi.create(input).get();
@@ -174,6 +175,7 @@
public void createCheckerUrlIsTrimmed() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.url = " http://example.com/my-checker ";
input.repository = allProjects.get();
CheckerInfo info = checkersApi.create(input).get();
@@ -188,6 +190,7 @@
public void createCheckerRepositoryIsTrimmed() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = " " + allProjects.get() + " ";
CheckerInfo info = checkersApi.create(input).get();
assertThat(info.repository).isEqualTo(allProjects.get());
@@ -212,7 +215,7 @@
public void createCheckersWithSameName() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
- input.name = "my-checker";
+ input.name = "My Checker";
input.repository = allProjects.get();
CheckerInfo info1 = checkersApi.create(input).get();
assertThat(info1.name).isEqualTo(input.name);
@@ -228,6 +231,7 @@
public void createCheckerWithExistingUuidFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = allProjects.get();
checkersApi.create(input).get();
@@ -239,6 +243,7 @@
@Test
public void createCheckerWithoutUuidFails() throws Exception {
CheckerInput input = new CheckerInput();
+ input.name = "My Checker";
input.repository = allProjects.get();
BadRequestException thrown =
@@ -250,6 +255,7 @@
public void createCheckerWithEmptyUuidFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "";
+ input.name = "My Checker";
input.repository = allProjects.get();
BadRequestException thrown =
@@ -261,6 +267,7 @@
public void createCheckerWithEmptyUuidAfterTrimFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = " ";
+ input.name = "My Checker";
input.repository = allProjects.get();
BadRequestException thrown =
@@ -272,6 +279,7 @@
public void createCheckerWithInvalidUuidFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = CheckerTestData.INVALID_UUID;
+ input.name = "My Checker";
input.repository = allProjects.get();
BadRequestException thrown =
@@ -283,6 +291,7 @@
public void createCheckerWithoutRepositoryFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
BadRequestException thrown =
assertThrows(BadRequestException.class, () -> checkersApi.create(input));
@@ -293,6 +302,7 @@
public void createCheckerWithEmptyRepositoryFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = "";
BadRequestException thrown =
@@ -304,6 +314,7 @@
public void createCheckerWithEmptyRepositoryAfterTrimFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = " ";
BadRequestException thrown =
@@ -315,6 +326,7 @@
public void createCheckerWithNonExistingRepositoryFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = "non-existing";
UnprocessableEntityException thrown =
@@ -326,6 +338,7 @@
public void createDisabledChecker() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = allProjects.get();
input.status = CheckerStatus.DISABLED;
@@ -337,6 +350,7 @@
public void createCheckerWithBlockingConditions() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = allProjects.get();
input.blocking = ImmutableSet.of(BlockingCondition.STATE_NOT_PASSING);
@@ -348,6 +362,7 @@
public void createCheckerWithQuery() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = allProjects.get();
input.query = "f:foo";
@@ -359,6 +374,7 @@
public void createCheckerWithEmptyQuery() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = allProjects.get();
input.query = "";
@@ -370,6 +386,7 @@
public void createCheckerWithEmptyQueryAfterTrim() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = allProjects.get();
input.query = " ";
@@ -381,6 +398,7 @@
public void createCheckerWithUnsupportedOperatorInQueryFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = allProjects.get();
input.query = CheckerTestData.QUERY_WITH_UNSUPPORTED_OPERATOR;
@@ -395,6 +413,7 @@
public void createCheckerWithInvalidQueryFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = allProjects.get();
input.query = CheckerTestData.INVALID_QUERY;
@@ -407,6 +426,7 @@
public void createCheckerWithTooLongQueryFails() throws Exception {
CheckerInput input = new CheckerInput();
input.uuid = "test:my-checker";
+ input.name = "My Checker";
input.repository = allProjects.get();
input.query = CheckerTestData.longQueryWithSupportedOperators(MAX_INDEX_TERMS * 2);
assertThat(CheckerQuery.clean(input.query)).isEqualTo(input.query);
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 cb3f695..3f1f132 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
@@ -255,6 +255,16 @@
}
@Test
+ public void getCheckerWithMissingNameFails() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+ checkerOperations.checker(checkerUuid).forInvalidation().unsetName().invalidate();
+
+ RestApiException thrown =
+ assertThrows(RestApiException.class, () -> getCheckerInfo(checkerUuid));
+ assertThat(thrown).hasMessageThat().contains("Cannot retrieve checker " + checkerUuid);
+ }
+
+ @Test
public void getCheckerWithMissingRepositoryFails() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
checkerOperations.checker(checkerUuid).forInvalidation().unsetRepository().invalidate();
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 590f82f..b41d9e8 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
@@ -89,6 +89,7 @@
CheckerUuid invalidCheckerUuid5 = checkerOperations.newChecker().create();
CheckerUuid invalidCheckerUuid6 = checkerOperations.newChecker().create();
CheckerUuid invalidCheckerUuid7 = checkerOperations.newChecker().create();
+ CheckerUuid invalidCheckerUuid8 = checkerOperations.newChecker().create();
checkerOperations
.checker(invalidCheckerUuid1)
.forInvalidation()
@@ -102,8 +103,9 @@
.invalidate();
checkerOperations.checker(invalidCheckerUuid4).forInvalidation().invalidStatus().invalidate();
checkerOperations.checker(invalidCheckerUuid5).forInvalidation().unsetUuid().invalidate();
- checkerOperations.checker(invalidCheckerUuid6).forInvalidation().unsetRepository().invalidate();
- checkerOperations.checker(invalidCheckerUuid7).forInvalidation().unsetStatus().invalidate();
+ checkerOperations.checker(invalidCheckerUuid6).forInvalidation().unsetName().invalidate();
+ checkerOperations.checker(invalidCheckerUuid7).forInvalidation().unsetRepository().invalidate();
+ checkerOperations.checker(invalidCheckerUuid8).forInvalidation().unsetStatus().invalidate();
List<CheckerInfo> allCheckers = checkersApi.all();
assertThat(allCheckers).containsExactly(checkerOperations.checker(checkerUuid).asInfo());
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 9650784..816ff5c 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
@@ -77,7 +77,10 @@
String checkerName1 = "Checker One";
CheckerUuid checkerUuid1 =
checkerOperations.newChecker().name(checkerName1).repository(project).create();
- CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
+
+ String checkerName2 = "Checker Two";
+ CheckerUuid checkerUuid2 =
+ checkerOperations.newChecker().name(checkerName2).repository(project).create();
CheckKey checkKey1 = CheckKey.create(project, patchSetId, checkerUuid1);
checkOperations.newCheck(checkKey1).state(CheckState.RUNNING).upsert();
@@ -93,6 +96,7 @@
CheckInfo expectedCheckInfo2 = checkOperations.check(checkKey2).asInfo();
expectedCheckInfo2.repository = project.get();
+ expectedCheckInfo2.checkerName = checkerName2;
expectedCheckInfo2.blocking = ImmutableSet.of();
expectedCheckInfo2.checkerStatus = CheckerStatus.ENABLED;
@@ -105,7 +109,10 @@
String checkerName1 = "Checker One";
CheckerUuid checkerUuid1 =
checkerOperations.newChecker().name(checkerName1).repository(project).create();
- CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
+
+ String checkerName2 = "Checker Two";
+ CheckerUuid checkerUuid2 =
+ checkerOperations.newChecker().name(checkerName2).repository(project).create();
CheckKey checkKey1 = CheckKey.create(project, patchSetId, checkerUuid1);
checkOperations.newCheck(checkKey1).state(CheckState.RUNNING).upsert();
@@ -121,6 +128,7 @@
CheckInfo expectedCheckInfo2 = checkOperations.check(checkKey2).asInfo();
expectedCheckInfo2.repository = project.get();
+ expectedCheckInfo2.checkerName = checkerName2;
expectedCheckInfo2.blocking = ImmutableSet.of();
expectedCheckInfo2.checkerStatus = CheckerStatus.ENABLED;
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 f78fee2..8a2d2d4 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
@@ -70,7 +70,7 @@
CheckerInfo foundChecker = getCheckerFromServer(checkerUuid);
assertThat(foundChecker.uuid).isEqualTo(checkerUuid.get());
- assertThat(foundChecker.name).isNull();
+ assertThat(foundChecker.name).isNotNull();
assertThat(foundChecker.repository).isEqualTo(allProjects.get());
assertThat(foundChecker.status).isEqualTo(CheckerStatus.ENABLED);
assertThat(foundChecker.query).isEqualTo("status:open");
@@ -257,9 +257,9 @@
CheckerUuid checkerUuid =
checkerOperations.newChecker().name("ABC-789-this-name-must-be-unique").create();
- Optional<String> checkerName = checkerOperations.checker(checkerUuid).get().getName();
+ String checkerName = checkerOperations.checker(checkerUuid).get().getName();
- assertThat(checkerName).hasValue("ABC-789-this-name-must-be-unique");
+ assertThat(checkerName).isEqualTo("ABC-789-this-name-must-be-unique");
}
@Test
@@ -416,8 +416,8 @@
checkerOperations.checker(checkerUuid).forUpdate().name("updated name").update();
- Optional<String> currentName = checkerOperations.checker(checkerUuid).get().getName();
- assertThat(currentName).hasValue("updated name");
+ String currentName = checkerOperations.checker(checkerUuid).get().getName();
+ assertThat(currentName).isEqualTo("updated name");
}
@Test
@@ -591,6 +591,17 @@
}
@Test
+ public void nameCanBeUnset() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+
+ checkerOperations.checker(checkerUuid).forInvalidation().unsetName().invalidate();
+
+ ConfigInvalidException thrown =
+ assertThrows(ConfigInvalidException.class, () -> checkers.getChecker(checkerUuid));
+ assertThat(thrown).hasMessageThat().contains("checker.name is not set in config file");
+ }
+
+ @Test
public void repositoryCanBeUnset() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
@@ -670,7 +681,7 @@
Checker checker = checkerOperations.checker(checkerUuid).get();
CheckerInfo checkerInfo = checkerOperations.checker(checkerUuid).asInfo();
assertThat(checkerInfo.uuid).isEqualTo(checker.getUuid().get());
- assertThat(checkerInfo.name).isEqualTo(checker.getName().get());
+ assertThat(checkerInfo.name).isEqualTo(checker.getName());
assertThat(checkerInfo.description).isEqualTo(checker.getDescription().get());
assertThat(checkerInfo.url).isEqualTo(checker.getUrl().get());
assertThat(checkerInfo.created).isEqualTo(checker.getCreated());
@@ -734,6 +745,7 @@
private CheckerInput createArbitraryCheckerInput() {
CheckerInput checkerInput = new CheckerInput();
checkerInput.uuid = "test:my-checker";
+ checkerInput.name = "My Checker";
checkerInput.repository = allProjects.get();
return checkerInput;
}
diff --git a/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java b/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java
index 15b85fe..a4d02b1 100644
--- a/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java
@@ -52,6 +52,7 @@
private TestRepository<?> testRepository;
private final CheckerUuid checkerUuid = CheckerUuid.parse("test:my-checker");
+ private final String checkerName = "My Checker";
private final Project.NameKey checkerRepository = Project.nameKey("my-repo");
private final TimeZone timeZone = TimeZone.getTimeZone("America/Los_Angeles");
@@ -82,23 +83,52 @@
}
@Test
- public void setNameDuringCreation() throws Exception {
- String anotherName = "another-name";
+ public void specifiedCheckerNameIsRespectedForNewChecker() throws Exception {
+ CheckerCreation checkerCreation =
+ getPrefilledCheckerCreationBuilder().setName(checkerName).build();
+ createChecker(checkerCreation);
- CheckerCreation checkerCreation = getPrefilledCheckerCreationBuilder().build();
+ CheckerConfig checkerConfig = loadChecker(checkerUuid);
+ assertThat(checkerConfig).hasName(checkerName);
+ assertThat(checkerConfig).configStringList("name").containsExactly("My Checker");
+ }
+
+ @Test
+ public void nameOfCheckerUpdateOverridesCheckerCreation() throws Exception {
+ String anotherName = "Another Name";
+
+ CheckerCreation checkerCreation =
+ getPrefilledCheckerCreationBuilder().setName(checkerName).build();
CheckerUpdate checkerUpdate = CheckerUpdate.builder().setName(anotherName).build();
createChecker(checkerCreation, checkerUpdate);
CheckerConfig checkerConfig = loadChecker(checkerCreation.getCheckerUuid());
- assertThat(checkerConfig).hasNameThat().value().isEqualTo(anotherName);
+ assertThat(checkerConfig).hasName(anotherName);
assertThat(checkerConfig).configStringList("name").containsExactly(anotherName);
}
@Test
+ public void nameOfNewCheckerMustNotBeEmpty() throws Exception {
+ CheckerCreation checkerCreation =
+ getPrefilledCheckerCreationBuilder().setName("").buildWithoutValidationForTesting();
+ CheckerConfig checkerConfig =
+ CheckerConfig.createForNewChecker(projectName, repository, checkerCreation);
+
+ try (MetaDataUpdate metaDataUpdate = createMetaDataUpdate()) {
+ Throwable thrown = assertThrows(Throwable.class, () -> checkerConfig.commit(metaDataUpdate));
+ assertThat(thrown).hasCauseThat().isInstanceOf(ConfigInvalidException.class);
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains(String.format("Name of the checker %s must be defined", checkerUuid));
+ }
+ }
+
+ @Test
public void descriptionDefaultsToOptionalEmpty() throws Exception {
CheckerCreation checkerCreation =
CheckerCreation.builder()
.setCheckerUuid(checkerUuid)
+ .setName(checkerName)
.setRepository(checkerRepository)
.build();
createChecker(checkerCreation);
@@ -137,6 +167,7 @@
CheckerCreation checkerCreation =
CheckerCreation.builder()
.setCheckerUuid(checkerUuid)
+ .setName(checkerName)
.setRepository(checkerRepository)
.build();
createChecker(checkerCreation);
@@ -202,7 +233,9 @@
@Test
public void repositoryOfNewCheckerMustNotBeEmpty() throws Exception {
CheckerCreation checkerCreation =
- getPrefilledCheckerCreationBuilder().setRepository(Project.nameKey("")).build();
+ getPrefilledCheckerCreationBuilder()
+ .setRepository(Project.nameKey(""))
+ .buildWithoutValidationForTesting();
CheckerConfig checkerConfig =
CheckerConfig.createForNewChecker(projectName, repository, checkerCreation);
@@ -237,6 +270,23 @@
}
@Test
+ public void nameInConfigMayNotBeUndefined() throws Exception {
+ populateCheckerConfig(
+ checkerUuid,
+ "[checker]\n uuid = "
+ + checkerUuid
+ + "\n repository = "
+ + checkerRepository
+ + "\n status = ENABLED");
+
+ ConfigInvalidException thrown =
+ assertThrows(ConfigInvalidException.class, () -> loadChecker(checkerUuid));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("checker.name is not set in config file for checker " + checkerUuid);
+ }
+
+ @Test
public void correctCommitMessageForCheckerUpdate() throws Exception {
createArbitraryChecker(checkerUuid);
assertThatCommitMessage(checkerUuid).isEqualTo("Create checker");
@@ -247,31 +297,39 @@
}
@Test
- public void nameCanBeUpdatedAndRemoved() throws Exception {
+ public void nameCanBeUpdated() throws Exception {
CheckerCreation checkerCreation =
CheckerCreation.builder()
.setCheckerUuid(checkerUuid)
+ .setName(checkerName)
.setRepository(checkerRepository)
.build();
createChecker(checkerCreation);
CheckerConfig checkerConfig = loadChecker(checkerUuid);
- assertThat(checkerConfig).hasNameThat().isAbsent();
+ assertThat(checkerConfig).hasName(checkerName);
- String newName = "new-name";
+ String newName = "New Name";
CheckerUpdate checkerUpdate = CheckerUpdate.builder().setName(newName).build();
updateChecker(checkerUuid, checkerUpdate);
checkerConfig = loadChecker(checkerUuid);
- assertThat(checkerConfig).hasNameThat().value().isEqualTo(newName);
+ assertThat(checkerConfig).hasName(newName);
assertThat(checkerConfig).configStringList("name").containsExactly(newName);
+ }
- checkerUpdate = CheckerUpdate.builder().setName("").build();
- updateChecker(checkerUuid, checkerUpdate);
+ @Test
+ public void nameCannotBeRemoved() throws Exception {
+ createArbitraryChecker(checkerUuid);
- checkerConfig = loadChecker(checkerUuid);
- assertThat(checkerConfig).hasNameThat().isAbsent();
- assertThat(checkerConfig).configStringList("name").isEmpty();
+ CheckerUpdate checkerUpdate =
+ CheckerUpdate.builder().setName("").buildWithoutValidationForTesting();
+
+ IOException thrown =
+ assertThrows(IOException.class, () -> updateChecker(checkerUuid, checkerUpdate));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains(String.format("Name of the checker %s must be defined", checkerUuid));
}
@Test
@@ -325,6 +383,7 @@
CheckerCreation checkerCreation =
CheckerCreation.builder()
.setCheckerUuid(checkerUuid)
+ .setName(checkerName)
.setRepository(checkerRepository)
.build();
createChecker(checkerCreation);
@@ -345,7 +404,9 @@
createArbitraryChecker(checkerUuid);
CheckerUpdate checkerUpdate =
- CheckerUpdate.builder().setRepository(Project.nameKey("")).build();
+ CheckerUpdate.builder()
+ .setRepository(Project.nameKey(""))
+ .buildWithoutValidationForTesting();
IOException thrown =
assertThrows(IOException.class, () -> updateChecker(checkerUuid, checkerUpdate));
@@ -514,7 +575,10 @@
}
private CheckerCreation.Builder getPrefilledCheckerCreationBuilder() {
- return CheckerCreation.builder().setCheckerUuid(checkerUuid).setRepository(checkerRepository);
+ return CheckerCreation.builder()
+ .setCheckerUuid(checkerUuid)
+ .setName(checkerName)
+ .setRepository(checkerRepository);
}
private CheckerConfig createChecker(CheckerCreation checkerCreation) throws Exception {
