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 <[email protected]> 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 [email protected].
