diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java index da64332b6e..d8beddce0c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java @@ -65,36 +65,7 @@ public enum CommitMergeStatus { /** */ NOT_FAST_FORWARD("Project policy requires all submissions to be a fast-forward.\n" + "\n" - + "Please rebase the change locally and upload again for review."), - - /** */ - INVALID_PROJECT_CONFIGURATION("Change contains an invalid project configuration."), - - /** */ - INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED( - "Change contains an invalid project configuration:\n" - + "One of the plugin configuration parameters has a value that is not permitted."), - - /** */ - INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_EDITABLE( - "Change contains an invalid project configuration:\n" - + "One of the plugin configuration parameters is not editable."), - - /** */ - INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND( - "Change contains an invalid project configuration:\n" - + "Parent project does not exist."), - - /** */ - INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT( - "Change contains an invalid project configuration:\n" - + "The root project cannot have a parent."), - - /** */ - SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN( - "Change contains a project configuration that changes the parent project.\n" - + "The change must be submitted by a Gerrit administrator."); - + + "Please rebase the change locally and upload again for review."); private String message; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index 414e2e832a..eeeb729bf2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -692,8 +692,7 @@ public class MergeOp implements AutoCloseable { mergeValidators.validatePreMerge( or.repo, commit, or.project, destBranch, ps.getId()); } catch (MergeValidationException mve) { - commit.setStatusCode(mve.getStatus()); - problems.put(changeId, mve.getStatus().toString()); + problems.put(changeId, mve.getMessage()); continue; } @@ -881,12 +880,6 @@ public class MergeOp implements AutoCloseable { case MANUAL_RECURSIVE_MERGE: case CANNOT_CHERRY_PICK_ROOT: case NOT_FAST_FORWARD: - case INVALID_PROJECT_CONFIGURATION: - case INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED: - case INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_EDITABLE: - case INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND: - case INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT: - case SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN: // TODO(dborowitz): Reformat these messages to be more appropriate for // short problem descriptions. problems.put(id, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationException.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationException.java index bfad0e3932..018ec1bc20 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationException.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationException.java @@ -14,19 +14,18 @@ package com.google.gerrit.server.git.validators; -import com.google.gerrit.server.git.CommitMergeStatus; import com.google.gerrit.server.validators.ValidationException; +/** + * Exception that occurs during a validation step before merging changes. + *

+ * Used by {@link MergeValidationListener}s provided by plugins. Messages should + * be considered human-readable. + */ public class MergeValidationException extends ValidationException { private static final long serialVersionUID = 1L; - private final CommitMergeStatus status; - public MergeValidationException(CommitMergeStatus status) { - super(status.toString()); - this.status = status; - } - - public CommitMergeStatus getStatus() { - return status; + public MergeValidationException(String msg) { + super(msg); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java index 6f70d46c75..e6585370ba 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java @@ -30,7 +30,6 @@ import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.ProjectConfigEntry; import com.google.gerrit.server.git.CodeReviewCommit; -import com.google.gerrit.server.git.CommitMergeStatus; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; @@ -75,6 +74,26 @@ public class MergeValidators { public static class ProjectConfigValidator implements MergeValidationListener { + private static final String INVALID_CONFIG = + "Change contains an invalid project configuration."; + private static final String PARENT_NOT_FOUND = + "Change contains an invalid project configuration:\n" + + "Parent project does not exist."; + private static final String PLUGIN_VALUE_NOT_EDITABLE = + "Change contains an invalid project configuration:\n" + + "One of the plugin configuration parameters is not editable."; + private static final String PLUGIN_VALUE_NOT_PERMITTED = + "Change contains an invalid project configuration:\n" + + "One of the plugin configuration parameters has a value that is not" + + " permitted."; + private static final String ROOT_NO_PARENT = + "Change contains an invalid project configuration:\n" + + "The root project cannot have a parent."; + private static final String SET_BY_ADMIN = + "Change contains a project configuration that changes the parent" + + " project.\n" + + "The change must be submitted by a Gerrit administrator."; + private final AllProjectsName allProjectsName; private final ReviewDb db; private final ProjectCache projectCache; @@ -119,27 +138,23 @@ public class MergeValidators { if (oldParent == null) { // update of the 'All-Projects' project if (newParent != null) { - throw new MergeValidationException(CommitMergeStatus. - INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT); + throw new MergeValidationException(ROOT_NO_PARENT); } } else { if (!oldParent.equals(newParent)) { PatchSetApproval psa = approvalsUtil.getSubmitter(db, commit.notes(), patchSetId); if (psa == null) { - throw new MergeValidationException(CommitMergeStatus. - SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN); + throw new MergeValidationException(SET_BY_ADMIN); } final IdentifiedUser submitter = identifiedUserFactory.create(psa.getAccountId()); if (!submitter.getCapabilities().canAdministrateServer()) { - throw new MergeValidationException(CommitMergeStatus. - SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN); + throw new MergeValidationException(SET_BY_ADMIN); } if (projectCache.get(newParent) == null) { - throw new MergeValidationException(CommitMergeStatus. - INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND); + throw new MergeValidationException(PARENT_NOT_FOUND); } } } @@ -155,19 +170,16 @@ public class MergeValidators { if ((value == null ? oldValue != null : !value.equals(oldValue)) && !configEntry.isEditable(destProject)) { - throw new MergeValidationException(CommitMergeStatus. - INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_EDITABLE); + throw new MergeValidationException(PLUGIN_VALUE_NOT_EDITABLE); } if (ProjectConfigEntry.Type.LIST.equals(configEntry.getType()) && value != null && !configEntry.getPermittedValues().contains(value)) { - throw new MergeValidationException(CommitMergeStatus. - INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED); + throw new MergeValidationException(PLUGIN_VALUE_NOT_PERMITTED); } } } catch (ConfigInvalidException | IOException e) { - throw new MergeValidationException(CommitMergeStatus. - INVALID_PROJECT_CONFIGURATION); + throw new MergeValidationException(INVALID_CONFIG); } } } diff --git a/plugins/cookbook-plugin b/plugins/cookbook-plugin index eea84e7e07..88b0984638 160000 --- a/plugins/cookbook-plugin +++ b/plugins/cookbook-plugin @@ -1 +1 @@ -Subproject commit eea84e7e07ecf6ebb70ea5a6b0cde67f5a5576af +Subproject commit 88b0984638857ac7139de83b18dc7cad23670b4d