GetCodeOwnerConfigFiles: Allow to include non-parsable files in the response
Some callers may be interested in all code owner config files, even if
they are non-parsable.
The new option to include non-parsable files cannot be used in
combination with the email option, since for non-parsable files we
cannot check if it contains a code owner with that email.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ia04d2e32abcf76cc035c76086c9308779d9eb77e
diff --git a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java
index 1836ca9..2b81afb 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java
@@ -32,8 +32,20 @@
CodeOwnerConfigFilesRequest codeOwnerConfigFiles() throws RestApiException;
abstract class CodeOwnerConfigFilesRequest {
+ private boolean includeNonParsableFiles;
private String email;
+ /** Includes non-parsable code owner config files into the result. */
+ public CodeOwnerConfigFilesRequest includeNonParsableFiles(boolean includeNonParsableFiles) {
+ this.includeNonParsableFiles = includeNonParsableFiles;
+ return this;
+ }
+
+ /** Whether non-parsable code owner config files should be included into the result. */
+ public boolean getIncludeNonParsableFiles() {
+ return includeNonParsableFiles;
+ }
+
/**
* Limits the returned code owner config files to those that contain the given email.
*
diff --git a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java
index 6ed89a9..aacc3e2 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java
@@ -60,6 +60,7 @@
@Override
public List<String> paths() throws RestApiException {
GetCodeOwnerConfigFiles getCodeOwnerConfigFiles = getCodeOwnerConfigFilesProvider.get();
+ getCodeOwnerConfigFiles.setIncludeNonParsableFiles(getIncludeNonParsableFiles());
getCodeOwnerConfigFiles.setEmail(getEmail());
return getCodeOwnerConfigFiles.apply(branchResource).value();
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigFiles.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigFiles.java
index 0ebf942..8190e7c 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigFiles.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigFiles.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackend;
@@ -48,9 +49,17 @@
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
private final CodeOwnerConfigScanner.Factory codeOwnerConfigScannerFactory;
+ private boolean includeNonParsableFiles;
private String email;
@Option(
+ name = "--include-non-parsable-files",
+ usage = "includes non-parseable code owner config files in the response")
+ public void setIncludeNonParsableFiles(boolean includeNonParsableFiles) {
+ this.includeNonParsableFiles = includeNonParsableFiles;
+ }
+
+ @Option(
name = "--email",
metaVar = "EMAIL",
usage = "limits the returned code owner config files to those that contain this email")
@@ -67,7 +76,9 @@
}
@Override
- public Response<List<String>> apply(BranchResource resource) {
+ public Response<List<String>> apply(BranchResource resource) throws BadRequestException {
+ validateOptions();
+
CodeOwnerBackend codeOwnerBackend =
codeOwnersPluginConfiguration.getBackend(resource.getBranchKey());
ImmutableList.Builder<Path> codeOwnerConfigs = ImmutableList.builder();
@@ -93,7 +104,11 @@
}
return true;
},
- CodeOwnerConfigScanner.ignoreInvalidCodeOwnerConfigFiles());
+ includeNonParsableFiles
+ ? (codeOwnerConfigFilePath, configInvalidException) -> {
+ codeOwnerConfigs.add(codeOwnerConfigFilePath);
+ }
+ : CodeOwnerConfigScanner.ignoreInvalidCodeOwnerConfigFiles());
return Response.ok(
codeOwnerConfigs.build().stream().map(Path::toString).collect(toImmutableList()));
}
@@ -120,4 +135,11 @@
}
return containsEmail;
}
+
+ private void validateOptions() throws BadRequestException {
+ if (email != null && includeNonParsableFiles) {
+ throw new BadRequestException(
+ "the options 'email' and 'include-non-parsable-files' are mutually exclusive");
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigFilesIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigFilesIT.java
index 50b795b..284f01b 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigFilesIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigFilesIT.java
@@ -15,9 +15,11 @@
package com.google.gerrit.plugins.codeowners.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.plugins.codeowners.JgitPath;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackend;
@@ -226,6 +228,50 @@
}
@Test
+ public void includeInvalidCodeOwnerConfigFiles() throws Exception {
+ String nameOfInvalidCodeOwnerConfigFile = getCodeOwnerConfigFileName();
+ createInvalidCodeOwnerConfig(nameOfInvalidCodeOwnerConfigFile);
+
+ CodeOwnerConfig.Key codeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .branch("master")
+ .codeOwnerConfigFiles()
+ .includeNonParsableFiles(true)
+ .paths())
+ .containsExactly(
+ JgitPath.of(nameOfInvalidCodeOwnerConfigFile).getAsAbsolutePath().toString(),
+ codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getFilePath());
+ }
+
+ @Test
+ public void includeNonParsableFilesAndEmailOptionsAreMutuallyExclusive() throws Exception {
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ projectCodeOwnersApiFactory
+ .project(project)
+ .branch("master")
+ .codeOwnerConfigFiles()
+ .includeNonParsableFiles(true)
+ .withEmail(admin.email())
+ .paths());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("the options 'email' and 'include-non-parsable-files' are mutually exclusive");
+ }
+
+ @Test
public void filterByEmail() throws Exception {
TestAccount user2 = accountCreator.user2();
TestAccount user3 = accountCreator.create("user3", "user3@example.com", "User3", null);
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index 7044cd8..c2aafff 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -182,8 +182,8 @@
| Field Name | | Description |
| ----------- | -------- | ----------- |
-| `email` | optional | Code owner email that must appear in the returned
-code owner config files.
+| `include-non-parsable-files` | optional | Includes non-parseable code owner config files in the response. By default `false`. Cannot be used in combination with the `email` option.
+| `email` | optional | Code owner email that must appear in the returned code owner config files.
#### Request
@@ -195,7 +195,8 @@
result also includes code owner config that use name prefixes
('\<prefix\>_OWNERS') or name extensions ('OWNERS_\<extension\>').
-Non-parseable code owner config files are omitted from the response.
+Non-parseable code owner config files are omitted from the response, unless the
+`include-non-parsable-files` option was set.
#### Response
