From 99ecafa159e34dad3fb5519b7cee99bf50ffc6e5 Mon Sep 17 00:00:00 2001 From: Ian Luo Date: Wed, 7 Nov 2018 14:21:42 +0800 Subject: [PATCH] =?UTF-8?q?[DUBBO-2489]=20MockClusterInvoker=20provides=20?= =?UTF-8?q?local=20forced=20mock=EF=BC=8CI=20tested=20it=20locally,=20but?= =?UTF-8?q?=20it=20doesn't=20work=20(#2742)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../apache/dubbo/config/AbstractConfig.java | 2 +- .../dubbo/config/AbstractInterfaceConfig.java | 53 ++++--- .../dubbo/config/AbstractMethodConfig.java | 10 +- .../apache/dubbo/config/ReferenceConfig.java | 3 +- .../apache/dubbo/config/ServiceConfig.java | 3 +- .../dubbo/config/AbstractConfigTest.java | 3 +- .../config/AbstractInterfaceConfigTest.java | 27 ++-- .../apache/dubbo/rpc/support/MockInvoker.java | 144 +++++++++++------- 8 files changed, 151 insertions(+), 94 deletions(-) diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java index fdaa3b16a1cf..b1aeeedc40a4 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java @@ -57,7 +57,7 @@ public abstract class AbstractConfig implements Serializable { private static final Pattern PATTERN_PATH = Pattern.compile("[/\\-$._0-9a-zA-Z]+"); - private static final Pattern PATTERN_NAME_HAS_SYMBOL = Pattern.compile("[:*,/\\-._0-9a-zA-Z]+"); + private static final Pattern PATTERN_NAME_HAS_SYMBOL = Pattern.compile("[:*,\\s/\\-._0-9a-zA-Z]+"); private static final Pattern PATTERN_KEY = Pattern.compile("[*,\\-._0-9a-zA-Z]+"); private static final Map legacyProperties = new HashMap(); diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java index 70ab62e4493a..3ff1cebb9e68 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java @@ -286,7 +286,36 @@ protected void checkInterfaceAndMethods(Class interfaceClass, List interfaceClass) { + void checkMock(Class interfaceClass) { + if (ConfigUtils.isEmpty(mock)) { + return; + } + + String normalizedMock = MockInvoker.normalizeMock(mock); + if (normalizedMock.startsWith(Constants.RETURN_PREFIX)) { + normalizedMock = normalizedMock.substring(Constants.RETURN_PREFIX.length()).trim(); + try { + MockInvoker.parseMockValue(normalizedMock); + } catch (Exception e) { + throw new IllegalStateException("Illegal mock return in "); + } + } else if (normalizedMock.startsWith(Constants.THROW_PREFIX)) { + normalizedMock = normalizedMock.substring(Constants.THROW_PREFIX.length()).trim(); + if (ConfigUtils.isNotEmpty(normalizedMock)) { + try { + MockInvoker.getThrowable(normalizedMock); + } catch (Exception e) { + throw new IllegalStateException("Illegal mock throw in "); + } + } + } else { + MockInvoker.getMockObject(normalizedMock, interfaceClass); + } + } + + void checkStub(Class interfaceClass) { if (ConfigUtils.isNotEmpty(local)) { Class localClass = ConfigUtils.isDefault(local) ? ReflectUtils.forName(interfaceClass.getName() + "Local") : ReflectUtils.forName(local); if (!interfaceClass.isAssignableFrom(localClass)) { @@ -309,26 +338,6 @@ protected void checkStubAndMock(Class interfaceClass) { throw new IllegalStateException("No such constructor \"public " + localClass.getSimpleName() + "(" + interfaceClass.getName() + ")\" in local implementation class " + localClass.getName()); } } - if (ConfigUtils.isNotEmpty(mock)) { - if (mock.startsWith(Constants.RETURN_PREFIX)) { - String value = mock.substring(Constants.RETURN_PREFIX.length()); - try { - MockInvoker.parseMockValue(value); - } catch (Exception e) { - throw new IllegalStateException("Illegal mock json value in "); - } - } else { - Class mockClass = ConfigUtils.isDefault(mock) ? ReflectUtils.forName(interfaceClass.getName() + "Mock") : ReflectUtils.forName(mock); - if (!interfaceClass.isAssignableFrom(mockClass)) { - throw new IllegalStateException("The mock implementation class " + mockClass.getName() + " not implement interface " + interfaceClass.getName()); - } - try { - mockClass.getConstructor(new Class[0]); - } catch (NoSuchMethodException e) { - throw new IllegalStateException("No such empty constructor \"public " + mockClass.getSimpleName() + "()\" in mock implementation class " + mockClass.getName()); - } - } - } } /** @@ -522,4 +531,4 @@ public String getScope() { public void setScope(String scope) { this.scope = scope; } -} \ No newline at end of file +} diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractMethodConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractMethodConfig.java index a2c3de65312c..a2263b8221e5 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractMethodConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractMethodConfig.java @@ -140,8 +140,14 @@ public void setMock(Boolean mock) { } public void setMock(String mock) { - if (mock != null && mock.startsWith(Constants.RETURN_PREFIX)) { + if (mock == null) { + return; + } + + if (mock.startsWith(Constants.RETURN_PREFIX) || mock.startsWith(Constants.THROW_PREFIX + " ")) { checkLength("mock", mock); + } else if (mock.startsWith(Constants.FAIL_PREFIX) || mock.startsWith(Constants.FORCE_PREFIX)) { + checkNameHasSymbol("mock", mock); } else { checkName("mock", mock); } @@ -181,4 +187,4 @@ public void setParameters(Map parameters) { this.parameters = parameters; } -} \ No newline at end of file +} diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java index 61ed531e1413..c39f3b67a59c 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java @@ -244,7 +244,8 @@ private void init() { } } checkApplication(); - checkStubAndMock(interfaceClass); + checkStub(interfaceClass); + checkMock(interfaceClass); Map map = new HashMap(); resolveAsyncInterface(interfaceClass, map); diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java index e2d3969e08b0..42abc009e8c3 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java @@ -309,7 +309,8 @@ protected synchronized void doExport() { checkRegistry(); checkProtocol(); appendProperties(this); - checkStubAndMock(interfaceClass); + checkStub(interfaceClass); + checkMock(interfaceClass); if (path == null || path.length() == 0) { path = interfaceName; } diff --git a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractConfigTest.java b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractConfigTest.java index ef7354dfd2b4..792bed697404 100644 --- a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractConfigTest.java +++ b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractConfigTest.java @@ -198,7 +198,8 @@ public void checkName() throws Exception { @Test public void checkNameHasSymbol() throws Exception { try { - AbstractConfig.checkNameHasSymbol("hello", ":*,/-0123abcdABCD"); + AbstractConfig.checkNameHasSymbol("hello", ":*,/ -0123\tabcdABCD"); + AbstractConfig.checkNameHasSymbol("mock", "force:return world"); } catch (Exception e) { TestCase.fail("the value should be legal."); } diff --git a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractInterfaceConfigTest.java b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractInterfaceConfigTest.java index 2ea6fb4a0ee0..3199eb969a62 100644 --- a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractInterfaceConfigTest.java +++ b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractInterfaceConfigTest.java @@ -185,63 +185,72 @@ public void checkInterfaceAndMethod5() throws Exception { public void checkStubAndMock1() throws Exception { InterfaceConfig interfaceConfig = new InterfaceConfig(); interfaceConfig.setLocal(GreetingLocal1.class.getName()); - interfaceConfig.checkStubAndMock(Greeting.class); + interfaceConfig.checkStub(Greeting.class); + interfaceConfig.checkMock(Greeting.class); } @Test(expected = IllegalStateException.class) public void checkStubAndMock2() throws Exception { InterfaceConfig interfaceConfig = new InterfaceConfig(); interfaceConfig.setLocal(GreetingLocal2.class.getName()); - interfaceConfig.checkStubAndMock(Greeting.class); + interfaceConfig.checkStub(Greeting.class); + interfaceConfig.checkMock(Greeting.class); } @Test public void checkStubAndMock3() throws Exception { InterfaceConfig interfaceConfig = new InterfaceConfig(); interfaceConfig.setLocal(GreetingLocal3.class.getName()); - interfaceConfig.checkStubAndMock(Greeting.class); + interfaceConfig.checkStub(Greeting.class); + interfaceConfig.checkMock(Greeting.class); } @Test(expected = IllegalStateException.class) public void checkStubAndMock4() throws Exception { InterfaceConfig interfaceConfig = new InterfaceConfig(); interfaceConfig.setStub(GreetingLocal1.class.getName()); - interfaceConfig.checkStubAndMock(Greeting.class); + interfaceConfig.checkStub(Greeting.class); + interfaceConfig.checkMock(Greeting.class); } @Test(expected = IllegalStateException.class) public void checkStubAndMock5() throws Exception { InterfaceConfig interfaceConfig = new InterfaceConfig(); interfaceConfig.setStub(GreetingLocal2.class.getName()); - interfaceConfig.checkStubAndMock(Greeting.class); + interfaceConfig.checkStub(Greeting.class); + interfaceConfig.checkMock(Greeting.class); } @Test public void checkStubAndMock6() throws Exception { InterfaceConfig interfaceConfig = new InterfaceConfig(); interfaceConfig.setStub(GreetingLocal3.class.getName()); - interfaceConfig.checkStubAndMock(Greeting.class); + interfaceConfig.checkStub(Greeting.class); + interfaceConfig.checkMock(Greeting.class); } @Test(expected = IllegalStateException.class) public void checkStubAndMock7() throws Exception { InterfaceConfig interfaceConfig = new InterfaceConfig(); interfaceConfig.setMock("return {a, b}"); - interfaceConfig.checkStubAndMock(Greeting.class); + interfaceConfig.checkStub(Greeting.class); + interfaceConfig.checkMock(Greeting.class); } @Test(expected = IllegalStateException.class) public void checkStubAndMock8() throws Exception { InterfaceConfig interfaceConfig = new InterfaceConfig(); interfaceConfig.setMock(GreetingMock1.class.getName()); - interfaceConfig.checkStubAndMock(Greeting.class); + interfaceConfig.checkStub(Greeting.class); + interfaceConfig.checkMock(Greeting.class); } @Test(expected = IllegalStateException.class) public void checkStubAndMock9() throws Exception { InterfaceConfig interfaceConfig = new InterfaceConfig(); interfaceConfig.setMock(GreetingMock2.class.getName()); - interfaceConfig.checkStubAndMock(Greeting.class); + interfaceConfig.checkStub(Greeting.class); + interfaceConfig.checkMock(Greeting.class); } @Test diff --git a/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/support/MockInvoker.java b/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/support/MockInvoker.java index 9f71c41f5abc..5d4d2fc259b5 100644 --- a/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/support/MockInvoker.java +++ b/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/support/MockInvoker.java @@ -96,26 +96,21 @@ public Result invoke(Invocation invocation) throws RpcException { if (StringUtils.isBlank(mock)) { throw new RpcException(new IllegalAccessException("mock can not be null. url :" + url)); } - mock = normallizeMock(URL.decode(mock)); - if (Constants.RETURN_PREFIX.trim().equalsIgnoreCase(mock.trim())) { - RpcResult result = new RpcResult(); - result.setValue(null); - return result; - } else if (mock.startsWith(Constants.RETURN_PREFIX)) { + mock = normalizeMock(URL.decode(mock)); + if (mock.startsWith(Constants.RETURN_PREFIX)) { mock = mock.substring(Constants.RETURN_PREFIX.length()).trim(); - mock = mock.replace('`', '"'); try { Type[] returnTypes = RpcUtils.getReturnTypes(invocation); Object value = parseMockValue(mock, returnTypes); return new RpcResult(value); } catch (Exception ew) { - throw new RpcException("mock return invoke error. method :" + invocation.getMethodName() + ", mock:" + mock + ", url: " + url, ew); + throw new RpcException("mock return invoke error. method :" + invocation.getMethodName() + + ", mock:" + mock + ", url: " + url, ew); } } else if (mock.startsWith(Constants.THROW_PREFIX)) { mock = mock.substring(Constants.THROW_PREFIX.length()).trim(); - mock = mock.replace('`', '"'); if (StringUtils.isBlank(mock)) { - throw new RpcException(" mocked exception for Service degradation. "); + throw new RpcException("mocked exception for service degradation."); } else { // user customized class Throwable t = getThrowable(mock); throw new RpcException(RpcException.BIZ_EXCEPTION, t); @@ -125,29 +120,29 @@ public Result invoke(Invocation invocation) throws RpcException { Invoker invoker = getInvoker(mock); return invoker.invoke(invocation); } catch (Throwable t) { - throw new RpcException("Failed to create mock implemention class " + mock, t); + throw new RpcException("Failed to create mock implementation class " + mock, t); } } } - private Throwable getThrowable(String throwstr) { - Throwable throwable = (Throwable) throwables.get(throwstr); + public static Throwable getThrowable(String throwstr) { + Throwable throwable = throwables.get(throwstr); if (throwable != null) { return throwable; - } else { - Throwable t = null; - try { - Class bizException = ReflectUtils.forName(throwstr); - Constructor constructor; - constructor = ReflectUtils.findConstructor(bizException, String.class); - t = (Throwable) constructor.newInstance(new Object[]{" mocked exception for Service degradation. "}); - if (throwables.size() < 1000) { - throwables.put(throwstr, t); - } - } catch (Exception e) { - throw new RpcException("mock throw error :" + throwstr + " argument error.", e); + } + + try { + Throwable t; + Class bizException = ReflectUtils.forName(throwstr); + Constructor constructor; + constructor = ReflectUtils.findConstructor(bizException, String.class); + t = (Throwable) constructor.newInstance(new Object[]{"mocked exception for service degradation."}); + if (throwables.size() < 1000) { + throwables.put(throwstr, t); } return t; + } catch (Exception e) { + throw new RpcException("mock throw error :" + throwstr + " argument error.", e); } } @@ -156,49 +151,84 @@ private Invoker getInvoker(String mockService) { Invoker invoker = (Invoker) mocks.get(mockService); if (invoker != null) { return invoker; - } else { - Class serviceType = (Class) ReflectUtils.forName(url.getServiceInterface()); - if (ConfigUtils.isDefault(mockService)) { - mockService = serviceType.getName() + "Mock"; - } + } - Class mockClass = ReflectUtils.forName(mockService); - if (!serviceType.isAssignableFrom(mockClass)) { - throw new IllegalArgumentException("The mock implemention class " + mockClass.getName() + " not implement interface " + serviceType.getName()); - } + Class serviceType = (Class) ReflectUtils.forName(url.getServiceInterface()); + T mockObject = (T) getMockObject(mockService, serviceType); + invoker = proxyFactory.getInvoker(mockObject, serviceType, url); + if (mocks.size() < 10000) { + mocks.put(mockService, invoker); + } + return invoker; + } - if (!serviceType.isAssignableFrom(mockClass)) { - throw new IllegalArgumentException("The mock implemention class " + mockClass.getName() + " not implement interface " + serviceType.getName()); - } - try { - T mockObject = (T) mockClass.newInstance(); - invoker = proxyFactory.getInvoker(mockObject, (Class) serviceType, url); - if (mocks.size() < 10000) { - mocks.put(mockService, invoker); - } - return invoker; - } catch (InstantiationException e) { - throw new IllegalStateException("No such empty constructor \"public " + mockClass.getSimpleName() + "()\" in mock implemention class " + mockClass.getName(), e); - } catch (IllegalAccessException e) { - throw new IllegalStateException(e); - } + @SuppressWarnings("unchecked") + public static Object getMockObject(String mockService, Class serviceType) { + if (ConfigUtils.isDefault(mockService)) { + mockService = serviceType.getName() + "Mock"; + } + + Class mockClass = ReflectUtils.forName(mockService); + if (!serviceType.isAssignableFrom(mockClass)) { + throw new IllegalStateException("The mock class " + mockClass.getName() + + " not implement interface " + serviceType.getName()); + } + + try { + return mockClass.newInstance(); + } catch (InstantiationException e) { + throw new IllegalStateException("No default constructor from mock class " + mockClass.getName(), e); + } catch (IllegalAccessException e) { + throw new IllegalStateException(e); } } - //mock=fail:throw - //mock=fail:return - //mock=xx.Service - private String normallizeMock(String mock) { - if (mock == null || mock.trim().length() == 0) { + + /** + * Normalize mock string: + * + *
    + *
  1. return => return null
    2. + *
  2. fail => default
    3. + *
  3. force => default
    4. + *
  4. fail:throw/return foo => throw/return foo
    5. + *
  5. force:throw/return foo => throw/return foo
    6. + *
