Support unsetting check URLs
Setting a check URL to an empty string should unset the check URL.
That's what we do for checker URLs too.
Also make the population of CheckUpdate in PostCheck consistent with how
CheckerUpdate is populated in UpdateChecker. This makes a bunch of
methods in CheckUpdate unneeded.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I17ab403ce55e146f66af1f89f14fd100c9c78c0e
diff --git a/java/com/google/gerrit/plugins/checks/CheckUpdate.java b/java/com/google/gerrit/plugins/checks/CheckUpdate.java
index 5813862..fd10148 100644
--- a/java/com/google/gerrit/plugins/checks/CheckUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/CheckUpdate.java
@@ -39,20 +39,12 @@
public abstract static class Builder {
public abstract Builder setState(CheckState state);
- public abstract Builder setState(Optional<CheckState> state);
-
public abstract Builder setUrl(String url);
- public abstract Builder setUrl(Optional<String> url);
-
public abstract Builder setStarted(Timestamp started);
- public abstract Builder setStarted(Optional<Timestamp> started);
-
public abstract Builder setFinished(Timestamp finished);
- public abstract Builder setFinished(Optional<Timestamp> finished);
-
public abstract CheckUpdate build();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java
index 3b0af57..3362cfb 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java
@@ -49,6 +49,10 @@
public abstract Builder setUrl(String url);
+ public Builder clearUrl() {
+ return setUrl("");
+ }
+
public abstract Builder setStarted(Timestamp started);
public abstract Builder setFinished(Timestamp finished);
diff --git a/java/com/google/gerrit/plugins/checks/api/PostCheck.java b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
index aba0995..ad011d7 100644
--- a/java/com/google/gerrit/plugins/checks/api/PostCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
@@ -111,11 +111,24 @@
}
private static CheckUpdate toCheckUpdate(CheckInput input) throws BadRequestException {
- return CheckUpdate.builder()
- .setState(Optional.ofNullable(input.state))
- .setUrl(input.url == null ? Optional.empty() : Optional.of(UrlValidator.clean(input.url)))
- .setStarted(Optional.ofNullable(input.started))
- .setFinished(Optional.ofNullable(input.finished))
- .build();
+ CheckUpdate.Builder checkUpdateBuilder = CheckUpdate.builder();
+
+ if (input.state != null) {
+ checkUpdateBuilder.setState(input.state);
+ }
+
+ if (input.url != null) {
+ checkUpdateBuilder.setUrl(UrlValidator.clean(input.url));
+ }
+
+ if (input.started != null) {
+ checkUpdateBuilder.setStarted(input.started);
+ }
+
+ if (input.finished != null) {
+ checkUpdateBuilder.setFinished(input.finished);
+ }
+
+ return checkUpdateBuilder.build();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java b/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
index 8f6482a..2b52000 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
@@ -1,5 +1,6 @@
package com.google.gerrit.plugins.checks.db;
+import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.plugins.checks.Check;
import com.google.gerrit.plugins.checks.CheckKey;
@@ -59,8 +60,8 @@
state = update.state().get();
modified = true;
}
- if (update.url().isPresent() && !update.url().get().equals(url)) {
- url = update.url().get();
+ if (update.url().isPresent() && !update.url().get().equals(Strings.nullToEmpty(url))) {
+ url = Strings.emptyToNull(update.url().get());
modified = true;
}
if (update.started().isPresent() && !update.started().get().equals(started)) {
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 2d97826..c39e68a 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
@@ -100,6 +100,17 @@
}
@Test
+ public void unsetUrl() throws Exception {
+ checkOperations.check(checkKey).forUpdate().setUrl("http://example.com/my-check").upsert();
+
+ CheckInput input = new CheckInput();
+ input.url = "";
+
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ assertThat(info.url).isNull();
+ }
+
+ @Test
public void cannotSetInvalidUrl() throws Exception {
CheckInput input = new CheckInput();
input.url = CheckTestData.INVALID_URL;
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
index d2d6d42..500daf8 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
@@ -141,6 +141,16 @@
}
@Test
+ public void requestingNoUrlIsPossibleForCheckCreation() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+ CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
+ checkOperations.newCheck(checkKey).clearUrl().upsert();
+
+ CheckInfo check = getCheckFromServer(checkKey);
+ assertThat(check.url).isNull();
+ }
+
+ @Test
public void specifiedStateIsRespectedForCheckCreation() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
@@ -261,6 +271,17 @@
}
@Test
+ public void emptyUrlOfExistingCheckCanBeRetrieved() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
+ checkOperations.newCheck(checkKey).clearUrl().upsert();
+
+ Optional<String> url = checkOperations.check(checkKey).get().url();
+
+ assertThat(url).isEmpty();
+ }
+
+ @Test
public void startedOfExistingCheckCanBeRetrieved() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
@@ -361,6 +382,18 @@
}
@Test
+ public void urlCanBeCleared() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
+ checkOperations.newCheck(checkKey).setUrl("http://original-url").upsert();
+
+ checkOperations.check(checkKey).forUpdate().clearUrl().upsert();
+
+ Optional<String> currentUrl = checkOperations.check(checkKey).get().url();
+ assertThat(currentUrl).isEmpty();
+ }
+
+ @Test
public void startedCanBeUpdated() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
