Merge changes If8f371e7,I968977a0,I84560336
* changes:
User guide: Document how to submit changes with files that have no code owners
Document that imports and global owners are always read from the current revisions
GetCodeOwnerBranchConfig: Include a flag if no code owner config file exists yet
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java
index 2cb9375..c0ae04b 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java
@@ -58,4 +58,16 @@
* <p>Not set if {@link #disabled} is {@code true}.
*/
public RequiredApprovalInfo overrideApproval;
+
+ /**
+ * Whether the branch doesn't contain any code owner config file yet.
+ *
+ * <p>If a branch doesn't contain any code owner config file yet, the projects owners are
+ * considered as code owners. Once a first code owner config file is added to the branch, the
+ * project owners are no longer code owners (unless code ownership is granted to them via the code
+ * owner config file).
+ *
+ * <p>Not set if {@code false} or if {@link #disabled} is {@code true}.
+ */
+ public Boolean noCodeOwnersDefined;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
index 737a172..72e6de0 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
@@ -32,6 +32,7 @@
import com.google.gerrit.plugins.codeowners.api.GeneralInfo;
import com.google.gerrit.plugins.codeowners.api.RequiredApprovalInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackendId;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigScanner;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.config.RequiredApproval;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -49,13 +50,16 @@
@Singleton
public class CodeOwnerProjectConfigJson {
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
+ private final CodeOwnerConfigScanner codeOwnerConfigScanner;
private final Provider<ListBranches> listBranches;
@Inject
CodeOwnerProjectConfigJson(
CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
+ CodeOwnerConfigScanner codeOwnerConfigScanner,
Provider<ListBranches> listBranches) {
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
+ this.codeOwnerConfigScanner = codeOwnerConfigScanner;
this.listBranches = listBranches;
}
@@ -93,6 +97,10 @@
info.requiredApproval = formatRequiredApprovalInfo(branchResource.getNameKey());
info.overrideApproval = formatOverrideApprovalInfo(branchResource.getNameKey());
+ boolean noCodeOwnersDefined =
+ !codeOwnerConfigScanner.containsAnyCodeOwnerConfigFile(branchResource.getBranchKey());
+ info.noCodeOwnersDefined = noCodeOwnersDefined ? noCodeOwnersDefined : null;
+
return info;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java
index 6e3d054..c2c22df 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java
@@ -61,7 +61,12 @@
private final GitRepositoryManager repoManager;
private String revision;
- @Option(name = "-revision", usage = "revision from which the code owner configs should be read")
+ @Option(
+ name = "-revision",
+ usage =
+ "revision from which the code owner configs in the branch should be read (imports from"
+ + " other branches or repositories as well as global code owners from refs/meta/config"
+ + " are still read from the current revisions)")
public void setRevision(String revision) {
this.revision = revision;
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
index 20958e3..37d0d23 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
@@ -75,6 +75,14 @@
@Test
public void getDefaultConfig() throws Exception {
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
assertThat(codeOwnerBranchConfigInfo.general.fileExtension).isNull();
@@ -90,6 +98,7 @@
assertThat(codeOwnerBranchConfigInfo.requiredApproval.value)
.isEqualTo(RequiredApprovalConfig.DEFAULT_VALUE);
assertThat(codeOwnerBranchConfigInfo.overrideApproval).isNull();
+ assertThat(codeOwnerBranchConfigInfo.noCodeOwnersDefined).isNull();
}
@Test
@@ -212,6 +221,14 @@
assertThat(codeOwnerBranchConfigInfo.general.implicitApprovals).isTrue();
}
+ @Test
+ public void getConfig_bootstrappingMode() throws Exception {
+ configureImplicitApprovals(project);
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.noCodeOwnersDefined).isTrue();
+ }
+
private void configureFileExtension(Project.NameKey project, String fileExtension)
throws Exception {
setConfig(project, null, GeneralConfig.KEY_FILE_EXTENSION, fileExtension);
diff --git a/javatests/com/google/gerrit/plugins/codeowners/restapi/BUILD b/javatests/com/google/gerrit/plugins/codeowners/restapi/BUILD
index 19e6cce..2c509ff 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/restapi/BUILD
+++ b/javatests/com/google/gerrit/plugins/codeowners/restapi/BUILD
@@ -6,6 +6,7 @@
deps = [
"//plugins/code-owners:code-owners__plugin",
"//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/acceptance",
+ "//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite",
"//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/testing",
],
)
diff --git a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
index 232576a..456ebc1 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
@@ -24,6 +24,7 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.LabelType;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
+import com.google.gerrit.plugins.codeowners.acceptance.testsuite.CodeOwnerConfigOperations;
import com.google.gerrit.plugins.codeowners.api.BackendInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerBranchConfigInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerProjectConfigInfo;
@@ -31,6 +32,7 @@
import com.google.gerrit.plugins.codeowners.api.MergeCommitStrategy;
import com.google.gerrit.plugins.codeowners.api.RequiredApprovalInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackendId;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigScanner;
import com.google.gerrit.plugins.codeowners.backend.findowners.FindOwnersBackend;
import com.google.gerrit.plugins.codeowners.backend.proto.ProtoBackend;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
@@ -62,15 +64,19 @@
@Inject private CurrentUser currentUser;
+ private CodeOwnerConfigOperations codeOwnerConfigOperations;
private CodeOwnerProjectConfigJson codeOwnerProjectConfigJson;
private FindOwnersBackend findOwnersBackend;
private ProtoBackend protoBackend;
@Before
public void setUpCodeOwnersPlugin() throws Exception {
+ codeOwnerConfigOperations =
+ plugin.getSysInjector().getInstance(CodeOwnerConfigOperations.class);
codeOwnerProjectConfigJson =
new CodeOwnerProjectConfigJson(
codeOwnersPluginConfiguration,
+ plugin.getSysInjector().getInstance(CodeOwnerConfigScanner.class),
plugin.getSysInjector().getInstance(new Key<Provider<ListBranches>>() {}));
findOwnersBackend = plugin.getSysInjector().getInstance(FindOwnersBackend.class);
protoBackend = plugin.getSysInjector().getInstance(ProtoBackend.class);
@@ -224,6 +230,14 @@
public void formatCodeOwnerBranchConfig() throws Exception {
createOwnersOverrideLabel();
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
when(codeOwnersPluginConfiguration.isDisabled(any(BranchNameKey.class))).thenReturn(false);
when(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
.thenReturn(findOwnersBackend);
@@ -256,6 +270,7 @@
assertThat(codeOwnerBranchConfigInfo.requiredApproval.value).isEqualTo(2);
assertThat(codeOwnerBranchConfigInfo.overrideApproval.label).isEqualTo("Owners-Override");
assertThat(codeOwnerBranchConfigInfo.overrideApproval.value).isEqualTo(1);
+ assertThat(codeOwnerBranchConfigInfo.noCodeOwnersDefined).isNull();
}
@Test
@@ -269,6 +284,33 @@
assertThat(codeOwnerBranchConfigInfo.backendId).isNull();
assertThat(codeOwnerBranchConfigInfo.requiredApproval).isNull();
assertThat(codeOwnerBranchConfigInfo.overrideApproval).isNull();
+ assertThat(codeOwnerBranchConfigInfo.noCodeOwnersDefined).isNull();
+ }
+
+ @Test
+ public void formatCodeOwnerBranchConfig_bootstrappingMode() throws Exception {
+ createOwnersOverrideLabel();
+
+ when(codeOwnersPluginConfiguration.isDisabled(any(BranchNameKey.class))).thenReturn(false);
+ when(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
+ .thenReturn(findOwnersBackend);
+ when(codeOwnersPluginConfiguration.getFileExtension(project)).thenReturn(Optional.of("foo"));
+ when(codeOwnersPluginConfiguration.getOverrideInfoUrl(project))
+ .thenReturn(Optional.of("http://foo.example.com"));
+ when(codeOwnersPluginConfiguration.getMergeCommitStrategy(project))
+ .thenReturn(MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION);
+ when(codeOwnersPluginConfiguration.areImplicitApprovalsEnabled(project)).thenReturn(true);
+ when(codeOwnersPluginConfiguration.getRequiredApproval(project))
+ .thenReturn(RequiredApproval.create(getDefaultCodeReviewLabel(), (short) 2));
+ when(codeOwnersPluginConfiguration.getOverrideApproval(project))
+ .thenReturn(
+ Optional.of(
+ RequiredApproval.create(
+ LabelType.withDefaultValues("Owners-Override"), (short) 1)));
+
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ codeOwnerProjectConfigJson.format(createBranchResource("refs/heads/master"));
+ assertThat(codeOwnerBranchConfigInfo.noCodeOwnersDefined).isTrue();
}
private ProjectResource createProjectResource() {
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index d3a28e3..9afd554 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -287,7 +287,7 @@
| `o` | optional | [Account option](../../../Documentation/rest-api-accounts.html#query-options) that controls which fields in the returned accounts should be populated. Can be specified multiple times. If not given, only the `_account_id` field for the account ID is populated.
| `O` | optional | [Account option](../../../Documentation/rest-api-accounts.html#query-options) in hex. For the explanation see `o` parameter.
| `limit`\|`n` | optional | Limit defining how many code owners should be returned at most. By default 10.
-| `revision` | optional | Revision from which the code owner configs should be read as commit SHA1. Can be used to read historic code owners. If not specified the code owner configs are read from the HEAD revision of the branch. Not supported for getting code owners for a path in a change.
+| `revision` | optional | Revision from which the code owner configs should be read as commit SHA1. Can be used to read historic code owners from this branch, but imports from other branches or repositories as well as global code owners from `refs/meta/config` are still read from the current revisions. If not specified the code owner configs are read from the HEAD revision of the branch. Not supported for getting code owners for a path in a change.
As a response a list of [CodeOwnerInfo](#code-owner-info) entities is returned.
The returned code owners are sorted by an internal score that expresses how good
@@ -531,6 +531,7 @@
| `backend_id`| optional | ID of the code owner backend that is configured for the branch. Not set if `disabled` is `true`.
| `required_approval` | optional | The approval that is required from code owners to approve the files in a change as [RequiredApprovalInfo](#required-approval-info) entity. The required approval defines which approval counts as code owner approval. Not set if `disabled` is `true`.
| `override_approval` | optional | The approval that is required to override the code owners submit check as [RequiredApprovalInfo](#required-approval-info) entity. If unset, overriding the code owners submit check is disabled. Not set if `disabled` is `true`.
+| `no_code_owners_defined` | optional | Whether the branch doesn't contain any code owner config file yet. If a branch doesn't contain any code owner config file yet, the projects owners are considered as code owners. Once a first code owner config file is added to the branch, the project owners are no longer code owners (unless code ownership is granted to them via the code owner config file). Not set if `false` or if `disabled` is `true`.
---
diff --git a/resources/Documentation/user-guide.md b/resources/Documentation/user-guide.md
index 43badb0..8e98f59 100644
--- a/resources/Documentation/user-guide.md
+++ b/resources/Documentation/user-guide.md
@@ -136,6 +136,22 @@
The smaller the distance the better we consider the code owner as
reviewer/approver for the path.
+## <a id="noCodeOwnersDefined">How to submit changes with files that have no code owners?
+
+If the code owners functionality is enabled, all touched files require an
+approval from a code owner. If files are touched for which no code owners are
+defined, the change can only be submitted with a [code owner
+override](#codeOwnerOverride).
+
+If the destination branch doesn't contain any [code owner config
+file](#codeOwnerConfigFiles) at all yet, the project owners are considered as
+code owners and can grant [code owner approvals](#codeOwnerApproval) for all
+files. This is to allow bootstrapping code owners and should be only a temporary
+state until the first [code owner config file](#codeOwnerConfigFiles) is added.
+Please note that the [code owner suggestion](#codeOwnerSuggestion) isn't working
+if no code owners are defined yet (project owners will not be suggested in this
+case).
+
## <a id="renames">Renames
When files/folders get renamed, their code owner configuration should stay