+ * + * @param mock mock string + * @return normalized mock string + */ + public static String normalizeMock(String mock) { + if (mock == null) { return mock; - } else if (ConfigUtils.isDefault(mock) || "fail".equalsIgnoreCase(mock.trim()) || "force".equalsIgnoreCase(mock.trim())) { - mock = url.getServiceInterface() + "Mock"; } + + mock = mock.trim(); + + if (mock.length() == 0) { + return mock; + } + + if (Constants.RETURN_KEY.equalsIgnoreCase(mock)) { + return Constants.RETURN_PREFIX + "null"; + } + + if (ConfigUtils.isDefault(mock) || "fail".equalsIgnoreCase(mock) || "force".equalsIgnoreCase(mock)) { + return "default"; + } + if (mock.startsWith(Constants.FAIL_PREFIX)) { mock = mock.substring(Constants.FAIL_PREFIX.length()).trim(); - } else if (mock.startsWith(Constants.FORCE_PREFIX)) { + } + + if (mock.startsWith(Constants.FORCE_PREFIX)) { mock = mock.substring(Constants.FORCE_PREFIX.length()).trim(); } + + if (mock.startsWith(Constants.RETURN_PREFIX) || mock.startsWith(Constants.THROW_PREFIX)) { + mock = mock.replace('`', '"'); + } + return mock; }