Allow to define and configure override label in the same commit
When a code-owners.config file is pushed we validate the override
configuration in it. This includes checking if the configured label
exists for the project. If the label didn't exist yet, but was
configured in the commit that is being pushed, we didn't take it into
account and wrongly rejected the commit, saying that the label doesn't
exist (because we read the labels from the head revision of
refs/meta/config and not from the commit that was being pushed). This is
fixed now, by reading the available labels from the commit that is being
pushed.
Change-Id: Ifecaca554605c123c62c347b54cb873f7cc7e09d
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfigValidator.java
index 2a29766..8f20fc1 100644
--- a/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfigValidator.java
@@ -14,8 +14,6 @@
package com.google.gerrit.plugins.codeowners.config;
-import static com.google.gerrit.server.project.ProjectCache.illegalState;
-
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
@@ -30,7 +28,7 @@
import com.google.gerrit.server.git.validators.CommitValidationMessage;
import com.google.gerrit.server.git.validators.ValidationMessage;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
-import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectLevelConfig;
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
@@ -41,6 +39,7 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
/** Validates modifications to the {@code code-owners.config} file in {@code refs/meta/config}. */
@Singleton
@@ -48,8 +47,9 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final String pluginName;
- private final ProjectCache projectCache;
private final GitRepositoryManager repoManager;
+ private final ProjectConfig.Factory projectConfigFactory;
+ private final ProjectState.Factory projectStateFactory;
private final ChangedFiles changedFiles;
private final GeneralConfig generalConfig;
private final StatusConfig statusConfig;
@@ -60,8 +60,9 @@
@Inject
CodeOwnersPluginConfigValidator(
@PluginName String pluginName,
- ProjectCache projectCache,
GitRepositoryManager repoManager,
+ ProjectConfig.Factory projectConfigFactory,
+ ProjectState.Factory projectStateFactory,
ChangedFiles changedFiles,
GeneralConfig generalConfig,
StatusConfig statusConfig,
@@ -69,8 +70,9 @@
RequiredApprovalConfig requiredApprovalConfig,
OverrideApprovalConfig overrideApprovalConfig) {
this.pluginName = pluginName;
- this.projectCache = projectCache;
this.repoManager = repoManager;
+ this.projectConfigFactory = projectConfigFactory;
+ this.projectStateFactory = projectStateFactory;
this.changedFiles = changedFiles;
this.generalConfig = generalConfig;
this.statusConfig = statusConfig;
@@ -93,11 +95,11 @@
return ImmutableList.of();
}
- ProjectState projectState = projectCache.get(project).orElseThrow(illegalState(project));
+ ProjectState projectState = getProjectState(project, receiveEvent.commit);
ProjectLevelConfig.Bare cfg = loadConfig(project, fileName, receiveEvent.commit);
validateConfig(projectState, fileName, cfg);
return ImmutableList.of();
- } catch (IOException | PatchListNotAvailableException e) {
+ } catch (IOException | ConfigInvalidException | PatchListNotAvailableException e) {
String errorMessage =
String.format(
"failed to validate file %s for revision %s in ref %s of project %s",
@@ -107,6 +109,15 @@
}
}
+ private ProjectState getProjectState(Project.NameKey projectName, RevCommit commit)
+ throws IOException, ConfigInvalidException {
+ try (Repository repo = repoManager.openRepository(projectName)) {
+ ProjectConfig projectConfig = projectConfigFactory.create(projectName);
+ projectConfig.load(repo, commit);
+ return projectStateFactory.create(projectConfig.getCacheable());
+ }
+ }
+
/**
* Whether the given file was changed in the given revision.
*
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersPluginConfigValidatorIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersPluginConfigValidatorIT.java
index 93504a7..1ea4183 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersPluginConfigValidatorIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersPluginConfigValidatorIT.java
@@ -37,8 +37,10 @@
import com.google.gerrit.plugins.codeowners.config.StatusConfig;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
+import org.eclipse.jgit.util.RawParseUtils;
import org.junit.Before;
import org.junit.Test;
@@ -369,6 +371,52 @@
}
@Test
+ public void defineAndConfigureOverrideLabelInSameCommit() throws Exception {
+ fetchRefsMetaConfig();
+
+ RevCommit head = getHead(testRepo.getRepository(), RefNames.REFS_CONFIG);
+ RevObject blob = testRepo.get(head.getTree(), "project.config");
+ byte[] data = testRepo.getRepository().open(blob).getCachedBytes(Integer.MAX_VALUE);
+ String projectConfigText = RawParseUtils.decode(data);
+
+ Config projectConfig = new Config();
+ projectConfig.fromText(projectConfigText);
+ String labelName = "Owners-Override";
+ projectConfig.setString("label", labelName, "function", "NoOp");
+ projectConfig.setStringList(
+ "label", labelName, "value", ImmutableList.of("0 Not Override", "+1 Override"));
+
+ Config codeOwnersConfig = new Config();
+ codeOwnersConfig.setString(
+ CodeOwnersPluginConfiguration.SECTION_CODE_OWNERS,
+ /* subsection= */ null,
+ OverrideApprovalConfig.KEY_OVERRIDE_APPROVAL,
+ "Owners-Override+1");
+
+ RevCommit commit =
+ testRepo.update(
+ RefNames.REFS_CONFIG,
+ testRepo
+ .commit()
+ .parent(head)
+ .message("Add test code owner config")
+ .author(admin.newIdent())
+ .committer(admin.newIdent())
+ .add("code-owners.config", codeOwnersConfig.toText())
+ .add("project.config", projectConfig.toText()));
+
+ testRepo.reset(commit);
+
+ PushResult r = pushRefsMetaConfig();
+ assertThat(r.getRemoteUpdate(RefNames.REFS_CONFIG).getStatus()).isEqualTo(Status.OK);
+ ImmutableSet<RequiredApproval> overrideApproval =
+ codeOwnersPluginConfiguration.getOverrideApproval(project);
+ assertThat(overrideApproval).hasSize(1);
+ assertThat(overrideApproval).element(0).hasLabelNameThat().isEqualTo("Owners-Override");
+ assertThat(overrideApproval).element(0).hasValueThat().isEqualTo(1);
+ }
+
+ @Test
public void configureMergeCommitStrategy() throws Exception {
fetchRefsMetaConfig();
