[SCB-2658]minor code improvement and add test case (#3303)
diff --git a/governance/src/main/java/org/apache/servicecomb/router/RouterFilter.java b/governance/src/main/java/org/apache/servicecomb/router/RouterFilter.java
index 78145bf..7e48b9c 100644
--- a/governance/src/main/java/org/apache/servicecomb/router/RouterFilter.java
+++ b/governance/src/main/java/org/apache/servicecomb/router/RouterFilter.java
@@ -46,7 +46,7 @@
}
public <T, E> List<T> getFilteredListOfServers(List<T> list,
- String targetServiceName, Map<String, String> headers, RouterDistributor<T, E> distributer) {
+ String targetServiceName, Map<String, String> headers, RouterDistributor<T, E> distributer) {
if (CollectionUtils.isEmpty(list)) {
return list;
}
diff --git a/governance/src/main/java/org/apache/servicecomb/router/match/RouterRuleMatcher.java b/governance/src/main/java/org/apache/servicecomb/router/match/RouterRuleMatcher.java
index a591ab9..a47da12 100644
--- a/governance/src/main/java/org/apache/servicecomb/router/match/RouterRuleMatcher.java
+++ b/governance/src/main/java/org/apache/servicecomb/router/match/RouterRuleMatcher.java
@@ -25,16 +25,13 @@
@Component
public class RouterRuleMatcher {
- private RouterRuleCache routerRuleCache;
+ private final RouterRuleCache routerRuleCache;
@Autowired
public RouterRuleMatcher(RouterRuleCache routerRuleCache) {
this.routerRuleCache = routerRuleCache;
}
- public RouterRuleMatcher() {
- }
-
public PolicyRuleItem match(String serviceName, Map<String, String> invokeHeader) {
for (PolicyRuleItem rule : routerRuleCache.getServiceInfoCacheMap().get(serviceName)
.getAllrule()) {
diff --git a/governance/src/main/java/org/apache/servicecomb/router/model/PolicyRuleItem.java b/governance/src/main/java/org/apache/servicecomb/router/model/PolicyRuleItem.java
index b13d599..edf9505 100644
--- a/governance/src/main/java/org/apache/servicecomb/router/model/PolicyRuleItem.java
+++ b/governance/src/main/java/org/apache/servicecomb/router/model/PolicyRuleItem.java
@@ -17,6 +17,7 @@
package org.apache.servicecomb.router.model;
import java.util.List;
+
import org.apache.servicecomb.router.exception.RouterIllegalParamException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -77,11 +78,7 @@
@Override
public int compareTo(PolicyRuleItem param) {
- if (param.precedence.equals(this.precedence)) {
- LOGGER.warn("the same canary precedence is not recommended");
- return 0;
- }
- return param.precedence > this.precedence ? 1 : -1;
+ return Integer.compare(param.precedence, this.precedence);
}
public Integer getPrecedence() {
diff --git a/governance/src/main/java/org/apache/servicecomb/router/model/ServiceInfoCache.java b/governance/src/main/java/org/apache/servicecomb/router/model/ServiceInfoCache.java
index a0a2952..15cc08c 100644
--- a/governance/src/main/java/org/apache/servicecomb/router/model/ServiceInfoCache.java
+++ b/governance/src/main/java/org/apache/servicecomb/router/model/ServiceInfoCache.java
@@ -24,29 +24,19 @@
* @Date 2019/10/17
**/
public class ServiceInfoCache {
-
- private List<PolicyRuleItem> allrule;
+ private final List<PolicyRuleItem> allrule;
/**
* for default version
*/
private TagItem latestVersionTag;
- public ServiceInfoCache() {
- }
-
public ServiceInfoCache(List<PolicyRuleItem> policyRuleItemList) {
- this.setAllrule(policyRuleItemList);
- // init tagitem
+ this.allrule = policyRuleItemList.stream().sorted().collect(Collectors.toList());
+
this.getAllrule().forEach(rule ->
rule.getRoute().forEach(RouteItem::initTagItem)
);
- // sort by precedence
- this.sortRule();
- }
-
- public void sortRule() {
- allrule = allrule.stream().sorted().collect(Collectors.toList());
}
public TagItem getNextInvokeVersion(PolicyRuleItem policyRuleItem) {
@@ -70,10 +60,6 @@
return allrule;
}
- public void setAllrule(List<PolicyRuleItem> allrule) {
- this.allrule = allrule;
- }
-
public TagItem getLatestVersionTag() {
return latestVersionTag;
}
diff --git a/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorTest.java b/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorDynamicConfig2Test.java
similarity index 70%
copy from governance/src/test/java/org/apache/servicecomb/router/RouterDistributorTest.java
copy to governance/src/test/java/org/apache/servicecomb/router/RouterDistributorDynamicConfig2Test.java
index f082c20..beb5bf4 100644
--- a/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorTest.java
+++ b/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorDynamicConfig2Test.java
@@ -17,6 +17,15 @@
package org.apache.servicecomb.router;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.servicecomb.governance.event.GovernanceConfigurationChangedEvent;
+import org.apache.servicecomb.governance.event.GovernanceEventManager;
import org.apache.servicecomb.router.cache.RouterRuleCache;
import org.apache.servicecomb.router.distribute.RouterDistributor;
import org.junit.Before;
@@ -31,45 +40,29 @@
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringRunner;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
@RunWith(SpringRunner.class)
@ContextConfiguration(locations = "classpath:META-INF/spring/*.xml", initializers = ConfigDataApplicationContextInitializer.class)
-public class RouterDistributorTest {
+public class RouterDistributorDynamicConfig2Test {
private static final String TARGET_SERVICE_NAME = "test_server";
private static final String RULE_STRING = ""
- + " - precedence: 2 #优先级\n"
- + " match: #匹配策略\n"
- + " source: xx #匹配某个服务名\n"
+ + " - precedence: 2\n"
+ + " route:\n"
+ + " - weight: 100\n"
+ + " tags:\n"
+ + " version: 2.0\n"
+ + " - precedence: 1\n"
+ + " match:\n"
+ " headers: #header匹配\n"
+ " appId:\n"
+ " regex: 01\n"
+ " caseInsensitive: false # 是否区分大小写，默认为false，区分大小写\n"
+ " userId:\n"
+ " exact: 01\n"
- + " route: #路由规则\n"
- + " - weight: 50\n"
- + " tags:\n"
- + " version: 1.1\n"
- + " - precedence: 1\n"
- + " match:\n"
- + " source: 1 #匹配某个服务名\n"
- + " headers: #header匹配\n"
- + " appId:\n"
- + " regex: 01\n"
- + " caseInsensitive: false # 是否区分大小写，默认为false，区分大小写\n"
- + " userId:\n"
- + " exact: 02\n"
+ " route:\n"
- + " - weight: 1\n"
+ + " - weight: 100\n"
+ " tags:\n"
- + " version: 1\n"
- + " app: a";
+ + " version: 1.0\n";
private Environment environment;
@@ -92,9 +85,6 @@
this.testDistributor = testDistributor;
}
- public RouterDistributorTest() {
- }
-
private final Map<String, Object> dynamicValues = new HashMap<>();
@Before
@@ -119,49 +109,30 @@
});
dynamicValues.put(RouterRuleCache.ROUTE_RULE_PREFIX + TARGET_SERVICE_NAME, RULE_STRING);
- }
-
- @Test
- public void testHeaderIsEmpty() {
- List<ServiceIns> list = getMockList();
- List<ServiceIns> serverList = mainFilter(list, Collections.emptyMap());
- Assertions.assertEquals(2, serverList.size());
+ Set<String> changedKeys = new HashSet<>();
+ changedKeys.add(RouterRuleCache.ROUTE_RULE_PREFIX + TARGET_SERVICE_NAME);
+ GovernanceConfigurationChangedEvent newEvent = new GovernanceConfigurationChangedEvent(changedKeys);
+ GovernanceEventManager.post(newEvent);
}
@Test
- public void testVersionNotMatch() {
- Map<String, String> headerMap = new HashMap<>();
- headerMap.put("userId", "01");
- headerMap.put("appId", "01");
- headerMap.put("format", "json");
- List<ServiceIns> list = getMockList();
- list.remove(1);
- List<ServiceIns> serverList = mainFilter(list, headerMap);
- Assertions.assertEquals(1, serverList.size());
- Assertions.assertEquals("01", serverList.get(0).getId());
- }
-
- @Test
- public void testVersionMatch() {
+ public void testMatchPrecedenceHigher() {
Map<String, String> headers = new HashMap<>();
headers.put("userId", "01");
headers.put("appId", "01");
- headers.put("format", "json");
- List<ServiceIns> serverList = mainFilter(getMockList(), headers);
- Assertions.assertEquals(1, serverList.size());
- Assertions.assertEquals("02", serverList.get(0).getId());
- }
- private List<ServiceIns> getMockList() {
List<ServiceIns> serverList = new ArrayList<>();
ServiceIns ins1 = new ServiceIns("01", TARGET_SERVICE_NAME);
ins1.setVersion("2.0");
ServiceIns ins2 = new ServiceIns("02", TARGET_SERVICE_NAME);
- ins2.addTags("app", "a");
+ ins2.setVersion("1.0");
serverList.add(ins1);
serverList.add(ins2);
- return serverList;
+
+ List<ServiceIns> resultServerList = mainFilter(serverList, headers);
+ Assertions.assertEquals(1, resultServerList.size());
+ Assertions.assertEquals("01", resultServerList.get(0).getId());
}
private List<ServiceIns> mainFilter(List<ServiceIns> serverList, Map<String, String> headers) {
diff --git a/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorTest.java b/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorDynamicConfigTest.java
similarity index 72%
rename from governance/src/test/java/org/apache/servicecomb/router/RouterDistributorTest.java
rename to governance/src/test/java/org/apache/servicecomb/router/RouterDistributorDynamicConfigTest.java
index f082c20..f57ed65 100644
--- a/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorTest.java
+++ b/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorDynamicConfigTest.java
@@ -17,6 +17,16 @@
package org.apache.servicecomb.router;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.servicecomb.governance.event.GovernanceConfigurationChangedEvent;
+import org.apache.servicecomb.governance.event.GovernanceEventManager;
import org.apache.servicecomb.router.cache.RouterRuleCache;
import org.apache.servicecomb.router.distribute.RouterDistributor;
import org.junit.Before;
@@ -31,21 +41,14 @@
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringRunner;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
@RunWith(SpringRunner.class)
@ContextConfiguration(locations = "classpath:META-INF/spring/*.xml", initializers = ConfigDataApplicationContextInitializer.class)
-public class RouterDistributorTest {
+public class RouterDistributorDynamicConfigTest {
private static final String TARGET_SERVICE_NAME = "test_server";
private static final String RULE_STRING = ""
+ " - precedence: 2 #优先级\n"
+ " match: #匹配策略\n"
- + " source: xx #匹配某个服务名\n"
+ " headers: #header匹配\n"
+ " appId:\n"
+ " regex: 01\n"
@@ -58,7 +61,6 @@
+ " version: 1.1\n"
+ " - precedence: 1\n"
+ " match:\n"
- + " source: 1 #匹配某个服务名\n"
+ " headers: #header匹配\n"
+ " appId:\n"
+ " regex: 01\n"
@@ -66,10 +68,21 @@
+ " userId:\n"
+ " exact: 02\n"
+ " route:\n"
- + " - weight: 1\n"
+ + " - weight: 100\n"
+ " tags:\n"
- + " version: 1\n"
- + " app: a";
+ + " version: 2.0\n"
+ + " - precedence: 3\n"
+ + " match:\n"
+ + " headers: #header匹配\n"
+ + " appId:\n"
+ + " regex: 01\n"
+ + " caseInsensitive: false # 是否区分大小写，默认为false，区分大小写\n"
+ + " userId:\n"
+ + " exact: 03\n"
+ + " route:\n"
+ + " - weight: 100\n"
+ + " tags:\n"
+ + " version: 1.0\n";
private Environment environment;
@@ -92,7 +105,7 @@
this.testDistributor = testDistributor;
}
- public RouterDistributorTest() {
+ public RouterDistributorDynamicConfigTest() {
}
private final Map<String, Object> dynamicValues = new HashMap<>();
@@ -119,6 +132,11 @@
});
dynamicValues.put(RouterRuleCache.ROUTE_RULE_PREFIX + TARGET_SERVICE_NAME, RULE_STRING);
+
+ Set<String> changedKeys = new HashSet<>();
+ changedKeys.add(RouterRuleCache.ROUTE_RULE_PREFIX + TARGET_SERVICE_NAME);
+ GovernanceConfigurationChangedEvent newEvent = new GovernanceConfigurationChangedEvent(changedKeys);
+ GovernanceEventManager.post(newEvent);
}
@@ -132,9 +150,8 @@
@Test
public void testVersionNotMatch() {
Map<String, String> headerMap = new HashMap<>();
- headerMap.put("userId", "01");
+ headerMap.put("userId", "02");
headerMap.put("appId", "01");
- headerMap.put("format", "json");
List<ServiceIns> list = getMockList();
list.remove(1);
List<ServiceIns> serverList = mainFilter(list, headerMap);
@@ -147,12 +164,41 @@
Map<String, String> headers = new HashMap<>();
headers.put("userId", "01");
headers.put("appId", "01");
- headers.put("format", "json");
List<ServiceIns> serverList = mainFilter(getMockList(), headers);
Assertions.assertEquals(1, serverList.size());
Assertions.assertEquals("02", serverList.get(0).getId());
}
+ @Test
+ public void testMatchPrecedenceLower() {
+ Map<String, String> headers = new HashMap<>();
+ headers.put("userId", "02");
+ headers.put("appId", "01");
+ List<ServiceIns> serverList = mainFilter(getMockList(), headers);
+ Assertions.assertEquals(1, serverList.size());
+ Assertions.assertEquals("01", serverList.get(0).getId());
+ }
+
+ @Test
+ public void testMatchPrecedenceHigher() {
+ Map<String, String> headers = new HashMap<>();
+ headers.put("userId", "03");
+ headers.put("appId", "01");
+
+ List<ServiceIns> serverList = new ArrayList<>();
+ ServiceIns ins1 = new ServiceIns("01", TARGET_SERVICE_NAME);
+ ins1.setVersion("2.0");
+ ServiceIns ins2 = new ServiceIns("02", TARGET_SERVICE_NAME);
+ ins2.addTags("app", "a");
+ ins2.setVersion("1.0");
+ serverList.add(ins1);
+ serverList.add(ins2);
+
+ List<ServiceIns> resultServerList = mainFilter(serverList, headers);
+ Assertions.assertEquals(1, resultServerList.size());
+ Assertions.assertEquals("02", resultServerList.get(0).getId());
+ }
+
private List<ServiceIns> getMockList() {
List<ServiceIns> serverList = new ArrayList<>();
ServiceIns ins1 = new ServiceIns("01", TARGET_SERVICE_NAME);
diff --git a/governance/src/test/java/org/apache/servicecomb/router/distribute/DistributeTest.java b/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorFileConfigTest.java
similarity index 94%
rename from governance/src/test/java/org/apache/servicecomb/router/distribute/DistributeTest.java
rename to governance/src/test/java/org/apache/servicecomb/router/RouterDistributorFileConfigTest.java
index e26e4f0..9e5be69 100644
--- a/governance/src/test/java/org/apache/servicecomb/router/distribute/DistributeTest.java
+++ b/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorFileConfigTest.java
@@ -14,10 +14,9 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.servicecomb.router.distribute;
+package org.apache.servicecomb.router;
-import org.apache.servicecomb.router.RouterFilter;
-import org.apache.servicecomb.router.ServiceIns;
+import org.apache.servicecomb.router.distribute.RouterDistributor;
import org.junit.Test;
import org.junit.jupiter.api.Assertions;
import org.junit.runner.RunWith;
@@ -32,7 +31,7 @@
@RunWith(SpringRunner.class)
@ContextConfiguration(locations = "classpath:META-INF/spring/*.xml", initializers = ConfigDataApplicationContextInitializer.class)
-public class DistributeTest {
+public class RouterDistributorFileConfigTest {
private static final String TARGET_SERVICE_NAME = "test_server1";
private RouterFilter routerFilter;
