Generate a warning when new rules.pl files are uploaded

With change I26236f3f we added a commit validator to disallow uploading
new prolog rules (rules.pl files). Modifications or deletions of
rules.pl files were still allowed. ESC decided to relax this though and
allow uploading new rules.pl files but generate a warning message.

Release-Notes: Emit a warning message if new rules.pl files are uploaded
Change-Id: I3d7eec2dcc74b1cc9abd40c8f45d1f382c24ca28
(cherry picked from commit 21bba3805b99ed068db19c23aa2fb88782803a39)

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