Merge "Generate a warning when new rules.pl files are uploaded" into stable-3.8

3 files changed, 17 insertions, 14 deletions

diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 46e09fd518..eee1c835bd 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -190,7 +190,7 @@ import com.google.gerrit.server.project.CommentLinkProvider;
 import com.google.gerrit.server.project.ProjectCacheImpl;
 import com.google.gerrit.server.project.ProjectNameLockManager;
 import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.PrologRulesBlockerValidator;
+import com.google.gerrit.server.project.PrologRulesWarningValidator;
 import com.google.gerrit.server.project.SubmitRequirementConfigValidator;
 import com.google.gerrit.server.project.SubmitRequirementsEvaluatorImpl;
 import com.google.gerrit.server.project.SubmitRuleEvaluator;
@@ -402,7 +402,7 @@ public class GerritGlobalModule extends FactoryModule {
     DynamicSet.setOf(binder(), CommitValidationListener.class);
     DynamicSet.bind(binder(), CommitValidationListener.class)
         .to(SubmitRequirementConfigValidator.class);
-    DynamicSet.bind(binder(), CommitValidationListener.class).to(PrologRulesBlockerValidator.class);
+    DynamicSet.bind(binder(), CommitValidationListener.class).to(PrologRulesWarningValidator.class);
     DynamicSet.setOf(binder(), CommentValidator.class);
     DynamicSet.setOf(binder(), ChangeMessageModifier.class);
     DynamicSet.setOf(binder(), RefOperationValidationListener.class);
diff --git a/java/com/google/gerrit/server/project/PrologRulesBlockerValidator.java b/java/com/google/gerrit/server/project/PrologRulesWarningValidator.java
index d4af2b2565..5683fe7974 100644
--- a/java/com/google/gerrit/server/project/PrologRulesBlockerValidator.java
+++ b/java/com/google/gerrit/server/project/PrologRulesWarningValidator.java
@@ -24,6 +24,7 @@ import com.google.gerrit.server.events.CommitReceivedEvent;
 import com.google.gerrit.server.git.validators.CommitValidationException;
 import com.google.gerrit.server.git.validators.CommitValidationListener;
 import com.google.gerrit.server.git.validators.CommitValidationMessage;
+import com.google.gerrit.server.git.validators.ValidationMessage;
 import com.google.gerrit.server.patch.DiffNotAvailableException;
 import com.google.gerrit.server.patch.DiffOperations;
 import com.google.gerrit.server.patch.DiffOptions;
@@ -35,17 +36,17 @@ import java.util.Map;
 import java.util.stream.Collectors;
 
 /**
- * A validator than bans creation of new Prolog rules. Modification and deletion will be allowed so
- * that clients can get rid of prolog rules. New rules should use submit-requirements instead.
+ * A validator than emits a warning for newly added prolog rules file via git push. Modification and
+ * deletion are allowed so that clients can get rid of prolog rules.
  */
 @Singleton
-public class PrologRulesBlockerValidator implements CommitValidationListener {
+public class PrologRulesWarningValidator implements CommitValidationListener {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
   private final DiffOperations diffOperations;
 
   @Inject
-  public PrologRulesBlockerValidator(DiffOperations diffOperations) {
+  public PrologRulesWarningValidator(DiffOperations diffOperations) {
     this.diffOperations = diffOperations;
   }
 
@@ -55,12 +56,11 @@ public class PrologRulesBlockerValidator implements CommitValidationListener {
     try {
       if (receiveEvent.refName.equals(RefNames.REFS_CONFIG)
           && isFileAdded(receiveEvent, RULES_PL_FILE)) {
-        throw new CommitValidationException(
-            "Uploading 'rule.pl' not allowed",
+        return ImmutableList.of(
             new CommitValidationMessage(
-                "Uploading a new 'rules.pl' file is not allowed."
-                    + " Please add submit-requirements instead.",
-                /*isError= */ true));
+                "Uploading a new 'rules.pl' file is discouraged."
+                    + " Please consider adding submit-requirements instead.",
+                ValidationMessage.Type.WARNING));
       }
     } catch (DiffNotAvailableException e) {
       logger.atWarning().withCause(e).log("Failed to retrieve the file diff.");
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 118b31b9d9..29ea7f45ce 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -159,6 +159,7 @@ import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.git.ObjectIds;
 import com.google.gerrit.httpd.raw.IndexPreloadingUtil;
 import com.google.gerrit.index.IndexConfig;
 import com.google.gerrit.index.query.PostFilterPredicate;
@@ -3607,10 +3608,12 @@ public class ChangeIT extends AbstractDaemonTest {
                     + "add_non_author_approval(S1,"
                     + " [label('Non-Author-Code-Review', need(_)) | S1]).")
             .to(RefNames.REFS_CONFIG);
-    pushResult.assertErrorStatus();
+    pushResult.assertOkStatus();
     pushResult.assertMessage(
-        "Uploading a new 'rules.pl' file is not allowed."
-            + " Please add submit-requirements instead.");
+        String.format(
+            "WARNING: commit %s: Uploading a new 'rules.pl' file is discouraged. "
+                + "Please consider adding submit-requirements instead.",
+            ObjectIds.abbreviateName(pushResult.getCommit())));
   }
 
   @Test