Merge changes Id885bde0,I6b9124e7,Ifdb65a25,I240cfb73
* changes:
Add method to test API to retrieve raw file content of code owner config
CodeOwnerResolver: Add null check for email in isEmailDomainAllowed
Do not reject '*' as email if allowed email domains are configured
CodeOwnerConfigValidatorIT: Test that code owner with allowed email domain can be added
diff --git a/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperations.java b/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperations.java
index 4d7f536..d98c735 100644
--- a/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperations.java
+++ b/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperations.java
@@ -110,6 +110,16 @@
String getFilePath();
/**
+ * Retrieves the raw file content of the code owner config.
+ *
+ * <p><strong>Note:</strong> This call will fail with an {@link IllegalStateException} if the
+ * requested code owner config doesn't exist.
+ *
+ * @return the raw file content of the code owner config
+ */
+ String getContent() throws Exception;
+
+ /**
* Starts the fluent chain to update a code owner config. The returned builder can be used to
* specify how the attributes of the code owner config should be modified. To update the code
* owner config for real, {@link TestCodeOwnerConfigUpdate.Builder#update()} must be called.
diff --git a/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperationsImpl.java b/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperationsImpl.java
index 1e98136..8462d52 100644
--- a/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperationsImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperationsImpl.java
@@ -25,11 +25,20 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnersUpdate;
import com.google.gerrit.plugins.codeowners.config.BackendConfig;
import com.google.gerrit.server.ServerInitiated;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.nio.file.Path;
import java.util.Optional;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevObject;
+import org.eclipse.jgit.treewalk.TreeWalk;
+import org.eclipse.jgit.util.RawParseUtils;
/**
* The implementation of {@link CodeOwnerConfigOperations}.
@@ -38,6 +47,7 @@
* the separation between interface and implementation to enhance clarity.
*/
public class CodeOwnerConfigOperationsImpl implements CodeOwnerConfigOperations {
+ private final GitRepositoryManager repoManager;
private final CodeOwners codeOwners;
private final Provider<CodeOwnersUpdate> codeOwnersUpdate;
private final ProjectCache projectCache;
@@ -45,10 +55,12 @@
@Inject
CodeOwnerConfigOperationsImpl(
+ GitRepositoryManager repoManager,
CodeOwners codeOwners,
@ServerInitiated Provider<CodeOwnersUpdate> codeOwnersUpdate,
ProjectCache projectCache,
BackendConfig backendConfig) {
+ this.repoManager = repoManager;
this.codeOwners = codeOwners;
this.codeOwnersUpdate = codeOwnersUpdate;
this.projectCache = projectCache;
@@ -145,6 +157,33 @@
}
@Override
+ public String getContent() throws Exception {
+ try (TestRepository<Repository> testRepo =
+ new TestRepository<>(repoManager.openRepository(codeOwnerConfigKey.project()))) {
+ Ref ref = testRepo.getRepository().exactRef(codeOwnerConfigKey.ref());
+ if (ref == null) {
+ throw new IllegalStateException(
+ String.format("code owner config %s does not exist", codeOwnerConfigKey));
+ }
+ RevCommit commit = testRepo.getRevWalk().parseCommit(ref.getObjectId());
+ try (TreeWalk tw =
+ TreeWalk.forPath(
+ testRepo.getRevWalk().getObjectReader(), getJGitFilePath(), commit.getTree())) {
+ if (tw == null) {
+ throw new IllegalStateException(
+ String.format("code owner config %s does not exist", codeOwnerConfigKey));
+ }
+ }
+ RevObject blob = testRepo.get(commit.getTree(), getJGitFilePath());
+ byte[] data = testRepo.getRepository().open(blob).getCachedBytes(Integer.MAX_VALUE);
+ return RawParseUtils.decode(data);
+ } catch (RepositoryNotFoundException e) {
+ throw new IllegalStateException(
+ String.format("code owner config %s does not exist", codeOwnerConfigKey));
+ }
+ }
+
+ @Override
public TestCodeOwnerConfigUpdate.Builder forUpdate() {
return TestCodeOwnerConfigUpdate.builder(this::updateCodeOwnerConfig);
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
index fdd4da2..3cb377b 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
@@ -370,6 +370,8 @@
* {@code false}
*/
public boolean isEmailDomainAllowed(String email) {
+ requireNonNull(email, "email");
+
ImmutableSet<String> allowedEmailDomains =
codeOwnersPluginConfiguration.getAllowedEmailDomains();
if (allowedEmailDomains.isEmpty()) {
@@ -377,6 +379,10 @@
return true;
}
+ if (email.equals(ALL_USERS_WILDCARD)) {
+ return true;
+ }
+
int emailAtIndex = email.lastIndexOf('@');
if (emailAtIndex >= 0 && emailAtIndex < email.length() - 1) {
String emailDomain = email.substring(emailAtIndex + 1);
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
index 142c0ae..357c5d8 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
@@ -43,6 +43,7 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigImportMode;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigImportType;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigReference;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerSet;
import com.google.gerrit.plugins.codeowners.backend.findowners.FindOwnersBackend;
import com.google.gerrit.plugins.codeowners.backend.findowners.FindOwnersCodeOwnerConfigParser;
@@ -99,6 +100,35 @@
}
@Test
+ public void canUploadConfigWhichAssignsCodeOwnershipToAllUsers() throws Exception {
+ testCanUploadConfigWhichAssignsCodeOwnershipToAllUsers();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.allowedEmailDomain", value = "example.com")
+ public void canUploadConfigWhichAssignsCodeOwnershipToAllUsers_restrictedAllowedEmailDomain()
+ throws Exception {
+ testCanUploadConfigWhichAssignsCodeOwnershipToAllUsers();
+ }
+
+ private void testCanUploadConfigWhichAssignsCodeOwnershipToAllUsers() throws Exception {
+ CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
+
+ // Create a code owner config without issues that assigns code ownership to all users.
+ PushOneCommit.Result r =
+ createChange(
+ "Add code owners",
+ codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
+ format(
+ CodeOwnerConfig.builder(codeOwnerConfigKey, TEST_REVISION)
+ .addCodeOwnerSet(
+ CodeOwnerSet.createWithoutPathExpressions(
+ CodeOwnerResolver.ALL_USERS_WILDCARD))
+ .build()));
+ assertOkWithHints(r, "code owner config files validated, no issues found");
+ }
+
+ @Test
public void canSubmitConfigWithoutIssues() throws Exception {
CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
@@ -608,6 +638,24 @@
@Test
@GerritConfig(name = "plugin.code-owners.allowedEmailDomain", value = "example.com")
+ public void canUploadConfigThatAssignsCodeOwnershipToAnEmailWithAnAllowedEmailDomain()
+ throws Exception {
+ CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
+
+ assertThat(admin.email()).endsWith("@example.com");
+ PushOneCommit.Result r =
+ createChange(
+ "Add code owners",
+ codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getJGitFilePath(),
+ format(
+ CodeOwnerConfig.builder(codeOwnerConfigKey, TEST_REVISION)
+ .addCodeOwnerSet(CodeOwnerSet.createWithoutPathExpressions(admin.email()))
+ .build()));
+ assertOkWithHints(r, "code owner config files validated, no issues found");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.allowedEmailDomain", value = "example.com")
public void cannotUploadConfigThatAssignsCodeOwnershipToAnEmailWithANonAllowedEmailDomain()
throws Exception {
CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperationsImplTest.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperationsImplTest.java
index 542fc3d..c0e9040 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/testsuite/CodeOwnerConfigOperationsImplTest.java
@@ -639,6 +639,58 @@
.isEqualTo("/foo/OWNERS");
}
+ @Test
+ public void getContent() throws Exception {
+ CodeOwnerConfig.Key codeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+ assertThat(codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getContent())
+ .isEqualTo(admin.email() + "\n");
+ }
+
+ @Test
+ public void getContent_nonExistingCodeOwnerConfig() throws Exception {
+ CodeOwnerConfig.Key codeOwnerConfigKey = CodeOwnerConfig.Key.create(project, "master", "/foo/");
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () -> codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getContent());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(String.format("code owner config %s does not exist", codeOwnerConfigKey));
+ }
+
+ @Test
+ public void getContent_nonExistingBranch() throws Exception {
+ CodeOwnerConfig.Key codeOwnerConfigKey =
+ CodeOwnerConfig.Key.create(project, "non-existing", "/foo/");
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () -> codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getContent());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(String.format("code owner config %s does not exist", codeOwnerConfigKey));
+ }
+
+ @Test
+ public void getContent_nonExistingProject() throws Exception {
+ CodeOwnerConfig.Key codeOwnerConfigKey =
+ CodeOwnerConfig.Key.create(Project.nameKey("non-existing"), "master", "/foo/");
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () -> codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getContent());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(String.format("code owner config %s does not exist", codeOwnerConfigKey));
+ }
+
private CodeOwnerConfig getCodeOwnerConfigFromServer(CodeOwnerConfig.Key codeOwnerConfigKey) {
return codeOwners
.getFromCurrentRevision(codeOwnerConfigKey)
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
index f390336..bd1bad1 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
@@ -322,6 +322,14 @@
}
@Test
+ public void isEmailDomainAllowedRequiresEmailToBeNonNull() throws Exception {
+ NullPointerException npe =
+ assertThrows(
+ NullPointerException.class, () -> codeOwnerResolver.get().isEmailDomainAllowed(null));
+ assertThat(npe).hasMessageThat().isEqualTo("email");
+ }
+
+ @Test
@GerritConfig(
name = "plugin.code-owners.allowedEmailDomain",
values = {"example.com", "example.net"})
@@ -334,6 +342,8 @@
assertThat(codeOwnerResolver.get().isEmailDomainAllowed("foo")).isFalse();
assertThat(codeOwnerResolver.get().isEmailDomainAllowed("foo@example.com@example.org"))
.isFalse();
+ assertThat(codeOwnerResolver.get().isEmailDomainAllowed(CodeOwnerResolver.ALL_USERS_WILDCARD))
+ .isTrue();
}
@Test
@@ -346,6 +356,8 @@
assertThat(codeOwnerResolver.get().isEmailDomainAllowed("foo")).isTrue();
assertThat(codeOwnerResolver.get().isEmailDomainAllowed("foo@example.com@example.org"))
.isTrue();
+ assertThat(codeOwnerResolver.get().isEmailDomainAllowed(CodeOwnerResolver.ALL_USERS_WILDCARD))
+ .isTrue();
}
@Test
