WillemJiang commented on code in PR #3377:
URL:
https://github.com/apache/servicecomb-java-chassis/pull/3377#discussion_r991737935
##########
governance/src/test/java/org/apache/servicecomb/governance/CustomMatchTest.java:
##########
@@ -0,0 +1,82 @@
+package org.apache.servicecomb.governance;
Review Comment:
We need the License header to the unit test file as well.
##########
governance/src/test/java/org/apache/servicecomb/governance/CustomMatchTest.java:
##########
@@ -0,0 +1,82 @@
+package org.apache.servicecomb.governance;
+
+import org.apache.servicecomb.governance.marker.CustomMatcher;
+import org.apache.servicecomb.governance.marker.GovernanceRequest;
+import org.apache.servicecomb.governance.marker.Matcher;
+import org.apache.servicecomb.governance.marker.RequestProcessor;
+import
org.apache.servicecomb.governance.mockclasses.service.MockConfigurationForCustomMatcher;
+import org.apache.servicecomb.governance.service.MatchersService;
+import org.apache.servicecomb.governance.utils.CustomMatch;
+import org.junit.Assert;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.ContextConfiguration;
+
+@SpringBootTest
+@ContextConfiguration(classes = {GovernanceConfiguration.class,
MockConfiguration.class, MockConfigurationForCustomMatcher.class})
+public class CustomMatchTest {
+
+ private RequestProcessor requestProcessor;
+
+ @Autowired
+ public void setRequestProcessor(RequestProcessor requestProcessor) {
+ this.requestProcessor = requestProcessor;
+ }
+
+ private Matcher generateMatcher(String customMatcherHandler, String
customMatcherParameters) {
+ CustomMatcher customMatcher = new CustomMatcher();
+ customMatcher.setCustomMatcherHandler(customMatcherHandler);
+ customMatcher.setCustomMatcherParameters(customMatcherParameters);
+ Matcher matcher = new Matcher();
+ matcher.setCustomMatcher(customMatcher);
+ return matcher;
+ }
+
+ @Test
+ public void test_should_pass_when_value_class_empty() {
+ GovernanceRequest request = new GovernanceRequest();
+ Matcher mockMatcher = generateMatcher("", "");
+ Assert.assertTrue(this.requestProcessor.match(request,mockMatcher));
+ mockMatcher = generateMatcher("", "bill");
+ Assert.assertTrue(this.requestProcessor.match(request,mockMatcher));
+ mockMatcher = generateMatcher("classWithAnnotation", "");
+ Assert.assertTrue(this.requestProcessor.match(request,mockMatcher));
+ }
+
+ @Test
+ public void test_should_pass_when_value_class_match() {
+ GovernanceRequest request = new GovernanceRequest();
+ Matcher mockMatcher = generateMatcher("classWithAnnotation", "bill");
+ Assert.assertTrue(this.requestProcessor.match(request,mockMatcher));
+ }
+
+ @Test
+ public void test_should_throw_exception_when_class_not_found() {
+ GovernanceRequest request = new GovernanceRequest();
+ try {
+ Matcher mockMatcher =
generateMatcher("classWithAnnotationNotFound", "bill");
+ this.requestProcessor.match(request,mockMatcher);
+ } catch (Exception e) {
+ Assert.assertTrue(e.getCause() instanceof ClassNotFoundException);
+ }
+ }
+
+ @Test
+ public void test_should_pass_when_multiple_value() {
+ GovernanceRequest request = new GovernanceRequest();
+ Matcher mockMatcher = generateMatcher("classWithAnnotation",
"bill,bill2");
+ Assert.assertTrue(this.requestProcessor.match(request,mockMatcher));
+ }
+
+ @Test
+ public void test_should_throw_exception_when_not_implements_interface() {
+ GovernanceRequest request = new GovernanceRequest();
+ try {
+ Matcher mockMatcher = generateMatcher("classNotImplments",
"bill,bill2");
+ this.requestProcessor.match(request,mockMatcher);
Review Comment:
need to add a line below this code.
fail("Except the exception is thrown here")
##########
governance/src/main/java/org/apache/servicecomb/governance/marker/RequestProcessor.java:
##########
@@ -103,4 +115,68 @@ private boolean operatorMatch(String str, RawOperator
rawOperator) {
}
return true;
}
+
+ private boolean customMatch(GovernanceRequest request, Matcher matcher) {
+ if (matcher.getCustomMatcher() == null) {
+ return true;
+ }
+ String customMatcherHandlerName =
matcher.getCustomMatcher().getCustomMatcherHandler();
+ String customMatcherParameters =
matcher.getCustomMatcher().getCustomMatcherParameters();
+
+ if (StringUtils.isEmpty(customMatcherHandlerName) ||
StringUtils.isEmpty(customMatcherParameters)) {
+ return true;
+ }
+
+ CustomMatch customMatcherHandler =
generateHandler(customMatcherHandlerName);
+ return customMatcherHandler.matchRequest(request, customMatcherParameters);
+ }
+
+ private CustomMatch getBeanByHandlerName(String customMatcherHandler) {
+ Object extractObject = null;
+ if (applicationContext.containsBean(customMatcherHandler)) {
+ extractObject = applicationContext.getBean(customMatcherHandler);
+ if (!(extractObject instanceof CustomMatch)) {
+ LOGGER.error("{} {}", customMatcherHandler,
CustomMatch.errorMessageForNotImplements);
+ throw new RuntimeException(customMatcherHandler +
CustomMatch.errorMessageForNotImplements);
+ }
+ return (CustomMatch)extractObject;
+ }
+ return null;
+ }
+
+
+ public CustomMatch generateHandler(String customMatcherHandler) {
+ CustomMatch extractObject = getBeanByHandlerName(customMatcherHandler);
+ if (extractObject != null) {
+ return extractObject;
+ }
+
Review Comment:
It could be better if we can add a info log to show we create a
extractionHandler here.
##########
governance/src/test/java/org/apache/servicecomb/governance/mockclasses/CustomMatchDemo.java:
##########
@@ -0,0 +1,16 @@
+package org.apache.servicecomb.governance.mockclasses;
Review Comment:
License header.
##########
governance/src/test/java/org/apache/servicecomb/governance/mockclasses/ClassNotImplments.java:
##########
@@ -0,0 +1,11 @@
+package org.apache.servicecomb.governance.mockclasses;
Review Comment:
License header.
##########
governance/src/test/java/org/apache/servicecomb/governance/mockclasses/service/MockConfigurationForCustomMatcher.java:
##########
@@ -0,0 +1,9 @@
+package org.apache.servicecomb.governance.mockclasses.service;
Review Comment:
License header.
##########
governance/src/test/java/org/apache/servicecomb/governance/mockclasses/ClassWithAnnotation.java:
##########
@@ -0,0 +1,22 @@
+package org.apache.servicecomb.governance.mockclasses;
Review Comment:
License header.
##########
governance/src/test/java/org/apache/servicecomb/governance/mockclasses/service/MockProfileClassUserService.java:
##########
@@ -0,0 +1,10 @@
+package org.apache.servicecomb.governance.mockclasses.service;
Review Comment:
License header.
##########
governance/src/main/java/org/apache/servicecomb/governance/marker/GovernanceRequest.java:
##########
@@ -102,4 +109,12 @@ public String getServiceName() {
public void setServiceName(String serviceName) {
this.serviceName = serviceName;
}
+
+ public Object getSourceRequest() {
+ return sourceRequest;
+ }
+
+ public void setSourceRequest(Object sourceRequest) {
+ this.sourceRequest = sourceRequest;
+ }
Review Comment:
I don't think it's reasonable to treat the sourceRequest as a Object.
If it just a URI, we can change the Object to String.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
