Fixes #198: Enhance rule checking in rule managers and Sentinel Dashboard (#202)

- Enhance rule checking in rule managers and Sentinel Dashboard frontend
- Enhance error information when reporting invalid fields in rule dialog

Signed-off-by: Eric Zhao <sczyh16@gmail.com>

sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/authority/AuthorityRuleManager.java

@@ -151,6 +151,7 @@ public final class AuthorityRuleManager {
     }
 
     static boolean isValidRule(AuthorityRule rule) {
-        return rule != null && !StringUtil.isBlank(rule.getResource());
+        return rule != null && !StringUtil.isBlank(rule.getResource())
+            && rule.getStrategy() >= 0 && StringUtil.isNotBlank(rule.getLimitApp());
     }
 }

sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeRule.java

@@ -59,6 +59,12 @@ public class DegradeRule extends AbstractRule {
     private static ScheduledExecutorService pool = Executors.newScheduledThreadPool(
         Runtime.getRuntime().availableProcessors(), new NamedThreadFactory("sentinel-degrade-reset-task", true));
 
+    public DegradeRule() {}
+
+    public DegradeRule(String resourceName) {
+        setResource(resourceName);
+    }
+
     /**
      * RT threshold or exception ratio threshold count.
      */
@@ -80,8 +86,9 @@ public class DegradeRule extends AbstractRule {
         return grade;
     }
 
-    public void setGrade(int grade) {
+    public DegradeRule setGrade(int grade) {
         this.grade = grade;
+        return this;
     }
 
     private AtomicLong passCount = new AtomicLong(0);
@@ -92,8 +99,9 @@ public class DegradeRule extends AbstractRule {
         return count;
     }
 
-    public void setCount(double count) {
+    public DegradeRule setCount(double count) {
         this.count = count;
+        return this;
     }
 
     public boolean isCut() {
@@ -112,8 +120,9 @@ public class DegradeRule extends AbstractRule {
         return timeWindow;
     }
 
-    public void setTimeWindow(int timeWindow) {
+    public DegradeRule setTimeWindow(int timeWindow) {
         this.timeWindow = timeWindow;
+        return this;
     }
 
     @Override

sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeRuleManager.java

@@ -167,7 +167,16 @@ public class DegradeRuleManager {
 
     }
 
-    private static boolean isValidRule(DegradeRule rule) {
-        return rule != null && !StringUtil.isBlank(rule.getResource()) && rule.getCount() >= 0;
+    public static boolean isValidRule(DegradeRule rule) {
+        boolean baseValid = rule != null && !StringUtil.isBlank(rule.getResource())
+            && rule.getCount() >= 0 && rule.getTimeWindow() > 0;
+        if (!baseValid) {
+            return false;
+        }
+        // Check exception ratio mode.
+        if (rule.getGrade() == RuleConstant.DEGRADE_GRADE_EXCEPTION_RATIO && rule.getCount() > 1) {
+            return false;
+        }
+        return true;
     }
 }

sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleManager.java

@@ -202,7 +202,31 @@ public class FlowRuleManager {
 
     }
 
-    private static boolean isValidRule(FlowRule rule) {
-        return rule != null && !StringUtil.isBlank(rule.getResource());
+    public static boolean isValidRule(FlowRule rule) {
+        boolean baseValid = rule != null && !StringUtil.isBlank(rule.getResource()) && rule.getCount() >= 0
+            && rule.getGrade() >= 0 && rule.getStrategy() >= 0 && rule.getControlBehavior() >= 0;
+        if (!baseValid) {
+            return false;
+        }
+        // Check strategy and control (shaping) behavior.
+        return checkStrategyField(rule) && checkControlBehaviorField(rule);
+    }
+
+    private static boolean checkStrategyField(/*@NonNull*/ FlowRule rule) {
+        if (rule.getStrategy() == RuleConstant.STRATEGY_RELATE || rule.getStrategy() == RuleConstant.STRATEGY_CHAIN) {
+            return StringUtil.isNotBlank(rule.getRefResource());
+        }
+        return true;
+    }
+
+    private static boolean checkControlBehaviorField(/*@NonNull*/ FlowRule rule) {
+        switch (rule.getControlBehavior()) {
+            case RuleConstant.CONTROL_BEHAVIOR_WARM_UP:
+                return rule.getWarmUpPeriodSec() > 0;
+            case RuleConstant.CONTROL_BEHAVIOR_RATE_LIMITER:
+                return rule.getMaxQueueingTimeMs() > 0;
+            default:
+                return true;
+        }
     }
 }

sentinel-core/src/test/java/com/alibaba/csp/sentinel/slots/block/authority/AuthorityRuleManagerTest.java

@@ -47,10 +47,13 @@ public class AuthorityRuleManagerTest {
         AuthorityRule ruleB = null;
         AuthorityRule ruleC = new AuthorityRule();
         ruleC.setResource("abc");
+        AuthorityRule ruleD = new AuthorityRule();
+        ruleD.setResource("bcd").setLimitApp("abc");
 
         assertFalse(AuthorityRuleManager.isValidRule(ruleA));
         assertFalse(AuthorityRuleManager.isValidRule(ruleB));
-        assertTrue(AuthorityRuleManager.isValidRule(ruleC));
+        assertFalse(AuthorityRuleManager.isValidRule(ruleC));
+        assertTrue(AuthorityRuleManager.isValidRule(ruleD));
     }
 
     @After

sentinel-core/src/test/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeRuleManagerTest.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright 1999-2018 Alibaba Group Holding Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.alibaba.csp.sentinel.slots.block.degrade;
+
+import com.alibaba.csp.sentinel.slots.block.RuleConstant;
+
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+/**
+ * Test cases for {@link DegradeRuleManager}.
+ *
+ * @author Eric Zhao
+ */
+public class DegradeRuleManagerTest {
+
+    @Test
+    public void testIsValidRule() {
+        DegradeRule rule1 = new DegradeRule("abc");
+        DegradeRule rule2 = new DegradeRule("cde")
+            .setCount(100)
+            .setGrade(RuleConstant.DEGRADE_GRADE_RT)
+            .setTimeWindow(-1);
+        DegradeRule rule3 = new DegradeRule("xx")
+            .setCount(1.1)
+            .setGrade(RuleConstant.DEGRADE_GRADE_EXCEPTION_RATIO)
+            .setTimeWindow(2);
+        DegradeRule rule4 = new DegradeRule("yy")
+            .setCount(-3)
+            .setGrade(RuleConstant.DEGRADE_GRADE_EXCEPTION_COUNT)
+            .setTimeWindow(2);
+        assertFalse(DegradeRuleManager.isValidRule(rule1));
+        assertFalse(DegradeRuleManager.isValidRule(rule2));
+        assertFalse(DegradeRuleManager.isValidRule(rule3));
+        assertTrue(DegradeRuleManager.isValidRule(rule3.setCount(1.0d)));
+        assertTrue(DegradeRuleManager.isValidRule(rule3.setCount(0.0d)));
+        assertFalse(DegradeRuleManager.isValidRule(rule4));
+    }
+}

sentinel-dashboard/src/main/webapp/resources/app/scripts/controllers/degrade.js

@@ -81,13 +81,41 @@ app.controller('DegradeCtl', ['$scope', '$stateParams', 'DegradeService', 'ngDia
       });
     };
 
+      function checkRuleValid(rule) {
+          if (rule.resource === undefined || rule.resource === '') {
+              alert('资源名称不能为空');
+              return false;
+          }
+          if (rule.grade === undefined || rule.grade < 0) {
+              alert('未知的降级类型');
+              return false;
+          }
+          if (rule.count === undefined || rule.count === '' || rule.count < 0) {
+              alert('降级阈值不能为空或小于 0');
+              return false;
+          }
+          if (rule.timeWindow === undefined || rule.timeWindow === '' || rule.timeWindow <= 0) {
+              alert('降级时间窗口必须大于 0');
+              return false;
+          }
+          // 异常比率类型.
+          if (rule.grade == 1 && rule.count > 1) {
+              alert('异常比率超出范围：[0.0 - 1.0]');
+              return false;
+          }
+          return true;
+      }
+
     $scope.saveRule = function () {
-      if ($scope.degradeRuleDialog.type == 'add') {
+      if (!checkRuleValid($scope.currentRule)) {
+        return;
+      }
+      if ($scope.degradeRuleDialog.type === 'add') {
         addNewRule($scope.currentRule);
-      } else if ($scope.degradeRuleDialog.type == 'edit') {
+      } else if ($scope.degradeRuleDialog.type === 'edit') {
         saveRule($scope.currentRule, true);
       }
-    }
+    };
 
     var confirmDialog;
     $scope.deleteRule = function (rule) {

sentinel-dashboard/src/main/webapp/resources/app/scripts/controllers/flow.js

@@ -88,13 +88,58 @@ app.controller('FlowCtl', ['$scope', '$stateParams', 'FlowService', 'ngDialog',
       });
     };
 
+    function notNumberAtLeastZero(num) {
+      return num === undefined || num === '' || isNaN(num) || num < 0;
+    }
+
+    function notNumberGreaterThanZero(num) {
+      return num === undefined || num === '' || isNaN(num) || num <= 0;
+    }
+
+    function checkRuleValid(rule) {
+          if (rule.resource === undefined || rule.resource === '') {
+              alert('资源名称不能为空');
+              return false;
+          }
+          if (rule.count === undefined || rule.count < 0) {
+              alert('限流阈值必须大于等于 0');
+              return false;
+          }
+          if (rule.strategy === undefined || rule.strategy < 0) {
+              alert('无效的流控模式');
+              return false;
+          }
+          if (rule.strategy == 1 || rule.strategy == 2) {
+            if (rule.refResource === undefined || rule.refResource === '') {
+                alert('请填写关联资源或入口');
+                return false;
+            }
+          }
+        if (rule.controlBehavior === undefined || rule.controlBehavior < 0) {
+            alert('无效的流控整形方式');
+            return false;
+        }
+        if (rule.controlBehavior == 1 && notNumberGreaterThanZero(rule.warmUpPeriodSec)) {
+            alert('预热时长必须大于 0');
+            return false;
+        }
+        if (rule.controlBehavior == 2 && notNumberGreaterThanZero(rule.maxQueueingTimeMs)) {
+            alert('排队超时时间必须大于 0');
+            return false;
+        }
+        return true;
+    }
+
     $scope.saveRule = function () {
-      if ($scope.flowRuleDialog.type == 'add') {
+      if (!checkRuleValid($scope.currentRule)) {
+        return;
+      }
+      if ($scope.flowRuleDialog.type === 'add') {
         addNewRule($scope.currentRule);
-      } else if ($scope.flowRuleDialog.type == 'edit') {
+      } else if ($scope.flowRuleDialog.type === 'edit') {
         saveRule($scope.currentRule, true);
       }
-    }
+    };
 
     var confirmDialog;
     $scope.deleteRule = function (rule) {
@@ -115,7 +160,7 @@ app.controller('FlowCtl', ['$scope', '$stateParams', 'FlowService', 'ngDialog',
     };
 
     $scope.confirm = function () {
-      if ($scope.confirmDialog.type == 'delete_rule') {
+      if ($scope.confirmDialog.type === 'delete_rule') {
         deleteRule($scope.currentRule);
       } else {
         console.error('error');

sentinel-dashboard/src/main/webapp/resources/app/views/dialog/degrade-rule-dialog.html

@@ -42,7 +42,7 @@
 						</div>
 						<label class="col-sm-2 control-label">时间窗口</label>
 						<div class="col-sm-4">
-							<input type='number' class="form-control highlight-border" ng-model='currentRule.timeWindow' placeholder="降级时间间隔, 单位秒" />
+							<input type='number' min="0" class="form-control highlight-border" ng-model='currentRule.timeWindow' placeholder="降级时间间隔, 单位秒" />
 						</div>
 					</div>
 				</form>

sentinel-dashboard/src/main/webapp/resources/app/views/dialog/flow-rule-dialog.html

@@ -15,7 +15,7 @@
           </div>
 
           <div class="form-group">
-            <label class="col-sm-2 control-label">流控应用</label>
+            <label class="col-sm-2 control-label" title="流控针对应用，即流量入口的调用来源（origin）">流控应用</label>
             <div class="col-sm-9">
               <input type="text" class="form-control highlight-border" ng-model='currentRule.limitApp' placeholder='指调用方，"default"表示所有应用。'
               />
@@ -32,7 +32,7 @@
             </div>
             <label class="col-sm-2 control-label">单机阈值</label>
             <div class="col-sm-3">
-              <input type='number' class="form-control highlight-border" ng-model='currentRule.count' placeholder="单机阈值" />
+              <input type='number' min="0" class="form-control highlight-border" ng-model='currentRule.count' placeholder="单机阈值" />
             </div>
           </div>