You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2018/03/22 09:16:38 UTC
[incubator-servicecomb-java-chassis] 03/05: Review comments
fix:Fault-Injection handler
This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-servicecomb-java-chassis.git
commit ee2b62627e47b003d85255174eecefc190077be7
Author: maheshrajus <ma...@huawei.com>
AuthorDate: Wed Mar 21 20:05:49 2018 +0530
Review comments fix:Fault-Injection handler
---
.../servicecomb/faultinjection/AbortFault.java | 6 +-
.../servicecomb/faultinjection/AbstractFault.java | 3 -
.../servicecomb/faultinjection/DelayFault.java | 3 +-
.../servicecomb/faultinjection/FaultExecutor.java | 2 +-
.../src/main/resources/config/cse.handler.xml | 2 +-
.../faultinjection/TestFaultInjectHandler.java | 67 +++++++++++-----------
6 files changed, 41 insertions(+), 42 deletions(-)
diff --git a/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/AbortFault.java b/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/AbortFault.java
index c878899..f53eef3 100644
--- a/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/AbortFault.java
+++ b/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/AbortFault.java
@@ -26,11 +26,11 @@ import org.springframework.stereotype.Component;
@Component
public class AbortFault extends AbstractFault {
- private static final Logger LOGGER = LoggerFactory.getLogger(FaultInjectionConfig.class);
+ private static final Logger LOGGER = LoggerFactory.getLogger(AbortFault.class);
@Override
public void injectFault(Invocation invocation, FaultParam faultParam, AsyncResponse asynResponse) {
- // get the config values related to delay.
+ // get the config values related to abort.
int abortPercent = FaultInjectionUtil.getFaultInjectionConfig(invocation,
"abort.percent");
@@ -43,7 +43,7 @@ public class AbortFault extends AbstractFault {
// check fault abort condition.
boolean isAbort = FaultInjectionUtil.checkFaultInjectionDelayAndAbort(faultParam.getReqCount(), abortPercent);
if (isAbort) {
- // get the config values related to delay percentage.
+ // get the config values related to abort percentage.
int errorCode = FaultInjectionUtil.getFaultInjectionConfig(invocation,
"abort.httpStatus");
diff --git a/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/AbstractFault.java b/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/AbstractFault.java
index db88a5e..e92a68e 100644
--- a/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/AbstractFault.java
+++ b/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/AbstractFault.java
@@ -17,9 +17,6 @@
package org.apache.servicecomb.faultinjection;
-import org.springframework.stereotype.Component;
-
-@Component
public abstract class AbstractFault implements Fault {
}
diff --git a/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/DelayFault.java b/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/DelayFault.java
index aaf2f63..e486ae3 100644
--- a/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/DelayFault.java
+++ b/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/DelayFault.java
@@ -28,7 +28,7 @@ import io.vertx.core.Vertx;
@Component
public class DelayFault extends AbstractFault {
- private static final Logger LOGGER = LoggerFactory.getLogger(FaultInjectionHandler.class);
+ private static final Logger LOGGER = LoggerFactory.getLogger(DelayFault.class);
@Override
public int getPriority() {
@@ -76,6 +76,7 @@ public class DelayFault extends AbstractFault {
asynResponse.success(new FaultResponse());
}
}
+ asynResponse.success(new FaultResponse());
}
}
diff --git a/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/FaultExecutor.java b/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/FaultExecutor.java
index 291b1c8..c5a0e9b 100644
--- a/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/FaultExecutor.java
+++ b/handlers/handler-fault-injection/src/main/java/org/apache/servicecomb/faultinjection/FaultExecutor.java
@@ -59,7 +59,7 @@ public class FaultExecutor {
asyncResponse.complete(response);
} else {
FaultResponse r = response.getResult();
- if (r.getStatusCode() != 0) {
+ if (r.getStatusCode() == FaultInjectionConst.FAULT_INJECTION_ERROR) {
asyncResponse.complete(response);
} else {
FaultExecutor.this.next(asyncResponse);
diff --git a/handlers/handler-fault-injection/src/main/resources/config/cse.handler.xml b/handlers/handler-fault-injection/src/main/resources/config/cse.handler.xml
index 477963b..4c1553e 100755
--- a/handlers/handler-fault-injection/src/main/resources/config/cse.handler.xml
+++ b/handlers/handler-fault-injection/src/main/resources/config/cse.handler.xml
@@ -16,6 +16,6 @@
-->
<config>
- <handler id="fault-injection"
+ <handler id="fault-injection-consumer"
class="org.apache.servicecomb.faultinjection.FaultInjectionHandler" />
</config>
diff --git a/handlers/handler-fault-injection/src/test/java/org/apache/servicecomb/faultinjection/TestFaultInjectHandler.java b/handlers/handler-fault-injection/src/test/java/org/apache/servicecomb/faultinjection/TestFaultInjectHandler.java
index 600c59a..255de43 100644
--- a/handlers/handler-fault-injection/src/test/java/org/apache/servicecomb/faultinjection/TestFaultInjectHandler.java
+++ b/handlers/handler-fault-injection/src/test/java/org/apache/servicecomb/faultinjection/TestFaultInjectHandler.java
@@ -55,6 +55,9 @@ public class TestFaultInjectHandler {
Response response;
+ boolean isDelay;
+
+ boolean isAbort;
@InjectMocks
FaultInjectionHandler faultHandler;
@@ -172,23 +175,22 @@ public class TestFaultInjectHandler {
Mockito.when(invocation.getSchemaId()).thenReturn("sayHelloSchema");
Mockito.when(invocation.getMicroserviceName()).thenReturn("hello");
- List<Fault> faultInjectionFeatureList = Arrays.asList(abortFault, delayFault);
+ List<Fault> faultInjectionFeatureList = Arrays.asList(abortFault);
handler.setFaultFeature(faultInjectionFeatureList);
- boolean validAssert;
- try {
- validAssert = true;
- handler.handle(invocation, asyncResp);
- } catch (Exception e) {
- validAssert = false;
- }
+ handler.handle(invocation, ar -> {
+ //this case no delay/no abort so reponse is null, it should not enter this in this block.
+ isDelay = true;
+ isAbort = true;
+ });
+ Assert.assertFalse(isDelay);
+ Assert.assertFalse(isAbort);
System.getProperties().remove("cse.governance.Consumer._global.policy.fault.protocols.rest.delay.fixedDelay");
System.getProperties().remove("cse.governance.Consumer._global.policy.fault.protocols.rest.delay.percent");
System.getProperties().remove("cse.governance.Consumer._global.policy.fault.protocols.rest.abort.percent");
System.getProperties().remove("cse.governance.Consumer._global.policy.fault.protocols.rest.abort.httpStatus");
- Assert.assertTrue(validAssert);
AtomicLong count = FaultInjectionUtil.getOperMetTotalReq("restMicroserviceQualifiedName3");
assertEquals(2, count.get());
}
@@ -217,20 +219,19 @@ public class TestFaultInjectHandler {
List<Fault> faultInjectionFeatureList = Arrays.asList(abortFault, delayFault);
handler.setFaultFeature(faultInjectionFeatureList);
- boolean validAssert;
- try {
- validAssert = true;
- handler.handle(invocation, asyncResp);
- } catch (Exception e) {
- validAssert = false;
- }
+ handler.handle(invocation, ar -> {
+ //this case no delay/no abort so reponse is null, it should not enter this in this block.
+ isDelay = true;
+ isAbort = true;
+ });
+ Assert.assertFalse(isDelay);
+ Assert.assertFalse(isAbort);
System.getProperties().remove("cse.governance.Consumer.carts.policy.fault.protocols.rest.delay.fixedDelay");
System.getProperties().remove("cse.governance.Consumer.carts.policy.fault.protocols.rest.delay.percent");
System.getProperties().remove("cse.governance.Consumer.carts.policy.fault.protocols.rest.abort.percent");
System.getProperties().remove("cse.governance.Consumer.carts.policy.fault.protocols.rest.abort.httpStatus");
- Assert.assertTrue(validAssert);
AtomicLong count = FaultInjectionUtil.getOperMetTotalReq("restMicroserviceQualifiedName4");
assertEquals(2, count.get());
}
@@ -263,13 +264,13 @@ public class TestFaultInjectHandler {
List<Fault> faultInjectionFeatureList = Arrays.asList(abortFault, delayFault);
handler.setFaultFeature(faultInjectionFeatureList);
- boolean validAssert;
- try {
- validAssert = true;
- handler.handle(invocation, asyncResp);
- } catch (Exception e) {
- validAssert = false;
- }
+ handler.handle(invocation, ar -> {
+ //this case no delay/no abort so reponse is null, it should not enter this in this block.
+ isDelay = true;
+ isAbort = true;
+ });
+ Assert.assertFalse(isDelay);
+ Assert.assertFalse(isAbort);
System.getProperties()
.remove("cse.governance.Consumer.carts.schemas.testSchema.policy.fault.protocols.rest.delay.fixedDelay");
@@ -280,7 +281,6 @@ public class TestFaultInjectHandler {
System.getProperties()
.remove("cse.governance.Consumer.carts.schemas.testSchema.policy.fault.protocols.rest.abort.httpStatus");
- Assert.assertTrue(validAssert);
AtomicLong count = FaultInjectionUtil.getOperMetTotalReq("restMicroserviceQualifiedName5");
assertEquals(2, count.get());
}
@@ -317,13 +317,14 @@ public class TestFaultInjectHandler {
List<Fault> faultInjectionFeatureList = Arrays.asList(abortFault, delayFault);
handler.setFaultFeature(faultInjectionFeatureList);
- boolean validAssert;
- try {
- validAssert = true;
- handler.handle(invocation, asyncResp);
- } catch (Exception e) {
- validAssert = false;
- }
+ handler.handle(invocation, ar -> {
+ //this case no delay/no abort so reponse is null, it should not enter this in this block.
+ isDelay = true;
+ isAbort = true;
+ });
+
+ Assert.assertFalse(isDelay);
+ Assert.assertFalse(isAbort);
System.getProperties()
.remove(
@@ -337,7 +338,7 @@ public class TestFaultInjectHandler {
System.getProperties()
.remove(
"cse.governance.Consumer.carts.schemas.testSchema.operations.sayHi.policy.fault.protocols.rest.abort.httpStatus");
- Assert.assertTrue(validAssert);
+
AtomicLong count = FaultInjectionUtil.getOperMetTotalReq("restMicroserviceQualifiedName6");
assertEquals(2, count.get());
}
--
To stop receiving notification emails like this one, please contact
liubao@apache.org.