Convert checks APIs to use CheckerUuid instead of string
Change-Id: I8d488bdc88b1fe1959c70d49fa554f8d7e9ebac7
diff --git a/java/com/google/gerrit/plugins/checks/CheckJson.java b/java/com/google/gerrit/plugins/checks/CheckJson.java
index 34a0998..bc2019b 100644
--- a/java/com/google/gerrit/plugins/checks/CheckJson.java
+++ b/java/com/google/gerrit/plugins/checks/CheckJson.java
@@ -22,7 +22,7 @@
public class CheckJson {
public CheckInfo format(Check check) {
CheckInfo info = new CheckInfo();
- info.checkerUuid = check.key().checkerUuid();
+ info.checkerUuid = check.key().checkerUuid().toString();
info.changeNumber = check.key().patchSet().changeId.id;
info.project = check.key().project().get();
info.patchSetId = check.key().patchSet().patchSetId;
diff --git a/java/com/google/gerrit/plugins/checks/CheckKey.java b/java/com/google/gerrit/plugins/checks/CheckKey.java
index 3099a76..3b5bc3a 100644
--- a/java/com/google/gerrit/plugins/checks/CheckKey.java
+++ b/java/com/google/gerrit/plugins/checks/CheckKey.java
@@ -25,9 +25,10 @@
public abstract PatchSet.Id patchSet();
- public abstract String checkerUuid();
+ public abstract CheckerUuid checkerUuid();
- public static CheckKey create(Project.NameKey project, PatchSet.Id patchSet, String checkerUuid) {
+ public static CheckKey create(
+ Project.NameKey project, PatchSet.Id patchSet, CheckerUuid checkerUuid) {
return new AutoValue_CheckKey(project, patchSet, checkerUuid);
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckResource.java b/java/com/google/gerrit/plugins/checks/api/CheckResource.java
index 2160465..958a00f 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckResource.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckResource.java
@@ -17,6 +17,7 @@
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.plugins.checks.Check;
+import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
@@ -47,7 +48,7 @@
return check;
}
- public String getCheckerUuid() {
+ public CheckerUuid getCheckerUuid() {
return check.key().checkerUuid();
}
diff --git a/java/com/google/gerrit/plugins/checks/api/Checks.java b/java/com/google/gerrit/plugins/checks/api/Checks.java
index 9e9be85..78beabe 100644
--- a/java/com/google/gerrit/plugins/checks/api/Checks.java
+++ b/java/com/google/gerrit/plugins/checks/api/Checks.java
@@ -16,13 +16,21 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.restapi.NotImplementedException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gwtorm.server.OrmException;
import java.io.IOException;
/** Java API to interact with {@code Check}s. */
public interface Checks {
- CheckApi id(String checkerUuid) throws RestApiException, IOException, OrmException;
+ CheckApi id(CheckerUuid checkerUuid) throws RestApiException, IOException, OrmException;
+
+ default CheckApi id(String uuidString) throws RestApiException, IOException, OrmException {
+ return id(
+ CheckerUuid.tryParse(uuidString)
+ .orElseThrow(() -> new ResourceNotFoundException("Not found: " + uuidString)));
+ }
CheckApi create(CheckInput input) throws RestApiException, IOException, OrmException;
@@ -34,7 +42,7 @@
*/
class NotImplemented implements Checks {
@Override
- public CheckApi id(String checkerUuid) throws RestApiException, IOException, OrmException {
+ public CheckApi id(CheckerUuid checkerUuid) throws RestApiException, IOException, OrmException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java b/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
index 4167f5c..47b5f77 100644
--- a/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
+++ b/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
@@ -15,7 +15,6 @@
package com.google.gerrit.plugins.checks.api;
import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -24,6 +23,7 @@
import com.google.gerrit.extensions.restapi.RestView;
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.Checks;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -52,10 +52,11 @@
@Override
public CheckResource parse(RevisionResource parent, IdString id)
- throws AuthException, ResourceNotFoundException, PermissionBackendException, IOException,
- OrmException {
+ throws RestApiException, PermissionBackendException, IOException, OrmException {
+ CheckerUuid checkerUuid =
+ CheckerUuid.tryParse(id.get()).orElseThrow(() -> new ResourceNotFoundException(id.get()));
CheckKey checkKey =
- CheckKey.create(parent.getProject(), parent.getPatchSet().getId(), id.get());
+ CheckKey.create(parent.getProject(), parent.getPatchSet().getId(), checkerUuid);
Optional<Check> check = checks.getCheck(checkKey);
return new CheckResource(
parent, check.orElseThrow(() -> new ResourceNotFoundException("Not found: " + id.get())));
diff --git a/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java b/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java
index b0edeec..1e66dbd 100644
--- a/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java
+++ b/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java
@@ -24,6 +24,7 @@
import com.google.gerrit.plugins.checks.CheckJson;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckUpdate;
+import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.Checks;
import com.google.gerrit.plugins.checks.ChecksUpdate;
import com.google.gerrit.server.UserInitiated;
@@ -66,16 +67,13 @@
}
@Override
- public CheckApi id(String checkerUuid) throws RestApiException, IOException, OrmException {
- if (checkerUuid == null || checkerUuid.isEmpty()) {
- throw new BadRequestException("checkerUuid is required");
- }
- // Check that the checker exists and throw a RestApiException if not.
- CheckerInfo checker = checkers.id(checkerUuid).get();
+ public CheckApi id(CheckerUuid checkerUuid) throws RestApiException, IOException, OrmException {
+ // Ensure that the checker exists and throw a RestApiException if not.
+ checkers.id(checkerUuid).get();
CheckKey checkKey =
CheckKey.create(
- revisionResource.getProject(), revisionResource.getPatchSet().getId(), checker.uuid);
+ revisionResource.getProject(), revisionResource.getPatchSet().getId(), checkerUuid);
Optional<Check> check = checks.getCheck(checkKey);
return checkApiImplFactory.create(
new CheckResource(
@@ -89,17 +87,17 @@
throw new BadRequestException("input is required");
}
if (input.checkerUuid == null) {
- throw new BadRequestException("checkerUuid is required");
+ throw new BadRequestException("checker_uuid is required");
}
if (input.state == null) {
throw new BadRequestException("state is required");
}
- // Check that the checker exists and throw a BadRequestException if not.
- CheckerInfo checker = checkers.id(input.checkerUuid).get();
+ // Ensure that the checker exists and throw a RestApiException if not.
+ CheckerUuid checkerUuid = CheckerUuid.parse(checkers.id(input.checkerUuid).get().uuid);
CheckKey checkKey =
CheckKey.create(
- revisionResource.getProject(), revisionResource.getPatchSet().getId(), checker.uuid);
+ revisionResource.getProject(), revisionResource.getPatchSet().getId(), checkerUuid);
CheckUpdate checkUpdate =
CheckUpdate.builder()
.setState(input.state)
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java b/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
index c02db7e..347def5 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
@@ -4,6 +4,7 @@
import com.google.gerrit.plugins.checks.Check;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckUpdate;
+import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
@@ -37,7 +38,7 @@
return newCheck.build();
}
- Check toCheck(Project.NameKey project, PatchSet.Id patchSetId, String checkerUuid) {
+ Check toCheck(Project.NameKey project, PatchSet.Id patchSetId, CheckerUuid checkerUuid) {
CheckKey key = CheckKey.create(project, patchSetId, checkerUuid);
return toCheck(key);
}
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
index 1bc0195..aafb475 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
@@ -91,8 +91,8 @@
.checks
.entrySet()
.stream()
- .map(e -> e.getValue().toCheck(projectName, psId, e.getKey()))
- .filter(c -> checkerUuids.contains(CheckerUuid.parse(c.key().checkerUuid())));
+ .map(e -> e.getValue().toCheck(projectName, psId, CheckerUuid.parse(e.getKey())))
+ .filter(c -> checkerUuids.contains(c.key().checkerUuid()));
}
/** Get all checkers that apply to a project. Might return a superset of checkers that apply. */
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
index 2701052..707ce8b 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
@@ -27,6 +27,7 @@
import com.google.gerrit.plugins.checks.CheckUpdate;
import com.google.gerrit.plugins.checks.Checker;
import com.google.gerrit.plugins.checks.CheckerRef;
+import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.Checkers;
import com.google.gerrit.plugins.checks.ChecksUpdate;
import com.google.gerrit.plugins.checks.api.CheckerStatus;
@@ -199,7 +200,7 @@
}
}
- private void assertCheckerPresentAndEnabled(String checkerUuid)
+ private void assertCheckerPresentAndEnabled(CheckerUuid checkerUuid)
throws ConfigInvalidException, IOException {
Checker checker =
checkers
@@ -237,7 +238,7 @@
}
NoteDbCheckMap checksForRevision = newNotes.get(revId);
- if (!checksForRevision.checks.containsKey(checkKey.checkerUuid())) {
+ if (!checksForRevision.checks.containsKey(checkKey.checkerUuid().toString())) {
if (operation == Operation.UPDATE) {
throw new IOException("Not found: " + checkKey.checkerUuid());
}
@@ -246,7 +247,7 @@
NoteDbCheck newCheck = NoteDbCheck.fromCheckUpdate(checkUpdate);
newCheck.created = Timestamp.from(personIdent.getWhen().toInstant());
newCheck.updated = newCheck.created;
- checksForRevision.checks.put(checkKey.checkerUuid(), newCheck);
+ checksForRevision.checks.put(checkKey.checkerUuid().toString(), newCheck);
writeNotesMap(newNotes, cb, ins);
return true;
} else if (operation == Operation.CREATE) {
@@ -254,7 +255,7 @@
}
// Update in place
- NoteDbCheck modifiedCheck = checksForRevision.checks.get(checkKey.checkerUuid());
+ NoteDbCheck modifiedCheck = checksForRevision.checks.get(checkKey.checkerUuid().toString());
boolean dirty = modifiedCheck.applyUpdate(checkUpdate);
if (!dirty) {
return false;
@@ -343,9 +344,10 @@
throw new IllegalStateException("revision " + revId + " not found");
}
Map<String, NoteDbCheck> checks = newNotes.get(revId).checks;
- if (!checks.containsKey(checkKey.checkerUuid())) {
- throw new IllegalStateException("checker " + checkKey.checkerUuid() + " not found");
+ String checkerUuidString = checkKey.checkerUuid().toString();
+ if (!checks.containsKey(checkerUuidString)) {
+ throw new IllegalStateException("checker " + checkerUuidString + " not found");
}
- return checks.get(checkKey.checkerUuid()).toCheck(checkKey);
+ return checks.get(checkerUuidString).toCheck(checkKey);
}
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/ChecksRestApiBindingsIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/ChecksRestApiBindingsIT.java
index 96886ed..a057204 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/ChecksRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/ChecksRestApiBindingsIT.java
@@ -49,7 +49,7 @@
@Test
public void checkEndpoints() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- CheckKey key = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid.toString());
+ CheckKey key = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
checkOperations.newCheck(key).setState(CheckState.RUNNING).upsert();
RestApiCallHelper.execute(
@@ -62,7 +62,7 @@
@Test
public void scopedCheckEndpoints() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- CheckKey key = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid.toString());
+ CheckKey key = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
checkOperations.newCheck(key).setState(CheckState.RUNNING).upsert();
RestApiCallHelper.execute(
adminRestSession,
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/GetCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/GetCheckIT.java
index 2124aa2..69ac4b1 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/GetCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/GetCheckIT.java
@@ -38,7 +38,7 @@
@Test
public void getCheck() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- CheckKey key = CheckKey.create(project, patchSetId, checkerUuid.toString());
+ CheckKey key = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(key).setState(CheckState.RUNNING).upsert();
CheckInfo info = checksApiFactory.revision(patchSetId).id(checkerUuid.toString()).get();
@@ -48,7 +48,7 @@
@Test
public void getCheckForDisabledCheckerThrowsNotFound() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- CheckKey key = CheckKey.create(project, patchSetId, checkerUuid.toString());
+ CheckKey key = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(key).setState(CheckState.RUNNING).upsert();
checkerOperations.checker(checkerUuid).forUpdate().disable().update();
@@ -61,7 +61,7 @@
@Test
public void getCheckForInvalidCheckerThrowsNotFound() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- CheckKey key = CheckKey.create(project, patchSetId, checkerUuid.toString());
+ CheckKey key = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(key).setState(CheckState.RUNNING).upsert();
checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
@@ -74,9 +74,9 @@
@Test
public void getNonExistingCheckFails() throws Exception {
exception.expect(ResourceNotFoundException.class);
- exception.expectMessage("Not found: non-existing");
+ exception.expectMessage("Not found: test:non-existing");
- checksApiFactory.revision(patchSetId).id("non-existing").get();
+ checksApiFactory.revision(patchSetId).id("test:non-existing").get();
}
@Test
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 77550be..54b3fe4 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
@@ -68,7 +68,7 @@
assertThat(info.created).isNotNull();
assertThat(info.updated).isNotNull();
- CheckKey key = CheckKey.create(project, patchSetId, checkerUuid.toString());
+ CheckKey key = CheckKey.create(project, patchSetId, checkerUuid);
PerCheckOperations perCheckOps = checkOperations.check(key);
// TODO(gerrit-team) Add a Truth subject for the notes map
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 2a1c4ae..159f9f2 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
@@ -38,10 +38,10 @@
CheckerUuid checker1Uuid = checkerOperations.newChecker().repository(project).create();
CheckerUuid checker2Uuid = checkerOperations.newChecker().repository(project).create();
- checkKey1 = CheckKey.create(project, patchSetId, checker1Uuid.toString());
+ checkKey1 = CheckKey.create(project, patchSetId, checker1Uuid);
checkOperations.newCheck(checkKey1).setState(CheckState.RUNNING).upsert();
- checkKey2 = CheckKey.create(project, patchSetId, checker2Uuid.toString());
+ checkKey2 = CheckKey.create(project, patchSetId, checker2Uuid);
checkOperations.newCheck(checkKey2).setState(CheckState.RUNNING).upsert();
}
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 6b5db88..f02a9b2 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
@@ -37,7 +37,7 @@
patchSetId = createChange().getPatchSetId();
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- checkKey = CheckKey.create(project, patchSetId, checkerUuid.toString());
+ checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).setState(CheckState.RUNNING).upsert();
}
