Separate checking logic from AuthorityRule and add test cases

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

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

@@ -15,13 +15,14 @@
  */
 package com.alibaba.csp.sentinel.slots.block.authority;
 
-import com.alibaba.csp.sentinel.util.StringUtil;
 import com.alibaba.csp.sentinel.context.Context;
 import com.alibaba.csp.sentinel.node.DefaultNode;
 import com.alibaba.csp.sentinel.slots.block.AbstractRule;
 import com.alibaba.csp.sentinel.slots.block.RuleConstant;
 
 /**
+ * Authority rule is designed for limiting by request origins.
+ *
  * @author youji.zj
  */
 public class AuthorityRule extends AbstractRule {
@@ -35,8 +36,9 @@ public class AuthorityRule extends AbstractRule {
         return strategy;
     }
 
-    public void setStrategy(int strategy) {
+    public AuthorityRule setStrategy(int strategy) {
         this.strategy = strategy;
+        return this;
     }
 
     @Override
@@ -59,38 +61,6 @@ public class AuthorityRule extends AbstractRule {
 
     @Override
     public boolean passCheck(Context context, DefaultNode node, int count, Object... args) {
-        String requester = context.getOrigin();
-
-        // Empty origin or empty limitApp will pass.
-        if (StringUtil.isEmpty(requester) || StringUtil.isEmpty(this.getLimitApp())) {
-            return true;
-        }
-
-        // Do exact match with origin name.
-        int pos = this.getLimitApp().indexOf(requester);
-        boolean contain = pos > -1;
-
-        if (contain) {
-            boolean exactlyMatch = false;
-            String[] appArray = this.getLimitApp().split(",");
-            for (String app : appArray) {
-                if (requester.equals(app)) {
-                    exactlyMatch = true;
-                    break;
-                }
-            }
-
-            contain = exactlyMatch;
-        }
-
-        if (strategy == RuleConstant.AUTHORITY_BLACK && contain) {
-            return false;
-        }
-
-        if (strategy == RuleConstant.AUTHORITY_WHITE && !contain) {
-            return false;
-        }
-
         return true;
     }
 

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

@@ -0,0 +1,68 @@
+/*
+ * 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.authority;
+
+import com.alibaba.csp.sentinel.context.Context;
+import com.alibaba.csp.sentinel.slots.block.RuleConstant;
+import com.alibaba.csp.sentinel.util.StringUtil;
+
+/**
+ * Rule checker for white/black list authority.
+ *
+ * @author Eric Zhao
+ * @since 0.2.0
+ */
+final class AuthorityRuleChecker {
+
+    static boolean passCheck(AuthorityRule rule, Context context) {
+        String requester = context.getOrigin();
+
+        // Empty origin or empty limitApp will pass.
+        if (StringUtil.isEmpty(requester) || StringUtil.isEmpty(rule.getLimitApp())) {
+            return true;
+        }
+
+        // Do exact match with origin name.
+        int pos = rule.getLimitApp().indexOf(requester);
+        boolean contain = pos > -1;
+
+        if (contain) {
+            boolean exactlyMatch = false;
+            String[] appArray = rule.getLimitApp().split(",");
+            for (String app : appArray) {
+                if (requester.equals(app)) {
+                    exactlyMatch = true;
+                    break;
+                }
+            }
+
+            contain = exactlyMatch;
+        }
+
+        int strategy = rule.getStrategy();
+        if (strategy == RuleConstant.AUTHORITY_BLACK && contain) {
+            return false;
+        }
+
+        if (strategy == RuleConstant.AUTHORITY_WHITE && !contain) {
+            return false;
+        }
+
+        return true;
+    }
+
+    private AuthorityRuleChecker() {}
+}

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

@@ -22,13 +22,9 @@ import java.util.concurrent.ConcurrentHashMap;
 
 import com.alibaba.csp.sentinel.log.RecordLog;
 import com.alibaba.csp.sentinel.util.StringUtil;
-import com.alibaba.csp.sentinel.context.Context;
-import com.alibaba.csp.sentinel.node.DefaultNode;
 import com.alibaba.csp.sentinel.property.DynamicSentinelProperty;
 import com.alibaba.csp.sentinel.property.PropertyListener;
 import com.alibaba.csp.sentinel.property.SentinelProperty;
-import com.alibaba.csp.sentinel.slotchain.ResourceWrapper;
-import com.alibaba.csp.sentinel.slots.block.BlockException;
 import com.alibaba.csp.sentinel.slots.block.flow.FlowRule;
 
 /**
@@ -38,7 +34,7 @@ import com.alibaba.csp.sentinel.slots.block.flow.FlowRule;
  * @author jialiang.linjl
  * @author Eric Zhao
  */
-public class AuthorityRuleManager {
+public final class AuthorityRuleManager {
 
     private static Map<String, List<AuthorityRule>> authorityRules
         = new ConcurrentHashMap<String, List<AuthorityRule>>();
@@ -59,6 +55,7 @@ public class AuthorityRuleManager {
             }
             property.addListener(listener);
             currentProperty = property;
+            RecordLog.info("[AuthorityRuleManager] Registering new property to authority rule manager");
         }
     }
 
@@ -71,24 +68,6 @@ public class AuthorityRuleManager {
         currentProperty.updateValue(rules);
     }
 
-    public static void checkAuthority(ResourceWrapper resource, Context context, DefaultNode node, int count)
-        throws BlockException {
-        if (authorityRules == null) {
-            return;
-        }
-
-        List<AuthorityRule> rules = authorityRules.get(resource.getName());
-        if (rules == null) {
-            return;
-        }
-
-        for (AuthorityRule rule : rules) {
-            if (!rule.passCheck(context, node, count)) {
-                throw new AuthorityException(context.getOrigin());
-            }
-        }
-    }
-
     public static boolean hasConfig(String resource) {
         return authorityRules.containsKey(resource);
     }
@@ -123,11 +102,18 @@ public class AuthorityRuleManager {
         }
 
         private Map<String, List<AuthorityRule>> loadAuthorityConf(List<AuthorityRule> list) {
-            if (list == null) {
-                return null;
-            }
             Map<String, List<AuthorityRule>> newRuleMap = new ConcurrentHashMap<String, List<AuthorityRule>>();
+
+            if (list == null || list.isEmpty()) {
+                return newRuleMap;
+            }
+
             for (AuthorityRule rule : list) {
+                if (!isValidRule(rule)) {
+                    RecordLog.warn("[AuthorityRuleManager] Ignoring invalid authority rule when loading new rules: " + rule);
+                    continue;
+                }
+
                 if (StringUtil.isBlank(rule.getLimitApp())) {
                     rule.setLimitApp(FlowRule.LIMIT_APP_DEFAULT);
                 }
@@ -160,4 +146,11 @@ public class AuthorityRuleManager {
         }
     }
 
+    static Map<String, List<AuthorityRule>> getAuthorityRules() {
+        return authorityRules;
+    }
+
+    static boolean isValidRule(AuthorityRule rule) {
+        return rule != null && !StringUtil.isBlank(rule.getResource());
+    }
 }

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

@@ -15,6 +15,9 @@
  */
 package com.alibaba.csp.sentinel.slots.block.authority;
 
+import java.util.List;
+import java.util.Map;
+
 import com.alibaba.csp.sentinel.context.Context;
 import com.alibaba.csp.sentinel.node.DefaultNode;
 import com.alibaba.csp.sentinel.slotchain.AbstractLinkedProcessorSlot;
@@ -25,13 +28,14 @@ import com.alibaba.csp.sentinel.slotchain.ResourceWrapper;
  * A {@link ProcessorSlot} that dedicates to {@link AuthorityRule} checking.
  *
  * @author leyou
+ * @author Eric Zhao
  */
 public class AuthoritySlot extends AbstractLinkedProcessorSlot<DefaultNode> {
 
     @Override
     public void entry(Context context, ResourceWrapper resourceWrapper, DefaultNode node, int count, Object... args)
         throws Throwable {
-        AuthorityRuleManager.checkAuthority(resourceWrapper, context, node, count);
+        checkBlackWhiteAuthority(resourceWrapper, context);
         fireEntry(context, resourceWrapper, node, count, args);
     }
 
@@ -39,4 +43,23 @@ public class AuthoritySlot extends AbstractLinkedProcessorSlot<DefaultNode> {
     public void exit(Context context, ResourceWrapper resourceWrapper, int count, Object... args) {
         fireExit(context, resourceWrapper, count, args);
     }
+
+    void checkBlackWhiteAuthority(ResourceWrapper resource, Context context) throws AuthorityException {
+        Map<String, List<AuthorityRule>> authorityRules = AuthorityRuleManager.getAuthorityRules();
+
+        if (authorityRules == null) {
+            return;
+        }
+
+        List<AuthorityRule> rules = authorityRules.get(resource.getName());
+        if (rules == null) {
+            return;
+        }
+
+        for (AuthorityRule rule : rules) {
+            if (!AuthorityRuleChecker.passCheck(rule, context)) {
+                throw new AuthorityException(context.getOrigin());
+            }
+        }
+    }
 }

sentinel-core/src/test/java/com/alibaba/csp/sentinel/TestUtil.java

@@ -0,0 +1,35 @@
+/*
+ * 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;
+
+import com.alibaba.csp.sentinel.context.Context;
+import com.alibaba.csp.sentinel.context.ContextUtil;
+
+/**
+ * @author Eric Zhao
+ */
+public final class TestUtil {
+
+    public static void cleanUpContext() {
+        Context context = ContextUtil.getContext();
+        if (context != null) {
+            context.setCurEntry(null);
+            ContextUtil.exit();
+        }
+    }
+
+    private TestUtil() {}
+}

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

@@ -0,0 +1,59 @@
+package com.alibaba.csp.sentinel.slots.block.authority;
+
+import com.alibaba.csp.sentinel.TestUtil;
+import com.alibaba.csp.sentinel.context.ContextUtil;
+import com.alibaba.csp.sentinel.slots.block.RuleConstant;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+/**
+ * Test cases for {@link AuthorityRuleChecker}.
+ *
+ * @author Eric Zhao
+ */
+public class AuthorityRuleCheckerTest {
+
+    @Before
+    public void setUp() {
+        TestUtil.cleanUpContext();
+    }
+
+    @Test
+    public void testPassCheck() {
+        String origin = "appA";
+        ContextUtil.enter("entrance", origin);
+        try {
+            String resourceName = "testPassCheck";
+            AuthorityRule ruleA = new AuthorityRule()
+                .setResource(resourceName)
+                .setLimitApp(origin + ",appB")
+                .as(AuthorityRule.class)
+                .setStrategy(RuleConstant.AUTHORITY_WHITE);
+            AuthorityRule ruleB = new AuthorityRule()
+                .setResource(resourceName)
+                .setLimitApp("appB")
+                .as(AuthorityRule.class)
+                .setStrategy(RuleConstant.AUTHORITY_WHITE);
+            AuthorityRule ruleC = new AuthorityRule()
+                .setResource(resourceName)
+                .setLimitApp(origin)
+                .as(AuthorityRule.class)
+                .setStrategy(RuleConstant.AUTHORITY_BLACK);
+            AuthorityRule ruleD = new AuthorityRule()
+                .setResource(resourceName)
+                .setLimitApp("appC")
+                .as(AuthorityRule.class)
+                .setStrategy(RuleConstant.AUTHORITY_BLACK);
+
+            assertTrue(AuthorityRuleChecker.passCheck(ruleA, ContextUtil.getContext()));
+            assertFalse(AuthorityRuleChecker.passCheck(ruleB, ContextUtil.getContext()));
+            assertFalse(AuthorityRuleChecker.passCheck(ruleC, ContextUtil.getContext()));
+            assertTrue(AuthorityRuleChecker.passCheck(ruleD, ContextUtil.getContext()));
+        } finally {
+            ContextUtil.exit();
+        }
+    }
+}

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

@@ -0,0 +1,60 @@
+package com.alibaba.csp.sentinel.slots.block.authority;
+
+import java.util.Collections;
+import java.util.List;
+
+import com.alibaba.csp.sentinel.slots.block.RuleConstant;
+
+import org.junit.After;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+/**
+ * Test cases for {@link AuthorityRuleManager}.
+ *
+ * @author Eric Zhao
+ */
+public class AuthorityRuleManagerTest {
+
+    @After
+    public void setUp() {
+        AuthorityRuleManager.loadRules(null);
+    }
+
+    @Test
+    public void testLoadRules() {
+        String resourceName = "testLoadRules";
+
+        AuthorityRule rule = new AuthorityRule();
+        rule.setResource(resourceName);
+        rule.setLimitApp("a,b");
+        rule.setStrategy(RuleConstant.AUTHORITY_WHITE);
+        AuthorityRuleManager.loadRules(Collections.singletonList(rule));
+
+        List<AuthorityRule> rules = AuthorityRuleManager.getRules();
+        assertEquals(1, rules.size());
+        assertEquals(rule, rules.get(0));
+
+        AuthorityRuleManager.loadRules(Collections.singletonList(new AuthorityRule()));
+        rules = AuthorityRuleManager.getRules();
+        assertEquals(0, rules.size());
+    }
+
+    @Test
+    public void testIsValidRule() {
+        AuthorityRule ruleA = new AuthorityRule();
+        AuthorityRule ruleB = null;
+        AuthorityRule ruleC = new AuthorityRule();
+        ruleC.setResource("abc");
+
+        assertFalse(AuthorityRuleManager.isValidRule(ruleA));
+        assertFalse(AuthorityRuleManager.isValidRule(ruleB));
+        assertTrue(AuthorityRuleManager.isValidRule(ruleC));
+    }
+
+    @After
+    public void tearDown() {
+        AuthorityRuleManager.loadRules(null);
+    }
+}

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

@@ -0,0 +1,132 @@
+package com.alibaba.csp.sentinel.slots.block.authority;
+
+import java.util.Collections;
+
+import com.alibaba.csp.sentinel.EntryType;
+import com.alibaba.csp.sentinel.TestUtil;
+import com.alibaba.csp.sentinel.context.ContextUtil;
+import com.alibaba.csp.sentinel.slotchain.ResourceWrapper;
+import com.alibaba.csp.sentinel.slotchain.StringResourceWrapper;
+import com.alibaba.csp.sentinel.slots.block.RuleConstant;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Test cases for {@link AuthoritySlot}.
+ *
+ * @author Eric Zhao
+ */
+public class AuthoritySlotTest {
+
+    private AuthoritySlot authoritySlot = new AuthoritySlot();
+
+    @Test
+    public void testCheckAuthorityNoExceptionItemsSuccess() throws Exception {
+        String origin = "appA";
+        String resourceName = "testCheckAuthorityNoExceptionItemsSuccess";
+        ResourceWrapper resourceWrapper = new StringResourceWrapper(resourceName, EntryType.IN);
+        ContextUtil.enter("entrance", origin);
+        try {
+            AuthorityRule ruleA = new AuthorityRule()
+                .setResource(resourceName)
+                .setLimitApp(origin + ",appB")
+                .as(AuthorityRule.class)
+                .setStrategy(RuleConstant.AUTHORITY_WHITE);
+
+            AuthorityRuleManager.loadRules(Collections.singletonList(ruleA));
+            authoritySlot.checkBlackWhiteAuthority(resourceWrapper, ContextUtil.getContext());
+
+            AuthorityRule ruleB = new AuthorityRule()
+                .setResource(resourceName)
+                .setLimitApp("appD")
+                .as(AuthorityRule.class)
+                .setStrategy(RuleConstant.AUTHORITY_BLACK);
+
+            AuthorityRuleManager.loadRules(Collections.singletonList(ruleB));
+            authoritySlot.checkBlackWhiteAuthority(resourceWrapper, ContextUtil.getContext());
+        } finally {
+            ContextUtil.exit();
+        }
+    }
+
+    @Test(expected = AuthorityException.class)
+    public void testCheckAuthorityNoExceptionItemsBlackFail() throws Exception {
+        String origin = "appA";
+        String resourceName = "testCheckAuthorityNoExceptionItemsBlackFail";
+        ResourceWrapper resourceWrapper = new StringResourceWrapper(resourceName, EntryType.IN);
+        ContextUtil.enter("entrance", origin);
+        try {
+            AuthorityRule ruleA = new AuthorityRule()
+                .setResource(resourceName)
+                .setLimitApp(origin + ",appC")
+                .as(AuthorityRule.class)
+                .setStrategy(RuleConstant.AUTHORITY_BLACK);
+
+            AuthorityRuleManager.loadRules(Collections.singletonList(ruleA));
+            authoritySlot.checkBlackWhiteAuthority(resourceWrapper, ContextUtil.getContext());
+        } finally {
+            ContextUtil.exit();
+        }
+    }
+
+    @Test(expected = AuthorityException.class)
+    public void testCheckAuthorityNoExceptionItemsWhiteFail() throws Exception {
+        String origin = "appA";
+        String resourceName = "testCheckAuthorityNoExceptionItemsWhiteFail";
+        ResourceWrapper resourceWrapper = new StringResourceWrapper(resourceName, EntryType.IN);
+        ContextUtil.enter("entrance", origin);
+        try {
+            AuthorityRule ruleB = new AuthorityRule()
+                .setResource(resourceName)
+                .setLimitApp("appB, appE")
+                .as(AuthorityRule.class)
+                .setStrategy(RuleConstant.AUTHORITY_WHITE);
+
+            AuthorityRuleManager.loadRules(Collections.singletonList(ruleB));
+            authoritySlot.checkBlackWhiteAuthority(resourceWrapper, ContextUtil.getContext());
+        } finally {
+            ContextUtil.exit();
+        }
+    }
+
+    /*@Test
+    public void testCheckAuthorityWithExceptionItemsSuccess() throws Exception {
+        String origin = "ipA";
+        String resourceName = "interfaceA";
+        ResourceWrapper resourceWrapper = new StringResourceWrapper(resourceName, EntryType.IN);
+        ContextUtil.enter("entrance", origin);
+        try {
+            AuthorityRule ruleEx = new AuthorityRule()
+                .setResource("methodXXX")
+                .setLimitApp(origin)
+                .as(AuthorityRule.class)
+                .setStrategy(RuleConstant.AUTHORITY_WHITE);
+
+            AuthorityRule ruleA = new AuthorityRule()
+                .setResource(resourceName)
+                .setLimitApp("appA,appB")
+                .as(AuthorityRule.class)
+                .setStrategy(RuleConstant.AUTHORITY_BLACK)
+                .addExceptionItem(ruleEx);
+
+            AuthorityRuleManager.loadRules(Collections.singletonList(ruleA));
+            authoritySlot.checkBlackWhiteAuthority(resourceWrapper, ContextUtil.getContext());
+        } finally {
+            ContextUtil.exit();
+        }
+    }*/
+
+    @Before
+    public void setUp() {
+        TestUtil.cleanUpContext();
+        AuthorityRuleManager.loadRules(null);
+    }
+
+    @After
+    public void tearDown() {
+        TestUtil.cleanUpContext();
+        AuthorityRuleManager.loadRules(null);
+    }
+}

sentinel-demo/sentinel-demo-basic/src/main/java/com/alibaba/csp/sentinel/demo/authority/AuthorityDemo.java

@@ -26,7 +26,7 @@ import com.alibaba.csp.sentinel.slots.block.authority.AuthorityRule;
 import com.alibaba.csp.sentinel.slots.block.authority.AuthorityRuleManager;
 
 /**
- * Authority rules is designed for limiting by request origins. In blacklist mode,
+ * Authority rule is designed for limiting by request origins. In blacklist mode,
  * requests will be blocked when blacklist contains current origin, otherwise will pass.
  * In whitelist mode, only requests from whitelist origin can pass.
  *