Merge "Adapt to API change in Gerrit core and handle case where branch rev not present"
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractPathResource.java b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractPathResource.java
index 75cc0eb..dd603fc 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractPathResource.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractPathResource.java
@@ -14,11 +14,10 @@
package com.google.gerrit.plugins.codeowners.restapi;
-import static com.google.common.base.Preconditions.checkState;
-
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.project.BranchResource;
@@ -58,16 +57,14 @@
private final ObjectId revision;
private final Path path;
- protected AbstractPathResource(BranchResource branchResource, Path path) {
+ protected AbstractPathResource(BranchResource branchResource, Path path)
+ throws ResourceNotFoundException {
+ if (branchResource.getRevision().isEmpty()) {
+ throw new ResourceNotFoundException(IdString.fromDecoded(branchResource.getName()));
+ }
+
this.branchNameKey = branchResource.getBranchKey();
-
- checkState(
- branchResource.getRevision() != null,
- "branch %s in project %s wasn't created yet",
- branchResource.getBranchKey().branch(),
- branchResource.getBranchKey().project().get());
- this.revision = ObjectId.fromString(branchResource.getRevision());
-
+ this.revision = ObjectId.fromString(branchResource.getRevision().get());
this.path = path;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
index d6470a2..ad8b388 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
@@ -148,7 +148,7 @@
@Override
public Response<CodeOwnerCheckInfo> apply(BranchResource branchResource)
throws BadRequestException, AuthException, IOException, ConfigInvalidException,
- PermissionBackendException {
+ PermissionBackendException, ResourceNotFoundException {
permissionBackend.currentUser().check(checkCodeOwnerCapability.getPermission());
validateInput(branchResource);
@@ -164,7 +164,7 @@
Set<String> annotations = new HashSet<>();
codeOwnerConfigHierarchy.visit(
branchResource.getBranchKey(),
- ObjectId.fromString(branchResource.getRevision()),
+ ObjectId.fromString(branchResource.getRevision().get()),
absolutePath,
codeOwnerConfig -> {
messages.add(
@@ -348,7 +348,11 @@
private void validateInput(BranchResource branchResource)
throws BadRequestException, AuthException, IOException, ConfigInvalidException,
- PermissionBackendException {
+ PermissionBackendException, ResourceNotFoundException {
+ if (branchResource.getRevision().isEmpty()) {
+ throw new ResourceNotFoundException(IdString.fromDecoded(branchResource.getName()));
+ }
+
if (email == null) {
throw new BadRequestException("email required");
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerConfigsInBranchCollection.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerConfigsInBranchCollection.java
index 9b13e81..3d324d5 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerConfigsInBranchCollection.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerConfigsInBranchCollection.java
@@ -49,7 +49,8 @@
}
@Override
- public PathResource parse(BranchResource branchResource, IdString id) throws BadRequestException {
+ public PathResource parse(BranchResource branchResource, IdString id)
+ throws BadRequestException, ResourceNotFoundException {
return PathResource.parse(branchResource, id);
}
@@ -72,11 +73,12 @@
static final TypeLiteral<RestView<PathResource>> PATH_KIND = new TypeLiteral<>() {};
static PathResource parse(BranchResource branchResource, IdString pathId)
- throws BadRequestException {
+ throws BadRequestException, ResourceNotFoundException {
return new PathResource(branchResource, parsePath(pathId));
}
- private PathResource(BranchResource branchResource, Path path) {
+ private PathResource(BranchResource branchResource, Path path)
+ throws ResourceNotFoundException {
super(branchResource, path);
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnersInBranchCollection.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnersInBranchCollection.java
index 0f4011c..feb1199 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnersInBranchCollection.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnersInBranchCollection.java
@@ -48,7 +48,8 @@
}
@Override
- public PathResource parse(BranchResource branchResource, IdString id) throws BadRequestException {
+ public PathResource parse(BranchResource branchResource, IdString id)
+ throws BadRequestException, ResourceNotFoundException {
return PathResource.parse(branchResource, id);
}
@@ -70,7 +71,7 @@
static final TypeLiteral<RestView<PathResource>> PATH_KIND = new TypeLiteral<>() {};
static PathResource parse(BranchResource branchResource, IdString pathId)
- throws BadRequestException {
+ throws BadRequestException, ResourceNotFoundException {
return new PathResource(branchResource, parsePath(pathId));
}
@@ -78,7 +79,8 @@
return new PathResource(getBranch(), revision, getPath());
}
- private PathResource(BranchResource branchResource, Path path) {
+ private PathResource(BranchResource branchResource, Path path)
+ throws ResourceNotFoundException {
super(branchResource, path);
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
index 7952b7e..83eb13f 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
@@ -24,6 +24,7 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
@@ -62,6 +63,7 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.util.Arrays;
+import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
import org.junit.Before;
import org.junit.Test;
@@ -88,6 +90,25 @@
}
@Test
+ public void checkCodeOwnerForNonExistingBranch() throws Exception {
+ RestResponse response =
+ adminRestSession.get(
+ String.format("/projects/%s/branches/non-existing/code_owners.check", project.get()));
+ response.assertNotFound();
+ }
+
+ @Test
+ public void checkCodeOwnerForSymbolicRefPointingToAnUnbornBranch() throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ repo.updateRef(Constants.HEAD, true).link("refs/heads/non-existing");
+ }
+ RestResponse response =
+ adminRestSession.get(
+ String.format("/projects/%s/branches/HEAD/code_owners.check", project.get()));
+ response.assertNotFound();
+ }
+
+ @Test
public void requiresEmail() throws Exception {
BadRequestException exception =
assertThrows(BadRequestException.class, () -> checkCodeOwner("/", /* email= */ null));
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigForPathInBranchIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigForPathInBranchIT.java
index 3048707..5ddd85f 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigForPathInBranchIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigForPathInBranchIT.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerConfigInfoSubject.assertThatOptional;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
@@ -25,6 +26,8 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
import com.google.gerrit.plugins.codeowners.util.JgitPath;
import java.util.Optional;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.Repository;
import org.junit.Test;
/**
@@ -39,6 +42,28 @@
public class GetCodeOwnerConfigForPathInBranchIT extends AbstractCodeOwnersIT {
@Test
@GerritConfig(name = "plugin.code-owners.enableExperimentalRestEndpoints", value = "true")
+ public void getCodeOwnerConfigForNonExistingBranch() throws Exception {
+ RestResponse response =
+ adminRestSession.get(
+ String.format(
+ "/projects/%s/branches/non-existing/code_owners.config/path", project.get()));
+ response.assertNotFound();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.enableExperimentalRestEndpoints", value = "true")
+ public void getCodeOwnerConfigFromSymbolicRefPointingToAnUnbornBranch() throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ repo.updateRef(Constants.HEAD, true).link("refs/heads/non-existing");
+ }
+ RestResponse response =
+ adminRestSession.get(
+ String.format("/projects/%s/branches/HEAD/code_owners.config/path", project.get()));
+ response.assertNotFound();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.enableExperimentalRestEndpoints", value = "true")
public void getNonExistingCodeOwnerConfig() throws Exception {
assertThatOptional(codeOwnerConfigsApiFactory.branch(project, "master").get("/")).isEmpty();
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnersForPathInBranchIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnersForPathInBranchIT.java
index dfd97d4..a78d420 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnersForPathInBranchIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnersForPathInBranchIT.java
@@ -21,6 +21,7 @@
import static java.util.stream.Collectors.toList;
import static org.junit.Assert.fail;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
@@ -38,6 +39,8 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerSetModification;
import com.google.inject.Inject;
import java.util.List;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Test;
@@ -60,6 +63,25 @@
}
@Test
+ public void getCodeOwnersForNonExistingBranch() throws Exception {
+ RestResponse response =
+ adminRestSession.get(
+ String.format("/projects/%s/branches/non-existing/code_owners/path", project.get()));
+ response.assertNotFound();
+ }
+
+ @Test
+ public void getCodeOwnersFromSymbolicRefPointingToAnUnbornBranch() throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ repo.updateRef(Constants.HEAD, true).link("refs/heads/non-existing");
+ }
+ RestResponse response =
+ adminRestSession.get(
+ String.format("/projects/%s/branches/HEAD/code_owners/path", project.get()));
+ response.assertNotFound();
+ }
+
+ @Test
public void getCodeOwnersOrderNotDefinedIfCodeOwnersHaveTheSameScoring() throws Exception {
TestAccount user2 = accountCreator.user2();
TestAccount user3 = accountCreator.create("user3", "user3@example.com", "User3", null);
